Merge pull request #5512 from adamreese/v3/linter-fixes

ref(pkg/action): refactoring dup code and linter fixes

pkg/action/action.go

@@ -44,7 +44,7 @@ var (
 	errMissingRelease = errors.New("no release provided")
 	// errInvalidRevision indicates that an invalid release revision number was provided.
 	errInvalidRevision = errors.New("invalid release revision")
-	//errInvalidName indicates that an invalid release name was provided
+	// errInvalidName indicates that an invalid release name was provided
 	errInvalidName = errors.New("invalid release name, must match regex ^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])+$ and the length must not longer than 53")
 )
 
@@ -126,12 +126,8 @@ func GetVersionSet(client discovery.ServerGroupsInterface) (chartutil.VersionSet
 }
 
 // recordRelease with an update operation in case reuse has been set.
-func (c *Configuration) recordRelease(r *release.Release, reuse bool) {
-	if reuse {
-		if err := c.Releases.Update(r); err != nil {
-			c.Log("warning: Failed to update release %s: %s", r.Name, err)
-		}
-	} else if err := c.Releases.Create(r); err != nil {
-		c.Log("warning: Failed to record release %s: %s", r.Name, err)
+func (c *Configuration) recordRelease(r *release.Release) {
+	if err := c.Releases.Update(r); err != nil {
+		c.Log("warning: Failed to update release %s: %s", r.Name, err)
 	}
 }

pkg/action/chart_save.go

@@ -37,7 +37,7 @@ func NewChartSave(cfg *Configuration) *ChartSave {
 }
 
 // Run executes the chart save operation
-func (a *ChartSave) Run(out io.Writer, path string, ref string) error {
+func (a *ChartSave) Run(out io.Writer, path, ref string) error {
 	path, err := filepath.Abs(path)
 	if err != nil {
 		return err

pkg/action/dependency.go

@@ -63,12 +63,13 @@ func (d *Dependency) List(chartpath string, out io.Writer) error {
 
 func (d *Dependency) dependencyStatus(chartpath string, dep *chart.Dependency) string {
 	filename := fmt.Sprintf("%s-%s.tgz", dep.Name, "*")
-	archives, err := filepath.Glob(filepath.Join(chartpath, "charts", filename))
-	if err != nil {
+
+	switch archives, err := filepath.Glob(filepath.Join(chartpath, "charts", filename)); {
+	case err != nil:
 		return "bad pattern"
-	} else if len(archives) > 1 {
+	case len(archives) > 1:
 		return "too many matches"
-	} else if len(archives) == 1 {
+	case len(archives) == 1:
 		archive := archives[0]
 		if _, err := os.Stat(archive); err == nil {
 			c, err := loader.Load(archive)

pkg/action/install.go

@@ -128,7 +128,7 @@ func (i *Install) Run(chrt *chart.Chart) (*release.Release, error) {
 
 	rel := i.createRelease(chrt, i.rawValues)
 	var manifestDoc *bytes.Buffer
-	rel.Hooks, manifestDoc, rel.Info.Notes, err = i.renderResources(chrt, valuesToRender, caps.APIVersions)
+	rel.Hooks, manifestDoc, rel.Info.Notes, err = i.cfg.renderResources(chrt, valuesToRender)
 	// Even for errors, attach this if available
 	if manifestDoc != nil {
 		rel.Manifest = manifestDoc.String()
@@ -172,7 +172,7 @@ func (i *Install) Run(chrt *chart.Chart) (*release.Release, error) {
 	if !i.DisableHooks {
 		if err := i.execHook(rel.Hooks, hooks.PreInstall); err != nil {
 			rel.SetStatus(release.StatusFailed, "failed pre-install: "+err.Error())
-			i.replaceRelease(rel)
+			_ = i.replaceRelease(rel)
 			return rel, err
 		}
 	}
@@ -190,7 +190,7 @@ func (i *Install) Run(chrt *chart.Chart) (*release.Release, error) {
 	if !i.DisableHooks {
 		if err := i.execHook(rel.Hooks, hooks.PostInstall); err != nil {
 			rel.SetStatus(release.StatusFailed, "failed post-install: "+err.Error())
-			i.replaceRelease(rel)
+			_ = i.replaceRelease(rel)
 			return rel, err
 		}
 	}
@@ -291,22 +291,23 @@ func (i *Install) replaceRelease(rel *release.Release) error {
 }
 
 // renderResources renders the templates in a chart
-func (i *Install) renderResources(ch *chart.Chart, values chartutil.Values, vs chartutil.VersionSet) ([]*release.Hook, *bytes.Buffer, string, error) {
-	hooks := []*release.Hook{}
+func (c *Configuration) renderResources(ch *chart.Chart, values chartutil.Values) ([]*release.Hook, *bytes.Buffer, string, error) {
+	hs := []*release.Hook{}
 	b := bytes.NewBuffer(nil)
 
+	caps := c.capabilities()
+
 	if ch.Metadata.KubeVersion != "" {
-		cap, _ := values["Capabilities"].(*chartutil.Capabilities)
-		gitVersion := cap.KubeVersion.String()
+		gitVersion := caps.KubeVersion.String()
 		k8sVersion := strings.Split(gitVersion, "+")[0]
 		if !version.IsCompatibleRange(ch.Metadata.KubeVersion, k8sVersion) {
-			return hooks, b, "", errors.Errorf("chart requires kubernetesVersion: %s which is incompatible with Kubernetes %s", ch.Metadata.KubeVersion, k8sVersion)
+			return hs, b, "", errors.Errorf("chart requires kubernetesVersion: %s which is incompatible with Kubernetes %s", ch.Metadata.KubeVersion, k8sVersion)
 		}
 	}
 
 	files, err := engine.Render(ch, values)
 	if err != nil {
-		return hooks, b, "", err
+		return hs, b, "", err
 	}
 
 	// NOTES.txt gets rendered like all the other files, but because it's not a hook nor a resource,
@@ -329,7 +330,7 @@ func (i *Install) renderResources(ch *chart.Chart, values chartutil.Values, vs c
 	// Sort hooks, manifests, and partials. Only hooks and manifests are returned,
 	// as partials are not used after renderer.Render. Empty manifests are also
 	// removed here.
-	hooks, manifests, err := releaseutil.SortManifests(files, vs, releaseutil.InstallOrder)
+	hs, manifests, err := releaseutil.SortManifests(files, caps.APIVersions, releaseutil.InstallOrder)
 	if err != nil {
 		// By catching parse errors here, we can prevent bogus releases from going
 		// to Kubernetes.
@@ -337,12 +338,12 @@ func (i *Install) renderResources(ch *chart.Chart, values chartutil.Values, vs c
 		// We return the files as a big blob of data to help the user debug parser
 		// errors.
 		for name, content := range files {
-			if len(strings.TrimSpace(content)) == 0 {
+			if strings.TrimSpace(content) == "" {
 				continue
 			}
 			fmt.Fprintf(b, "---\n# Source: %s\n%s\n", name, content)
 		}
-		return hooks, b, "", err
+		return hs, b, "", err
 	}
 
 	// Aggregate all valid manifests into one big doc.
@@ -350,7 +351,7 @@ func (i *Install) renderResources(ch *chart.Chart, values chartutil.Values, vs c
 		fmt.Fprintf(b, "---\n# Source: %s\n%s\n", m.Name, m.Content)
 	}
 
-	return hooks, b, notes, nil
+	return hs, b, notes, nil
 }
 
 // validateManifest checks to see whether the given manifest is valid for the current Kubernetes

pkg/action/package.go

@@ -71,7 +71,7 @@ func (p *Package) Run(path string) (string, error) {
 	ch.Values = combinedVals
 
 	// If version is set, modify the version.
-	if len(p.Version) != 0 {
+	if p.Version != "" {
 		if err := setVersion(ch, p.Version); err != nil {
 			return "", err
 		}

pkg/action/resource_policy.go

@@ -71,7 +71,7 @@ func summarizeKeptManifests(manifests []releaseutil.Manifest, kubeClient kube.Ku
 		if message == "" {
 			message = "These resources were kept due to the resource policy:\n"
 		}
-		message = message + details
+		message += details
 	}
 	return message
 }

pkg/action/rollback.go

@@ -155,8 +155,8 @@ func (r *Rollback) performRollback(currentRelease, targetRelease *release.Releas
 		currentRelease.Info.Status = release.StatusSuperseded
 		targetRelease.Info.Status = release.StatusFailed
 		targetRelease.Info.Description = msg
-		r.cfg.recordRelease(currentRelease, true)
-		r.cfg.recordRelease(targetRelease, true)
+		r.cfg.recordRelease(currentRelease)
+		r.cfg.recordRelease(targetRelease)
 		return targetRelease, err
 	}
 
@@ -175,7 +175,7 @@ func (r *Rollback) performRollback(currentRelease, targetRelease *release.Releas
 	for _, rel := range deployed {
 		r.cfg.Log("superseding previous deployment %d", rel.Version)
 		rel.Info.Status = release.StatusSuperseded
-		r.cfg.recordRelease(rel, true)
+		r.cfg.recordRelease(rel)
 	}
 
 	targetRelease.Info.Status = release.StatusDeployed

pkg/action/show_test.go

@@ -49,15 +49,15 @@ func TestShow(t *testing.T) {
 	}
 
 	expect := []string{
-		strings.Replace(strings.TrimSpace(string(cdata)), "\r", "", -1),
-		strings.Replace(strings.TrimSpace(string(data)), "\r", "", -1),
-		strings.Replace(strings.TrimSpace(string(readmeData)), "\r", "", -1),
+		strings.ReplaceAll(strings.TrimSpace(string(cdata)), "\r", ""),
+		strings.ReplaceAll(strings.TrimSpace(string(data)), "\r", ""),
+		strings.ReplaceAll(strings.TrimSpace(string(readmeData)), "\r", ""),
 	}
 
 	// Problem: ghodss/yaml doesn't marshal into struct order. To solve, we
 	// have to carefully craft the Chart.yaml to match.
 	for i, got := range parts {
-		got = strings.Replace(strings.TrimSpace(got), "\r", "", -1)
+		got = strings.ReplaceAll(strings.TrimSpace(got), "\r", "")
 		if got != expect[i] {
 			t.Errorf("Expected\n%q\nGot\n%q\n", expect[i], got)
 		}

pkg/action/upgrade.go

@@ -19,9 +19,7 @@ package action
 import (
 	"bytes"
 	"fmt"
-	"path"
 	"sort"
-	"strings"
 	"time"
 
 	"github.com/pkg/errors"
@@ -29,12 +27,9 @@ import (
 
 	"helm.sh/helm/pkg/chart"
 	"helm.sh/helm/pkg/chartutil"
-	"helm.sh/helm/pkg/engine"
 	"helm.sh/helm/pkg/hooks"
 	"helm.sh/helm/pkg/kube"
 	"helm.sh/helm/pkg/release"
-	"helm.sh/helm/pkg/releaseutil"
-	"helm.sh/helm/pkg/version"
 )
 
 // Upgrade is the action for upgrading releases.
@@ -164,7 +159,7 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart) (*release.Rele
 		return nil, nil, err
 	}
 
-	hooks, manifestDoc, notesTxt, err := u.renderResources(chart, valuesToRender, caps.APIVersions)
+	hooks, manifestDoc, notesTxt, err := u.cfg.renderResources(chart, valuesToRender)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -213,8 +208,8 @@ func (u *Upgrade) performUpgrade(originalRelease, upgradedRelease *release.Relea
 		u.cfg.Log("warning: %s", msg)
 		upgradedRelease.Info.Status = release.StatusFailed
 		upgradedRelease.Info.Description = msg
-		u.cfg.recordRelease(originalRelease, true)
-		u.cfg.recordRelease(upgradedRelease, true)
+		u.cfg.recordRelease(originalRelease)
+		u.cfg.recordRelease(upgradedRelease)
 		return upgradedRelease, err
 	}
 
@@ -226,7 +221,7 @@ func (u *Upgrade) performUpgrade(originalRelease, upgradedRelease *release.Relea
 	}
 
 	originalRelease.Info.Status = release.StatusSuperseded
-	u.cfg.recordRelease(originalRelease, true)
+	u.cfg.recordRelease(originalRelease)
 
 	upgradedRelease.Info.Status = release.StatusDeployed
 	upgradedRelease.Info.Description = "Upgrade complete"
@@ -297,73 +292,8 @@ func newCapabilities(dc discovery.DiscoveryInterface) (*chartutil.Capabilities,
 	}, nil
 }
 
-func (u *Upgrade) renderResources(ch *chart.Chart, values chartutil.Values, vs chartutil.VersionSet) ([]*release.Hook, *bytes.Buffer, string, error) {
-	if ch.Metadata.KubeVersion != "" {
-		cap, _ := values["Capabilities"].(*chartutil.Capabilities)
-		gitVersion := cap.KubeVersion.String()
-		k8sVersion := strings.Split(gitVersion, "+")[0]
-		if !version.IsCompatibleRange(ch.Metadata.KubeVersion, k8sVersion) {
-			return nil, nil, "", errors.Errorf("chart requires kubernetesVersion: %s which is incompatible with Kubernetes %s", ch.Metadata.KubeVersion, k8sVersion)
-		}
-	}
-
-	u.cfg.Log("rendering %s chart using values", ch.Name())
-	files, err := engine.Render(ch, values)
-	if err != nil {
-		return nil, nil, "", err
-	}
-
-	// NOTES.txt gets rendered like all the other files, but because it's not a hook nor a resource,
-	// pull it out of here into a separate file so that we can actually use the output of the rendered
-	// text file. We have to spin through this map because the file contains path information, so we
-	// look for terminating NOTES.txt. We also remove it from the files so that we don't have to skip
-	// it in the sortHooks.
-	notes := ""
-	for k, v := range files {
-		if strings.HasSuffix(k, notesFileSuffix) {
-			// Only apply the notes if it belongs to the parent chart
-			// Note: Do not use filePath.Join since it creates a path with \ which is not expected
-			if k == path.Join(ch.Name(), "templates", notesFileSuffix) {
-				notes = v
-			}
-			delete(files, k)
-		}
-	}
-
-	// Sort hooks, manifests, and partials. Only hooks and manifests are returned,
-	// as partials are not used after renderer.Render. Empty manifests are also
-	// removed here.
-	hooks, manifests, err := releaseutil.SortManifests(files, vs, releaseutil.InstallOrder)
-	if err != nil {
-		// By catching parse errors here, we can prevent bogus releases from going
-		// to Kubernetes.
-		//
-		// We return the files as a big blob of data to help the user debug parser
-		// errors.
-		b := bytes.NewBuffer(nil)
-		for name, content := range files {
-			if len(strings.TrimSpace(content)) == 0 {
-				continue
-			}
-			b.WriteString("\n---\n# Source: " + name + "\n")
-			b.WriteString(content)
-		}
-		return nil, b, "", err
-	}
-
-	// Aggregate all valid manifests into one big doc.
-	b := bytes.NewBuffer(nil)
-	for _, m := range manifests {
-		b.WriteString("\n---\n# Source: " + m.Name + "\n")
-		b.WriteString(m.Content)
-	}
-
-	return hooks, b, notes, nil
-}
-
 func validateManifest(c kube.KubernetesClient, ns string, manifest []byte) error {
-	r := bytes.NewReader(manifest)
-	_, err := c.BuildUnstructured(ns, r)
+	_, err := c.BuildUnstructured(ns, bytes.NewReader(manifest))
 	return err
 }
 

pkg/chart/loader/archive.go

@@ -90,7 +90,7 @@ func LoadArchive(in io.Reader) (*chart.Chart, error) {
 		n := strings.Join(parts[1:], delimiter)
 
 		// Normalize the path to the / delimiter
-		n = strings.Replace(n, delimiter, "/", -1)
+		n = strings.ReplaceAll(n, delimiter, "/")
 
 		if parts[0] == "Chart.yaml" {
 			return nil, errors.New("chart yaml not in base directory")

pkg/chartutil/create.go

@@ -429,5 +429,5 @@ func Create(chartfile *chart.Metadata, dir string) (string, error) {
 // transform performs a string replacement of the specified source for
 // a given key with the replacement string
 func transform(src, replacement string) []byte {
-	return []byte(strings.Replace(src, "<CHARTNAME>", replacement, -1))
+	return []byte(strings.ReplaceAll(src, "<CHARTNAME>", replacement))
 }

pkg/chartutil/values.go

@@ -315,7 +315,6 @@ type ReleaseOptions struct {
 //
 // This takes both ReleaseOptions and Capabilities to merge into the render values.
 func ToRenderValues(chrt *chart.Chart, chrtVals map[string]interface{}, options ReleaseOptions, caps *Capabilities) (Values, error) {
-
 	top := map[string]interface{}{
 		"Chart":        chrt.Metadata,
 		"Capabilities": caps,

pkg/engine/engine.go

@@ -186,8 +186,8 @@ func (e Engine) renderWithReferences(tpls, referenceTpls map[string]renderable)
 		// is set. Since missing=error will never get here, we do not need to handle
 		// the Strict case.
 		f := &chart.File{
-			Name: strings.Replace(file, "/templates", "/manifests", -1),
-			Data: []byte(strings.Replace(buf.String(), "<no value>", "", -1)),
+			Name: strings.ReplaceAll(file, "/templates", "/manifests"),
+			Data: []byte(strings.ReplaceAll(buf.String(), "<no value>", "")),
 		}
 		rendered[file] = string(f.Data)
 	}

pkg/kube/client.go

@@ -614,7 +614,7 @@ func scrubValidationError(err error) error {
 	const stopValidateMessage = "if you choose to ignore these errors, turn validation off with --validate=false"
 
 	if strings.Contains(err.Error(), stopValidateMessage) {
-		return goerrors.New(strings.Replace(err.Error(), "; "+stopValidateMessage, "", -1))
+		return goerrors.New(strings.ReplaceAll(err.Error(), "; "+stopValidateMessage, ""))
 	}
 	return err
 }

pkg/plugin/cache/cache.go

@@ -65,10 +65,10 @@ func Key(repo string) (string, error) {
 	}
 	key = key + u.Host
 	if u.Path != "" {
-		key = key + strings.Replace(u.Path, "/", "-", -1)
+		key = key + strings.ReplaceAll(u.Path, "/", "-")
 	}
 
-	key = strings.Replace(key, ":", "-", -1)
+	key = strings.ReplaceAll(key, ":", "-")
 
 	return key, nil
 }

pkg/registry/cache.go

@@ -253,12 +253,12 @@ func (cache *filesystemCache) TableRows() ([][]interface{}, error) {
 // escape sanitizes a registry URL to remove characters such as ":"
 // which are illegal on windows
 func escape(s string) string {
-	return strings.Replace(s, ":", "_", -1)
+	return strings.ReplaceAll(s, ":", "_")
 }
 
 // escape reverses escape
 func unescape(s string) string {
-	return strings.Replace(s, "_", ":", -1)
+	return strings.ReplaceAll(s, "_", ":")
 }
 
 // printChartSummary prints details about a chart layers