From 9f620b857a30ab377473ae626d68f3b3125ac928 Mon Sep 17 00:00:00 2001 From: Matt Farina Date: Thu, 19 Dec 2024 10:27:38 -0500 Subject: [PATCH 1/2] Update to Go 1.23 Multiple changes were made to pass linting. Some Go built-in names are being used for variables (e.g., min). This happens in the Go source itself including the Go standard library and is not always a bad practice. To handle allowing some built-in names to be used the linter config is updated to allow (via opt-in) some names to pass. This allows us to still check for re-use of Go built-in names and opt-in to any new uses. There were also several cases where a value was checked for nil before checking its length when this is already handled by len() or the types default value. These were cleaned up. The license validation was updated because it was checking everything in the .git directory including all remote content that was local. The previous vendor directory was from a time prior to Go modules when Helm handled dependencies differently. It was no longer needed. Signed-off-by: Matt Farina (cherry picked from commit 5727f56a967418f9254f3604789547b5d7509260) --- .github/workflows/build-test.yml | 2 +- .github/workflows/golangci-lint.yml | 2 +- .github/workflows/govulncheck.yml | 2 +- .github/workflows/release.yml | 4 ++-- .golangci.yml | 20 ++++++++++++++++++++ cmd/helm/plugin_uninstall.go | 2 +- cmd/helm/plugin_update.go | 2 +- cmd/helm/status.go | 2 +- go.mod | 2 +- pkg/action/hooks.go | 2 +- pkg/engine/engine.go | 4 ++-- pkg/kube/client.go | 2 +- pkg/kube/wait.go | 2 +- pkg/registry/util.go | 2 +- scripts/validate-license.sh | 2 +- 15 files changed, 36 insertions(+), 16 deletions(-) diff --git a/.github/workflows/build-test.yml b/.github/workflows/build-test.yml index 79208fb37..e59d186f0 100644 --- a/.github/workflows/build-test.yml +++ b/.github/workflows/build-test.yml @@ -22,7 +22,7 @@ jobs: - name: Setup Go uses: actions/setup-go@41dfa10bad2bb2ae585af6ee5bb4d7d973ad74ed # pin@5.1.0 with: - go-version: '1.22' + go-version: '1.23' check-latest: true - name: Test source headers are present run: make test-source-headers diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index 746f7c306..04edf8d64 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -18,7 +18,7 @@ jobs: - name: Setup Go uses: actions/setup-go@41dfa10bad2bb2ae585af6ee5bb4d7d973ad74ed # pin@5.1.0 with: - go-version: '1.22' + go-version: '1.23' check-latest: true - name: golangci-lint uses: golangci/golangci-lint-action@971e284b6050e8a5849b72094c50ab08da042db8 #pin@6.1.1 diff --git a/.github/workflows/govulncheck.yml b/.github/workflows/govulncheck.yml index ea9d09526..2dda4c06e 100644 --- a/.github/workflows/govulncheck.yml +++ b/.github/workflows/govulncheck.yml @@ -16,7 +16,7 @@ jobs: - name: Setup Go uses: actions/setup-go@41dfa10bad2bb2ae585af6ee5bb4d7d973ad74ed # pin@5.1.0 with: - go-version: '1.22' + go-version: '1.23' check-latest: true - name: govulncheck uses: golang/govulncheck-action@b625fbe08f3bccbe446d94fbf87fcc875a4f50ee # pin@1.0.4 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 9d51acef7..011e38187 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -27,7 +27,7 @@ jobs: - name: Setup Go uses: actions/setup-go@41dfa10bad2bb2ae585af6ee5bb4d7d973ad74ed # pin@5.1.0 with: - go-version: '1.22.7' + go-version: '1.23' - name: Run unit tests run: make test-coverage @@ -83,7 +83,7 @@ jobs: - name: Setup Go uses: actions/setup-go@41dfa10bad2bb2ae585af6ee5bb4d7d973ad74ed # pin@5.1.0 with: - go-version: '1.22' + go-version: '1.23' check-latest: true - name: Run unit tests diff --git a/.golangci.yml b/.golangci.yml index 3cf50a0d4..90b84d77d 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -23,3 +23,23 @@ linters-settings: local-prefixes: helm.sh/helm/v3 dupl: threshold: 400 +issues: + exclude-rules: + # 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" \ No newline at end of file diff --git a/cmd/helm/plugin_uninstall.go b/cmd/helm/plugin_uninstall.go index 607baab2e..458a92cc8 100644 --- a/cmd/helm/plugin_uninstall.go +++ b/cmd/helm/plugin_uninstall.go @@ -78,7 +78,7 @@ func (o *pluginUninstallOptions) run(out io.Writer) error { } } if len(errorPlugins) > 0 { - return errors.Errorf(strings.Join(errorPlugins, "\n")) + return errors.New(strings.Join(errorPlugins, "\n")) } return nil } diff --git a/cmd/helm/plugin_update.go b/cmd/helm/plugin_update.go index 3f6d963fb..eb7505696 100644 --- a/cmd/helm/plugin_update.go +++ b/cmd/helm/plugin_update.go @@ -81,7 +81,7 @@ func (o *pluginUpdateOptions) run(out io.Writer) error { } } if len(errorPlugins) > 0 { - return errors.Errorf(strings.Join(errorPlugins, "\n")) + return errors.New(strings.Join(errorPlugins, "\n")) } return nil } diff --git a/cmd/helm/status.go b/cmd/helm/status.go index 725b3f367..faeedc525 100644 --- a/cmd/helm/status.go +++ b/cmd/helm/status.go @@ -144,7 +144,7 @@ func (s statusPrinter) WriteTable(out io.Writer) error { _, _ = fmt.Fprintf(out, "DESCRIPTION: %s\n", s.release.Info.Description) } - if s.showResources && s.release.Info.Resources != nil && len(s.release.Info.Resources) > 0 { + if s.showResources && len(s.release.Info.Resources) > 0 { buf := new(bytes.Buffer) printFlags := get.NewHumanPrintFlags() typePrinter, _ := printFlags.ToPrinter("") diff --git a/go.mod b/go.mod index df0018e9a..4f5254ce9 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module helm.sh/helm/v3 -go 1.22.0 +go 1.23.0 require ( github.com/AdaLogics/go-fuzz-headers v0.0.0-20230811130428-ced1acdcaa24 diff --git a/pkg/action/hooks.go b/pkg/action/hooks.go index 4bffb6ae0..e1cbab5e1 100644 --- a/pkg/action/hooks.go +++ b/pkg/action/hooks.go @@ -44,7 +44,7 @@ func (cfg *Configuration) execHook(rl *release.Release, hook release.HookEvent, for _, h := range executingHooks { // Set default delete policy to before-hook-creation - if h.DeletePolicies == nil || len(h.DeletePolicies) == 0 { + if len(h.DeletePolicies) == 0 { // TODO(jlegrone): Only apply before-hook-creation delete policy to run to completion // resources. For all other resource types update in place if a // resource with the same name already exists and is owned by the diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index df3a600a3..d8ee313e1 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -206,7 +206,7 @@ func (e Engine) initFunMap(t *template.Template) { log.Printf("[INFO] Missing required value: %s", warn) return "", nil } - return val, errors.Errorf(warnWrap(warn)) + return val, errors.New(warnWrap(warn)) } else if _, ok := val.(string); ok { if val == "" { if e.LintMode { @@ -214,7 +214,7 @@ func (e Engine) initFunMap(t *template.Template) { log.Printf("[INFO] Missing required value: %s", warn) return "", nil } - return val, errors.Errorf(warnWrap(warn)) + return val, errors.New(warnWrap(warn)) } } return val, nil diff --git a/pkg/kube/client.go b/pkg/kube/client.go index 4d93c91b9..5f16f2bfb 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -435,7 +435,7 @@ func (c *Client) Update(original, target ResourceList, force bool) (*Result, err case err != nil: return res, err case len(updateErrors) != 0: - return res, errors.Errorf(strings.Join(updateErrors, " && ")) + return res, errors.New(strings.Join(updateErrors, " && ")) } for _, info := range original.Difference(target) { diff --git a/pkg/kube/wait.go b/pkg/kube/wait.go index 36110d0de..bdafc8255 100644 --- a/pkg/kube/wait.go +++ b/pkg/kube/wait.go @@ -153,7 +153,7 @@ func SelectorsForObject(object runtime.Object) (selector labels.Selector, err er case *batchv1.Job: selector, err = metav1.LabelSelectorAsSelector(t.Spec.Selector) case *corev1.Service: - if t.Spec.Selector == nil || len(t.Spec.Selector) == 0 { + if len(t.Spec.Selector) == 0 { return nil, fmt.Errorf("invalid service '%s': Service is defined without a selector", t.Name) } selector = labels.SelectorFromSet(t.Spec.Selector) diff --git a/pkg/registry/util.go b/pkg/registry/util.go index 727cdae03..4454105c9 100644 --- a/pkg/registry/util.go +++ b/pkg/registry/util.go @@ -208,7 +208,7 @@ func generateChartOCIAnnotations(meta *chart.Metadata, creationTime string) map[ chartOCIAnnotations = addToMap(chartOCIAnnotations, ocispec.AnnotationSource, meta.Sources[0]) } - if meta.Maintainers != nil && len(meta.Maintainers) > 0 { + if len(meta.Maintainers) > 0 { var maintainerSb strings.Builder for maintainerIdx, maintainer := range meta.Maintainers { diff --git a/scripts/validate-license.sh b/scripts/validate-license.sh index dc247436f..f67812ca5 100755 --- a/scripts/validate-license.sh +++ b/scripts/validate-license.sh @@ -19,7 +19,7 @@ IFS=$'\n\t' find_files() { find . -not \( \ \( \ - -wholename './vendor' \ + -wholename './.git' \ -o -wholename '*testdata*' \ -o -wholename '*third_party*' \ \) -prune \ From 037c18af3521c78cff41cfb84db01fb5847a98fb Mon Sep 17 00:00:00 2001 From: Matt Farina Date: Thu, 19 Dec 2024 13:45:12 -0500 Subject: [PATCH 2/2] Update golangci-lint version Signed-off-by: Matt Farina (cherry picked from commit 66f84e510e12edbc02ad0e102ca17ad3edfd53e3) --- .github/workflows/golangci-lint.yml | 2 +- .golangci.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index 04edf8d64..c1c8aeb22 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -23,4 +23,4 @@ jobs: - name: golangci-lint uses: golangci/golangci-lint-action@971e284b6050e8a5849b72094c50ab08da042db8 #pin@6.1.1 with: - version: v1.58 + version: v1.62 diff --git a/.golangci.yml b/.golangci.yml index 90b84d77d..a3e5119d4 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -42,4 +42,4 @@ issues: text: "redefines-builtin-id: redefinition of the built-in function min" - linters: - revive - text: "redefines-builtin-id: redefinition of the built-in function new" \ No newline at end of file + text: "redefines-builtin-id: redefinition of the built-in function new"