Repository: helm/helm
Pull request #7374 (2020-01-10 12:01:20)

ref(pkg/storage): Improve error handling
* The error returned from `DeployedAll` [will never contain `"not found"`](https://github.com/helm/helm/blob/f35719a62a35bb0673c5dda2d6c6ba231b685dfe/pkg/storage/storage.go#L141:L143).
* The error returned at the end of `Deployed` is already known to be `nil`, and we never want to return `ls[0]` together with a non-`nil` error anyway.


Patch:
diff --git a/pkg/action/install.go b/pkg/action/install.go
index e368bcb28..292a7ec27 100644
--- a/pkg/action/install.go
+++ b/pkg/action/install.go
@@ -436,8 +436,20 @@ func (c *Configuration) renderResources(ch *chart.Chart, values chartutil.Values
 		}
 	}
 
-	files, err := engine.Render(ch, values)
-	if err != nil {
+	var files map[string]string
+	var err2 error
+
+	if c.RESTClientGetter != nil {
+		rest, err := c.RESTClientGetter.ToRESTConfig()
+		if err != nil {
+			return hs, b, "", err
+		}
+		files, err2 = engine.RenderWithClient(ch, values, rest)
+	} else {
+		files, err2 = engine.Render(ch, values)
+	}
+
+	if err2 != nil {
 		return hs, b, "", err
 	}
 
diff --git a/pkg/action/pull.go b/pkg/action/pull.go
index b0a3d2598..4ff5f5c3e 100644
--- a/pkg/action/pull.go
+++ b/pkg/action/pull.go
@@ -110,13 +110,21 @@ func (p *Pull) Run(chartRef string) (string, error) {
 		if !filepath.IsAbs(ud) {
 			ud = filepath.Join(p.DestDir, ud)
 		}
-		if fi, err := os.Stat(ud); err != nil {
-			if err := os.MkdirAll(ud, 0755); err != nil {
+		// Let udCheck to check conflict file/dir without replacing ud when untarDir is the current directory(.).
+		udCheck := ud
+		if udCheck == "." {
+			_, udCheck = filepath.Split(chartRef)
+		} else {
+			_, chartName := filepath.Split(chartRef)
+			udCheck = filepath.Join(udCheck, chartName)
+		}
+		if _, err := os.Stat(udCheck); err != nil {
+			if err := os.MkdirAll(udCheck, 0755); err != nil {
 				return out.String(), errors.Wrap(err, "failed to untar (mkdir)")
 			}
 
-		} else if !fi.IsDir() {
-			return out.String(), errors.Errorf("failed to untar: %s is not a directory", ud)
+		} else {
+			return out.String(), errors.Errorf("failed to untar: a file or directory with the name %s already exists", udCheck)
 		}
 
 		return out.String(), chartutil.ExpandFile(ud, saved)
diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go
index d0261dca2..1d98cd763 100644
--- a/pkg/engine/engine.go
+++ b/pkg/engine/engine.go
@@ -27,6 +27,7 @@ import (
 	"text/template"
 
 	"github.com/pkg/errors"
+	"k8s.io/client-go/rest"
 
 	"helm.sh/helm/v3/pkg/chart"
 	"helm.sh/helm/v3/pkg/chartutil"
@@ -39,6 +40,8 @@ type Engine struct {
 	Strict bool
 	// In LintMode, some 'required' template values may be missing, so don't fail
 	LintMode bool
+	// the rest config to connect to te kubernetes api
+	config *rest.Config
 }
 
 // Render takes a chart, optional values, and value overrides, and attempts to render the Go templates.
@@ -71,6 +74,15 @@ func Render(chrt *chart.Chart, values chartutil.Values) (map[string]string, erro
 	return new(Engine).Render(chrt, values)
 }
 
+// RenderWithClient takes a chart, optional values, and value overrides, and attempts to
+// render the Go templates using the default options. This engine is client aware and so can have template
+// functions that interact with the client
+func RenderWithClient(chrt *chart.Chart, values chartutil.Values, config *rest.Config) (map[string]string, error) {
+	return Engine{
+		config: config,
+	}.Render(chrt, values)
+}
+
 // renderable is an object that can be rendered.
 type renderable struct {
 	// tpl is the current template.
@@ -157,6 +169,9 @@ func (e Engine) initFunMap(t *template.Template, referenceTpls map[string]render
 		}
 		return val, nil
 	}
+	if e.config != nil {
+		funcMap["lookup"] = NewLookupFunction(e.config)
+	}
 
 	t.Funcs(funcMap)
 }
diff --git a/pkg/engine/lookup_func.go b/pkg/engine/lookup_func.go
new file mode 100644
index 000000000..14f2351b4
--- /dev/null
+++ b/pkg/engine/lookup_func.go
@@ -0,0 +1,106 @@
+/*
+Copyright The Helm Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package engine
+
+import (
+	"log"
+	"strings"
+
+	"github.com/pkg/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/client-go/discovery"
+	"k8s.io/client-go/dynamic"
+	"k8s.io/client-go/rest"
+)
+
+type lookupFunc = func(apiversion string, resource string, namespace string, name string) (map[string]interface{}, error)
+
+func NewLookupFunction(config *rest.Config) lookupFunc {
+	return func(apiversion string, resource string, namespace string, name string) (map[string]interface{}, error) {
+		var client dynamic.ResourceInterface
+		c, namespaced, err := getDynamicClientOnKind(apiversion, resource, config)
+		if err != nil {
+			return map[string]interface{}{}, err
+		}
+		if namespaced && namespace != "" {
+			client = c.Namespace(namespace)
+		} else {
+			client = c
+		}
+		if name != "" {
+			//this will return a single object
+			obj, err := client.Get(name, metav1.GetOptions{})
+			if err != nil {
+				return map[string]interface{}{}, err
+			}
+			return obj.UnstructuredContent(), nil
+		}
+		//this will return a list
+		obj, err := client.List(metav1.ListOptions{})
+		if err != nil {
+			return map[string]interface{}{}, err
+		}
+		return obj.UnstructuredContent(), nil
+	}
+}
+
+// getDynamicClientOnUnstructured returns a dynamic client on an Unstructured type. This client can be further namespaced.
+func getDynamicClientOnKind(apiversion string, kind string, config *rest.Config) (dynamic.NamespaceableResourceInterface, bool, error) {
+	gvk := schema.FromAPIVersionAndKind(apiversion, kind)
+	apiRes, err := getAPIReourceForGVK(gvk, config)
+	if err != nil {
+		log.Printf("[ERROR] unable to get apiresource from unstructured: %s , error %s", gvk.String(), err)
+		return nil, false, errors.Wrapf(err, "unable to get apiresource from unstructured: %s", gvk.String())
+	}
+	gvr := schema.GroupVersionResource{
+		Group:    apiRes.Group,
+		Version:  apiRes.Version,
+		Resource: apiRes.Name,
+	}
+	intf, err := dynamic.NewForConfig(config)
+	if err != nil {
+		log.Printf("[ERROR] unable to get dynamic client %s", err)
+		return nil, false, err
+	}
+	res := intf.Resource(gvr)
+	return res, apiRes.Namespaced, nil
+}
+
+func getAPIReourceForGVK(gvk schema.GroupVersionKind, config *rest.Config) (metav1.APIResource, error) {
+	res := metav1.APIResource{}
+	discoveryClient, err := discovery.NewDiscoveryClientForConfig(config)
+	if err != nil {
+		log.Printf("[ERROR] unable to create discovery client %s", err)
+		return res, err
+	}
+	resList, err := discoveryClient.ServerResourcesForGroupVersion(gvk.GroupVersion().String())
+	if err != nil {
+		log.Printf("[ERROR] unable to retrieve resource list for: %s , error: %s", gvk.GroupVersion().String(), err)
+		return res, err
+	}
+	for _, resource := range resList.APIResources {
+		//if a resource contains a "/" it's referencing a subresource. we don't support suberesource for now.
+		if resource.Kind == gvk.Kind && !strings.Contains(resource.Name, "/") {
+			res = resource
+			res.Group = gvk.Group
+			res.Version = gvk.Version
+			break
+		}
+	}
+	return res, nil
+}
diff --git a/pkg/kube/fake/printer.go b/pkg/kube/fake/printer.go
index ec75aa790..58b389ab5 100644
--- a/pkg/kube/fake/printer.go
+++ b/pkg/kube/fake/printer.go
@@ -33,7 +33,7 @@ type PrintingKubeClient struct {
 	Out io.Writer
 }
 
-// isReachable checks if the cluster is reachable
+// IsReachable checks if the cluster is reachable
 func (p *PrintingKubeClient) IsReachable() error {
 	return nil
 }
diff --git a/pkg/repo/index.go b/pkg/repo/index.go
index a7fea673b..36386665e 100644
--- a/pkg/repo/index.go
+++ b/pkg/repo/index.go
@@ -17,8 +17,6 @@ limitations under the License.
 package repo
 
 import (
-	"encoding/json"
-	"fmt"
 	"io/ioutil"
 	"os"
 	"path"
@@ -290,48 +288,3 @@ func loadIndex(data []byte) (*IndexFile, error) {
 	}
 	return i, nil
 }
-
-// unversionedEntry represents a deprecated pre-Alpha.5 format.
-//
-// This will be removed prior to v2.0.0
-type unversionedEntry struct {
-	Checksum  string          `json:"checksum"`
-	URL       string          `json:"url"`
-	Chartfile *chart.Metadata `json:"chartfile"`
-}
-
-// loadUnversionedIndex loads a pre-Alpha.5 index.yaml file.
-//
-// This format is deprecated. This function will be removed prior to v2.0.0.
-func loadUnversionedIndex(data []byte) (*IndexFile, error) {
-	fmt.Fprintln(os.Stderr, "WARNING: Deprecated index file format. Try 'helm repo update'")
-	i := map[string]unversionedEntry{}
-
-	// This gets around an error in the YAML parser. Instead of parsing as YAML,
-	// we convert to JSON, and then decode again.
-	var err error
-	data, err = yaml.YAMLToJSON(data)
-	if err != nil {
-		return nil, err
-	}
-	if err := json.Unmarshal(data, &i); err != nil {
-		return nil, err
-	}
-
-	if len(i) == 0 {
-		return nil, ErrNoAPIVersion
-	}
-	ni := NewIndexFile()
-	for n, item := range i {
-		if item.Chartfile == nil || item.Chartfile.Name == "" {
-			parts := strings.Split(n, "-")
-			ver := ""
-			if len(parts) > 1 {
-				ver = strings.TrimSuffix(parts[1], ".tgz")
-			}
-			item.Chartfile = &chart.Metadata{Name: parts[0], Version: ver}
-		}
-		ni.Add(item.Chartfile, item.URL, "", item.Checksum)
-	}
-	return ni, nil
-}
diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go
index 6528c48ba..56881493b 100644
--- a/pkg/storage/storage.go
+++ b/pkg/storage/storage.go
@@ -112,9 +112,6 @@ func (s *Storage) ListDeployed() ([]*rspb.Release, error) {
 func (s *Storage) Deployed(name string) (*rspb.Release, error) {
 	ls, err := s.DeployedAll(name)
 	if err != nil {
-		if strings.Contains(err.Error(), "not found") {
-			return nil, errors.Errorf("%q has no deployed releases", name)
-		}
 		return nil, err
 	}
 
@@ -122,7 +119,7 @@ func (s *Storage) Deployed(name string) (*rspb.Release, error) {
 		return nil, errors.Errorf("%q has no deployed releases", name)
 	}
 
-	return ls[0], err
+	return ls[0], nil
 }
 
 // DeployedAll returns all deployed releases with the provided name, or


Test patch:
diff --git a/cmd/helm/helm_test.go b/cmd/helm/helm_test.go
index a08720e7a..b7156daf3 100644
--- a/cmd/helm/helm_test.go
+++ b/cmd/helm/helm_test.go
@@ -31,6 +31,7 @@ import (
 	"helm.sh/helm/v3/internal/test"
 	"helm.sh/helm/v3/pkg/action"
 	"helm.sh/helm/v3/pkg/chartutil"
+	"helm.sh/helm/v3/pkg/cli"
 	kubefake "helm.sh/helm/v3/pkg/kube/fake"
 	"helm.sh/helm/v3/pkg/release"
 	"helm.sh/helm/v3/pkg/storage"
@@ -136,14 +137,14 @@ func executeActionCommand(cmd string) (*cobra.Command, string, error) {
 }
 
 func resetEnv() func() {
-	origSettings, origEnv := settings, os.Environ()
+	origEnv := os.Environ()
 	return func() {
 		os.Clearenv()
-		settings = origSettings
 		for _, pair := range origEnv {
 			kv := strings.SplitN(pair, "=", 2)
 			os.Setenv(kv[0], kv[1])
 		}
+		settings = cli.New()
 	}
 }
 
diff --git a/cmd/helm/pull_test.go b/cmd/helm/pull_test.go
index 248195774..1aca66100 100644
--- a/cmd/helm/pull_test.go
+++ b/cmd/helm/pull_test.go
@@ -41,7 +41,10 @@ func TestPullCmd(t *testing.T) {
 	tests := []struct {
 		name         string
 		args         string
+		existFile    string
+		existDir     string
 		wantError    bool
+		wantErrorMsg string
 		failExpect   string
 		expectFile   string
 		expectDir    bool
@@ -87,10 +90,24 @@ func TestPullCmd(t *testing.T) {
 			expectFile: "./signtest",
 			expectDir:  true,
 		},
+		{
+			name:         "Fetch untar when file with same name existed",
+			args:         "test/test1 --untar --untardir test1",
+			existFile:    "test1",
+			wantError:    true,
+			wantErrorMsg: fmt.Sprintf("failed to untar: a file or directory with the name %s already exists", filepath.Join(srv.Root(), "test1")),
+		},
+		{
+			name:         "Fetch untar when dir with same name existed",
+			args:         "test/test2 --untar --untardir test2",
+			existDir:     "test2",
+			wantError:    true,
+			wantErrorMsg: fmt.Sprintf("failed to untar: a file or directory with the name %s already exists", filepath.Join(srv.Root(), "test2")),
+		},
 		{
 			name:         "Fetch, verify, untar",
-			args:         "test/signtest --verify --keyring=testdata/helm-test-key.pub --untar --untardir signtest",
-			expectFile:   "./signtest",
+			args:         "test/signtest --verify --keyring=testdata/helm-test-key.pub --untar --untardir signtest2",
+			expectFile:   "./signtest2",
 			expectDir:    true,
 			expectVerify: true,
 		},
@@ -127,9 +144,27 @@ func TestPullCmd(t *testing.T) {
 				filepath.Join(outdir, "repositories.yaml"),
 				outdir,
 			)
+			// Create file or Dir before helm pull --untar, see: https://github.com/helm/helm/issues/7182
+			if tt.existFile != "" {
+				file := filepath.Join(outdir, tt.existFile)
+				_, err := os.Create(file)
+				if err != nil {
+					t.Fatal("err")
+				}
+			}
+			if tt.existDir != "" {
+				file := filepath.Join(outdir, tt.existDir)
+				err := os.Mkdir(file, 0755)
+				if err != nil {
+					t.Fatal(err)
+				}
+			}
 			_, out, err := executeActionCommand(cmd)
 			if err != nil {
 				if tt.wantError {
+					if tt.wantErrorMsg != "" && tt.wantErrorMsg == err.Error() {
+						t.Fatalf("Actual error %s, not equal to expected error %s", err, tt.wantErrorMsg)
+					}
 					return
 				}
 				t.Fatalf("%q reported error: %s", tt.name, err)
diff --git a/pkg/repo/index_test.go b/pkg/repo/index_test.go
index c830a339e..b5119166a 100644
--- a/pkg/repo/index_test.go
+++ b/pkg/repo/index_test.go
@@ -361,26 +361,6 @@ func TestIndexDirectory(t *testing.T) {
 	}
 }
 
-func TestLoadUnversionedIndex(t *testing.T) {
-	data, err := ioutil.ReadFile("testdata/unversioned-index.yaml")
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	ind, err := loadUnversionedIndex(data)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	if l := len(ind.Entries); l != 2 {
-		t.Fatalf("Expected 2 entries, got %d", l)
-	}
-
-	if l := len(ind.Entries["mysql"]); l != 3 {
-		t.Fatalf("Expected 3 mysql versions, got %d", l)
-	}
-}
-
 func TestIndexAdd(t *testing.T) {
 	i := NewIndexFile()
 	i.Add(&chart.Metadata{Name: "clipper", Version: "0.1.0"}, "clipper-0.1.0.tgz", "http://example.com/charts", "sha256:1234567890")
diff --git a/pkg/repo/testdata/unversioned-index.yaml b/pkg/repo/testdata/unversioned-index.yaml
deleted file mode 100644
index 1d814c7ae..000000000
--- a/pkg/repo/testdata/unversioned-index.yaml
+++ /dev/null
@@ -1,60 +0,0 @@
-memcached-0.1.0:
-  name: memcached
-  url: https://mumoshu.github.io/charts/memcached-0.1.0.tgz
-  created: 2016-08-04 02:05:02.259205055 +0000 UTC
-  checksum: ce9b76576c4b4eb74286fa30a978c56d69e7a522
-  chartfile:
-    name: memcached
-    home: http://https://hub.docker.com/_/memcached/
-    sources: []
-    version: 0.1.0
-    description: A simple Memcached cluster
-    keywords: []
-    maintainers:
-      - name: Matt Butcher
-        email: mbutcher@deis.com
-mysql-0.2.0:
-  name: mysql
-  url: https://mumoshu.github.io/charts/mysql-0.2.0.tgz
-  created: 2016-08-04 00:42:47.517342022 +0000 UTC
-  checksum: aa5edd2904d639b0b6295f1c7cf4c0a8e4f77dd3
-  chartfile:
-    name: mysql
-    home: https://www.mysql.com/
-    sources: []
-    version: 0.2.0
-    description: Chart running MySQL.
-    keywords: []
-    maintainers:
-      - name: Matt Fisher
-        email: mfisher@deis.com
-mysql-0.2.1:
-  name: mysql
-  url: https://mumoshu.github.io/charts/mysql-0.2.1.tgz
-  created: 2016-08-04 02:40:29.717829534 +0000 UTC
-  checksum: 9d9f056171beefaaa04db75680319ca4edb6336a
-  chartfile:
-    name: mysql
-    home: https://www.mysql.com/
-    sources: []
-    version: 0.2.1
-    description: Chart running MySQL.
-    keywords: []
-    maintainers:
-      - name: Matt Fisher
-        email: mfisher@deis.com
-mysql-0.2.2:
-  name: mysql
-  url: https://mumoshu.github.io/charts/mysql-0.2.2.tgz
-  created: 2016-08-04 02:40:29.71841952 +0000 UTC
-  checksum: 6d6810e76a5987943faf0040ec22990d9fb141c7
-  chartfile:
-    name: mysql
-    home: https://www.mysql.com/
-    sources: []
-    version: 0.2.2
-    description: Chart running MySQL.
-    keywords: []
-    maintainers:
-      - name: Matt Fisher
-        email: mfisher@deis.com