diff --git a/.github/ISSUE_TEMPLATE/bug-report.yaml b/.github/ISSUE_TEMPLATE/bug-report.yaml new file mode 100644 index 000000000..4309d800b --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug-report.yaml @@ -0,0 +1,69 @@ +name: Bug Report +description: Report a bug encountered in Helm +labels: kind/bug +body: + - type: textarea + id: problem + attributes: + label: What happened? + description: | + Please provide as much info as possible. Not doing so may result in your bug not being addressed in a timely manner. + validations: + required: true + + - type: textarea + id: expected + attributes: + label: What did you expect to happen? + validations: + required: true + + - type: textarea + id: repro + attributes: + label: How can we reproduce it (as minimally and precisely as possible)? + description: | + Please list steps someone can follow to trigger the issue. + + For example: + 1. Run `helm install mychart ./path-to-chart -f values.yaml --debug` + 2. Observe the following error: ... + + You can include: + - a sample `values.yaml` block + - a link to a chart + - specific `helm` commands used + + This helps others reproduce and debug your issue more effectively. + validations: + required: true + + - type: textarea + id: helmVersion + attributes: + label: Helm version + value: | +
+ ```console + $ helm version + # paste output here + ``` +
+ validations: + required: true + + - type: textarea + id: kubeVersion + attributes: + label: Kubernetes version + value: | +
+ + ```console + $ kubectl version + # paste output here + ``` + +
+ validations: + required: true diff --git a/.github/ISSUE_TEMPLATE/documentation.yaml b/.github/ISSUE_TEMPLATE/documentation.yaml new file mode 100644 index 000000000..bb1b7537c --- /dev/null +++ b/.github/ISSUE_TEMPLATE/documentation.yaml @@ -0,0 +1,27 @@ +name: Documentation +description: Report any mistakes or missing information from the documentation or the examples +labels: kind/documentation +body: + - type: markdown + attributes: + value: | + ⚠️ **Note**: Most documentation lives in [helm/helm-www](https://github.com/helm/helm-www). + If your issue is about Helm website documentation or examples, please [open an issue there](https://github.com/helm/helm-www/issues/new/choose). + + - type: textarea + id: feature + attributes: + label: What would you like to be added? + description: | + Link to the issue (please include a link to the specific documentation or example). + Link to the issue raised in [Helm Documentation Improvement Proposal](https://github.com/helm/helm-www) + validations: + required: true + + - type: textarea + id: rationale + attributes: + label: Why is this needed? + validations: + required: true + diff --git a/.github/ISSUE_TEMPLATE/feature.yaml b/.github/ISSUE_TEMPLATE/feature.yaml new file mode 100644 index 000000000..45b9c3f94 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/feature.yaml @@ -0,0 +1,21 @@ +name: Enhancement/feature +description: Provide supporting details for a feature in development +labels: kind/feature +body: + - type: textarea + id: feature + attributes: + label: What would you like to be added? + description: | + Feature requests are unlikely to make progress as issues. + Initial discussion and ideas can happen on an issue. + But significant changes or features must be proposed as a [Helm Improvement Proposal](https://github.com/helm/community/blob/main/hips/hip-0001.md) (HIP) + validations: + required: true + + - type: textarea + id: rationale + attributes: + label: Why is this needed? + validations: + required: true diff --git a/.github/env b/.github/env index b321f6ef7..4384ba074 100644 --- a/.github/env +++ b/.github/env @@ -1,2 +1,2 @@ GOLANG_VERSION=1.24 -GOLANGCI_LINT_VERSION=v2.0.2 +GOLANGCI_LINT_VERSION=v2.1.0 diff --git a/.github/issue_template.md b/.github/issue_template.md deleted file mode 100644 index 48f48e5b6..000000000 --- a/.github/issue_template.md +++ /dev/null @@ -1,9 +0,0 @@ - - -Output of `helm version`: - -Output of `kubectl version`: - -Cloud Provider/Platform (AKS, GKE, Minikube etc.): - - diff --git a/.github/workflows/build-test.yml b/.github/workflows/build-test.yml index 6ed7092dc..11a5c49ec 100644 --- a/.github/workflows/build-test.yml +++ b/.github/workflows/build-test.yml @@ -22,7 +22,7 @@ jobs: - name: Add variables to environment file run: cat ".github/env" >> "$GITHUB_ENV" - name: Setup Go - uses: actions/setup-go@0aaccfd150d50ccaeb58ebd88d36e91967a5f35b # pin@5.4.0 + uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # pin@5.5.0 with: go-version: '${{ env.GOLANG_VERSION }}' check-latest: true diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index 7ecbcb95d..3059b05a2 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -17,11 +17,11 @@ jobs: - name: Add variables to environment file run: cat ".github/env" >> "$GITHUB_ENV" - name: Setup Go - uses: actions/setup-go@0aaccfd150d50ccaeb58ebd88d36e91967a5f35b # pin@5.4.0 + uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # pin@5.5.0 with: go-version: '${{ env.GOLANG_VERSION }}' check-latest: true - name: golangci-lint - uses: golangci/golangci-lint-action@1481404843c368bc19ca9406f87d6e0fc97bdcfd #pin@7.0.0 + uses: golangci/golangci-lint-action@4afd733a84b1f43292c63897423277bb7f4313a9 #pin@8.0.0 with: version: ${{ env.GOLANGCI_LINT_VERSION }} diff --git a/.github/workflows/govulncheck.yml b/.github/workflows/govulncheck.yml index 6befb7954..67cfa4c36 100644 --- a/.github/workflows/govulncheck.yml +++ b/.github/workflows/govulncheck.yml @@ -18,7 +18,7 @@ jobs: - name: Add variables to environment file run: cat ".github/env" >> "$GITHUB_ENV" - name: Setup Go - uses: actions/setup-go@0aaccfd150d50ccaeb58ebd88d36e91967a5f35b # pin@5.4.0 + uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # pin@5.5.0 with: go-version: '${{ env.GOLANG_VERSION }}' check-latest: true diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 38d13a175..96138caf1 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -28,7 +28,7 @@ jobs: run: cat ".github/env" >> "$GITHUB_ENV" - name: Setup Go - uses: actions/setup-go@0aaccfd150d50ccaeb58ebd88d36e91967a5f35b # pin@5.4.0 + uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # pin@5.5.0 with: go-version: '${{ env.GOLANG_VERSION }}' - name: Run unit tests @@ -85,7 +85,7 @@ jobs: run: cat ".github/env" >> "$GITHUB_ENV" - name: Setup Go - uses: actions/setup-go@0aaccfd150d50ccaeb58ebd88d36e91967a5f35b # pin@5.4.0 + uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # pin@5.5.0 with: go-version: '${{ env.GOLANG_VERSION }}' check-latest: true diff --git a/.github/workflows/scorecards.yml b/.github/workflows/scorecards.yml index a8c2e8a15..4b135bb2a 100644 --- a/.github/workflows/scorecards.yml +++ b/.github/workflows/scorecards.yml @@ -33,7 +33,7 @@ jobs: persist-credentials: false - name: "Run analysis" - uses: ossf/scorecard-action@f49aabe0b5af0936a0987cfb85d86b75731b0186 # v2.4.1 + uses: ossf/scorecard-action@05b42c624433fc40578a4040d5cf5e36ddca8cde # v2.4.2 with: results_file: results.sarif results_format: sarif diff --git a/.golangci.yml b/.golangci.yml index 4599bb88d..a9b13c35f 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -20,13 +20,17 @@ linters: enable: - depguard - dupl + - gomodguard - govet - ineffassign - misspell - nakedret - revive - staticcheck + - thelper - unused + - usestdlibvars + - usetesting exclusions: generated: lax @@ -54,6 +58,13 @@ linters: dupl: threshold: 400 + gomodguard: + blocked: + modules: + - github.com/evanphx/json-patch: + recommendations: + - github.com/evanphx/json-patch/v5 + run: timeout: 10m diff --git a/README.md b/README.md index 5f4d71d4c..ef994e742 100644 --- a/README.md +++ b/README.md @@ -5,6 +5,7 @@ [![GoDoc](https://img.shields.io/static/v1?label=godoc&message=reference&color=blue)](https://pkg.go.dev/helm.sh/helm/v4) [![CII Best Practices](https://bestpractices.coreinfrastructure.org/projects/3131/badge)](https://bestpractices.coreinfrastructure.org/projects/3131) [![OpenSSF Scorecard](https://api.scorecard.dev/projects/github.com/helm/helm/badge)](https://scorecard.dev/viewer/?uri=github.com/helm/helm) +[![LFX Health Score](https://img.shields.io/static/v1?label=Health%20Score&message=Healthy&color=A7F3D0&logo=linuxfoundation&logoColor=white&style=flat)](https://insights.linuxfoundation.org/project/helm) Helm is a tool for managing Charts. Charts are packages of pre-configured Kubernetes resources. @@ -56,7 +57,7 @@ including installing pre-releases. ## Docs -Get started with the [Quick Start guide](https://helm.sh/docs/intro/quickstart/) or plunge into the [complete documentation](https://helm.sh/docs) +Get started with the [Quick Start guide](https://helm.sh/docs/intro/quickstart/) or plunge into the [complete documentation](https://helm.sh/docs). ## Roadmap diff --git a/cmd/helm/helm.go b/cmd/helm/helm.go index eefce5158..0e912cda4 100644 --- a/cmd/helm/helm.go +++ b/cmd/helm/helm.go @@ -41,7 +41,6 @@ func main() { } if err := cmd.Execute(); err != nil { - slog.Debug("error", slog.Any("error", err)) switch e := err.(type) { case helmcmd.PluginError: os.Exit(e.Code) diff --git a/go.mod b/go.mod index e6e2bfa97..799a521bf 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 github.com/cyphar/filepath-securejoin v0.4.1 github.com/distribution/distribution/v3 v3.0.0 - github.com/evanphx/json-patch v5.9.11+incompatible + github.com/evanphx/json-patch/v5 v5.9.11 github.com/fluxcd/cli-utils v0.36.0-flux.13 github.com/foxcpp/go-mockdns v1.1.0 github.com/gobwas/glob v0.2.3 @@ -27,25 +27,25 @@ require ( github.com/opencontainers/image-spec v1.1.1 github.com/phayes/freeport v0.0.0-20220201140144-74d24b5ae9f5 github.com/rubenv/sql-migrate v1.8.0 - github.com/santhosh-tekuri/jsonschema/v6 v6.0.1 + github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 github.com/spf13/cobra v1.9.1 github.com/spf13/pflag v1.0.6 github.com/stretchr/testify v1.10.0 - golang.org/x/crypto v0.37.0 - golang.org/x/term v0.31.0 - golang.org/x/text v0.24.0 + golang.org/x/crypto v0.39.0 + golang.org/x/term v0.32.0 + golang.org/x/text v0.26.0 gopkg.in/yaml.v3 v3.0.1 - k8s.io/api v0.33.0 - k8s.io/apiextensions-apiserver v0.33.0 - k8s.io/apimachinery v0.33.0 - k8s.io/apiserver v0.33.0 - k8s.io/cli-runtime v0.33.0 - k8s.io/client-go v0.33.0 + k8s.io/api v0.33.2 + k8s.io/apiextensions-apiserver v0.33.2 + k8s.io/apimachinery v0.33.2 + k8s.io/apiserver v0.33.2 + k8s.io/cli-runtime v0.33.2 + k8s.io/client-go v0.33.2 k8s.io/klog/v2 v2.130.1 - k8s.io/kubectl v0.33.0 - oras.land/oras-go/v2 v2.5.0 - sigs.k8s.io/controller-runtime v0.20.4 - sigs.k8s.io/yaml v1.4.0 + k8s.io/kubectl v0.33.2 + oras.land/oras-go/v2 v2.6.0 + sigs.k8s.io/controller-runtime v0.21.0 + sigs.k8s.io/yaml v1.5.0 ) require ( @@ -68,7 +68,6 @@ require ( github.com/docker/go-events v0.0.0-20190806004212-e31b211e4f1c // indirect github.com/docker/go-metrics v0.0.1 // indirect github.com/emicklei/go-restful/v3 v3.12.1 // indirect - github.com/evanphx/json-patch/v5 v5.9.11 // indirect github.com/exponent-io/jsonpath v0.0.0-20210407135951-1de76d718b3f // indirect github.com/fatih/color v1.13.0 // indirect github.com/felixge/httpsnoop v1.0.4 // indirect @@ -155,13 +154,15 @@ require ( go.opentelemetry.io/otel/sdk/metric v1.32.0 // indirect go.opentelemetry.io/otel/trace v1.34.0 // indirect go.opentelemetry.io/proto/otlp v1.4.0 // indirect - golang.org/x/mod v0.24.0 // indirect - golang.org/x/net v0.39.0 // indirect + go.yaml.in/yaml/v2 v2.4.2 // indirect + go.yaml.in/yaml/v3 v3.0.3 // indirect + golang.org/x/mod v0.25.0 // indirect + golang.org/x/net v0.40.0 // indirect golang.org/x/oauth2 v0.29.0 // indirect - golang.org/x/sync v0.13.0 // indirect - golang.org/x/sys v0.32.0 // indirect + golang.org/x/sync v0.15.0 // indirect + golang.org/x/sys v0.33.0 // indirect golang.org/x/time v0.11.0 // indirect - golang.org/x/tools v0.32.0 // indirect + golang.org/x/tools v0.33.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20241209162323-e6fa225c2576 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20241209162323-e6fa225c2576 // indirect google.golang.org/grpc v1.68.1 // indirect @@ -169,7 +170,7 @@ require ( gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect - k8s.io/component-base v0.33.0 // indirect + k8s.io/component-base v0.33.2 // indirect k8s.io/kube-openapi v0.0.0-20250318190949-c8a335a9a2ff // indirect k8s.io/utils v0.0.0-20250321185631-1f6e0b77f77e // indirect sigs.k8s.io/json v0.0.0-20241014173422-cfa47c3a1cc8 // indirect diff --git a/go.sum b/go.sum index c4327a97a..77591443e 100644 --- a/go.sum +++ b/go.sum @@ -77,8 +77,6 @@ github.com/docker/go-metrics v0.0.1 h1:AgB/0SvBxihN0X8OR4SjsblXkbMvalQ8cjmtKQ2rQ github.com/docker/go-metrics v0.0.1/go.mod h1:cG1hvH2utMXtqgqqYE9plW6lDxS3/5ayHzueweSI3Vw= github.com/emicklei/go-restful/v3 v3.12.1 h1:PJMDIM/ak7btuL8Ex0iYET9hxM3CI2sjZtzpL63nKAU= github.com/emicklei/go-restful/v3 v3.12.1/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc= -github.com/evanphx/json-patch v5.9.11+incompatible h1:ixHHqfcGvxhWkniF1tWxBHA0yb4Z+d1UQi45df52xW8= -github.com/evanphx/json-patch v5.9.11+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk= github.com/evanphx/json-patch/v5 v5.9.11 h1:/8HVnzMq13/3x9TPvjG08wUGqBTmZBsCWzjTM0wiaDU= github.com/evanphx/json-patch/v5 v5.9.11/go.mod h1:3j+LviiESTElxA4p3EMKAB9HXj3/XEtnUf6OZxqIQTM= github.com/exponent-io/jsonpath v0.0.0-20210407135951-1de76d718b3f h1:Wl78ApPPB2Wvf/TIe2xdyJxTlb6obmF18d8QdkxNDu4= @@ -292,8 +290,8 @@ github.com/rubenv/sql-migrate v1.8.0 h1:dXnYiJk9k3wetp7GfQbKJcPHjVJL6YK19tKj8t2N github.com/rubenv/sql-migrate v1.8.0/go.mod h1:F2bGFBwCU+pnmbtNYDeKvSuvL6lBVtXDXUUv5t+u1qw= github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= -github.com/santhosh-tekuri/jsonschema/v6 v6.0.1 h1:PKK9DyHxif4LZo+uQSgXNqs0jj5+xZwwfKHgph2lxBw= -github.com/santhosh-tekuri/jsonschema/v6 v6.0.1/go.mod h1:JXeL+ps8p7/KNMjDQk3TCwPpBy0wYklyWTfbkIzdIFU= +github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 h1:KRzFb2m7YtdldCEkzs6KqmJw4nqEVZGK7IN2kJkjTuQ= +github.com/santhosh-tekuri/jsonschema/v6 v6.0.2/go.mod h1:JXeL+ps8p7/KNMjDQk3TCwPpBy0wYklyWTfbkIzdIFU= github.com/sergi/go-diff v1.2.0 h1:XU+rvMAioB0UC3q1MFrIQy4Vo5/4VsRDQQXHsEya6xQ= github.com/sergi/go-diff v1.2.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM= github.com/shopspring/decimal v1.4.0 h1:bxl37RwXBklmTi0C79JfXCEBD1cqqHt0bbgBAGFp81k= @@ -378,6 +376,10 @@ go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0= go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= go.uber.org/zap v1.27.0 h1:aJMhYGrd5QSmlpLMr2MftRKl7t8J8PTZPA732ud/XR8= go.uber.org/zap v1.27.0/go.mod h1:GB2qFLM7cTU87MWRP2mPIjqfIDnGu+VIO4V/SdhGo2E= +go.yaml.in/yaml/v2 v2.4.2 h1:DzmwEr2rDGHl7lsFgAHxmNz/1NlQ7xLIrlN2h5d1eGI= +go.yaml.in/yaml/v2 v2.4.2/go.mod h1:081UH+NErpNdqlCXm3TtEran0rJZGxAYx9hb/ELlsPU= +go.yaml.in/yaml/v3 v3.0.3 h1:bXOww4E/J3f66rav3pX3m8w6jDE4knZjGOw8b5Y6iNE= +go.yaml.in/yaml/v3 v3.0.3/go.mod h1:tBHosrYAkRZjRAOREWbDnBXUf08JOwYq++0QNwQiWzI= golang.org/x/crypto v0.0.0-20180904163835-0709b304e793/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= @@ -386,16 +388,16 @@ golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5y golang.org/x/crypto v0.13.0/go.mod h1:y6Z2r+Rw4iayiXXAIxJIDAJ1zMW4yaTpebo8fPOliYc= golang.org/x/crypto v0.14.0/go.mod h1:MVFd36DqK4CsrnJYDkBA3VC4m2GkXAM0PvzMCn4JQf4= golang.org/x/crypto v0.15.0/go.mod h1:4ChreQoLWfG3xLDer1WdlH5NdlQ3+mwnQq1YTKY+72g= -golang.org/x/crypto v0.37.0 h1:kJNSjF/Xp7kU0iB2Z+9viTPMW4EqqsrywMXLJOOsXSE= -golang.org/x/crypto v0.37.0/go.mod h1:vg+k43peMZ0pUMhYmVAWysMK35e6ioLh3wB8ZCAfbVc= +golang.org/x/crypto v0.39.0 h1:SHs+kF4LP+f+p14esP5jAoDpHU8Gu/v9lFRK6IT5imM= +golang.org/x/crypto v0.39.0/go.mod h1:L+Xg3Wf6HoL4Bn4238Z6ft6KfEpN0tJGo53AAPC632U= golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/mod v0.12.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/mod v0.14.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= -golang.org/x/mod v0.24.0 h1:ZfthKaKaT4NrhGVZHO1/WDTwGES4De8KtWO0SIbNJMU= -golang.org/x/mod v0.24.0/go.mod h1:IXM97Txy2VM4PJ3gI61r1YEk/gAj6zAHN3AdZt6S9Ww= +golang.org/x/mod v0.25.0 h1:n7a+ZbQKQA/Ysbyb0/6IbB1H/X41mKgbhfv7AfG/44w= +golang.org/x/mod v0.25.0/go.mod h1:IXM97Txy2VM4PJ3gI61r1YEk/gAj6zAHN3AdZt6S9Ww= golang.org/x/net v0.0.0-20181114220301-adae6a3d119a/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190613194153-d28f0bde5980/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= @@ -409,8 +411,8 @@ golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg= golang.org/x/net v0.15.0/go.mod h1:idbUs1IY1+zTqbi8yxTbhexhEEk5ur9LInksu6HrEpk= golang.org/x/net v0.17.0/go.mod h1:NxSsAGuq816PNPmqtQdLE42eU2Fs7NoRIZrHJAlaCOE= golang.org/x/net v0.18.0/go.mod h1:/czyP5RqHAH4odGYxBJ1qz0+CE5WZ+2j1YgoEo8F2jQ= -golang.org/x/net v0.39.0 h1:ZCu7HMWDxpXpaiKdhzIfaltL9Lp31x/3fCP11bc6/fY= -golang.org/x/net v0.39.0/go.mod h1:X7NRbYVEA+ewNkCNyJ513WmMdQ3BineSwVtN2zD/d+E= +golang.org/x/net v0.40.0 h1:79Xs7wF06Gbdcg4kdCCIQArK11Z1hr5POQ6+fIYHNuY= +golang.org/x/net v0.40.0/go.mod h1:y0hY0exeL2Pku80/zKK7tpntoX23cqL3Oa6njdgRtds= golang.org/x/oauth2 v0.29.0 h1:WdYw2tdTK1S8olAzWHdgeqfy+Mtm9XNhv/xJsY65d98= golang.org/x/oauth2 v0.29.0/go.mod h1:onh5ek6nERTohokkhCD/y2cV4Do3fxFHFuAejCkRWT8= golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -423,8 +425,8 @@ golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.3.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y= golang.org/x/sync v0.4.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y= golang.org/x/sync v0.5.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= -golang.org/x/sync v0.13.0 h1:AauUjRAJ9OSnvULf/ARrrVywoJDy0YS2AwQ98I37610= -golang.org/x/sync v0.13.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= +golang.org/x/sync v0.15.0 h1:KWH3jNZsfyT6xfAfKiz6MRNmd46ByHDYaZ7KSkCtdW8= +golang.org/x/sync v0.15.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20181116152217-5ac8a444bdc5/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -446,8 +448,8 @@ golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.14.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/sys v0.32.0 h1:s77OFDvIQeibCmezSnk/q6iAfkdiQaJi4VzroCFrN20= -golang.org/x/sys v0.32.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= +golang.org/x/sys v0.33.0 h1:q3i8TbbEz+JRD9ywIRlyRAQbM0qF7hu24q3teo2hbuw= +golang.org/x/sys v0.33.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= @@ -455,8 +457,8 @@ golang.org/x/term v0.8.0/go.mod h1:xPskH00ivmX89bAKVGSKKtLOWNx2+17Eiy94tnKShWo= golang.org/x/term v0.12.0/go.mod h1:owVbMEjm3cBLCHdkQu9b1opXd4ETQWc3BhuQGKgXgvU= golang.org/x/term v0.13.0/go.mod h1:LTmsnFJwVN6bCy1rVCoS+qHT1HhALEFxKncY3WNNh4U= golang.org/x/term v0.14.0/go.mod h1:TySc+nGkYR6qt8km8wUhuFRTVSMIX3XPR58y2lC8vww= -golang.org/x/term v0.31.0 h1:erwDkOK1Msy6offm1mOgvspSkslFnIGsFnxOKoufg3o= -golang.org/x/term v0.31.0/go.mod h1:R4BeIy7D95HzImkxGkTW1UQTtP54tio2RyHz7PwK0aw= +golang.org/x/term v0.32.0 h1:DR4lr0TjUs3epypdhTOkMmuF5CDFJ/8pOnbzMZPQ7bg= +golang.org/x/term v0.32.0/go.mod h1:uZG1FhGx848Sqfsq4/DlJr3xGGsYMu/L5GW4abiaEPQ= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= @@ -464,8 +466,8 @@ golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= golang.org/x/text v0.13.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE= golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= -golang.org/x/text v0.24.0 h1:dd5Bzh4yt5KYA8f9CJHCP4FB4D51c2c6JvN37xJJkJ0= -golang.org/x/text v0.24.0/go.mod h1:L8rBsPeo2pSS+xqN0d5u2ikmjtmoJbDBT1b7nHvFCdU= +golang.org/x/text v0.26.0 h1:P42AVeLghgTYr4+xUnTRKDMqpar+PtX7KWuNQL21L8M= +golang.org/x/text v0.26.0/go.mod h1:QK15LZJUUQVJxhz7wXgxSy/CJaTFjd0G+YLonydOVQA= golang.org/x/time v0.11.0 h1:/bpjEDfN9tkoN/ryeYHnv5hcMlc8ncjMcM4XBk5NWV0= golang.org/x/time v0.11.0/go.mod h1:CDIdPxbZBQxdj6cxyCIdrNogrJKMJ7pr37NYpMcMDSg= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= @@ -476,8 +478,8 @@ golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= golang.org/x/tools v0.13.0/go.mod h1:HvlwmtVNQAhOuCjW7xxvovg8wbNq7LwfXh/k7wXUl58= golang.org/x/tools v0.15.0/go.mod h1:hpksKq4dtpQWS1uQ61JkdqWM3LscIS6Slf+VVkm+wQk= -golang.org/x/tools v0.32.0 h1:Q7N1vhpkQv7ybVzLFtTjvQya2ewbwNDZzUgfXGqtMWU= -golang.org/x/tools v0.32.0/go.mod h1:ZxrU41P/wAbZD8EDa6dDCa6XfpkhJ7HFMjHJXfBDu8s= +golang.org/x/tools v0.33.0 h1:4qz2S3zmRxbGIhDIAgjxvFutSvH5EfnsYrRBj0UI0bc= +golang.org/x/tools v0.33.0/go.mod h1:CIJMaWEY88juyUfo7UbgPqbC8rU2OqfAV1h2Qp0oMYI= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= @@ -504,32 +506,32 @@ gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -k8s.io/api v0.33.0 h1:yTgZVn1XEe6opVpP1FylmNrIFWuDqe2H0V8CT5gxfIU= -k8s.io/api v0.33.0/go.mod h1:CTO61ECK/KU7haa3qq8sarQ0biLq2ju405IZAd9zsiM= -k8s.io/apiextensions-apiserver v0.33.0 h1:d2qpYL7Mngbsc1taA4IjJPRJ9ilnsXIrndH+r9IimOs= -k8s.io/apiextensions-apiserver v0.33.0/go.mod h1:VeJ8u9dEEN+tbETo+lFkwaaZPg6uFKLGj5vyNEwwSzc= -k8s.io/apimachinery v0.33.0 h1:1a6kHrJxb2hs4t8EE5wuR/WxKDwGN1FKH3JvDtA0CIQ= -k8s.io/apimachinery v0.33.0/go.mod h1:BHW0YOu7n22fFv/JkYOEfkUYNRN0fj0BlvMFWA7b+SM= -k8s.io/apiserver v0.33.0 h1:QqcM6c+qEEjkOODHppFXRiw/cE2zP85704YrQ9YaBbc= -k8s.io/apiserver v0.33.0/go.mod h1:EixYOit0YTxt8zrO2kBU7ixAtxFce9gKGq367nFmqI8= -k8s.io/cli-runtime v0.33.0 h1:Lbl/pq/1o8BaIuyn+aVLdEPHVN665tBAXUePs8wjX7c= -k8s.io/cli-runtime v0.33.0/go.mod h1:QcA+r43HeUM9jXFJx7A+yiTPfCooau/iCcP1wQh4NFw= -k8s.io/client-go v0.33.0 h1:UASR0sAYVUzs2kYuKn/ZakZlcs2bEHaizrrHUZg0G98= -k8s.io/client-go v0.33.0/go.mod h1:kGkd+l/gNGg8GYWAPr0xF1rRKvVWvzh9vmZAMXtaKOg= -k8s.io/component-base v0.33.0 h1:Ot4PyJI+0JAD9covDhwLp9UNkUja209OzsJ4FzScBNk= -k8s.io/component-base v0.33.0/go.mod h1:aXYZLbw3kihdkOPMDhWbjGCO6sg+luw554KP51t8qCU= +k8s.io/api v0.33.2 h1:YgwIS5jKfA+BZg//OQhkJNIfie/kmRsO0BmNaVSimvY= +k8s.io/api v0.33.2/go.mod h1:fhrbphQJSM2cXzCWgqU29xLDuks4mu7ti9vveEnpSXs= +k8s.io/apiextensions-apiserver v0.33.2 h1:6gnkIbngnaUflR3XwE1mCefN3YS8yTD631JXQhsU6M8= +k8s.io/apiextensions-apiserver v0.33.2/go.mod h1:IvVanieYsEHJImTKXGP6XCOjTwv2LUMos0YWc9O+QP8= +k8s.io/apimachinery v0.33.2 h1:IHFVhqg59mb8PJWTLi8m1mAoepkUNYmptHsV+Z1m5jY= +k8s.io/apimachinery v0.33.2/go.mod h1:BHW0YOu7n22fFv/JkYOEfkUYNRN0fj0BlvMFWA7b+SM= +k8s.io/apiserver v0.33.2 h1:KGTRbxn2wJagJowo29kKBp4TchpO1DRO3g+dB/KOJN4= +k8s.io/apiserver v0.33.2/go.mod h1:9qday04wEAMLPWWo9AwqCZSiIn3OYSZacDyu/AcoM/M= +k8s.io/cli-runtime v0.33.2 h1:koNYQKSDdq5AExa/RDudXMhhtFasEg48KLS2KSAU74Y= +k8s.io/cli-runtime v0.33.2/go.mod h1:gnhsAWpovqf1Zj5YRRBBU7PFsRc6NkEkwYNQE+mXL88= +k8s.io/client-go v0.33.2 h1:z8CIcc0P581x/J1ZYf4CNzRKxRvQAwoAolYPbtQes+E= +k8s.io/client-go v0.33.2/go.mod h1:9mCgT4wROvL948w6f6ArJNb7yQd7QsvqavDeZHvNmHo= +k8s.io/component-base v0.33.2 h1:sCCsn9s/dG3ZrQTX/Us0/Sx2R0G5kwa0wbZFYoVp/+0= +k8s.io/component-base v0.33.2/go.mod h1:/41uw9wKzuelhN+u+/C59ixxf4tYQKW7p32ddkYNe2k= k8s.io/klog/v2 v2.130.1 h1:n9Xl7H1Xvksem4KFG4PYbdQCQxqc/tTUyrgXaOhHSzk= k8s.io/klog/v2 v2.130.1/go.mod h1:3Jpz1GvMt720eyJH1ckRHK1EDfpxISzJ7I9OYgaDtPE= k8s.io/kube-openapi v0.0.0-20250318190949-c8a335a9a2ff h1:/usPimJzUKKu+m+TE36gUyGcf03XZEP0ZIKgKj35LS4= k8s.io/kube-openapi v0.0.0-20250318190949-c8a335a9a2ff/go.mod h1:5jIi+8yX4RIb8wk3XwBo5Pq2ccx4FP10ohkbSKCZoK8= -k8s.io/kubectl v0.33.0 h1:HiRb1yqibBSCqic4pRZP+viiOBAnIdwYDpzUFejs07g= -k8s.io/kubectl v0.33.0/go.mod h1:gAlGBuS1Jq1fYZ9AjGWbI/5Vk3M/VW2DK4g10Fpyn/0= +k8s.io/kubectl v0.33.2 h1:7XKZ6DYCklu5MZQzJe+CkCjoGZwD1wWl7t/FxzhMz7Y= +k8s.io/kubectl v0.33.2/go.mod h1:8rC67FB8tVTYraovAGNi/idWIK90z2CHFNMmGJZJ3KI= k8s.io/utils v0.0.0-20250321185631-1f6e0b77f77e h1:KqK5c/ghOm8xkHYhlodbp6i6+r+ChV2vuAuVRdFbLro= k8s.io/utils v0.0.0-20250321185631-1f6e0b77f77e/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= -oras.land/oras-go/v2 v2.5.0 h1:o8Me9kLY74Vp5uw07QXPiitjsw7qNXi8Twd+19Zf02c= -oras.land/oras-go/v2 v2.5.0/go.mod h1:z4eisnLP530vwIOUOJeBIj0aGI0L1C3d53atvCBqZHg= -sigs.k8s.io/controller-runtime v0.20.4 h1:X3c+Odnxz+iPTRobG4tp092+CvBU9UK0t/bRf+n0DGU= -sigs.k8s.io/controller-runtime v0.20.4/go.mod h1:xg2XB0K5ShQzAgsoujxuKN4LNXR2LfwwHsPj7Iaw+XY= +oras.land/oras-go/v2 v2.6.0 h1:X4ELRsiGkrbeox69+9tzTu492FMUu7zJQW6eJU+I2oc= +oras.land/oras-go/v2 v2.6.0/go.mod h1:magiQDfG6H1O9APp+rOsvCPcW1GD2MM7vgnKY0Y+u1o= +sigs.k8s.io/controller-runtime v0.21.0 h1:CYfjpEuicjUecRk+KAeyYh+ouUBn4llGyDYytIGcJS8= +sigs.k8s.io/controller-runtime v0.21.0/go.mod h1:OSg14+F65eWqIu4DceX7k/+QRAbTTvxeQSNSOQpukWM= sigs.k8s.io/json v0.0.0-20241014173422-cfa47c3a1cc8 h1:gBQPwqORJ8d8/YNZWEjoZs7npUVDpVXUUOFfW6CgAqE= sigs.k8s.io/json v0.0.0-20241014173422-cfa47c3a1cc8/go.mod h1:mdzfpAEoE6DHQEN0uh9ZbOCuHbLK5wOm7dK4ctXE9Tg= sigs.k8s.io/kustomize/api v0.19.0 h1:F+2HB2mU1MSiR9Hp1NEgoU2q9ItNOaBJl0I4Dlus5SQ= @@ -541,5 +543,6 @@ sigs.k8s.io/randfill v1.0.0 h1:JfjMILfT8A6RbawdsK2JXGBR5AQVfd+9TbzrlneTyrU= sigs.k8s.io/randfill v1.0.0/go.mod h1:XeLlZ/jmk4i1HRopwe7/aU3H5n1zNUcX6TM94b3QxOY= sigs.k8s.io/structured-merge-diff/v4 v4.6.0 h1:IUA9nvMmnKWcj5jl84xn+T5MnlZKThmUW1TdblaLVAc= sigs.k8s.io/structured-merge-diff/v4 v4.6.0/go.mod h1:dDy58f92j70zLsuZVuUX5Wp9vtxXpaZnkPGWeqDfCps= -sigs.k8s.io/yaml v1.4.0 h1:Mk1wCc2gy/F0THH0TAp1QYyJNzRm2KCLy3o5ASXVI5E= sigs.k8s.io/yaml v1.4.0/go.mod h1:Ejl7/uTz7PSA4eKMyQCUTnhZYNmLIl+5c2lQPGR2BPY= +sigs.k8s.io/yaml v1.5.0 h1:M10b2U7aEUY6hRtU870n2VTPgR5RZiL/I6Lcc2F4NUQ= +sigs.k8s.io/yaml v1.5.0/go.mod h1:wZs27Rbxoai4C0f8/9urLZtZtF3avA3gKvGyPdDqTO4= diff --git a/internal/monocular/search.go b/internal/monocular/search.go index 6912be2ce..fcf04b7a4 100644 --- a/internal/monocular/search.go +++ b/internal/monocular/search.go @@ -129,7 +129,7 @@ func (c *Client) Search(term string) ([]SearchResult, error) { } defer res.Body.Close() - if res.StatusCode != 200 { + if res.StatusCode != http.StatusOK { return nil, fmt.Errorf("failed to fetch %s : %s", p.String(), res.Status) } diff --git a/internal/sympath/walk_test.go b/internal/sympath/walk_test.go index d4e2ceeaa..1eba8b996 100644 --- a/internal/sympath/walk_test.go +++ b/internal/sympath/walk_test.go @@ -76,6 +76,7 @@ func walkTree(n *Node, path string, f func(path string, n *Node)) { } func makeTree(t *testing.T) { + t.Helper() walkTree(tree, tree.name, func(path string, n *Node) { if n.entries == nil { if n.symLinkedTo != "" { @@ -99,6 +100,7 @@ func makeTree(t *testing.T) { } func checkMarks(t *testing.T, report bool) { + t.Helper() walkTree(tree, tree.name, func(path string, n *Node) { if n.marks != n.expectedMarks && report { t.Errorf("node %s mark = %d; expected %d", path, n.marks, n.expectedMarks) diff --git a/internal/test/ensure/ensure.go b/internal/test/ensure/ensure.go index 0d8dd9abc..a72f48c2d 100644 --- a/internal/test/ensure/ensure.go +++ b/internal/test/ensure/ensure.go @@ -29,12 +29,12 @@ import ( func HelmHome(t *testing.T) { t.Helper() base := t.TempDir() - os.Setenv(xdg.CacheHomeEnvVar, base) - os.Setenv(xdg.ConfigHomeEnvVar, base) - os.Setenv(xdg.DataHomeEnvVar, base) - os.Setenv(helmpath.CacheHomeEnvVar, "") - os.Setenv(helmpath.ConfigHomeEnvVar, "") - os.Setenv(helmpath.DataHomeEnvVar, "") + t.Setenv(xdg.CacheHomeEnvVar, base) + t.Setenv(xdg.ConfigHomeEnvVar, base) + t.Setenv(xdg.DataHomeEnvVar, base) + t.Setenv(helmpath.CacheHomeEnvVar, "") + t.Setenv(helmpath.ConfigHomeEnvVar, "") + t.Setenv(helmpath.DataHomeEnvVar, "") } // TempFile ensures a temp file for unit testing purposes. @@ -46,9 +46,10 @@ func HelmHome(t *testing.T) { // tempdir := TempFile(t, "foo", []byte("bar")) // filename := filepath.Join(tempdir, "foo") func TempFile(t *testing.T, name string, data []byte) string { + t.Helper() path := t.TempDir() filename := filepath.Join(path, name) - if err := os.WriteFile(filename, data, 0755); err != nil { + if err := os.WriteFile(filename, data, 0o755); err != nil { t.Fatal(err) } return path diff --git a/internal/third_party/dep/fs/fs_test.go b/internal/third_party/dep/fs/fs_test.go index 22c59868c..4c59d17fe 100644 --- a/internal/third_party/dep/fs/fs_test.go +++ b/internal/third_party/dep/fs/fs_test.go @@ -457,6 +457,7 @@ func TestCopyFileFail(t *testing.T) { // files this function creates. It is the caller's responsibility to call // this function before the test is done running, whether there's an error or not. func setupInaccessibleDir(t *testing.T, op func(dir string) error) func() { + t.Helper() dir := t.TempDir() subdir := filepath.Join(dir, "dir") diff --git a/internal/tlsutil/tls_test.go b/internal/tlsutil/tls_test.go index eb1cc183e..3d7e75c86 100644 --- a/internal/tlsutil/tls_test.go +++ b/internal/tlsutil/tls_test.go @@ -30,8 +30,9 @@ const ( ) func testfile(t *testing.T, file string) (path string) { - var err error - if path, err = filepath.Abs(filepath.Join(tlsTestDir, file)); err != nil { + t.Helper() + path, err := filepath.Abs(filepath.Join(tlsTestDir, file)) + if err != nil { t.Fatalf("error getting absolute path to test file %q: %v", file, err) } return path diff --git a/pkg/action/action.go b/pkg/action/action.go index 6905f3f44..40194dfd7 100644 --- a/pkg/action/action.go +++ b/pkg/action/action.go @@ -26,6 +26,7 @@ import ( "path" "path/filepath" "strings" + "sync" "text/template" "k8s.io/apimachinery/pkg/api/meta" @@ -86,6 +87,8 @@ type Configuration struct { // HookOutputFunc called with container name and returns and expects writer that will receive the log output. HookOutputFunc func(namespace, pod, container string) io.Writer + + mutex sync.Mutex } // renderResources renders the templates in a chart diff --git a/pkg/action/action_test.go b/pkg/action/action_test.go index f808163fb..9436abef5 100644 --- a/pkg/action/action_test.go +++ b/pkg/action/action_test.go @@ -40,6 +40,7 @@ import ( var verbose = flag.Bool("test.log", false, "enable test logging (debug by default)") func actionConfigFixture(t *testing.T) *Configuration { + t.Helper() return actionConfigFixtureWithDummyResources(t, nil) } diff --git a/pkg/action/hooks.go b/pkg/action/hooks.go index 8db0d51f8..1213e87e2 100644 --- a/pkg/action/hooks.go +++ b/pkg/action/hooks.go @@ -49,13 +49,7 @@ func (cfg *Configuration) execHook(rl *release.Release, hook release.HookEvent, for i, h := range executingHooks { // Set default delete policy to before-hook-creation - 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 - // current release. - h.DeletePolicies = []release.HookDeletePolicy{release.HookBeforeHookCreation} - } + cfg.hookSetDeletePolicy(h) if err := cfg.deleteHookByPolicy(h, release.HookBeforeHookCreation, waitStrategy, timeout); err != nil { return err @@ -154,7 +148,7 @@ func (cfg *Configuration) deleteHookByPolicy(h *release.Hook, policy release.Hoo if h.Kind == "CustomResourceDefinition" { return nil } - if hookHasDeletePolicy(h, policy) { + if cfg.hookHasDeletePolicy(h, policy) { resources, err := cfg.KubeClient.Build(bytes.NewBufferString(h.Manifest), false) if err != nil { return fmt.Errorf("unable to build kubernetes object for deleting hook %s: %w", h.Path, err) @@ -188,13 +182,24 @@ func (cfg *Configuration) deleteHooksByPolicy(hooks []*release.Hook, policy rele // hookHasDeletePolicy determines whether the defined hook deletion policy matches the hook deletion polices // supported by helm. If so, mark the hook as one should be deleted. -func hookHasDeletePolicy(h *release.Hook, policy release.HookDeletePolicy) bool { - for _, v := range h.DeletePolicies { - if policy == v { - return true - } +func (cfg *Configuration) hookHasDeletePolicy(h *release.Hook, policy release.HookDeletePolicy) bool { + cfg.mutex.Lock() + defer cfg.mutex.Unlock() + return slices.Contains(h.DeletePolicies, policy) +} + +// hookSetDeletePolicy determines whether the defined hook deletion policy matches the hook deletion polices +// supported by helm. If so, mark the hook as one should be deleted. +func (cfg *Configuration) hookSetDeletePolicy(h *release.Hook) { + cfg.mutex.Lock() + defer cfg.mutex.Unlock() + 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 + // current release. + h.DeletePolicies = []release.HookDeletePolicy{release.HookBeforeHookCreation} } - return false } // outputLogsByPolicy outputs a pods logs if the hook policy instructs it to diff --git a/pkg/action/hooks_test.go b/pkg/action/hooks_test.go index 9ca42ec6a..ad1de2c59 100644 --- a/pkg/action/hooks_test.go +++ b/pkg/action/hooks_test.go @@ -167,6 +167,7 @@ func TestInstallRelease_HooksOutputLogsOnSuccessAndFailure(t *testing.T) { } func runInstallForHooksWithSuccess(t *testing.T, manifest, expectedNamespace string, shouldOutput bool) { + t.Helper() var expectedOutput string if shouldOutput { expectedOutput = fmt.Sprintf("attempted to output logs for namespace: %s", expectedNamespace) @@ -190,6 +191,7 @@ func runInstallForHooksWithSuccess(t *testing.T, manifest, expectedNamespace str } func runInstallForHooksWithFailure(t *testing.T, manifest, expectedNamespace string, shouldOutput bool) { + t.Helper() var expectedOutput string if shouldOutput { expectedOutput = fmt.Sprintf("attempted to output logs for namespace: %s", expectedNamespace) diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index e39674c80..6c2c91d0a 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -116,6 +116,7 @@ func installActionWithConfig(config *Configuration) *Install { } func installAction(t *testing.T) *Install { + t.Helper() config := actionConfigFixture(t) instAction := NewInstall(config) instAction.Namespace = "spaced" @@ -130,7 +131,7 @@ func TestInstallRelease(t *testing.T) { instAction := installAction(t) vals := map[string]interface{}{} - ctx, done := context.WithCancel(context.Background()) + ctx, done := context.WithCancel(t.Context()) res, err := instAction.RunWithContext(ctx, buildChart(), vals) if err != nil { t.Fatalf("Failed install: %s", err) @@ -446,7 +447,9 @@ func TestInstallReleaseIncorrectTemplate_DryRun(t *testing.T) { instAction.DryRun = true vals := map[string]interface{}{} _, err := instAction.Run(buildChart(withSampleIncludingIncorrectTemplates()), vals) - expectedErr := "\"hello/templates/incorrect\" at <.Values.bad.doh>: nil pointer evaluating interface {}.doh" + expectedErr := `hello/templates/incorrect:1:10 + executing "hello/templates/incorrect" at <.Values.bad.doh>: + nil pointer evaluating interface {}.doh` if err == nil { t.Fatalf("Install should fail containing error: %s", expectedErr) } @@ -556,7 +559,7 @@ func TestInstallRelease_Wait_Interrupted(t *testing.T) { instAction.WaitStrategy = kube.StatusWatcherStrategy vals := map[string]interface{}{} - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithCancel(t.Context()) time.AfterFunc(time.Second, cancel) goroutines := runtime.NumGoroutine() @@ -640,7 +643,7 @@ func TestInstallRelease_Atomic_Interrupted(t *testing.T) { instAction.Atomic = true vals := map[string]interface{}{} - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithCancel(t.Context()) time.AfterFunc(time.Second, cancel) goroutines := runtime.NumGoroutine() diff --git a/pkg/action/list_test.go b/pkg/action/list_test.go index e41949310..b6f89fa1e 100644 --- a/pkg/action/list_test.go +++ b/pkg/action/list_test.go @@ -64,13 +64,14 @@ func TestList_Empty(t *testing.T) { } func newListFixture(t *testing.T) *List { + t.Helper() return NewList(actionConfigFixture(t)) } func TestList_OneNamespace(t *testing.T) { is := assert.New(t) lister := newListFixture(t) - makeMeSomeReleases(lister.cfg.Releases, t) + makeMeSomeReleases(t, lister.cfg.Releases) list, err := lister.Run() is.NoError(err) is.Len(list, 3) @@ -79,7 +80,7 @@ func TestList_OneNamespace(t *testing.T) { func TestList_AllNamespaces(t *testing.T) { is := assert.New(t) lister := newListFixture(t) - makeMeSomeReleases(lister.cfg.Releases, t) + makeMeSomeReleases(t, lister.cfg.Releases) lister.AllNamespaces = true lister.SetStateMask() list, err := lister.Run() @@ -91,7 +92,7 @@ func TestList_Sort(t *testing.T) { is := assert.New(t) lister := newListFixture(t) lister.Sort = ByNameDesc // Other sorts are tested elsewhere - makeMeSomeReleases(lister.cfg.Releases, t) + makeMeSomeReleases(t, lister.cfg.Releases) list, err := lister.Run() is.NoError(err) is.Len(list, 3) @@ -104,7 +105,7 @@ func TestList_Limit(t *testing.T) { is := assert.New(t) lister := newListFixture(t) lister.Limit = 2 - makeMeSomeReleases(lister.cfg.Releases, t) + makeMeSomeReleases(t, lister.cfg.Releases) list, err := lister.Run() is.NoError(err) is.Len(list, 2) @@ -117,7 +118,7 @@ func TestList_BigLimit(t *testing.T) { is := assert.New(t) lister := newListFixture(t) lister.Limit = 20 - makeMeSomeReleases(lister.cfg.Releases, t) + makeMeSomeReleases(t, lister.cfg.Releases) list, err := lister.Run() is.NoError(err) is.Len(list, 3) @@ -133,7 +134,7 @@ func TestList_LimitOffset(t *testing.T) { lister := newListFixture(t) lister.Limit = 2 lister.Offset = 1 - makeMeSomeReleases(lister.cfg.Releases, t) + makeMeSomeReleases(t, lister.cfg.Releases) list, err := lister.Run() is.NoError(err) is.Len(list, 2) @@ -148,7 +149,7 @@ func TestList_LimitOffsetOutOfBounds(t *testing.T) { lister := newListFixture(t) lister.Limit = 2 lister.Offset = 3 // Last item is index 2 - makeMeSomeReleases(lister.cfg.Releases, t) + makeMeSomeReleases(t, lister.cfg.Releases) list, err := lister.Run() is.NoError(err) is.Len(list, 0) @@ -163,7 +164,7 @@ func TestList_LimitOffsetOutOfBounds(t *testing.T) { func TestList_StateMask(t *testing.T) { is := assert.New(t) lister := newListFixture(t) - makeMeSomeReleases(lister.cfg.Releases, t) + makeMeSomeReleases(t, lister.cfg.Releases) one, err := lister.cfg.Releases.Get("one", 1) is.NoError(err) one.SetStatus(release.StatusUninstalled, "uninstalled") @@ -193,7 +194,7 @@ func TestList_StateMaskWithStaleRevisions(t *testing.T) { lister := newListFixture(t) lister.StateMask = ListFailed - makeMeSomeReleasesWithStaleFailure(lister.cfg.Releases, t) + makeMeSomeReleasesWithStaleFailure(t, lister.cfg.Releases) res, err := lister.Run() @@ -205,7 +206,7 @@ func TestList_StateMaskWithStaleRevisions(t *testing.T) { is.Equal("failed", res[0].Name) } -func makeMeSomeReleasesWithStaleFailure(store *storage.Storage, t *testing.T) { +func makeMeSomeReleasesWithStaleFailure(t *testing.T, store *storage.Storage) { t.Helper() one := namedReleaseStub("clean", release.StatusDeployed) one.Namespace = "default" @@ -242,7 +243,7 @@ func TestList_Filter(t *testing.T) { is := assert.New(t) lister := newListFixture(t) lister.Filter = "th." - makeMeSomeReleases(lister.cfg.Releases, t) + makeMeSomeReleases(t, lister.cfg.Releases) res, err := lister.Run() is.NoError(err) @@ -254,13 +255,13 @@ func TestList_FilterFailsCompile(t *testing.T) { is := assert.New(t) lister := newListFixture(t) lister.Filter = "t[h.{{{" - makeMeSomeReleases(lister.cfg.Releases, t) + makeMeSomeReleases(t, lister.cfg.Releases) _, err := lister.Run() is.Error(err) } -func makeMeSomeReleases(store *storage.Storage, t *testing.T) { +func makeMeSomeReleases(t *testing.T, store *storage.Storage) { t.Helper() one := releaseStub() one.Name = "one" diff --git a/pkg/action/uninstall_test.go b/pkg/action/uninstall_test.go index a83e4bc75..8b148522c 100644 --- a/pkg/action/uninstall_test.go +++ b/pkg/action/uninstall_test.go @@ -28,6 +28,7 @@ import ( ) func uninstallAction(t *testing.T) *Uninstall { + t.Helper() config := actionConfigFixture(t) unAction := NewUninstall(config) return unAction diff --git a/pkg/action/upgrade_test.go b/pkg/action/upgrade_test.go index 19869f6d6..e20955560 100644 --- a/pkg/action/upgrade_test.go +++ b/pkg/action/upgrade_test.go @@ -36,6 +36,7 @@ import ( ) func upgradeAction(t *testing.T) *Upgrade { + t.Helper() config := actionConfigFixture(t) upAction := NewUpgrade(config) upAction.Namespace = "spaced" @@ -56,7 +57,7 @@ func TestUpgradeRelease_Success(t *testing.T) { upAction.WaitStrategy = kube.StatusWatcherStrategy vals := map[string]interface{}{} - ctx, done := context.WithCancel(context.Background()) + ctx, done := context.WithCancel(t.Context()) res, err := upAction.RunWithContext(ctx, rel.Name, buildChart(), vals) done() req.NoError(err) @@ -383,7 +384,6 @@ func TestUpgradeRelease_Pending(t *testing.T) { } func TestUpgradeRelease_Interrupted_Wait(t *testing.T) { - is := assert.New(t) req := require.New(t) @@ -399,8 +399,7 @@ func TestUpgradeRelease_Interrupted_Wait(t *testing.T) { upAction.WaitStrategy = kube.StatusWatcherStrategy vals := map[string]interface{}{} - ctx := context.Background() - ctx, cancel := context.WithCancel(ctx) + ctx, cancel := context.WithCancel(t.Context()) time.AfterFunc(time.Second, cancel) res, err := upAction.RunWithContext(ctx, rel.Name, buildChart(), vals) @@ -408,11 +407,9 @@ func TestUpgradeRelease_Interrupted_Wait(t *testing.T) { req.Error(err) is.Contains(res.Info.Description, "Upgrade \"interrupted-release\" failed: context canceled") is.Equal(res.Info.Status, release.StatusFailed) - } func TestUpgradeRelease_Interrupted_Atomic(t *testing.T) { - is := assert.New(t) req := require.New(t) @@ -428,8 +425,7 @@ func TestUpgradeRelease_Interrupted_Atomic(t *testing.T) { upAction.Atomic = true vals := map[string]interface{}{} - ctx := context.Background() - ctx, cancel := context.WithCancel(ctx) + ctx, cancel := context.WithCancel(t.Context()) time.AfterFunc(time.Second, cancel) res, err := upAction.RunWithContext(ctx, rel.Name, buildChart(), vals) @@ -445,7 +441,7 @@ func TestUpgradeRelease_Interrupted_Atomic(t *testing.T) { } func TestMergeCustomLabels(t *testing.T) { - var tests = [][3]map[string]string{ + tests := [][3]map[string]string{ {nil, nil, map[string]string{}}, {map[string]string{}, map[string]string{}, map[string]string{}}, {map[string]string{"k1": "v1", "k2": "v2"}, nil, map[string]string{"k1": "v1", "k2": "v2"}}, @@ -550,7 +546,7 @@ func TestUpgradeRelease_DryRun(t *testing.T) { upAction.DryRun = true vals := map[string]interface{}{} - ctx, done := context.WithCancel(context.Background()) + ctx, done := context.WithCancel(t.Context()) res, err := upAction.RunWithContext(ctx, rel.Name, buildChart(withSampleSecret()), vals) done() req.NoError(err) @@ -566,7 +562,7 @@ func TestUpgradeRelease_DryRun(t *testing.T) { upAction.HideSecret = true vals = map[string]interface{}{} - ctx, done = context.WithCancel(context.Background()) + ctx, done = context.WithCancel(t.Context()) res, err = upAction.RunWithContext(ctx, rel.Name, buildChart(withSampleSecret()), vals) done() req.NoError(err) @@ -582,7 +578,7 @@ func TestUpgradeRelease_DryRun(t *testing.T) { upAction.DryRun = false vals = map[string]interface{}{} - ctx, done = context.WithCancel(context.Background()) + ctx, done = context.WithCancel(t.Context()) _, err = upAction.RunWithContext(ctx, rel.Name, buildChart(withSampleSecret()), vals) done() req.Error(err) diff --git a/pkg/action/validate.go b/pkg/action/validate.go index 22db74041..e1021860f 100644 --- a/pkg/action/validate.go +++ b/pkg/action/validate.go @@ -18,6 +18,7 @@ package action import ( "fmt" + "maps" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" @@ -194,11 +195,7 @@ func mergeAnnotations(obj runtime.Object, annotations map[string]string) error { // merge two maps, always taking the value on the right func mergeStrStrMaps(current, desired map[string]string) map[string]string { result := make(map[string]string) - for k, v := range current { - result[k] = v - } - for k, desiredVal := range desired { - result[k] = desiredVal - } + maps.Copy(result, current) + maps.Copy(result, desired) return result } diff --git a/pkg/chart/v2/loader/archive_test.go b/pkg/chart/v2/loader/archive_test.go index 4d6db9ed4..d16c47563 100644 --- a/pkg/chart/v2/loader/archive_test.go +++ b/pkg/chart/v2/loader/archive_test.go @@ -33,6 +33,7 @@ func TestLoadArchiveFiles(t *testing.T) { name: "empty input should return no files", generate: func(_ *tar.Writer) {}, check: func(t *testing.T, _ []*BufferedFile, err error) { + t.Helper() if err.Error() != "no files in chart archive" { t.Fatalf(`expected "no files in chart archive", got [%#v]`, err) } @@ -61,6 +62,7 @@ func TestLoadArchiveFiles(t *testing.T) { } }, check: func(t *testing.T, files []*BufferedFile, err error) { + t.Helper() if err != nil { t.Fatalf(`got unwanted error [%#v] for tar file with pax_global_header content`, err) } diff --git a/pkg/chart/v2/loader/load.go b/pkg/chart/v2/loader/load.go index 7838b577f..75c73e959 100644 --- a/pkg/chart/v2/loader/load.go +++ b/pkg/chart/v2/loader/load.go @@ -19,11 +19,11 @@ package loader import ( "bufio" "bytes" - "encoding/json" "errors" "fmt" "io" "log" + "maps" "os" "path/filepath" "strings" @@ -223,10 +223,7 @@ func LoadValues(data io.Reader) (map[string]interface{}, error) { } return nil, fmt.Errorf("error reading yaml document: %w", err) } - if err := yaml.Unmarshal(raw, ¤tMap, func(d *json.Decoder) *json.Decoder { - d.UseNumber() - return d - }); err != nil { + if err := yaml.Unmarshal(raw, ¤tMap); err != nil { return nil, fmt.Errorf("cannot unmarshal yaml document: %w", err) } values = MergeMaps(values, currentMap) @@ -238,9 +235,7 @@ func LoadValues(data io.Reader) (map[string]interface{}, error) { // If the value is a map, the maps will be merged recursively. func MergeMaps(a, b map[string]interface{}) map[string]interface{} { out := make(map[string]interface{}, len(a)) - for k, v := range a { - out[k] = v - } + maps.Copy(out, a) for k, v := range b { if v, ok := v.(map[string]interface{}); ok { if bv, ok := out[k]; ok { diff --git a/pkg/chart/v2/loader/load_test.go b/pkg/chart/v2/loader/load_test.go index 2e16b8560..41154421c 100644 --- a/pkg/chart/v2/loader/load_test.go +++ b/pkg/chart/v2/loader/load_test.go @@ -648,6 +648,7 @@ func verifyChart(t *testing.T, c *chart.Chart) { } func verifyDependencies(t *testing.T, c *chart.Chart) { + t.Helper() if len(c.Metadata.Dependencies) != 2 { t.Errorf("Expected 2 dependencies, got %d", len(c.Metadata.Dependencies)) } @@ -670,6 +671,7 @@ func verifyDependencies(t *testing.T, c *chart.Chart) { } func verifyDependenciesLock(t *testing.T, c *chart.Chart) { + t.Helper() if len(c.Metadata.Dependencies) != 2 { t.Errorf("Expected 2 dependencies, got %d", len(c.Metadata.Dependencies)) } @@ -692,10 +694,12 @@ func verifyDependenciesLock(t *testing.T, c *chart.Chart) { } func verifyFrobnitz(t *testing.T, c *chart.Chart) { + t.Helper() verifyChartFileAndTemplate(t, c, "frobnitz") } func verifyChartFileAndTemplate(t *testing.T, c *chart.Chart, name string) { + t.Helper() if c.Metadata == nil { t.Fatal("Metadata is nil") } @@ -750,6 +754,7 @@ func verifyChartFileAndTemplate(t *testing.T, c *chart.Chart, name string) { } func verifyBomStripped(t *testing.T, files []*chart.File) { + t.Helper() for _, file := range files { if bytes.HasPrefix(file.Data, utf8bom) { t.Errorf("Byte Order Mark still present in processed file %s", file.Name) diff --git a/pkg/chart/v2/util/capabilities.go b/pkg/chart/v2/util/capabilities.go index d4b420b2f..23b6d46fa 100644 --- a/pkg/chart/v2/util/capabilities.go +++ b/pkg/chart/v2/util/capabilities.go @@ -17,6 +17,7 @@ package util import ( "fmt" + "slices" "strconv" "github.com/Masterminds/semver/v3" @@ -102,12 +103,7 @@ type VersionSet []string // // vs.Has("apps/v1") func (v VersionSet) Has(apiVersion string) bool { - for _, x := range v { - if x == apiVersion { - return true - } - } - return false + return slices.Contains(v, apiVersion) } func allKnownVersions() VersionSet { diff --git a/pkg/chart/v2/util/chartfile.go b/pkg/chart/v2/util/chartfile.go index 6748c6a91..1f9c712b2 100644 --- a/pkg/chart/v2/util/chartfile.go +++ b/pkg/chart/v2/util/chartfile.go @@ -39,7 +39,7 @@ func LoadChartfile(filename string) (*chart.Metadata, error) { return y, err } -// StrictLoadChartFile loads a Chart.yaml into a *chart.Metadata using a strict unmarshaling +// StrictLoadChartfile loads a Chart.yaml into a *chart.Metadata using a strict unmarshaling func StrictLoadChartfile(filename string) (*chart.Metadata, error) { b, err := os.ReadFile(filename) if err != nil { diff --git a/pkg/chart/v2/util/chartfile_test.go b/pkg/chart/v2/util/chartfile_test.go index a2896b235..00c530b8a 100644 --- a/pkg/chart/v2/util/chartfile_test.go +++ b/pkg/chart/v2/util/chartfile_test.go @@ -34,7 +34,7 @@ func TestLoadChartfile(t *testing.T) { } func verifyChartfile(t *testing.T, f *chart.Metadata, name string) { - + t.Helper() if f == nil { //nolint:staticcheck t.Fatal("Failed verifyChartfile because f is nil") } diff --git a/pkg/chart/v2/util/coalesce.go b/pkg/chart/v2/util/coalesce.go index 76dfdfa1a..a3e0f5ae8 100644 --- a/pkg/chart/v2/util/coalesce.go +++ b/pkg/chart/v2/util/coalesce.go @@ -19,6 +19,7 @@ package util import ( "fmt" "log" + "maps" "github.com/mitchellh/copystructure" @@ -182,9 +183,7 @@ func coalesceGlobals(printf printFn, dest, src map[string]interface{}, prefix st func copyMap(src map[string]interface{}) map[string]interface{} { m := make(map[string]interface{}, len(src)) - for k, v := range src { - m[k] = v - } + maps.Copy(m, src) return m } diff --git a/pkg/chart/v2/util/coalesce_test.go b/pkg/chart/v2/util/coalesce_test.go index 3d4ee4fa8..e2c45a435 100644 --- a/pkg/chart/v2/util/coalesce_test.go +++ b/pkg/chart/v2/util/coalesce_test.go @@ -19,6 +19,7 @@ package util import ( "encoding/json" "fmt" + "maps" "testing" "github.com/stretchr/testify/assert" @@ -144,9 +145,7 @@ func TestCoalesceValues(t *testing.T) { // to CoalesceValues as argument, so that we can // use it for asserting later valsCopy := make(Values, len(vals)) - for key, value := range vals { - valsCopy[key] = value - } + maps.Copy(valsCopy, vals) v, err := CoalesceValues(c, vals) if err != nil { @@ -304,9 +303,7 @@ func TestMergeValues(t *testing.T) { // to MergeValues as argument, so that we can // use it for asserting later valsCopy := make(Values, len(vals)) - for key, value := range vals { - valsCopy[key] = value - } + maps.Copy(valsCopy, vals) v, err := MergeValues(c, vals) if err != nil { diff --git a/pkg/chart/v2/util/dependencies_test.go b/pkg/chart/v2/util/dependencies_test.go index 9b7fe3bef..d645d7bf5 100644 --- a/pkg/chart/v2/util/dependencies_test.go +++ b/pkg/chart/v2/util/dependencies_test.go @@ -15,7 +15,6 @@ limitations under the License. package util import ( - "encoding/json" "os" "path/filepath" "sort" @@ -134,7 +133,7 @@ func TestDependencyEnabled(t *testing.T) { } } -// extractCharts recursively searches chart dependencies returning all charts found +// extractChartNames recursively searches chart dependencies returning all charts found func extractChartNames(c *chart.Chart) []string { var out []string var fn func(c *chart.Chart) @@ -238,20 +237,6 @@ func TestProcessDependencyImportValues(t *testing.T) { if b := strconv.FormatBool(pv); b != vv { t.Errorf("failed to match imported bool value %v with expected %v for key %q", b, vv, kk) } - case json.Number: - if fv, err := pv.Float64(); err == nil { - if sfv := strconv.FormatFloat(fv, 'f', -1, 64); sfv != vv { - t.Errorf("failed to match imported float value %v with expected %v for key %q", sfv, vv, kk) - } - } - if iv, err := pv.Int64(); err == nil { - if siv := strconv.FormatInt(iv, 10); siv != vv { - t.Errorf("failed to match imported int value %v with expected %v for key %q", siv, vv, kk) - } - } - if pv.String() != vv { - t.Errorf("failed to match imported string value %q with expected %q for key %q", pv, vv, kk) - } default: if pv != vv { t.Errorf("failed to match imported string value %q with expected %q for key %q", pv, vv, kk) @@ -356,10 +341,6 @@ func TestProcessDependencyImportValuesMultiLevelPrecedence(t *testing.T) { if s := strconv.FormatFloat(pv, 'f', -1, 64); s != vv { t.Errorf("failed to match imported float value %v with expected %v", s, vv) } - case json.Number: - if pv.String() != vv { - t.Errorf("failed to match imported string value %q with expected %q", pv, vv) - } default: if pv != vv { t.Errorf("failed to match imported string value %q with expected %q", pv, vv) @@ -558,6 +539,7 @@ func TestDependentChartsWithSomeSubchartsSpecifiedInDependency(t *testing.T) { } func validateDependencyTree(t *testing.T, c *chart.Chart) { + t.Helper() for _, dependency := range c.Dependencies() { if dependency.Parent() != c { if dependency.Parent() != c { diff --git a/pkg/chart/v2/util/jsonschema_test.go b/pkg/chart/v2/util/jsonschema_test.go index d781aa4be..3279eb0db 100644 --- a/pkg/chart/v2/util/jsonschema_test.go +++ b/pkg/chart/v2/util/jsonschema_test.go @@ -55,8 +55,8 @@ func TestValidateAgainstInvalidSingleSchema(t *testing.T) { errString = err.Error() } - expectedErrString := "unable to validate schema: runtime error: invalid " + - "memory address or nil pointer dereference" + expectedErrString := `"file:///values.schema.json#" is not valid against metaschema: jsonschema validation failed with 'https://json-schema.org/draft/2020-12/schema#' +- at '': got number, want boolean or object` if errString != expectedErrString { t.Errorf("Error string :\n`%s`\ndoes not match expected\n`%s`", errString, expectedErrString) } diff --git a/pkg/chart/v2/util/values.go b/pkg/chart/v2/util/values.go index 42b1a28e8..6850e8b9b 100644 --- a/pkg/chart/v2/util/values.go +++ b/pkg/chart/v2/util/values.go @@ -17,7 +17,6 @@ limitations under the License. package util import ( - "encoding/json" "errors" "fmt" "io" @@ -106,10 +105,7 @@ func tableLookup(v Values, simple string) (Values, error) { // ReadValues will parse YAML byte data into a Values. func ReadValues(data []byte) (vals Values, err error) { - err = yaml.Unmarshal(data, &vals, func(d *json.Decoder) *json.Decoder { - d.UseNumber() - return d - }) + err = yaml.Unmarshal(data, &vals) if len(vals) == 0 { vals = Values{} } diff --git a/pkg/chart/v2/util/values_test.go b/pkg/chart/v2/util/values_test.go index 6a5400f78..1a25fafb8 100644 --- a/pkg/chart/v2/util/values_test.go +++ b/pkg/chart/v2/util/values_test.go @@ -224,6 +224,7 @@ chapter: } func matchValues(t *testing.T, data map[string]interface{}) { + t.Helper() if data["poet"] != "Coleridge" { t.Errorf("Unexpected poet: %s", data["poet"]) } diff --git a/pkg/cli/environment_test.go b/pkg/cli/environment_test.go index 8a3b87936..52326eeff 100644 --- a/pkg/cli/environment_test.go +++ b/pkg/cli/environment_test.go @@ -38,7 +38,6 @@ func TestSetNamespace(t *testing.T) { if settings.namespace != "testns" { t.Errorf("Expected namespace testns, got %s", settings.namespace) } - } func TestEnvSettings(t *testing.T) { @@ -126,7 +125,7 @@ func TestEnvSettings(t *testing.T) { defer resetEnv()() for k, v := range tt.envvars { - os.Setenv(k, v) + t.Setenv(k, v) } flags := pflag.NewFlagSet("testing", pflag.ContinueOnError) @@ -233,10 +232,7 @@ func TestEnvOrBool(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { if tt.env != "" { - t.Cleanup(func() { - os.Unsetenv(tt.env) - }) - os.Setenv(tt.env, tt.val) + t.Setenv(tt.env, tt.val) } actual := envBoolOr(tt.env, tt.def) if actual != tt.expected { diff --git a/pkg/cmd/completion_test.go b/pkg/cmd/completion_test.go index 872da25f3..375a9a97d 100644 --- a/pkg/cmd/completion_test.go +++ b/pkg/cmd/completion_test.go @@ -27,6 +27,7 @@ import ( // Check if file completion should be performed according to parameter 'shouldBePerformed' func checkFileCompletion(t *testing.T, cmdName string, shouldBePerformed bool) { + t.Helper() storage := storageFixture() storage.Create(&release.Release{ Name: "myrelease", @@ -64,6 +65,7 @@ func TestCompletionFileCompletion(t *testing.T) { } func checkReleaseCompletion(t *testing.T, cmdName string, multiReleasesAllowed bool) { + t.Helper() multiReleaseTestGolden := "output/empty_nofile_comp.txt" if multiReleasesAllowed { multiReleaseTestGolden = "output/release_list_repeat_comp.txt" diff --git a/pkg/cmd/create_test.go b/pkg/cmd/create_test.go index 26eabbfc3..103cd3bc0 100644 --- a/pkg/cmd/create_test.go +++ b/pkg/cmd/create_test.go @@ -33,7 +33,7 @@ func TestCreateCmd(t *testing.T) { ensure.HelmHome(t) cname := "testchart" dir := t.TempDir() - defer testChdir(t, dir)() + defer t.Chdir(dir) // Run a create if _, _, err := executeActionCommand("create " + cname); err != nil { @@ -64,19 +64,19 @@ func TestCreateStarterCmd(t *testing.T) { ensure.HelmHome(t) cname := "testchart" defer resetEnv()() - os.MkdirAll(helmpath.CachePath(), 0755) - defer testChdir(t, helmpath.CachePath())() + os.MkdirAll(helmpath.CachePath(), 0o755) + defer t.Chdir(helmpath.CachePath()) // Create a starter. starterchart := helmpath.DataPath("starters") - os.MkdirAll(starterchart, 0755) + os.MkdirAll(starterchart, 0o755) if dest, err := chartutil.Create("starterchart", starterchart); err != nil { t.Fatalf("Could not create chart: %s", err) } else { t.Logf("Created %s", dest) } tplpath := filepath.Join(starterchart, "starterchart", "templates", "foo.tpl") - if err := os.WriteFile(tplpath, []byte("test"), 0644); err != nil { + if err := os.WriteFile(tplpath, []byte("test"), 0o644); err != nil { t.Fatalf("Could not write template: %s", err) } @@ -122,7 +122,6 @@ func TestCreateStarterCmd(t *testing.T) { if !found { t.Error("Did not find foo.tpl") } - } func TestCreateStarterAbsoluteCmd(t *testing.T) { @@ -132,19 +131,19 @@ func TestCreateStarterAbsoluteCmd(t *testing.T) { // Create a starter. starterchart := helmpath.DataPath("starters") - os.MkdirAll(starterchart, 0755) + os.MkdirAll(starterchart, 0o755) if dest, err := chartutil.Create("starterchart", starterchart); err != nil { t.Fatalf("Could not create chart: %s", err) } else { t.Logf("Created %s", dest) } tplpath := filepath.Join(starterchart, "starterchart", "templates", "foo.tpl") - if err := os.WriteFile(tplpath, []byte("test"), 0644); err != nil { + if err := os.WriteFile(tplpath, []byte("test"), 0o644); err != nil { t.Fatalf("Could not write template: %s", err) } - os.MkdirAll(helmpath.CachePath(), 0755) - defer testChdir(t, helmpath.CachePath())() + os.MkdirAll(helmpath.CachePath(), 0o755) + defer t.Chdir(helmpath.CachePath()) starterChartPath := filepath.Join(starterchart, "starterchart") diff --git a/pkg/cmd/dependency_update_test.go b/pkg/cmd/dependency_update_test.go index a450d4b22..9646c6816 100644 --- a/pkg/cmd/dependency_update_test.go +++ b/pkg/cmd/dependency_update_test.go @@ -250,6 +250,7 @@ func TestDependencyUpdateCmd_WithRepoThatWasNotAdded(t *testing.T) { } func setupMockRepoServer(t *testing.T) *repotest.Server { + t.Helper() srv := repotest.NewTempServer( t, repotest.WithChartSourceGlob("testdata/testcharts/*.tgz"), diff --git a/pkg/cmd/flags_test.go b/pkg/cmd/flags_test.go index 9d416f216..cbc2e6419 100644 --- a/pkg/cmd/flags_test.go +++ b/pkg/cmd/flags_test.go @@ -29,6 +29,7 @@ import ( ) func outputFlagCompletionTest(t *testing.T, cmdName string) { + t.Helper() releasesMockWithStatus := func(info *release.Info, hooks ...*release.Hook) []*release.Release { info.LastDeployed = helmtime.Unix(1452902400, 0).UTC() return []*release.Release{{ diff --git a/pkg/cmd/helpers_test.go b/pkg/cmd/helpers_test.go index b48f802b5..5d71fecad 100644 --- a/pkg/cmd/helpers_test.go +++ b/pkg/cmd/helpers_test.go @@ -149,15 +149,3 @@ func resetEnv() func() { settings = cli.New() } } - -func testChdir(t *testing.T, dir string) func() { - t.Helper() - old, err := os.Getwd() - if err != nil { - t.Fatal(err) - } - if err := os.Chdir(dir); err != nil { - t.Fatal(err) - } - return func() { os.Chdir(old) } -} diff --git a/pkg/cmd/history_test.go b/pkg/cmd/history_test.go index 594d93d21..d26ed9ecf 100644 --- a/pkg/cmd/history_test.go +++ b/pkg/cmd/history_test.go @@ -75,6 +75,7 @@ func TestHistoryOutputCompletion(t *testing.T) { } func revisionFlagCompletionTest(t *testing.T, cmdName string) { + t.Helper() mk := func(name string, vers int, status release.Status) *release.Release { return release.Mock(&release.MockReleaseOptions{ Name: name, diff --git a/pkg/cmd/install.go b/pkg/cmd/install.go index cbec33a80..3496a4bbd 100644 --- a/pkg/cmd/install.go +++ b/pkg/cmd/install.go @@ -25,6 +25,7 @@ import ( "log/slog" "os" "os/signal" + "slices" "syscall" "time" @@ -350,13 +351,7 @@ func compInstall(args []string, toComplete string, client *action.Install) ([]st func validateDryRunOptionFlag(dryRunOptionFlagValue string) error { // Validate dry-run flag value with a set of allowed value allowedDryRunValues := []string{"false", "true", "none", "client", "server"} - isAllowed := false - for _, v := range allowedDryRunValues { - if dryRunOptionFlagValue == v { - isAllowed = true - break - } - } + isAllowed := slices.Contains(allowedDryRunValues, dryRunOptionFlagValue) if !isAllowed { return errors.New("invalid dry-run flag. Flag must one of the following: false, true, none, client, server") } diff --git a/pkg/cmd/list.go b/pkg/cmd/list.go index 69a4ff36d..5af43adad 100644 --- a/pkg/cmd/list.go +++ b/pkg/cmd/list.go @@ -20,6 +20,7 @@ import ( "fmt" "io" "os" + "slices" "strconv" "github.com/gosuri/uitable" @@ -203,13 +204,7 @@ func filterReleases(releases []*release.Release, ignoredReleaseNames []string) [ var filteredReleases []*release.Release for _, rel := range releases { - found := false - for _, ignoredName := range ignoredReleaseNames { - if rel.Name == ignoredName { - found = true - break - } - } + found := slices.Contains(ignoredReleaseNames, rel.Name) if !found { filteredReleases = append(filteredReleases, rel) } diff --git a/pkg/cmd/load_plugins.go b/pkg/cmd/load_plugins.go index 2eef1fb3c..385990d82 100644 --- a/pkg/cmd/load_plugins.go +++ b/pkg/cmd/load_plugins.go @@ -23,6 +23,7 @@ import ( "os" "os/exec" "path/filepath" + "slices" "strconv" "strings" "syscall" @@ -163,10 +164,8 @@ func manuallyProcessArgs(args []string) ([]string, []string) { } isKnown := func(v string) string { - for _, i := range kvargs { - if i == v { - return v - } + if slices.Contains(kvargs, v) { + return v } return "" } diff --git a/pkg/cmd/package_test.go b/pkg/cmd/package_test.go index 54358fc12..b17684aa6 100644 --- a/pkg/cmd/package_test.go +++ b/pkg/cmd/package_test.go @@ -111,9 +111,9 @@ func TestPackage(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { cachePath := t.TempDir() - defer testChdir(t, cachePath)() + defer t.Chdir(cachePath) - if err := os.MkdirAll("toot", 0777); err != nil { + if err := os.MkdirAll("toot", 0o777); err != nil { t.Fatal(err) } diff --git a/pkg/cmd/plugin_list.go b/pkg/cmd/plugin_list.go index fdd66ec0a..5bb9ff68d 100644 --- a/pkg/cmd/plugin_list.go +++ b/pkg/cmd/plugin_list.go @@ -19,6 +19,7 @@ import ( "fmt" "io" "log/slog" + "slices" "github.com/gosuri/uitable" "github.com/spf13/cobra" @@ -60,13 +61,7 @@ func filterPlugins(plugins []*plugin.Plugin, ignoredPluginNames []string) []*plu var filteredPlugins []*plugin.Plugin for _, plugin := range plugins { - found := false - for _, ignoredName := range ignoredPluginNames { - if plugin.Metadata.Name == ignoredName { - found = true - break - } - } + found := slices.Contains(ignoredPluginNames, plugin.Metadata.Name) if !found { filteredPlugins = append(filteredPlugins, plugin) } diff --git a/pkg/cmd/plugin_test.go b/pkg/cmd/plugin_test.go index 7c36698b1..74f7a276a 100644 --- a/pkg/cmd/plugin_test.go +++ b/pkg/cmd/plugin_test.go @@ -79,7 +79,6 @@ func TestManuallyProcessArgs(t *testing.T) { t.Errorf("expected unknown flag %d to be %q, got %q", i, expectUnknown[i], k) } } - } func TestLoadPlugins(t *testing.T) { @@ -276,6 +275,7 @@ func TestLoadPluginsForCompletion(t *testing.T) { } func checkCommand(t *testing.T, plugins []*cobra.Command, tests []staticCompletionDetails) { + t.Helper() if len(plugins) != len(tests) { t.Fatalf("Expected commands %v, got %v", tests, plugins) } @@ -326,7 +326,6 @@ func checkCommand(t *testing.T, plugins []*cobra.Command, tests []staticCompleti } func TestPluginDynamicCompletion(t *testing.T) { - tests := []cmdTestCase{{ name: "completion for plugin", cmd: "__complete args ''", @@ -363,7 +362,7 @@ func TestLoadPlugins_HelmNoPlugins(t *testing.T) { settings.PluginsDirectory = "testdata/helmhome/helm/plugins" settings.RepositoryConfig = "testdata/helmhome/helm/repository" - os.Setenv("HELM_NO_PLUGINS", "1") + t.Setenv("HELM_NO_PLUGINS", "1") out := bytes.NewBuffer(nil) cmd := &cobra.Command{} @@ -376,7 +375,6 @@ func TestLoadPlugins_HelmNoPlugins(t *testing.T) { } func TestPluginCmdsCompletion(t *testing.T) { - tests := []cmdTestCase{{ name: "completion for plugin update", cmd: "__complete plugin update ''", diff --git a/pkg/cmd/registry_login.go b/pkg/cmd/registry_login.go index 3719c1c17..1350fb244 100644 --- a/pkg/cmd/registry_login.go +++ b/pkg/cmd/registry_login.go @@ -34,6 +34,10 @@ import ( const registryLoginDesc = ` Authenticate to a remote registry. + +For example for Github Container Registry: + + echo "$GITHUB_TOKEN" | helm registry login ghcr.io -u $GITHUB_USER --password-stdin ` type registryLoginOptions struct { diff --git a/pkg/cmd/release_testing.go b/pkg/cmd/release_testing.go index 4904aa9f1..1dac28534 100644 --- a/pkg/cmd/release_testing.go +++ b/pkg/cmd/release_testing.go @@ -17,6 +17,7 @@ limitations under the License. package cmd import ( + "errors" "fmt" "io" "regexp" @@ -85,7 +86,7 @@ func newReleaseTestCmd(cfg *action.Configuration, out io.Writer) *cobra.Command // Print a newline to stdout to separate the output fmt.Fprintln(out) if err := client.GetPodLogs(out, rel); err != nil { - return err + return errors.Join(runErr, err) } } diff --git a/pkg/cmd/repo_add.go b/pkg/cmd/repo_add.go index 24c1eecab..187234486 100644 --- a/pkg/cmd/repo_add.go +++ b/pkg/cmd/repo_add.go @@ -52,6 +52,7 @@ type repoAddOptions struct { passCredentialsAll bool forceUpdate bool allowDeprecatedRepos bool + timeout time.Duration certFile string keyFile string @@ -96,6 +97,7 @@ func newRepoAddCmd(out io.Writer) *cobra.Command { f.BoolVar(&o.insecureSkipTLSverify, "insecure-skip-tls-verify", false, "skip tls certificate checks for the repository") f.BoolVar(&o.allowDeprecatedRepos, "allow-deprecated-repos", false, "by default, this command will not allow adding official repos that have been permanently deleted. This disables that behavior") f.BoolVar(&o.passCredentialsAll, "pass-credentials", false, "pass credentials to all domains") + f.DurationVar(&o.timeout, "timeout", getter.DefaultHTTPTimeout*time.Second, "time to wait for the index file download to complete") return cmd } @@ -199,7 +201,7 @@ func (o *repoAddOptions) run(out io.Writer) error { return nil } - r, err := repo.NewChartRepository(&c, getter.All(settings)) + r, err := repo.NewChartRepository(&c, getter.All(settings, getter.WithTimeout(o.timeout))) if err != nil { return err } diff --git a/pkg/cmd/repo_add_test.go b/pkg/cmd/repo_add_test.go index 05b5ee53e..aa6c4eaad 100644 --- a/pkg/cmd/repo_add_test.go +++ b/pkg/cmd/repo_add_test.go @@ -50,7 +50,7 @@ func TestRepoAddCmd(t *testing.T) { defer srv2.Stop() tmpdir := filepath.Join(t.TempDir(), "path-component.yaml/data") - if err := os.MkdirAll(tmpdir, 0777); err != nil { + if err := os.MkdirAll(tmpdir, 0o777); err != nil { t.Fatal(err) } repoFile := filepath.Join(tmpdir, "repositories.yaml") @@ -99,7 +99,7 @@ func TestRepoAdd(t *testing.T) { forceUpdate: false, repoFile: repoFile, } - os.Setenv(xdg.CacheHomeEnvVar, rootDir) + t.Setenv(xdg.CacheHomeEnvVar, rootDir) if err := o.run(io.Discard); err != nil { t.Error(err) @@ -153,7 +153,7 @@ func TestRepoAddCheckLegalName(t *testing.T) { forceUpdate: false, repoFile: repoFile, } - os.Setenv(xdg.CacheHomeEnvVar, rootDir) + t.Setenv(xdg.CacheHomeEnvVar, rootDir) wantErrorMsg := fmt.Sprintf("repository name (%s) contains '/', please specify a different name without '/'", testRepoName) @@ -191,6 +191,7 @@ func TestRepoAddConcurrentHiddenFile(t *testing.T) { } func repoAddConcurrent(t *testing.T, testName, repoFile string) { + t.Helper() ts := repotest.NewTempServer( t, repotest.WithChartSourceGlob("testdata/testserver/*.*"), diff --git a/pkg/cmd/repo_list.go b/pkg/cmd/repo_list.go index 60c879984..70f57992e 100644 --- a/pkg/cmd/repo_list.go +++ b/pkg/cmd/repo_list.go @@ -17,7 +17,6 @@ limitations under the License. package cmd import ( - "errors" "fmt" "io" @@ -37,10 +36,14 @@ func newRepoListCmd(out io.Writer) *cobra.Command { Short: "list chart repositories", Args: require.NoArgs, ValidArgsFunction: noMoreArgsCompFunc, - RunE: func(_ *cobra.Command, _ []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { + // The error is silently ignored. If no repository file exists, it cannot be loaded, + // or the file isn't the right format to be parsed the error is ignored. The + // repositories will be 0. f, _ := repo.LoadFile(settings.RepositoryConfig) if len(f.Repositories) == 0 && outfmt != output.JSON && outfmt != output.YAML { - return errors.New("no repositories to show") + fmt.Fprintln(cmd.ErrOrStderr(), "no repositories to show") + return nil } return outfmt.Write(out, &repoListWriter{f.Repositories}) diff --git a/pkg/cmd/repo_list_test.go b/pkg/cmd/repo_list_test.go index 1da5484cc..2f6a9e4ad 100644 --- a/pkg/cmd/repo_list_test.go +++ b/pkg/cmd/repo_list_test.go @@ -17,6 +17,8 @@ limitations under the License. package cmd import ( + "fmt" + "path/filepath" "testing" ) @@ -27,3 +29,26 @@ func TestRepoListOutputCompletion(t *testing.T) { func TestRepoListFileCompletion(t *testing.T) { checkFileCompletion(t, "repo list", false) } + +func TestRepoList(t *testing.T) { + rootDir := t.TempDir() + repoFile := filepath.Join(rootDir, "repositories.yaml") + repoFile2 := "testdata/repositories.yaml" + + tests := []cmdTestCase{ + { + name: "list with no repos", + cmd: fmt.Sprintf("repo list --repository-config %s --repository-cache %s", repoFile, rootDir), + golden: "output/repo-list-empty.txt", + wantError: false, + }, + { + name: "list with repos", + cmd: fmt.Sprintf("repo list --repository-config %s --repository-cache %s", repoFile2, rootDir), + golden: "output/repo-list.txt", + wantError: false, + }, + } + + runTestCmd(t, tests) +} diff --git a/pkg/cmd/repo_remove_test.go b/pkg/cmd/repo_remove_test.go index b8bc7179a..bd8757812 100644 --- a/pkg/cmd/repo_remove_test.go +++ b/pkg/cmd/repo_remove_test.go @@ -153,6 +153,7 @@ func createCacheFiles(rootDir string, repoName string) (cacheIndexFile string, c } func testCacheFiles(t *testing.T, cacheIndexFile string, cacheChartsFile string, repoName string) { + t.Helper() if _, err := os.Stat(cacheIndexFile); err == nil { t.Errorf("Error cache index file was not removed for repository %s", repoName) } diff --git a/pkg/cmd/repo_update.go b/pkg/cmd/repo_update.go index 9f4a603ae..54318bf29 100644 --- a/pkg/cmd/repo_update.go +++ b/pkg/cmd/repo_update.go @@ -22,6 +22,7 @@ import ( "io" "slices" "sync" + "time" "github.com/spf13/cobra" @@ -46,6 +47,7 @@ type repoUpdateOptions struct { repoFile string repoCache string names []string + timeout time.Duration } func newRepoUpdateCmd(out io.Writer) *cobra.Command { @@ -68,6 +70,9 @@ func newRepoUpdateCmd(out io.Writer) *cobra.Command { }, } + f := cmd.Flags() + f.DurationVar(&o.timeout, "timeout", getter.DefaultHTTPTimeout*time.Second, "time to wait for the index file download to complete") + return cmd } @@ -94,7 +99,7 @@ func (o *repoUpdateOptions) run(out io.Writer) error { for _, cfg := range f.Repositories { if updateAllRepos || isRepoRequested(cfg.Name, o.names) { - r, err := repo.NewChartRepository(cfg, getter.All(settings)) + r, err := repo.NewChartRepository(cfg, getter.All(settings, getter.WithTimeout(o.timeout))) if err != nil { return err } @@ -113,14 +118,19 @@ func updateCharts(repos []*repo.ChartRepository, out io.Writer) error { var wg sync.WaitGroup failRepoURLChan := make(chan string, len(repos)) + writeMutex := sync.Mutex{} for _, re := range repos { wg.Add(1) go func(re *repo.ChartRepository) { defer wg.Done() if _, err := re.DownloadIndexFile(); err != nil { + writeMutex.Lock() + defer writeMutex.Unlock() fmt.Fprintf(out, "...Unable to get an update from the %q chart repository (%s):\n\t%s\n", re.Config.Name, re.Config.URL, err) failRepoURLChan <- re.Config.URL } else { + writeMutex.Lock() + defer writeMutex.Unlock() fmt.Fprintf(out, "...Successfully got an update from the %q chart repository\n", re.Config.Name) } }(re) diff --git a/pkg/cmd/require/args_test.go b/pkg/cmd/require/args_test.go index cd5850650..b6c430fc0 100644 --- a/pkg/cmd/require/args_test.go +++ b/pkg/cmd/require/args_test.go @@ -63,6 +63,7 @@ type testCase struct { } func runTestCases(t *testing.T, testCases []testCase) { + t.Helper() for i, tc := range testCases { t.Run(fmt.Sprint(i), func(t *testing.T) { cmd := &cobra.Command{ diff --git a/pkg/cmd/root_test.go b/pkg/cmd/root_test.go index 9521a5aa2..84e3d9ed2 100644 --- a/pkg/cmd/root_test.go +++ b/pkg/cmd/root_test.go @@ -80,7 +80,7 @@ func TestRootCmd(t *testing.T) { ensure.HelmHome(t) for k, v := range tt.envvars { - os.Setenv(k, v) + t.Setenv(k, v) } if _, _, err := executeActionCommand(tt.args); err != nil { diff --git a/pkg/cmd/template_test.go b/pkg/cmd/template_test.go index c478fced4..a6c848e08 100644 --- a/pkg/cmd/template_test.go +++ b/pkg/cmd/template_test.go @@ -22,18 +22,6 @@ import ( "testing" ) -func TestTemplateCmdWithToml(t *testing.T) { - - tests := []cmdTestCase{ - { - name: "check toToml function rendering", - cmd: fmt.Sprintf("template '%s'", "testdata/testcharts/issue-totoml"), - golden: "output/issue-totoml.txt", - }, - } - runTestCmd(t, tests) -} - var chartPath = "testdata/testcharts/subchart" func TestTemplateCmd(t *testing.T) { diff --git a/pkg/cmd/testdata/output/issue-totoml.txt b/pkg/cmd/testdata/output/issue-totoml.txt deleted file mode 100644 index 06cf4bb8d..000000000 --- a/pkg/cmd/testdata/output/issue-totoml.txt +++ /dev/null @@ -1,8 +0,0 @@ ---- -# Source: issue-totoml/templates/configmap.yaml -apiVersion: v1 -kind: ConfigMap -metadata: - name: issue-totoml -data: | - key = 13 diff --git a/pkg/cmd/testdata/output/repo-list-empty.txt b/pkg/cmd/testdata/output/repo-list-empty.txt new file mode 100644 index 000000000..c6edb659a --- /dev/null +++ b/pkg/cmd/testdata/output/repo-list-empty.txt @@ -0,0 +1 @@ +no repositories to show diff --git a/pkg/cmd/testdata/output/repo-list.txt b/pkg/cmd/testdata/output/repo-list.txt new file mode 100644 index 000000000..edbd0ecc1 --- /dev/null +++ b/pkg/cmd/testdata/output/repo-list.txt @@ -0,0 +1,4 @@ +NAME URL +charts https://charts.helm.sh/stable +firstexample http://firstexample.com +secondexample http://secondexample.com diff --git a/pkg/cmd/testdata/testcharts/issue-totoml/Chart.yaml b/pkg/cmd/testdata/testcharts/issue-totoml/Chart.yaml deleted file mode 100644 index f4be7a213..000000000 --- a/pkg/cmd/testdata/testcharts/issue-totoml/Chart.yaml +++ /dev/null @@ -1,3 +0,0 @@ -apiVersion: v2 -name: issue-totoml -version: 0.1.0 diff --git a/pkg/cmd/testdata/testcharts/issue-totoml/templates/configmap.yaml b/pkg/cmd/testdata/testcharts/issue-totoml/templates/configmap.yaml deleted file mode 100644 index 621e70d48..000000000 --- a/pkg/cmd/testdata/testcharts/issue-totoml/templates/configmap.yaml +++ /dev/null @@ -1,6 +0,0 @@ -apiVersion: v1 -kind: ConfigMap -metadata: - name: issue-totoml -data: | - {{ .Values.global | toToml }} diff --git a/pkg/cmd/testdata/testcharts/issue-totoml/values.yaml b/pkg/cmd/testdata/testcharts/issue-totoml/values.yaml deleted file mode 100644 index dd0140449..000000000 --- a/pkg/cmd/testdata/testcharts/issue-totoml/values.yaml +++ /dev/null @@ -1,2 +0,0 @@ -global: - key: 13 \ No newline at end of file diff --git a/pkg/cmd/upgrade_test.go b/pkg/cmd/upgrade_test.go index 8a840f149..d7375dcad 100644 --- a/pkg/cmd/upgrade_test.go +++ b/pkg/cmd/upgrade_test.go @@ -193,7 +193,7 @@ func TestUpgradeCmd(t *testing.T) { func TestUpgradeWithValue(t *testing.T) { releaseName := "funny-bunny-v2" - relMock, ch, chartPath := prepareMockRelease(releaseName, t) + relMock, ch, chartPath := prepareMockRelease(t, releaseName) defer resetEnv()() @@ -220,7 +220,7 @@ func TestUpgradeWithValue(t *testing.T) { func TestUpgradeWithStringValue(t *testing.T) { releaseName := "funny-bunny-v3" - relMock, ch, chartPath := prepareMockRelease(releaseName, t) + relMock, ch, chartPath := prepareMockRelease(t, releaseName) defer resetEnv()() @@ -248,7 +248,7 @@ func TestUpgradeWithStringValue(t *testing.T) { func TestUpgradeInstallWithSubchartNotes(t *testing.T) { releaseName := "wacky-bunny-v1" - relMock, ch, _ := prepareMockRelease(releaseName, t) + relMock, ch, _ := prepareMockRelease(t, releaseName) defer resetEnv()() @@ -280,7 +280,7 @@ func TestUpgradeInstallWithSubchartNotes(t *testing.T) { func TestUpgradeWithValuesFile(t *testing.T) { releaseName := "funny-bunny-v4" - relMock, ch, chartPath := prepareMockRelease(releaseName, t) + relMock, ch, chartPath := prepareMockRelease(t, releaseName) defer resetEnv()() @@ -308,7 +308,7 @@ func TestUpgradeWithValuesFile(t *testing.T) { func TestUpgradeWithValuesFromStdin(t *testing.T) { releaseName := "funny-bunny-v5" - relMock, ch, chartPath := prepareMockRelease(releaseName, t) + relMock, ch, chartPath := prepareMockRelease(t, releaseName) defer resetEnv()() @@ -340,7 +340,7 @@ func TestUpgradeWithValuesFromStdin(t *testing.T) { func TestUpgradeInstallWithValuesFromStdin(t *testing.T) { releaseName := "funny-bunny-v6" - _, _, chartPath := prepareMockRelease(releaseName, t) + _, _, chartPath := prepareMockRelease(t, releaseName) defer resetEnv()() @@ -368,7 +368,8 @@ func TestUpgradeInstallWithValuesFromStdin(t *testing.T) { } -func prepareMockRelease(releaseName string, t *testing.T) (func(n string, v int, ch *chart.Chart) *release.Release, *chart.Chart, string) { +func prepareMockRelease(t *testing.T, releaseName string) (func(n string, v int, ch *chart.Chart) *release.Release, *chart.Chart, string) { + t.Helper() tmpChart := t.TempDir() configmapData, err := os.ReadFile("testdata/testcharts/upgradetest/templates/configmap.yaml") if err != nil { @@ -445,7 +446,7 @@ func TestUpgradeFileCompletion(t *testing.T) { func TestUpgradeInstallWithLabels(t *testing.T) { releaseName := "funny-bunny-labels" - _, _, chartPath := prepareMockRelease(releaseName, t) + _, _, chartPath := prepareMockRelease(t, releaseName) defer resetEnv()() @@ -471,7 +472,8 @@ func TestUpgradeInstallWithLabels(t *testing.T) { } } -func prepareMockReleaseWithSecret(releaseName string, t *testing.T) (func(n string, v int, ch *chart.Chart) *release.Release, *chart.Chart, string) { +func prepareMockReleaseWithSecret(t *testing.T, releaseName string) (func(n string, v int, ch *chart.Chart) *release.Release, *chart.Chart, string) { + t.Helper() tmpChart := t.TempDir() configmapData, err := os.ReadFile("testdata/testcharts/chart-with-secret/templates/configmap.yaml") if err != nil { @@ -512,7 +514,7 @@ func prepareMockReleaseWithSecret(releaseName string, t *testing.T) (func(n stri func TestUpgradeWithDryRun(t *testing.T) { releaseName := "funny-bunny-labels" - _, _, chartPath := prepareMockReleaseWithSecret(releaseName, t) + _, _, chartPath := prepareMockReleaseWithSecret(t, releaseName) defer resetEnv()() diff --git a/pkg/downloader/chart_downloader_test.go b/pkg/downloader/chart_downloader_test.go index 26dcc58ff..766afede1 100644 --- a/pkg/downloader/chart_downloader_test.go +++ b/pkg/downloader/chart_downloader_test.go @@ -46,6 +46,7 @@ func TestResolveChartRef(t *testing.T) { {name: "reference, querystring repo", ref: "testing-querystring/alpine", expect: "http://example.com/alpine-1.2.3.tgz?key=value"}, {name: "reference, testing-relative repo", ref: "testing-relative/foo", expect: "http://example.com/helm/charts/foo-1.2.3.tgz"}, {name: "reference, testing-relative repo", ref: "testing-relative/bar", expect: "http://example.com/helm/bar-1.2.3.tgz"}, + {name: "reference, testing-relative repo", ref: "testing-relative/baz", expect: "http://example.com/path/to/baz-1.2.3.tgz"}, {name: "reference, testing-relative-trailing-slash repo", ref: "testing-relative-trailing-slash/foo", expect: "http://example.com/helm/charts/foo-1.2.3.tgz"}, {name: "reference, testing-relative-trailing-slash repo", ref: "testing-relative-trailing-slash/bar", expect: "http://example.com/helm/bar-1.2.3.tgz"}, {name: "encoded URL", ref: "encoded-url/foobar", expect: "http://example.com/with%2Fslash/charts/foobar-4.2.1.tgz"}, diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index e884e12d4..348c78edb 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -25,7 +25,6 @@ import ( "log" "net/url" "os" - "path" "path/filepath" "regexp" "strings" @@ -728,7 +727,6 @@ func (m *Manager) findChartURL(name, version, repoURL string, repos map[string]* } for _, cr := range repos { - if urlutil.Equal(repoURL, cr.Config.URL) { var entry repo.ChartVersions entry, err = findEntryByName(name, cr) @@ -745,7 +743,7 @@ func (m *Manager) findChartURL(name, version, repoURL string, repos map[string]* //nolint:nakedret return } - url, err = normalizeURL(repoURL, ve.URLs[0]) + url, err = repo.ResolveReferenceURL(repoURL, ve.URLs[0]) if err != nil { //nolint:nakedret return @@ -811,24 +809,6 @@ func versionEquals(v1, v2 string) bool { return sv1.Equal(sv2) } -func normalizeURL(baseURL, urlOrPath string) (string, error) { - u, err := url.Parse(urlOrPath) - if err != nil { - return urlOrPath, err - } - if u.IsAbs() { - return u.String(), nil - } - u2, err := url.Parse(baseURL) - if err != nil { - return urlOrPath, fmt.Errorf("base URL failed to parse: %w", err) - } - - u2.RawPath = path.Join(u2.RawPath, urlOrPath) - u2.Path = path.Join(u2.Path, urlOrPath) - return u2.String(), nil -} - // loadChartRepositories reads the repositories.yaml, and then builds a map of // ChartRepositories. // diff --git a/pkg/downloader/manager_test.go b/pkg/downloader/manager_test.go index fecc8fbef..53955c45b 100644 --- a/pkg/downloader/manager_test.go +++ b/pkg/downloader/manager_test.go @@ -53,26 +53,6 @@ func TestVersionEquals(t *testing.T) { } } -func TestNormalizeURL(t *testing.T) { - tests := []struct { - name, base, path, expect string - }{ - {name: "basic URL", base: "https://example.com", path: "http://helm.sh/foo", expect: "http://helm.sh/foo"}, - {name: "relative path", base: "https://helm.sh/charts", path: "foo", expect: "https://helm.sh/charts/foo"}, - {name: "Encoded path", base: "https://helm.sh/a%2Fb/charts", path: "foo", expect: "https://helm.sh/a%2Fb/charts/foo"}, - } - - for _, tt := range tests { - got, err := normalizeURL(tt.base, tt.path) - if err != nil { - t.Errorf("%s: error %s", tt.name, err) - continue - } else if got != tt.expect { - t.Errorf("%s: expected %q, got %q", tt.name, tt.expect, got) - } - } -} - func TestFindChartURL(t *testing.T) { var b bytes.Buffer m := &Manager{ @@ -134,6 +114,31 @@ func TestFindChartURL(t *testing.T) { if passcredentialsall != false { t.Errorf("Unexpected passcredentialsall %t", passcredentialsall) } + + name = "foo" + version = "1.2.3" + repoURL = "http://example.com/helm" + + churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err = m.findChartURL(name, version, repoURL, repos) + if err != nil { + t.Fatal(err) + } + + if churl != "http://example.com/helm/charts/foo-1.2.3.tgz" { + t.Errorf("Unexpected URL %q", churl) + } + if username != "" { + t.Errorf("Unexpected username %q", username) + } + if password != "" { + t.Errorf("Unexpected password %q", password) + } + if passcredentialsall != false { + t.Errorf("Unexpected passcredentialsall %t", passcredentialsall) + } + if insecureSkipTLSVerify { + t.Errorf("Unexpected insecureSkipTLSVerify %t", insecureSkipTLSVerify) + } } func TestGetRepoNames(t *testing.T) { @@ -437,6 +442,7 @@ func TestUpdateWithNoRepo(t *testing.T) { // Parent chart includes local-subchart 0.1.0 subchart from a fake repository, by default. // If each of these main fields (name, version, repository) is not supplied by dep param, default value will be used. func checkBuildWithOptionalFields(t *testing.T, chartName string, dep chart.Dependency) { + t.Helper() // Set up a fake repo srv := repotest.NewTempServer( t, diff --git a/pkg/downloader/testdata/repository/testing-relative-index.yaml b/pkg/downloader/testdata/repository/testing-relative-index.yaml index ba27ed257..9524daf6e 100644 --- a/pkg/downloader/testdata/repository/testing-relative-index.yaml +++ b/pkg/downloader/testdata/repository/testing-relative-index.yaml @@ -26,3 +26,16 @@ entries: version: 1.2.3 checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d apiVersion: v2 + baz: + - name: baz + description: Baz Chart With Absolute Path + home: https://helm.sh/helm + keywords: [] + maintainers: [] + sources: + - https://github.com/helm/charts + urls: + - /path/to/baz-1.2.3.tgz + version: 1.2.3 + checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d + apiVersion: v2 diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 839ad4a31..6e47a0e39 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -20,6 +20,7 @@ import ( "errors" "fmt" "log/slog" + "maps" "path" "path/filepath" "regexp" @@ -33,6 +34,18 @@ import ( chartutil "helm.sh/helm/v4/pkg/chart/v2/util" ) +// taken from https://cs.opensource.google/go/go/+/refs/tags/go1.23.6:src/text/template/exec.go;l=141 +// > "template: %s: executing %q at <%s>: %s" +var execErrFmt = regexp.MustCompile(`^template: (?P(?U).+): executing (?P(?U).+) at (?P(?U).+): (?P(?U).+)(?P( template:.*)?)$`) + +// taken from https://cs.opensource.google/go/go/+/refs/tags/go1.23.6:src/text/template/exec.go;l=138 +// > "template: %s: %s" +var execErrFmtWithoutTemplate = regexp.MustCompile(`^template: (?P(?U).+): (?P.*)(?P( template:.*)?)$`) + +// taken from https://cs.opensource.google/go/go/+/refs/tags/go1.23.6:src/text/template/exec.go;l=191 +// > "template: no template %q associated with template %q" +var execErrNoTemplateAssociated = regexp.MustCompile(`^template: no template (?P.*) associated with template (?P(.*)?)$`) + // Engine is an implementation of the Helm rendering implementation for templates. type Engine struct { // If strict is enabled, template rendering will fail if a template references @@ -249,9 +262,7 @@ func (e Engine) initFunMap(t *template.Template) { } // Set custom template funcs - for k, v := range e.CustomTemplateFuncs { - funcMap[k] = v - } + maps.Copy(funcMap, e.CustomTemplateFuncs) t.Funcs(funcMap) } @@ -304,7 +315,7 @@ func (e Engine) render(tpls map[string]renderable) (rendered map[string]string, vals["Template"] = chartutil.Values{"Name": filename, "BasePath": tpls[filename].basePath} var buf strings.Builder if err := t.ExecuteTemplate(&buf, filename, vals); err != nil { - return map[string]string{}, cleanupExecError(filename, err) + return map[string]string{}, reformatExecErrorMsg(filename, err) } // Work around the issue where Go will emit "" even if Options(missing=zero) @@ -330,7 +341,33 @@ func cleanupParseError(filename string, err error) error { return fmt.Errorf("parse error at (%s): %s", string(location), errMsg) } -func cleanupExecError(filename string, err error) error { +type TraceableError struct { + location string + message string + executedFunction string +} + +func (t TraceableError) String() string { + var errorString strings.Builder + if t.location != "" { + fmt.Fprintf(&errorString, "%s\n ", t.location) + } + if t.executedFunction != "" { + fmt.Fprintf(&errorString, "%s\n ", t.executedFunction) + } + if t.message != "" { + fmt.Fprintf(&errorString, "%s\n", t.message) + } + return errorString.String() +} + +// reformatExecErrorMsg takes an error message for template rendering and formats it into a formatted +// multi-line error string +func reformatExecErrorMsg(filename string, err error) error { + // This function matches the error message against regex's for the text/template package. + // If the regex's can parse out details from that error message such as the line number, template it failed on, + // and error description, then it will construct a new error that displays these details in a structured way. + // If there are issues with parsing the error message, the err passed into the function should return instead. if _, isExecError := err.(template.ExecError); !isExecError { return err } @@ -349,8 +386,46 @@ func cleanupExecError(filename string, err error) error { if len(parts) >= 2 { return fmt.Errorf("execution error at (%s): %s", string(location), parts[1]) } + current := err + fileLocations := []TraceableError{} + for current != nil { + var traceable TraceableError + if matches := execErrFmt.FindStringSubmatch(current.Error()); matches != nil { + templateName := matches[execErrFmt.SubexpIndex("templateName")] + functionName := matches[execErrFmt.SubexpIndex("functionName")] + locationName := matches[execErrFmt.SubexpIndex("location")] + errMsg := matches[execErrFmt.SubexpIndex("errMsg")] + traceable = TraceableError{ + location: templateName, + message: errMsg, + executedFunction: "executing " + functionName + " at " + locationName + ":", + } + } else if matches := execErrFmtWithoutTemplate.FindStringSubmatch(current.Error()); matches != nil { + templateName := matches[execErrFmt.SubexpIndex("templateName")] + errMsg := matches[execErrFmt.SubexpIndex("errMsg")] + traceable = TraceableError{ + location: templateName, + message: errMsg, + } + } else if matches := execErrNoTemplateAssociated.FindStringSubmatch(current.Error()); matches != nil { + traceable = TraceableError{ + message: current.Error(), + } + } else { + return err + } + if len(fileLocations) == 0 || fileLocations[len(fileLocations)-1] != traceable { + fileLocations = append(fileLocations, traceable) + } + current = errors.Unwrap(current) + } + + var finalErrorString strings.Builder + for _, fileLocation := range fileLocations { + fmt.Fprintf(&finalErrorString, "%s", fileLocation.String()) + } - return err + return errors.New(strings.TrimSpace(finalErrorString.String())) } func sortTemplates(tpls map[string]renderable) []string { diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index 68e0158fa..f4228fbd7 100644 --- a/pkg/engine/engine_test.go +++ b/pkg/engine/engine_test.go @@ -24,6 +24,8 @@ import ( "testing" "text/template" + "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -1289,16 +1291,82 @@ func TestRenderTplMissingKeyString(t *testing.T) { t.Errorf("Expected error, got %v", out) return } - switch err.(type) { - case (template.ExecError): - errTxt := fmt.Sprint(err) - if !strings.Contains(errTxt, "noSuchKey") { - t.Errorf("Expected error to contain 'noSuchKey', got %s", errTxt) - } - default: - // Some unexpected error. - t.Fatal(err) + errTxt := fmt.Sprint(err) + if !strings.Contains(errTxt, "noSuchKey") { + t.Errorf("Expected error to contain 'noSuchKey', got %s", errTxt) + } + +} + +func TestNestedHelpersProducesMultilineStacktrace(t *testing.T) { + c := &chart.Chart{ + Metadata: &chart.Metadata{Name: "NestedHelperFunctions"}, + Templates: []*chart.File{ + {Name: "templates/svc.yaml", Data: []byte( + `name: {{ include "nested_helper.name" . }}`, + )}, + {Name: "templates/_helpers_1.tpl", Data: []byte( + `{{- define "nested_helper.name" -}}{{- include "common.names.get_name" . -}}{{- end -}}`, + )}, + {Name: "charts/common/templates/_helpers_2.tpl", Data: []byte( + `{{- define "common.names.get_name" -}}{{- .Values.nonexistant.key | trunc 63 | trimSuffix "-" -}}{{- end -}}`, + )}, + }, + } + + expectedErrorMessage := `NestedHelperFunctions/templates/svc.yaml:1:9 + executing "NestedHelperFunctions/templates/svc.yaml" at : + error calling include: +NestedHelperFunctions/templates/_helpers_1.tpl:1:39 + executing "nested_helper.name" at : + error calling include: +NestedHelperFunctions/charts/common/templates/_helpers_2.tpl:1:49 + executing "common.names.get_name" at <.Values.nonexistant.key>: + nil pointer evaluating interface {}.key` + + v := chartutil.Values{} + + val, _ := chartutil.CoalesceValues(c, v) + vals := map[string]interface{}{ + "Values": val.AsMap(), } + _, err := Render(c, vals) + + assert.NotNil(t, err) + assert.Equal(t, expectedErrorMessage, err.Error()) +} + +func TestMultilineNoTemplateAssociatedError(t *testing.T) { + c := &chart.Chart{ + Metadata: &chart.Metadata{Name: "multiline"}, + Templates: []*chart.File{ + {Name: "templates/svc.yaml", Data: []byte( + `name: {{ include "nested_helper.name" . }}`, + )}, + {Name: "templates/test.yaml", Data: []byte( + `{{ toYaml .Values }}`, + )}, + {Name: "charts/common/templates/_helpers_2.tpl", Data: []byte( + `{{ toYaml .Values }}`, + )}, + }, + } + + expectedErrorMessage := `multiline/templates/svc.yaml:1:9 + executing "multiline/templates/svc.yaml" at : + error calling include: +template: no template "nested_helper.name" associated with template "gotpl"` + + v := chartutil.Values{} + + val, _ := chartutil.CoalesceValues(c, v) + vals := map[string]interface{}{ + "Values": val.AsMap(), + } + _, err := Render(c, vals) + + assert.NotNil(t, err) + assert.Equal(t, expectedErrorMessage, err.Error()) } func TestRenderCustomTemplateFuncs(t *testing.T) { diff --git a/pkg/engine/funcs.go b/pkg/engine/funcs.go index d03a818c2..a97f8f104 100644 --- a/pkg/engine/funcs.go +++ b/pkg/engine/funcs.go @@ -19,6 +19,7 @@ package engine import ( "bytes" "encoding/json" + "maps" "strings" "text/template" @@ -51,10 +52,12 @@ func funcMap() template.FuncMap { "toToml": toTOML, "fromToml": fromTOML, "toYaml": toYAML, + "mustToYaml": mustToYAML, "toYamlPretty": toYAMLPretty, "fromYaml": fromYAML, "fromYamlArray": fromYAMLArray, "toJson": toJSON, + "mustToJson": mustToJSON, "fromJson": fromJSON, "fromJsonArray": fromJSONArray, @@ -71,9 +74,7 @@ func funcMap() template.FuncMap { }, } - for k, v := range extra { - f[k] = v - } + maps.Copy(f, extra) return f } @@ -91,6 +92,19 @@ func toYAML(v interface{}) string { return strings.TrimSuffix(string(data), "\n") } +// mustToYAML takes an interface, marshals it to yaml, and returns a string. +// It will panic if there is an error. +// +// This is designed to be called from a template when need to ensure that the +// output YAML is valid. +func mustToYAML(v interface{}) string { + data, err := yaml.Marshal(v) + if err != nil { + panic(err) + } + return strings.TrimSuffix(string(data), "\n") +} + func toYAMLPretty(v interface{}) string { var data bytes.Buffer encoder := goYaml.NewEncoder(&data) @@ -176,6 +190,19 @@ func toJSON(v interface{}) string { return string(data) } +// mustToJSON takes an interface, marshals it to json, and returns a string. +// It will panic if there is an error. +// +// This is designed to be called from a template when need to ensure that the +// output JSON is valid. +func mustToJSON(v interface{}) string { + data, err := json.Marshal(v) + if err != nil { + panic(err) + } + return string(data) +} + // fromJSON converts a JSON document into a map[string]interface{}. // // This is not a general-purpose JSON parser, and will not parse all valid diff --git a/pkg/engine/funcs_test.go b/pkg/engine/funcs_test.go index a7e2506a3..71a72e2e4 100644 --- a/pkg/engine/funcs_test.go +++ b/pkg/engine/funcs_test.go @@ -135,6 +135,43 @@ keyInElement1 = "valueInElement1"`, assert.NoError(t, err) assert.Equal(t, tt.expect, b.String(), tt.tpl) } + + loopMap := map[string]interface{}{ + "foo": "bar", + } + loopMap["loop"] = []interface{}{loopMap} + + mustFuncsTests := []struct { + tpl string + expect interface{} + vars interface{} + }{{ + tpl: `{{ mustToYaml . }}`, + vars: loopMap, + }, { + tpl: `{{ mustToJson . }}`, + vars: loopMap, + }, { + tpl: `{{ toYaml . }}`, + expect: "", // should return empty string and swallow error + vars: loopMap, + }, { + tpl: `{{ toJson . }}`, + expect: "", // should return empty string and swallow error + vars: loopMap, + }, + } + + for _, tt := range mustFuncsTests { + var b strings.Builder + err := template.Must(template.New("test").Funcs(funcMap()).Parse(tt.tpl)).Execute(&b, tt.vars) + if tt.expect != nil { + assert.NoError(t, err) + assert.Equal(t, tt.expect, b.String(), tt.tpl) + } else { + assert.Error(t, err) + } + } } // This test to check a function provided by sprig is due to a change in a diff --git a/pkg/gates/gates_test.go b/pkg/gates/gates_test.go index 6bdd17ed6..4d77199e6 100644 --- a/pkg/gates/gates_test.go +++ b/pkg/gates/gates_test.go @@ -23,14 +23,13 @@ import ( const name string = "HELM_EXPERIMENTAL_FEATURE" func TestIsEnabled(t *testing.T) { - os.Unsetenv(name) g := Gate(name) if g.IsEnabled() { t.Errorf("feature gate shows as available, but the environment variable %s was not set", name) } - os.Setenv(name, "1") + t.Setenv(name, "1") if !g.IsEnabled() { t.Errorf("feature gate shows as disabled, but the environment variable %s was set", name) diff --git a/pkg/getter/getter.go b/pkg/getter/getter.go index 743ac569b..5605e043f 100644 --- a/pkg/getter/getter.go +++ b/pkg/getter/getter.go @@ -20,6 +20,7 @@ import ( "bytes" "fmt" "net/http" + "slices" "time" "helm.sh/helm/v4/pkg/cli" @@ -163,12 +164,7 @@ type Provider struct { // Provides returns true if the given scheme is supported by this Provider. func (p Provider) Provides(scheme string) bool { - for _, i := range p.Schemes { - if i == scheme { - return true - } - } - return false + return slices.Contains(p.Schemes, scheme) } // Providers is a collection of Provider objects. @@ -195,24 +191,32 @@ const ( var defaultOptions = []Option{WithTimeout(time.Second * DefaultHTTPTimeout)} -var httpProvider = Provider{ - Schemes: []string{"http", "https"}, - New: func(options ...Option) (Getter, error) { - options = append(options, defaultOptions...) - return NewHTTPGetter(options...) - }, -} - -var ociProvider = Provider{ - Schemes: []string{registry.OCIScheme}, - New: NewOCIGetter, +func Getters(extraOpts ...Option) Providers { + return Providers{ + Provider{ + Schemes: []string{"http", "https"}, + New: func(options ...Option) (Getter, error) { + options = append(options, defaultOptions...) + options = append(options, extraOpts...) + return NewHTTPGetter(options...) + }, + }, + Provider{ + Schemes: []string{registry.OCIScheme}, + New: func(options ...Option) (Getter, error) { + options = append(options, defaultOptions...) + options = append(options, extraOpts...) + return NewOCIGetter(options...) + }, + }, + } } // All finds all of the registered getters as a list of Provider instances. // Currently, the built-in getters and the discovered plugins with downloader // notations are collected. -func All(settings *cli.EnvSettings) Providers { - result := Providers{httpProvider, ociProvider} +func All(settings *cli.EnvSettings, opts ...Option) Providers { + result := Getters(opts...) pluginDownloaders, _ := collectPlugins(settings) result = append(result, pluginDownloaders...) return result diff --git a/pkg/getter/getter_test.go b/pkg/getter/getter_test.go index a14301900..83920e809 100644 --- a/pkg/getter/getter_test.go +++ b/pkg/getter/getter_test.go @@ -17,6 +17,7 @@ package getter import ( "testing" + "time" "helm.sh/helm/v4/pkg/cli" ) @@ -52,6 +53,23 @@ func TestProviders(t *testing.T) { } } +func TestProvidersWithTimeout(t *testing.T) { + want := time.Hour + getters := Getters(WithTimeout(want)) + getter, err := getters.ByScheme("http") + if err != nil { + t.Error(err) + } + client, err := getter.(*HTTPGetter).httpClient() + if err != nil { + t.Error(err) + } + got := client.Timeout + if got != want { + t.Errorf("Expected %q, got %q", want, got) + } +} + func TestAll(t *testing.T) { env := cli.New() env.PluginsDirectory = pluginDir diff --git a/pkg/getter/httpgetter_test.go b/pkg/getter/httpgetter_test.go index 510fffd13..a997c7f03 100644 --- a/pkg/getter/httpgetter_test.go +++ b/pkg/getter/httpgetter_test.go @@ -576,6 +576,7 @@ func TestHttpClientInsecureSkipVerify(t *testing.T) { } func verifyInsecureSkipVerify(t *testing.T, g *HTTPGetter, caseName string, expectedValue bool) *http.Transport { + t.Helper() returnVal, err := g.httpClient() if err != nil { diff --git a/pkg/helmpath/home_unix_test.go b/pkg/helmpath/home_unix_test.go index 6e4189bc9..a64c9bcd6 100644 --- a/pkg/helmpath/home_unix_test.go +++ b/pkg/helmpath/home_unix_test.go @@ -16,7 +16,6 @@ package helmpath import ( - "os" "runtime" "testing" @@ -24,9 +23,9 @@ import ( ) func TestHelmHome(t *testing.T) { - os.Setenv(xdg.CacheHomeEnvVar, "/cache") - os.Setenv(xdg.ConfigHomeEnvVar, "/config") - os.Setenv(xdg.DataHomeEnvVar, "/data") + t.Setenv(xdg.CacheHomeEnvVar, "/cache") + t.Setenv(xdg.ConfigHomeEnvVar, "/config") + t.Setenv(xdg.DataHomeEnvVar, "/data") isEq := func(t *testing.T, got, expected string) { t.Helper() if expected != got { @@ -40,7 +39,7 @@ func TestHelmHome(t *testing.T) { isEq(t, DataPath(), "/data/helm") // test to see if lazy-loading environment variables at runtime works - os.Setenv(xdg.CacheHomeEnvVar, "/cache2") + t.Setenv(xdg.CacheHomeEnvVar, "/cache2") isEq(t, CachePath(), "/cache2/helm") } diff --git a/pkg/helmpath/lazypath_darwin_test.go b/pkg/helmpath/lazypath_darwin_test.go index e04e20756..e3006d0d5 100644 --- a/pkg/helmpath/lazypath_darwin_test.go +++ b/pkg/helmpath/lazypath_darwin_test.go @@ -40,7 +40,7 @@ func TestDataPath(t *testing.T) { t.Errorf("expected '%s', got '%s'", expected, lazy.dataPath(testFile)) } - os.Setenv(xdg.DataHomeEnvVar, "/tmp") + t.Setenv(xdg.DataHomeEnvVar, "/tmp") expected = filepath.Join("/tmp", appName, testFile) @@ -58,7 +58,7 @@ func TestConfigPath(t *testing.T) { t.Errorf("expected '%s', got '%s'", expected, lazy.configPath(testFile)) } - os.Setenv(xdg.ConfigHomeEnvVar, "/tmp") + t.Setenv(xdg.ConfigHomeEnvVar, "/tmp") expected = filepath.Join("/tmp", appName, testFile) @@ -76,7 +76,7 @@ func TestCachePath(t *testing.T) { t.Errorf("expected '%s', got '%s'", expected, lazy.cachePath(testFile)) } - os.Setenv(xdg.CacheHomeEnvVar, "/tmp") + t.Setenv(xdg.CacheHomeEnvVar, "/tmp") expected = filepath.Join("/tmp", appName, testFile) diff --git a/pkg/helmpath/lazypath_unix_test.go b/pkg/helmpath/lazypath_unix_test.go index 534735d10..4b0f2429b 100644 --- a/pkg/helmpath/lazypath_unix_test.go +++ b/pkg/helmpath/lazypath_unix_test.go @@ -16,7 +16,6 @@ package helmpath import ( - "os" "path/filepath" "testing" @@ -32,15 +31,13 @@ const ( ) func TestDataPath(t *testing.T) { - os.Unsetenv(xdg.DataHomeEnvVar) - expected := filepath.Join(homedir.HomeDir(), ".local", "share", appName, testFile) if lazy.dataPath(testFile) != expected { t.Errorf("expected '%s', got '%s'", expected, lazy.dataPath(testFile)) } - os.Setenv(xdg.DataHomeEnvVar, "/tmp") + t.Setenv(xdg.DataHomeEnvVar, "/tmp") expected = filepath.Join("/tmp", appName, testFile) @@ -50,15 +47,13 @@ func TestDataPath(t *testing.T) { } func TestConfigPath(t *testing.T) { - os.Unsetenv(xdg.ConfigHomeEnvVar) - expected := filepath.Join(homedir.HomeDir(), ".config", appName, testFile) if lazy.configPath(testFile) != expected { t.Errorf("expected '%s', got '%s'", expected, lazy.configPath(testFile)) } - os.Setenv(xdg.ConfigHomeEnvVar, "/tmp") + t.Setenv(xdg.ConfigHomeEnvVar, "/tmp") expected = filepath.Join("/tmp", appName, testFile) @@ -68,15 +63,13 @@ func TestConfigPath(t *testing.T) { } func TestCachePath(t *testing.T) { - os.Unsetenv(xdg.CacheHomeEnvVar) - expected := filepath.Join(homedir.HomeDir(), ".cache", appName, testFile) if lazy.cachePath(testFile) != expected { t.Errorf("expected '%s', got '%s'", expected, lazy.cachePath(testFile)) } - os.Setenv(xdg.CacheHomeEnvVar, "/tmp") + t.Setenv(xdg.CacheHomeEnvVar, "/tmp") expected = filepath.Join("/tmp", appName, testFile) diff --git a/pkg/ignore/rules.go b/pkg/ignore/rules.go index 5281c3d59..3511c2d40 100644 --- a/pkg/ignore/rules.go +++ b/pkg/ignore/rules.go @@ -170,10 +170,10 @@ func (r *Rules) parseRule(rule string) error { rule = strings.TrimSuffix(rule, "/") } - if strings.HasPrefix(rule, "/") { + if after, ok := strings.CutPrefix(rule, "/"); ok { // Require path matches the root path. p.match = func(n string, _ os.FileInfo) bool { - rule = strings.TrimPrefix(rule, "/") + rule = after ok, err := filepath.Match(rule, n) if err != nil { slog.Error("failed to compile", "rule", rule, slog.Any("error", err)) diff --git a/pkg/kube/client.go b/pkg/kube/client.go index a812fc198..78ed4e088 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -30,7 +30,7 @@ import ( "strings" "sync" - jsonpatch "github.com/evanphx/json-patch" + jsonpatch "github.com/evanphx/json-patch/v5" v1 "k8s.io/api/core/v1" apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" @@ -585,10 +585,14 @@ func batchPerform(infos ResourceList, fn func(*resource.Info) error, errs chan<- } } +var createMutex sync.Mutex + func createResource(info *resource.Info) error { return retry.RetryOnConflict( retry.DefaultRetry, func() error { + createMutex.Lock() + defer createMutex.Unlock() obj, err := resource.NewHelper(info.Client, info.Mapping).WithFieldManager(getManagedFieldsManager()).Create(info.Namespace, true, info.Object) if err != nil { return err diff --git a/pkg/kube/client_test.go b/pkg/kube/client_test.go index 56c7eebc9..cd83a7f9e 100644 --- a/pkg/kube/client_test.go +++ b/pkg/kube/client_test.go @@ -41,8 +41,10 @@ import ( cmdtesting "k8s.io/kubectl/pkg/cmd/testing" ) -var unstructuredSerializer = resource.UnstructuredPlusDefaultContentConfig().NegotiatedSerializer -var codec = scheme.Codecs.LegacyCodec(scheme.Scheme.PrioritizedVersionsAllGroups()...) +var ( + unstructuredSerializer = resource.UnstructuredPlusDefaultContentConfig().NegotiatedSerializer + codec = scheme.Codecs.LegacyCodec(scheme.Scheme.PrioritizedVersionsAllGroups()...) +) func objBody(obj runtime.Object) io.ReadCloser { return io.NopCloser(bytes.NewReader([]byte(runtime.EncodeOrDie(codec, obj)))) @@ -107,6 +109,7 @@ func newResponseJSON(code int, json []byte) (*http.Response, error) { } func newTestClient(t *testing.T) *Client { + t.Helper() testFactory := cmdtesting.NewTestFactory() t.Cleanup(testFactory.Cleanup) @@ -138,15 +141,15 @@ func TestCreate(t *testing.T) { actions = append(actions, path+":"+method) t.Logf("got request %s %s", path, method) switch { - case path == "/namespaces/default/pods" && method == "POST": + case path == "/namespaces/default/pods" && method == http.MethodPost: if strings.Contains(body, "starfish") { if iterationCounter < 2 { iterationCounter++ - return newResponseJSON(409, resourceQuotaConflict) + return newResponseJSON(http.StatusConflict, resourceQuotaConflict) } - return newResponse(200, &listA.Items[0]) + return newResponse(http.StatusOK, &listA.Items[0]) } - return newResponseJSON(409, resourceQuotaConflict) + return newResponseJSON(http.StatusConflict, resourceQuotaConflict) default: t.Fatalf("unexpected request: %s %s", method, path) return nil, nil @@ -213,6 +216,7 @@ func TestCreate(t *testing.T) { } func testUpdate(t *testing.T, threeWayMerge bool) { + t.Helper() listA := newPodList("starfish", "otter", "squid") listB := newPodList("starfish", "otter", "dolphin") listC := newPodList("starfish", "otter", "dolphin") @@ -230,11 +234,11 @@ func testUpdate(t *testing.T, threeWayMerge bool) { actions = append(actions, p+":"+m) t.Logf("got request %s %s", p, m) switch { - case p == "/namespaces/default/pods/starfish" && m == "GET": - return newResponse(200, &listA.Items[0]) - case p == "/namespaces/default/pods/otter" && m == "GET": - return newResponse(200, &listA.Items[1]) - case p == "/namespaces/default/pods/otter" && m == "PATCH": + case p == "/namespaces/default/pods/starfish" && m == http.MethodGet: + return newResponse(http.StatusOK, &listA.Items[0]) + case p == "/namespaces/default/pods/otter" && m == http.MethodGet: + return newResponse(http.StatusOK, &listA.Items[1]) + case p == "/namespaces/default/pods/otter" && m == http.MethodPatch: data, err := io.ReadAll(req.Body) if err != nil { t.Fatalf("could not dump request: %s", err) @@ -244,10 +248,10 @@ func testUpdate(t *testing.T, threeWayMerge bool) { if string(data) != expected { t.Errorf("expected patch\n%s\ngot\n%s", expected, string(data)) } - return newResponse(200, &listB.Items[0]) - case p == "/namespaces/default/pods/dolphin" && m == "GET": - return newResponse(404, notFoundBody()) - case p == "/namespaces/default/pods/starfish" && m == "PATCH": + return newResponse(http.StatusOK, &listB.Items[0]) + case p == "/namespaces/default/pods/dolphin" && m == http.MethodGet: + return newResponse(http.StatusNotFound, notFoundBody()) + case p == "/namespaces/default/pods/starfish" && m == http.MethodPatch: data, err := io.ReadAll(req.Body) if err != nil { t.Fatalf("could not dump request: %s", err) @@ -257,17 +261,17 @@ func testUpdate(t *testing.T, threeWayMerge bool) { if string(data) != expected { t.Errorf("expected patch\n%s\ngot\n%s", expected, string(data)) } - return newResponse(200, &listB.Items[0]) - case p == "/namespaces/default/pods" && m == "POST": + return newResponse(http.StatusOK, &listB.Items[0]) + case p == "/namespaces/default/pods" && m == http.MethodPost: if iterationCounter < 2 { iterationCounter++ - return newResponseJSON(409, resourceQuotaConflict) + return newResponseJSON(http.StatusConflict, resourceQuotaConflict) } - return newResponse(200, &listB.Items[1]) - case p == "/namespaces/default/pods/squid" && m == "DELETE": - return newResponse(200, &listB.Items[1]) - case p == "/namespaces/default/pods/squid" && m == "GET": - return newResponse(200, &listB.Items[2]) + return newResponse(http.StatusOK, &listB.Items[1]) + case p == "/namespaces/default/pods/squid" && m == http.MethodDelete: + return newResponse(http.StatusOK, &listB.Items[1]) + case p == "/namespaces/default/pods/squid" && m == http.MethodGet: + return newResponse(http.StatusOK, &listB.Items[2]) default: t.Fatalf("unexpected request: %s %s", req.Method, req.URL.Path) return nil, nil @@ -485,7 +489,7 @@ func TestWait(t *testing.T) { p, m := req.URL.Path, req.Method t.Logf("got request %s %s", p, m) switch { - case p == "/api/v1/namespaces/default/pods/starfish" && m == "GET": + case p == "/api/v1/namespaces/default/pods/starfish" && m == http.MethodGet: pod := &podList.Items[0] if created != nil && time.Since(*created) >= time.Second*5 { pod.Status.Conditions = []v1.PodCondition{ @@ -495,8 +499,8 @@ func TestWait(t *testing.T) { }, } } - return newResponse(200, pod) - case p == "/api/v1/namespaces/default/pods/otter" && m == "GET": + return newResponse(http.StatusOK, pod) + case p == "/api/v1/namespaces/default/pods/otter" && m == http.MethodGet: pod := &podList.Items[1] if created != nil && time.Since(*created) >= time.Second*5 { pod.Status.Conditions = []v1.PodCondition{ @@ -506,8 +510,8 @@ func TestWait(t *testing.T) { }, } } - return newResponse(200, pod) - case p == "/api/v1/namespaces/default/pods/squid" && m == "GET": + return newResponse(http.StatusOK, pod) + case p == "/api/v1/namespaces/default/pods/squid" && m == http.MethodGet: pod := &podList.Items[2] if created != nil && time.Since(*created) >= time.Second*5 { pod.Status.Conditions = []v1.PodCondition{ @@ -517,15 +521,15 @@ func TestWait(t *testing.T) { }, } } - return newResponse(200, pod) - case p == "/namespaces/default/pods" && m == "POST": + return newResponse(http.StatusOK, pod) + case p == "/namespaces/default/pods" && m == http.MethodPost: resources, err := c.Build(req.Body, false) if err != nil { t.Fatal(err) } now := time.Now() created = &now - return newResponse(200, resources[0].Object) + return newResponse(http.StatusOK, resources[0].Object) default: t.Fatalf("unexpected request: %s %s", req.Method, req.URL.Path) return nil, nil @@ -570,19 +574,19 @@ func TestWaitJob(t *testing.T) { p, m := req.URL.Path, req.Method t.Logf("got request %s %s", p, m) switch { - case p == "/apis/batch/v1/namespaces/default/jobs/starfish" && m == "GET": + case p == "/apis/batch/v1/namespaces/default/jobs/starfish" && m == http.MethodGet: if created != nil && time.Since(*created) >= time.Second*5 { job.Status.Succeeded = 1 } - return newResponse(200, job) - case p == "/namespaces/default/jobs" && m == "POST": + return newResponse(http.StatusOK, job) + case p == "/namespaces/default/jobs" && m == http.MethodPost: resources, err := c.Build(req.Body, false) if err != nil { t.Fatal(err) } now := time.Now() created = &now - return newResponse(200, resources[0].Object) + return newResponse(http.StatusOK, resources[0].Object) default: t.Fatalf("unexpected request: %s %s", req.Method, req.URL.Path) return nil, nil @@ -627,21 +631,21 @@ func TestWaitDelete(t *testing.T) { p, m := req.URL.Path, req.Method t.Logf("got request %s %s", p, m) switch { - case p == "/namespaces/default/pods/starfish" && m == "GET": + case p == "/namespaces/default/pods/starfish" && m == http.MethodGet: if deleted != nil && time.Since(*deleted) >= time.Second*5 { - return newResponse(404, notFoundBody()) + return newResponse(http.StatusNotFound, notFoundBody()) } - return newResponse(200, &pod) - case p == "/namespaces/default/pods/starfish" && m == "DELETE": + return newResponse(http.StatusOK, &pod) + case p == "/namespaces/default/pods/starfish" && m == http.MethodDelete: now := time.Now() deleted = &now - return newResponse(200, &pod) - case p == "/namespaces/default/pods" && m == "POST": + return newResponse(http.StatusOK, &pod) + case p == "/namespaces/default/pods" && m == http.MethodPost: resources, err := c.Build(req.Body, false) if err != nil { t.Fatal(err) } - return newResponse(200, resources[0].Object) + return newResponse(http.StatusOK, resources[0].Object) default: t.Fatalf("unexpected request: %s %s", req.Method, req.URL.Path) return nil, nil @@ -718,7 +722,6 @@ func TestReal(t *testing.T) { } func TestGetPodList(t *testing.T) { - namespace := "some-namespace" names := []string{"dave", "jimmy"} var responsePodList v1.PodList @@ -733,7 +736,6 @@ func TestGetPodList(t *testing.T) { clientAssertions := assert.New(t) clientAssertions.NoError(err) clientAssertions.Equal(&responsePodList, podList) - } func TestOutputContainerLogsForPodList(t *testing.T) { @@ -820,11 +822,11 @@ spec: apiVersion: v1 kind: Service metadata: - name: redis-slave + name: redis-replica labels: app: redis tier: backend - role: slave + role: replica spec: ports: # the port that this service should serve on @@ -832,24 +834,24 @@ spec: selector: app: redis tier: backend - role: slave + role: replica --- apiVersion: extensions/v1beta1 kind: Deployment metadata: - name: redis-slave + name: redis-replica spec: replicas: 2 template: metadata: labels: app: redis - role: slave + role: replica tier: backend spec: containers: - - name: slave - image: gcr.io/google_samples/gb-redisslave:v1 + - name: replica + image: gcr.io/google_samples/gb-redisreplica:v1 resources: requests: cpu: 100m @@ -964,7 +966,7 @@ func (c createPatchTestCase) run(t *testing.T) { restClient := &fake.RESTClient{ NegotiatedSerializer: unstructuredSerializer, Resp: &http.Response{ - StatusCode: 200, + StatusCode: http.StatusOK, Body: objBody(c.actual), Header: header, }, diff --git a/pkg/kube/ready_test.go b/pkg/kube/ready_test.go index 9d1dfd272..db0d02cbe 100644 --- a/pkg/kube/ready_test.go +++ b/pkg/kube/ready_test.go @@ -60,7 +60,7 @@ func Test_ReadyChecker_IsReady_Pod(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &corev1.Pod{}, Name: "foo", Namespace: defaultNamespace}, }, pod: newPodWithCondition("foo", corev1.ConditionTrue), @@ -75,7 +75,7 @@ func Test_ReadyChecker_IsReady_Pod(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &corev1.Pod{}, Name: "foo", Namespace: defaultNamespace}, }, pod: newPodWithCondition("bar", corev1.ConditionTrue), @@ -90,7 +90,7 @@ func Test_ReadyChecker_IsReady_Pod(t *testing.T) { checkJobs: tt.fields.checkJobs, pausedAsReady: tt.fields.pausedAsReady, } - if _, err := c.client.CoreV1().Pods(defaultNamespace).Create(context.TODO(), tt.pod, metav1.CreateOptions{}); err != nil { + if _, err := c.client.CoreV1().Pods(defaultNamespace).Create(t.Context(), tt.pod, metav1.CreateOptions{}); err != nil { t.Errorf("Failed to create Pod error: %v", err) return } @@ -132,7 +132,7 @@ func Test_ReadyChecker_IsReady_Job(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &batchv1.Job{}, Name: "foo", Namespace: defaultNamespace}, }, job: newJob("bar", 1, intToInt32(1), 1, 0), @@ -147,7 +147,7 @@ func Test_ReadyChecker_IsReady_Job(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &batchv1.Job{}, Name: "foo", Namespace: defaultNamespace}, }, job: newJob("foo", 1, intToInt32(1), 1, 0), @@ -162,7 +162,7 @@ func Test_ReadyChecker_IsReady_Job(t *testing.T) { checkJobs: tt.fields.checkJobs, pausedAsReady: tt.fields.pausedAsReady, } - if _, err := c.client.BatchV1().Jobs(defaultNamespace).Create(context.TODO(), tt.job, metav1.CreateOptions{}); err != nil { + if _, err := c.client.BatchV1().Jobs(defaultNamespace).Create(t.Context(), tt.job, metav1.CreateOptions{}); err != nil { t.Errorf("Failed to create Job error: %v", err) return } @@ -204,7 +204,7 @@ func Test_ReadyChecker_IsReady_Deployment(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &appsv1.Deployment{}, Name: "foo", Namespace: defaultNamespace}, }, replicaSet: newReplicaSet("foo", 0, 0, true), @@ -220,7 +220,7 @@ func Test_ReadyChecker_IsReady_Deployment(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &appsv1.Deployment{}, Name: "foo", Namespace: defaultNamespace}, }, replicaSet: newReplicaSet("foo", 0, 0, true), @@ -236,11 +236,11 @@ func Test_ReadyChecker_IsReady_Deployment(t *testing.T) { checkJobs: tt.fields.checkJobs, pausedAsReady: tt.fields.pausedAsReady, } - if _, err := c.client.AppsV1().Deployments(defaultNamespace).Create(context.TODO(), tt.deployment, metav1.CreateOptions{}); err != nil { + if _, err := c.client.AppsV1().Deployments(defaultNamespace).Create(t.Context(), tt.deployment, metav1.CreateOptions{}); err != nil { t.Errorf("Failed to create Deployment error: %v", err) return } - if _, err := c.client.AppsV1().ReplicaSets(defaultNamespace).Create(context.TODO(), tt.replicaSet, metav1.CreateOptions{}); err != nil { + if _, err := c.client.AppsV1().ReplicaSets(defaultNamespace).Create(t.Context(), tt.replicaSet, metav1.CreateOptions{}); err != nil { t.Errorf("Failed to create ReplicaSet error: %v", err) return } @@ -281,7 +281,7 @@ func Test_ReadyChecker_IsReady_PersistentVolumeClaim(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &corev1.PersistentVolumeClaim{}, Name: "foo", Namespace: defaultNamespace}, }, pvc: newPersistentVolumeClaim("foo", corev1.ClaimPending), @@ -296,7 +296,7 @@ func Test_ReadyChecker_IsReady_PersistentVolumeClaim(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &corev1.PersistentVolumeClaim{}, Name: "foo", Namespace: defaultNamespace}, }, pvc: newPersistentVolumeClaim("bar", corev1.ClaimPending), @@ -311,7 +311,7 @@ func Test_ReadyChecker_IsReady_PersistentVolumeClaim(t *testing.T) { checkJobs: tt.fields.checkJobs, pausedAsReady: tt.fields.pausedAsReady, } - if _, err := c.client.CoreV1().PersistentVolumeClaims(defaultNamespace).Create(context.TODO(), tt.pvc, metav1.CreateOptions{}); err != nil { + if _, err := c.client.CoreV1().PersistentVolumeClaims(defaultNamespace).Create(t.Context(), tt.pvc, metav1.CreateOptions{}); err != nil { t.Errorf("Failed to create PersistentVolumeClaim error: %v", err) return } @@ -352,7 +352,7 @@ func Test_ReadyChecker_IsReady_Service(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &corev1.Service{}, Name: "foo", Namespace: defaultNamespace}, }, svc: newService("foo", corev1.ServiceSpec{Type: corev1.ServiceTypeLoadBalancer, ClusterIP: ""}), @@ -367,7 +367,7 @@ func Test_ReadyChecker_IsReady_Service(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &corev1.Service{}, Name: "foo", Namespace: defaultNamespace}, }, svc: newService("bar", corev1.ServiceSpec{Type: corev1.ServiceTypeExternalName, ClusterIP: ""}), @@ -382,7 +382,7 @@ func Test_ReadyChecker_IsReady_Service(t *testing.T) { checkJobs: tt.fields.checkJobs, pausedAsReady: tt.fields.pausedAsReady, } - if _, err := c.client.CoreV1().Services(defaultNamespace).Create(context.TODO(), tt.svc, metav1.CreateOptions{}); err != nil { + if _, err := c.client.CoreV1().Services(defaultNamespace).Create(t.Context(), tt.svc, metav1.CreateOptions{}); err != nil { t.Errorf("Failed to create Service error: %v", err) return } @@ -423,7 +423,7 @@ func Test_ReadyChecker_IsReady_DaemonSet(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &appsv1.DaemonSet{}, Name: "foo", Namespace: defaultNamespace}, }, ds: newDaemonSet("foo", 0, 0, 1, 0, true), @@ -438,7 +438,7 @@ func Test_ReadyChecker_IsReady_DaemonSet(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &appsv1.DaemonSet{}, Name: "foo", Namespace: defaultNamespace}, }, ds: newDaemonSet("bar", 0, 1, 1, 1, true), @@ -453,7 +453,7 @@ func Test_ReadyChecker_IsReady_DaemonSet(t *testing.T) { checkJobs: tt.fields.checkJobs, pausedAsReady: tt.fields.pausedAsReady, } - if _, err := c.client.AppsV1().DaemonSets(defaultNamespace).Create(context.TODO(), tt.ds, metav1.CreateOptions{}); err != nil { + if _, err := c.client.AppsV1().DaemonSets(defaultNamespace).Create(t.Context(), tt.ds, metav1.CreateOptions{}); err != nil { t.Errorf("Failed to create DaemonSet error: %v", err) return } @@ -494,7 +494,7 @@ func Test_ReadyChecker_IsReady_StatefulSet(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &appsv1.StatefulSet{}, Name: "foo", Namespace: defaultNamespace}, }, ss: newStatefulSet("foo", 1, 0, 0, 1, true), @@ -509,7 +509,7 @@ func Test_ReadyChecker_IsReady_StatefulSet(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &appsv1.StatefulSet{}, Name: "foo", Namespace: defaultNamespace}, }, ss: newStatefulSet("bar", 1, 0, 1, 1, true), @@ -524,7 +524,7 @@ func Test_ReadyChecker_IsReady_StatefulSet(t *testing.T) { checkJobs: tt.fields.checkJobs, pausedAsReady: tt.fields.pausedAsReady, } - if _, err := c.client.AppsV1().StatefulSets(defaultNamespace).Create(context.TODO(), tt.ss, metav1.CreateOptions{}); err != nil { + if _, err := c.client.AppsV1().StatefulSets(defaultNamespace).Create(t.Context(), tt.ss, metav1.CreateOptions{}); err != nil { t.Errorf("Failed to create StatefulSet error: %v", err) return } @@ -565,7 +565,7 @@ func Test_ReadyChecker_IsReady_ReplicationController(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &corev1.ReplicationController{}, Name: "foo", Namespace: defaultNamespace}, }, rc: newReplicationController("foo", false), @@ -580,7 +580,7 @@ func Test_ReadyChecker_IsReady_ReplicationController(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &corev1.ReplicationController{}, Name: "foo", Namespace: defaultNamespace}, }, rc: newReplicationController("bar", false), @@ -595,7 +595,7 @@ func Test_ReadyChecker_IsReady_ReplicationController(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &corev1.ReplicationController{}, Name: "foo", Namespace: defaultNamespace}, }, rc: newReplicationController("foo", true), @@ -610,7 +610,7 @@ func Test_ReadyChecker_IsReady_ReplicationController(t *testing.T) { checkJobs: tt.fields.checkJobs, pausedAsReady: tt.fields.pausedAsReady, } - if _, err := c.client.CoreV1().ReplicationControllers(defaultNamespace).Create(context.TODO(), tt.rc, metav1.CreateOptions{}); err != nil { + if _, err := c.client.CoreV1().ReplicationControllers(defaultNamespace).Create(t.Context(), tt.rc, metav1.CreateOptions{}); err != nil { t.Errorf("Failed to create ReplicationController error: %v", err) return } @@ -651,7 +651,7 @@ func Test_ReadyChecker_IsReady_ReplicaSet(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &appsv1.ReplicaSet{}, Name: "foo", Namespace: defaultNamespace}, }, rs: newReplicaSet("foo", 1, 1, true), @@ -666,7 +666,7 @@ func Test_ReadyChecker_IsReady_ReplicaSet(t *testing.T) { pausedAsReady: false, }, args: args{ - ctx: context.TODO(), + ctx: t.Context(), resource: &resource.Info{Object: &appsv1.ReplicaSet{}, Name: "foo", Namespace: defaultNamespace}, }, rs: newReplicaSet("bar", 1, 1, false), @@ -1014,12 +1014,12 @@ func Test_ReadyChecker_podsReadyForObject(t *testing.T) { t.Run(tt.name, func(t *testing.T) { c := NewReadyChecker(fake.NewClientset()) for _, pod := range tt.existPods { - if _, err := c.client.CoreV1().Pods(defaultNamespace).Create(context.TODO(), &pod, metav1.CreateOptions{}); err != nil { + if _, err := c.client.CoreV1().Pods(defaultNamespace).Create(t.Context(), &pod, metav1.CreateOptions{}); err != nil { t.Errorf("Failed to create Pod error: %v", err) return } } - got, err := c.podsReadyForObject(context.TODO(), tt.args.namespace, tt.args.obj) + got, err := c.podsReadyForObject(t.Context(), tt.args.namespace, tt.args.obj) if (err != nil) != tt.wantErr { t.Errorf("podsReadyForObject() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/pkg/kube/resource.go b/pkg/kube/resource.go index 600f256b3..d88b171f0 100644 --- a/pkg/kube/resource.go +++ b/pkg/kube/resource.go @@ -81,5 +81,5 @@ func (r ResourceList) Intersect(rs ResourceList) ResourceList { // isMatchingInfo returns true if infos match on Name and GroupVersionKind. func isMatchingInfo(a, b *resource.Info) bool { - return a.Name == b.Name && a.Namespace == b.Namespace && a.Mapping.GroupVersionKind.Kind == b.Mapping.GroupVersionKind.Kind && a.Mapping.GroupVersionKind.Group == b.Mapping.GroupVersionKind.Group + return a.Name == b.Name && a.Namespace == b.Namespace && a.Mapping.GroupVersionKind == b.Mapping.GroupVersionKind } diff --git a/pkg/kube/resource_test.go b/pkg/kube/resource_test.go index c405ca382..ccc613c1b 100644 --- a/pkg/kube/resource_test.go +++ b/pkg/kube/resource_test.go @@ -59,3 +59,42 @@ func TestResourceList(t *testing.T) { t.Error("expected intersect to return bar") } } + +func TestIsMatchingInfo(t *testing.T) { + gvk := schema.GroupVersionKind{Group: "group1", Version: "version1", Kind: "pod"} + resourceInfo := resource.Info{Name: "name1", Namespace: "namespace1", Mapping: &meta.RESTMapping{GroupVersionKind: gvk}} + + gvkDiffGroup := schema.GroupVersionKind{Group: "diff", Version: "version1", Kind: "pod"} + resourceInfoDiffGroup := resource.Info{Name: "name1", Namespace: "namespace1", Mapping: &meta.RESTMapping{GroupVersionKind: gvkDiffGroup}} + if isMatchingInfo(&resourceInfo, &resourceInfoDiffGroup) { + t.Error("expected resources not equal") + } + + gvkDiffVersion := schema.GroupVersionKind{Group: "group1", Version: "diff", Kind: "pod"} + resourceInfoDiffVersion := resource.Info{Name: "name1", Namespace: "namespace1", Mapping: &meta.RESTMapping{GroupVersionKind: gvkDiffVersion}} + if isMatchingInfo(&resourceInfo, &resourceInfoDiffVersion) { + t.Error("expected resources not equal") + } + + gvkDiffKind := schema.GroupVersionKind{Group: "group1", Version: "version1", Kind: "deployment"} + resourceInfoDiffKind := resource.Info{Name: "name1", Namespace: "namespace1", Mapping: &meta.RESTMapping{GroupVersionKind: gvkDiffKind}} + if isMatchingInfo(&resourceInfo, &resourceInfoDiffKind) { + t.Error("expected resources not equal") + } + + resourceInfoDiffName := resource.Info{Name: "diff", Namespace: "namespace1", Mapping: &meta.RESTMapping{GroupVersionKind: gvk}} + if isMatchingInfo(&resourceInfo, &resourceInfoDiffName) { + t.Error("expected resources not equal") + } + + resourceInfoDiffNamespace := resource.Info{Name: "name1", Namespace: "diff", Mapping: &meta.RESTMapping{GroupVersionKind: gvk}} + if isMatchingInfo(&resourceInfo, &resourceInfoDiffNamespace) { + t.Error("expected resources not equal") + } + + gvkEqual := schema.GroupVersionKind{Group: "group1", Version: "version1", Kind: "pod"} + resourceInfoEqual := resource.Info{Name: "name1", Namespace: "namespace1", Mapping: &meta.RESTMapping{GroupVersionKind: gvkEqual}} + if !isMatchingInfo(&resourceInfo, &resourceInfoEqual) { + t.Error("expected resources to be equal") + } +} diff --git a/pkg/kube/statuswait_test.go b/pkg/kube/statuswait_test.go index 0b309b22d..4b06da896 100644 --- a/pkg/kube/statuswait_test.go +++ b/pkg/kube/statuswait_test.go @@ -154,6 +154,7 @@ spec: ` func getGVR(t *testing.T, mapper meta.RESTMapper, obj *unstructured.Unstructured) schema.GroupVersionResource { + t.Helper() gvk := obj.GroupVersionKind() mapping, err := mapper.RESTMapping(gvk.GroupKind(), gvk.Version) require.NoError(t, err) @@ -161,6 +162,7 @@ func getGVR(t *testing.T, mapper meta.RESTMapper, obj *unstructured.Unstructured } func getRuntimeObjFromManifests(t *testing.T, manifests []string) []runtime.Object { + t.Helper() objects := []runtime.Object{} for _, manifest := range manifests { m := make(map[string]interface{}) @@ -173,6 +175,7 @@ func getRuntimeObjFromManifests(t *testing.T, manifests []string) []runtime.Obje } func getResourceListFromRuntimeObjs(t *testing.T, c *Client, objs []runtime.Object) ResourceList { + t.Helper() resourceList := ResourceList{} for _, obj := range objs { list, err := c.Build(objBody(obj), false) diff --git a/pkg/kube/wait.go b/pkg/kube/wait.go index ebb5b3257..8a3bacdcc 100644 --- a/pkg/kube/wait.go +++ b/pkg/kube/wait.go @@ -117,7 +117,7 @@ func (hw *legacyWaiter) isRetryableHTTPStatusCode(httpStatusCode int32) bool { return httpStatusCode == 0 || httpStatusCode == http.StatusTooManyRequests || (httpStatusCode >= 500 && httpStatusCode != http.StatusNotImplemented) } -// waitForDeletedResources polls to check if all the resources are deleted or a timeout is reached +// WaitForDelete polls to check if all the resources are deleted or a timeout is reached func (hw *legacyWaiter) WaitForDelete(deleted ResourceList, timeout time.Duration) error { slog.Debug("beginning wait for resources to be deleted", "count", len(deleted), "timeout", timeout) diff --git a/pkg/lint/rules/template.go b/pkg/lint/rules/template.go index 135ebf90a..72b81f191 100644 --- a/pkg/lint/rules/template.go +++ b/pkg/lint/rules/template.go @@ -25,6 +25,7 @@ import ( "os" "path" "path/filepath" + "slices" "strings" "k8s.io/apimachinery/pkg/api/validation" @@ -206,10 +207,8 @@ func validateAllowedExtension(fileName string) error { ext := filepath.Ext(fileName) validExtensions := []string{".yaml", ".yml", ".tpl", ".txt"} - for _, b := range validExtensions { - if b == ext { - return nil - } + if slices.Contains(validExtensions, ext) { + return nil } return fmt.Errorf("file extension '%s' not valid. Valid extensions are .yaml, .yml, .tpl, or .txt", ext) diff --git a/pkg/plugin/installer/base_test.go b/pkg/plugin/installer/base_test.go index f4dd6d6be..732ac7927 100644 --- a/pkg/plugin/installer/base_test.go +++ b/pkg/plugin/installer/base_test.go @@ -14,7 +14,6 @@ limitations under the License. package installer // import "helm.sh/helm/v4/pkg/plugin/installer" import ( - "os" "testing" ) @@ -37,12 +36,11 @@ func TestPath(t *testing.T) { for _, tt := range tests { - os.Setenv("HELM_PLUGINS", tt.helmPluginsDir) + t.Setenv("HELM_PLUGINS", tt.helmPluginsDir) baseIns := newBase(tt.source) baseInsPath := baseIns.Path() if baseInsPath != tt.expectPath { t.Errorf("expected name %s, got %s", tt.expectPath, baseInsPath) } - os.Unsetenv("HELM_PLUGINS") } } diff --git a/pkg/plugin/installer/http_installer.go b/pkg/plugin/installer/http_installer.go index 7b6f28db1..3bcf71208 100644 --- a/pkg/plugin/installer/http_installer.go +++ b/pkg/plugin/installer/http_installer.go @@ -27,6 +27,7 @@ import ( "path" "path/filepath" "regexp" + "slices" "strings" securejoin "github.com/cyphar/filepath-securejoin" @@ -196,10 +197,8 @@ func cleanJoin(root, dest string) (string, error) { // We want to alert the user that something bad was attempted. Cleaning it // is not a good practice. - for _, part := range strings.Split(dest, "/") { - if part == ".." { - return "", errors.New("path contains '..', which is illegal") - } + if slices.Contains(strings.Split(dest, "/"), "..") { + return "", errors.New("path contains '..', which is illegal") } // If a path is absolute, the creator of the TAR is doing something shady. diff --git a/pkg/plugin/installer/local_installer_test.go b/pkg/plugin/installer/local_installer_test.go index b28920af4..9effcd2c4 100644 --- a/pkg/plugin/installer/local_installer_test.go +++ b/pkg/plugin/installer/local_installer_test.go @@ -20,12 +20,14 @@ import ( "path/filepath" "testing" + "helm.sh/helm/v4/internal/test/ensure" "helm.sh/helm/v4/pkg/helmpath" ) var _ Installer = new(LocalInstaller) func TestLocalInstaller(t *testing.T) { + ensure.HelmHome(t) // Make a temp dir tdir := t.TempDir() if err := os.WriteFile(filepath.Join(tdir, "plugin.yaml"), []byte{}, 0644); err != nil { diff --git a/pkg/plugin/installer/vcs_installer_test.go b/pkg/plugin/installer/vcs_installer_test.go index fbb5d354e..491d58a3f 100644 --- a/pkg/plugin/installer/vcs_installer_test.go +++ b/pkg/plugin/installer/vcs_installer_test.go @@ -19,6 +19,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "testing" "github.com/Masterminds/vcs" @@ -119,6 +120,8 @@ func TestVCSInstallerNonExistentVersion(t *testing.T) { if err := Install(i); err == nil { t.Fatalf("expected error for version does not exists, got none") + } else if strings.Contains(err.Error(), "Could not resolve host: github.com") { + t.Skip("Unable to run test without Internet access") } else if err.Error() != fmt.Sprintf("requested version %q does not exist for plugin %q", version, source) { t.Fatalf("expected error for version does not exists, got (%v)", err) } @@ -146,7 +149,11 @@ func TestVCSInstallerUpdate(t *testing.T) { // Install plugin before update if err := Install(i); err != nil { - t.Fatal(err) + if strings.Contains(err.Error(), "Could not resolve host: github.com") { + t.Skip("Unable to run test without Internet access") + } else { + t.Fatal(err) + } } // Test FindSource method for positive result diff --git a/pkg/postrender/exec_test.go b/pkg/postrender/exec_test.go index 2b091cc12..a10ad2cc4 100644 --- a/pkg/postrender/exec_test.go +++ b/pkg/postrender/exec_test.go @@ -60,11 +60,7 @@ func TestGetFullPath(t *testing.T) { t.Run("binary in PATH resolves correctly", func(t *testing.T) { testpath := setupTestingScript(t) - realPath := os.Getenv("PATH") - os.Setenv("PATH", filepath.Dir(testpath)) - defer func() { - os.Setenv("PATH", realPath) - }() + t.Setenv("PATH", filepath.Dir(testpath)) fullPath, err := getFullPath(filepath.Base(testpath)) is.NoError(err) @@ -183,7 +179,7 @@ func setupTestingScript(t *testing.T) (filepath string) { t.Fatalf("unable to write tempfile for testing: %s", err) } - err = f.Chmod(0755) + err = f.Chmod(0o755) if err != nil { t.Fatalf("unable to make tempfile executable for testing: %s", err) } diff --git a/pkg/provenance/sign_test.go b/pkg/provenance/sign_test.go index 69a6dad5b..9a60fd19c 100644 --- a/pkg/provenance/sign_test.go +++ b/pkg/provenance/sign_test.go @@ -276,7 +276,7 @@ func TestDecodeSignature(t *testing.T) { t.Fatal(err) } - f, err := os.CreateTemp("", "helm-test-sig-") + f, err := os.CreateTemp(t.TempDir(), "helm-test-sig-") if err != nil { t.Fatal(err) } diff --git a/pkg/pusher/pusher.go b/pkg/pusher/pusher.go index c4c766748..e3c767be9 100644 --- a/pkg/pusher/pusher.go +++ b/pkg/pusher/pusher.go @@ -18,6 +18,7 @@ package pusher import ( "fmt" + "slices" "helm.sh/helm/v4/pkg/cli" "helm.sh/helm/v4/pkg/registry" @@ -86,12 +87,7 @@ type Provider struct { // Provides returns true if the given scheme is supported by this Provider. func (p Provider) Provides(scheme string) bool { - for _, i := range p.Schemes { - if i == scheme { - return true - } - } - return false + return slices.Contains(p.Schemes, scheme) } // Providers is a collection of Provider objects. diff --git a/pkg/registry/client.go b/pkg/registry/client.go index 2d131dc47..3ea68f181 100644 --- a/pkg/registry/client.go +++ b/pkg/registry/client.go @@ -100,27 +100,8 @@ func NewClient(options ...ClientOption) (*Client, error) { client.credentialsFile = helmpath.ConfigPath(CredentialsFileBasename) } if client.httpClient == nil { - type cloner[T any] interface { - Clone() T - } - - // try to copy (clone) the http.DefaultTransport so any mutations we - // perform on it (e.g. TLS config) are not reflected globally - // follow https://github.com/golang/go/issues/39299 for a more elegant - // solution in the future - transport := http.DefaultTransport - if t, ok := transport.(cloner[*http.Transport]); ok { - transport = t.Clone() - } else if t, ok := transport.(cloner[http.RoundTripper]); ok { - // this branch will not be used with go 1.20, it was added - // optimistically to try to clone if the http.DefaultTransport - // implementation changes, still the Clone method in that case - // might not return http.RoundTripper... - transport = t.Clone() - } - client.httpClient = &http.Client{ - Transport: retry.NewTransport(transport), + Transport: NewTransport(client.debug), } } @@ -249,19 +230,20 @@ func (c *Client) Login(host string, options ...LoginOption) error { return err } reg.PlainHTTP = c.plainHTTP + cred := auth.Credential{Username: c.username, Password: c.password} + c.authorizer.ForceAttemptOAuth2 = true reg.Client = c.authorizer ctx := context.Background() - cred, err := c.authorizer.Credential(ctx, host) - if err != nil { - return fmt.Errorf("fetching credentials for %q: %w", host, err) - } - if err := reg.Ping(ctx); err != nil { - return fmt.Errorf("authenticating to %q: %w", host, err) + c.authorizer.ForceAttemptOAuth2 = false + if err := reg.Ping(ctx); err != nil { + return fmt.Errorf("authenticating to %q: %w", host, err) + } } key := credentials.ServerAddressFromRegistry(host) + key = credentials.ServerAddressFromHostname(key) if err := c.credentialsStore.Put(ctx, key, cred); err != nil { return err } @@ -296,6 +278,11 @@ func ensureTLSConfig(client *auth.Client) (*tls.Config, error) { switch t := t.Base.(type) { case *http.Transport: transport = t + case *LoggingTransport: + switch t := t.RoundTripper.(type) { + case *http.Transport: + transport = t + } } } @@ -463,7 +450,7 @@ func (c *Client) Pull(ref string, options ...PullOption) (*PullResult, error) { PreCopy: func(_ context.Context, desc ocispec.Descriptor) error { mediaType := desc.MediaType if i := sort.SearchStrings(allowedMediaTypes, mediaType); i >= len(allowedMediaTypes) || allowedMediaTypes[i] != mediaType { - return fmt.Errorf("media type %q is not allowed, found in descriptor with digest: %q", mediaType, desc.Digest) + return oras.SkipNode } mu.Lock() @@ -477,7 +464,6 @@ func (c *Client) Pull(ref string, options ...PullOption) (*PullResult, error) { return nil, err } - descriptors = append(descriptors, manifest) descriptors = append(descriptors, layers...) numDescriptors := len(descriptors) @@ -685,19 +671,9 @@ func (c *Client) Push(data []byte, ref string, options ...PushOption) (*PushResu }) ociAnnotations := generateOCIAnnotations(meta, operation.creationTime) - manifest := ocispec.Manifest{ - Versioned: specs.Versioned{SchemaVersion: 2}, - Config: configDescriptor, - Layers: layers, - Annotations: ociAnnotations, - } - - manifestData, err := json.Marshal(manifest) - if err != nil { - return nil, err - } - manifestDescriptor, err := oras.TagBytes(ctx, memoryStore, ocispec.MediaTypeImageManifest, manifestData, ref) + manifestDescriptor, err := c.tagManifest(ctx, memoryStore, configDescriptor, + layers, ociAnnotations, parsedRef) if err != nil { return nil, err } @@ -898,3 +874,24 @@ func (c *Client) ValidateReference(ref, version string, u *url.URL) (*url.URL, e return u, err } + +// tagManifest prepares and tags a manifest in memory storage +func (c *Client) tagManifest(ctx context.Context, memoryStore *memory.Store, + configDescriptor ocispec.Descriptor, layers []ocispec.Descriptor, + ociAnnotations map[string]string, parsedRef reference) (ocispec.Descriptor, error) { + + manifest := ocispec.Manifest{ + Versioned: specs.Versioned{SchemaVersion: 2}, + Config: configDescriptor, + Layers: layers, + Annotations: ociAnnotations, + } + + manifestData, err := json.Marshal(manifest) + if err != nil { + return ocispec.Descriptor{}, err + } + + return oras.TagBytes(ctx, memoryStore, ocispec.MediaTypeImageManifest, + manifestData, parsedRef.String()) +} diff --git a/pkg/registry/client_test.go b/pkg/registry/client_test.go new file mode 100644 index 000000000..2ffd691c2 --- /dev/null +++ b/pkg/registry/client_test.go @@ -0,0 +1,53 @@ +/* +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 registry + +import ( + "io" + "testing" + + ocispec "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/stretchr/testify/require" + "oras.land/oras-go/v2/content/memory" +) + +// Inspired by oras test +// https://github.com/oras-project/oras-go/blob/05a2b09cbf2eab1df691411884dc4df741ec56ab/content_test.go#L1802 +func TestTagManifestTransformsReferences(t *testing.T) { + memStore := memory.New() + client := &Client{out: io.Discard} + ctx := t.Context() + + refWithPlus := "test-registry.io/charts/test:1.0.0+metadata" + expectedRef := "test-registry.io/charts/test:1.0.0_metadata" // + becomes _ + + configDesc := ocispec.Descriptor{MediaType: ConfigMediaType, Digest: "sha256:config", Size: 100} + layers := []ocispec.Descriptor{{MediaType: ChartLayerMediaType, Digest: "sha256:layer", Size: 200}} + + parsedRef, err := newReference(refWithPlus) + require.NoError(t, err) + + desc, err := client.tagManifest(ctx, memStore, configDesc, layers, nil, parsedRef) + require.NoError(t, err) + + transformedDesc, err := memStore.Resolve(ctx, expectedRef) + require.NoError(t, err, "Should find the reference with _ instead of +") + require.Equal(t, desc.Digest, transformedDesc.Digest) + + _, err = memStore.Resolve(ctx, refWithPlus) + require.Error(t, err, "Should NOT find the reference with the original +") +} diff --git a/pkg/registry/reference_test.go b/pkg/registry/reference_test.go index 31317d18f..b6872cc37 100644 --- a/pkg/registry/reference_test.go +++ b/pkg/registry/reference_test.go @@ -19,6 +19,7 @@ package registry import "testing" func verify(t *testing.T, actual reference, registry, repository, tag, digest string) { + t.Helper() if registry != actual.orasReference.Registry { t.Errorf("Oras reference registry expected %v actual %v", registry, actual.Registry) } diff --git a/pkg/registry/transport.go b/pkg/registry/transport.go new file mode 100644 index 000000000..7b9c6744b --- /dev/null +++ b/pkg/registry/transport.go @@ -0,0 +1,175 @@ +/* +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 registry + +import ( + "bytes" + "fmt" + "io" + "log/slog" + "mime" + "net/http" + "strings" + "sync/atomic" + + "oras.land/oras-go/v2/registry/remote/retry" +) + +var ( + // requestCount records the number of logged request-response pairs and will + // be used as the unique id for the next pair. + requestCount uint64 + + // toScrub is a set of headers that should be scrubbed from the log. + toScrub = []string{ + "Authorization", + "Set-Cookie", + } +) + +// payloadSizeLimit limits the maximum size of the response body to be printed. +const payloadSizeLimit int64 = 16 * 1024 // 16 KiB + +// LoggingTransport is an http.RoundTripper that keeps track of the in-flight +// request and add hooks to report HTTP tracing events. +type LoggingTransport struct { + http.RoundTripper +} + +// NewTransport creates and returns a new instance of LoggingTransport +func NewTransport(debug bool) *retry.Transport { + type cloner[T any] interface { + Clone() T + } + + // try to copy (clone) the http.DefaultTransport so any mutations we + // perform on it (e.g. TLS config) are not reflected globally + // follow https://github.com/golang/go/issues/39299 for a more elegant + // solution in the future + transport := http.DefaultTransport + if t, ok := transport.(cloner[*http.Transport]); ok { + transport = t.Clone() + } else if t, ok := transport.(cloner[http.RoundTripper]); ok { + // this branch will not be used with go 1.20, it was added + // optimistically to try to clone if the http.DefaultTransport + // implementation changes, still the Clone method in that case + // might not return http.RoundTripper... + transport = t.Clone() + } + if debug { + transport = &LoggingTransport{RoundTripper: transport} + } + + return retry.NewTransport(transport) +} + +// RoundTrip calls base round trip while keeping track of the current request. +func (t *LoggingTransport) RoundTrip(req *http.Request) (resp *http.Response, err error) { + id := atomic.AddUint64(&requestCount, 1) - 1 + + slog.Debug("Request", "id", id, "url", req.URL, "method", req.Method, "header", logHeader(req.Header)) + resp, err = t.RoundTripper.RoundTrip(req) + if err != nil { + slog.Debug("Response", "id", id, "error", err) + } else if resp != nil { + slog.Debug("Response", "id", id, "status", resp.Status, "header", logHeader(resp.Header), "body", logResponseBody(resp)) + } else { + slog.Debug("Response", "id", id, "response", "nil") + } + + return resp, err +} + +// logHeader prints out the provided header keys and values, with auth header scrubbed. +func logHeader(header http.Header) string { + if len(header) > 0 { + headers := []string{} + for k, v := range header { + for _, h := range toScrub { + if strings.EqualFold(k, h) { + v = []string{"*****"} + } + } + headers = append(headers, fmt.Sprintf(" %q: %q", k, strings.Join(v, ", "))) + } + return strings.Join(headers, "\n") + } + return " Empty header" +} + +// logResponseBody prints out the response body if it is printable and within size limit. +func logResponseBody(resp *http.Response) string { + if resp.Body == nil || resp.Body == http.NoBody { + return " No response body to print" + } + + // non-applicable body is not printed and remains untouched for subsequent processing + contentType := resp.Header.Get("Content-Type") + if contentType == "" { + return " Response body without a content type is not printed" + } + if !isPrintableContentType(contentType) { + return fmt.Sprintf(" Response body of content type %q is not printed", contentType) + } + + buf := bytes.NewBuffer(nil) + body := resp.Body + // restore the body by concatenating the read body with the remaining body + resp.Body = struct { + io.Reader + io.Closer + }{ + Reader: io.MultiReader(buf, body), + Closer: body, + } + // read the body up to limit+1 to check if the body exceeds the limit + if _, err := io.CopyN(buf, body, payloadSizeLimit+1); err != nil && err != io.EOF { + return fmt.Sprintf(" Error reading response body: %v", err) + } + + readBody := buf.String() + if len(readBody) == 0 { + return " Response body is empty" + } + if containsCredentials(readBody) { + return " Response body redacted due to potential credentials" + } + if len(readBody) > int(payloadSizeLimit) { + return readBody[:payloadSizeLimit] + "\n...(truncated)" + } + return readBody +} + +// isPrintableContentType returns true if the contentType is printable. +func isPrintableContentType(contentType string) bool { + mediaType, _, err := mime.ParseMediaType(contentType) + if err != nil { + return false + } + + switch mediaType { + case "application/json", // JSON types + "text/plain", "text/html": // text types + return true + } + return strings.HasSuffix(mediaType, "+json") +} + +// containsCredentials returns true if the body contains potential credentials. +func containsCredentials(body string) bool { + return strings.Contains(body, `"token"`) || strings.Contains(body, `"access_token"`) +} diff --git a/pkg/registry/transport_test.go b/pkg/registry/transport_test.go new file mode 100644 index 000000000..b4990c526 --- /dev/null +++ b/pkg/registry/transport_test.go @@ -0,0 +1,399 @@ +/* +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 registry + +import ( + "bytes" + "errors" + "io" + "net/http" + "testing" +) + +var errMockRead = errors.New("mock read error") + +type errorReader struct{} + +func (e *errorReader) Read(_ []byte) (n int, err error) { + return 0, errMockRead +} + +func Test_isPrintableContentType(t *testing.T) { + tests := []struct { + name string + contentType string + want bool + }{ + { + name: "Empty content type", + contentType: "", + want: false, + }, + { + name: "General JSON type", + contentType: "application/json", + want: true, + }, + { + name: "General JSON type with charset", + contentType: "application/json; charset=utf-8", + want: true, + }, + { + name: "Random type with application/json prefix", + contentType: "application/jsonwhatever", + want: false, + }, + { + name: "Manifest type in JSON", + contentType: "application/vnd.oci.image.manifest.v1+json", + want: true, + }, + { + name: "Manifest type in JSON with charset", + contentType: "application/vnd.oci.image.manifest.v1+json; charset=utf-8", + want: true, + }, + { + name: "Random content type in JSON", + contentType: "application/whatever+json", + want: true, + }, + { + name: "Plain text type", + contentType: "text/plain", + want: true, + }, + { + name: "Plain text type with charset", + contentType: "text/plain; charset=utf-8", + want: true, + }, + { + name: "Random type with text/plain prefix", + contentType: "text/plainnnnn", + want: false, + }, + { + name: "HTML type", + contentType: "text/html", + want: true, + }, + { + name: "Plain text type with charset", + contentType: "text/html; charset=utf-8", + want: true, + }, + { + name: "Random type with text/html prefix", + contentType: "text/htmlllll", + want: false, + }, + { + name: "Binary type", + contentType: "application/octet-stream", + want: false, + }, + { + name: "Unknown type", + contentType: "unknown/unknown", + want: false, + }, + { + name: "Invalid type", + contentType: "text/", + want: false, + }, + { + name: "Random string", + contentType: "random123!@#", + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := isPrintableContentType(tt.contentType); got != tt.want { + t.Errorf("isPrintableContentType() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_logResponseBody(t *testing.T) { + tests := []struct { + name string + resp *http.Response + want string + wantData []byte + }{ + { + name: "Nil body", + resp: &http.Response{ + Body: nil, + Header: http.Header{"Content-Type": []string{"application/json"}}, + }, + want: " No response body to print", + }, + { + name: "No body", + wantData: nil, + resp: &http.Response{ + Body: http.NoBody, + ContentLength: 100, // in case of HEAD response, the content length is set but the body is empty + Header: http.Header{"Content-Type": []string{"application/json"}}, + }, + want: " No response body to print", + }, + { + name: "Empty body", + wantData: []byte(""), + resp: &http.Response{ + Body: io.NopCloser(bytes.NewReader([]byte(""))), + ContentLength: 0, + Header: http.Header{"Content-Type": []string{"text/plain"}}, + }, + want: " Response body is empty", + }, + { + name: "Unknown content length", + wantData: []byte("whatever"), + resp: &http.Response{ + Body: io.NopCloser(bytes.NewReader([]byte("whatever"))), + ContentLength: -1, + Header: http.Header{"Content-Type": []string{"text/plain"}}, + }, + want: "whatever", + }, + { + name: "Missing content type header", + wantData: []byte("whatever"), + resp: &http.Response{ + Body: io.NopCloser(bytes.NewReader([]byte("whatever"))), + ContentLength: 8, + }, + want: " Response body without a content type is not printed", + }, + { + name: "Empty content type header", + wantData: []byte("whatever"), + resp: &http.Response{ + Body: io.NopCloser(bytes.NewReader([]byte("whatever"))), + ContentLength: 8, + Header: http.Header{"Content-Type": []string{""}}, + }, + want: " Response body without a content type is not printed", + }, + { + name: "Non-printable content type", + wantData: []byte("binary data"), + resp: &http.Response{ + Body: io.NopCloser(bytes.NewReader([]byte("binary data"))), + ContentLength: 11, + Header: http.Header{"Content-Type": []string{"application/octet-stream"}}, + }, + want: " Response body of content type \"application/octet-stream\" is not printed", + }, + { + name: "Body at the limit", + wantData: bytes.Repeat([]byte("a"), int(payloadSizeLimit)), + resp: &http.Response{ + Body: io.NopCloser(bytes.NewReader(bytes.Repeat([]byte("a"), int(payloadSizeLimit)))), + ContentLength: payloadSizeLimit, + Header: http.Header{"Content-Type": []string{"text/plain"}}, + }, + want: string(bytes.Repeat([]byte("a"), int(payloadSizeLimit))), + }, + { + name: "Body larger than limit", + wantData: bytes.Repeat([]byte("a"), int(payloadSizeLimit+1)), + resp: &http.Response{ + Body: io.NopCloser(bytes.NewReader(bytes.Repeat([]byte("a"), int(payloadSizeLimit+1)))), // 1 byte larger than limit + ContentLength: payloadSizeLimit + 1, + Header: http.Header{"Content-Type": []string{"text/plain"}}, + }, + want: string(bytes.Repeat([]byte("a"), int(payloadSizeLimit))) + "\n...(truncated)", + }, + { + name: "Printable content type within limit", + wantData: []byte("data"), + resp: &http.Response{ + Body: io.NopCloser(bytes.NewReader([]byte("data"))), + ContentLength: 4, + Header: http.Header{"Content-Type": []string{"text/plain"}}, + }, + want: "data", + }, + { + name: "Actual body size is larger than content length", + wantData: []byte("data"), + resp: &http.Response{ + Body: io.NopCloser(bytes.NewReader([]byte("data"))), + ContentLength: 3, // mismatched content length + Header: http.Header{"Content-Type": []string{"text/plain"}}, + }, + want: "data", + }, + { + name: "Actual body size is larger than content length and exceeds limit", + wantData: bytes.Repeat([]byte("a"), int(payloadSizeLimit+1)), + resp: &http.Response{ + Body: io.NopCloser(bytes.NewReader(bytes.Repeat([]byte("a"), int(payloadSizeLimit+1)))), // 1 byte larger than limit + ContentLength: 1, // mismatched content length + Header: http.Header{"Content-Type": []string{"text/plain"}}, + }, + want: string(bytes.Repeat([]byte("a"), int(payloadSizeLimit))) + "\n...(truncated)", + }, + { + name: "Actual body size is smaller than content length", + wantData: []byte("data"), + resp: &http.Response{ + Body: io.NopCloser(bytes.NewReader([]byte("data"))), + ContentLength: 5, // mismatched content length + Header: http.Header{"Content-Type": []string{"text/plain"}}, + }, + want: "data", + }, + { + name: "Body contains token", + resp: &http.Response{ + Body: io.NopCloser(bytes.NewReader([]byte(`{"token":"12345"}`))), + ContentLength: 17, + Header: http.Header{"Content-Type": []string{"application/json"}}, + }, + wantData: []byte(`{"token":"12345"}`), + want: " Response body redacted due to potential credentials", + }, + { + name: "Body contains access_token", + resp: &http.Response{ + Body: io.NopCloser(bytes.NewReader([]byte(`{"access_token":"12345"}`))), + ContentLength: 17, + Header: http.Header{"Content-Type": []string{"application/json"}}, + }, + wantData: []byte(`{"access_token":"12345"}`), + want: " Response body redacted due to potential credentials", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := logResponseBody(tt.resp); got != tt.want { + t.Errorf("logResponseBody() = %v, want %v", got, tt.want) + } + // validate the response body + if tt.resp.Body != nil { + readBytes, err := io.ReadAll(tt.resp.Body) + if err != nil { + t.Errorf("failed to read body after logResponseBody(), err= %v", err) + } + if !bytes.Equal(readBytes, tt.wantData) { + t.Errorf("resp.Body after logResponseBody() = %v, want %v", readBytes, tt.wantData) + } + if closeErr := tt.resp.Body.Close(); closeErr != nil { + t.Errorf("failed to close body after logResponseBody(), err= %v", closeErr) + } + } + }) + } +} + +func Test_logResponseBody_error(t *testing.T) { + tests := []struct { + name string + resp *http.Response + want string + }{ + { + name: "Error reading body", + resp: &http.Response{ + Body: io.NopCloser(&errorReader{}), + ContentLength: 10, + Header: http.Header{"Content-Type": []string{"text/plain"}}, + }, + want: " Error reading response body: mock read error", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := logResponseBody(tt.resp); got != tt.want { + t.Errorf("logResponseBody() = %v, want %v", got, tt.want) + } + if closeErr := tt.resp.Body.Close(); closeErr != nil { + t.Errorf("failed to close body after logResponseBody(), err= %v", closeErr) + } + }) + } +} + +func Test_containsCredentials(t *testing.T) { + tests := []struct { + name string + body string + want bool + }{ + { + name: "Contains token keyword", + body: `{"token": "12345"}`, + want: true, + }, + { + name: "Contains quoted token keyword", + body: `whatever "token" blah`, + want: true, + }, + { + name: "Contains unquoted token keyword", + body: `whatever token blah`, + want: false, + }, + { + name: "Contains access_token keyword", + body: `{"access_token": "12345"}`, + want: true, + }, + { + name: "Contains quoted access_token keyword", + body: `whatever "access_token" blah`, + want: true, + }, + { + name: "Contains unquoted access_token keyword", + body: `whatever access_token blah`, + want: false, + }, + { + name: "Does not contain credentials", + body: `{"key": "value"}`, + want: false, + }, + { + name: "Empty body", + body: ``, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := containsCredentials(tt.body); got != tt.want { + t.Errorf("containsCredentials() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pkg/registry/util.go b/pkg/registry/util.go index e63dda43a..b31ab63fe 100644 --- a/pkg/registry/util.go +++ b/pkg/registry/util.go @@ -21,6 +21,7 @@ import ( "fmt" "io" "net/http" + "slices" "strings" "time" @@ -45,12 +46,7 @@ func IsOCI(url string) bool { // ContainsTag determines whether a tag is found in a provided list of tags func ContainsTag(tags []string, tag string) bool { - for _, t := range tags { - if tag == t { - return true - } - } - return false + return slices.Contains(tags, tag) } func GetTagMatchingVersionOrConstraint(tags []string, versionString string) (string, error) { diff --git a/pkg/registry/utils_test.go b/pkg/registry/utils_test.go index fe07c769a..e8fcba4e3 100644 --- a/pkg/registry/utils_test.go +++ b/pkg/registry/utils_test.go @@ -141,7 +141,7 @@ func setup(suite *TestSuite, tlsEnabled, insecure bool) *registry.Registry { suite.Nil(err, "no error creating mock DNS server") suite.srv.PatchNet(net.DefaultResolver) - config.HTTP.Addr = fmt.Sprintf(":%d", port) + config.HTTP.Addr = fmt.Sprintf("127.0.0.1:%d", port) config.HTTP.DrainTimeout = time.Duration(10) * time.Second config.Storage = map[string]configuration.Parameters{"inmemory": map[string]interface{}{}} @@ -182,7 +182,7 @@ func initCompromisedRegistryTestServer() string { s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if strings.Contains(r.URL.Path, "manifests") { w.Header().Set("Content-Type", "application/vnd.oci.image.manifest.v1+json") - w.WriteHeader(200) + w.WriteHeader(http.StatusOK) fmt.Fprintf(w, `{ "schemaVersion": 2, "config": { "mediaType": "%s", @@ -199,16 +199,16 @@ func initCompromisedRegistryTestServer() string { }`, ConfigMediaType, ChartLayerMediaType) } else if r.URL.Path == "/v2/testrepo/supposedlysafechart/blobs/sha256:a705ee2789ab50a5ba20930f246dbd5cc01ff9712825bb98f57ee8414377f133" { w.Header().Set("Content-Type", "application/json") - w.WriteHeader(200) + w.WriteHeader(http.StatusOK) w.Write([]byte("{\"name\":\"mychart\",\"version\":\"0.1.0\",\"description\":\"A Helm chart for Kubernetes\\n" + "an 'application' or a 'library' chart.\",\"apiVersion\":\"v2\",\"appVersion\":\"1.16.0\",\"type\":" + "\"application\"}")) } else if r.URL.Path == "/v2/testrepo/supposedlysafechart/blobs/sha256:ca978112ca1bbdcafac231b39a23dc4da786eff8147c4e72b9807785afee48bb" { w.Header().Set("Content-Type", ChartLayerMediaType) - w.WriteHeader(200) + w.WriteHeader(http.StatusOK) w.Write([]byte("b")) } else { - w.WriteHeader(500) + w.WriteHeader(http.StatusInternalServerError) } })) diff --git a/pkg/release/util/sorter_test.go b/pkg/release/util/sorter_test.go index 8a766efc9..7ca540441 100644 --- a/pkg/release/util/sorter_test.go +++ b/pkg/release/util/sorter_test.go @@ -43,6 +43,7 @@ func tsRelease(name string, vers int, dur time.Duration, status rspb.Status) *rs } func check(t *testing.T, by string, fn func(int, int) bool) { + t.Helper() for i := len(releases) - 1; i > 0; i-- { if fn(i, i-1) { t.Errorf("release at positions '(%d,%d)' not sorted by %s", i-1, i, by) diff --git a/pkg/repo/chartrepo_test.go b/pkg/repo/chartrepo_test.go index c29c95a7e..05e034dd8 100644 --- a/pkg/repo/chartrepo_test.go +++ b/pkg/repo/chartrepo_test.go @@ -70,7 +70,7 @@ func TestIndexCustomSchemeDownload(t *testing.T) { } repo.CachePath = t.TempDir() - tempIndexFile, err := os.CreateTemp("", "test-repo") + tempIndexFile, err := os.CreateTemp(t.TempDir(), "test-repo") if err != nil { t.Fatalf("Failed to create temp index file: %v", err) } @@ -224,11 +224,15 @@ func TestResolveReferenceURL(t *testing.T) { for _, tt := range []struct { baseURL, refURL, chartURL string }{ + {"http://localhost:8123/", "/nginx-0.2.0.tgz", "http://localhost:8123/nginx-0.2.0.tgz"}, {"http://localhost:8123/charts/", "nginx-0.2.0.tgz", "http://localhost:8123/charts/nginx-0.2.0.tgz"}, + {"http://localhost:8123/charts/", "/nginx-0.2.0.tgz", "http://localhost:8123/nginx-0.2.0.tgz"}, {"http://localhost:8123/charts-with-no-trailing-slash", "nginx-0.2.0.tgz", "http://localhost:8123/charts-with-no-trailing-slash/nginx-0.2.0.tgz"}, {"http://localhost:8123", "https://charts.helm.sh/stable/nginx-0.2.0.tgz", "https://charts.helm.sh/stable/nginx-0.2.0.tgz"}, {"http://localhost:8123/charts%2fwith%2fescaped%2fslash", "nginx-0.2.0.tgz", "http://localhost:8123/charts%2fwith%2fescaped%2fslash/nginx-0.2.0.tgz"}, + {"http://localhost:8123/charts%2fwith%2fescaped%2fslash", "/nginx-0.2.0.tgz", "http://localhost:8123/nginx-0.2.0.tgz"}, {"http://localhost:8123/charts?with=queryparameter", "nginx-0.2.0.tgz", "http://localhost:8123/charts/nginx-0.2.0.tgz?with=queryparameter"}, + {"http://localhost:8123/charts?with=queryparameter", "/nginx-0.2.0.tgz", "http://localhost:8123/nginx-0.2.0.tgz?with=queryparameter"}, } { chartURL, err := ResolveReferenceURL(tt.baseURL, tt.refURL) if err != nil { diff --git a/pkg/repo/index_test.go b/pkg/repo/index_test.go index 2a33cd1a9..d40719b12 100644 --- a/pkg/repo/index_test.go +++ b/pkg/repo/index_test.go @@ -352,6 +352,7 @@ func TestDownloadIndexFile(t *testing.T) { } func verifyLocalIndex(t *testing.T, i *IndexFile) { + t.Helper() numEntries := len(i.Entries) if numEntries != 3 { t.Errorf("Expected 3 entries in index file but got %d", numEntries) @@ -450,6 +451,7 @@ func verifyLocalIndex(t *testing.T, i *IndexFile) { } func verifyLocalChartsFile(t *testing.T, chartsContent []byte, indexContent *IndexFile) { + t.Helper() var expected, reald []string for chart := range indexContent.Entries { expected = append(expected, chart) diff --git a/pkg/repo/repo_test.go b/pkg/repo/repo_test.go index c2087ebbe..bdaa61eda 100644 --- a/pkg/repo/repo_test.go +++ b/pkg/repo/repo_test.go @@ -197,7 +197,7 @@ func TestWriteFile(t *testing.T) { }, ) - file, err := os.CreateTemp("", "helm-repo") + file, err := os.CreateTemp(t.TempDir(), "helm-repo") if err != nil { t.Errorf("failed to create test-file (%v)", err) } diff --git a/pkg/repo/repotest/server.go b/pkg/repo/repotest/server.go index 709a6f5fd..ea9d5290c 100644 --- a/pkg/repo/repotest/server.go +++ b/pkg/repo/repotest/server.go @@ -16,7 +16,6 @@ limitations under the License. package repotest import ( - "context" "crypto/tls" "fmt" "net/http" @@ -42,6 +41,7 @@ import ( ) func BasicAuthMiddleware(t *testing.T) http.HandlerFunc { + t.Helper() return http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { username, password, ok := r.BasicAuth() if !ok || username != "username" || password != "password" { @@ -89,11 +89,8 @@ type Server struct { // // The temp dir will be removed by testing package automatically when test finished. func NewTempServer(t *testing.T, options ...ServerOption) *Server { - - docrootTempDir, err := os.MkdirTemp("", "helm-repotest-") - if err != nil { - t.Fatal(err) - } + t.Helper() + docrootTempDir := t.TempDir() srv := newServer(t, docrootTempDir, options...) @@ -110,6 +107,7 @@ func NewTempServer(t *testing.T, options ...ServerOption) *Server { // Create the server, but don't yet start it func newServer(t *testing.T, docroot string, options ...ServerOption) *Server { + t.Helper() absdocroot, err := filepath.Abs(docroot) if err != nil { t.Fatal(err) @@ -162,6 +160,7 @@ func WithDependingChart(c *chart.Chart) OCIServerOpt { } func NewOCIServer(t *testing.T, dir string) (*OCIServer, error) { + t.Helper() testHtpasswdFileBasename := "authtest.htpasswd" testUsername, testPassword := "username", "password" @@ -170,7 +169,7 @@ func NewOCIServer(t *testing.T, dir string) (*OCIServer, error) { t.Fatal("error generating bcrypt password for test htpasswd file") } htpasswdPath := filepath.Join(dir, testHtpasswdFileBasename) - err = os.WriteFile(htpasswdPath, []byte(fmt.Sprintf("%s:%s\n", testUsername, string(pwBytes))), 0644) + err = os.WriteFile(htpasswdPath, []byte(fmt.Sprintf("%s:%s\n", testUsername, string(pwBytes))), 0o644) if err != nil { t.Fatalf("error creating test htpasswd file") } @@ -194,7 +193,7 @@ func NewOCIServer(t *testing.T, dir string) (*OCIServer, error) { registryURL := fmt.Sprintf("localhost:%d", port) - r, err := registry.NewRegistry(context.Background(), config) + r, err := registry.NewRegistry(t.Context(), config) if err != nil { t.Fatal(err) } @@ -209,6 +208,7 @@ func NewOCIServer(t *testing.T, dir string) (*OCIServer, error) { } func (srv *OCIServer) Run(t *testing.T, opts ...OCIServerOpt) { + t.Helper() cfg := &OCIServerRunConfig{} for _, fn := range opts { fn(cfg) @@ -327,7 +327,7 @@ func (s *Server) CopyCharts(origin string) ([]string, error) { if err != nil { return []string{}, err } - if err := os.WriteFile(newname, data, 0644); err != nil { + if err := os.WriteFile(newname, data, 0o644); err != nil { return []string{}, err } copied[i] = newname @@ -351,7 +351,7 @@ func (s *Server) CreateIndex() error { } ifile := filepath.Join(s.docroot, "index.yaml") - return os.WriteFile(ifile, d, 0644) + return os.WriteFile(ifile, d, 0o644) } func (s *Server) start() { @@ -403,5 +403,5 @@ func setTestingRepository(url, fname string) error { Name: "test", URL: url, }) - return r.WriteFile(fname, 0640) + return r.WriteFile(fname, 0o640) } diff --git a/pkg/repo/repotest/server_test.go b/pkg/repo/repotest/server_test.go index cf68e5110..4d62ef8ed 100644 --- a/pkg/repo/repotest/server_test.go +++ b/pkg/repo/repotest/server_test.go @@ -92,7 +92,7 @@ func TestServer(t *testing.T) { if err != nil { t.Fatal(err) } - if res.StatusCode != 404 { + if res.StatusCode != http.StatusNotFound { t.Fatalf("Expected 404, got %d", res.StatusCode) } } @@ -140,7 +140,7 @@ func TestNewTempServer(t *testing.T) { res.Body.Close() - if res.StatusCode != 200 { + if res.StatusCode != http.StatusOK { t.Errorf("Expected 200, got %d", res.StatusCode) } @@ -153,7 +153,7 @@ func TestNewTempServer(t *testing.T) { } res.Body.Close() - if res.StatusCode != 200 { + if res.StatusCode != http.StatusOK { t.Errorf("Expected 200, got %d", res.StatusCode) } } @@ -198,7 +198,7 @@ func TestNewTempServer(t *testing.T) { if err != nil { t.Fatal(err) } - if res.StatusCode != 404 { + if res.StatusCode != http.StatusNotFound { t.Fatalf("Expected 404, got %d", res.StatusCode) } }) diff --git a/pkg/repo/repotest/tlsconfig.go b/pkg/repo/repotest/tlsconfig.go index 3914a4d3f..3ea7338ff 100644 --- a/pkg/repo/repotest/tlsconfig.go +++ b/pkg/repo/repotest/tlsconfig.go @@ -26,6 +26,7 @@ import ( ) func MakeTestTLSConfig(t *testing.T, path string) *tls.Config { + t.Helper() ca, pub, priv := filepath.Join(path, "rootca.crt"), filepath.Join(path, "crt.pem"), filepath.Join(path, "key.pem") insecure := false diff --git a/pkg/storage/driver/mock_test.go b/pkg/storage/driver/mock_test.go index 1dda258bb..7dba5fea2 100644 --- a/pkg/storage/driver/mock_test.go +++ b/pkg/storage/driver/mock_test.go @@ -52,6 +52,7 @@ func testKey(name string, vers int) string { } func tsFixtureMemory(t *testing.T) *Memory { + t.Helper() hs := []*rspb.Release{ // rls-a releaseStub("rls-a", 4, "default", rspb.StatusDeployed), @@ -83,6 +84,7 @@ func tsFixtureMemory(t *testing.T) *Memory { // newTestFixtureCfgMaps initializes a MockConfigMapsInterface. // ConfigMaps are created for each release provided. func newTestFixtureCfgMaps(t *testing.T, releases ...*rspb.Release) *ConfigMaps { + t.Helper() var mock MockConfigMapsInterface mock.Init(t, releases...) @@ -98,6 +100,7 @@ type MockConfigMapsInterface struct { // Init initializes the MockConfigMapsInterface with the set of releases. func (mock *MockConfigMapsInterface) Init(t *testing.T, releases ...*rspb.Release) { + t.Helper() mock.objects = map[string]*v1.ConfigMap{} for _, rls := range releases { @@ -169,6 +172,7 @@ func (mock *MockConfigMapsInterface) Delete(_ context.Context, name string, _ me // newTestFixtureSecrets initializes a MockSecretsInterface. // Secrets are created for each release provided. func newTestFixtureSecrets(t *testing.T, releases ...*rspb.Release) *Secrets { + t.Helper() var mock MockSecretsInterface mock.Init(t, releases...) @@ -184,6 +188,7 @@ type MockSecretsInterface struct { // Init initializes the MockSecretsInterface with the set of releases. func (mock *MockSecretsInterface) Init(t *testing.T, releases ...*rspb.Release) { + t.Helper() mock.objects = map[string]*v1.Secret{} for _, rls := range releases { @@ -254,6 +259,7 @@ func (mock *MockSecretsInterface) Delete(_ context.Context, name string, _ metav // newTestFixtureSQL mocks the SQL database (for testing purposes) func newTestFixtureSQL(t *testing.T, _ ...*rspb.Release) (*SQL, sqlmock.Sqlmock) { + t.Helper() sqlDB, mock, err := sqlmock.New() if err != nil { t.Fatalf("error when opening stub database connection: %v", err) diff --git a/pkg/storage/driver/sql.go b/pkg/storage/driver/sql.go index c3740b9a3..46f6c6b2e 100644 --- a/pkg/storage/driver/sql.go +++ b/pkg/storage/driver/sql.go @@ -19,6 +19,7 @@ package driver // import "helm.sh/helm/v4/pkg/storage/driver" import ( "fmt" "log/slog" + "maps" "sort" "strconv" "time" @@ -367,9 +368,7 @@ func (s *SQL) List(filter func(*rspb.Release) bool) ([]*rspb.Release, error) { slog.Debug("failed to get release custom labels", "namespace", record.Namespace, "key", record.Key, slog.Any("error", err)) return nil, err } - for k, v := range getReleaseSystemLabels(release) { - release.Labels[k] = v - } + maps.Copy(release.Labels, getReleaseSystemLabels(release)) if filter(release) { releases = append(releases, release) diff --git a/pkg/storage/driver/util.go b/pkg/storage/driver/util.go index 0abbe41b2..ca8e23cc2 100644 --- a/pkg/storage/driver/util.go +++ b/pkg/storage/driver/util.go @@ -22,6 +22,7 @@ import ( "encoding/base64" "encoding/json" "io" + "slices" rspb "helm.sh/helm/v4/pkg/release/v1" ) @@ -88,12 +89,7 @@ func decodeRelease(data string) (*rspb.Release, error) { // Checks if label is system func isSystemLabel(key string) bool { - for _, v := range GetSystemLabels() { - if key == v { - return true - } - } - return false + return slices.Contains(GetSystemLabels(), key) } // Removes system labels from labels map diff --git a/pkg/time/time.go b/pkg/time/time.go index 5b3a0ccdc..16973b455 100644 --- a/pkg/time/time.go +++ b/pkg/time/time.go @@ -65,6 +65,7 @@ func Parse(layout, value string) (Time, error) { t, err := time.Parse(layout, value) return Time{Time: t}, err } + func ParseInLocation(layout, value string, loc *time.Location) (Time, error) { t, err := time.ParseInLocation(layout, value, loc) return Time{Time: t}, err diff --git a/pkg/time/time_test.go b/pkg/time/time_test.go index 20f0f8e29..342ca4a10 100644 --- a/pkg/time/time_test.go +++ b/pkg/time/time_test.go @@ -20,64 +20,134 @@ import ( "encoding/json" "testing" "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) var ( - testingTime, _ = Parse(time.RFC3339, "1977-09-02T22:04:05Z") - testingTimeString = `"1977-09-02T22:04:05Z"` + timeParseString = `"1977-09-02T22:04:05Z"` + timeString = "1977-09-02 22:04:05 +0000 UTC" ) -func TestNonZeroValueMarshal(t *testing.T) { +func givenTime(t *testing.T) Time { + t.Helper() + result, err := Parse(time.RFC3339, "1977-09-02T22:04:05Z") + require.NoError(t, err) + return result +} + +func TestDate(t *testing.T) { + testingTime := givenTime(t) + got := Date(1977, 9, 2, 22, 04, 05, 0, time.UTC) + assert.Equal(t, timeString, got.String()) + assert.True(t, testingTime.Equal(got)) + assert.True(t, got.Equal(testingTime)) +} + +func TestNow(t *testing.T) { + testingTime := givenTime(t) + got := Now() + assert.True(t, testingTime.Before(got)) + assert.True(t, got.After(testingTime)) +} + +func TestTime_Add(t *testing.T) { + testingTime := givenTime(t) + got := testingTime.Add(time.Hour) + assert.Equal(t, timeString, testingTime.String()) + assert.Equal(t, "1977-09-02 23:04:05 +0000 UTC", got.String()) +} + +func TestTime_AddDate(t *testing.T) { + testingTime := givenTime(t) + got := testingTime.AddDate(1, 1, 1) + assert.Equal(t, "1978-10-03 22:04:05 +0000 UTC", got.String()) +} + +func TestTime_In(t *testing.T) { + testingTime := givenTime(t) + edt, err := time.LoadLocation("America/New_York") + assert.NoError(t, err) + got := testingTime.In(edt) + assert.Equal(t, "America/New_York", got.Location().String()) +} + +func TestTime_MarshalJSONNonZero(t *testing.T) { + testingTime := givenTime(t) res, err := json.Marshal(testingTime) - if err != nil { - t.Fatal(err) - } - if testingTimeString != string(res) { - t.Errorf("expected a marshaled value of %s, got %s", testingTimeString, res) - } + assert.NoError(t, err) + assert.Equal(t, timeParseString, string(res)) } -func TestZeroValueMarshal(t *testing.T) { +func TestTime_MarshalJSONZeroValue(t *testing.T) { res, err := json.Marshal(Time{}) - if err != nil { - t.Fatal(err) - } - if string(res) != emptyString { - t.Errorf("expected zero value to marshal to empty string, got %s", res) - } + assert.NoError(t, err) + assert.Equal(t, `""`, string(res)) } -func TestNonZeroValueUnmarshal(t *testing.T) { +func TestTime_Round(t *testing.T) { + testingTime := givenTime(t) + got := testingTime.Round(time.Hour) + assert.Equal(t, timeString, testingTime.String()) + assert.Equal(t, "1977-09-02 22:00:00 +0000 UTC", got.String()) +} + +func TestTime_Sub(t *testing.T) { + testingTime := givenTime(t) + before, err := Parse(time.RFC3339, "1977-09-01T22:04:05Z") + require.NoError(t, err) + got := testingTime.Sub(before) + assert.Equal(t, "24h0m0s", got.String()) +} + +func TestTime_Truncate(t *testing.T) { + testingTime := givenTime(t) + got := testingTime.Truncate(time.Hour) + assert.Equal(t, timeString, testingTime.String()) + assert.Equal(t, "1977-09-02 22:00:00 +0000 UTC", got.String()) +} + +func TestTime_UTC(t *testing.T) { + edtTime, err := Parse(time.RFC3339, "1977-09-03T05:04:05+07:00") + require.NoError(t, err) + got := edtTime.UTC() + assert.Equal(t, timeString, got.String()) +} + +func TestTime_UnmarshalJSONNonZeroValue(t *testing.T) { + testingTime := givenTime(t) var myTime Time - err := json.Unmarshal([]byte(testingTimeString), &myTime) - if err != nil { - t.Fatal(err) - } - if !myTime.Equal(testingTime) { - t.Errorf("expected time to be equal to %v, got %v", testingTime, myTime) - } + err := json.Unmarshal([]byte(timeParseString), &myTime) + assert.NoError(t, err) + assert.True(t, testingTime.Equal(myTime)) } -func TestEmptyStringUnmarshal(t *testing.T) { +func TestTime_UnmarshalJSONEmptyString(t *testing.T) { var myTime Time err := json.Unmarshal([]byte(emptyString), &myTime) - if err != nil { - t.Fatal(err) - } - if !myTime.IsZero() { - t.Errorf("expected time to be equal to zero value, got %v", myTime) - } + assert.NoError(t, err) + assert.True(t, myTime.IsZero()) +} + +func TestTime_UnmarshalJSONNullString(t *testing.T) { + var myTime Time + err := json.Unmarshal([]byte("null"), &myTime) + assert.NoError(t, err) + assert.True(t, myTime.IsZero()) } -func TestZeroValueUnmarshal(t *testing.T) { +func TestTime_UnmarshalJSONZeroValue(t *testing.T) { // This test ensures that we can unmarshal any time value that was output // with the current go default value of "0001-01-01T00:00:00Z" var myTime Time err := json.Unmarshal([]byte(`"0001-01-01T00:00:00Z"`), &myTime) - if err != nil { - t.Fatal(err) - } - if !myTime.IsZero() { - t.Errorf("expected time to be equal to zero value, got %v", myTime) - } + assert.NoError(t, err) + assert.True(t, myTime.IsZero()) +} + +func TestUnix(t *testing.T) { + got := Unix(242085845, 0) + assert.Equal(t, int64(242085845), got.Unix()) + assert.Equal(t, timeString, got.UTC().String()) }