diff --git a/pkg/state/chart_dependencies_rewrite_test.go b/pkg/state/chart_dependencies_rewrite_test.go index 09ebc893..a51ae31a 100644 --- a/pkg/state/chart_dependencies_rewrite_test.go +++ b/pkg/state/chart_dependencies_rewrite_test.go @@ -17,7 +17,7 @@ func TestRewriteChartDependencies(t *testing.T) { chartYaml string expectModified bool expectError bool - validate func(t *testing.T, chartPath string) + validate func(t *testing.T, modifiedChartYaml string) }{ { name: "no Chart.yaml exists", @@ -46,13 +46,8 @@ dependencies: `, expectModified: false, expectError: false, - validate: func(t *testing.T, chartPath string) { - data, err := os.ReadFile(filepath.Join(chartPath, "Chart.yaml")) - if err != nil { - t.Fatalf("failed to read Chart.yaml: %v", err) - } - content := string(data) - if !strings.Contains(content, "file:///absolute/path/to/chart") { + validate: func(t *testing.T, modifiedChartYaml string) { + if !strings.Contains(modifiedChartYaml, "file:///absolute/path/to/chart") { t.Errorf("absolute path should not be modified") } }, @@ -69,20 +64,14 @@ dependencies: `, expectModified: true, expectError: false, - validate: func(t *testing.T, chartPath string) { - data, err := os.ReadFile(filepath.Join(chartPath, "Chart.yaml")) - if err != nil { - t.Fatalf("failed to read Chart.yaml: %v", err) - } - content := string(data) - + validate: func(t *testing.T, modifiedChartYaml string) { // Should have been converted to absolute path - if strings.Contains(content, "file://../relative-chart") { + if strings.Contains(modifiedChartYaml, "file://../relative-chart") { t.Errorf("relative path should have been converted to absolute") } // Should now have an absolute path - if !strings.Contains(content, "file://") { + if !strings.Contains(modifiedChartYaml, "file://") { t.Errorf("should still have file:// prefix") } }, @@ -108,30 +97,24 @@ dependencies: `, expectModified: true, expectError: false, - validate: func(t *testing.T, chartPath string) { - data, err := os.ReadFile(filepath.Join(chartPath, "Chart.yaml")) - if err != nil { - t.Fatalf("failed to read Chart.yaml: %v", err) - } - content := string(data) - + validate: func(t *testing.T, modifiedChartYaml string) { // HTTPS repo should remain unchanged - if !strings.Contains(content, "https://charts.example.com") { + if !strings.Contains(modifiedChartYaml, "https://charts.example.com") { t.Errorf("https repository should not be modified") } // Relative path should be converted - if strings.Contains(content, "file://../relative-chart") { + if strings.Contains(modifiedChartYaml, "file://../relative-chart") { t.Errorf("relative file:// path should have been converted") } // Absolute path should remain unchanged - if !strings.Contains(content, "file:///absolute/chart") { + if !strings.Contains(modifiedChartYaml, "file:///absolute/chart") { t.Errorf("absolute file:// path should not be modified") } // OCI repo should remain unchanged - if !strings.Contains(content, "oci://registry.example.com") { + if !strings.Contains(modifiedChartYaml, "oci://registry.example.com") { t.Errorf("oci repository should not be modified") } }, @@ -154,21 +137,50 @@ dependencies: `, expectModified: true, expectError: false, - validate: func(t *testing.T, chartPath string) { - data, err := os.ReadFile(filepath.Join(chartPath, "Chart.yaml")) - if err != nil { - t.Fatalf("failed to read Chart.yaml: %v", err) - } - content := string(data) - + validate: func(t *testing.T, modifiedChartYaml string) { // All relative paths should be converted - if strings.Contains(content, "file://../chart1") || - strings.Contains(content, "file://./chart2") || - strings.Contains(content, "file://../../../chart3") { + if strings.Contains(modifiedChartYaml, "file://../chart1") || + strings.Contains(modifiedChartYaml, "file://./chart2") || + strings.Contains(modifiedChartYaml, "file://../../../chart3") { t.Errorf("all relative paths should have been converted") } }, }, + { + name: "extra fields", + chartYaml: `apiVersion: v2 +name: test-chart +version: 1.0.0 +dependencies: + - name: dep1 + repository: https://charts.example.com + version: 1.0.0 + condition: dep.install + import-values: + - child: persistence + parent: global.persistence + extra-field2: bbb +extra-field: aaa +`, + expectModified: false, + expectError: false, + validate: func(t *testing.T, modifiedChartYaml string) { + requiredFields := []string{ + "condition: dep.install", + "import-values:", + "child: persistence", + "parent: global.persistence", + "extra-field2: bbb", + "extra-field: aaa", + } + + for _, field := range requiredFields { + if !strings.Contains(modifiedChartYaml, field) { + t.Errorf("field %q should be preserved", field) + } + } + }, + }, } for _, tt := range tests { @@ -216,26 +228,34 @@ dependencies: t.Fatalf("unexpected error: %v", err) } + if tt.chartYaml == "" { + return + } + + modifiedChartBytes, err := os.ReadFile(filepath.Join(tempDir, "Chart.yaml")) + if err != nil { + t.Fatalf("failed to read Chart.yaml: %v", err) + } + modifiedChartYaml := string(modifiedChartBytes) + // Validate the modified Chart.yaml if tt.validate != nil { - tt.validate(t, tempDir) + tt.validate(t, modifiedChartYaml) } // Call cleanup and verify restoration - if tt.chartYaml != "" { - cleanup() + cleanup() - // Read restored content - restoredContent, err := os.ReadFile(chartYamlPath) - if err != nil { - t.Fatalf("failed to read restored Chart.yaml: %v", err) - } + // Read restored content + restoredContent, err := os.ReadFile(chartYamlPath) + if err != nil { + t.Fatalf("failed to read restored Chart.yaml: %v", err) + } - // Verify content was restored - if string(restoredContent) != string(originalContent) { - t.Errorf("cleanup did not restore original content\noriginal:\n%s\nrestored:\n%s", - string(originalContent), string(restoredContent)) - } + // Verify content was restored + if string(restoredContent) != string(originalContent) { + t.Errorf("cleanup did not restore original content\noriginal:\n%s\nrestored:\n%s", + string(originalContent), string(restoredContent)) } }) } @@ -319,6 +339,7 @@ dependencies: - name: dep1 repository: file://../relative-chart version: 1.0.0 + condition: test ` chartYamlPath := filepath.Join(tempDir, "Chart.yaml") @@ -346,9 +367,7 @@ dependencies: content := string(modifiedContent) - // Verify top-level fields are preserved - // Note: The implementation preserves top-level chart metadata but may not preserve - // extra dependency-level fields (like condition, tags) that are not in the ChartDependency struct + // Verify fields are preserved requiredFields := []string{ "apiVersion: v2", "name: test-chart", @@ -356,6 +375,7 @@ dependencies: "description: A test chart", "keywords:", "maintainers:", + "condition:", } for _, field := range requiredFields { diff --git a/pkg/state/helmx.go b/pkg/state/helmx.go index 988afc08..b337cd30 100644 --- a/pkg/state/helmx.go +++ b/pkg/state/helmx.go @@ -303,8 +303,9 @@ func (st *HelmState) appendShowOnlyFlags(flags []string, showOnly []string) []st } type Chartify struct { - Opts *chartify.ChartifyOpts - Clean func() + Opts *chartify.ChartifyOpts + Clean func() + NeedsChartifyForLocalDir bool } func (st *HelmState) downloadChartWithGoGetter(r *ReleaseSpec) (string, error) { @@ -375,6 +376,7 @@ func (st *HelmState) PrepareChartify(helm helmexec.Interface, release *ReleaseSp if stat, _ := os.Stat(dir); stat != nil && stat.IsDir() { if exists, err := st.fs.FileExists(filepath.Join(dir, "Chart.yaml")); err == nil && !exists { shouldRun = true + c.NeedsChartifyForLocalDir = true } } diff --git a/pkg/state/state.go b/pkg/state/state.go index a1742d12..40da9105 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -1453,9 +1453,9 @@ func (st *HelmState) rewriteChartDependencies(chartPath string) (func(), error) // Parse Chart.yaml type ChartDependency struct { - Name string `yaml:"name"` - Repository string `yaml:"repository"` - Version string `yaml:"version"` + Name string `yaml:"name"` + Repository string `yaml:"repository"` + Data map[string]interface{} `yaml:",inline"` } type ChartMeta struct { Dependencies []ChartDependency `yaml:"dependencies,omitempty"` @@ -1773,7 +1773,7 @@ func (st *HelmState) prepareChartForRelease(release *ReleaseSpec, helm helmexec. skipRefreshDefault := release.SkipRefresh == nil && st.HelmDefaults.SkipRefresh skipRefresh := !isLocal || skipRefreshGlobal || skipRefreshRelease || skipRefreshDefault - if chartification != nil && helmfileCommand != "pull" { + if chartification != nil && (helmfileCommand != "pull" || chartification.NeedsChartifyForLocalDir) { // Issue #2297: Normalize local chart paths before chartification // When using transformers with local charts like "../chart", the chartify process // needs an absolute path, otherwise it tries "helm pull ../chart" which fails diff --git a/test/integration/run.sh b/test/integration/run.sh index 0507e3b7..9fc3aa37 100755 --- a/test/integration/run.sh +++ b/test/integration/run.sh @@ -96,6 +96,7 @@ ${kubectl} create namespace ${test_ns} || fail "Could not create namespace ${tes # TEST CASES---------------------------------------------------------------------------------------------------------- +. ${dir}/test-cases/chart-deps-condition.sh . ${dir}/test-cases/fetch-forl-local-chart.sh . ${dir}/test-cases/suppress-output-line-regex.sh . ${dir}/test-cases/chartify-jsonPatches-and-strategicMergePatches.sh @@ -107,6 +108,7 @@ ${kubectl} create namespace ${test_ns} || fail "Could not create namespace ${tes . ${dir}/test-cases/skip-diff-output.sh . ${dir}/test-cases/v1-subhelmfile-multi-bases-with-array-values.sh . ${dir}/test-cases/kustomized-fetch.sh +. ${dir}/test-cases/issue-2503-kustomize-fetch.sh . ${dir}/test-cases/regression.sh . ${dir}/test-cases/secretssops.sh . ${dir}/test-cases/yaml-overwrite.sh diff --git a/test/integration/test-cases/chart-deps-condition.sh b/test/integration/test-cases/chart-deps-condition.sh new file mode 100644 index 00000000..83b0efde --- /dev/null +++ b/test/integration/test-cases/chart-deps-condition.sh @@ -0,0 +1,13 @@ +chart_deps_condition_input_dir="${cases_dir}/chart-deps-condition/input" + +chart_deps_condition_tmp=$(mktemp -d) +actual=${chart_deps_condition_tmp}/actual.yaml +expected="${cases_dir}/chart-deps-condition/output/template" + +test_start "chart with dependencies and condition" + +# --concurrency 1 is a workaround for https://github.com/helmfile/helmfile/issues/2502 +${helmfile} -f ${chart_deps_condition_input_dir}/helmfile.yaml template --skip-deps --concurrency 1 > ${actual} || fail "\"helmfile template\" shouldn't fail" +./dyff between -bs ${expected} ${actual} || fail "\"helmfile template\" should be consistent" + +test_pass "chart with dependencies and condition" diff --git a/test/integration/test-cases/chart-deps-condition/input/chart/Chart.yaml b/test/integration/test-cases/chart-deps-condition/input/chart/Chart.yaml new file mode 100644 index 00000000..4da6b8c5 --- /dev/null +++ b/test/integration/test-cases/chart-deps-condition/input/chart/Chart.yaml @@ -0,0 +1,14 @@ +dependencies: + - name: subchart1 + repository: file://charts/subchart1 + condition: subchart1.install + version: 1.0.0 + - name: subchart2 + repository: file://charts/subchart2 + version: 2.0.0 +apiVersion: v2 +appVersion: 3.0.0 +description: Test helm charts dependencies +name: testchartsdeps +type: application +version: 3.0.0 diff --git a/test/integration/test-cases/chart-deps-condition/input/chart/charts/subchart1/Chart.yaml b/test/integration/test-cases/chart-deps-condition/input/chart/charts/subchart1/Chart.yaml new file mode 100644 index 00000000..9d82a5c9 --- /dev/null +++ b/test/integration/test-cases/chart-deps-condition/input/chart/charts/subchart1/Chart.yaml @@ -0,0 +1,6 @@ +apiVersion: v2 +name: subchart1 +description: Test sub helm chart 1 +type: application +version: 1.0.0 +appVersion: "1.0.0" diff --git a/test/integration/test-cases/chart-deps-condition/input/chart/charts/subchart1/templates/cm.yaml b/test/integration/test-cases/chart-deps-condition/input/chart/charts/subchart1/templates/cm.yaml new file mode 100644 index 00000000..dd3d1095 --- /dev/null +++ b/test/integration/test-cases/chart-deps-condition/input/chart/charts/subchart1/templates/cm.yaml @@ -0,0 +1,6 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: testsub1 +data: + sub: 1 diff --git a/test/integration/test-cases/chart-deps-condition/input/chart/charts/subchart2/Chart.yaml b/test/integration/test-cases/chart-deps-condition/input/chart/charts/subchart2/Chart.yaml new file mode 100644 index 00000000..53c41e13 --- /dev/null +++ b/test/integration/test-cases/chart-deps-condition/input/chart/charts/subchart2/Chart.yaml @@ -0,0 +1,6 @@ +apiVersion: v2 +name: subchart2 +description: Test sub helm chart 2 +type: application +version: 2.0.0 +appVersion: "2.0.0" diff --git a/test/integration/test-cases/chart-deps-condition/input/chart/charts/subchart2/templates/cm.yaml b/test/integration/test-cases/chart-deps-condition/input/chart/charts/subchart2/templates/cm.yaml new file mode 100644 index 00000000..01084d75 --- /dev/null +++ b/test/integration/test-cases/chart-deps-condition/input/chart/charts/subchart2/templates/cm.yaml @@ -0,0 +1,6 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: testsub2 +data: + sub: 2 diff --git a/test/integration/test-cases/chart-deps-condition/input/chart/values.yaml b/test/integration/test-cases/chart-deps-condition/input/chart/values.yaml new file mode 100644 index 00000000..687024b1 --- /dev/null +++ b/test/integration/test-cases/chart-deps-condition/input/chart/values.yaml @@ -0,0 +1,2 @@ +subchart1: + install: true diff --git a/test/integration/test-cases/chart-deps-condition/input/helmfile.yaml b/test/integration/test-cases/chart-deps-condition/input/helmfile.yaml new file mode 100644 index 00000000..8e096afb --- /dev/null +++ b/test/integration/test-cases/chart-deps-condition/input/helmfile.yaml @@ -0,0 +1,57 @@ +# Test that subcharts 'condition' is working properly + +releases: +# Releases without patches, so not using chartify +- name: testdefault + chart: ./chart + namespace: testdefault +- name: testinstallfalse + chart: ./chart + namespace: testinstallfalse + values: + - subchart1: + install: false +- name: testinstalltrue + chart: ./chart + namespace: testcinstalltrue + values: + - subchart1: + install: true + +# Releases with patches, so using chartify +- name: testchartifydefault + chart: ./chart + namespace: testchartifydefault + strategicMergePatches: + - apiVersion: v1 + kind: ConfigMap + metadata: + name: testsub2 + data: + sub: 42 +- name: testchartifyinstallfalse + chart: ./chart + namespace: testchartifyinstallfalse + values: + - subchart1: + install: false + strategicMergePatches: + - apiVersion: v1 + kind: ConfigMap + metadata: + name: testsub2 + data: + sub: 42 +- name: testchartifyinstalltrue + chart: ./chart + namespace: testchartifyinstalltrue + values: + - subchart1: + install: true + strategicMergePatches: + - apiVersion: v1 + kind: ConfigMap + metadata: + name: testsub2 + data: + sub: 42 diff --git a/test/integration/test-cases/chart-deps-condition/output/template b/test/integration/test-cases/chart-deps-condition/output/template new file mode 100644 index 00000000..4644365f --- /dev/null +++ b/test/integration/test-cases/chart-deps-condition/output/template @@ -0,0 +1,86 @@ +--- +# Source: testchartsdeps/charts/subchart1/templates/cm.yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: testsub1 +data: + sub: 1 +--- +# Source: testchartsdeps/charts/subchart2/templates/cm.yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: testsub2 +data: + sub: 2 + +--- +# Source: testchartsdeps/charts/subchart2/templates/cm.yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: testsub2 +data: + sub: 2 + +--- +# Source: testchartsdeps/charts/subchart1/templates/cm.yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: testsub1 +data: + sub: 1 +--- +# Source: testchartsdeps/charts/subchart2/templates/cm.yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: testsub2 +data: + sub: 2 + +--- +# Source: testchartsdeps/templates/patched_resources.yaml +apiVersion: v1 +data: + sub: 1 +kind: ConfigMap +metadata: + name: testsub1 +--- +# Source: testchartsdeps/templates/patched_resources.yaml +apiVersion: v1 +data: + sub: 42 +kind: ConfigMap +metadata: + name: testsub2 + +--- +# Source: testchartsdeps/templates/patched_resources.yaml +apiVersion: v1 +data: + sub: 42 +kind: ConfigMap +metadata: + name: testsub2 + +--- +# Source: testchartsdeps/templates/patched_resources.yaml +apiVersion: v1 +data: + sub: 1 +kind: ConfigMap +metadata: + name: testsub1 +--- +# Source: testchartsdeps/templates/patched_resources.yaml +apiVersion: v1 +data: + sub: 42 +kind: ConfigMap +metadata: + name: testsub2 + diff --git a/test/integration/test-cases/issue-2503-kustomize-fetch.sh b/test/integration/test-cases/issue-2503-kustomize-fetch.sh new file mode 100644 index 00000000..110fc255 --- /dev/null +++ b/test/integration/test-cases/issue-2503-kustomize-fetch.sh @@ -0,0 +1,14 @@ +issue_2503_input_dir="${cases_dir}/issue-2503-kustomize-fetch/input" + +test_start "issue-2503 helmfile fetch with kustomization directory" + +info "Testing helmfile fetch with local kustomization directory (issue #2503)" + +for i in $(seq 3); do + info "Testing helmfile fetch with kustomization #$i" + issue_2503_tmp=$(mktemp -d) + ${helmfile} -f ${issue_2503_input_dir}/helmfile.yaml fetch --output-dir ${issue_2503_tmp} || fail "\"helmfile fetch\" shouldn't fail with kustomization directory" + rm -fr ${issue_2503_tmp} +done + +test_pass "issue-2503 helmfile fetch with kustomization directory" diff --git a/test/integration/test-cases/issue-2503-kustomize-fetch/input/helmfile.yaml b/test/integration/test-cases/issue-2503-kustomize-fetch/input/helmfile.yaml new file mode 100644 index 00000000..2dfe0f91 --- /dev/null +++ b/test/integration/test-cases/issue-2503-kustomize-fetch/input/helmfile.yaml @@ -0,0 +1,3 @@ +releases: +- name: test-kustomize + chart: ./kustomize diff --git a/test/integration/test-cases/issue-2503-kustomize-fetch/input/kustomize/configmap.yaml b/test/integration/test-cases/issue-2503-kustomize-fetch/input/kustomize/configmap.yaml new file mode 100644 index 00000000..4d913f17 --- /dev/null +++ b/test/integration/test-cases/issue-2503-kustomize-fetch/input/kustomize/configmap.yaml @@ -0,0 +1,6 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: test-configmap +data: + key: value diff --git a/test/integration/test-cases/issue-2503-kustomize-fetch/input/kustomize/kustomization.yaml b/test/integration/test-cases/issue-2503-kustomize-fetch/input/kustomize/kustomization.yaml new file mode 100644 index 00000000..4eb24440 --- /dev/null +++ b/test/integration/test-cases/issue-2503-kustomize-fetch/input/kustomize/kustomization.yaml @@ -0,0 +1,5 @@ +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization + +resources: +- configmap.yaml