From 18ed1cf720be9f00bc61c1925d642ad188d6a6d5 Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Fri, 11 Apr 2025 12:20:49 +0200 Subject: [PATCH 01/11] Migrate to last golangci-lint and golangci-lint-action Close dependabot https://github.com/helm/helm/pull/30706 Signed-off-by: Benoit Tigeot --- .github/workflows/golangci-lint.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index 6fbbd2c53..a3f6ba359 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -21,6 +21,6 @@ jobs: go-version: '1.23' check-latest: true - name: golangci-lint - uses: golangci/golangci-lint-action@55c2c1448f86e01eaae002a5a3a9624417608d84 #pin@6.5.2 + uses: golangci/golangci-lint-action@1481404843c368bc19ca9406f87d6e0fc97bdcfd #pin@7.0.0 with: - version: v1.62 + version: v2.0.2 From d8785481680d8fd98419a529f11b3a89f17bbb85 Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Fri, 11 Apr 2025 12:54:12 +0200 Subject: [PATCH 02/11] Migrate golint to v2 ``` WARN The configuration comments are not migrated. WARN Details about the migration: https://golangci-lint.run/product/migration-guide/ WARN The configuration `run.timeout` is ignored. By default, in v2, the timeout is disabled. ``` I've backported comment and timeout Signed-off-by: Benoit Tigeot --- .golangci.yml | 80 +++++++++++++++++++++++++++++++-------------------- 1 file changed, 49 insertions(+), 31 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index ff0dad5f6..f0d45e5ea 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,45 +1,63 @@ +version: "2" run: timeout: 10m - linters: - disable-all: true + default: none enable: - dupl - - gofmt - - goimports - - gosimple - govet - ineffassign - misspell - nakedret - revive - - unused - staticcheck - -linters-settings: - gofmt: - simplify: true - goimports: - local-prefixes: helm.sh/helm/v4 - dupl: - threshold: 400 -issues: - exclude-rules: + - unused + settings: + dupl: + threshold: 400 + exclusions: # Helm, and the Go source code itself, sometimes uses these names outside their built-in # functions. As the Go source code has re-used these names it's ok for Helm to do the same. # Linting will look for redefinition of built-in id's but we opt-in to the ones we choose to use. - - linters: - - revive - text: "redefines-builtin-id: redefinition of the built-in function append" - - linters: - - revive - text: "redefines-builtin-id: redefinition of the built-in function clear" - - linters: - - revive - text: "redefines-builtin-id: redefinition of the built-in function max" - - linters: - - revive - text: "redefines-builtin-id: redefinition of the built-in function min" - - linters: - - revive - text: "redefines-builtin-id: redefinition of the built-in function new" + generated: lax + presets: + - comments + - common-false-positives + - legacy + - std-error-handling + rules: + - linters: + - revive + text: 'redefines-builtin-id: redefinition of the built-in function append' + - linters: + - revive + text: 'redefines-builtin-id: redefinition of the built-in function clear' + - linters: + - revive + text: 'redefines-builtin-id: redefinition of the built-in function max' + - linters: + - revive + text: 'redefines-builtin-id: redefinition of the built-in function min' + - linters: + - revive + text: 'redefines-builtin-id: redefinition of the built-in function new' + paths: + - third_party$ + - builtin$ + - examples$ +formatters: + enable: + - gofmt + - goimports + settings: + gofmt: + simplify: true + goimports: + local-prefixes: + - helm.sh/helm/v4 + exclusions: + generated: lax + paths: + - third_party$ + - builtin$ + - examples$ From 7fe554e7a870bd14bea8da5fe577663156ffcc7f Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Mon, 14 Apr 2025 10:34:10 +0200 Subject: [PATCH 03/11] Fix naked return errors > xinternal/third_party/dep/fs/fs.go:175:3: naked return in func `copyFile` with 59 lines of code (nakedret) Signed-off-by: Benoit Tigeot --- internal/third_party/dep/fs/fs.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/third_party/dep/fs/fs.go b/internal/third_party/dep/fs/fs.go index d29bb5f87..8202ee1d5 100644 --- a/internal/third_party/dep/fs/fs.go +++ b/internal/third_party/dep/fs/fs.go @@ -172,28 +172,28 @@ func copyFile(src, dst string) (err error) { in, err := os.Open(src) if err != nil { - return + return err } defer in.Close() out, err := os.Create(dst) if err != nil { - return + return err } if _, err = io.Copy(out, in); err != nil { out.Close() - return + return err } // Check for write errors on Close if err = out.Close(); err != nil { - return + return err } si, err := os.Stat(src) if err != nil { - return + return err } // Temporary fix for Go < 1.9 @@ -205,7 +205,7 @@ func copyFile(src, dst string) (err error) { } err = os.Chmod(dst, si.Mode()) - return + return err } // cloneSymlink will create a new symlink that points to the resolved path of sl. From a23b96276a8a8fd12089f1333cbdcb03ca44709c Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Mon, 14 Apr 2025 10:34:46 +0200 Subject: [PATCH 04/11] var mu is unused (unused) Signed-off-by: Benoit Tigeot --- internal/third_party/dep/fs/fs_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/internal/third_party/dep/fs/fs_test.go b/internal/third_party/dep/fs/fs_test.go index d42c3f110..909fa4d00 100644 --- a/internal/third_party/dep/fs/fs_test.go +++ b/internal/third_party/dep/fs/fs_test.go @@ -36,14 +36,9 @@ import ( "os/exec" "path/filepath" "runtime" - "sync" "testing" ) -var ( - mu sync.Mutex -) - func TestRenameWithFallback(t *testing.T) { dir := t.TempDir() From b2ac216763761d8d7b5af73a426da87c4264a232 Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Mon, 14 Apr 2025 10:35:32 +0200 Subject: [PATCH 05/11] func cleanUpDir is unused Signed-off-by: Benoit Tigeot --- internal/third_party/dep/fs/fs_test.go | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/internal/third_party/dep/fs/fs_test.go b/internal/third_party/dep/fs/fs_test.go index 909fa4d00..22c59868c 100644 --- a/internal/third_party/dep/fs/fs_test.go +++ b/internal/third_party/dep/fs/fs_test.go @@ -33,7 +33,6 @@ package fs import ( "os" - "os/exec" "path/filepath" "runtime" "testing" @@ -355,19 +354,6 @@ func TestCopyFile(t *testing.T) { } } -func cleanUpDir(dir string) { - // NOTE(mattn): It seems that sometimes git.exe is not dead - // when cleanUpDir() is called. But we do not know any way to wait for it. - if runtime.GOOS == "windows" { - mu.Lock() - exec.Command(`taskkill`, `/F`, `/IM`, `git.exe`).Run() - mu.Unlock() - } - if dir != "" { - os.RemoveAll(dir) - } -} - func TestCopyFileSymlink(t *testing.T) { tempdir := t.TempDir() From a9b77323671ffd93b7d5c423772676e839f4679c Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Mon, 14 Apr 2025 10:39:09 +0200 Subject: [PATCH 06/11] could remove embedded field X from selector Signed-off-by: Benoit Tigeot --- pkg/action/install.go | 6 +++--- pkg/action/show.go | 4 ++-- pkg/action/upgrade.go | 4 ++-- pkg/chart/v2/util/save.go | 4 ++-- pkg/cmd/flags.go | 6 +++--- pkg/cmd/install.go | 4 ++-- pkg/cmd/show.go | 2 +- pkg/cmd/upgrade.go | 4 ++-- pkg/kube/ready.go | 20 ++++++++++---------- pkg/plugin/installer/http_installer.go | 2 +- pkg/repo/index_test.go | 8 ++++---- pkg/storage/driver/cfgmaps.go | 6 +++--- pkg/storage/driver/mock_test.go | 12 ++++++------ pkg/storage/driver/secrets.go | 6 +++--- pkg/storage/storage.go | 10 +++++----- pkg/time/time.go | 2 +- 16 files changed, 50 insertions(+), 50 deletions(-) diff --git a/pkg/action/install.go b/pkg/action/install.go index 25c48c762..d05aae505 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -143,19 +143,19 @@ func NewInstall(cfg *Configuration) *Install { in := &Install{ cfg: cfg, } - in.ChartPathOptions.registryClient = cfg.RegistryClient + in.registryClient = cfg.RegistryClient return in } // SetRegistryClient sets the registry client for the install action func (i *Install) SetRegistryClient(registryClient *registry.Client) { - i.ChartPathOptions.registryClient = registryClient + i.registryClient = registryClient } // GetRegistryClient get the registry client. func (i *Install) GetRegistryClient() *registry.Client { - return i.ChartPathOptions.registryClient + return i.registryClient } func (i *Install) installCRDs(crds []chart.CRD) error { diff --git a/pkg/action/show.go b/pkg/action/show.go index 8f9da58e9..f9843941b 100644 --- a/pkg/action/show.go +++ b/pkg/action/show.go @@ -69,14 +69,14 @@ func NewShow(output ShowOutputFormat, cfg *Configuration) *Show { sh := &Show{ OutputFormat: output, } - sh.ChartPathOptions.registryClient = cfg.RegistryClient + sh.registryClient = cfg.RegistryClient return sh } // SetRegistryClient sets the registry client to use when pulling a chart from a registry. func (s *Show) SetRegistryClient(client *registry.Client) { - s.ChartPathOptions.registryClient = client + s.registryClient = client } // Run executes 'helm show' against the given release. diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index ea09c8ed0..b32bf256e 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -132,14 +132,14 @@ func NewUpgrade(cfg *Configuration) *Upgrade { up := &Upgrade{ cfg: cfg, } - up.ChartPathOptions.registryClient = cfg.RegistryClient + up.registryClient = cfg.RegistryClient return up } // SetRegistryClient sets the registry client to use when fetching charts. func (u *Upgrade) SetRegistryClient(client *registry.Client) { - u.ChartPathOptions.registryClient = client + u.registryClient = client } // Run executes the upgrade on the given release. diff --git a/pkg/chart/v2/util/save.go b/pkg/chart/v2/util/save.go index e1285ac88..dfa10915f 100644 --- a/pkg/chart/v2/util/save.go +++ b/pkg/chart/v2/util/save.go @@ -130,8 +130,8 @@ func Save(c *chart.Chart, outDir string) (string, error) { // Wrap in gzip writer zipper := gzip.NewWriter(f) - zipper.Header.Extra = headerBytes - zipper.Header.Comment = "Helm" + zipper.Extra = headerBytes + zipper.Comment = "Helm" // Wrap in tar writer twriter := tar.NewWriter(zipper) diff --git a/pkg/cmd/flags.go b/pkg/cmd/flags.go index eb829c21e..74c3c8352 100644 --- a/pkg/cmd/flags.go +++ b/pkg/cmd/flags.go @@ -260,7 +260,7 @@ func compVersionFlag(chartRef string, _ string) ([]string, cobra.ShellCompDirect var versions []string if indexFile, err := repo.LoadIndexFile(path); err == nil { for _, details := range indexFile.Entries[chartName] { - appVersion := details.Metadata.AppVersion + appVersion := details.AppVersion appVersionDesc := "" if appVersion != "" { appVersionDesc = fmt.Sprintf("App: %s, ", appVersion) @@ -271,10 +271,10 @@ func compVersionFlag(chartRef string, _ string) ([]string, cobra.ShellCompDirect createdDesc = fmt.Sprintf("Created: %s ", created) } deprecated := "" - if details.Metadata.Deprecated { + if details.Deprecated { deprecated = "(deprecated)" } - versions = append(versions, fmt.Sprintf("%s\t%s%s%s", details.Metadata.Version, appVersionDesc, createdDesc, deprecated)) + versions = append(versions, fmt.Sprintf("%s\t%s%s%s", details.Version, appVersionDesc, createdDesc, deprecated)) } } diff --git a/pkg/cmd/install.go b/pkg/cmd/install.go index ee018c88a..e35df7801 100644 --- a/pkg/cmd/install.go +++ b/pkg/cmd/install.go @@ -242,7 +242,7 @@ func runInstall(args []string, client *action.Install, valueOpts *values.Options } client.ReleaseName = name - cp, err := client.ChartPathOptions.LocateChart(chart, settings) + cp, err := client.LocateChart(chart, settings) if err != nil { return nil, err } @@ -279,7 +279,7 @@ func runInstall(args []string, client *action.Install, valueOpts *values.Options man := &downloader.Manager{ Out: out, ChartPath: cp, - Keyring: client.ChartPathOptions.Keyring, + Keyring: client.Keyring, SkipUpdate: false, Getters: p, RepositoryConfig: settings.RepositoryConfig, diff --git a/pkg/cmd/show.go b/pkg/cmd/show.go index 22d8bee49..1c7e7be44 100644 --- a/pkg/cmd/show.go +++ b/pkg/cmd/show.go @@ -218,7 +218,7 @@ func runShow(args []string, client *action.Show) (string, error) { client.Version = ">0.0.0-0" } - cp, err := client.ChartPathOptions.LocateChart(args[0], settings) + cp, err := client.LocateChart(args[0], settings) if err != nil { return "", err } diff --git a/pkg/cmd/upgrade.go b/pkg/cmd/upgrade.go index 2e0f16212..d1f2ad8e3 100644 --- a/pkg/cmd/upgrade.go +++ b/pkg/cmd/upgrade.go @@ -178,7 +178,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { client.Version = ">0.0.0-0" } - chartPath, err := client.ChartPathOptions.LocateChart(args[1], settings) + chartPath, err := client.LocateChart(args[1], settings) if err != nil { return err } @@ -205,7 +205,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { man := &downloader.Manager{ Out: out, ChartPath: chartPath, - Keyring: client.ChartPathOptions.Keyring, + Keyring: client.Keyring, SkipUpdate: false, Getters: p, RepositoryConfig: settings.RepositoryConfig, diff --git a/pkg/kube/ready.go b/pkg/kube/ready.go index feda44d63..11df4371c 100644 --- a/pkg/kube/ready.go +++ b/pkg/kube/ready.go @@ -288,8 +288,8 @@ func (c *ReadyChecker) deploymentReady(rs *appsv1.ReplicaSet, dep *appsv1.Deploy return false } // Verify the generation observed by the deployment controller matches the spec generation - if dep.Status.ObservedGeneration != dep.ObjectMeta.Generation { - slog.Debug("Deployment is not ready, observedGeneration does not match spec generation", "namespace", dep.GetNamespace(), "name", dep.GetName(), "actualGeneration", dep.Status.ObservedGeneration, "expectedGeneration", dep.ObjectMeta.Generation) + if dep.Status.ObservedGeneration != dep.Generation { + slog.Debug("Deployment is not ready, observedGeneration does not match spec generation", "namespace", dep.GetNamespace(), "name", dep.GetName(), "actualGeneration", dep.Status.ObservedGeneration, "expectedGeneration", dep.Generation) return false } @@ -304,8 +304,8 @@ func (c *ReadyChecker) deploymentReady(rs *appsv1.ReplicaSet, dep *appsv1.Deploy func (c *ReadyChecker) daemonSetReady(ds *appsv1.DaemonSet) bool { // Verify the generation observed by the daemonSet controller matches the spec generation - if ds.Status.ObservedGeneration != ds.ObjectMeta.Generation { - slog.Debug("DaemonSet is not ready, observedGeneration does not match spec generation", "namespace", ds.GetNamespace(), "name", ds.GetName(), "observedGeneration", ds.Status.ObservedGeneration, "expectedGeneration", ds.ObjectMeta.Generation) + if ds.Status.ObservedGeneration != ds.Generation { + slog.Debug("DaemonSet is not ready, observedGeneration does not match spec generation", "namespace", ds.GetNamespace(), "name", ds.GetName(), "observedGeneration", ds.Status.ObservedGeneration, "expectedGeneration", ds.Generation) return false } @@ -381,8 +381,8 @@ func (c *ReadyChecker) crdReady(crd apiextv1.CustomResourceDefinition) bool { func (c *ReadyChecker) statefulSetReady(sts *appsv1.StatefulSet) bool { // Verify the generation observed by the statefulSet controller matches the spec generation - if sts.Status.ObservedGeneration != sts.ObjectMeta.Generation { - slog.Debug("StatefulSet is not ready, observedGeneration doest not match spec generation", "namespace", sts.GetNamespace(), "name", sts.GetName(), "actualGeneration", sts.Status.ObservedGeneration, "expectedGeneration", sts.ObjectMeta.Generation) + if sts.Status.ObservedGeneration != sts.Generation { + slog.Debug("StatefulSet is not ready, observedGeneration doest not match spec generation", "namespace", sts.GetNamespace(), "name", sts.GetName(), "actualGeneration", sts.Status.ObservedGeneration, "expectedGeneration", sts.Generation) return false } @@ -435,8 +435,8 @@ func (c *ReadyChecker) statefulSetReady(sts *appsv1.StatefulSet) bool { func (c *ReadyChecker) replicationControllerReady(rc *corev1.ReplicationController) bool { // Verify the generation observed by the replicationController controller matches the spec generation - if rc.Status.ObservedGeneration != rc.ObjectMeta.Generation { - slog.Debug("ReplicationController is not ready, observedGeneration doest not match spec generation", "namespace", rc.GetNamespace(), "name", rc.GetName(), "actualGeneration", rc.Status.ObservedGeneration, "expectedGeneration", rc.ObjectMeta.Generation) + if rc.Status.ObservedGeneration != rc.Generation { + slog.Debug("ReplicationController is not ready, observedGeneration doest not match spec generation", "namespace", rc.GetNamespace(), "name", rc.GetName(), "actualGeneration", rc.Status.ObservedGeneration, "expectedGeneration", rc.Generation) return false } return true @@ -444,8 +444,8 @@ func (c *ReadyChecker) replicationControllerReady(rc *corev1.ReplicationControll func (c *ReadyChecker) replicaSetReady(rs *appsv1.ReplicaSet) bool { // Verify the generation observed by the replicaSet controller matches the spec generation - if rs.Status.ObservedGeneration != rs.ObjectMeta.Generation { - slog.Debug("ReplicaSet is not ready, observedGeneration doest not match spec generation", "namespace", rs.GetNamespace(), "name", rs.GetName(), "actualGeneration", rs.Status.ObservedGeneration, "expectedGeneration", rs.ObjectMeta.Generation) + if rs.Status.ObservedGeneration != rs.Generation { + slog.Debug("ReplicaSet is not ready, observedGeneration doest not match spec generation", "namespace", rs.GetNamespace(), "name", rs.GetName(), "actualGeneration", rs.Status.ObservedGeneration, "expectedGeneration", rs.Generation) return false } return true diff --git a/pkg/plugin/installer/http_installer.go b/pkg/plugin/installer/http_installer.go index cc45787bf..36bbba651 100644 --- a/pkg/plugin/installer/http_installer.go +++ b/pkg/plugin/installer/http_installer.go @@ -157,7 +157,7 @@ func (i *HTTPInstaller) Update() error { // Path is overridden because we want to join on the plugin name not the file name func (i HTTPInstaller) Path() string { - if i.base.Source == "" { + if i.Source == "" { return "" } return helmpath.DataPath("plugins", i.PluginName) diff --git a/pkg/repo/index_test.go b/pkg/repo/index_test.go index f50c7e65e..2a33cd1a9 100644 --- a/pkg/repo/index_test.go +++ b/pkg/repo/index_test.go @@ -123,17 +123,17 @@ func TestIndexFile(t *testing.T) { } cv, err := i.Get("setter", "0.1.9") - if err == nil && !strings.Contains(cv.Metadata.Version, "0.1.9") { - t.Errorf("Unexpected version: %s", cv.Metadata.Version) + if err == nil && !strings.Contains(cv.Version, "0.1.9") { + t.Errorf("Unexpected version: %s", cv.Version) } cv, err = i.Get("setter", "0.1.9+alpha") - if err != nil || cv.Metadata.Version != "0.1.9+alpha" { + if err != nil || cv.Version != "0.1.9+alpha" { t.Errorf("Expected version: 0.1.9+alpha") } cv, err = i.Get("setter", "0.1.8") - if err != nil || cv.Metadata.Version != "0.1.8" { + if err != nil || cv.Version != "0.1.8" { t.Errorf("Expected version: 0.1.8") } } diff --git a/pkg/storage/driver/cfgmaps.go b/pkg/storage/driver/cfgmaps.go index 3e4acfd81..ca9f0fa28 100644 --- a/pkg/storage/driver/cfgmaps.go +++ b/pkg/storage/driver/cfgmaps.go @@ -78,7 +78,7 @@ func (cfgmaps *ConfigMaps) Get(key string) (*rspb.Release, error) { slog.Debug("failed to decode data", "key", key, slog.Any("error", err)) return nil, err } - r.Labels = filterSystemLabels(obj.ObjectMeta.Labels) + r.Labels = filterSystemLabels(obj.Labels) // return the release object return r, nil } @@ -107,7 +107,7 @@ func (cfgmaps *ConfigMaps) List(filter func(*rspb.Release) bool) ([]*rspb.Releas continue } - rls.Labels = item.ObjectMeta.Labels + rls.Labels = item.Labels if filter(rls) { results = append(results, rls) @@ -146,7 +146,7 @@ func (cfgmaps *ConfigMaps) Query(labels map[string]string) ([]*rspb.Release, err slog.Debug("failed to decode release", slog.Any("error", err)) continue } - rls.Labels = item.ObjectMeta.Labels + rls.Labels = item.Labels results = append(results, rls) } return results, nil diff --git a/pkg/storage/driver/mock_test.go b/pkg/storage/driver/mock_test.go index 54fda0542..1dda258bb 100644 --- a/pkg/storage/driver/mock_test.go +++ b/pkg/storage/driver/mock_test.go @@ -130,7 +130,7 @@ func (mock *MockConfigMapsInterface) List(_ context.Context, opts metav1.ListOpt } for _, cfgmap := range mock.objects { - if labelSelector.Matches(kblabels.Set(cfgmap.ObjectMeta.Labels)) { + if labelSelector.Matches(kblabels.Set(cfgmap.Labels)) { list.Items = append(list.Items, *cfgmap) } } @@ -139,7 +139,7 @@ func (mock *MockConfigMapsInterface) List(_ context.Context, opts metav1.ListOpt // Create creates a new ConfigMap. func (mock *MockConfigMapsInterface) Create(_ context.Context, cfgmap *v1.ConfigMap, _ metav1.CreateOptions) (*v1.ConfigMap, error) { - name := cfgmap.ObjectMeta.Name + name := cfgmap.Name if object, ok := mock.objects[name]; ok { return object, apierrors.NewAlreadyExists(v1.Resource("tests"), name) } @@ -149,7 +149,7 @@ func (mock *MockConfigMapsInterface) Create(_ context.Context, cfgmap *v1.Config // Update updates a ConfigMap. func (mock *MockConfigMapsInterface) Update(_ context.Context, cfgmap *v1.ConfigMap, _ metav1.UpdateOptions) (*v1.ConfigMap, error) { - name := cfgmap.ObjectMeta.Name + name := cfgmap.Name if _, ok := mock.objects[name]; !ok { return nil, apierrors.NewNotFound(v1.Resource("tests"), name) } @@ -216,7 +216,7 @@ func (mock *MockSecretsInterface) List(_ context.Context, opts metav1.ListOption } for _, secret := range mock.objects { - if labelSelector.Matches(kblabels.Set(secret.ObjectMeta.Labels)) { + if labelSelector.Matches(kblabels.Set(secret.Labels)) { list.Items = append(list.Items, *secret) } } @@ -225,7 +225,7 @@ func (mock *MockSecretsInterface) List(_ context.Context, opts metav1.ListOption // Create creates a new Secret. func (mock *MockSecretsInterface) Create(_ context.Context, secret *v1.Secret, _ metav1.CreateOptions) (*v1.Secret, error) { - name := secret.ObjectMeta.Name + name := secret.Name if object, ok := mock.objects[name]; ok { return object, apierrors.NewAlreadyExists(v1.Resource("tests"), name) } @@ -235,7 +235,7 @@ func (mock *MockSecretsInterface) Create(_ context.Context, secret *v1.Secret, _ // Update updates a Secret. func (mock *MockSecretsInterface) Update(_ context.Context, secret *v1.Secret, _ metav1.UpdateOptions) (*v1.Secret, error) { - name := secret.ObjectMeta.Name + name := secret.Name if _, ok := mock.objects[name]; !ok { return nil, apierrors.NewNotFound(v1.Resource("tests"), name) } diff --git a/pkg/storage/driver/secrets.go b/pkg/storage/driver/secrets.go index a69f1ed65..4af38a66e 100644 --- a/pkg/storage/driver/secrets.go +++ b/pkg/storage/driver/secrets.go @@ -72,7 +72,7 @@ func (secrets *Secrets) Get(key string) (*rspb.Release, error) { } // found the secret, decode the base64 data string r, err := decodeRelease(string(obj.Data["release"])) - r.Labels = filterSystemLabels(obj.ObjectMeta.Labels) + r.Labels = filterSystemLabels(obj.Labels) return r, errors.Wrapf(err, "get: failed to decode data %q", key) } @@ -99,7 +99,7 @@ func (secrets *Secrets) List(filter func(*rspb.Release) bool) ([]*rspb.Release, continue } - rls.Labels = item.ObjectMeta.Labels + rls.Labels = item.Labels if filter(rls) { results = append(results, rls) @@ -137,7 +137,7 @@ func (secrets *Secrets) Query(labels map[string]string) ([]*rspb.Release, error) slog.Debug("failed to decode release", "key", item.Name, slog.Any("error", err)) continue } - rls.Labels = item.ObjectMeta.Labels + rls.Labels = item.Labels results = append(results, rls) } return results, nil diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index f98daeba6..927b33c44 100644 --- a/pkg/storage/storage.go +++ b/pkg/storage/storage.go @@ -88,14 +88,14 @@ func (s *Storage) Delete(name string, version int) (*rspb.Release, error) { // storage backend fails to retrieve the releases. func (s *Storage) ListReleases() ([]*rspb.Release, error) { slog.Debug("listing all releases in storage") - return s.Driver.List(func(_ *rspb.Release) bool { return true }) + return s.List(func(_ *rspb.Release) bool { return true }) } // ListUninstalled returns all releases with Status == UNINSTALLED. An error is returned // if the storage backend fails to retrieve the releases. func (s *Storage) ListUninstalled() ([]*rspb.Release, error) { slog.Debug("listing uninstalled releases in storage") - return s.Driver.List(func(rls *rspb.Release) bool { + return s.List(func(rls *rspb.Release) bool { return relutil.StatusFilter(rspb.StatusUninstalled).Check(rls) }) } @@ -104,7 +104,7 @@ func (s *Storage) ListUninstalled() ([]*rspb.Release, error) { // if the storage backend fails to retrieve the releases. func (s *Storage) ListDeployed() ([]*rspb.Release, error) { slog.Debug("listing all deployed releases in storage") - return s.Driver.List(func(rls *rspb.Release) bool { + return s.List(func(rls *rspb.Release) bool { return relutil.StatusFilter(rspb.StatusDeployed).Check(rls) }) } @@ -133,7 +133,7 @@ func (s *Storage) Deployed(name string) (*rspb.Release, error) { func (s *Storage) DeployedAll(name string) ([]*rspb.Release, error) { slog.Debug("getting deployed releases", "name", name) - ls, err := s.Driver.Query(map[string]string{ + ls, err := s.Query(map[string]string{ "name": name, "owner": "helm", "status": "deployed", @@ -152,7 +152,7 @@ func (s *Storage) DeployedAll(name string) ([]*rspb.Release, error) { func (s *Storage) History(name string) ([]*rspb.Release, error) { slog.Debug("getting release history", "name", name) - return s.Driver.Query(map[string]string{"name": name, "owner": "helm"}) + return s.Query(map[string]string{"name": name, "owner": "helm"}) } // removeLeastRecent removes items from history until the length number of releases diff --git a/pkg/time/time.go b/pkg/time/time.go index 13b1211e6..5b3a0ccdc 100644 --- a/pkg/time/time.go +++ b/pkg/time/time.go @@ -41,7 +41,7 @@ func Now() Time { } func (t Time) MarshalJSON() ([]byte, error) { - if t.Time.IsZero() { + if t.IsZero() { return []byte(emptyString), nil } From a6d0335bbb464f324816e2a001b9216d46481e5f Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Mon, 14 Apr 2025 10:49:49 +0200 Subject: [PATCH 07/11] Use fmt.Fprintf(...) instead of ... Signed-off-by: Benoit Tigeot --- pkg/action/install.go | 2 +- pkg/cmd/template.go | 2 +- pkg/registry/utils_test.go | 6 ++---- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/pkg/action/install.go b/pkg/action/install.go index d05aae505..5f159c382 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -628,7 +628,7 @@ func writeToFile(outputDir string, name string, data string, appendData bool) er defer f.Close() - _, err = f.WriteString(fmt.Sprintf("---\n# Source: %s\n%s\n", name, data)) + _, err = fmt.Fprintf(f, "---\n# Source: %s\n%s\n", name, data) if err != nil { return err diff --git a/pkg/cmd/template.go b/pkg/cmd/template.go index 25ff31ade..f96b25e30 100644 --- a/pkg/cmd/template.go +++ b/pkg/cmd/template.go @@ -230,7 +230,7 @@ func writeToFile(outputDir string, name string, data string, appendData bool) er defer f.Close() - _, err = f.WriteString(fmt.Sprintf("---\n# Source: %s\n%s\n", name, data)) + _, err = fmt.Fprintf(f, "---\n# Source: %s\n%s\n", name, data) if err != nil { return err diff --git a/pkg/registry/utils_test.go b/pkg/registry/utils_test.go index 8e6943222..fe07c769a 100644 --- a/pkg/registry/utils_test.go +++ b/pkg/registry/utils_test.go @@ -184,9 +184,7 @@ func initCompromisedRegistryTestServer() string { w.Header().Set("Content-Type", "application/vnd.oci.image.manifest.v1+json") w.WriteHeader(200) - // layers[0] is the blob []byte("a") - w.Write([]byte( - fmt.Sprintf(`{ "schemaVersion": 2, "config": { + fmt.Fprintf(w, `{ "schemaVersion": 2, "config": { "mediaType": "%s", "digest": "sha256:a705ee2789ab50a5ba20930f246dbd5cc01ff9712825bb98f57ee8414377f133", "size": 181 @@ -198,7 +196,7 @@ func initCompromisedRegistryTestServer() string { "size": 1 } ] -}`, ConfigMediaType, ChartLayerMediaType))) +}`, ConfigMediaType, ChartLayerMediaType) } else if r.URL.Path == "/v2/testrepo/supposedlysafechart/blobs/sha256:a705ee2789ab50a5ba20930f246dbd5cc01ff9712825bb98f57ee8414377f133" { w.Header().Set("Content-Type", "application/json") w.WriteHeader(200) From 1654664b782e45e162f51a2abbdd7ca82ea82793 Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Mon, 14 Apr 2025 10:51:21 +0200 Subject: [PATCH 08/11] could use strings.ReplaceAll instead Signed-off-by: Benoit Tigeot --- internal/test/test.go | 2 +- pkg/cmd/docs.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/test/test.go b/internal/test/test.go index e6821282c..e071d3160 100644 --- a/internal/test/test.go +++ b/internal/test/test.go @@ -92,5 +92,5 @@ func update(filename string, in []byte) error { } func normalize(in []byte) []byte { - return bytes.Replace(in, []byte("\r\n"), []byte("\n"), -1) + return bytes.ReplaceAll(in, []byte("\r\n"), []byte("\n")) } diff --git a/pkg/cmd/docs.go b/pkg/cmd/docs.go index b3fd773f9..a22d96c4d 100644 --- a/pkg/cmd/docs.go +++ b/pkg/cmd/docs.go @@ -86,7 +86,7 @@ func (o *docsOptions) run(_ io.Writer) error { hdrFunc := func(filename string) string { base := filepath.Base(filename) name := strings.TrimSuffix(base, path.Ext(base)) - title := cases.Title(language.Und, cases.NoLower).String(strings.Replace(name, "_", " ", -1)) + title := cases.Title(language.Und, cases.NoLower).String(strings.ReplaceAll(name, "_", " ")) return fmt.Sprintf("---\ntitle: \"%s\"\n---\n\n", title) } From 374805deb4f1cd43a3310c6d42117929a724eb3d Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Mon, 14 Apr 2025 10:51:55 +0200 Subject: [PATCH 09/11] error strings should not be capitalized Signed-off-by: Benoit Tigeot --- pkg/cmd/repo_update.go | 2 +- pkg/cmd/repo_update_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/repo_update.go b/pkg/cmd/repo_update.go index 12de2bdaa..a905406cd 100644 --- a/pkg/cmd/repo_update.go +++ b/pkg/cmd/repo_update.go @@ -137,7 +137,7 @@ func updateCharts(repos []*repo.ChartRepository, out io.Writer) error { } if len(repoFailList) > 0 { - return fmt.Errorf("Failed to update the following repositories: %s", + return fmt.Errorf("failed to update the following repositories: %s", repoFailList) } diff --git a/pkg/cmd/repo_update_test.go b/pkg/cmd/repo_update_test.go index aa8f52beb..b0deff1ae 100644 --- a/pkg/cmd/repo_update_test.go +++ b/pkg/cmd/repo_update_test.go @@ -193,7 +193,7 @@ func TestUpdateChartsFailWithError(t *testing.T) { t.Error("Repo update should return error because update of repository fails and 'fail-on-repo-update-fail' flag set") return } - var expectedErr = "Failed to update the following repositories" + var expectedErr = "failed to update the following repositories" var receivedErr = err.Error() if !strings.Contains(receivedErr, expectedErr) { t.Errorf("Expected error (%s) but got (%s) instead", expectedErr, receivedErr) From eb65ce280bdd5c809b2ed2a6d9c44a721a4444f6 Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Mon, 14 Apr 2025 10:53:46 +0200 Subject: [PATCH 10/11] could apply De Morgan's law Signed-off-by: Benoit Tigeot --- pkg/cmd/repo_list.go | 2 +- pkg/kube/ready.go | 4 ++-- pkg/lint/rules/template.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/repo_list.go b/pkg/cmd/repo_list.go index 5b6113a13..71324dc85 100644 --- a/pkg/cmd/repo_list.go +++ b/pkg/cmd/repo_list.go @@ -39,7 +39,7 @@ func newRepoListCmd(out io.Writer) *cobra.Command { ValidArgsFunction: noMoreArgsCompFunc, RunE: func(_ *cobra.Command, _ []string) error { f, _ := repo.LoadFile(settings.RepositoryConfig) - if len(f.Repositories) == 0 && !(outfmt == output.JSON || outfmt == output.YAML) { + if len(f.Repositories) == 0 && outfmt != output.JSON && outfmt != output.YAML { return errors.New("no repositories to show") } diff --git a/pkg/kube/ready.go b/pkg/kube/ready.go index 11df4371c..7a06c72f9 100644 --- a/pkg/kube/ready.go +++ b/pkg/kube/ready.go @@ -294,7 +294,7 @@ func (c *ReadyChecker) deploymentReady(rs *appsv1.ReplicaSet, dep *appsv1.Deploy } expectedReady := *dep.Spec.Replicas - deploymentutil.MaxUnavailable(*dep) - if !(rs.Status.ReadyReplicas >= expectedReady) { + if rs.Status.ReadyReplicas < expectedReady { slog.Debug("Deployment does not have enough pods ready", "namespace", dep.GetNamespace(), "name", dep.GetName(), "readyPods", rs.Status.ReadyReplicas, "totalPods", expectedReady) return false } @@ -328,7 +328,7 @@ func (c *ReadyChecker) daemonSetReady(ds *appsv1.DaemonSet) bool { } expectedReady := int(ds.Status.DesiredNumberScheduled) - maxUnavailable - if !(int(ds.Status.NumberReady) >= expectedReady) { + if int(ds.Status.NumberReady) < expectedReady { slog.Debug("DaemonSet does not have enough Pods ready", "namespace", ds.GetNamespace(), "name", ds.GetName(), "readyPods", ds.Status.NumberReady, "totalPods", expectedReady) return false } diff --git a/pkg/lint/rules/template.go b/pkg/lint/rules/template.go index 4d421f5bf..81a18b411 100644 --- a/pkg/lint/rules/template.go +++ b/pkg/lint/rules/template.go @@ -287,7 +287,7 @@ func validateMatchSelector(yamlStruct *K8sYamlStruct, manifest string) error { switch yamlStruct.Kind { case "Deployment", "ReplicaSet", "DaemonSet", "StatefulSet": // verify that matchLabels or matchExpressions is present - if !(strings.Contains(manifest, "matchLabels") || strings.Contains(manifest, "matchExpressions")) { + if !strings.Contains(manifest, "matchLabels") && !strings.Contains(manifest, "matchExpressions") { return fmt.Errorf("a %s must contain matchLabels or matchExpressions, and %q does not", yamlStruct.Kind, yamlStruct.Metadata.Name) } } From a1c2f47c08b643812fa80464fd49de6d4d927d66 Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Mon, 14 Apr 2025 10:55:42 +0200 Subject: [PATCH 11/11] avoid using reflect.DeepEqual with errors Signed-off-by: Benoit Tigeot --- pkg/storage/driver/cfgmaps_test.go | 5 ++--- pkg/storage/driver/secrets_test.go | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/pkg/storage/driver/cfgmaps_test.go b/pkg/storage/driver/cfgmaps_test.go index 8ba6832fa..a563eb7d9 100644 --- a/pkg/storage/driver/cfgmaps_test.go +++ b/pkg/storage/driver/cfgmaps_test.go @@ -16,6 +16,7 @@ package driver import ( "encoding/base64" "encoding/json" + "errors" "reflect" "testing" @@ -242,10 +243,8 @@ func TestConfigMapDelete(t *testing.T) { if !reflect.DeepEqual(rel, rls) { t.Errorf("Expected {%v}, got {%v}", rel, rls) } - - // fetch the deleted release _, err = cfgmaps.Get(key) - if !reflect.DeepEqual(ErrReleaseNotFound, err) { + if !errors.Is(err, ErrReleaseNotFound) { t.Errorf("Expected {%v}, got {%v}", ErrReleaseNotFound, err) } } diff --git a/pkg/storage/driver/secrets_test.go b/pkg/storage/driver/secrets_test.go index 7affc81ab..9e45bae67 100644 --- a/pkg/storage/driver/secrets_test.go +++ b/pkg/storage/driver/secrets_test.go @@ -16,6 +16,7 @@ package driver import ( "encoding/base64" "encoding/json" + "errors" "reflect" "testing" @@ -242,10 +243,8 @@ func TestSecretDelete(t *testing.T) { if !reflect.DeepEqual(rel, rls) { t.Errorf("Expected {%v}, got {%v}", rel, rls) } - - // fetch the deleted release _, err = secrets.Get(key) - if !reflect.DeepEqual(ErrReleaseNotFound, err) { + if !errors.Is(err, ErrReleaseNotFound) { t.Errorf("Expected {%v}, got {%v}", ErrReleaseNotFound, err) } }