mirror of
https://github.com/helmfile/helmfile.git
synced 2026-10-03 20:15:07 +02:00
fix: refresh Chart.lock after rewriting file:// dependencies (#2587)
* fix: refresh Chart.lock after rewriting file:// dependencies
`rewriteChartDependencies` rewrites relative `file://` repository URLs in
Chart.yaml to absolute paths so chartify can resolve them from a temp
directory. That mutates the Chart.yaml dependencies block, which
invalidates the Chart.lock digest (helm computes it as
`sha256(json.Marshal([2][]Dependency{req, lock}))` over the dependencies).
Once the lock is out of sync, downstream `helm dependency build` errors
with "the lock file (Chart.lock) is out of sync with the dependencies
file (Chart.yaml)" and chartify falls back to `helm dependency update`.
`dep update` then re-resolves Chart.yaml's version constraints against
the chart repo, so any constraint that admits newer versions
(e.g. `version: "*"`, `~1.0`) silently picks up a newer dependency on
every render — even though Chart.lock pins a specific version.
Repro:
- Chart.yaml has `version: "*"` for some-dep, Chart.lock pins 4.1.0,
upstream now publishes 4.2.0.
- `helm template .` honors the cached `charts/some-dep-4.1.0.tgz`.
- `helmfile template` produces 4.2.0, because it triggered chartify
(via jsonPatches/strategic-merge/kustomize/etc), which copied the
chart, ran `dep build` against an out-of-sync lock, fell back to
`dep up`, and re-resolved the wildcard.
This commit refreshes Chart.lock alongside Chart.yaml in the temp copy:
- Mirror the rewritten file:// repository URLs onto matching entries in
Chart.lock's dependencies. Without this, `helm dep build` would resolve
the lock's relative `file://` paths against the temp chart directory
and fail with "directory ... not found".
- Recompute the digest using helm's resolver.HashReq algorithm
(`sha256(json.Marshal([2][]chart.Dependency{req, lock}))`). The
algorithm is small and stable; resolver.HashReq itself lives in an
internal package, so it's inlined here.
- Locked versions are preserved verbatim — only the repository URL is
updated and the digest recomputed. Chart.lock remains the source of
truth for which versions get installed.
- The original Chart.lock on disk is never modified; only the temp copy
is rewritten.
Adds TestRewriteChartDependencies_RefreshesChartLock covering digest
recomputation, file:// URL mirroring, version preservation, untouched
non-file:// deps, and original-on-disk integrity.
Signed-off-by: Shane Starcher <shane.starcher@gmail.com>
* fix: address Copilot review issues for Chart.lock refresh
- Map all helm Dependency fields (alias, condition, tags, import-values,
enabled) when building the request slice for digest computation, not
just name/version/repository. This ensures the recomputed digest
matches Helm's resolver.HashReq for all dependency shapes.
- Match lock entries by Name + Alias (not Name alone) to correctly
handle charts with duplicate dependency names distinguished by alias.
- Log a warning when reading Chart.lock fails with a non-NotExist error,
while still treating a missing Chart.lock as expected.
- Add test case exercising dependencies with alias, condition, tags, and
import-values fields, including same-name deps disambiguated by alias.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Shane Starcher <shane.starcher@gmail.com>
* build(deps): bump github.com/helmfile/chartify to v0.26.4
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Shane Starcher <shane.starcher@gmail.com>
* fix: normalize import-values for JSON marshaling and improve test coverage
- Normalize import-values using maputil.RecursivelyStringifyMapKey before
assigning to helmchart.Dependency.ImportValues. When go-yaml v2 decodes
nested maps (e.g. import-values entries with child/parent keys), they
become map[interface{}]interface{} which json.Marshal cannot encode.
This would silently prevent Chart.lock rewriting. The normalization
converts all map keys to strings, making the value JSON-safe.
- Improve TestRewriteChartDependencies_RefreshesChartLockWithExtraFields
to prove that extra fields (condition, tags, import-values) actually
affect the computed digest by comparing digests with and without those
fields and asserting they differ.
Signed-off-by: Shane Starcher <shane.starcher@gmail.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: normalize lock ImportValues and fix digest test isolation
- Normalize lock.Dependencies ImportValues via RecursivelyStringifyMapKey
before json.Marshal, preventing failures when go-yaml v2 decodes nested
maps as map[interface{}]interface{}.
- Fix TestRewriteChartDependencies_RefreshesChartLockWithExtraFields to use
a shared root directory so both chart variants resolve file:// paths to
the same absolute location, isolating digest differences to field content.
- Add TestRewriteChartDependencies_GoYamlV2ImportValues exercising the
HELMFILE_GO_YAML_V3=false path with import-values containing nested maps.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Shane Starcher <shane.starcher@gmail.com>
* fix: add exact digest verification test against Helm's HashReq
Add TestRewriteChartDependencies_DigestMatchesHelmHashReq which computes
the expected digest independently using the same algorithm as Helm's
resolver.HashReq and asserts the rewritten Chart.lock matches exactly.
This guards against producing a digest that is "different" yet still
rejected by `helm dependency build`.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Shane Starcher <shane.starcher@gmail.com>
---------
Signed-off-by: Shane Starcher <shane.starcher@gmail.com>
Co-authored-by: Shane Starcher <shane.starcher@gmail.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Shane Starcher
co-authored by
Claude Opus 4.6
Shane Starcher
Claude Opus 4.6
Shane Starcher
parent
e96a27734a
commit
23802e7a95
@@ -17,7 +17,7 @@ require (
|
||||
github.com/hashicorp/go-cty-funcs v0.1.0
|
||||
github.com/hashicorp/go-getter/v2 v2.2.3
|
||||
github.com/hashicorp/hcl/v2 v2.24.0
|
||||
github.com/helmfile/chartify v0.26.3
|
||||
github.com/helmfile/chartify v0.26.4
|
||||
github.com/helmfile/vals v0.44.0
|
||||
github.com/spf13/cobra v1.10.2
|
||||
github.com/spf13/pflag v1.0.10
|
||||
|
||||
@@ -532,8 +532,8 @@ github.com/hashicorp/jsonapi v1.4.3-0.20250220162346-81a76b606f3e h1:xwy/1T0cxHW
|
||||
github.com/hashicorp/jsonapi v1.4.3-0.20250220162346-81a76b606f3e/go.mod h1:kWfdn49yCjQvbpnvY1dxxAuAFzISwrrMDQOcu6NsFoM=
|
||||
github.com/hashicorp/vault/api v1.23.0 h1:gXgluBsSECfRWTSW9niY2jwg2e9mMJc4WoHNv4g3h6A=
|
||||
github.com/hashicorp/vault/api v1.23.0/go.mod h1:zransKiB9ftp+kgY8ydjnvCU7Wk8i9L0DYWpXeMj9ko=
|
||||
github.com/helmfile/chartify v0.26.3 h1:2wR0yfqtP/yG9y6uqM6nSKZ7W0E+nhhGGRsl14TOVVs=
|
||||
github.com/helmfile/chartify v0.26.3/go.mod h1:/ReUGTnbNHIV5tKAGXODkRtS7HwnUiJi2EXbJ34RzgY=
|
||||
github.com/helmfile/chartify v0.26.4 h1:pIzVe+mqBiBMlJEH3qUVKgFQKV/m4vGOVccdYWY4VbI=
|
||||
github.com/helmfile/chartify v0.26.4/go.mod h1:jnMhinkuwSMfgPPNb3JYges/13xkXPEdUVnh1eGxTOQ=
|
||||
github.com/helmfile/vals v0.44.0 h1:9Yf5JDIl3JUHE1XWR9GopurvAbuXowCSsgUShB4aWcI=
|
||||
github.com/helmfile/vals v0.44.0/go.mod h1:siAvy7f4VPPCrgLGzDOW21ZbvR6Tbf9g7oGRme9fMH4=
|
||||
github.com/hinshun/vt10x v0.0.0-20220119200601-820417d04eec h1:qv2VnGeEQHchGaZ/u7lxST/RaJw+cv273q79D81Xbog=
|
||||
|
||||
@@ -1,6 +1,9 @@
|
||||
package state
|
||||
|
||||
import (
|
||||
"crypto/sha256"
|
||||
"encoding/hex"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
@@ -9,8 +12,10 @@ import (
|
||||
"testing"
|
||||
|
||||
"go.uber.org/zap"
|
||||
helmchart "helm.sh/helm/v3/pkg/chart"
|
||||
|
||||
"github.com/helmfile/helmfile/pkg/filesystem"
|
||||
"github.com/helmfile/helmfile/pkg/runtime"
|
||||
"github.com/helmfile/helmfile/pkg/yaml"
|
||||
)
|
||||
|
||||
@@ -645,3 +650,462 @@ dependencies:
|
||||
t.Errorf("expected original dependency repository %q, got %q", wantRepository, chartMeta.Dependencies[0].Repository)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRewriteChartDependencies_RefreshesChartLock verifies that when Chart.yaml has
|
||||
// its file:// dependencies rewritten to absolute paths, an existing Chart.lock is
|
||||
// also updated in the temp copy: the digest is recomputed (otherwise `helm dep
|
||||
// build` would error with "lock out of sync") and matching file:// repository URLs
|
||||
// are mirrored over from the rewritten Chart.yaml (otherwise `helm dep build` would
|
||||
// resolve the lock's relative file:// path against the temp directory and fail).
|
||||
// Locked versions are preserved verbatim.
|
||||
func TestRewriteChartDependencies_RefreshesChartLock(t *testing.T) {
|
||||
tempDir := t.TempDir()
|
||||
|
||||
chartYaml := `apiVersion: v2
|
||||
name: parent-chart
|
||||
version: 1.0.0
|
||||
dependencies:
|
||||
- name: local-dep
|
||||
repository: file://../local-dep
|
||||
version: 1.0.0
|
||||
- name: remote-dep
|
||||
repository: https://example.com/charts
|
||||
version: "*"
|
||||
`
|
||||
if err := os.WriteFile(filepath.Join(tempDir, "Chart.yaml"), []byte(chartYaml), 0644); err != nil {
|
||||
t.Fatalf("writing Chart.yaml: %v", err)
|
||||
}
|
||||
|
||||
const originalDigest = "sha256:0000000000000000000000000000000000000000000000000000000000000000"
|
||||
chartLock := `dependencies:
|
||||
- name: local-dep
|
||||
repository: file://../local-dep
|
||||
version: 1.0.0
|
||||
- name: remote-dep
|
||||
repository: https://example.com/charts
|
||||
version: 1.2.3
|
||||
digest: ` + originalDigest + `
|
||||
generated: "2024-01-01T00:00:00Z"
|
||||
`
|
||||
if err := os.WriteFile(filepath.Join(tempDir, "Chart.lock"), []byte(chartLock), 0644); err != nil {
|
||||
t.Fatalf("writing Chart.lock: %v", err)
|
||||
}
|
||||
|
||||
logger := zap.NewNop().Sugar()
|
||||
st := &HelmState{
|
||||
basePath: tempDir,
|
||||
fs: filesystem.DefaultFileSystem(),
|
||||
logger: logger,
|
||||
}
|
||||
|
||||
rewrittenPath, cleanup, err := st.rewriteChartDependencies(tempDir)
|
||||
if err != nil {
|
||||
t.Fatalf("rewriteChartDependencies failed: %v", err)
|
||||
}
|
||||
defer cleanup()
|
||||
|
||||
if rewrittenPath == tempDir {
|
||||
t.Fatalf("expected a temp copy to be created, got original path %q", rewrittenPath)
|
||||
}
|
||||
|
||||
lockData, err := os.ReadFile(filepath.Join(rewrittenPath, "Chart.lock"))
|
||||
if err != nil {
|
||||
t.Fatalf("reading rewritten Chart.lock: %v", err)
|
||||
}
|
||||
|
||||
var lock struct {
|
||||
Dependencies []struct {
|
||||
Name string `yaml:"name"`
|
||||
Repository string `yaml:"repository"`
|
||||
Version string `yaml:"version"`
|
||||
} `yaml:"dependencies"`
|
||||
Digest string `yaml:"digest"`
|
||||
Generated string `yaml:"generated"`
|
||||
}
|
||||
if err := yaml.Unmarshal(lockData, &lock); err != nil {
|
||||
t.Fatalf("parsing rewritten Chart.lock: %v", err)
|
||||
}
|
||||
|
||||
if lock.Digest == originalDigest {
|
||||
t.Errorf("expected digest to be recomputed; still %q", lock.Digest)
|
||||
}
|
||||
if !strings.HasPrefix(lock.Digest, "sha256:") {
|
||||
t.Errorf("expected sha256 digest, got %q", lock.Digest)
|
||||
}
|
||||
|
||||
if len(lock.Dependencies) != 2 {
|
||||
t.Fatalf("expected 2 lock dependencies, got %d", len(lock.Dependencies))
|
||||
}
|
||||
|
||||
// The local file:// dependency's repository must be mirrored to the absolute
|
||||
// path so `helm dep build` can resolve it from the temp chart directory.
|
||||
localDep := lock.Dependencies[0]
|
||||
if localDep.Name != "local-dep" {
|
||||
t.Fatalf("expected first lock dep name 'local-dep', got %q", localDep.Name)
|
||||
}
|
||||
if !filepath.IsAbs(strings.TrimPrefix(localDep.Repository, "file://")) {
|
||||
t.Errorf("expected local-dep repository to be an absolute file:// path, got %q", localDep.Repository)
|
||||
}
|
||||
if localDep.Version != "1.0.0" {
|
||||
t.Errorf("expected local-dep version preserved as 1.0.0, got %q", localDep.Version)
|
||||
}
|
||||
|
||||
// Remote (non-file://) deps must be untouched.
|
||||
remoteDep := lock.Dependencies[1]
|
||||
if remoteDep.Repository != "https://example.com/charts" {
|
||||
t.Errorf("expected remote dep repository unchanged, got %q", remoteDep.Repository)
|
||||
}
|
||||
if remoteDep.Version != "1.2.3" {
|
||||
t.Errorf("expected remote dep version preserved as 1.2.3, got %q", remoteDep.Version)
|
||||
}
|
||||
|
||||
// The original Chart.lock on disk must be untouched.
|
||||
originalLock, err := os.ReadFile(filepath.Join(tempDir, "Chart.lock"))
|
||||
if err != nil {
|
||||
t.Fatalf("reading original Chart.lock: %v", err)
|
||||
}
|
||||
if string(originalLock) != chartLock {
|
||||
t.Errorf("original Chart.lock was modified; expected unchanged content")
|
||||
}
|
||||
}
|
||||
|
||||
// TestRewriteChartDependencies_RefreshesChartLockWithExtraFields verifies that
|
||||
// Chart.lock digest recomputation includes all dependency fields (alias, condition,
|
||||
// tags, import-values, enabled) — not just name/repository/version — so the digest
|
||||
// stays compatible with Helm's resolver.HashReq for charts using those fields.
|
||||
// It proves field coverage by running two chart variants under a shared root
|
||||
// (so file:// paths resolve to the same absolute location) and asserting the
|
||||
// digests differ only due to extra fields.
|
||||
func TestRewriteChartDependencies_RefreshesChartLockWithExtraFields(t *testing.T) {
|
||||
// Use a shared root so both chart variants resolve file://../local-dep to the
|
||||
// same absolute path — isolating the digest difference to field content only.
|
||||
sharedRoot := t.TempDir()
|
||||
chartDir := filepath.Join(sharedRoot, "parent")
|
||||
if err := os.MkdirAll(chartDir, 0755); err != nil {
|
||||
t.Fatalf("creating chart dir: %v", err)
|
||||
}
|
||||
|
||||
// Run rewriteChartDependencies for a given Chart.yaml and return the recomputed digest.
|
||||
getDigest := func(t *testing.T, chartYaml, chartLock string) string {
|
||||
t.Helper()
|
||||
if err := os.WriteFile(filepath.Join(chartDir, "Chart.yaml"), []byte(chartYaml), 0644); err != nil {
|
||||
t.Fatalf("writing Chart.yaml: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(chartDir, "Chart.lock"), []byte(chartLock), 0644); err != nil {
|
||||
t.Fatalf("writing Chart.lock: %v", err)
|
||||
}
|
||||
logger := zap.NewNop().Sugar()
|
||||
st := &HelmState{
|
||||
basePath: chartDir,
|
||||
fs: filesystem.DefaultFileSystem(),
|
||||
logger: logger,
|
||||
}
|
||||
rewrittenPath, cleanup, err := st.rewriteChartDependencies(chartDir)
|
||||
if err != nil {
|
||||
t.Fatalf("rewriteChartDependencies failed: %v", err)
|
||||
}
|
||||
defer cleanup()
|
||||
lockData, err := os.ReadFile(filepath.Join(rewrittenPath, "Chart.lock"))
|
||||
if err != nil {
|
||||
t.Fatalf("reading rewritten Chart.lock: %v", err)
|
||||
}
|
||||
var lock struct {
|
||||
Digest string `yaml:"digest"`
|
||||
}
|
||||
if err := yaml.Unmarshal(lockData, &lock); err != nil {
|
||||
t.Fatalf("parsing rewritten Chart.lock: %v", err)
|
||||
}
|
||||
return lock.Digest
|
||||
}
|
||||
|
||||
const originalDigest = "sha256:0000000000000000000000000000000000000000000000000000000000000000"
|
||||
baseLock := `dependencies:
|
||||
- name: local-dep
|
||||
repository: file://../local-dep
|
||||
version: 1.0.0
|
||||
alias: my-local
|
||||
- name: local-dep
|
||||
repository: file://../local-dep-alt
|
||||
version: 2.0.0
|
||||
alias: my-local-alt
|
||||
digest: ` + originalDigest + `
|
||||
generated: "2024-01-01T00:00:00Z"
|
||||
`
|
||||
|
||||
// Chart.yaml with extra fields (alias, condition, tags, import-values).
|
||||
chartYamlWithExtras := `apiVersion: v2
|
||||
name: parent-chart
|
||||
version: 1.0.0
|
||||
dependencies:
|
||||
- name: local-dep
|
||||
repository: file://../local-dep
|
||||
version: 1.0.0
|
||||
alias: my-local
|
||||
condition: local-dep.enabled
|
||||
tags:
|
||||
- frontend
|
||||
- optional
|
||||
import-values:
|
||||
- child: config
|
||||
parent: global.config
|
||||
- name: local-dep
|
||||
repository: file://../local-dep-alt
|
||||
version: 2.0.0
|
||||
alias: my-local-alt
|
||||
`
|
||||
|
||||
// Same chart without condition/tags/import-values — only alias remains.
|
||||
chartYamlWithoutExtras := `apiVersion: v2
|
||||
name: parent-chart
|
||||
version: 1.0.0
|
||||
dependencies:
|
||||
- name: local-dep
|
||||
repository: file://../local-dep
|
||||
version: 1.0.0
|
||||
alias: my-local
|
||||
- name: local-dep
|
||||
repository: file://../local-dep-alt
|
||||
version: 2.0.0
|
||||
alias: my-local-alt
|
||||
`
|
||||
|
||||
digestWith := getDigest(t, chartYamlWithExtras, baseLock)
|
||||
digestWithout := getDigest(t, chartYamlWithoutExtras, baseLock)
|
||||
|
||||
if !strings.HasPrefix(digestWith, "sha256:") {
|
||||
t.Errorf("expected sha256 digest, got %q", digestWith)
|
||||
}
|
||||
if digestWith == originalDigest {
|
||||
t.Errorf("expected digest to be recomputed; still %q", digestWith)
|
||||
}
|
||||
if digestWith == digestWithout {
|
||||
t.Errorf("digest should differ when extra fields (condition, tags, import-values) are present, but both are %q", digestWith)
|
||||
}
|
||||
|
||||
// Also verify alias-based matching: both deps have name "local-dep" but
|
||||
// different aliases; both should get their file:// paths rewritten.
|
||||
if err := os.WriteFile(filepath.Join(chartDir, "Chart.yaml"), []byte(chartYamlWithExtras), 0644); err != nil {
|
||||
t.Fatalf("writing Chart.yaml: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(chartDir, "Chart.lock"), []byte(baseLock), 0644); err != nil {
|
||||
t.Fatalf("writing Chart.lock: %v", err)
|
||||
}
|
||||
logger := zap.NewNop().Sugar()
|
||||
st := &HelmState{
|
||||
basePath: chartDir,
|
||||
fs: filesystem.DefaultFileSystem(),
|
||||
logger: logger,
|
||||
}
|
||||
rewrittenPath, cleanup, err := st.rewriteChartDependencies(chartDir)
|
||||
if err != nil {
|
||||
t.Fatalf("rewriteChartDependencies failed: %v", err)
|
||||
}
|
||||
defer cleanup()
|
||||
|
||||
lockData, err := os.ReadFile(filepath.Join(rewrittenPath, "Chart.lock"))
|
||||
if err != nil {
|
||||
t.Fatalf("reading rewritten Chart.lock: %v", err)
|
||||
}
|
||||
var lock struct {
|
||||
Dependencies []struct {
|
||||
Name string `yaml:"name"`
|
||||
Repository string `yaml:"repository"`
|
||||
Version string `yaml:"version"`
|
||||
Alias string `yaml:"alias"`
|
||||
} `yaml:"dependencies"`
|
||||
}
|
||||
if err := yaml.Unmarshal(lockData, &lock); err != nil {
|
||||
t.Fatalf("parsing rewritten Chart.lock: %v", err)
|
||||
}
|
||||
if len(lock.Dependencies) != 2 {
|
||||
t.Fatalf("expected 2 lock dependencies, got %d", len(lock.Dependencies))
|
||||
}
|
||||
|
||||
dep1 := lock.Dependencies[0]
|
||||
if dep1.Alias != "my-local" {
|
||||
t.Errorf("expected first lock dep alias 'my-local', got %q", dep1.Alias)
|
||||
}
|
||||
if !filepath.IsAbs(strings.TrimPrefix(dep1.Repository, "file://")) {
|
||||
t.Errorf("expected first dep repository to be an absolute file:// path, got %q", dep1.Repository)
|
||||
}
|
||||
|
||||
dep2 := lock.Dependencies[1]
|
||||
if dep2.Alias != "my-local-alt" {
|
||||
t.Errorf("expected second lock dep alias 'my-local-alt', got %q", dep2.Alias)
|
||||
}
|
||||
if !filepath.IsAbs(strings.TrimPrefix(dep2.Repository, "file://")) {
|
||||
t.Errorf("expected second dep repository to be an absolute file:// path, got %q", dep2.Repository)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRewriteChartDependencies_GoYamlV2ImportValues verifies that Chart.lock
|
||||
// refresh works under go-yaml v2 (HELMFILE_GO_YAML_V3=false), where nested
|
||||
// maps in import-values decode as map[interface{}]interface{} which json.Marshal
|
||||
// cannot handle without normalization.
|
||||
func TestRewriteChartDependencies_GoYamlV2ImportValues(t *testing.T) {
|
||||
prev := runtime.GoYamlV3
|
||||
runtime.GoYamlV3 = false
|
||||
t.Cleanup(func() {
|
||||
runtime.GoYamlV3 = prev
|
||||
})
|
||||
|
||||
tempDir := t.TempDir()
|
||||
|
||||
chartYaml := `apiVersion: v2
|
||||
name: parent-chart
|
||||
version: 1.0.0
|
||||
dependencies:
|
||||
- name: local-dep
|
||||
repository: file://../local-dep
|
||||
version: 1.0.0
|
||||
import-values:
|
||||
- child: config
|
||||
parent: global.config
|
||||
`
|
||||
chartLock := `dependencies:
|
||||
- name: local-dep
|
||||
repository: file://../local-dep
|
||||
version: 1.0.0
|
||||
import-values:
|
||||
- child: config
|
||||
parent: global.config
|
||||
digest: sha256:0000000000000000000000000000000000000000000000000000000000000000
|
||||
generated: "2024-01-01T00:00:00Z"
|
||||
`
|
||||
|
||||
if err := os.WriteFile(filepath.Join(tempDir, "Chart.yaml"), []byte(chartYaml), 0644); err != nil {
|
||||
t.Fatalf("writing Chart.yaml: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(tempDir, "Chart.lock"), []byte(chartLock), 0644); err != nil {
|
||||
t.Fatalf("writing Chart.lock: %v", err)
|
||||
}
|
||||
|
||||
logger := zap.NewNop().Sugar()
|
||||
st := &HelmState{
|
||||
basePath: tempDir,
|
||||
fs: filesystem.DefaultFileSystem(),
|
||||
logger: logger,
|
||||
}
|
||||
|
||||
rewrittenPath, cleanup, err := st.rewriteChartDependencies(tempDir)
|
||||
if err != nil {
|
||||
t.Fatalf("rewriteChartDependencies failed: %v", err)
|
||||
}
|
||||
defer cleanup()
|
||||
|
||||
lockData, err := os.ReadFile(filepath.Join(rewrittenPath, "Chart.lock"))
|
||||
if err != nil {
|
||||
t.Fatalf("reading rewritten Chart.lock: %v", err)
|
||||
}
|
||||
|
||||
var lock struct {
|
||||
Digest string `yaml:"digest"`
|
||||
}
|
||||
if err := yaml.Unmarshal(lockData, &lock); err != nil {
|
||||
t.Fatalf("parsing rewritten Chart.lock: %v", err)
|
||||
}
|
||||
|
||||
if !strings.HasPrefix(lock.Digest, "sha256:") {
|
||||
t.Errorf("expected sha256 digest, got %q", lock.Digest)
|
||||
}
|
||||
const originalDigest = "sha256:0000000000000000000000000000000000000000000000000000000000000000"
|
||||
if lock.Digest == originalDigest {
|
||||
t.Errorf("expected digest to be recomputed; still %q", lock.Digest)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRewriteChartDependencies_DigestMatchesHelmHashReq verifies the recomputed
|
||||
// digest matches what Helm's resolver.HashReq would produce for a known input.
|
||||
// This guards against producing a digest that is "different" but still rejected
|
||||
// by `helm dependency build`.
|
||||
func TestRewriteChartDependencies_DigestMatchesHelmHashReq(t *testing.T) {
|
||||
tempDir := t.TempDir()
|
||||
|
||||
chartYaml := `apiVersion: v2
|
||||
name: test-chart
|
||||
version: 1.0.0
|
||||
dependencies:
|
||||
- name: dep-a
|
||||
repository: file://../dep-a
|
||||
version: 2.0.0
|
||||
condition: dep-a.enabled
|
||||
tags:
|
||||
- backend
|
||||
`
|
||||
chartLock := `dependencies:
|
||||
- name: dep-a
|
||||
repository: file://../dep-a
|
||||
version: 2.0.0
|
||||
digest: sha256:0000000000000000000000000000000000000000000000000000000000000000
|
||||
generated: "2024-01-01T00:00:00Z"
|
||||
`
|
||||
|
||||
if err := os.WriteFile(filepath.Join(tempDir, "Chart.yaml"), []byte(chartYaml), 0644); err != nil {
|
||||
t.Fatalf("writing Chart.yaml: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(tempDir, "Chart.lock"), []byte(chartLock), 0644); err != nil {
|
||||
t.Fatalf("writing Chart.lock: %v", err)
|
||||
}
|
||||
|
||||
logger := zap.NewNop().Sugar()
|
||||
st := &HelmState{
|
||||
basePath: tempDir,
|
||||
fs: filesystem.DefaultFileSystem(),
|
||||
logger: logger,
|
||||
}
|
||||
|
||||
rewrittenPath, cleanup, err := st.rewriteChartDependencies(tempDir)
|
||||
if err != nil {
|
||||
t.Fatalf("rewriteChartDependencies failed: %v", err)
|
||||
}
|
||||
defer cleanup()
|
||||
|
||||
lockData, err := os.ReadFile(filepath.Join(rewrittenPath, "Chart.lock"))
|
||||
if err != nil {
|
||||
t.Fatalf("reading rewritten Chart.lock: %v", err)
|
||||
}
|
||||
|
||||
var lock struct {
|
||||
Dependencies []*helmchart.Dependency `yaml:"dependencies"`
|
||||
Digest string `yaml:"digest"`
|
||||
}
|
||||
if err := yaml.Unmarshal(lockData, &lock); err != nil {
|
||||
t.Fatalf("parsing rewritten Chart.lock: %v", err)
|
||||
}
|
||||
|
||||
// Compute the expected digest independently using Helm's HashReq algorithm:
|
||||
// sha256(json.Marshal([2][]*chart.Dependency{req, lock}))
|
||||
// where req = rewritten Chart.yaml deps, lock = rewritten Chart.lock deps.
|
||||
absDepA, err := filepath.Abs(filepath.Join(tempDir, "../dep-a"))
|
||||
if err != nil {
|
||||
t.Fatalf("resolving absolute path: %v", err)
|
||||
}
|
||||
|
||||
req := []*helmchart.Dependency{
|
||||
{
|
||||
Name: "dep-a",
|
||||
Repository: "file://" + absDepA,
|
||||
Version: "2.0.0",
|
||||
Condition: "dep-a.enabled",
|
||||
Tags: []string{"backend"},
|
||||
},
|
||||
}
|
||||
lockDeps := []*helmchart.Dependency{
|
||||
{
|
||||
Name: "dep-a",
|
||||
Repository: "file://" + absDepA,
|
||||
Version: "2.0.0",
|
||||
},
|
||||
}
|
||||
|
||||
payload, err := json.Marshal([2][]*helmchart.Dependency{req, lockDeps})
|
||||
if err != nil {
|
||||
t.Fatalf("marshaling expected digest payload: %v", err)
|
||||
}
|
||||
sum := sha256.Sum256(payload)
|
||||
expectedDigest := "sha256:" + hex.EncodeToString(sum[:])
|
||||
|
||||
if lock.Digest != expectedDigest {
|
||||
t.Errorf("digest mismatch with Helm's HashReq algorithm:\n got: %s\n want: %s", lock.Digest, expectedDigest)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -4,7 +4,9 @@ import (
|
||||
"bytes"
|
||||
gocontext "context"
|
||||
"crypto/sha1"
|
||||
"crypto/sha256"
|
||||
"encoding/hex"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
@@ -27,6 +29,7 @@ import (
|
||||
"github.com/helmfile/vals"
|
||||
"github.com/tatsushid/go-prettytable"
|
||||
"go.uber.org/zap"
|
||||
helmchart "helm.sh/helm/v3/pkg/chart"
|
||||
cliv3 "helm.sh/helm/v3/pkg/cli"
|
||||
cliv4 "helm.sh/helm/v4/pkg/cli"
|
||||
|
||||
@@ -36,6 +39,7 @@ import (
|
||||
"github.com/helmfile/helmfile/pkg/event"
|
||||
"github.com/helmfile/helmfile/pkg/filesystem"
|
||||
"github.com/helmfile/helmfile/pkg/helmexec"
|
||||
"github.com/helmfile/helmfile/pkg/maputil"
|
||||
"github.com/helmfile/helmfile/pkg/remote"
|
||||
"github.com/helmfile/helmfile/pkg/tmpl"
|
||||
"github.com/helmfile/helmfile/pkg/yaml"
|
||||
@@ -1527,6 +1531,121 @@ func (st *HelmState) rewriteChartDependencies(chartPath string) (string, func(),
|
||||
|
||||
st.logger.Debugf("Rewrote Chart.yaml with absolute dependency paths at %s", tempChartYamlPath)
|
||||
|
||||
// Rewriting Chart.yaml invalidates Chart.lock's digest, since helm computes the
|
||||
// digest over the JSON-marshaled dependencies block. If the lock isn't refreshed,
|
||||
// downstream `helm dependency build` errors with "lock file is out of sync with
|
||||
// the dependencies file" and falls back to `dependency update`, which re-resolves
|
||||
// version constraints (e.g. `version: "*"`) against the chart repo and silently
|
||||
// pulls newer dependency versions. The version pins in the lock are still the
|
||||
// intended truth — only the rewritten file:// repository URL changed. Mirror the
|
||||
// rewrite into the lock and recompute the digest so `dep build` accepts it.
|
||||
tempChartLockPath := filepath.Join(tempDir, "Chart.lock")
|
||||
lockData, lockErr := st.fs.ReadFile(tempChartLockPath)
|
||||
if lockErr != nil && !os.IsNotExist(lockErr) {
|
||||
st.logger.Warnf("Failed to read Chart.lock at %s: %v", tempChartLockPath, lockErr)
|
||||
}
|
||||
if lockErr == nil {
|
||||
var lock struct {
|
||||
Dependencies []*helmchart.Dependency `yaml:"dependencies,omitempty"`
|
||||
Digest string `yaml:"digest,omitempty"`
|
||||
Generated string `yaml:"generated,omitempty"`
|
||||
}
|
||||
if err := yaml.Unmarshal(lockData, &lock); err != nil {
|
||||
st.logger.Warnf("Failed to parse Chart.lock at %s: %v", tempChartLockPath, err)
|
||||
} else {
|
||||
// Build the request slice (rewritten Chart.yaml dependencies) using helm's
|
||||
// own chart.Dependency type so the JSON used for hashing matches helm's
|
||||
// exactly. All supported fields must be mapped, not just name/repository/
|
||||
// version, because helm's digest algorithm hashes the full Dependency struct.
|
||||
req := make([]*helmchart.Dependency, 0, len(chartMeta.Dependencies))
|
||||
for _, d := range chartMeta.Dependencies {
|
||||
dep := &helmchart.Dependency{
|
||||
Name: d.Name,
|
||||
Repository: d.Repository,
|
||||
}
|
||||
if v, ok := d.Data["version"].(string); ok {
|
||||
dep.Version = v
|
||||
}
|
||||
if v, ok := d.Data["condition"].(string); ok {
|
||||
dep.Condition = v
|
||||
}
|
||||
if v, ok := d.Data["alias"].(string); ok {
|
||||
dep.Alias = v
|
||||
}
|
||||
if v, ok := d.Data["enabled"].(bool); ok {
|
||||
dep.Enabled = v
|
||||
}
|
||||
if v, ok := d.Data["tags"].([]interface{}); ok {
|
||||
tags := make([]string, 0, len(v))
|
||||
for _, t := range v {
|
||||
if s, ok := t.(string); ok {
|
||||
tags = append(tags, s)
|
||||
}
|
||||
}
|
||||
dep.Tags = tags
|
||||
}
|
||||
if v, ok := d.Data["import-values"].([]interface{}); ok {
|
||||
normalized, err := maputil.RecursivelyStringifyMapKey(v)
|
||||
if err != nil {
|
||||
st.logger.Warnf("Failed to normalize import-values for dependency %s: %v", d.Name, err)
|
||||
} else {
|
||||
dep.ImportValues = normalized.([]interface{})
|
||||
}
|
||||
}
|
||||
req = append(req, dep)
|
||||
}
|
||||
|
||||
// Mirror the rewritten file:// repository URLs onto matching lock entries.
|
||||
// Without this, `helm dependency build` would resolve the lock's relative
|
||||
// file:// paths against the (moved) chart directory and fail with
|
||||
// "directory ... not found". Versions in the lock are left untouched.
|
||||
// Match on Name + Alias to handle charts with duplicate dependency names
|
||||
// distinguished by alias.
|
||||
for _, ld := range lock.Dependencies {
|
||||
if !strings.HasPrefix(ld.Repository, "file://") {
|
||||
continue
|
||||
}
|
||||
for _, rd := range req {
|
||||
if rd.Name == ld.Name && rd.Alias == ld.Alias && strings.HasPrefix(rd.Repository, "file://") {
|
||||
ld.Repository = rd.Repository
|
||||
break
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Normalize lock.Dependencies ImportValues to avoid json.Marshal failures
|
||||
// when go-yaml v2 decodes nested maps as map[interface{}]interface{}.
|
||||
for _, ld := range lock.Dependencies {
|
||||
if ld.ImportValues != nil {
|
||||
normalized, err := maputil.RecursivelyStringifyMapKey(ld.ImportValues)
|
||||
if err != nil {
|
||||
st.logger.Warnf("Failed to normalize import-values in Chart.lock for dependency %s: %v", ld.Name, err)
|
||||
} else {
|
||||
ld.ImportValues = normalized.([]interface{})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Replicates helm's resolver.HashReq:
|
||||
// json.Marshal([2][]*chart.Dependency{req, lock}) → sha256 hex.
|
||||
// resolver.HashReq lives in helm.sh/helm/v3/internal/resolver, so we
|
||||
// inline the (small, stable) algorithm rather than importing it.
|
||||
if payload, err := json.Marshal([2][]*helmchart.Dependency{req, lock.Dependencies}); err != nil {
|
||||
st.logger.Warnf("Failed to marshal deps for Chart.lock digest at %s: %v", tempChartLockPath, err)
|
||||
} else {
|
||||
sum := sha256.Sum256(payload)
|
||||
lock.Digest = "sha256:" + hex.EncodeToString(sum[:])
|
||||
if updated, err := yaml.Marshal(&lock); err != nil {
|
||||
st.logger.Warnf("Failed to marshal Chart.lock at %s: %v", tempChartLockPath, err)
|
||||
} else if err := st.fs.WriteFile(tempChartLockPath, updated, 0644); err != nil {
|
||||
st.logger.Warnf("Failed to write Chart.lock at %s: %v", tempChartLockPath, err)
|
||||
} else {
|
||||
st.logger.Debugf("Refreshed Chart.lock digest at %s after Chart.yaml rewrite", tempChartLockPath)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
cleanup := func() {
|
||||
if removeErr := st.fs.RemoveAll(tempDir); removeErr != nil {
|
||||
st.logger.Warnf("Failed to remove temp chart directory %s: %v", tempDir, removeErr)
|
||||
|
||||
Reference in New Issue
Block a user