diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index 914af6a0a..5971ada24 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -21,6 +21,6 @@ jobs: go-version: '1.23' check-latest: true - name: golangci-lint - uses: golangci/golangci-lint-action@051d91933864810ecd5e2ea2cfd98f6a5bca5347 #pin@6.3.2 + uses: golangci/golangci-lint-action@2226d7cb06a077cd73e56eedd38eecad18e5d837 #pin@6.5.0 with: version: v1.62 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0c2f88453..8ab93403d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -276,12 +276,26 @@ Like any good open source project, we use Pull Requests (PRs) to track code chan or explicitly request another OWNER do that for them. - If the owner of a PR is _not_ listed in `OWNERS`, any core maintainer may merge the PR. -#### Documentation PRs +### Documentation PRs Documentation PRs should be made on the docs repo: . Keeping Helm's documentation up to date is highly desirable, and is recommended for all user facing changes. Accurate and helpful documentation is critical for effectively communicating Helm's behavior to a wide audience. Small, ad-hoc changes/PRs to Helm which introduce user facing changes, which would benefit from documentation changes, should apply the `docs needed` label. Larger changes associated with a HIP should track docs via that HIP. The `docs needed` label doesn't block PRs, and maintainers/PR reviewers should apply discretion judging in whether the `docs needed` label should be applied. +### Profiling PRs + +If your contribution requires profiling to check memory and/or CPU usage, you can set `HELM_PPROF_CPU_PROFILE=/path/to/cpu.prof` and/or `HELM_PPROF_MEM_PROFILE=/path/to/mem.prof` environment variables to collect runtime profiling data for analysis. You can use Golang's [pprof](https://github.com/google/pprof/blob/main/doc/README.md) tool to inspect the results. + +Example analysing collected profiling data +``` +HELM_PPROF_CPU_PROFILE=cpu.prof HELM_PPROF_MEM_PROFILE=mem.prof helm show all bitnami/nginx + +# Visualize graphs. You need to have installed graphviz package in your system +go tool pprof -http=":8000" cpu.prof + +go tool pprof -http=":8001" mem.prof +``` + ## The Triager Each week, one of the core maintainers will serve as the designated "triager" starting after the diff --git a/cmd/helm/dependency_build_test.go b/cmd/helm/dependency_build_test.go index 189378ce5..1258db3f8 100644 --- a/cmd/helm/dependency_build_test.go +++ b/cmd/helm/dependency_build_test.go @@ -29,11 +29,11 @@ import ( ) func TestDependencyBuildCmd(t *testing.T) { - srv, err := repotest.NewTempServerWithCleanup(t, "testdata/testcharts/*.tgz") + srv := repotest.NewTempServer( + t, + repotest.WithChartSourceGlob("testdata/testcharts/*.tgz"), + ) defer srv.Stop() - if err != nil { - t.Fatal(err) - } rootDir := srv.Root() srv.LinkIndices() diff --git a/cmd/helm/dependency_update_test.go b/cmd/helm/dependency_update_test.go index 82a6b875d..7cf3e8e0a 100644 --- a/cmd/helm/dependency_update_test.go +++ b/cmd/helm/dependency_update_test.go @@ -32,10 +32,10 @@ import ( ) func TestDependencyUpdateCmd(t *testing.T) { - srv, err := repotest.NewTempServerWithCleanup(t, "testdata/testcharts/*.tgz") - if err != nil { - t.Fatal(err) - } + srv := repotest.NewTempServer( + t, + repotest.WithChartSourceGlob("testdata/testcharts/*.tgz"), + ) defer srv.Stop() t.Logf("Listening on directory %s", srv.Root()) @@ -151,10 +151,10 @@ func TestDependencyUpdateCmd_DoNotDeleteOldChartsOnError(t *testing.T) { defer resetEnv()() ensure.HelmHome(t) - srv, err := repotest.NewTempServerWithCleanup(t, "testdata/testcharts/*.tgz") - if err != nil { - t.Fatal(err) - } + srv := repotest.NewTempServer( + t, + repotest.WithChartSourceGlob("testdata/testcharts/*.tgz"), + ) defer srv.Stop() t.Logf("Listening on directory %s", srv.Root()) @@ -248,10 +248,10 @@ func TestDependencyUpdateCmd_WithRepoThatWasNotAdded(t *testing.T) { } func setupMockRepoServer(t *testing.T) *repotest.Server { - srv, err := repotest.NewTempServerWithCleanup(t, "testdata/testcharts/*.tgz") - if err != nil { - t.Fatal(err) - } + srv := repotest.NewTempServer( + t, + repotest.WithChartSourceGlob("testdata/testcharts/*.tgz"), + ) t.Logf("Listening on directory %s", srv.Root()) diff --git a/cmd/helm/flags.go b/cmd/helm/flags.go index 3d159babd..8d0f644d6 100644 --- a/cmd/helm/flags.go +++ b/cmd/helm/flags.go @@ -47,7 +47,7 @@ func addValueOptionsFlags(f *pflag.FlagSet, v *values.Options) { f.StringArrayVar(&v.Values, "set", []string{}, "set values on the command line (can specify multiple or separate values with commas: key1=val1,key2=val2)") f.StringArrayVar(&v.StringValues, "set-string", []string{}, "set STRING values on the command line (can specify multiple or separate values with commas: key1=val1,key2=val2)") f.StringArrayVar(&v.FileValues, "set-file", []string{}, "set values from respective files specified via the command line (can specify multiple or separate values with commas: key1=path1,key2=path2)") - f.StringArrayVar(&v.JSONValues, "set-json", []string{}, "set JSON values on the command line (can specify multiple or separate values with commas: key1=jsonval1,key2=jsonval2)") + f.StringArrayVar(&v.JSONValues, "set-json", []string{}, "set JSON values on the command line (can specify multiple or separate values with commas: key1=jsonval1,key2=jsonval2 or using json format: {\"key1\": jsonval1, \"key2\": \"jsonval2\"})") f.StringArrayVar(&v.LiteralValues, "set-literal", []string{}, "set a literal STRING value on the command line") } diff --git a/cmd/helm/install.go b/cmd/helm/install.go index ec651140c..fe09dfc53 100644 --- a/cmd/helm/install.go +++ b/cmd/helm/install.go @@ -52,7 +52,7 @@ or use the '--set' flag and pass configuration from the command line, to force a string value use '--set-string'. You can use '--set-file' to set individual values from a file when the value itself is too long for the command line or is dynamically generated. You can also use '--set-json' to set json values -(scalars/objects/arrays) from the command line. +(scalars/objects/arrays) from the command line. Additionally, you can use '--set-json' and passing json object as a string. $ helm install -f myvalues.yaml myredis ./redis @@ -72,6 +72,9 @@ or $ helm install --set-json 'master.sidecars=[{"name":"sidecar","image":"myImage","imagePullPolicy":"Always","ports":[{"name":"portname","containerPort":1234}]}]' myredis ./redis +or + + $ helm install --set-json '{"master":{"sidecars":[{"name":"sidecar","image":"myImage","imagePullPolicy":"Always","ports":[{"name":"portname","containerPort":1234}]}]}}' myredis ./redis You can specify the '--values'/'-f' flag multiple times. The priority will be given to the last (right-most) file specified. For example, if both myvalues.yaml and override.yaml diff --git a/cmd/helm/install_test.go b/cmd/helm/install_test.go index e7b6e1dff..be8480423 100644 --- a/cmd/helm/install_test.go +++ b/cmd/helm/install_test.go @@ -27,19 +27,13 @@ import ( ) func TestInstall(t *testing.T) { - srv, err := repotest.NewTempServerWithCleanup(t, "testdata/testcharts/*.tgz*") - if err != nil { - t.Fatal(err) - } + srv := repotest.NewTempServer( + t, + repotest.WithChartSourceGlob("testdata/testcharts/*.tgz*"), + repotest.WithMiddleware(repotest.BasicAuthMiddleware(t)), + ) defer srv.Stop() - srv.WithMiddleware(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { - username, password, ok := r.BasicAuth() - if !ok || username != "username" || password != "password" { - t.Errorf("Expected request to use basic auth and for username == 'username' and password == 'password', got '%v', '%s', '%s'", ok, username, password) - } - })) - srv2 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { http.FileServer(http.Dir(srv.Root())).ServeHTTP(w, r) })) diff --git a/cmd/helm/profiling.go b/cmd/helm/profiling.go new file mode 100644 index 000000000..950ad15da --- /dev/null +++ b/cmd/helm/profiling.go @@ -0,0 +1,91 @@ +/* +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 main + +import ( + "errors" + "fmt" + "os" + "runtime" + "runtime/pprof" +) + +var ( + cpuProfileFile *os.File + cpuProfilePath string + memProfilePath string +) + +func init() { + cpuProfilePath = os.Getenv("HELM_PPROF_CPU_PROFILE") + memProfilePath = os.Getenv("HELM_PPROF_MEM_PROFILE") +} + +// startProfiling starts profiling CPU usage if HELM_PPROF_CPU_PROFILE is set +// to a file path. It returns an error if the file could not be created or +// CPU profiling could not be started. +func startProfiling() error { + if cpuProfilePath != "" { + var err error + cpuProfileFile, err = os.Create(cpuProfilePath) + if err != nil { + return fmt.Errorf("could not create CPU profile: %w", err) + } + if err := pprof.StartCPUProfile(cpuProfileFile); err != nil { + cpuProfileFile.Close() + cpuProfileFile = nil + return fmt.Errorf("could not start CPU profile: %w", err) + } + } + return nil +} + +// stopProfiling stops profiling CPU and memory usage. +// It writes memory profile to the file path specified in HELM_PPROF_MEM_PROFILE +// environment variable. +func stopProfiling() error { + errs := []error{} + + // Stop CPU profiling if it was started + if cpuProfileFile != nil { + pprof.StopCPUProfile() + err := cpuProfileFile.Close() + if err != nil { + errs = append(errs, err) + } + cpuProfileFile = nil + } + + if memProfilePath != "" { + f, err := os.Create(memProfilePath) + if err != nil { + errs = append(errs, err) + } + defer f.Close() + + runtime.GC() // get up-to-date statistics + if err := pprof.WriteHeapProfile(f); err != nil { + errs = append(errs, err) + } + } + + if err := errors.Join(errs...); err != nil { + return fmt.Errorf("error(s) while stopping profiling: %w", err) + } + + return nil +} diff --git a/cmd/helm/pull_test.go b/cmd/helm/pull_test.go index 160e95c76..1110a6bdf 100644 --- a/cmd/helm/pull_test.go +++ b/cmd/helm/pull_test.go @@ -28,10 +28,10 @@ import ( ) func TestPullCmd(t *testing.T) { - srv, err := repotest.NewTempServerWithCleanup(t, "testdata/testcharts/*.tgz*") - if err != nil { - t.Fatal(err) - } + srv := repotest.NewTempServer( + t, + repotest.WithChartSourceGlob("testdata/testcharts/*.tgz*"), + ) defer srv.Stop() ociSrv, err := repotest.NewOCIServer(t, srv.Root()) @@ -257,19 +257,13 @@ func TestPullCmd(t *testing.T) { } func TestPullWithCredentialsCmd(t *testing.T) { - srv, err := repotest.NewTempServerWithCleanup(t, "testdata/testcharts/*.tgz*") - if err != nil { - t.Fatal(err) - } + srv := repotest.NewTempServer( + t, + repotest.WithChartSourceGlob("testdata/testcharts/*.tgz*"), + repotest.WithMiddleware(repotest.BasicAuthMiddleware(t)), + ) defer srv.Stop() - srv.WithMiddleware(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { - username, password, ok := r.BasicAuth() - if !ok || username != "username" || password != "password" { - t.Errorf("Expected request to use basic auth and for username == 'username' and password == 'password', got '%v', '%s', '%s'", ok, username, password) - } - })) - srv2 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { http.FileServer(http.Dir(srv.Root())).ServeHTTP(w, r) })) diff --git a/cmd/helm/repo_add_test.go b/cmd/helm/repo_add_test.go index 15ad835e6..35911d5ae 100644 --- a/cmd/helm/repo_add_test.go +++ b/cmd/helm/repo_add_test.go @@ -34,22 +34,21 @@ import ( ) func TestRepoAddCmd(t *testing.T) { - srv, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*") - if err != nil { - t.Fatal(err) - } + srv := repotest.NewTempServer( + t, + repotest.WithChartSourceGlob("testdata/testserver/*.*"), + ) defer srv.Stop() // A second test server is setup to verify URL changing - srv2, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*") - if err != nil { - t.Fatal(err) - } + srv2 := repotest.NewTempServer( + t, + repotest.WithChartSourceGlob("testdata/testserver/*.*"), + ) defer srv2.Stop() tmpdir := filepath.Join(t.TempDir(), "path-component.yaml/data") - err = os.MkdirAll(tmpdir, 0777) - if err != nil { + if err := os.MkdirAll(tmpdir, 0777); err != nil { t.Fatal(err) } repoFile := filepath.Join(tmpdir, "repositories.yaml") @@ -81,10 +80,10 @@ func TestRepoAddCmd(t *testing.T) { } func TestRepoAdd(t *testing.T) { - ts, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*") - if err != nil { - t.Fatal(err) - } + ts := repotest.NewTempServer( + t, + repotest.WithChartSourceGlob("testdata/testserver/*.*"), + ) defer ts.Stop() rootDir := t.TempDir() @@ -134,10 +133,10 @@ func TestRepoAdd(t *testing.T) { } func TestRepoAddCheckLegalName(t *testing.T) { - ts, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*") - if err != nil { - t.Fatal(err) - } + ts := repotest.NewTempServer( + t, + repotest.WithChartSourceGlob("testdata/testserver/*.*"), + ) defer ts.Stop() defer resetEnv()() @@ -190,10 +189,10 @@ func TestRepoAddConcurrentHiddenFile(t *testing.T) { } func repoAddConcurrent(t *testing.T, testName, repoFile string) { - ts, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*") - if err != nil { - t.Fatal(err) - } + ts := repotest.NewTempServer( + t, + repotest.WithChartSourceGlob("testdata/testserver/*.*"), + ) defer ts.Stop() var wg sync.WaitGroup @@ -240,7 +239,11 @@ func TestRepoAddFileCompletion(t *testing.T) { } func TestRepoAddWithPasswordFromStdin(t *testing.T) { - srv := repotest.NewTempServerWithCleanupAndBasicAuth(t, "testdata/testserver/*.*") + srv := repotest.NewTempServer( + t, + repotest.WithChartSourceGlob("testdata/testserver/*.*"), + repotest.WithMiddleware(repotest.BasicAuthMiddleware(t)), + ) defer srv.Stop() defer resetEnv()() diff --git a/cmd/helm/repo_remove_test.go b/cmd/helm/repo_remove_test.go index dcfbd99f8..7e6609671 100644 --- a/cmd/helm/repo_remove_test.go +++ b/cmd/helm/repo_remove_test.go @@ -30,10 +30,10 @@ import ( ) func TestRepoRemove(t *testing.T) { - ts, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*") - if err != nil { - t.Fatal(err) - } + ts := repotest.NewTempServer( + t, + repotest.WithChartSourceGlob("testdata/testserver/*.*"), + ) defer ts.Stop() rootDir := t.TempDir() @@ -162,10 +162,11 @@ func testCacheFiles(t *testing.T, cacheIndexFile string, cacheChartsFile string, } func TestRepoRemoveCompletion(t *testing.T) { - ts, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*") - if err != nil { - t.Fatal(err) - } + ts := repotest.NewTempServer( + t, + repotest.WithChartSourceGlob("testdata/testserver/*.*"), + ) + defer ts.Stop() rootDir := t.TempDir() diff --git a/cmd/helm/repo_update_test.go b/cmd/helm/repo_update_test.go index 13369c7cc..7e379da91 100644 --- a/cmd/helm/repo_update_test.go +++ b/cmd/helm/repo_update_test.go @@ -106,10 +106,11 @@ func TestUpdateCustomCacheCmd(t *testing.T) { cachePath := filepath.Join(rootDir, "updcustomcache") os.Mkdir(cachePath, os.ModePerm) - ts, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*") - if err != nil { - t.Fatal(err) - } + ts := repotest.NewTempServer( + t, + repotest.WithChartSourceGlob("testdata/testserver/*.*"), + ) + defer ts.Stop() o := &repoUpdateOptions{ @@ -130,10 +131,9 @@ func TestUpdateCharts(t *testing.T) { defer resetEnv()() ensure.HelmHome(t) - ts, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*") - if err != nil { - t.Fatal(err) - } + ts := repotest.NewTempServer(t, + repotest.WithChartSourceGlob("testdata/testserver/*.*"), + ) defer ts.Stop() r, err := repo.NewChartRepository(&repo.Entry{ @@ -165,10 +165,10 @@ func TestUpdateChartsFail(t *testing.T) { defer resetEnv()() ensure.HelmHome(t) - ts, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*") - if err != nil { - t.Fatal(err) - } + ts := repotest.NewTempServer( + t, + repotest.WithChartSourceGlob("testdata/testserver/*.*"), + ) defer ts.Stop() var invalidURL = ts.URL() + "55" @@ -198,10 +198,10 @@ func TestUpdateChartsFailWithError(t *testing.T) { defer resetEnv()() ensure.HelmHome(t) - ts, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*") - if err != nil { - t.Fatal(err) - } + ts := repotest.NewTempServer( + t, + repotest.WithChartSourceGlob("testdata/testserver/*.*"), + ) defer ts.Stop() var invalidURL = ts.URL() + "55" diff --git a/cmd/helm/require/args_test.go b/cmd/helm/require/args_test.go index 5a84a42d0..cd5850650 100644 --- a/cmd/helm/require/args_test.go +++ b/cmd/helm/require/args_test.go @@ -71,7 +71,8 @@ func runTestCases(t *testing.T, testCases []testCase) { Args: tc.validateFunc, } cmd.SetArgs(tc.args) - cmd.SetOutput(io.Discard) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) err := cmd.Execute() if tc.wantError == "" { diff --git a/cmd/helm/root.go b/cmd/helm/root.go index d21d5e04b..dd3ddeab7 100644 --- a/cmd/helm/root.go +++ b/cmd/helm/root.go @@ -95,6 +95,16 @@ func newRootCmd(actionConfig *action.Configuration, out io.Writer, args []string Short: "The Helm package manager for Kubernetes.", Long: globalUsage, SilenceUsage: true, + PersistentPreRun: func(_ *cobra.Command, _ []string) { + if err := startProfiling(); err != nil { + log.Printf("Warning: Failed to start profiling: %v", err) + } + }, + PersistentPostRun: func(_ *cobra.Command, _ []string) { + if err := stopProfiling(); err != nil { + log.Printf("Warning: Failed to stop profiling: %v", err) + } + }, } flags := cmd.PersistentFlags() diff --git a/cmd/helm/show_test.go b/cmd/helm/show_test.go index 098335f09..0598095b5 100644 --- a/cmd/helm/show_test.go +++ b/cmd/helm/show_test.go @@ -26,10 +26,10 @@ import ( ) func TestShowPreReleaseChart(t *testing.T) { - srv, err := repotest.NewTempServerWithCleanup(t, "testdata/testcharts/*.tgz*") - if err != nil { - t.Fatal(err) - } + srv := repotest.NewTempServer( + t, + repotest.WithChartSourceGlob("testdata/testcharts/*.tgz*"), + ) defer srv.Stop() if err := srv.LinkIndices(); err != nil { diff --git a/cmd/helm/upgrade.go b/cmd/helm/upgrade.go index 7b4267894..6684f9ebf 100644 --- a/cmd/helm/upgrade.go +++ b/cmd/helm/upgrade.go @@ -53,7 +53,7 @@ or use the '--set' flag and pass configuration from the command line, to force s values, use '--set-string'. You can use '--set-file' to set individual values from a file when the value itself is too long for the command line or is dynamically generated. You can also use '--set-json' to set json values -(scalars/objects/arrays) from the command line. +(scalars/objects/arrays) from the command line. Additionally, you can use '--set-json' and passing json object as a string. You can specify the '--values'/'-f' flag multiple times. The priority will be given to the last (right-most) file specified. For example, if both myvalues.yaml and override.yaml diff --git a/go.mod b/go.mod index d4bb5755d..c84140350 100644 --- a/go.mod +++ b/go.mod @@ -29,21 +29,21 @@ require ( github.com/phayes/freeport v0.0.0-20220201140144-74d24b5ae9f5 github.com/pkg/errors v0.9.1 github.com/rubenv/sql-migrate v1.7.1 - github.com/spf13/cobra v1.8.1 + github.com/spf13/cobra v1.9.1 github.com/spf13/pflag v1.0.6 github.com/stretchr/testify v1.10.0 github.com/xeipuuv/gojsonschema v1.2.0 golang.org/x/crypto v0.33.0 golang.org/x/term v0.29.0 golang.org/x/text v0.22.0 - k8s.io/api v0.32.1 - k8s.io/apiextensions-apiserver v0.32.1 - k8s.io/apimachinery v0.32.1 - k8s.io/apiserver v0.32.1 - k8s.io/cli-runtime v0.32.1 - k8s.io/client-go v0.32.1 + k8s.io/api v0.32.2 + k8s.io/apiextensions-apiserver v0.32.2 + k8s.io/apimachinery v0.32.2 + k8s.io/apiserver v0.32.2 + k8s.io/cli-runtime v0.32.2 + k8s.io/client-go v0.32.2 k8s.io/klog/v2 v2.130.1 - k8s.io/kubectl v0.32.1 + k8s.io/kubectl v0.32.2 oras.land/oras-go/v2 v2.5.0 sigs.k8s.io/yaml v1.4.0 ) @@ -63,7 +63,7 @@ require ( github.com/containerd/log v0.1.0 // indirect github.com/containerd/platforms v0.2.1 // indirect github.com/coreos/go-systemd/v22 v22.5.0 // indirect - github.com/cpuguy83/go-md2man/v2 v2.0.4 // indirect + github.com/cpuguy83/go-md2man/v2 v2.0.6 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect github.com/distribution/reference v0.6.0 // indirect @@ -174,7 +174,7 @@ require ( gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect - k8s.io/component-base v0.32.1 // indirect + k8s.io/component-base v0.32.2 // indirect k8s.io/kube-openapi v0.0.0-20241105132330-32ad38e42d3f // indirect k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738 // indirect sigs.k8s.io/json v0.0.0-20241010143419-9aa6b5e7a4b3 // indirect diff --git a/go.sum b/go.sum index 4eaeeaf3a..995e14598 100644 --- a/go.sum +++ b/go.sum @@ -58,8 +58,8 @@ github.com/containerd/platforms v0.2.1 h1:zvwtM3rz2YHPQsF2CHYM8+KtB5dvhISiXh5ZpS github.com/containerd/platforms v0.2.1/go.mod h1:XHCb+2/hzowdiut9rkudds9bE5yJ7npe7dG/wG+uFPw= github.com/coreos/go-systemd/v22 v22.5.0 h1:RrqgGjYQKalulkV8NGVIfkXQf6YYmOyiJKk8iXXhfZs= github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc= -github.com/cpuguy83/go-md2man/v2 v2.0.4 h1:wfIWP927BUkWJb2NmU/kNDYIBTh/ziUX91+lVfRxZq4= -github.com/cpuguy83/go-md2man/v2 v2.0.4/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= +github.com/cpuguy83/go-md2man/v2 v2.0.6 h1:XJtiaUW6dEEqVuZiMTn1ldk455QWwEIsMIJlo5vtkx0= +github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/creack/pty v1.1.18 h1:n56/Zwd5o6whRC5PMGretI4IdRLlmBXYNjScPaBgsbY= github.com/creack/pty v1.1.18/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4= @@ -313,9 +313,8 @@ github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ github.com/sirupsen/logrus v1.9.3/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ= github.com/spf13/cast v1.7.0 h1:ntdiHjuueXFgm5nzDRdOS4yfT43P5Fnud6DH50rz/7w= github.com/spf13/cast v1.7.0/go.mod h1:ancEpBxwJDODSW/UG4rDrAqiKolqNNh2DX3mk86cAdo= -github.com/spf13/cobra v1.8.1 h1:e5/vxKd/rZsfSJMUX1agtjeTDf+qv1/JdBF8gg5k9ZM= -github.com/spf13/cobra v1.8.1/go.mod h1:wHxEcudfqmLYa8iTfL+OuZPbBZkmvliBWKIezN3kD9Y= -github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +github.com/spf13/cobra v1.9.1 h1:CXSaggrXdbHK9CF+8ywj8Amf7PBRmPCOJugH954Nnlo= +github.com/spf13/cobra v1.9.1/go.mod h1:nDyEzZ8ogv936Cinf6g1RU9MRY64Ir93oCnqb9wxYW0= github.com/spf13/pflag v1.0.6 h1:jFzHGLGAlb3ruxLB8MhbI6A8+AQX/2eW4qeyNZXNp2o= github.com/spf13/pflag v1.0.6/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= @@ -519,26 +518,26 @@ 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.32.1 h1:f562zw9cy+GvXzXf0CKlVQ7yHJVYzLfL6JAS4kOAaOc= -k8s.io/api v0.32.1/go.mod h1:/Yi/BqkuueW1BgpoePYBRdDYfjPF5sgTr5+YqDZra5k= -k8s.io/apiextensions-apiserver v0.32.1 h1:hjkALhRUeCariC8DiVmb5jj0VjIc1N0DREP32+6UXZw= -k8s.io/apiextensions-apiserver v0.32.1/go.mod h1:sxWIGuGiYov7Io1fAS2X06NjMIk5CbRHc2StSmbaQto= -k8s.io/apimachinery v0.32.1 h1:683ENpaCBjma4CYqsmZyhEzrGz6cjn1MY/X2jB2hkZs= -k8s.io/apimachinery v0.32.1/go.mod h1:GpHVgxoKlTxClKcteaeuF1Ul/lDVb74KpZcxcmLDElE= -k8s.io/apiserver v0.32.1 h1:oo0OozRos66WFq87Zc5tclUX2r0mymoVHRq8JmR7Aak= -k8s.io/apiserver v0.32.1/go.mod h1:UcB9tWjBY7aryeI5zAgzVJB/6k7E97bkr1RgqDz0jPw= -k8s.io/cli-runtime v0.32.1 h1:19nwZPlYGJPUDbhAxDIS2/oydCikvKMHsxroKNGA2mM= -k8s.io/cli-runtime v0.32.1/go.mod h1:NJPbeadVFnV2E7B7vF+FvU09mpwYlZCu8PqjzfuOnkY= -k8s.io/client-go v0.32.1 h1:otM0AxdhdBIaQh7l1Q0jQpmo7WOFIk5FFa4bg6YMdUU= -k8s.io/client-go v0.32.1/go.mod h1:aTTKZY7MdxUaJ/KiUs8D+GssR9zJZi77ZqtzcGXIiDg= -k8s.io/component-base v0.32.1 h1:/5IfJ0dHIKBWysGV0yKTFfacZ5yNV1sulPh3ilJjRZk= -k8s.io/component-base v0.32.1/go.mod h1:j1iMMHi/sqAHeG5z+O9BFNCF698a1u0186zkjMZQ28w= +k8s.io/api v0.32.2 h1:bZrMLEkgizC24G9eViHGOPbW+aRo9duEISRIJKfdJuw= +k8s.io/api v0.32.2/go.mod h1:hKlhk4x1sJyYnHENsrdCWw31FEmCijNGPJO5WzHiJ6Y= +k8s.io/apiextensions-apiserver v0.32.2 h1:2YMk285jWMk2188V2AERy5yDwBYrjgWYggscghPCvV4= +k8s.io/apiextensions-apiserver v0.32.2/go.mod h1:GPwf8sph7YlJT3H6aKUWtd0E+oyShk/YHWQHf/OOgCA= +k8s.io/apimachinery v0.32.2 h1:yoQBR9ZGkA6Rgmhbp/yuT9/g+4lxtsGYwW6dR6BDPLQ= +k8s.io/apimachinery v0.32.2/go.mod h1:GpHVgxoKlTxClKcteaeuF1Ul/lDVb74KpZcxcmLDElE= +k8s.io/apiserver v0.32.2 h1:WzyxAu4mvLkQxwD9hGa4ZfExo3yZZaYzoYvvVDlM6vw= +k8s.io/apiserver v0.32.2/go.mod h1:PEwREHiHNU2oFdte7BjzA1ZyjWjuckORLIK/wLV5goM= +k8s.io/cli-runtime v0.32.2 h1:aKQR4foh9qeyckKRkNXUccP9moxzffyndZAvr+IXMks= +k8s.io/cli-runtime v0.32.2/go.mod h1:a/JpeMztz3xDa7GCyyShcwe55p8pbcCVQxvqZnIwXN8= +k8s.io/client-go v0.32.2 h1:4dYCD4Nz+9RApM2b/3BtVvBHw54QjMFUl1OLcJG5yOA= +k8s.io/client-go v0.32.2/go.mod h1:fpZ4oJXclZ3r2nDOv+Ux3XcJutfrwjKTCHz2H3sww94= +k8s.io/component-base v0.32.2 h1:1aUL5Vdmu7qNo4ZsE+569PV5zFatM9hl+lb3dEea2zU= +k8s.io/component-base v0.32.2/go.mod h1:PXJ61Vx9Lg+P5mS8TLd7bCIr+eMJRQTyXe8KvkrvJq0= 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-20241105132330-32ad38e42d3f h1:GA7//TjRY9yWGy1poLzYYJJ4JRdzg3+O6e8I+e+8T5Y= k8s.io/kube-openapi v0.0.0-20241105132330-32ad38e42d3f/go.mod h1:R/HEjbvWI0qdfb8viZUeVZm0X6IZnxAydC7YU42CMw4= -k8s.io/kubectl v0.32.1 h1:/btLtXLQUU1rWx8AEvX9jrb9LaI6yeezt3sFALhB8M8= -k8s.io/kubectl v0.32.1/go.mod h1:sezNuyWi1STk4ZNPVRIFfgjqMI6XMf+oCVLjZen/pFQ= +k8s.io/kubectl v0.32.2 h1:TAkag6+XfSBgkqK9I7ZvwtF0WVtUAvK8ZqTt+5zi1Us= +k8s.io/kubectl v0.32.2/go.mod h1:+h/NQFSPxiDZYX/WZaWw9fwYezGLISP0ud8nQKg+3g8= k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738 h1:M3sRQVHv7vB20Xc2ybTt7ODCeFj6JSWYFzOFnYeS6Ro= k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= oras.land/oras-go/v2 v2.5.0 h1:o8Me9kLY74Vp5uw07QXPiitjsw7qNXi8Twd+19Zf02c= diff --git a/pkg/action/release_testing.go b/pkg/action/release_testing.go index 2539a7f65..1568b0683 100644 --- a/pkg/action/release_testing.go +++ b/pkg/action/release_testing.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "io" + "slices" "sort" "time" @@ -75,7 +76,7 @@ func (r *ReleaseTesting) Run(name string) (*release.Release, error) { executingHooks := []*release.Hook{} if len(r.Filters[ExcludeNameFilter]) != 0 { for _, h := range rel.Hooks { - if contains(r.Filters[ExcludeNameFilter], h.Name) { + if slices.Contains(r.Filters[ExcludeNameFilter], h.Name) { skippedHooks = append(skippedHooks, h) } else { executingHooks = append(executingHooks, h) @@ -86,7 +87,7 @@ func (r *ReleaseTesting) Run(name string) (*release.Release, error) { if len(r.Filters[IncludeNameFilter]) != 0 { executingHooks = nil for _, h := range rel.Hooks { - if contains(r.Filters[IncludeNameFilter], h.Name) { + if slices.Contains(r.Filters[IncludeNameFilter], h.Name) { executingHooks = append(executingHooks, h) } else { skippedHooks = append(skippedHooks, h) @@ -119,10 +120,10 @@ func (r *ReleaseTesting) GetPodLogs(out io.Writer, rel *release.Release) error { for _, h := range hooksByWight { for _, e := range h.Events { if e == release.HookTest { - if contains(r.Filters[ExcludeNameFilter], h.Name) { + if slices.Contains(r.Filters[ExcludeNameFilter], h.Name) { continue } - if len(r.Filters[IncludeNameFilter]) > 0 && !contains(r.Filters[IncludeNameFilter], h.Name) { + if len(r.Filters[IncludeNameFilter]) > 0 && !slices.Contains(r.Filters[IncludeNameFilter], h.Name) { continue } req := client.CoreV1().Pods(r.Namespace).GetLogs(h.Name, &v1.PodLogOptions{}) @@ -142,12 +143,3 @@ func (r *ReleaseTesting) GetPodLogs(out io.Writer, rel *release.Release) error { } return nil } - -func contains(arr []string, value string) bool { - for _, item := range arr { - if item == value { - return true - } - } - return false -} diff --git a/pkg/cli/values/options.go b/pkg/cli/values/options.go index 24c47ecba..add2f72d5 100644 --- a/pkg/cli/values/options.go +++ b/pkg/cli/values/options.go @@ -17,6 +17,7 @@ limitations under the License. package values import ( + "encoding/json" "io" "net/url" "os" @@ -62,8 +63,19 @@ func (opts *Options) MergeValues(p getter.Providers) (map[string]interface{}, er // User specified a value via --set-json for _, value := range opts.JSONValues { - if err := strvals.ParseJSON(value, base); err != nil { - return nil, errors.Errorf("failed parsing --set-json data %s", value) + trimmedValue := strings.TrimSpace(value) + if len(trimmedValue) > 0 && trimmedValue[0] == '{' { + // If value is JSON object format, parse it as map + var jsonMap map[string]interface{} + if err := json.Unmarshal([]byte(trimmedValue), &jsonMap); err != nil { + return nil, errors.Errorf("failed parsing --set-json data JSON: %s", value) + } + base = mergeMaps(base, jsonMap) + } else { + // Otherwise, parse it as key=value format + if err := strvals.ParseJSON(value, base); err != nil { + return nil, errors.Errorf("failed parsing --set-json data %s", value) + } } } @@ -143,5 +155,5 @@ func readFile(filePath string, p getter.Providers) ([]byte, error) { if err != nil { return nil, err } - return data.Bytes(), err + return data.Bytes(), nil } diff --git a/pkg/cli/values/options_test.go b/pkg/cli/values/options_test.go index 9182e3cc8..5197a1b5e 100644 --- a/pkg/cli/values/options_test.go +++ b/pkg/cli/values/options_test.go @@ -23,7 +23,7 @@ import ( "helm.sh/helm/v4/pkg/getter" ) -func TestMergeValues(t *testing.T) { +func TestMergeMaps(t *testing.T) { nestedMap := map[string]interface{}{ "foo": "bar", "baz": map[string]string{ @@ -86,3 +86,97 @@ func TestReadFile(t *testing.T) { t.Errorf("Expected error when has special strings") } } + +func TestMergeValues(t *testing.T) { + tests := []struct { + name string + opts Options + expected map[string]interface{} + wantErr bool + }{ + { + name: "set-json object", + opts: Options{ + JSONValues: []string{`{"foo": {"bar": "baz"}}`}, + }, + expected: map[string]interface{}{ + "foo": map[string]interface{}{ + "bar": "baz", + }, + }, + }, + { + name: "set-json key=value", + opts: Options{ + JSONValues: []string{"foo.bar=[1,2,3]"}, + }, + expected: map[string]interface{}{ + "foo": map[string]interface{}{ + "bar": []interface{}{1.0, 2.0, 3.0}, + }, + }, + }, + { + name: "set regular value", + opts: Options{ + Values: []string{"foo=bar"}, + }, + expected: map[string]interface{}{ + "foo": "bar", + }, + }, + { + name: "set string value", + opts: Options{ + StringValues: []string{"foo=123"}, + }, + expected: map[string]interface{}{ + "foo": "123", + }, + }, + { + name: "set literal value", + opts: Options{ + LiteralValues: []string{"foo=true"}, + }, + expected: map[string]interface{}{ + "foo": "true", + }, + }, + { + name: "multiple options", + opts: Options{ + Values: []string{"a=foo"}, + StringValues: []string{"b=bar"}, + JSONValues: []string{`{"c": "foo1"}`}, + LiteralValues: []string{"d=bar1"}, + }, + expected: map[string]interface{}{ + "a": "foo", + "b": "bar", + "c": "foo1", + "d": "bar1", + }, + }, + { + name: "invalid json", + opts: Options{ + JSONValues: []string{`{invalid`}, + }, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := tt.opts.MergeValues(getter.Providers{}) + if (err != nil) != tt.wantErr { + t.Errorf("MergeValues() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !tt.wantErr && !reflect.DeepEqual(got, tt.expected) { + t.Errorf("MergeValues() = %v, want %v", got, tt.expected) + } + }) + } +} diff --git a/pkg/downloader/chart_downloader_test.go b/pkg/downloader/chart_downloader_test.go index 1d28e3c22..26dcc58ff 100644 --- a/pkg/downloader/chart_downloader_test.go +++ b/pkg/downloader/chart_downloader_test.go @@ -175,7 +175,11 @@ func TestIsTar(t *testing.T) { } func TestDownloadTo(t *testing.T) { - srv := repotest.NewTempServerWithCleanupAndBasicAuth(t, "testdata/*.tgz*") + srv := repotest.NewTempServer( + t, + repotest.WithChartSourceGlob("testdata/*.tgz*"), + repotest.WithMiddleware(repotest.BasicAuthMiddleware(t)), + ) defer srv.Stop() if err := srv.CreateIndex(); err != nil { t.Fatal(err) @@ -222,12 +226,11 @@ func TestDownloadTo(t *testing.T) { func TestDownloadTo_TLS(t *testing.T) { // Set up mock server w/ tls enabled - srv, err := repotest.NewTempServerWithCleanup(t, "testdata/*.tgz*") - srv.Stop() - if err != nil { - t.Fatal(err) - } - srv.StartTLS() + srv := repotest.NewTempServer( + t, + repotest.WithChartSourceGlob("testdata/*.tgz*"), + repotest.WithTLSConfig(repotest.MakeTestTLSConfig(t, "../../testdata")), + ) defer srv.Stop() if err := srv.CreateIndex(); err != nil { t.Fatal(err) @@ -249,7 +252,13 @@ func TestDownloadTo_TLS(t *testing.T) { RepositoryConfig: repoConfig, RepositoryCache: repoCache, }), - Options: []getter.Option{}, + Options: []getter.Option{ + getter.WithTLSClientConfig( + "", + "", + filepath.Join("../../testdata/rootca.crt"), + ), + }, } cname := "test/signtest" dest := srv.Root() @@ -278,10 +287,10 @@ func TestDownloadTo_VerifyLater(t *testing.T) { dest := t.TempDir() // Set up a fake repo - srv, err := repotest.NewTempServerWithCleanup(t, "testdata/*.tgz*") - if err != nil { - t.Fatal(err) - } + srv := repotest.NewTempServer( + t, + repotest.WithChartSourceGlob("testdata/*.tgz*"), + ) defer srv.Stop() if err := srv.LinkIndices(); err != nil { t.Fatal(err) diff --git a/pkg/downloader/manager_test.go b/pkg/downloader/manager_test.go index a0c0afa43..6a963f603 100644 --- a/pkg/downloader/manager_test.go +++ b/pkg/downloader/manager_test.go @@ -293,10 +293,10 @@ version: 0.1.0` func TestUpdateBeforeBuild(t *testing.T) { // Set up a fake repo - srv, err := repotest.NewTempServerWithCleanup(t, "testdata/*.tgz*") - if err != nil { - t.Fatal(err) - } + srv := repotest.NewTempServer( + t, + repotest.WithChartSourceGlob("testdata/*.tgz*"), + ) defer srv.Stop() if err := srv.LinkIndices(); err != nil { t.Fatal(err) @@ -348,13 +348,11 @@ func TestUpdateBeforeBuild(t *testing.T) { } // Update before Build. see issue: https://github.com/helm/helm/issues/7101 - err = m.Update() - if err != nil { + if err := m.Update(); err != nil { t.Fatal(err) } - err = m.Build() - if err != nil { + if err := m.Build(); err != nil { t.Fatal(err) } } @@ -364,10 +362,10 @@ func TestUpdateBeforeBuild(t *testing.T) { // to be fetched. func TestUpdateWithNoRepo(t *testing.T) { // Set up a fake repo - srv, err := repotest.NewTempServerWithCleanup(t, "testdata/*.tgz*") - if err != nil { - t.Fatal(err) - } + srv := repotest.NewTempServer( + t, + repotest.WithChartSourceGlob("testdata/*.tgz*"), + ) defer srv.Stop() if err := srv.LinkIndices(); err != nil { t.Fatal(err) @@ -423,8 +421,7 @@ func TestUpdateWithNoRepo(t *testing.T) { } // Test the update - err = m.Update() - if err != nil { + if err := m.Update(); err != nil { t.Fatal(err) } } @@ -437,10 +434,10 @@ func TestUpdateWithNoRepo(t *testing.T) { // 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) { // Set up a fake repo - srv, err := repotest.NewTempServerWithCleanup(t, "testdata/*.tgz*") - if err != nil { - t.Fatal(err) - } + srv := repotest.NewTempServer( + t, + repotest.WithChartSourceGlob("testdata/*.tgz*"), + ) defer srv.Stop() if err := srv.LinkIndices(); err != nil { t.Fatal(err) @@ -488,14 +485,12 @@ func checkBuildWithOptionalFields(t *testing.T, chartName string, dep chart.Depe } // First build will update dependencies and create Chart.lock file. - err = m.Build() - if err != nil { + if err := m.Build(); err != nil { t.Fatal(err) } // Second build should be passed. See PR #6655. - err = m.Build() - if err != nil { + if err := m.Build(); err != nil { t.Fatal(err) } } diff --git a/pkg/kube/roundtripper.go b/pkg/kube/roundtripper.go index fdb103529..551d3009b 100644 --- a/pkg/kube/roundtripper.go +++ b/pkg/kube/roundtripper.go @@ -49,7 +49,7 @@ func (rt *RetryingRoundTripper) roundTrip(req *http.Request, retry int, prevResp b, err := io.ReadAll(resp.Body) resp.Body.Close() if err != nil { - return resp, rtErr + return resp, err } var ke kubernetesError @@ -58,10 +58,10 @@ func (rt *RetryingRoundTripper) roundTrip(req *http.Request, retry int, prevResp r.Seek(0, io.SeekStart) resp.Body = io.NopCloser(r) if err != nil { - return resp, rtErr + return resp, nil } if ke.Code < 500 { - return resp, rtErr + return resp, nil } // Matches messages like "etcdserver: leader changed" if strings.HasSuffix(ke.Message, "etcdserver: leader changed") { @@ -71,7 +71,7 @@ func (rt *RetryingRoundTripper) roundTrip(req *http.Request, retry int, prevResp if strings.HasSuffix(ke.Message, "raft proposal dropped") { return rt.roundTrip(req, retry-1, resp) } - return resp, rtErr + return resp, nil } type kubernetesError struct { diff --git a/pkg/plugin/installer/vcs_installer.go b/pkg/plugin/installer/vcs_installer.go index 8153550b2..3967e46cd 100644 --- a/pkg/plugin/installer/vcs_installer.go +++ b/pkg/plugin/installer/vcs_installer.go @@ -63,7 +63,7 @@ func NewVCSInstaller(source, version string) (*VCSInstaller, error) { Version: version, base: newBase(source), } - return i, err + return i, nil } // Install clones a remote repository and installs into the plugin directory. diff --git a/pkg/provenance/sign_test.go b/pkg/provenance/sign_test.go index bf6848368..69a6dad5b 100644 --- a/pkg/provenance/sign_test.go +++ b/pkg/provenance/sign_test.go @@ -34,7 +34,7 @@ const ( // phrase. Use `gpg --export-secret-keys helm-test` to export the secret. testKeyfile = "testdata/helm-test-key.secret" - // testPasswordKeyFile is a keyfile with a password. + // testPasswordKeyfile is a keyfile with a password. testPasswordKeyfile = "testdata/helm-password-key.secret" // testPubfile is the public key file. diff --git a/pkg/repo/repotest/server.go b/pkg/repo/repotest/server.go index c7b674d04..0155c54d8 100644 --- a/pkg/repo/repotest/server.go +++ b/pkg/repo/repotest/server.go @@ -17,6 +17,7 @@ package repotest import ( "context" + "crypto/tls" "fmt" "net/http" "net/http/httptest" @@ -33,7 +34,6 @@ import ( "golang.org/x/crypto/bcrypt" "sigs.k8s.io/yaml" - "helm.sh/helm/v4/internal/tlsutil" "helm.sh/helm/v4/pkg/chart" "helm.sh/helm/v4/pkg/chart/loader" "helm.sh/helm/v4/pkg/chartutil" @@ -41,34 +41,103 @@ import ( "helm.sh/helm/v4/pkg/repo" ) -// NewTempServerWithCleanup creates a server inside of a temp dir. +func BasicAuthMiddleware(t *testing.T) http.HandlerFunc { + return http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + username, password, ok := r.BasicAuth() + if !ok || username != "username" || password != "password" { + t.Errorf("Expected request to use basic auth and for username == 'username' and password == 'password', got '%v', '%s', '%s'", ok, username, password) + } + }) +} + +type ServerOption func(*testing.T, *Server) + +func WithTLSConfig(tlsConfig *tls.Config) ServerOption { + return func(_ *testing.T, server *Server) { + server.tlsConfig = tlsConfig + } +} + +func WithMiddleware(middleware http.HandlerFunc) ServerOption { + return func(_ *testing.T, server *Server) { + server.middleware = middleware + } +} + +func WithChartSourceGlob(glob string) ServerOption { + return func(_ *testing.T, server *Server) { + server.chartSourceGlob = glob + } +} + +// Server is an implementation of a repository server for testing. +type Server struct { + docroot string + srv *httptest.Server + middleware http.HandlerFunc + tlsConfig *tls.Config + chartSourceGlob string +} + +// NewTempServer creates a server inside of a temp dir. // // If the passed in string is not "", it will be treated as a shell glob, and files // will be copied from that path to the server's docroot. // -// The caller is responsible for stopping the server. +// The server is started automatically. The caller is responsible for stopping +// the server. +// // The temp dir will be removed by testing package automatically when test finished. -func NewTempServerWithCleanup(t *testing.T, glob string) (*Server, error) { - srv, err := NewTempServer(glob) +func NewTempServer(t *testing.T, options ...ServerOption) *Server { + + docrootTempDir, err := os.MkdirTemp("", "helm-repotest-") + if err != nil { + t.Fatal(err) + } + + srv := newServer(t, docrootTempDir, options...) + t.Cleanup(func() { os.RemoveAll(srv.docroot) }) - return srv, err + + if srv.chartSourceGlob != "" { + if _, err := srv.CopyCharts(srv.chartSourceGlob); err != nil { + t.Fatal(err) + } + } + + return srv } -// Set up a fake repo with basic auth enabled -func NewTempServerWithCleanupAndBasicAuth(t *testing.T, glob string) *Server { - srv, err := NewTempServerWithCleanup(t, glob) - srv.Stop() +// Create the server, but don't yet start it +func newServer(t *testing.T, docroot string, options ...ServerOption) *Server { + absdocroot, err := filepath.Abs(docroot) if err != nil { t.Fatal(err) } - srv.WithMiddleware(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { - username, password, ok := r.BasicAuth() - if !ok || username != "username" || password != "password" { - t.Errorf("Expected request to use basic auth and for username == 'username' and password == 'password', got '%v', '%s', '%s'", ok, username, password) + + s := &Server{ + docroot: absdocroot, + } + + for _, option := range options { + option(t, s) + } + + s.srv = httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if s.middleware != nil { + s.middleware.ServeHTTP(w, r) } + http.FileServer(http.Dir(s.Root())).ServeHTTP(w, r) })) - srv.Start() - return srv + + s.start() + + // Add the testing repository as the only repo. Server must be started for the server's URL to be valid + if err := setTestingRepository(s.URL(), filepath.Join(s.docroot, "repositories.yaml")); err != nil { + t.Fatal(err) + } + + return s } type OCIServer struct { @@ -239,69 +308,6 @@ func (srv *OCIServer) Run(t *testing.T, opts ...OCIServerOpt) { result.Chart.Digest, result.Chart.Size) } -// NewTempServer creates a server inside of a temp dir. -// -// If the passed in string is not "", it will be treated as a shell glob, and files -// will be copied from that path to the server's docroot. -// -// The caller is responsible for destroying the temp directory as well as stopping -// the server. -// -// Deprecated: use NewTempServerWithCleanup -func NewTempServer(glob string) (*Server, error) { - tdir, err := os.MkdirTemp("", "helm-repotest-") - if err != nil { - return nil, err - } - srv := NewServer(tdir) - - if glob != "" { - if _, err := srv.CopyCharts(glob); err != nil { - srv.Stop() - return srv, err - } - } - - return srv, nil -} - -// NewServer creates a repository server for testing. -// -// docroot should be a temp dir managed by the caller. -// -// This will start the server, serving files off of the docroot. -// -// Use CopyCharts to move charts into the repository and then index them -// for service. -func NewServer(docroot string) *Server { - root, err := filepath.Abs(docroot) - if err != nil { - panic(err) - } - srv := &Server{ - docroot: root, - } - srv.Start() - // Add the testing repository as the only repo. - if err := setTestingRepository(srv.URL(), filepath.Join(root, "repositories.yaml")); err != nil { - panic(err) - } - return srv -} - -// Server is an implementation of a repository server for testing. -type Server struct { - docroot string - srv *httptest.Server - middleware http.HandlerFunc -} - -// WithMiddleware injects middleware in front of the server. This can be used to inject -// additional functionality like layering in an authentication frontend. -func (s *Server) WithMiddleware(middleware http.HandlerFunc) { - s.middleware = middleware -} - // Root gets the docroot for the server. func (s *Server) Root() string { return s.docroot @@ -348,50 +354,12 @@ func (s *Server) CreateIndex() error { return os.WriteFile(ifile, d, 0644) } -func (s *Server) Start() { - s.srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if s.middleware != nil { - s.middleware.ServeHTTP(w, r) - } - http.FileServer(http.Dir(s.docroot)).ServeHTTP(w, r) - })) -} - -func (s *Server) StartTLS() { - cd := "../../testdata" - ca, pub, priv := filepath.Join(cd, "rootca.crt"), filepath.Join(cd, "crt.pem"), filepath.Join(cd, "key.pem") - insecure := false - - s.srv = httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if s.middleware != nil { - s.middleware.ServeHTTP(w, r) - } - http.FileServer(http.Dir(s.Root())).ServeHTTP(w, r) - })) - tlsConf, err := tlsutil.NewTLSConfig( - tlsutil.WithInsecureSkipVerify(insecure), - tlsutil.WithCertKeyPairFiles(pub, priv), - tlsutil.WithCAFile(ca), - ) - if err != nil { - panic(err) - } - tlsConf.ServerName = "helm.sh" - s.srv.TLS = tlsConf - s.srv.StartTLS() - - // Set up repositories config with ca file - repoConfig := filepath.Join(s.Root(), "repositories.yaml") - - r := repo.NewFile() - r.Add(&repo.Entry{ - Name: "test", - URL: s.URL(), - CAFile: filepath.Join("../../testdata", "rootca.crt"), - }) - - if err := r.WriteFile(repoConfig, 0600); err != nil { - panic(err) +func (s *Server) start() { + if s.tlsConfig != nil { + s.srv.TLS = s.tlsConfig + s.srv.StartTLS() + } else { + s.srv.Start() } } @@ -411,6 +379,10 @@ func (s *Server) URL() string { return s.srv.URL } +func (s *Server) Client() *http.Client { + return s.srv.Client() +} + // LinkIndices links the index created with CreateIndex and makes a symbolic link to the cache index. // // This makes it possible to simulate a local cache of a repository. @@ -422,6 +394,10 @@ func (s *Server) LinkIndices() error { // setTestingRepository sets up a testing repository.yaml with only the given URL. func setTestingRepository(url, fname string) error { + if url == "" { + panic("no url") + } + r := repo.NewFile() r.Add(&repo.Entry{ Name: "test", diff --git a/pkg/repo/repotest/server_test.go b/pkg/repo/repotest/server_test.go index 6d15925db..cf68e5110 100644 --- a/pkg/repo/repotest/server_test.go +++ b/pkg/repo/repotest/server_test.go @@ -19,6 +19,7 @@ import ( "io" "net/http" "path/filepath" + "strings" "testing" "sigs.k8s.io/yaml" @@ -34,7 +35,7 @@ func TestServer(t *testing.T) { rootDir := t.TempDir() - srv := NewServer(rootDir) + srv := newServer(t, rootDir) defer srv.Stop() c, err := srv.CopyCharts("testdata/*.tgz") @@ -99,18 +100,123 @@ func TestServer(t *testing.T) { func TestNewTempServer(t *testing.T) { ensure.HelmHome(t) - srv, err := NewTempServerWithCleanup(t, "testdata/examplechart-0.1.0.tgz") - if err != nil { - t.Fatal(err) + type testCase struct { + options []ServerOption } - defer srv.Stop() - res, err := http.Head(srv.URL() + "/examplechart-0.1.0.tgz") - res.Body.Close() - if err != nil { - t.Error(err) + testCases := map[string]testCase{ + "plainhttp": { + options: []ServerOption{ + WithChartSourceGlob("testdata/examplechart-0.1.0.tgz"), + }, + }, + "tls": { + options: []ServerOption{ + WithChartSourceGlob("testdata/examplechart-0.1.0.tgz"), + WithTLSConfig(MakeTestTLSConfig(t, "../../../testdata")), + }, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + srv := NewTempServer( + t, + tc.options..., + ) + defer srv.Stop() + + if srv.srv.URL == "" { + t.Fatal("unstarted server") + } + + client := srv.Client() + + { + res, err := client.Head(srv.URL() + "/repositories.yaml") + if err != nil { + t.Error(err) + } + + res.Body.Close() + + if res.StatusCode != 200 { + t.Errorf("Expected 200, got %d", res.StatusCode) + } + + } + + { + res, err := client.Head(srv.URL() + "/examplechart-0.1.0.tgz") + if err != nil { + t.Error(err) + } + res.Body.Close() + + if res.StatusCode != 200 { + t.Errorf("Expected 200, got %d", res.StatusCode) + } + } + + res, err := client.Get(srv.URL() + "/examplechart-0.1.0.tgz") + res.Body.Close() + if err != nil { + t.Fatal(err) + } + + if res.ContentLength < 500 { + t.Errorf("Expected at least 500 bytes of data, got %d", res.ContentLength) + } + + res, err = client.Get(srv.URL() + "/index.yaml") + if err != nil { + t.Fatal(err) + } + + data, err := io.ReadAll(res.Body) + res.Body.Close() + if err != nil { + t.Fatal(err) + } + + m := repo.NewIndexFile() + if err := yaml.Unmarshal(data, m); err != nil { + t.Fatal(err) + } + + if l := len(m.Entries); l != 1 { + t.Fatalf("Expected 1 entry, got %d", l) + } + + expect := "examplechart" + if !m.Has(expect, "0.1.0") { + t.Errorf("missing %q", expect) + } + + res, err = client.Get(srv.URL() + "/index.yaml-nosuchthing") + res.Body.Close() + if err != nil { + t.Fatal(err) + } + if res.StatusCode != 404 { + t.Fatalf("Expected 404, got %d", res.StatusCode) + } + }) } - if res.StatusCode != 200 { - t.Errorf("Expected 200, got %d", res.StatusCode) + +} + +func TestNewTempServer_TLS(t *testing.T) { + ensure.HelmHome(t) + + srv := NewTempServer( + t, + WithChartSourceGlob("testdata/examplechart-0.1.0.tgz"), + WithTLSConfig(MakeTestTLSConfig(t, "../../../testdata")), + ) + defer srv.Stop() + + if !strings.HasPrefix(srv.URL(), "https://") { + t.Fatal("non-TLS server") } } diff --git a/pkg/repo/repotest/tlsconfig.go b/pkg/repo/repotest/tlsconfig.go new file mode 100644 index 000000000..3914a4d3f --- /dev/null +++ b/pkg/repo/repotest/tlsconfig.go @@ -0,0 +1,43 @@ +/* +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 repotest + +import ( + "crypto/tls" + "path/filepath" + "testing" + + "helm.sh/helm/v4/internal/tlsutil" + + "github.com/stretchr/testify/require" +) + +func MakeTestTLSConfig(t *testing.T, path string) *tls.Config { + ca, pub, priv := filepath.Join(path, "rootca.crt"), filepath.Join(path, "crt.pem"), filepath.Join(path, "key.pem") + + insecure := false + tlsConf, err := tlsutil.NewTLSConfig( + tlsutil.WithInsecureSkipVerify(insecure), + tlsutil.WithCertKeyPairFiles(pub, priv), + tlsutil.WithCAFile(ca), + ) + //require.Nil(t, err, err.Error()) + require.Nil(t, err) + + tlsConf.ServerName = "helm.sh" + + return tlsConf +} diff --git a/scripts/get-helm-3 b/scripts/get-helm-3 index 5f19bd3fa..3aa44daee 100755 --- a/scripts/get-helm-3 +++ b/scripts/get-helm-3 @@ -279,7 +279,7 @@ testVersion() { help () { echo "Accepted cli arguments are:" echo -e "\t[--help|-h ] ->> prints this help" - echo -e "\t[--version|-v ] . When not defined it fetches the latest release from GitHub" + echo -e "\t[--version|-v ] . When not defined it fetches the latest release tag from the Helm CDN" echo -e "\te.g. --version v3.0.0 or -v canary" echo -e "\t[--no-sudo] ->> install without sudo" }