Merge remote-tracking branch 'origin/main' into fix-include-needs-transitive-1003

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>
This commit is contained in:
copilot-swe-agent[bot] 2026-03-27 08:23:26 +00:00
commit f7a81518ed
17 changed files with 308 additions and 60 deletions

View File

@ -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 {

View File

@ -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
}
}

View File

@ -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

View File

@ -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

View File

@ -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"

View File

@ -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

View File

@ -0,0 +1,6 @@
apiVersion: v2
name: subchart1
description: Test sub helm chart 1
type: application
version: 1.0.0
appVersion: "1.0.0"

View File

@ -0,0 +1,6 @@
apiVersion: v1
kind: ConfigMap
metadata:
name: testsub1
data:
sub: 1

View File

@ -0,0 +1,6 @@
apiVersion: v2
name: subchart2
description: Test sub helm chart 2
type: application
version: 2.0.0
appVersion: "2.0.0"

View File

@ -0,0 +1,6 @@
apiVersion: v1
kind: ConfigMap
metadata:
name: testsub2
data:
sub: 2

View File

@ -0,0 +1,2 @@
subchart1:
install: true

View File

@ -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

View File

@ -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

View File

@ -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"

View File

@ -0,0 +1,3 @@
releases:
- name: test-kustomize
chart: ./kustomize

View File

@ -0,0 +1,6 @@
apiVersion: v1
kind: ConfigMap
metadata:
name: test-configmap
data:
key: value

View File

@ -0,0 +1,5 @@
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
- configmap.yaml