From b305a501e84d4a6a4a881b3550bcd6b2d7424490 Mon Sep 17 00:00:00 2001 From: Ashmit Bhardwaj Date: Thu, 13 Mar 2025 13:28:21 +0000 Subject: [PATCH 1/9] added documentation and test cases for api-versions flag Signed-off-by: Ashmit Bhardwaj --- pkg/cmd/template.go | 2 +- pkg/cmd/template_test.go | 7 ++++++- .../output/template-with-api-version.txt | 1 + .../issue-7233/charts/alpine-0.1.0.tgz | Bin 0 -> 1166 bytes .../testcharts/subchart/templates/service.yaml | 3 +++ 5 files changed, 11 insertions(+), 2 deletions(-) create mode 100644 pkg/cmd/testdata/testcharts/issue-7233/charts/alpine-0.1.0.tgz diff --git a/pkg/cmd/template.go b/pkg/cmd/template.go index 7a565ef85..bb0319264 100644 --- a/pkg/cmd/template.go +++ b/pkg/cmd/template.go @@ -201,7 +201,7 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { f.BoolVar(&skipTests, "skip-tests", false, "skip tests from templated output") f.BoolVar(&client.IsUpgrade, "is-upgrade", false, "set .Release.IsUpgrade instead of .Release.IsInstall") f.StringVar(&kubeVersion, "kube-version", "", "Kubernetes version used for Capabilities.KubeVersion") - f.StringSliceVarP(&extraAPIs, "api-versions", "a", []string{}, "Kubernetes api versions used for Capabilities.APIVersions") + f.StringSliceVarP(&extraAPIs, "api-versions", "a", []string{}, "Kubernetes api versions used for Capabilities.APIVersions ( Can specify multiple or separate values with commands. Example --api-versions value1 --api-versions value2 or --api-versions value1, value2)") f.BoolVar(&client.UseReleaseName, "release-name", false, "use release name in the output-dir path.") bindPostRenderFlag(cmd, &client.PostRenderer) diff --git a/pkg/cmd/template_test.go b/pkg/cmd/template_test.go index a6c848e08..5bcccf5d0 100644 --- a/pkg/cmd/template_test.go +++ b/pkg/cmd/template_test.go @@ -83,7 +83,12 @@ func TestTemplateCmd(t *testing.T) { }, { name: "check kube api versions", - cmd: fmt.Sprintf("template --api-versions helm.k8s.io/test '%s'", chartPath), + cmd: fmt.Sprintf("template --api-versions helm.k8s.io/test,helm.k8s.io/test2 '%s'", chartPath), + golden: "output/template-with-api-version.txt", + }, + { + name: "check kube api versions", + cmd: fmt.Sprintf("template --api-versions helm.k8s.io/test --api-versions helm.k8s.io/test2 '%s'", chartPath), golden: "output/template-with-api-version.txt", }, { diff --git a/pkg/cmd/testdata/output/template-with-api-version.txt b/pkg/cmd/testdata/output/template-with-api-version.txt index 7e1c35001..8b6074cdb 100644 --- a/pkg/cmd/testdata/output/template-with-api-version.txt +++ b/pkg/cmd/testdata/output/template-with-api-version.txt @@ -75,6 +75,7 @@ metadata: kube-version/minor: "20" kube-version/version: "v1.20.0" kube-api-version/test: v1 + kube-api-version/test2: v2 spec: type: ClusterIP ports: diff --git a/pkg/cmd/testdata/testcharts/issue-7233/charts/alpine-0.1.0.tgz b/pkg/cmd/testdata/testcharts/issue-7233/charts/alpine-0.1.0.tgz new file mode 100644 index 0000000000000000000000000000000000000000..87db21817badafc36ad9eb7b7a12e35261c72202 GIT binary patch literal 1166 zcmV;91abQxiwG0|00000|0w_~VMtOiV@ORlOnEsqVl!4SWK%V1T2nbTPgYhoO;>Dc zVQyr3R8em|NM&qo0PI-XZsRr+&b6LmpuJto@+rakF8FtWf6;caNQ(uEqNvBx*djua zDoI&!y?*zC2RX6QO$v1F^q{#BM3En7hV#t~g}h>dhW{&2nPiJ4zxeu+rfGUPow{e5 zrrl>cp3N>s)2qwrY&M!)rWfgGJe^%$KzagQ9!aeYs*Ch5@6|Q#A0k+>Poy-HVh)>8 zgjIETjVH;QILb+9idDu9y`_rFEg&pWvkb0X@W8iB)OS$HJSt>Kb100d^n5rhh?j{j z+%pnrKlyNrw5(M|dL9i9lh@1?^)kt1>E`=In^u=J86|1-($4x9O5pznc}@C63E(;Y zr{lEC|EqM|^Zy+3!O^nZ4gxb@TAyCdD}n!p%8H{QJ@f;EO6FfJ8$84R@nkyg@P9U) z_WVDKbn5>t6ZsrI$~-E`V2%do5rD7V@otyg5GT}>*tD_V4|cocldl|pAdUbyE{SWq z4B!>~P!PmeRmp9=Bqhj@PM92)C~!^rV7ZO`0$>_yO*t3|MqzdfO~JAPjFdb;<*xal z91zxQLjp3_w8DE67O!d!E2#kUk_in>A)!HGETkrara%**ls7{ILWRn(tmdfPUKUD` zuB6ax$;sj+ZqZXV%;AA+z9+R|8Np}xwU&lpzkk{lOlX55qZl$NHY9AP1ts2+24;Xz zZ=FvV0k(PvKqCP;2e|@M5PWk$wFdYG3rv9q8d3zAA~e={+L`QEbsh30Q(GO647Eqz zwes=OLkqXtLE^6&*M}zap>vpiY&F(7Gya^R*0>G|wvDCx-~-bm--o4t58L>_1n^ys zGvX+0BVAEuBR1%v3)yHWWMg%xwe`GaWyQ%OBz{)II0r7}tq(e=P;1HuO!!$2E4jF;I0;hp8*SpL9n>* zK^KrSMVXOm+olSe*EXcKt&&!kw#-4pOaT~WvbLY(%2qfm$&k`5_=5}X0#=OEJ@BJ! zt6f63d2-R|h8NH(nMBc#vW5%=xZ=n;n1_z=rh9YS3CZwa@k<~3K?48j&M5rlM1KLE zz5h=p9sW;8qfyWQbBG~rYz_AR0bCdid>H(FVzigjN#MV222Vx*Kf(X*{eN;bzUujZ g4msw(4TGNkz4X#cFaHJkJpcgz|4sJjy8s{n0PDUu(*OVf literal 0 HcmV?d00001 diff --git a/pkg/cmd/testdata/testcharts/subchart/templates/service.yaml b/pkg/cmd/testdata/testcharts/subchart/templates/service.yaml index fee94dced..19c931cc3 100644 --- a/pkg/cmd/testdata/testcharts/subchart/templates/service.yaml +++ b/pkg/cmd/testdata/testcharts/subchart/templates/service.yaml @@ -11,6 +11,9 @@ metadata: {{- if .Capabilities.APIVersions.Has "helm.k8s.io/test" }} kube-api-version/test: v1 {{- end }} +{{- if .Capabilities.APIVersions.Has "helm.k8s.io/test2" }} + kube-api-version/test2: v2 +{{- end }} spec: type: {{ .Values.service.type }} ports: From 0389407cbc7e0a11b3e3c11a5654fb7c93976f97 Mon Sep 17 00:00:00 2001 From: Ashmit Bhardwaj Date: Wed, 30 Apr 2025 07:43:40 +0000 Subject: [PATCH 2/9] removed unnecessary binary file Signed-off-by: Ashmit Bhardwaj --- .../issue-7233/charts/alpine-0.1.0.tgz | Bin 1166 -> 0 bytes 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 pkg/cmd/testdata/testcharts/issue-7233/charts/alpine-0.1.0.tgz diff --git a/pkg/cmd/testdata/testcharts/issue-7233/charts/alpine-0.1.0.tgz b/pkg/cmd/testdata/testcharts/issue-7233/charts/alpine-0.1.0.tgz deleted file mode 100644 index 87db21817badafc36ad9eb7b7a12e35261c72202..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 1166 zcmV;91abQxiwG0|00000|0w_~VMtOiV@ORlOnEsqVl!4SWK%V1T2nbTPgYhoO;>Dc zVQyr3R8em|NM&qo0PI-XZsRr+&b6LmpuJto@+rakF8FtWf6;caNQ(uEqNvBx*djua zDoI&!y?*zC2RX6QO$v1F^q{#BM3En7hV#t~g}h>dhW{&2nPiJ4zxeu+rfGUPow{e5 zrrl>cp3N>s)2qwrY&M!)rWfgGJe^%$KzagQ9!aeYs*Ch5@6|Q#A0k+>Poy-HVh)>8 zgjIETjVH;QILb+9idDu9y`_rFEg&pWvkb0X@W8iB)OS$HJSt>Kb100d^n5rhh?j{j z+%pnrKlyNrw5(M|dL9i9lh@1?^)kt1>E`=In^u=J86|1-($4x9O5pznc}@C63E(;Y zr{lEC|EqM|^Zy+3!O^nZ4gxb@TAyCdD}n!p%8H{QJ@f;EO6FfJ8$84R@nkyg@P9U) z_WVDKbn5>t6ZsrI$~-E`V2%do5rD7V@otyg5GT}>*tD_V4|cocldl|pAdUbyE{SWq z4B!>~P!PmeRmp9=Bqhj@PM92)C~!^rV7ZO`0$>_yO*t3|MqzdfO~JAPjFdb;<*xal z91zxQLjp3_w8DE67O!d!E2#kUk_in>A)!HGETkrara%**ls7{ILWRn(tmdfPUKUD` zuB6ax$;sj+ZqZXV%;AA+z9+R|8Np}xwU&lpzkk{lOlX55qZl$NHY9AP1ts2+24;Xz zZ=FvV0k(PvKqCP;2e|@M5PWk$wFdYG3rv9q8d3zAA~e={+L`QEbsh30Q(GO647Eqz zwes=OLkqXtLE^6&*M}zap>vpiY&F(7Gya^R*0>G|wvDCx-~-bm--o4t58L>_1n^ys zGvX+0BVAEuBR1%v3)yHWWMg%xwe`GaWyQ%OBz{)II0r7}tq(e=P;1HuO!!$2E4jF;I0;hp8*SpL9n>* zK^KrSMVXOm+olSe*EXcKt&&!kw#-4pOaT~WvbLY(%2qfm$&k`5_=5}X0#=OEJ@BJ! zt6f63d2-R|h8NH(nMBc#vW5%=xZ=n;n1_z=rh9YS3CZwa@k<~3K?48j&M5rlM1KLE zz5h=p9sW;8qfyWQbBG~rYz_AR0bCdid>H(FVzigjN#MV222Vx*Kf(X*{eN;bzUujZ g4msw(4TGNkz4X#cFaHJkJpcgz|4sJjy8s{n0PDUu(*OVf From e060fbe1851221487394bd56de631522812732d3 Mon Sep 17 00:00:00 2001 From: Ashmit Bhardwaj Date: Mon, 9 Jun 2025 06:11:05 +0000 Subject: [PATCH 3/9] updated docs Signed-off-by: Ashmit Bhardwaj --- pkg/cmd/template.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/template.go b/pkg/cmd/template.go index bb0319264..ac20a45b3 100644 --- a/pkg/cmd/template.go +++ b/pkg/cmd/template.go @@ -201,7 +201,7 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { f.BoolVar(&skipTests, "skip-tests", false, "skip tests from templated output") f.BoolVar(&client.IsUpgrade, "is-upgrade", false, "set .Release.IsUpgrade instead of .Release.IsInstall") f.StringVar(&kubeVersion, "kube-version", "", "Kubernetes version used for Capabilities.KubeVersion") - f.StringSliceVarP(&extraAPIs, "api-versions", "a", []string{}, "Kubernetes api versions used for Capabilities.APIVersions ( Can specify multiple or separate values with commands. Example --api-versions value1 --api-versions value2 or --api-versions value1, value2)") + f.StringSliceVarP(&extraAPIs, "api-versions", "a", []string{}, "Kubernetes api versions used for Capabilities.APIVersions (multiple can be specified)") f.BoolVar(&client.UseReleaseName, "release-name", false, "use release name in the output-dir path.") bindPostRenderFlag(cmd, &client.PostRenderer) From afd63fed77909448d3ff08720d1898a63b90fc13 Mon Sep 17 00:00:00 2001 From: Thiago Presa Date: Thu, 26 Jun 2025 20:24:25 -0300 Subject: [PATCH 4/9] test: increase test coverage for pkg/pusher Signed-off-by: Thiago Presa --- pkg/pusher/ocipusher_test.go | 332 +++++++++++++++++++++++++++++++++++ 1 file changed, 332 insertions(+) diff --git a/pkg/pusher/ocipusher_test.go b/pkg/pusher/ocipusher_test.go index 760da8404..24f52a7ad 100644 --- a/pkg/pusher/ocipusher_test.go +++ b/pkg/pusher/ocipusher_test.go @@ -1,3 +1,5 @@ +//go:build !windows + /* Copyright The Helm Authors. Licensed under the Apache License, Version 2.0 (the "License"); @@ -16,7 +18,10 @@ limitations under the License. package pusher import ( + "io" + "os" "path/filepath" + "strings" "testing" "helm.sh/helm/v4/pkg/registry" @@ -94,3 +99,330 @@ func TestNewOCIPusher(t *testing.T) { t.Errorf("Expected NewOCIPusher to contain %p as RegistryClient, got %p", registryClient, op.opts.registryClient) } } + +func TestOCIPusher_Push_ErrorHandling(t *testing.T) { + tests := []struct { + name string + chartRef string + expectedError string + setupFunc func() string + }{ + { + name: "non-existent file", + chartRef: "/non/existent/file.tgz", + expectedError: "no such file", + }, + { + name: "directory instead of file", + expectedError: "cannot push directory, must provide chart archive (.tgz)", + setupFunc: func() string { + tempDir := t.TempDir() + return tempDir + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pusher, err := NewOCIPusher() + if err != nil { + t.Fatal(err) + } + + chartRef := tt.chartRef + if tt.setupFunc != nil { + chartRef = tt.setupFunc() + } + + err = pusher.Push(chartRef, "oci://localhost:5000/test") + if err == nil { + t.Fatal("Expected error but got none") + } + + if !strings.Contains(err.Error(), tt.expectedError) { + t.Errorf("Expected error containing %q, got %q", tt.expectedError, err.Error()) + } + }) + } +} + +func TestOCIPusher_newRegistryClient(t *testing.T) { + cd := "../../testdata" + join := filepath.Join + ca, pub, priv := join(cd, "rootca.crt"), join(cd, "crt.pem"), join(cd, "key.pem") + + tests := []struct { + name string + opts []Option + expectError bool + errorContains string + }{ + { + name: "plain HTTP", + opts: []Option{WithPlainHTTP(true)}, + }, + { + name: "with TLS client config", + opts: []Option{ + WithTLSClientConfig(pub, priv, ca), + }, + }, + { + name: "with insecure skip TLS verify", + opts: []Option{ + WithInsecureSkipTLSVerify(true), + }, + }, + { + name: "with cert and key only", + opts: []Option{ + WithTLSClientConfig(pub, priv, ""), + }, + }, + { + name: "with CA file only", + opts: []Option{ + WithTLSClientConfig("", "", ca), + }, + }, + { + name: "default client without options", + opts: []Option{}, + }, + { + name: "invalid cert file", + opts: []Option{ + WithTLSClientConfig("/non/existent/cert.pem", priv, ca), + }, + expectError: true, + errorContains: "can't create TLS config", + }, + { + name: "invalid key file", + opts: []Option{ + WithTLSClientConfig(pub, "/non/existent/key.pem", ca), + }, + expectError: true, + errorContains: "can't create TLS config", + }, + { + name: "invalid CA file", + opts: []Option{ + WithTLSClientConfig("", "", "/non/existent/ca.crt"), + }, + expectError: true, + errorContains: "can't create TLS config", + }, + { + name: "combined TLS options", + opts: []Option{ + WithTLSClientConfig(pub, priv, ca), + WithInsecureSkipTLSVerify(true), + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pusher, err := NewOCIPusher(tt.opts...) + if err != nil { + t.Fatal(err) + } + + op, ok := pusher.(*OCIPusher) + if !ok { + t.Fatal("Expected *OCIPusher") + } + + client, err := op.newRegistryClient() + if tt.expectError { + if err == nil { + t.Fatal("Expected error but got none") + } + if tt.errorContains != "" && !strings.Contains(err.Error(), tt.errorContains) { + t.Errorf("Expected error containing %q, got %q", tt.errorContains, err.Error()) + } + } else { + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if client == nil { + t.Fatal("Expected non-nil registry client") + } + } + }) + } +} + +func TestOCIPusher_Push_ChartOperations(t *testing.T) { + // Path to test charts + chartPath := "../../pkg/cmd/testdata/testcharts/compressedchart-0.1.0.tgz" + chartWithProvPath := "../../pkg/cmd/testdata/testcharts/signtest-0.1.0.tgz" + + tests := []struct { + name string + chartRef string + href string + options []Option + setupFunc func(t *testing.T) (string, func()) + expectError bool + errorContains string + }{ + { + name: "invalid chart file", + chartRef: "../../pkg/action/testdata/charts/corrupted-compressed-chart.tgz", + href: "oci://localhost:5000/test", + expectError: true, + errorContains: "does not appear to be a gzipped archive", + }, + { + name: "chart read error", + setupFunc: func(t *testing.T) (string, func()) { + t.Helper() + // Create a valid chart file that we'll make unreadable + tempDir := t.TempDir() + tempChart := filepath.Join(tempDir, "temp-chart.tgz") + + // Copy a valid chart + src, err := os.Open(chartPath) + if err != nil { + t.Fatal(err) + } + defer src.Close() + + dst, err := os.Create(tempChart) + if err != nil { + t.Fatal(err) + } + + if _, err := io.Copy(dst, src); err != nil { + t.Fatal(err) + } + dst.Close() + + // Make the file unreadable + if err := os.Chmod(tempChart, 0000); err != nil { + t.Fatal(err) + } + + return tempChart, func() { + os.Chmod(tempChart, 0644) // Restore permissions for cleanup + } + }, + href: "oci://localhost:5000/test", + expectError: true, + errorContains: "permission denied", + }, + { + name: "push with provenance file - loading phase", + chartRef: chartWithProvPath, + href: "oci://registry.example.com/charts", + setupFunc: func(t *testing.T) (string, func()) { + t.Helper() + // Copy chart and create a .prov file for it + tempDir := t.TempDir() + tempChart := filepath.Join(tempDir, "signtest-0.1.0.tgz") + tempProv := filepath.Join(tempDir, "signtest-0.1.0.tgz.prov") + + // Copy chart file + src, err := os.Open(chartWithProvPath) + if err != nil { + t.Fatal(err) + } + defer src.Close() + + dst, err := os.Create(tempChart) + if err != nil { + t.Fatal(err) + } + + if _, err := io.Copy(dst, src); err != nil { + t.Fatal(err) + } + dst.Close() + + // Create provenance file + if err := os.WriteFile(tempProv, []byte("test provenance data"), 0644); err != nil { + t.Fatal(err) + } + + return tempChart, func() {} + }, + expectError: true, // Will fail at the registry push step + errorContains: "", // Error depends on registry client behavior + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + chartRef := tt.chartRef + var cleanup func() + + if tt.setupFunc != nil { + chartRef, cleanup = tt.setupFunc(t) + if cleanup != nil { + defer cleanup() + } + } + + // Skip test if chart file doesn't exist and we're not expecting an error + if _, err := os.Stat(chartRef); err != nil && !tt.expectError { + t.Skipf("Test chart %s not found, skipping test", chartRef) + } + + pusher, err := NewOCIPusher(tt.options...) + if err != nil { + t.Fatal(err) + } + + err = pusher.Push(chartRef, tt.href) + + if tt.expectError { + if err == nil { + t.Fatal("Expected error but got none") + } + if tt.errorContains != "" && !strings.Contains(err.Error(), tt.errorContains) { + t.Errorf("Expected error containing %q, got %q", tt.errorContains, err.Error()) + } + } else { + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + } + }) + } +} + +func TestOCIPusher_Push_MultipleOptions(t *testing.T) { + chartPath := "../../pkg/cmd/testdata/testcharts/compressedchart-0.1.0.tgz" + + // Skip test if chart file doesn't exist + if _, err := os.Stat(chartPath); err != nil { + t.Skipf("Test chart %s not found, skipping test", chartPath) + } + + pusher, err := NewOCIPusher() + if err != nil { + t.Fatal(err) + } + + // Test that multiple options are applied correctly + err = pusher.Push(chartPath, "oci://localhost:5000/test", + WithPlainHTTP(true), + WithInsecureSkipTLSVerify(true), + ) + + // We expect an error since we're not actually pushing to a registry + if err == nil { + t.Fatal("Expected error when pushing without a valid registry") + } + + // Verify options were applied + op := pusher.(*OCIPusher) + if !op.opts.plainHTTP { + t.Error("Expected plainHTTP option to be applied") + } + if !op.opts.insecureSkipTLSverify { + t.Error("Expected insecureSkipTLSverify option to be applied") + } +} From 3b26ddc22ba4c99292dba724170b2e30a94aebef Mon Sep 17 00:00:00 2001 From: Zach Burgess Date: Tue, 1 Jul 2025 15:29:19 -0700 Subject: [PATCH 5/9] Update tests in create_test.go and package_test.go to work in a temp dir. Signed-off-by: Zach Burgess --- pkg/cmd/create_test.go | 8 +++----- pkg/cmd/package_test.go | 3 +-- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/create_test.go b/pkg/cmd/create_test.go index 103cd3bc0..2a4ca95ca 100644 --- a/pkg/cmd/create_test.go +++ b/pkg/cmd/create_test.go @@ -30,10 +30,9 @@ import ( ) func TestCreateCmd(t *testing.T) { + t.Chdir(t.TempDir()) ensure.HelmHome(t) cname := "testchart" - dir := t.TempDir() - defer t.Chdir(dir) // Run a create if _, _, err := executeActionCommand("create " + cname); err != nil { @@ -64,9 +63,7 @@ func TestCreateStarterCmd(t *testing.T) { ensure.HelmHome(t) cname := "testchart" defer resetEnv()() - os.MkdirAll(helmpath.CachePath(), 0o755) - defer t.Chdir(helmpath.CachePath()) - + t.Chdir(t.TempDir()) // Create a starter. starterchart := helmpath.DataPath("starters") os.MkdirAll(starterchart, 0o755) @@ -125,6 +122,7 @@ func TestCreateStarterCmd(t *testing.T) { } func TestCreateStarterAbsoluteCmd(t *testing.T) { + t.Chdir(t.TempDir()) defer resetEnv()() ensure.HelmHome(t) cname := "testchart" diff --git a/pkg/cmd/package_test.go b/pkg/cmd/package_test.go index b17684aa6..349cb662f 100644 --- a/pkg/cmd/package_test.go +++ b/pkg/cmd/package_test.go @@ -110,8 +110,7 @@ func TestPackage(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - cachePath := t.TempDir() - defer t.Chdir(cachePath) + t.Chdir(t.TempDir()) if err := os.MkdirAll("toot", 0o777); err != nil { t.Fatal(err) From c48a3435f526ccfd064d44a0e73dbdc5f4516f4f Mon Sep 17 00:00:00 2001 From: Zach Burgess Date: Wed, 2 Jul 2025 09:30:59 -0700 Subject: [PATCH 6/9] Remove unnecessary calls for changing directory to `helmpath.CachePath`. This was only set on some tests in create_test.go and isn't affecting the test. Signed-off-by: Zach Burgess --- pkg/cmd/create_test.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/cmd/create_test.go b/pkg/cmd/create_test.go index 2a4ca95ca..90ed90eff 100644 --- a/pkg/cmd/create_test.go +++ b/pkg/cmd/create_test.go @@ -60,10 +60,10 @@ func TestCreateCmd(t *testing.T) { } func TestCreateStarterCmd(t *testing.T) { + t.Chdir(t.TempDir()) ensure.HelmHome(t) cname := "testchart" defer resetEnv()() - t.Chdir(t.TempDir()) // Create a starter. starterchart := helmpath.DataPath("starters") os.MkdirAll(starterchart, 0o755) @@ -140,9 +140,6 @@ func TestCreateStarterAbsoluteCmd(t *testing.T) { t.Fatalf("Could not write template: %s", err) } - os.MkdirAll(helmpath.CachePath(), 0o755) - defer t.Chdir(helmpath.CachePath()) - starterChartPath := filepath.Join(starterchart, "starterchart") // Run a create From eaf40b4b4fba2006df55e5f3d8014641a97045ac Mon Sep 17 00:00:00 2001 From: Zach Burgess Date: Wed, 2 Jul 2025 16:20:38 -0700 Subject: [PATCH 7/9] Call `ensure.HelmHome()` in package_test.go Signed-off-by: Zach Burgess --- pkg/cmd/package_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/cmd/package_test.go b/pkg/cmd/package_test.go index 349cb662f..db4a2523a 100644 --- a/pkg/cmd/package_test.go +++ b/pkg/cmd/package_test.go @@ -23,6 +23,7 @@ import ( "strings" "testing" + "helm.sh/helm/v4/internal/test/ensure" chart "helm.sh/helm/v4/pkg/chart/v2" "helm.sh/helm/v4/pkg/chart/v2/loader" ) @@ -111,6 +112,7 @@ func TestPackage(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Chdir(t.TempDir()) + ensure.HelmHome(t) if err := os.MkdirAll("toot", 0o777); err != nil { t.Fatal(err) From 5283915c57ba2f474f74f76987c14b9c770746c0 Mon Sep 17 00:00:00 2001 From: George Jenkins Date: Wed, 2 Jul 2025 20:18:34 -0700 Subject: [PATCH 8/9] Remove deprecated '--create-pods' flag Signed-off-by: George Jenkins --- pkg/action/rollback.go | 10 --------- pkg/action/upgrade.go | 49 ------------------------------------------ pkg/cmd/rollback.go | 1 - pkg/cmd/upgrade.go | 2 -- 4 files changed, 62 deletions(-) diff --git a/pkg/action/rollback.go b/pkg/action/rollback.go index ac8a28fe0..1dc0c7f84 100644 --- a/pkg/action/rollback.go +++ b/pkg/action/rollback.go @@ -41,7 +41,6 @@ type Rollback struct { WaitForJobs bool DisableHooks bool DryRun bool - Recreate bool // will (if true) recreate pods after a rollback. Force bool // will (if true) force resource upgrade through uninstall/recreate if needed CleanupOnFail bool MaxHistory int // MaxHistory limits the maximum number of revisions saved per release @@ -211,15 +210,6 @@ func (r *Rollback) performRollback(currentRelease, targetRelease *release.Releas return targetRelease, err } - if r.Recreate { - // NOTE: Because this is not critical for a release to succeed, we just - // log if an error occurs and continue onward. If we ever introduce log - // levels, we should make these error level logs so users are notified - // that they'll need to go do the cleanup on their own - if err := recreate(r.cfg, results.Updated); err != nil { - slog.Error(err.Error()) - } - } waiter, err := r.cfg.KubeClient.GetWaiter(r.WaitStrategy) if err != nil { return nil, fmt.Errorf("unable to set metadata visitor from target release: %w", err) diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index e2d2ead69..271bc8aa9 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -26,7 +26,6 @@ import ( "sync" "time" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/cli-runtime/pkg/resource" chart "helm.sh/helm/v4/pkg/chart/v2" @@ -88,8 +87,6 @@ type Upgrade struct { ReuseValues bool // ResetThenReuseValues will reset the values to the chart's built-ins then merge with user's last supplied values. ResetThenReuseValues bool - // Recreate will (if true) recreate pods after a rollback. - Recreate bool // MaxHistory limits the maximum number of revisions saved per release MaxHistory int // Atomic, if true, will roll back on failure. @@ -436,15 +433,6 @@ func (u *Upgrade) releasingUpgrade(c chan<- resultMessage, upgradedRelease *rele return } - if u.Recreate { - // NOTE: Because this is not critical for a release to succeed, we just - // log if an error occurs and continue onward. If we ever introduce log - // levels, we should make these error level logs so users are notified - // that they'll need to go do the cleanup on their own - if err := recreate(u.cfg, results.Updated); err != nil { - slog.Error(err.Error()) - } - } waiter, err := u.cfg.KubeClient.GetWaiter(u.WaitStrategy) if err != nil { u.cfg.recordRelease(originalRelease) @@ -537,7 +525,6 @@ func (u *Upgrade) failRelease(rel *release.Release, created kube.ResourceList, e } rollin.WaitForJobs = u.WaitForJobs rollin.DisableHooks = u.DisableHooks - rollin.Recreate = u.Recreate rollin.Force = u.Force rollin.Timeout = u.Timeout if rollErr := rollin.Run(rel.Name); rollErr != nil { @@ -602,42 +589,6 @@ func validateManifest(c kube.Interface, manifest []byte, openAPIValidation bool) return err } -// recreate captures all the logic for recreating pods for both upgrade and -// rollback. If we end up refactoring rollback to use upgrade, this can just be -// made an unexported method on the upgrade action. -func recreate(cfg *Configuration, resources kube.ResourceList) error { - for _, res := range resources { - versioned := kube.AsVersioned(res) - selector, err := kube.SelectorsForObject(versioned) - if err != nil { - // If no selector is returned, it means this object is - // definitely not a pod, so continue onward - continue - } - - client, err := cfg.KubernetesClientSet() - if err != nil { - return fmt.Errorf("unable to recreate pods for object %s/%s because an error occurred: %w", res.Namespace, res.Name, err) - } - - pods, err := client.CoreV1().Pods(res.Namespace).List(context.Background(), metav1.ListOptions{ - LabelSelector: selector.String(), - }) - if err != nil { - return fmt.Errorf("unable to recreate pods for object %s/%s because an error occurred: %w", res.Namespace, res.Name, err) - } - - // Restart pods - for _, pod := range pods.Items { - // Delete each pod for get them restarted with changed spec. - if err := client.CoreV1().Pods(pod.Namespace).Delete(context.Background(), pod.Name, *metav1.NewPreconditionDeleteOptions(string(pod.UID))); err != nil { - return fmt.Errorf("unable to recreate pods for object %s/%s because an error occurred: %w", res.Namespace, res.Name, err) - } - } - } - return nil -} - func objectKey(r *resource.Info) string { gvk := r.Object.GetObjectKind().GroupVersionKind() return fmt.Sprintf("%s/%s/%s/%s", gvk.GroupVersion().String(), gvk.Kind, r.Namespace, r.Name) diff --git a/pkg/cmd/rollback.go b/pkg/cmd/rollback.go index 1823432dc..6658d3fd6 100644 --- a/pkg/cmd/rollback.go +++ b/pkg/cmd/rollback.go @@ -77,7 +77,6 @@ func newRollbackCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { f := cmd.Flags() f.BoolVar(&client.DryRun, "dry-run", false, "simulate a rollback") - f.BoolVar(&client.Recreate, "recreate-pods", false, "performs pods restart for the resource if applicable") f.BoolVar(&client.Force, "force", false, "force resource update through delete/recreate if needed") f.BoolVar(&client.DisableHooks, "no-hooks", false, "prevent hooks from running during rollback") f.DurationVar(&client.Timeout, "timeout", 300*time.Second, "time to wait for any individual Kubernetes operation (like Jobs for hooks)") diff --git a/pkg/cmd/upgrade.go b/pkg/cmd/upgrade.go index b93fa6e64..d4e7b4852 100644 --- a/pkg/cmd/upgrade.go +++ b/pkg/cmd/upgrade.go @@ -268,8 +268,6 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { f.StringVar(&client.DryRunOption, "dry-run", "", "simulate an install. If --dry-run is set with no option being specified or as '--dry-run=client', it will not attempt cluster connections. Setting '--dry-run=server' allows attempting cluster connections.") f.BoolVar(&client.HideSecret, "hide-secret", false, "hide Kubernetes Secrets when also using the --dry-run flag") f.Lookup("dry-run").NoOptDefVal = "client" - f.BoolVar(&client.Recreate, "recreate-pods", false, "performs pods restart for the resource if applicable") - f.MarkDeprecated("recreate-pods", "functionality will no longer be updated. Consult the documentation for other methods to recreate pods") f.BoolVar(&client.Force, "force", false, "force resource updates through a replacement strategy") f.BoolVar(&client.DisableHooks, "no-hooks", false, "disable pre/post upgrade hooks") f.BoolVar(&client.DisableOpenAPIValidation, "disable-openapi-validation", false, "if set, the upgrade process will not validate rendered templates against the Kubernetes OpenAPI Schema") From a3bcc5b1847e9ed9a3f9334cbdeebc787063b865 Mon Sep 17 00:00:00 2001 From: George Jenkins Date: Wed, 2 Jul 2025 21:14:22 -0700 Subject: [PATCH 9/9] fix: 'TestRunLinterRule' stateful test Signed-off-by: George Jenkins --- pkg/lint/support/message_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/lint/support/message_test.go b/pkg/lint/support/message_test.go index 55675eeee..ce5b5e42e 100644 --- a/pkg/lint/support/message_test.go +++ b/pkg/lint/support/message_test.go @@ -21,7 +21,6 @@ import ( "testing" ) -var linter = Linter{} var errLint = errors.New("lint failed") func TestRunLinterRule(t *testing.T) { @@ -45,6 +44,7 @@ func TestRunLinterRule(t *testing.T) { {-1, errLint, 4, false, ErrorSev}, } + linter := Linter{} for _, test := range tests { isValid := linter.RunLinterRule(test.Severity, "chart", test.LintError) if len(linter.Messages) != test.ExpectedMessages {