public_git/helmfile
1
0
Fork 0
mirror of https://github.com/helmfile/helmfile.git synced 2026-10-18 19:01:50 +02:00
Code Issues Projects Releases Wiki Activity

refactor: improve readability of preBuildTransitiveSubchartDeps

Browse Source 
Extract three focused helpers from the monolithic recursive walker:

- parseLocalFileDeps: reads Chart.yaml, returns local file:// dep dirs.
  Pure data extraction, easy to test independently.
- resolveFileDep: resolves a single file:// URL to an absolute dir path.
  Encapsulates the file:// prefix check + relative-to-absolute logic.
- execHelmDep: thin wrapper around exec.Command that handles logging.
  Eliminates duplicated exec+log patterns between build and update.

Simplify prebuildChartDeps (the recursive worker): it now reads "parse
deps → recurse into each → build this chart" with no inline YAML parsing
or path resolution noise.

Net: -106/+83 lines, same behavior, all tests pass.

Signed-off-by: yxxhero <aiopsclub@163.com>
This commit is contained in:
yxxhero
2026-07-02 06:37:50 +08:00
committed by yxxhero
parent d3cfe04d47
commit 3dda0385e5
1 changed files with 87 additions and 110 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/state/state.go
+87 -110
View File
@@ -1835,58 +1835,48 @@ func (st *HelmState) rewriteChartDependencies(chartPath string) (string, func(),
}
// 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.
// dependencies declared in chartPath/Chart.yaml and runs `helm dependency build`
// on each chart's source directory bottom-up (dependencies before dependents).
//
// 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".
// This fixes envelope charts whose subcharts declare their own file:// deps:
// helm's `dependency build` on the parent packages each subchart as .tgz without
// recursively building the subchart's own deps, so the nested subchart's
// resources are silently absent and patches targeting them fail with
// "no resource matches". See https://github.com/helmfile/helmfile/issues/851.
//
// 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.
// Best-effort: errors are logged, not returned, so a single broken subchart
// doesn't abort the entire release.
func (st *HelmState) preBuildTransitiveSubchartDeps(chartPath string) {
visited := make(map[string]bool)
st.preBuildSubchartDepsRecursive(chartPath, visited)
st.prebuildChartDeps(chartPath, make(map[string]bool))
}
// 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) {
// prebuildChartDeps is the recursive worker. It visits each file:// dependency
// depth-first, then runs helm dependency build on chartDir itself.
func (st *HelmState) prebuildChartDeps(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] {
if err != nil || 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.
deps, ok := st.parseLocalFileDeps(absChartDir)
if !ok {
return
}
for _, depDir := range deps {
st.prebuildChartDeps(depDir, visited)
}
st.runHelmDepBuild(absChartDir)
}
// parseLocalFileDeps reads chartDir/Chart.yaml and returns the absolute paths
// of all local file:// dependencies that exist as directories. Returns ok=false
// if Chart.yaml is missing, unreadable, or has no file:// deps.
func (st *HelmState) parseLocalFileDeps(chartDir string) (depDirs []string, ok bool) {
data, err := st.fs.ReadFile(filepath.Join(chartDir, "Chart.yaml"))
if err != nil {
return nil, false
}
type chartDep struct {
Repository string `yaml:"repository"`
@@ -1896,92 +1886,79 @@ func (st *HelmState) preBuildSubchartDepsRecursive(chartDir string, visited map[
}
var meta chartMeta
if err := yaml.Unmarshal(data, &meta); err != nil {
st.logger.Debugf("preBuildSubchartDeps: failed to parse %s: %v", chartYamlPath, err)
return
st.logger.Debugf("preBuildSubchartDeps: failed to parse %s/Chart.yaml: %v", chartDir, err)
return nil, false
}
// 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
dir, found := st.resolveFileDep(dep.Repository, chartDir)
if found {
depDirs = append(depDirs, dir)
}
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)
return depDirs, true
}
// runHelmDependencyBuild runs `helm dependency build` on chartDir, falling back
// to `helm dependency update` if the lock file is missing or out of sync. It
// acquires a per-chartDir mutex (via getNamedRWMutex) so that concurrent
// releases referencing the same local chart don't race on helm's writes to the
// chart's charts/ and Chart.lock (same pattern as forcedDownloadChart for #768).
// resolveFileDep resolves a file:// repository URL to an absolute directory path.
// Returns (dir, true) if the dependency is a local file:// URL pointing to an
// existing directory; ("", false) otherwise.
func (st *HelmState) resolveFileDep(repo, parentDir string) (string, bool) {
const fileScheme = "file://"
if !strings.HasPrefix(repo, fileScheme) {
return "", false
}
path := strings.TrimPrefix(repo, fileScheme)
if !filepath.IsAbs(path) {
path = filepath.Join(parentDir, path)
}
if !st.fs.DirectoryExistsAt(path) {
return "", false
}
return path, true
}
// runHelmDepBuild runs `helm dependency build` on chartDir with a per-dir lock
// to prevent concurrent releases from corrupting charts/*.tgz (same pattern as
// forcedDownloadChart for #768). Falls back to `helm dependency update` when
// Chart.lock is missing or stale.
//
// `--skip-refresh` is passed to avoid a slow `helm repo update` for every
// subchart in the tree. This is safe for file://-only deps (the common case for
// envelope charts). For subcharts that also declare https:// deps, users should
// run `helm repo update` separately; chartify's own `helm dependency build`
// (which does NOT skip refresh) runs afterwards and will catch stale repos.
//
// Errors from lock-out-of-sync are expected and logged at debug level; other
// failures (helm missing, network errors for remote deps) are logged at warn
// level so users can diagnose why patches later fail with "no resource matches".
func (st *HelmState) runHelmDependencyBuild(chartDir string) {
// --skip-refresh avoids a slow `helm repo update` per subchart; file:// deps
// (the envelope-chart use case) don't need the repo cache. For mixed file:// +
// https:// deps, chartify's own helm dep build (without --skip-refresh) runs
// afterwards as a safety net.
func (st *HelmState) runHelmDepBuild(chartDir string) {
mu := st.getNamedRWMutex("prebuild-deps:" + chartDir)
mu.Lock()
defer mu.Unlock()
helmBin := st.DefaultHelmBinary
if helmBin == "" {
helmBin = "helm"
}
// Serialize per-chart-dir so concurrent releases referencing the same local
// chart tree don't corrupt charts/*.tgz or Chart.lock. The mutex is
// process-wide (rwMutexMap), matching forcedDownloadChart's approach (#768).
mu := st.getNamedRWMutex("prebuild-deps:" + chartDir)
mu.Lock()
defer mu.Unlock()
// 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()
}
// Try `dependency build` first (honors Chart.lock). Fall back to `update`
// (re-resolves from Chart.yaml) when the lock is missing or stale.
output, err := st.execHelmDep(helmBin, "build", chartDir)
if err != nil && isLockOutOfSync(output) {
st.logger.Debugf("preBuildSubchartDeps: falling back to `dependency update` for %s", chartDir)
_, _ = st.execHelmDep(helmBin, "update", chartDir)
}
if err != nil {
// Non-lock errors (helm missing, network failures, malformed Chart.yaml)
// surface as warnings so users can diagnose why strategicMergePatches
// later fail with "no resource matches". Lock-out-of-sync errors that
// also fail on `dependency update` are also warned here.
st.logger.Warnf("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.
// execHelmDep executes `helm dependency <subcmd> <chartDir> --skip-refresh`,
// logs the result, and returns the combined output and error.
func (st *HelmState) execHelmDep(helmBin, subcmd, chartDir string) ([]byte, error) {
output, err := exec.Command(helmBin, "dependency", subcmd, chartDir, "--skip-refresh").CombinedOutput()
if err != nil {
st.logger.Warnf("preBuildSubchartDeps: helm dependency %s failed for %s: %v\n%s", subcmd, chartDir, err, string(output))
} else {
st.logger.Debugf("preBuildSubchartDeps: helm dependency %s succeeded for %s", subcmd, chartDir)
}
return output, err
}
// isLockOutOfSync returns true when helm's output indicates Chart.lock is
// missing or stale — the signal to fall back from `build` to `update`.
func isLockOutOfSync(output []byte) bool {
msg := string(output)
return strings.Contains(msg, "out of sync") ||