From bebe006405a5483a0aa743c7203a08bc6c9db29a Mon Sep 17 00:00:00 2001 From: Hathoute Date: Sun, 16 Nov 2025 02:16:12 +0100 Subject: [PATCH 1/3] fix: include version test for dependency-update flag Closes #31455 Signed-off-by: Hathoute --- internal/test/test.go | 31 ++++++++++++++++--- pkg/action/dependency_test.go | 2 +- pkg/action/install.go | 2 +- pkg/chart/common.go | 8 +++++ pkg/chart/dependency.go | 8 +++++ pkg/chart/interfaces.go | 2 ++ pkg/cmd/helpers_test.go | 11 ++++--- pkg/cmd/install_test.go | 9 ++++++ .../output/install-with-outdated-deps.txt | 12 +++++++ .../.helmignore | 21 +++++++++++++ .../chart-with-outdated-dependency/Chart.yaml | 8 +++++ .../charts/oci-dependent-chart/.helmignore | 23 ++++++++++++++ .../charts/oci-dependent-chart/Chart.yaml | 6 ++++ .../oci-dependent-chart/templates/NOTES.txt | 22 +++++++++++++ .../templates/tests/test-connection.yaml | 15 +++++++++ .../charts/oci-dependent-chart/values.yaml | 3 ++ .../values.yaml | 4 +++ 17 files changed, 176 insertions(+), 11 deletions(-) create mode 100644 pkg/cmd/testdata/output/install-with-outdated-deps.txt create mode 100644 pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/.helmignore create mode 100644 pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/Chart.yaml create mode 100644 pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/charts/oci-dependent-chart/.helmignore create mode 100644 pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/charts/oci-dependent-chart/Chart.yaml create mode 100644 pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/charts/oci-dependent-chart/templates/NOTES.txt create mode 100644 pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/charts/oci-dependent-chart/templates/tests/test-connection.yaml create mode 100644 pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/charts/oci-dependent-chart/values.yaml create mode 100644 pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/values.yaml diff --git a/internal/test/test.go b/internal/test/test.go index 202e015ab..1a7cce735 100644 --- a/internal/test/test.go +++ b/internal/test/test.go @@ -22,6 +22,7 @@ import ( "fmt" "os" "path/filepath" + "text/template" ) // UpdateGolden writes out the golden files with the latest values, rather than failing the test. @@ -40,10 +41,10 @@ type HelperT interface { } // AssertGoldenString asserts that the given string matches the contents of the given file. -func AssertGoldenString(t TestingT, actual, filename string) { +func AssertGoldenString(t TestingT, actual, filename string, goldenArgs map[string]string) { t.Helper() - if err := compare([]byte(actual), path(filename)); err != nil { + if err := compare([]byte(actual), path(filename), goldenArgs); err != nil { t.Fatalf("%v\n", err) } } @@ -56,7 +57,7 @@ func AssertGoldenFile(t TestingT, actualFileName string, expectedFilename string if err != nil { t.Fatalf("%v", err) } - AssertGoldenString(t, string(actual), expectedFilename) + AssertGoldenString(t, string(actual), expectedFilename, map[string]string{}) } func path(filename string) string { @@ -66,7 +67,7 @@ func path(filename string) string { return filepath.Join("testdata", filename) } -func compare(actual []byte, filename string) error { +func compare(actual []byte, filename string, templates map[string]string) error { actual = normalize(actual) if err := update(filename, actual); err != nil { return err @@ -77,12 +78,34 @@ func compare(actual []byte, filename string) error { return fmt.Errorf("unable to read testdata %s: %w", filename, err) } expected = normalize(expected) + + if templates != nil && len(templates) > 0 { + expected, err = templateString(expected, templates) + if err != nil { + return fmt.Errorf("unable to template testdata %s: %w", filename, err) + } + } + if !bytes.Equal(expected, actual) { return fmt.Errorf("does not match golden file %s\n\nWANT:\n'%s'\n\nGOT:\n'%s'", filename, expected, actual) } return nil } +func templateString(content []byte, data map[string]string) ([]byte, error) { + tmpl, err := template.New("content").Parse(string(content)) + if err != nil { + return nil, err + } + + var buf bytes.Buffer + if err := tmpl.Execute(&buf, data); err != nil { + return nil, err + } + + return buf.Bytes(), nil +} + func update(filename string, in []byte) error { if !*updateGolden { return nil diff --git a/pkg/action/dependency_test.go b/pkg/action/dependency_test.go index 5be7bf5a9..977fdcaee 100644 --- a/pkg/action/dependency_test.go +++ b/pkg/action/dependency_test.go @@ -59,7 +59,7 @@ func TestList(t *testing.T) { if err := NewDependency().List(tcase.chart, &buf); err != nil { t.Fatal(err) } - test.AssertGoldenString(t, buf.String(), tcase.golden) + test.AssertGoldenString(t, buf.String(), tcase.golden, map[string]string{}) } } diff --git a/pkg/action/install.go b/pkg/action/install.go index 50df13c05..c6c803744 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -840,7 +840,7 @@ OUTER: if err != nil { return err } - if dac.Name() == rac.Name() { + if dac.Name() == rac.Name() && dac.Version() == rac.Version() { continue OUTER } } diff --git a/pkg/chart/common.go b/pkg/chart/common.go index cec2c7091..ad2d8b4eb 100644 --- a/pkg/chart/common.go +++ b/pkg/chart/common.go @@ -51,6 +51,10 @@ func (r *v2Accessor) Name() string { return r.chrt.Metadata.Name } +func (r *v2Accessor) Version() string { + return r.chrt.Metadata.Version +} + func (r *v2Accessor) IsRoot() bool { return r.chrt.IsRoot() } @@ -120,6 +124,10 @@ func (r *v3Accessor) Name() string { return r.chrt.Metadata.Name } +func (r *v3Accessor) Version() string { + return r.chrt.Metadata.Version +} + func (r *v3Accessor) IsRoot() bool { return r.chrt.IsRoot() } diff --git a/pkg/chart/dependency.go b/pkg/chart/dependency.go index 864fe6d2c..af06510b9 100644 --- a/pkg/chart/dependency.go +++ b/pkg/chart/dependency.go @@ -47,6 +47,10 @@ func (r *v2DependencyAccessor) Name() string { return r.dep.Name } +func (r *v2DependencyAccessor) Version() string { + return r.dep.Version +} + func (r *v2DependencyAccessor) Alias() string { return r.dep.Alias } @@ -59,6 +63,10 @@ func (r *v3DependencyAccessor) Name() string { return r.dep.Name } +func (r *v3DependencyAccessor) Version() string { + return r.dep.Version +} + func (r *v3DependencyAccessor) Alias() string { return r.dep.Alias } diff --git a/pkg/chart/interfaces.go b/pkg/chart/interfaces.go index 6d94ad3ea..e9ecb295c 100644 --- a/pkg/chart/interfaces.go +++ b/pkg/chart/interfaces.go @@ -25,6 +25,7 @@ type Dependency any type Accessor interface { Name() string + Version() string IsRoot() bool MetadataAsMap() map[string]any Files() []*common.File @@ -40,5 +41,6 @@ type Accessor interface { type DependencyAccessor interface { Name() string + Version() string Alias() string } diff --git a/pkg/cmd/helpers_test.go b/pkg/cmd/helpers_test.go index 08065499e..5a3e16819 100644 --- a/pkg/cmd/helpers_test.go +++ b/pkg/cmd/helpers_test.go @@ -69,7 +69,7 @@ func runTestCmd(t *testing.T, tests []cmdTestCase) { t.Errorf("expected no error, got: '%v'", err) } if tt.golden != "" { - test.AssertGoldenString(t, out, tt.golden) + test.AssertGoldenString(t, out, tt.golden, tt.goldenArgs) } }) } @@ -129,10 +129,11 @@ func executeActionCommandStdinC(store *storage.Storage, in *os.File, cmd string) // cmdTestCase describes a test case that works with releases. type cmdTestCase struct { - name string - cmd string - golden string - wantError bool + name string + cmd string + golden string + goldenArgs map[string]string + wantError bool // Rels are the available releases at the start of the test. rels []*release.Release // Number of repeats (in case a feature was previously flaky and the test checks diff --git a/pkg/cmd/install_test.go b/pkg/cmd/install_test.go index 8d3435e03..6927613fd 100644 --- a/pkg/cmd/install_test.go +++ b/pkg/cmd/install_test.go @@ -153,6 +153,15 @@ func TestInstall(t *testing.T) { cmd: "install --dependency-update updeps testdata/testcharts/chart-with-subchart-update", golden: "output/chart-with-subchart-update.txt", }, + // Install chart with update-dependency + { + name: "install chart with outdated dependencies", + cmd: fmt.Sprintf("install --dependency-update --repository-cache %s --repository-config %s updeps testdata/testcharts/chart-with-outdated-dependency --username username --password password", srv.Root(), repoFile), + golden: "output/install-with-outdated-deps.txt", + goldenArgs: map[string]string{ + "repoUrl": srv.URL(), + }, + }, // Install, chart with bad dependencies in Chart.yaml in /charts { name: "install chart with bad dependencies in Chart.yaml", diff --git a/pkg/cmd/testdata/output/install-with-outdated-deps.txt b/pkg/cmd/testdata/output/install-with-outdated-deps.txt new file mode 100644 index 000000000..90823aa91 --- /dev/null +++ b/pkg/cmd/testdata/output/install-with-outdated-deps.txt @@ -0,0 +1,12 @@ +Hang tight while we grab the latest from your chart repositories... +...Successfully got an update from the "test" chart repository +Update Complete. ⎈Happy Helming!⎈ +Saving 1 charts +Downloading oci-dependent-chart from repo {{ .repoUrl }} +Deleting outdated charts +NAME: updeps +LAST DEPLOYED: Fri Sep 2 22:04:05 1977 +NAMESPACE: default +STATUS: deployed +REVISION: 1 +DESCRIPTION: Install complete diff --git a/pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/.helmignore b/pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/.helmignore new file mode 100644 index 000000000..f0c131944 --- /dev/null +++ b/pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/.helmignore @@ -0,0 +1,21 @@ +# Patterns to ignore when building packages. +# This supports shell glob matching, relative path matching, and +# negation (prefixed with !). Only one pattern per line. +.DS_Store +# Common VCS dirs +.git/ +.gitignore +.bzr/ +.bzrignore +.hg/ +.hgignore +.svn/ +# Common backup files +*.swp +*.bak +*.tmp +*~ +# Various IDEs +.project +.idea/ +*.tmproj diff --git a/pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/Chart.yaml b/pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/Chart.yaml new file mode 100644 index 000000000..74c09f0bb --- /dev/null +++ b/pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/Chart.yaml @@ -0,0 +1,8 @@ +apiVersion: v1 +description: A Helm chart for Kubernetes +name: chart-with-outdated-dependency +version: 0.1.0 +dependencies: + - name: oci-dependent-chart + version: 0.1.0 + repository: "@test" diff --git a/pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/charts/oci-dependent-chart/.helmignore b/pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/charts/oci-dependent-chart/.helmignore new file mode 100644 index 000000000..0e8a0eb36 --- /dev/null +++ b/pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/charts/oci-dependent-chart/.helmignore @@ -0,0 +1,23 @@ +# Patterns to ignore when building packages. +# This supports shell glob matching, relative path matching, and +# negation (prefixed with !). Only one pattern per line. +.DS_Store +# Common VCS dirs +.git/ +.gitignore +.bzr/ +.bzrignore +.hg/ +.hgignore +.svn/ +# Common backup files +*.swp +*.bak +*.tmp +*.orig +*~ +# Various IDEs +.project +.idea/ +*.tmproj +.vscode/ diff --git a/pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/charts/oci-dependent-chart/Chart.yaml b/pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/charts/oci-dependent-chart/Chart.yaml new file mode 100644 index 000000000..8706d1fbf --- /dev/null +++ b/pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/charts/oci-dependent-chart/Chart.yaml @@ -0,0 +1,6 @@ +apiVersion: v2 +appVersion: 1.16.0 +description: A Helm chart for Kubernetes +name: oci-dependent-chart +type: application +version: 0.0.1 diff --git a/pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/charts/oci-dependent-chart/templates/NOTES.txt b/pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/charts/oci-dependent-chart/templates/NOTES.txt new file mode 100644 index 000000000..ca300c9e2 --- /dev/null +++ b/pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/charts/oci-dependent-chart/templates/NOTES.txt @@ -0,0 +1,22 @@ +1. Get the application URL by running these commands: +{{- if .Values.ingress.enabled }} +{{- range $host := .Values.ingress.hosts }} + {{- range .paths }} + http{{ if $.Values.ingress.tls }}s{{ end }}://{{ $host.host }}{{ . }} + {{- end }} +{{- end }} +{{- else if contains "NodePort" .Values.service.type }} + export NODE_PORT=$(kubectl get --namespace {{ .Release.Namespace }} -o jsonpath="{.spec.ports[0].nodePort}" services {{ include "oci-dependent-chart.fullname" . }}) + export NODE_IP=$(kubectl get nodes --namespace {{ .Release.Namespace }} -o jsonpath="{.items[0].status.addresses[0].address}") + echo http://$NODE_IP:$NODE_PORT +{{- else if contains "LoadBalancer" .Values.service.type }} + NOTE: It may take a few minutes for the LoadBalancer IP to be available. + You can watch the status of by running 'kubectl get --namespace {{ .Release.Namespace }} svc -w {{ include "oci-dependent-chart.fullname" . }}' + export SERVICE_IP=$(kubectl get svc --namespace {{ .Release.Namespace }} {{ include "oci-dependent-chart.fullname" . }} --template "{{"{{ range (index .status.loadBalancer.ingress 0) }}{{.}}{{ end }}"}}") + echo http://$SERVICE_IP:{{ .Values.service.port }} +{{- else if contains "ClusterIP" .Values.service.type }} + export POD_NAME=$(kubectl get pods --namespace {{ .Release.Namespace }} -l "app.kubernetes.io/name={{ include "oci-dependent-chart.name" . }},app.kubernetes.io/instance={{ .Release.Name }}" -o jsonpath="{.items[0].metadata.name}") + export CONTAINER_PORT=$(kubectl get pod --namespace {{ .Release.Namespace }} $POD_NAME -o jsonpath="{.spec.containers[0].ports[0].containerPort}") + echo "Visit http://127.0.0.1:8080 to use your application" + kubectl --namespace {{ .Release.Namespace }} port-forward $POD_NAME 8080:$CONTAINER_PORT +{{- end }} diff --git a/pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/charts/oci-dependent-chart/templates/tests/test-connection.yaml b/pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/charts/oci-dependent-chart/templates/tests/test-connection.yaml new file mode 100644 index 000000000..9e9e26bbc --- /dev/null +++ b/pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/charts/oci-dependent-chart/templates/tests/test-connection.yaml @@ -0,0 +1,15 @@ +apiVersion: v1 +kind: Pod +metadata: + name: "{{ include "oci-dependent-chart.fullname" . }}-test-connection" + labels: + {{- include "oci-dependent-chart.labels" . | nindent 4 }} + annotations: + "helm.sh/hook": test +spec: + containers: + - name: wget + image: busybox + command: ['wget'] + args: ['{{ include "oci-dependent-chart.fullname" . }}:{{ .Values.service.port }}'] + restartPolicy: Never diff --git a/pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/charts/oci-dependent-chart/values.yaml b/pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/charts/oci-dependent-chart/values.yaml new file mode 100644 index 000000000..18c5d421e --- /dev/null +++ b/pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/charts/oci-dependent-chart/values.yaml @@ -0,0 +1,3 @@ +# Default values for oci-dependent-chart. +# This is a YAML-formatted file. +# Declare variables to be passed into your templates. diff --git a/pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/values.yaml b/pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/values.yaml new file mode 100644 index 000000000..d57f76b07 --- /dev/null +++ b/pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/values.yaml @@ -0,0 +1,4 @@ +# Default values for reqtest. +# This is a YAML-formatted file. +# Declare name/value pairs to be passed into your templates. +# name: value From 04a77112eb3a41c5c7ed1b700e8167ea6331d972 Mon Sep 17 00:00:00 2001 From: Baptiste ROUX Date: Thu, 16 Apr 2026 10:10:17 +0200 Subject: [PATCH 2/3] fix(test): update template condition check Signed-off-by: Baptiste ROUX --- internal/test/test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/test/test.go b/internal/test/test.go index 1a7cce735..a01ffc8cd 100644 --- a/internal/test/test.go +++ b/internal/test/test.go @@ -79,7 +79,7 @@ func compare(actual []byte, filename string, templates map[string]string) error } expected = normalize(expected) - if templates != nil && len(templates) > 0 { + if len(templates) > 0 { expected, err = templateString(expected, templates) if err != nil { return fmt.Errorf("unable to template testdata %s: %w", filename, err) From 919cbfaac90aeaa26842e9f057d00b9023ec38a1 Mon Sep 17 00:00:00 2001 From: Baptiste ROUX Date: Thu, 16 Apr 2026 11:58:55 +0200 Subject: [PATCH 3/3] fix(install): use compatible version range for dependency check Previously, the dependency check used exact version matching, which caused false positives when a compatible version was already installed. Now it uses `chartutil.IsCompatibleRange` to respect semantic versioning constraints. - Update install logic to check version compatibility instead of equality - Adjust test case to include a version in the dependency - Add thread-safe mutex for WaitOptions recording in fake kube client Signed-off-by: Baptiste ROUX --- pkg/action/install.go | 2 +- pkg/action/install_test.go | 2 +- pkg/cmd/testdata/output/install-with-outdated-deps.txt | 6 ------ .../testcharts/chart-with-outdated-dependency/.gitignore | 1 + pkg/kube/fake/failing_kube_client.go | 4 ++++ 5 files changed, 7 insertions(+), 8 deletions(-) create mode 100644 pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/.gitignore diff --git a/pkg/action/install.go b/pkg/action/install.go index c6c803744..ef10b9b71 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -840,7 +840,7 @@ OUTER: if err != nil { return err } - if dac.Name() == rac.Name() && dac.Version() == rac.Version() { + if dac.Name() == rac.Name() && chartutil.IsCompatibleRange(rac.Version(), dac.Version()) { continue OUTER } } diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index 05ca9a75e..301fc688e 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -1224,7 +1224,7 @@ func TestInstallCRDs_WaiterError(t *testing.T) { } func TestCheckDependencies(t *testing.T) { - dependency := chart.Dependency{Name: "hello"} + dependency := chart.Dependency{Name: "hello", Version: "0.1.0"} mockChart := buildChart(withDependency()) assert.Nil(t, CheckDependencies(mockChart, []ci.Dependency{&dependency})) diff --git a/pkg/cmd/testdata/output/install-with-outdated-deps.txt b/pkg/cmd/testdata/output/install-with-outdated-deps.txt index 90823aa91..9e3d6f83a 100644 --- a/pkg/cmd/testdata/output/install-with-outdated-deps.txt +++ b/pkg/cmd/testdata/output/install-with-outdated-deps.txt @@ -1,9 +1,3 @@ -Hang tight while we grab the latest from your chart repositories... -...Successfully got an update from the "test" chart repository -Update Complete. ⎈Happy Helming!⎈ -Saving 1 charts -Downloading oci-dependent-chart from repo {{ .repoUrl }} -Deleting outdated charts NAME: updeps LAST DEPLOYED: Fri Sep 2 22:04:05 1977 NAMESPACE: default diff --git a/pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/.gitignore b/pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/.gitignore new file mode 100644 index 000000000..68d93c64f --- /dev/null +++ b/pkg/cmd/testdata/testcharts/chart-with-outdated-dependency/.gitignore @@ -0,0 +1 @@ +requirements.lock diff --git a/pkg/kube/fake/failing_kube_client.go b/pkg/kube/fake/failing_kube_client.go index 0f7787f79..1289b35c6 100644 --- a/pkg/kube/fake/failing_kube_client.go +++ b/pkg/kube/fake/failing_kube_client.go @@ -19,6 +19,7 @@ package fake import ( "io" + "sync" "time" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -49,6 +50,7 @@ type FailingKubeClient struct { WaitDuration time.Duration // RecordedWaitOptions stores the WaitOptions passed to GetWaiter for testing RecordedWaitOptions []kube.WaitOption + mu sync.Mutex } var _ kube.Interface = &FailingKubeClient{} @@ -160,7 +162,9 @@ func (f *FailingKubeClient) GetWaiter(ws kube.WaitStrategy) (kube.Waiter, error) func (f *FailingKubeClient) GetWaiterWithOptions(ws kube.WaitStrategy, opts ...kube.WaitOption) (kube.Waiter, error) { // Record the WaitOptions for testing + f.mu.Lock() f.RecordedWaitOptions = append(f.RecordedWaitOptions, opts...) + f.mu.Unlock() waiter, _ := f.PrintingKubeClient.GetWaiterWithOptions(ws, opts...) printingKubeWaiter, _ := waiter.(*PrintingKubeWaiter) return &FailingKubeWaiter{