mirror of
https://github.com/helmfile/helmfile.git
synced 2026-10-22 12:05:19 +02:00
fix: propagate --oci-plain-http to chartify for all chart-preparing commands (#2826)
* fix: propagate --oci-plain-http to chartify for all chart-preparing commands The global --oci-plain-http flag was only wired into ChartPrepareOptions for the template command. For apply, sync, diff, lint, unittest, fetch (and destroy/test, which share the same chartify path), the flag never reached chartify, so its internal 'helm dependency up' against OCI chart dependencies was issued without --plain-http and failed against plain-HTTP OCI registries (the per-repository plainHttp field only covers the helm pull of the release chart, not chartify's dependency update). Add HelmOCIPlainHTTP: a.HelmOCIPlainHTTP to the ChartPrepareOptions of these commands, mirroring Template(), and add a regression test that fetches a release with an adhoc OCI dependency from a plain-HTTP local registry. Fixes #2824 Signed-off-by: yxxhero <yunxuan8603@gmail.com> Signed-off-by: yxxhero <aiopsclub@163.com> * test: use lint (not fetch) to exercise chartify OCI plain-http path The pull/fetch command skips chartify (except for local dirs without Chart.yaml) and runs helmexec.BuildDeps instead, which already handles --plain-http at the exec layer. lint goes through chartify like template/apply/sync/diff do, and needs no cluster, so it reproduces issue #2824: before the fix, chartify's 'helm dependency up' on the adhoc OCI dependency fails against the plain-HTTP registry on Helm 4. Signed-off-by: yxxhero <yunxuan8603@gmail.com> Signed-off-by: yxxhero <aiopsclub@163.com> * test: align lint snapshot goldens with actual helm output Fix the Helm 3 golden with the exact output captured from CI (helm lint prints the chartified chart path and extra blank lines). The Helm 4 golden follows the same shape plus the Pulled/Digest lines emitted by helm dependency up. Signed-off-by: yxxhero <yunxuan8603@gmail.com> Signed-off-by: yxxhero <aiopsclub@163.com> * test: add fetch regression case exercising chartify OCI plain-http path helmfile fetch only enters chartification when the chart is a local directory without Chart.yaml (NeedsChartifyForLocalDir) — a packaged or regular local chart goes through helm dependency build instead, which already handles --plain-http at the helmexec layer. The new case uses a manifest-only chart directory plus an adhoc OCI dependency, so chartify's own 'helm dependency up' must receive the propagated --oci-plain-http on Helm 4, directly protecting the Fetch() wiring added in this PR. To reference the case-local fixture, the snapshot harness now rewrites './chart/' in inputs to the chart directory shipped next to the case (mirroring the existing '../../charts/' rewrite). The fixture cannot live in the shared testdata/charts directory because every chart there is helm-packaged and pushed to the test registry. Addresses the review finding that only the Lint assignment was covered. Signed-off-by: yxxhero <aiopsclub@163.com> * test: align fetch goldens with actual helm dep build output For a chartified manifest-only chart, chartify's ReplaceWithRendered strips the Chart.yaml dependencies field and removes charts/ after rendering the adhoc dependency into templates/. The follow-up `helm dependency build` therefore finds no declared dependencies and returns silently (helm3/helm4 Manager short-circuit), so the captured output is just the "Building dependency" log line — confirmed by the Helm 3 CI diff. Helm 4.3.0's Manager has the identical early-return path, so its golden matches. The regression signal remains the command's exit code: without the propagated --oci-plain-http, chartify's `helm dependency up` fails on Helm 4 and the whole fetch errors out. Signed-off-by: yxxhero <aiopsclub@163.com> --------- Signed-off-by: yxxhero <yunxuan8603@gmail.com> Signed-off-by: yxxhero <aiopsclub@163.com>
This commit is contained in:
yxxhero
@@ -192,6 +192,7 @@ func (a *App) Diff(c DiffConfigProvider) error {
|
||||
Validate: c.Validate(),
|
||||
Concurrency: c.Concurrency(),
|
||||
IncludeTransitiveNeeds: c.IncludeNeeds(),
|
||||
HelmOCIPlainHTTP: a.HelmOCIPlainHTTP,
|
||||
PrefetchSharedRemoteCharts: true,
|
||||
}, func() []error {
|
||||
msg, matched, affected, errs = a.diff(run, c)
|
||||
@@ -350,6 +351,7 @@ func (a *App) Lint(c LintConfigProvider) error {
|
||||
SkipCleanup: c.SkipCleanup(),
|
||||
Concurrency: c.Concurrency(),
|
||||
IncludeTransitiveNeeds: c.IncludeNeeds(),
|
||||
HelmOCIPlainHTTP: a.HelmOCIPlainHTTP,
|
||||
}, func() []error {
|
||||
ok, lintErrs, errs = a.lint(run, c)
|
||||
return append(errs, lintErrs...)
|
||||
@@ -393,6 +395,7 @@ func (a *App) Unittest(c UnittestConfigProvider) error {
|
||||
SkipCleanup: c.SkipCleanup(),
|
||||
Concurrency: c.Concurrency(),
|
||||
IncludeTransitiveNeeds: c.IncludeTransitiveNeeds(),
|
||||
HelmOCIPlainHTTP: a.HelmOCIPlainHTTP,
|
||||
}, func() []error {
|
||||
ok, unittestErrs, errs = a.unittest(run, c)
|
||||
return append(errs, unittestErrs...)
|
||||
@@ -473,6 +476,7 @@ func (a *App) Fetch(c FetchConfigProvider) error {
|
||||
OutputDir: c.OutputDir(),
|
||||
OutputDirTemplate: c.OutputDirTemplate(),
|
||||
Concurrency: c.Concurrency(),
|
||||
HelmOCIPlainHTTP: a.HelmOCIPlainHTTP,
|
||||
}, func() []error {
|
||||
if c.WriteOutput() {
|
||||
for i := range run.state.Releases {
|
||||
@@ -533,6 +537,7 @@ func (a *App) Sync(c SyncConfigProvider) error {
|
||||
IncludeTransitiveNeeds: c.IncludeNeeds(),
|
||||
Validate: c.Validate(),
|
||||
Concurrency: c.Concurrency(),
|
||||
HelmOCIPlainHTTP: a.HelmOCIPlainHTTP,
|
||||
TemplateArgs: c.TemplateArgs(),
|
||||
PrefetchSharedRemoteCharts: true,
|
||||
}, func() []error {
|
||||
@@ -592,6 +597,7 @@ func (a *App) Apply(c ApplyConfigProvider) error {
|
||||
SkipCleanup: c.SkipCleanup(),
|
||||
Validate: c.Validate(),
|
||||
Concurrency: c.Concurrency(),
|
||||
HelmOCIPlainHTTP: a.HelmOCIPlainHTTP,
|
||||
IncludeTransitiveNeeds: c.IncludeNeeds(),
|
||||
TemplateArgs: c.TemplateArgs(),
|
||||
PrefetchSharedRemoteCharts: true,
|
||||
@@ -655,6 +661,7 @@ func (a *App) Destroy(c DestroyConfigProvider) error {
|
||||
AllowFailedReleases: c.AllowFailedReleases(),
|
||||
SkipDeps: c.SkipDeps(),
|
||||
Concurrency: c.Concurrency(),
|
||||
HelmOCIPlainHTTP: a.HelmOCIPlainHTTP,
|
||||
DeleteWait: c.DeleteWait(),
|
||||
DeleteTimeout: c.DeleteTimeout(),
|
||||
}, func() []error {
|
||||
@@ -685,6 +692,7 @@ func (a *App) Test(c TestConfigProvider) error {
|
||||
AllowFailedReleases: c.AllowFailedReleases(),
|
||||
SkipDeps: c.SkipDeps(),
|
||||
Concurrency: c.Concurrency(),
|
||||
HelmOCIPlainHTTP: a.HelmOCIPlainHTTP,
|
||||
}, func() []error {
|
||||
errs = a.test(run, c)
|
||||
return errs
|
||||
|
||||
@@ -8,3 +8,4 @@ The `testdata` directory is composed of:
|
||||
- `snapshot/$NAME/input.yaml`: The input helmfile config for the test case of `$NAME`
|
||||
- `snapshot/$NAME/output.yaml`: The expected output of the helmfile command
|
||||
- `snapshot/$NAME/config.yaml`: The snapshot test configuration file. See the `Config` struct defined in `snapshot_test.go` for more information
|
||||
- `snapshot/$NAME/chart`: Optional chart fixture local to the test case, referenced from the input as `./chart/`. Unlike `charts`, it is never packaged/pushed to the test registry, so it may hold directories without `Chart.yaml` (raw manifests that must go through chartify)
|
||||
|
||||
@@ -112,6 +112,18 @@ func prepareInputFile(t *testing.T, originalFile, tmpDir string, hostPort int, c
|
||||
// breaking relative paths like ../../charts/raw-0.1.0
|
||||
inputStr = strings.ReplaceAll(inputStr, "../../charts/", chartsDir+"/")
|
||||
|
||||
// Convert test-case-local chart references like `./chart/` to an absolute
|
||||
// path pointing at the `chart` directory shipped next to the case's input
|
||||
// file. This lets a case bundle chart fixtures that cannot live in the
|
||||
// shared ../../charts directory — e.g. a directory without Chart.yaml,
|
||||
// which must stay unpackaged because setupLocalDockerRegistry runs
|
||||
// `helm package` on every chart under testdata/charts.
|
||||
caseDir, err := filepath.Abs(filepath.Dir(originalFile))
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to determine test case directory for %s: %v", originalFile, err)
|
||||
}
|
||||
inputStr = strings.ReplaceAll(inputStr, "./chart/", filepath.Join(caseDir, "chart")+"/")
|
||||
|
||||
// Convert relative postrenderer paths to absolute paths for Helm 3 only
|
||||
// Helm 3 resolves postrenderer paths relative to the helmfile location.
|
||||
// When the input file is copied to a temp directory, relative paths break.
|
||||
|
||||
+6
@@ -0,0 +1,6 @@
|
||||
apiVersion: v1
|
||||
kind: ConfigMap
|
||||
metadata:
|
||||
name: foo
|
||||
data:
|
||||
foo: FOO
|
||||
+6
@@ -0,0 +1,6 @@
|
||||
localDockerRegistry:
|
||||
enabled: true
|
||||
# Port is not specified, will be dynamically allocated to avoid conflicts
|
||||
chartifyTempDir: temp2824fetch
|
||||
helmfileArgs:
|
||||
- fetch
|
||||
Vendored
+21
@@ -0,0 +1,21 @@
|
||||
# The primary chart is a local directory WITHOUT Chart.yaml. That is the only
|
||||
# chart shape for which `helmfile fetch` enters chartification
|
||||
# (NeedsChartifyForLocalDir in pkg/state). Chartify then materializes a
|
||||
# temporary chart and resolves the adhoc OCI dependency with its own
|
||||
# `helm dependency up`, which requires the propagated --oci-plain-http on
|
||||
# Helm 4. Fetching a packaged or regular local chart never chartifies, so this
|
||||
# case uses a manifest-only directory to protect the Fetch wiring.
|
||||
#
|
||||
# After chartify renders the adhoc dependency into the chart, it strips the
|
||||
# Chart.yaml dependencies field and removes charts/, so the follow-up
|
||||
# `helm dependency build` is a silent no-op (helm3/helm4 return early when
|
||||
# Chart.yaml declares no dependencies) — the golden is just its "Building
|
||||
# dependency" log line. The regression signal is the command itself failing
|
||||
# when chartify's `helm dependency up` lacks --plain-http on Helm 4.
|
||||
releases:
|
||||
- name: foo
|
||||
chart: ./chart/
|
||||
dependencies:
|
||||
- alias: dep
|
||||
chart: oci://localhost:$REGISTRY_PORT/myrepo/raw
|
||||
version: 0.1.0
|
||||
Vendored
+1
@@ -0,0 +1 @@
|
||||
Building dependency release=foo, chart=$WD/temp2824fetch/foo
|
||||
+1
@@ -0,0 +1 @@
|
||||
Building dependency release=foo, chart=$WD/temp2824fetch/foo
|
||||
+6
@@ -0,0 +1,6 @@
|
||||
localDockerRegistry:
|
||||
enabled: true
|
||||
# Port is not specified, will be dynamically allocated to avoid conflicts
|
||||
chartifyTempDir: temp2824
|
||||
helmfileArgs:
|
||||
- lint
|
||||
Vendored
+7
@@ -0,0 +1,7 @@
|
||||
releases:
|
||||
- name: foo
|
||||
chart: ../../charts/raw-0.1.0
|
||||
dependencies:
|
||||
- alias: dep
|
||||
chart: oci://localhost:$REGISTRY_PORT/myrepo/raw
|
||||
version: 0.1.0
|
||||
Vendored
+12
@@ -0,0 +1,12 @@
|
||||
Building dependency release=foo, chart=$WD/temp2824/foo
|
||||
Saving 1 charts
|
||||
Downloading raw from repo oci://localhost:$REGISTRY_PORT/myrepo
|
||||
Pulled: localhost:$REGISTRY_PORT/myrepo/raw:0.1.0
|
||||
Digest: sha256:$DIGEST
|
||||
Deleting outdated charts
|
||||
Linting release=foo, chart=$WD/temp2824/foo
|
||||
==> Linting __workingdir__/temp2824/foo
|
||||
[INFO] Chart.yaml: icon is recommended
|
||||
|
||||
1 chart(s) linted, 0 chart(s) failed
|
||||
|
||||
+10
@@ -0,0 +1,10 @@
|
||||
Building dependency release=foo, chart=$WD/temp2824/foo
|
||||
Saving 1 charts
|
||||
Downloading raw from repo oci://localhost:$REGISTRY_PORT/myrepo
|
||||
Deleting outdated charts
|
||||
Linting release=foo, chart=$WD/temp2824/foo
|
||||
==> Linting __workingdir__/temp2824/foo
|
||||
[INFO] Chart.yaml: icon is recommended
|
||||
|
||||
1 chart(s) linted, 0 chart(s) failed
|
||||
|
||||
Reference in New Issue
Block a user