From 08504ad6f637c779822adfa500b9ac7e45a5d201 Mon Sep 17 00:00:00 2001 From: yxxhero Date: Sun, 28 Jun 2026 21:54:21 +0800 Subject: [PATCH] fix: pre-build transitive subchart deps for envelope charts (#851) Helm's `dependency build` on the parent chart fetches each file:// subchart and packages it as .tgz, but does NOT recursively build the subchart's own dependencies first. For envelope charts whose subcharts declare their own local file:// dependencies, the resulting .tgz lacks the nested subchart's resources. chartify's `helm template` then silently drops those resources, and strategicMergePatches/jsonPatches targeting them fail with "no resource matches". Fix: add `preBuildTransitiveSubchartDeps`, called from processChartification before chartify runs (gated on !skipDeps). It walks the transitive file:// dependency tree depth-first and runs `helm dependency build` bottom-up on each subchart source directory, so each subchart's deps are pre-built before the parent packages them. The function is best-effort: errors are logged at debug level and do not abort the operation, matching helmfile's existing behavior of tolerating already-built deps. Verified with a reproduction case that fails before the fix (`Error: no resource matches strategic merge patch ".../nested-sm.[noNs]"`) and passes after it. Resolves #851 Signed-off-by: yxxhero --- docs/advanced-features.md | 18 ++ pkg/state/prebuild_subchart_deps_test.go | 216 ++++++++++++++++++ pkg/state/state.go | 147 ++++++++++++ test/integration/run.sh | 1 + test/integration/test-cases/issue-851.sh | 74 ++++++ .../issue-851/input/envelope/Chart.yaml | 11 + .../input/envelope/templates/parent-cm.yaml | 6 + .../test-cases/issue-851/input/helmfile.yaml | 46 ++++ .../issue-851/input/nested/Chart.yaml | 4 + .../input/nested/templates/nested-sm.yaml | 8 + .../issue-851/input/sub1/Chart.yaml | 4 + .../input/sub1/templates/sub1-sm.yaml | 8 + .../issue-851/input/sub2/Chart.yaml | 8 + .../input/sub2/templates/sub2-sm.yaml | 8 + 14 files changed, 559 insertions(+) create mode 100644 pkg/state/prebuild_subchart_deps_test.go create mode 100755 test/integration/test-cases/issue-851.sh create mode 100644 test/integration/test-cases/issue-851/input/envelope/Chart.yaml create mode 100644 test/integration/test-cases/issue-851/input/envelope/templates/parent-cm.yaml create mode 100644 test/integration/test-cases/issue-851/input/helmfile.yaml create mode 100644 test/integration/test-cases/issue-851/input/nested/Chart.yaml create mode 100644 test/integration/test-cases/issue-851/input/nested/templates/nested-sm.yaml create mode 100644 test/integration/test-cases/issue-851/input/sub1/Chart.yaml create mode 100644 test/integration/test-cases/issue-851/input/sub1/templates/sub1-sm.yaml create mode 100644 test/integration/test-cases/issue-851/input/sub2/Chart.yaml create mode 100644 test/integration/test-cases/issue-851/input/sub2/templates/sub2-sm.yaml diff --git a/docs/advanced-features.md b/docs/advanced-features.md index 63238b51..26ea1a4d 100644 --- a/docs/advanced-features.md +++ b/docs/advanced-features.md @@ -285,6 +285,24 @@ There's also `releases[].jsonPatches` that works similarly to `strategicMergePat Please also see [test/advanced/helmfile.yaml](https://github.com/helmfile/helmfile/tree/master/test/advanced/helmfile.yaml) for an example of patching support and more. +#### Patching envelope charts (charts whose subcharts have local dependencies) + +`strategicMergePatches`, `jsonPatches`, and `transformers` all work on resources defined +anywhere in the chart, including resources rendered by transitive subcharts (a subchart's +own dependencies). + +For local envelope charts whose subcharts declare their own `file://` dependencies, Helmfile +runs `helm dependency build` bottom-up across the transitive dependency tree before chartify +runs. Without this, helm's `dependency build` on the parent chart would fetch each file:// +subchart and package it as a `.tgz` **without** recursively building the subchart's own deps +first. The resulting `.tgz` would lack the nested subchart's resources, chartify's +`helm template` would silently drop them, and patches targeting those resources would fail +with `no resource matches`. +See [issue #851](https://github.com/helmfile/helmfile/issues/851). + +To opt out of dependency building (e.g. for read-only `template` invocations), use +`--skip-deps` or set `helmDefaults.skipDeps: true` / `releases[].skipDeps: true`. + ### `transformers` You can set `transformers` to apply [Kustomize's transformers](https://github.com/kubernetes-sigs/kustomize/blob/master/examples/configureBuiltinPlugin.md#configuring-the-builtin-plugins-instead). diff --git a/pkg/state/prebuild_subchart_deps_test.go b/pkg/state/prebuild_subchart_deps_test.go new file mode 100644 index 00000000..2a2de187 --- /dev/null +++ b/pkg/state/prebuild_subchart_deps_test.go @@ -0,0 +1,216 @@ +package state + +import ( + "os" + "path/filepath" + "testing" + "time" + + "go.uber.org/zap" + + "github.com/helmfile/helmfile/pkg/filesystem" +) + +// writeChart creates a chart directory with Chart.yaml and the given templates. +// deps maps dependency name → repository URL (skipped if empty). +func writeChart(t *testing.T, dir, name string, deps map[string]string, templates map[string]string) { + t.Helper() + chartYaml := "apiVersion: v2\nname: " + name + "\nversion: 0.1.0\n" + if len(deps) > 0 { + chartYaml += "dependencies:\n" + for depName, repo := range deps { + chartYaml += " - name: " + depName + "\n repository: " + repo + "\n version: 0.1.0\n" + } + } + if err := os.MkdirAll(dir, 0755); err != nil { + t.Fatalf("mkdir %s: %v", dir, err) + } + if err := os.WriteFile(filepath.Join(dir, "Chart.yaml"), []byte(chartYaml), 0644); err != nil { + t.Fatalf("write Chart.yaml in %s: %v", dir, err) + } + if len(templates) > 0 { + tmplDir := filepath.Join(dir, "templates") + if err := os.MkdirAll(tmplDir, 0755); err != nil { + t.Fatalf("mkdir %s: %v", tmplDir, err) + } + for filename, content := range templates { + if err := os.WriteFile(filepath.Join(tmplDir, filename), []byte(content), 0644); err != nil { + t.Fatalf("write %s: %v", filename, err) + } + } + } +} + +// TestPreBuildTransitiveSubchartDeps_VisitsAllTransitiveDeps verifies that +// preBuildTransitiveSubchartDeps walks the full transitive file:// dependency +// tree by checking that helm dep build runs on every chart dir. +// +// We assert this by checking that sub2/charts/nested-* exists after the call — +// that's the exact artifact that helm's non-recursive dep build fails to +// produce for envelope charts (issue #851). +func TestPreBuildTransitiveSubchartDeps_VisitsAllTransitiveDeps(t *testing.T) { + if testing.Short() { + t.Skip("skipping in short mode (requires helm)") + } + + rootDir, err := os.MkdirTemp("", "helmfile-prebuild-") + if err != nil { + t.Fatalf("mkdtemp: %v", err) + } + defer os.RemoveAll(rootDir) + + // envelope → sub1, sub2 + // sub2 → nested + writeChart(t, filepath.Join(rootDir, "envelope"), "envelope", + map[string]string{"sub1": "file://../sub1", "sub2": "file://../sub2"}, + map[string]string{"cm.yaml": "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: parent-cm\n"}, + ) + writeChart(t, filepath.Join(rootDir, "sub1"), "sub1", nil, + map[string]string{"sm.yaml": "apiVersion: monitoring.coreos.com/v1\nkind: ServiceMonitor\nmetadata:\n name: sub1-sm\n"}, + ) + writeChart(t, filepath.Join(rootDir, "sub2"), "sub2", + map[string]string{"nested": "file://../nested"}, + map[string]string{"sm.yaml": "apiVersion: monitoring.coreos.com/v1\nkind: ServiceMonitor\nmetadata:\n name: sub2-sm\n"}, + ) + writeChart(t, filepath.Join(rootDir, "nested"), "nested", nil, + map[string]string{"sm.yaml": "apiVersion: monitoring.coreos.com/v1\nkind: ServiceMonitor\nmetadata:\n name: nested-sm\n"}, + ) + + st := &HelmState{ + logger: zap.NewNop().Sugar(), + fs: filesystem.DefaultFileSystem(), + ReleaseSetSpec: ReleaseSetSpec{ + DefaultHelmBinary: "helm", + }, + } + + st.preBuildTransitiveSubchartDeps(filepath.Join(rootDir, "envelope")) + + // The critical assertion: sub2/charts/ should now contain nested (as + // .tgz or unpacked). Without the fix, helm dep build on the parent chart + // produces sub2.tgz WITHOUT nested, and the nested ServiceMonitor is + // silently dropped from the chartify output. + sub2Charts := filepath.Join(rootDir, "sub2", "charts") + entries, err := os.ReadDir(sub2Charts) + if err != nil { + t.Fatalf("sub2/charts should exist after preBuildTransitiveSubchartDeps ran helm dep build on sub2: %v", err) + } + foundNested := false + for _, e := range entries { + name := e.Name() + if name == "nested" || hasNestedTgzPrefix(name) { + foundNested = true + break + } + } + if !foundNested { + names := make([]string, 0, len(entries)) + for _, e := range entries { + names = append(names, e.Name()) + } + t.Fatalf("sub2/charts should contain nested after pre-build; got entries: %v", names) + } + + // envelope/charts should contain sub1 and sub2. + envCharts := filepath.Join(rootDir, "envelope", "charts") + if entries, err := os.ReadDir(envCharts); err != nil { + t.Fatalf("envelope/charts should exist after pre-build: %v", err) + } else if len(entries) == 0 { + t.Fatalf("envelope/charts should contain sub1 and sub2 after pre-build") + } +} + +func hasNestedTgzPrefix(name string) bool { + return len(name) > len("nested-") && name[:7] == "nested-" +} + +// TestPreBuildTransitiveSubchartDeps_HandlesMissingChartYaml verifies the +// function is a no-op (no panic) when the chart dir has no Chart.yaml. +func TestPreBuildTransitiveSubchartDeps_HandlesMissingChartYaml(t *testing.T) { + rootDir, err := os.MkdirTemp("", "helmfile-prebuild-noyaml-") + if err != nil { + t.Fatalf("mkdtemp: %v", err) + } + defer os.RemoveAll(rootDir) + + st := &HelmState{ + logger: zap.NewNop().Sugar(), + fs: filesystem.DefaultFileSystem(), + ReleaseSetSpec: ReleaseSetSpec{ + DefaultHelmBinary: "helm", + }, + } + + // Should not panic. + st.preBuildTransitiveSubchartDeps(rootDir) +} + +// TestPreBuildTransitiveSubchartDeps_HandlesCircularDeps verifies the function +// terminates when the file:// dependency graph has a cycle. +func TestPreBuildTransitiveSubchartDeps_HandlesCircularDeps(t *testing.T) { + if testing.Short() { + t.Skip("skipping in short mode (requires helm)") + } + + rootDir, err := os.MkdirTemp("", "helmfile-prebuild-cycle-") + if err != nil { + t.Fatalf("mkdtemp: %v", err) + } + defer os.RemoveAll(rootDir) + + // a → b → a (cycle). Use absolute file:// paths so each can resolve the other. + aDir := filepath.Join(rootDir, "a") + bDir := filepath.Join(rootDir, "b") + writeChart(t, aDir, "a", map[string]string{"b": "file://" + bDir}, nil) + writeChart(t, bDir, "b", map[string]string{"a": "file://" + aDir}, nil) + + st := &HelmState{ + logger: zap.NewNop().Sugar(), + fs: filesystem.DefaultFileSystem(), + ReleaseSetSpec: ReleaseSetSpec{ + DefaultHelmBinary: "helm", + }, + } + + // Should terminate, not infinite-loop. + done := make(chan struct{}) + go func() { + defer close(done) + st.preBuildTransitiveSubchartDeps(aDir) + }() + select { + case <-done: + // success — terminated + case <-time.After(30 * time.Second): + t.Fatal("preBuildTransitiveSubchartDeps did not terminate on circular deps") + } +} + +// TestPreBuildTransitiveSubchartDeps_NoOpOnRemoteDeps verifies the function +// skips dependencies declared with non-file:// repositories (https://, oci://). +func TestPreBuildTransitiveSubchartDeps_NoOpOnRemoteDeps(t *testing.T) { + rootDir, err := os.MkdirTemp("", "helmfile-prebuild-remote-") + if err != nil { + t.Fatalf("mkdtemp: %v", err) + } + defer os.RemoveAll(rootDir) + + // Chart with only a remote dep. preBuildTransitiveSubchartDeps should not + // recurse into anything (no file:// deps to follow) and should not panic + // when helm dep build can't resolve the remote dep in the test environment. + writeChart(t, filepath.Join(rootDir, "chart"), "chart", + map[string]string{"remote": "https://charts.example.com"}, + nil, + ) + + st := &HelmState{ + logger: zap.NewNop().Sugar(), + fs: filesystem.DefaultFileSystem(), + ReleaseSetSpec: ReleaseSetSpec{ + DefaultHelmBinary: "helm", + }, + } + + // Should not panic; helm dep build failure is logged and swallowed. + st.preBuildTransitiveSubchartDeps(filepath.Join(rootDir, "chart")) +} diff --git a/pkg/state/state.go b/pkg/state/state.go index eb5a7bfd..88c2ae7e 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -12,6 +12,7 @@ import ( "io" "net/url" "os" + "os/exec" "path/filepath" "regexp" "runtime" @@ -1833,11 +1834,157 @@ func (st *HelmState) rewriteChartDependencies(chartPath string) (string, func(), return tempDir, cleanup, nil } +// preBuildTransitiveSubchartDeps walks the transitive tree of local file:// +// dependencies declared in chartPath/Chart.yaml (and recursively, each +// dependency's own Chart.yaml), and runs `helm dependency build` on each +// dependency's source directory in bottom-up order. +// +// This is required for envelope charts (charts whose subcharts themselves have +// local file:// dependencies). Helm's `helm dependency build` on the parent +// chart fetches each file:// subchart and packages it as a .tgz archive, but +// does NOT recursively build the subchart's own dependencies first. The +// resulting .tgz therefore lacks the nested subchart's resources. When chartify +// later runs helm template --output-dir on the chart, resources defined in the +// nested subchart are silently absent, and strategicMergePatches/jsonPatches +// targeting those resources fail with "no resource matches". +// +// By pre-building each subchart's dependencies bottom-up, the .tgz that helm +// later produces for the parent includes the nested subchart's deps, so all +// resources are rendered and patches resolve correctly. +// +// See https://github.com/helmfile/helmfile/issues/851. +// +// This function is best-effort: failures to read a Chart.yaml or run helm on a +// subchart are logged at debug level and do not abort the top-level operation, +// matching helmfile's behavior of not hard-failing on subchart dependency +// issues when the chart may already have its deps pre-built. +func (st *HelmState) preBuildTransitiveSubchartDeps(chartPath string) { + visited := make(map[string]bool) + st.preBuildSubchartDepsRecursive(chartPath, visited) +} + +// preBuildSubchartDepsRecursive is the recursive worker for +// preBuildTransitiveSubchartDeps. It visits each file:// dependency of chartDir +// depth-first (so dependencies are built before the charts that depend on +// them), then runs `helm dependency build` on chartDir itself. +// +// `visited` tracks absolute chart directory paths to prevent infinite recursion +// on circular file:// dependency graphs. +func (st *HelmState) preBuildSubchartDepsRecursive(chartDir string, visited map[string]bool) { + absChartDir, err := filepath.Abs(chartDir) + if err != nil { + st.logger.Debugf("preBuildSubchartDeps: failed to resolve abs path for %s: %v", chartDir, err) + return + } + if visited[absChartDir] { + return + } + visited[absChartDir] = true + + chartYamlPath := filepath.Join(absChartDir, "Chart.yaml") + data, err := st.fs.ReadFile(chartYamlPath) + if err != nil { + // No Chart.yaml or unreadable — nothing to pre-build. + return + } + + type chartDep struct { + Repository string `yaml:"repository"` + } + type chartMeta struct { + Dependencies []chartDep `yaml:"dependencies,omitempty"` + } + var meta chartMeta + if err := yaml.Unmarshal(data, &meta); err != nil { + st.logger.Debugf("preBuildSubchartDeps: failed to parse %s: %v", chartYamlPath, err) + return + } + + // First, recurse into each local file:// dependency so its own deps are + // pre-built before we build this chart's deps. + for _, dep := range meta.Dependencies { + if !strings.HasPrefix(dep.Repository, "file://") { + continue + } + depPath := strings.TrimPrefix(dep.Repository, "file://") + if !filepath.IsAbs(depPath) { + depPath = filepath.Join(absChartDir, depPath) + } + // Only pre-build local directory dependencies (not archives, not remote). + if !st.fs.DirectoryExistsAt(depPath) { + continue + } + st.preBuildSubchartDepsRecursive(depPath, visited) + } + + // Then run `helm dependency build` on this chart dir. If Chart.lock exists + // and is in sync, helm uses it (preserving pinned versions). If the lock is + // missing or stale, fall back to `helm dependency update`. Errors are logged + // but not propagated: the chart may already have its deps pre-built, in + // which case chartify's own helm dep build will succeed later. + st.runHelmDependencyBuild(absChartDir) +} + +// runHelmDependencyBuild runs `helm dependency build` on chartDir, falling back +// to `helm dependency update` if the lock file is missing or out of sync. It +// uses --skip-refresh to avoid hitting the network for local file:// deps (the +// only kind preBuildSubchartDepsRecursive descends into). Errors are logged at +// debug level and otherwise discarded. +func (st *HelmState) runHelmDependencyBuild(chartDir string) { + helmBin := st.DefaultHelmBinary + if helmBin == "" { + helmBin = "helm" + } + + // Prefer `dependency build` (honors Chart.lock) over `dependency update` + // (re-resolves version constraints against repos, which can silently pull + // newer versions). Same strategy chartify uses internally. + cmd := exec.Command(helmBin, "dependency", "build", chartDir, "--skip-refresh") + output, err := cmd.CombinedOutput() + if err != nil { + // `dependency build` fails when Chart.lock is missing or out of sync + // with Chart.yaml. Fall back to `dependency update`, which re-resolves + // from Chart.yaml. This matches chartify's fallback behavior. + if isLockOutOfSync(output) { + st.logger.Debugf("preBuildSubchartDeps: `helm dependency build` failed for %s (lock out of sync), falling back to `dependency update`: %v", chartDir, err) + cmd = exec.Command(helmBin, "dependency", "update", chartDir, "--skip-refresh") + output, err = cmd.CombinedOutput() + } + } + if err != nil { + st.logger.Debugf("preBuildSubchartDeps: helm dependency command failed for %s: %v\n%s", chartDir, err, string(output)) + return + } + st.logger.Debugf("preBuildSubchartDeps: built dependencies for %s", chartDir) +} + +// isLockOutOfSync returns true when the helm output indicates the Chart.lock +// is missing or out of sync with Chart.yaml — the case where falling back from +// `dependency build` to `dependency update` is the right move. +func isLockOutOfSync(output []byte) bool { + msg := string(output) + return strings.Contains(msg, "out of sync") || + strings.Contains(msg, "lock file is out of date") || + strings.Contains(msg, "no lock file") || + strings.Contains(msg, "Chart.lock not found") +} + // Otherwise, if a chart is not a helm chart, it will call "chartify" to turn it into a chart. // // If exists, it will also patch resources by json patches, strategic-merge patches, and injectors. // processChartification handles the chartification process func (st *HelmState) processChartification(chartification *Chartify, release *ReleaseSpec, chartPath string, opts ChartPrepareOptions, skipDeps bool, helmfileCommand string) (string, bool, error) { + // Pre-build transitive local file:// subchart dependencies before chartify runs. + // helm's `dependency build` on the parent chart fetches each file:// subchart and + // packages it as .tgz, but does NOT recursively build the subchart's own deps. + // Without this pre-build step, envelope charts whose subcharts have their own + // local file:// dependencies silently drop the nested subcharts' resources, and + // strategicMergePatches/jsonPatches targeting those resources fail with + // "no resource matches". See https://github.com/helmfile/helmfile/issues/851. + if !skipDeps && st.fs.DirectoryExistsAt(chartPath) { + st.preBuildTransitiveSubchartDeps(chartPath) + } + // Rewrite relative file:// dependencies in Chart.yaml to absolute paths before chartify processes them // This prevents errors like "Error: directory /tmp/chartify.../argocd-application not found" // when Chart.yaml contains dependencies like "file://../argocd-application" diff --git a/test/integration/run.sh b/test/integration/run.sh index cfd1e8de..174b41d7 100755 --- a/test/integration/run.sh +++ b/test/integration/run.sh @@ -132,6 +132,7 @@ ${kubectl} create namespace ${test_ns} || fail "Could not create namespace ${tes . ${dir}/test-cases/issue-2247.sh . ${dir}/test-cases/issue-2097.sh . ${dir}/test-cases/issue-2291.sh +. ${dir}/test-cases/issue-851.sh . ${dir}/test-cases/oci-parallel-pull.sh . ${dir}/test-cases/issue-2297-local-chart-transformers.sh . ${dir}/test-cases/issue-2309-kube-context-template.sh diff --git a/test/integration/test-cases/issue-851.sh b/test/integration/test-cases/issue-851.sh new file mode 100755 index 00000000..53e64e91 --- /dev/null +++ b/test/integration/test-cases/issue-851.sh @@ -0,0 +1,74 @@ +#!/usr/bin/env bash + +# Test for issue #851: Strategic merge with envelope charts does not work +# Issue: https://github.com/helmfile/helmfile/issues/851 +# +# Problem: When an envelope chart's subchart declares its own local file:// +# dependency, helm's `dependency build` on the parent chart fetches the subchart +# and packages it as .tgz, but does NOT recursively build the subchart's own +# deps first. The resulting .tgz lacks the nested subchart's resources. chartify's +# helm template silently drops those resources, and strategicMergePatches/jsonPatches +# targeting them fail with "no resource matches". +# +# Fix: preBuildTransitiveSubchartDeps walks the transitive file:// dep tree and +# runs `helm dependency build` bottom-up on each subchart source dir before +# chartify runs, so every subchart .tgz includes its own nested deps. + +issue_851_input_dir="${cases_dir}/issue-851/input" + +# Copy the chart tree to a temp dir so the test doesn't leave generated +# Chart.lock / charts/*.tgz files in the repo. preBuildTransitiveSubchartDeps +# runs `helm dependency build` on subchart source directories, which creates +# those artifacts. The test must not pollute the source tree. +issue_851_tmp_dir=$(mktemp -d) +cp -r "${issue_851_input_dir}/." "${issue_851_tmp_dir}/" + +cleanup_issue_851() { + rm -rf "${issue_851_tmp_dir}" +} +trap cleanup_issue_851 EXIT + +test_start "issue-851: strategic merge patches work with envelope charts" + +info "Step 1: Templating envelope chart whose subchart has its own file:// dep" +${helmfile} -f "${issue_851_tmp_dir}/helmfile.yaml" template > "${issue_851_tmp_dir}/templated.yaml" 2>&1 +code=$? + +if [ $code -ne 0 ]; then + cat "${issue_851_tmp_dir}/templated.yaml" + fail "helmfile template failed — envelope chart with nested subchart patches should succeed" +fi + +info "✓ helmfile template succeeded" + +# All three ServiceMonitors must be present, including the one defined in sub2's +# OWN dependency (nested). Without the fix, nested-sm is silently dropped. +for expected_name in sub1-sm sub2-sm nested-sm; do + if ! grep -q "name: ${expected_name}" "${issue_851_tmp_dir}/templated.yaml"; then + cat "${issue_851_tmp_dir}/templated.yaml" + fail "Expected ServiceMonitor ${expected_name} not found (envelope chart nested subchart resources dropped)" + fi + info "✓ ServiceMonitor ${expected_name} present" +done + +# Each ServiceMonitor must reflect its strategicMergePatch. +for expected_port in metrics-sub1 metrics-sub2 metrics-nested; do + if ! grep -q "port: ${expected_port}" "${issue_851_tmp_dir}/templated.yaml"; then + cat "${issue_851_tmp_dir}/templated.yaml" + fail "Patch targeting ${expected_port} was not applied (issue 851 regression)" + fi +done +info "✓ all strategicMergePatches applied (sub1-sm, sub2-sm, nested-sm)" + +# The critical assertion: nested-sm's patch applied. Without the fix, the nested +# subchart's resources are missing and this patch fails with "no resource matches". +if ! grep -q "port: metrics-nested" "${issue_851_tmp_dir}/templated.yaml"; then + cat "${issue_851_tmp_dir}/templated.yaml" + fail "Patch for nested-sm was not applied — envelope chart regression (issue 851)" +fi +info "✓ nested-sm patch applied (issue 851 fixed)" + +cleanup_issue_851 +trap - EXIT + +test_pass "issue-851: strategic merge patches work with envelope charts" diff --git a/test/integration/test-cases/issue-851/input/envelope/Chart.yaml b/test/integration/test-cases/issue-851/input/envelope/Chart.yaml new file mode 100644 index 00000000..771b7a43 --- /dev/null +++ b/test/integration/test-cases/issue-851/input/envelope/Chart.yaml @@ -0,0 +1,11 @@ +apiVersion: v2 +name: envelope +version: 0.1.0 +description: Envelope chart whose subchart has its own local file:// dep (issue 851) +dependencies: + - name: sub1 + version: 0.1.0 + repository: file://../sub1 + - name: sub2 + version: 0.1.0 + repository: file://../sub2 diff --git a/test/integration/test-cases/issue-851/input/envelope/templates/parent-cm.yaml b/test/integration/test-cases/issue-851/input/envelope/templates/parent-cm.yaml new file mode 100644 index 00000000..c0b2d35c --- /dev/null +++ b/test/integration/test-cases/issue-851/input/envelope/templates/parent-cm.yaml @@ -0,0 +1,6 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: parent-cm +data: + key: parent-value diff --git a/test/integration/test-cases/issue-851/input/helmfile.yaml b/test/integration/test-cases/issue-851/input/helmfile.yaml new file mode 100644 index 00000000..a6f91bb8 --- /dev/null +++ b/test/integration/test-cases/issue-851/input/helmfile.yaml @@ -0,0 +1,46 @@ +# Test for issue #851: Strategic merge with envelope charts does not work +# https://github.com/helmfile/helmfile/issues/851 +# +# Problem: When an envelope chart's subchart declares its own local file:// +# dependency, helm's `dependency build` on the parent chart fetches the subchart +# and packages it as .tgz, but does NOT recursively build the subchart's own deps +# first. The resulting .tgz lacks the nested subchart's resources. When chartify +# runs helm template, resources from the nested subchart are silently absent, +# and strategicMergePatches targeting them fail with "no resource matches". +# +# Fix: preBuildTransitiveSubchartDeps walks the transitive file:// dep tree and +# runs `helm dependency build` bottom-up on each subchart source dir before +# chartify runs, so every subchart .tgz includes its own nested deps. + +releases: + - name: envelope + namespace: test + chart: ./envelope + strategicMergePatches: + - apiVersion: monitoring.coreos.com/v1 + kind: ServiceMonitor + metadata: + name: sub1-sm + spec: + endpoints: + - port: metrics-sub1 + interval: 10s + - apiVersion: monitoring.coreos.com/v1 + kind: ServiceMonitor + metadata: + name: sub2-sm + spec: + endpoints: + - port: metrics-sub2 + interval: 15s + # The critical patch: targets a resource defined in sub2's OWN dependency + # (nested). Without the fix, this resource is dropped from chartify's + # patching set and the patch fails with "no resource matches". + - apiVersion: monitoring.coreos.com/v1 + kind: ServiceMonitor + metadata: + name: nested-sm + spec: + endpoints: + - port: metrics-nested + interval: 20s diff --git a/test/integration/test-cases/issue-851/input/nested/Chart.yaml b/test/integration/test-cases/issue-851/input/nested/Chart.yaml new file mode 100644 index 00000000..ed62dd08 --- /dev/null +++ b/test/integration/test-cases/issue-851/input/nested/Chart.yaml @@ -0,0 +1,4 @@ +apiVersion: v2 +name: nested +version: 0.1.0 +description: Deepest nested subchart — its resources are silently dropped without the fix diff --git a/test/integration/test-cases/issue-851/input/nested/templates/nested-sm.yaml b/test/integration/test-cases/issue-851/input/nested/templates/nested-sm.yaml new file mode 100644 index 00000000..465e3f89 --- /dev/null +++ b/test/integration/test-cases/issue-851/input/nested/templates/nested-sm.yaml @@ -0,0 +1,8 @@ +apiVersion: monitoring.coreos.com/v1 +kind: ServiceMonitor +metadata: + name: nested-sm +spec: + endpoints: + - port: web + interval: 30s diff --git a/test/integration/test-cases/issue-851/input/sub1/Chart.yaml b/test/integration/test-cases/issue-851/input/sub1/Chart.yaml new file mode 100644 index 00000000..e2ffc501 --- /dev/null +++ b/test/integration/test-cases/issue-851/input/sub1/Chart.yaml @@ -0,0 +1,4 @@ +apiVersion: v2 +name: sub1 +version: 0.1.0 +description: Leaf subchart (no deps) diff --git a/test/integration/test-cases/issue-851/input/sub1/templates/sub1-sm.yaml b/test/integration/test-cases/issue-851/input/sub1/templates/sub1-sm.yaml new file mode 100644 index 00000000..4ab481a5 --- /dev/null +++ b/test/integration/test-cases/issue-851/input/sub1/templates/sub1-sm.yaml @@ -0,0 +1,8 @@ +apiVersion: monitoring.coreos.com/v1 +kind: ServiceMonitor +metadata: + name: sub1-sm +spec: + endpoints: + - port: web + interval: 30s diff --git a/test/integration/test-cases/issue-851/input/sub2/Chart.yaml b/test/integration/test-cases/issue-851/input/sub2/Chart.yaml new file mode 100644 index 00000000..4a4a8b46 --- /dev/null +++ b/test/integration/test-cases/issue-851/input/sub2/Chart.yaml @@ -0,0 +1,8 @@ +apiVersion: v2 +name: sub2 +version: 0.1.0 +description: Subchart with its own local file:// dep (the envelope chart scenario) +dependencies: + - name: nested + version: 0.1.0 + repository: file://../nested diff --git a/test/integration/test-cases/issue-851/input/sub2/templates/sub2-sm.yaml b/test/integration/test-cases/issue-851/input/sub2/templates/sub2-sm.yaml new file mode 100644 index 00000000..057e3a00 --- /dev/null +++ b/test/integration/test-cases/issue-851/input/sub2/templates/sub2-sm.yaml @@ -0,0 +1,8 @@ +apiVersion: monitoring.coreos.com/v1 +kind: ServiceMonitor +metadata: + name: sub2-sm +spec: + endpoints: + - port: web + interval: 30s