From 5ac1e1fb98a42ad23e224222cbcbd53eb0f529cf Mon Sep 17 00:00:00 2001 From: Sergii Iefremov Date: Wed, 3 Aug 2022 14:35:10 -0400 Subject: [PATCH] feat: fix needs error with context that includes slash Signed-off-by: Sergii Iefremov --- pkg/app/diff_test.go | 62 ++++++++++++++++++- ...e_when_foo_needs_bar_with_context_override | 50 +++++++++++++++ ...n_releaseb_needs_releasea_with_aws_context | 54 ++++++++++++++++ pkg/state/state.go | 4 +- 4 files changed, 167 insertions(+), 3 deletions(-) create mode 100644 pkg/app/testdata/app_diff_test_1/upgrade_when_foo_needs_bar_with_context_override create mode 100644 pkg/app/testdata/app_diff_test_1/upgrade_when_releaseb_needs_releasea_with_aws_context diff --git a/pkg/app/diff_test.go b/pkg/app/diff_test.go index 7f22edb4..5302bae7 100644 --- a/pkg/app/diff_test.go +++ b/pkg/app/diff_test.go @@ -160,6 +160,7 @@ func TestDiff(t *testing.T) { upgraded []exectest.Release deleted []exectest.Release log string + overrideContext string }{ // // complex test cases for smoke testing @@ -341,6 +342,58 @@ releases: }, upgraded: []exectest.Release{}, }, + { + name: "upgrade when foo needs bar with context override", + loc: location(), + overrideContext: "hello/world", + files: map[string]string{ + "/path/to/helmfile.yaml": ` +helmDefaults: + kubeContext: hello/world +releases: +- name: bar + chart: mychart2 +- name: foo + chart: mychart1 + needs: + - bar +`, + }, + detailedExitcode: true, + error: "Identified at least one change", + diffs: map[exectest.DiffKey]error{ + {Name: "bar", Chart: "mychart2", Flags: "--kube-contexthello/world--detailed-exitcode"}: helmexec.ExitError{Code: 2}, + {Name: "foo", Chart: "mychart1", Flags: "--kube-contexthello/world--detailed-exitcode"}: helmexec.ExitError{Code: 2}, + }, + upgraded: []exectest.Release{}, + }, + { + name: "upgrade when releaseB needs releaseA with AWS context", + loc: location(), + overrideContext: "arn:aws:eks:us-east-1:1234567890:cluster/myekscluster", + files: map[string]string{ + "/path/to/helmfile.yaml": ` +releases: +- name: releaseA + chart: mychart1 + namespace: namespaceA + kubeContext: arn:aws:eks:us-east-1:1234567890:cluster/myekscluster +- name: releaseB + chart: mychart2 + namespace: namespaceA + kubeContext: arn:aws:eks:us-east-1:1234567890:cluster/myekscluster + needs: + - arn:aws:eks:us-east-1:1234567890:cluster/myekscluster/namespaceA/releaseA +`, + }, + detailedExitcode: true, + error: "Identified at least one change", + diffs: map[exectest.DiffKey]error{ + {Name: "releaseB", Chart: "mychart2", Flags: "--kube-contextarn:aws:eks:us-east-1:1234567890:cluster/myekscluster--namespacenamespaceA--detailed-exitcode"}: helmexec.ExitError{Code: 2}, + {Name: "releaseA", Chart: "mychart1", Flags: "--kube-contextarn:aws:eks:us-east-1:1234567890:cluster/myekscluster--namespacenamespaceA--detailed-exitcode"}: helmexec.ExitError{Code: 2}, + }, + upgraded: []exectest.Release{}, + }, { name: "upgrade when bar needs foo", loc: location(), @@ -1291,15 +1344,20 @@ changing working directory back to "/path/to" t.Errorf("unexpected error creating vals runtime: %v", err) } + overrideKubeContext := "default" + if tc.overrideContext != "" { + overrideKubeContext = tc.overrideContext + } + app := appWithFs(&App{ OverrideHelmBinary: DefaultHelmBinary, glob: filepath.Glob, abs: filepath.Abs, - OverrideKubeContext: "default", + OverrideKubeContext: overrideKubeContext, Env: "default", Logger: logger, helms: map[helmKey]helmexec.Interface{ - createHelmKey("helm", "default"): helm, + createHelmKey("helm", overrideKubeContext): helm, }, valsRuntime: valsRuntime, }, tc.files) diff --git a/pkg/app/testdata/app_diff_test_1/upgrade_when_foo_needs_bar_with_context_override b/pkg/app/testdata/app_diff_test_1/upgrade_when_foo_needs_bar_with_context_override new file mode 100644 index 00000000..5756135c --- /dev/null +++ b/pkg/app/testdata/app_diff_test_1/upgrade_when_foo_needs_bar_with_context_override @@ -0,0 +1,50 @@ +processing file "helmfile.yaml" in directory "." +changing working directory to "/path/to" +first-pass rendering starting for "helmfile.yaml.part.0": inherited=&{default map[] map[]}, overrode= +first-pass uses: &{default map[] map[]} +first-pass rendering output of "helmfile.yaml.part.0": + 0: + 1: helmDefaults: + 2: kubeContext: hello/world + 3: releases: + 4: - name: bar + 5: chart: mychart2 + 6: - name: foo + 7: chart: mychart1 + 8: needs: + 9: - bar +10: + +first-pass produced: &{default map[] map[]} +first-pass rendering result of "helmfile.yaml.part.0": {default map[] map[]} +vals: +map[] +defaultVals:[] +second-pass rendering result of "helmfile.yaml.part.0": + 0: + 1: helmDefaults: + 2: kubeContext: hello/world + 3: releases: + 4: - name: bar + 5: chart: mychart2 + 6: - name: foo + 7: chart: mychart1 + 8: needs: + 9: - bar +10: + +merged environment: &{default map[] map[]} +2 release(s) found in helmfile.yaml + +processing 2 groups of releases in this order: +GROUP RELEASES +1 hello/world//bar +2 hello/world//foo + +processing releases in group 1/2: hello/world//bar +processing releases in group 2/2: hello/world//foo +Affected releases are: + bar (mychart2) UPDATED + foo (mychart1) UPDATED + +changing working directory back to "/path/to" diff --git a/pkg/app/testdata/app_diff_test_1/upgrade_when_releaseb_needs_releasea_with_aws_context b/pkg/app/testdata/app_diff_test_1/upgrade_when_releaseb_needs_releasea_with_aws_context new file mode 100644 index 00000000..8d994feb --- /dev/null +++ b/pkg/app/testdata/app_diff_test_1/upgrade_when_releaseb_needs_releasea_with_aws_context @@ -0,0 +1,54 @@ +processing file "helmfile.yaml" in directory "." +changing working directory to "/path/to" +first-pass rendering starting for "helmfile.yaml.part.0": inherited=&{default map[] map[]}, overrode= +first-pass uses: &{default map[] map[]} +first-pass rendering output of "helmfile.yaml.part.0": + 0: + 1: releases: + 2: - name: releaseA + 3: chart: mychart1 + 4: namespace: namespaceA + 5: kubeContext: arn:aws:eks:us-east-1:1234567890:cluster/myekscluster + 6: - name: releaseB + 7: chart: mychart2 + 8: namespace: namespaceA + 9: kubeContext: arn:aws:eks:us-east-1:1234567890:cluster/myekscluster +10: needs: +11: - arn:aws:eks:us-east-1:1234567890:cluster/myekscluster/namespaceA/releaseA +12: + +first-pass produced: &{default map[] map[]} +first-pass rendering result of "helmfile.yaml.part.0": {default map[] map[]} +vals: +map[] +defaultVals:[] +second-pass rendering result of "helmfile.yaml.part.0": + 0: + 1: releases: + 2: - name: releaseA + 3: chart: mychart1 + 4: namespace: namespaceA + 5: kubeContext: arn:aws:eks:us-east-1:1234567890:cluster/myekscluster + 6: - name: releaseB + 7: chart: mychart2 + 8: namespace: namespaceA + 9: kubeContext: arn:aws:eks:us-east-1:1234567890:cluster/myekscluster +10: needs: +11: - arn:aws:eks:us-east-1:1234567890:cluster/myekscluster/namespaceA/releaseA +12: + +merged environment: &{default map[] map[]} +2 release(s) found in helmfile.yaml + +processing 2 groups of releases in this order: +GROUP RELEASES +1 arn:aws:eks:us-east-1:1234567890:cluster/myekscluster/namespaceA/releaseA +2 arn:aws:eks:us-east-1:1234567890:cluster/myekscluster/namespaceA/releaseB + +processing releases in group 1/2: arn:aws:eks:us-east-1:1234567890:cluster/myekscluster/namespaceA/releaseA +processing releases in group 2/2: arn:aws:eks:us-east-1:1234567890:cluster/myekscluster/namespaceA/releaseB +Affected releases are: + releaseA (mychart1) UPDATED + releaseB (mychart2) UPDATED + +changing working directory back to "/path/to" diff --git a/pkg/state/state.go b/pkg/state/state.go index fbf84361..b50993ef 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -399,7 +399,9 @@ func (st *HelmState) ApplyOverrides(spec *ReleaseSpec) { } if len(components) > 2 { - kubecontext = components[len(components)-3] + // Join all ID components except last 2 (namespace and name) into kubecontext. + // Should be safe because resource names do not contain slashes. + kubecontext = strings.Join(components[:len(components)-2], "/") } else { kubecontext = spec.KubeContext }