From 4a805931e3e244940c57a9a8a4ddda16d4ffaf29 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Fri, 17 Jul 2026 20:18:31 +0200 Subject: [PATCH 1/5] feat(workload): warn when a HelmRelease valuesFrom can't be resolved offline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit workload validate/scan render HelmRelease charts offline so the real applied manifests are checked. Values come from spec.values + valuesFrom ConfigMap/Secret refs resolved from the in-repo stream. A non-optional valuesFrom whose object is not in the stream (typically cluster-managed) was silently dropped: the chart still rendered, but with incomplete values, so the offline output could diverge from what Flux applies with no signal to the user. Surface this through the existing render-degradation channel as a new DegradationPartialValues kind with an accurate warning ("rendered ... with incomplete values ... valuesFrom X could not be resolved offline"), distinct from the "skipped Helm render" message (the chart did render). Optional references are Flux-tolerated and stay silent. Warning only — no change to pass/fail, exit codes, or rendered output. Applies to both validate and scan via the shared render path. Extract lookupValuesRef as the single resolvability check shared by applyValuesFrom and the new unresolvedValueRefs inspector. Tests cover all three states (non-optional-missing warns, optional-missing silent, resolvable silent) at both the render-package and end-to-end command levels. Fixes #6213 Part of #5344 Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/cli/cmd/workload/render.go | 9 +- pkg/cli/cmd/workload/validate_render_test.go | 63 ++++++++++++ pkg/svc/gitops/render/render.go | 66 +++++++++++- pkg/svc/gitops/render/render_test.go | 101 +++++++++++++++++++ pkg/svc/gitops/render/resolver.go | 33 ++++-- 5 files changed, 258 insertions(+), 14 deletions(-) diff --git a/pkg/cli/cmd/workload/render.go b/pkg/cli/cmd/workload/render.go index 57d86cbc68..8a6d436048 100644 --- a/pkg/cli/cmd/workload/render.go +++ b/pkg/cli/cmd/workload/render.go @@ -143,9 +143,16 @@ func warnDegradations(cmd *cobra.Command, degradations []render.Degradation) { continue } + content := "skipped Helm render for HelmRelease %s (validating the resource as-is): %s" + if degradation.Kind == render.DegradationPartialValues { + content = "rendered HelmRelease %s with incomplete values — valuesFrom %s " + + "could not be resolved offline (cluster-managed or in another kustomization); " + + "validation/scan may differ from the cluster" + } + notify.WriteMessage(notify.Message{ Type: notify.WarningType, - Content: "skipped Helm render for HelmRelease %s (validating the resource as-is): %s", + Content: content, Args: []any{degradation.HelmRelease, degradation.Reason}, Writer: cmd.ErrOrStderr(), }) diff --git a/pkg/cli/cmd/workload/validate_render_test.go b/pkg/cli/cmd/workload/validate_render_test.go index 9c7f2dc8c0..1d4e1ae394 100644 --- a/pkg/cli/cmd/workload/validate_render_test.go +++ b/pkg/cli/cmd/workload/validate_render_test.go @@ -151,6 +151,69 @@ func TestValidateDegradesWhenChartUnreachable(t *testing.T) { assert.Contains(t, output, "skipped Helm render", "degradation should warn the user") } +// TestValidateWarnsOnUnresolvableValuesFrom verifies the render-fidelity honesty +// contract end-to-end: a HelmRelease whose non-optional valuesFrom references a +// ConfigMap absent from the offline stream (typically cluster-managed) still +// renders and passes, but the user is warned that values were incomplete — not +// the misleading "skipped Helm render" message, since the chart DID render. +func TestValidateWarnsOnUnresolvableValuesFrom(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + chartURL := localChartURL(t, "validchart") + files := map[string]string{ + "kustomization.yaml": `apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: + - ocirepository.yaml + - helmrelease.yaml +`, + "ocirepository.yaml": `apiVersion: source.toolkit.fluxcd.io/v1 +kind: OCIRepository +metadata: + name: chart + namespace: flux-system +spec: + interval: 5m + url: ` + chartURL + ` +`, + "helmrelease.yaml": `apiVersion: helm.toolkit.fluxcd.io/v2 +kind: HelmRelease +metadata: + name: app + namespace: flux-system +spec: + interval: 5m + chartRef: + kind: OCIRepository + name: chart + valuesFrom: + - kind: ConfigMap + name: cluster-settings +`, + } + + for name, content := range files { + require.NoError(t, os.WriteFile(filepath.Join(dir, name), []byte(content), 0o600)) + } + + output, err := runValidate(t, dir, "--skip-kinds", "OCIRepository") + require.NoError(t, err, "an unresolvable valuesFrom must warn, not fail the run") + assert.Contains(t, output, "incomplete values", "the user should be warned values were partial") + assert.Contains( + t, + output, + "cluster-settings", + "the warning should name the unresolved reference", + ) + assert.NotContains( + t, + output, + "skipped Helm render", + "the chart did render, so the skipped-render message must not appear", + ) +} + // TestValidateSingleHelmReleaseFileStaysGreen guards the ksail-test-gen-smoke CI // canary: validating a single HelmRelease file (whose source is not present) // must not attempt rendering and must pass as a CR-schema check. diff --git a/pkg/svc/gitops/render/render.go b/pkg/svc/gitops/render/render.go index b2ac120e16..25d3e17443 100644 --- a/pkg/svc/gitops/render/render.go +++ b/pkg/svc/gitops/render/render.go @@ -5,6 +5,7 @@ import ( "context" "encoding/base64" "errors" + "fmt" "maps" "sort" "strings" @@ -51,12 +52,32 @@ type Document struct { Provenance Provenance } -// Degradation records a HelmRelease that could not be rendered offline. The -// caller falls back to validating/scanning the HelmRelease CR itself. Silent -// degradations are expected in normal repos (e.g. a source object that lives in -// a different Flux Kustomization) and should not be surfaced as warnings. +// DegradationKind classifies why a HelmRelease's offline render is less than +// full-fidelity, so callers can phrase an accurate warning. +type DegradationKind int + +const ( + // DegradationSkippedRender marks a HelmRelease whose chart could not be + // rendered offline at all; the caller falls back to validating/scanning the + // HelmRelease CR itself. This is the zero value, so a Degradation left + // without an explicit Kind keeps this meaning. + DegradationSkippedRender DegradationKind = iota + // DegradationPartialValues marks a HelmRelease that DID render, but with a + // non-optional valuesFrom reference that could not be resolved from the + // offline stream — so the rendered children may differ from what Flux applies + // with the real value present. + DegradationPartialValues +) + +// Degradation records a way a HelmRelease's offline render fell short of what +// Flux would apply. For DegradationSkippedRender the caller falls back to +// validating/scanning the HelmRelease CR itself; for DegradationPartialValues +// the children were rendered but with incomplete values. Silent degradations +// are expected in normal repos (e.g. a source object that lives in a different +// Flux Kustomization) and should not be surfaced as warnings. type Degradation struct { HelmRelease string // "namespace/name" + Kind DegradationKind Reason string Err error Silent bool @@ -192,6 +213,43 @@ func expandHelmRelease( newDocument(child, parseObjectMeta(child), provenance), ) } + + // The chart rendered, but a non-optional valuesFrom that could not be resolved + // offline means the children were produced with incomplete values — surface + // that so the shift-left gate is honest about its coverage. + result.Degradations = append(result.Degradations, unresolvedValueRefs(&helmRelease, index)...) +} + +// unresolvedValueRefs returns a DegradationPartialValues for each non-optional +// valuesFrom reference on the HelmRelease that cannot be resolved from the +// offline stream. Such a reference (typically a cluster-managed ConfigMap or +// Secret, or one defined in another Flux Kustomization) is dropped from the +// offline render, so the rendered output can diverge from what Flux applies with +// the real value present. Optional references are Flux-tolerated (helm-controller +// ignores a not-found optional ref) and produce no degradation. +func unresolvedValueRefs(helmRelease *helmv2.HelmRelease, sources SourceIndex) []Degradation { + key := helmRelease.Namespace + "/" + helmRelease.Name + + var degradations []Degradation + + for index := range helmRelease.Spec.ValuesFrom { + ref := helmRelease.Spec.ValuesFrom[index] + if ref.Optional { + continue + } + + if _, ok := lookupValuesRef(ref, helmRelease.Namespace, sources); ok { + continue + } + + degradations = append(degradations, Degradation{ + HelmRelease: key, + Kind: DegradationPartialValues, + Reason: fmt.Sprintf("%s %q (key %q)", ref.Kind, ref.Name, ref.GetValuesKey()), + }) + } + + return degradations } // isSilentDegradation reports whether a degradation is expected in normal repos diff --git a/pkg/svc/gitops/render/render_test.go b/pkg/svc/gitops/render/render_test.go index 92f9b3ddad..306df46b32 100644 --- a/pkg/svc/gitops/render/render_test.go +++ b/pkg/svc/gitops/render/render_test.go @@ -250,3 +250,104 @@ metadata: func joinDocsString(docs ...string) string { return strings.Join(docs, "\n---\n") } + +// helmReleaseValuesFromYAML references a ConfigMap "app-config" via valuesFrom. +// Whether that ConfigMap is in the stream determines if the offline render is +// full-fidelity. +const ( + helmReleaseValuesFromYAML = `apiVersion: helm.toolkit.fluxcd.io/v2 +kind: HelmRelease +metadata: + name: podinfo + namespace: flux-system +spec: + chartRef: + kind: OCIRepository + name: podinfo + valuesFrom: + - kind: ConfigMap + name: app-config` + + helmReleaseValuesFromOptionalYAML = helmReleaseValuesFromYAML + ` + optional: true` + + appConfigYAML = `apiVersion: v1 +kind: ConfigMap +metadata: + name: app-config + namespace: flux-system +data: + values.yaml: | + replicaCount: 3` +) + +// TestExpandDegradesOnUnresolvableValuesFrom pins the render-fidelity honesty +// contract: a HelmRelease whose non-optional valuesFrom ConfigMap/Secret is +// absent from the offline stream still renders, but records a +// DegradationPartialValues so the shift-left gate does not silently under-report +// its coverage. An optional reference, or a resolvable one, degrades nothing. +func TestExpandDegradesOnUnresolvableValuesFrom(t *testing.T) { + t.Parallel() + + resolver := fakeResolver{ + render: func(_ *helmv2.HelmRelease, _ render.SourceIndex) (string, error) { + return renderedDeployment, nil + }, + } + + tests := []struct { + name string + stream []byte + wantDegraded bool + }{ + { + name: "non-optional valuesFrom absent from the stream degrades", + stream: joinDocs(helmReleaseValuesFromYAML), + wantDegraded: true, + }, + { + name: "optional valuesFrom absent from the stream is tolerated", + stream: joinDocs(helmReleaseValuesFromOptionalYAML), + wantDegraded: false, + }, + { + name: "resolvable valuesFrom does not degrade", + stream: joinDocs(helmReleaseValuesFromYAML, appConfigYAML), + wantDegraded: false, + }, + } + + for _, testCase := range tests { + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + result, err := render.Expand( + context.Background(), + testCase.stream, + render.Options{Resolver: resolver}, + ) + require.NoError(t, err) + + // The chart renders regardless of values coverage. + _, hasDeployment := findDoc(result.Documents, "Deployment") + require.True(t, hasDeployment, "chart should render regardless of values coverage") + + if !testCase.wantDegraded { + assert.Empty(t, result.Degradations) + + return + } + + require.Len(t, result.Degradations, 1) + degradation := result.Degradations[0] + assert.Equal(t, "flux-system/podinfo", degradation.HelmRelease) + assert.Equal(t, render.DegradationPartialValues, degradation.Kind) + assert.False( + t, + degradation.Silent, + "an unresolvable non-optional valuesFrom should warn", + ) + assert.Contains(t, degradation.Reason, "app-config") + }) + } +} diff --git a/pkg/svc/gitops/render/resolver.go b/pkg/svc/gitops/render/resolver.go index 3bdf1cdc54..5bbfcead25 100644 --- a/pkg/svc/gitops/render/resolver.go +++ b/pkg/svc/gitops/render/resolver.go @@ -302,17 +302,16 @@ func buildValues(helmRelease *helmv2.HelmRelease, sources SourceIndex) (string, return string(raw), nil } -// applyValuesFrom merges one valuesFrom reference into values, resolving it from -// the in-repo ConfigMap/Secret index. References to objects not present in the -// stream (typically cluster-managed) are skipped. A reference with a targetPath -// injects its single flat value at that path (see applyTargetPathValue); -// otherwise the referenced value is YAML-merged at the root. -func applyValuesFrom( - values map[string]any, +// lookupValuesRef resolves a valuesFrom reference's raw value from the in-stream +// ConfigMap/Secret index, returning it and whether it was found. It is the single +// source of truth for "can this reference be resolved offline?", shared by +// applyValuesFrom (which merges the value) and unresolvedValueRefs (which reports +// when it cannot be resolved). +func lookupValuesRef( ref meta.ValuesReference, namespace string, sources SourceIndex, -) { +) (string, bool) { var data map[string]string switch ref.Kind { @@ -321,10 +320,26 @@ func applyValuesFrom( case "Secret": data = sources.Secrets[namespace+"/"+ref.Name] default: - return + return "", false } raw, ok := data[ref.GetValuesKey()] + + return raw, ok +} + +// applyValuesFrom merges one valuesFrom reference into values, resolving it from +// the in-repo ConfigMap/Secret index. References to objects not present in the +// stream (typically cluster-managed) are skipped. A reference with a targetPath +// injects its single flat value at that path (see applyTargetPathValue); +// otherwise the referenced value is YAML-merged at the root. +func applyValuesFrom( + values map[string]any, + ref meta.ValuesReference, + namespace string, + sources SourceIndex, +) { + raw, ok := lookupValuesRef(ref, namespace, sources) if !ok { return } From 90a526955b18106d54605885e6849f10a4db10e5 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Fri, 17 Jul 2026 20:50:56 +0200 Subject: [PATCH 2/5] test(render): extract valuesFrom degradation assertion to satisfy funlen Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/svc/gitops/render/render_test.go | 39 +++++++++++++++------------- 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/pkg/svc/gitops/render/render_test.go b/pkg/svc/gitops/render/render_test.go index 306df46b32..6624d0c993 100644 --- a/pkg/svc/gitops/render/render_test.go +++ b/pkg/svc/gitops/render/render_test.go @@ -327,27 +327,30 @@ func TestExpandDegradesOnUnresolvableValuesFrom(t *testing.T) { render.Options{Resolver: resolver}, ) require.NoError(t, err) + assertValuesFromDegradation(t, result, testCase.wantDegraded) + }) + } +} - // The chart renders regardless of values coverage. - _, hasDeployment := findDoc(result.Documents, "Deployment") - require.True(t, hasDeployment, "chart should render regardless of values coverage") +// assertValuesFromDegradation checks that the chart rendered and that a partial- +// values degradation is present exactly when wantDegraded is true. +func assertValuesFromDegradation(t *testing.T, result render.Result, wantDegraded bool) { + t.Helper() - if !testCase.wantDegraded { - assert.Empty(t, result.Degradations) + // The chart renders regardless of values coverage. + _, hasDeployment := findDoc(result.Documents, "Deployment") + require.True(t, hasDeployment, "chart should render regardless of values coverage") - return - } + if !wantDegraded { + assert.Empty(t, result.Degradations) - require.Len(t, result.Degradations, 1) - degradation := result.Degradations[0] - assert.Equal(t, "flux-system/podinfo", degradation.HelmRelease) - assert.Equal(t, render.DegradationPartialValues, degradation.Kind) - assert.False( - t, - degradation.Silent, - "an unresolvable non-optional valuesFrom should warn", - ) - assert.Contains(t, degradation.Reason, "app-config") - }) + return } + + require.Len(t, result.Degradations, 1) + degradation := result.Degradations[0] + assert.Equal(t, "flux-system/podinfo", degradation.HelmRelease) + assert.Equal(t, render.DegradationPartialValues, degradation.Kind) + assert.False(t, degradation.Silent, "an unresolvable non-optional valuesFrom should warn") + assert.Contains(t, degradation.Reason, "app-config") } From ea487ef60099aed5ff1a5cbf4158c80e83b50036 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Sat, 18 Jul 2026 09:31:15 +0200 Subject: [PATCH 3/5] fix(render): report optional valuesFrom whose referent lacks the values key Flux forgives only a not-found optional referent: per helm-controller's API, "a not found error for the values reference is ignored, but any ValuesKey, TargetPath or transient error will still result in a reconciliation failure". The previous unconditional skip of optional refs therefore hid a real failure - a valuesKey typo against a ConfigMap that IS in the stream rendered without those values and reported nothing. Split referent lookup from key lookup so an absent optional referent stays silent while a present one missing its key is always reported. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/svc/gitops/render/render.go | 52 +++++++++++++++++++--------- pkg/svc/gitops/render/render_test.go | 22 ++++++++++++ pkg/svc/gitops/render/resolver.go | 37 +++++++++++++++----- 3 files changed, 87 insertions(+), 24 deletions(-) diff --git a/pkg/svc/gitops/render/render.go b/pkg/svc/gitops/render/render.go index 25d3e17443..5a9622185d 100644 --- a/pkg/svc/gitops/render/render.go +++ b/pkg/svc/gitops/render/render.go @@ -12,6 +12,7 @@ import ( "github.com/devantler-tech/ksail/v7/pkg/fsutil" helmv2 "github.com/fluxcd/helm-controller/api/v2" + meta "github.com/fluxcd/pkg/apis/meta" sourcev1 "github.com/fluxcd/source-controller/api/v1" "sigs.k8s.io/yaml" ) @@ -220,13 +221,16 @@ func expandHelmRelease( result.Degradations = append(result.Degradations, unresolvedValueRefs(&helmRelease, index)...) } -// unresolvedValueRefs returns a DegradationPartialValues for each non-optional -// valuesFrom reference on the HelmRelease that cannot be resolved from the -// offline stream. Such a reference (typically a cluster-managed ConfigMap or -// Secret, or one defined in another Flux Kustomization) is dropped from the -// offline render, so the rendered output can diverge from what Flux applies with -// the real value present. Optional references are Flux-tolerated (helm-controller -// ignores a not-found optional ref) and produce no degradation. +// unresolvedValueRefs returns a DegradationPartialValues for each valuesFrom +// reference on the HelmRelease whose value the offline render could not obtain, +// so the rendered output can diverge from what Flux applies with the real value +// present. It mirrors helm-controller's own tolerance rule: "a not found error +// for the values reference is ignored [when optional], but any ValuesKey, +// TargetPath or transient error will still result in a reconciliation failure". +// So only an ABSENT optional referent is silent (typically cluster-managed, or +// defined in another Flux Kustomization); a referent that IS in the stream but +// lacks the requested valuesKey fails in-cluster even when marked optional — a +// `valuesKey` typo is exactly that case — and is always reported. func unresolvedValueRefs(helmRelease *helmv2.HelmRelease, sources SourceIndex) []Degradation { key := helmRelease.Namespace + "/" + helmRelease.Name @@ -234,24 +238,40 @@ func unresolvedValueRefs(helmRelease *helmv2.HelmRelease, sources SourceIndex) [ for index := range helmRelease.Spec.ValuesFrom { ref := helmRelease.Spec.ValuesFrom[index] - if ref.Optional { - continue - } - if _, ok := lookupValuesRef(ref, helmRelease.Namespace, sources); ok { + data, referentFound := lookupValuesSource(ref, helmRelease.Namespace, sources) + if !referentFound { + // Flux ignores a not-found optional referent; a missing required one + // means the chart rendered with incomplete values. + if ref.Optional { + continue + } + + degradations = append(degradations, partialValuesDegradation(key, ref)) + continue } - degradations = append(degradations, Degradation{ - HelmRelease: key, - Kind: DegradationPartialValues, - Reason: fmt.Sprintf("%s %q (key %q)", ref.Kind, ref.Name, ref.GetValuesKey()), - }) + // The referent is in the stream, so a missing valuesKey is a hard + // reconciliation failure in Flux regardless of `optional`. + if _, ok := data[ref.GetValuesKey()]; !ok { + degradations = append(degradations, partialValuesDegradation(key, ref)) + } } return degradations } +// partialValuesDegradation builds the DegradationPartialValues entry naming the +// valuesFrom reference whose value could not be obtained offline. +func partialValuesDegradation(key string, ref meta.ValuesReference) Degradation { + return Degradation{ + HelmRelease: key, + Kind: DegradationPartialValues, + Reason: fmt.Sprintf("%s %q (key %q)", ref.Kind, ref.Name, ref.GetValuesKey()), + } +} + // isSilentDegradation reports whether a degradation is expected in normal repos // and should not be surfaced as a user-facing warning. func isSilentDegradation(err error) bool { diff --git a/pkg/svc/gitops/render/render_test.go b/pkg/svc/gitops/render/render_test.go index 6624d0c993..a4d7264892 100644 --- a/pkg/svc/gitops/render/render_test.go +++ b/pkg/svc/gitops/render/render_test.go @@ -271,6 +271,15 @@ spec: helmReleaseValuesFromOptionalYAML = helmReleaseValuesFromYAML + ` optional: true` + // helmReleaseValuesFromOptionalMissingKeyYAML marks the reference optional but + // requests a valuesKey the referenced ConfigMap does not carry (a typo of + // "values.yaml"). helm-controller forgives only a NOT-FOUND optional referent — + // "any ValuesKey, TargetPath or transient error will still result in a + // reconciliation failure" — so this must still be reported. + helmReleaseValuesFromOptionalMissingKeyYAML = helmReleaseValuesFromYAML + ` + optional: true + valuesKey: valuse.yaml` + appConfigYAML = `apiVersion: v1 kind: ConfigMap metadata: @@ -310,6 +319,19 @@ func TestExpandDegradesOnUnresolvableValuesFrom(t *testing.T) { stream: joinDocs(helmReleaseValuesFromOptionalYAML), wantDegraded: false, }, + { + // Flux forgives only a not-found optional referent; a present one + // missing the requested valuesKey still fails reconciliation, so a + // valuesKey typo must not be silently swallowed by `optional: true`. + name: "optional valuesFrom present but missing its valuesKey degrades", + stream: joinDocs(helmReleaseValuesFromOptionalMissingKeyYAML, appConfigYAML), + wantDegraded: true, + }, + { + name: "optional valuesFrom resolvable from the stream does not degrade", + stream: joinDocs(helmReleaseValuesFromOptionalYAML, appConfigYAML), + wantDegraded: false, + }, { name: "resolvable valuesFrom does not degrade", stream: joinDocs(helmReleaseValuesFromYAML, appConfigYAML), diff --git a/pkg/svc/gitops/render/resolver.go b/pkg/svc/gitops/render/resolver.go index 5bbfcead25..1e600c8d50 100644 --- a/pkg/svc/gitops/render/resolver.go +++ b/pkg/svc/gitops/render/resolver.go @@ -312,14 +312,8 @@ func lookupValuesRef( namespace string, sources SourceIndex, ) (string, bool) { - var data map[string]string - - switch ref.Kind { - case "ConfigMap": - data = sources.ConfigMaps[namespace+"/"+ref.Name] - case "Secret": - data = sources.Secrets[namespace+"/"+ref.Name] - default: + data, found := lookupValuesSource(ref, namespace, sources) + if !found { return "", false } @@ -328,6 +322,33 @@ func lookupValuesRef( return raw, ok } +// lookupValuesSource resolves the referenced ConfigMap/Secret itself from the +// in-stream index, returning its data and whether the *referent* is present — +// distinct from whether the requested valuesKey exists inside it. Flux forgives +// only a not-found optional referent: per the helm-controller API, "a not found +// error for the values reference is ignored, but any ValuesKey, TargetPath or +// transient error will still result in a reconciliation failure". Reporting the +// two cases apart is what lets unresolvedValueRefs stay silent for a genuinely +// absent optional referent while still flagging a present one whose key is missing. +func lookupValuesSource( + ref meta.ValuesReference, + namespace string, + sources SourceIndex, +) (map[string]string, bool) { + switch ref.Kind { + case "ConfigMap": + data, ok := sources.ConfigMaps[namespace+"/"+ref.Name] + + return data, ok + case "Secret": + data, ok := sources.Secrets[namespace+"/"+ref.Name] + + return data, ok + default: + return nil, false + } +} + // applyValuesFrom merges one valuesFrom reference into values, resolving it from // the in-repo ConfigMap/Secret index. References to objects not present in the // stream (typically cluster-managed) are skipped. A reference with a targetPath From e2c0dff7ee49bfe69d42945023515786a48896ed Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Sat, 18 Jul 2026 09:37:55 +0200 Subject: [PATCH 4/5] fix(workload): name the real cause when a valuesFrom key is missing The missing-key case reused the absent-referent warning, which blames a cluster-managed or cross-kustomization source. That misdirects: the referent IS in the repo, so the actionable cause is the key itself (typically a valuesKey typo). Give it its own degradation kind and a warning that says so. Found by exercising the new warning as a user. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/cli/cmd/workload/render.go | 12 +++++++++++- pkg/svc/gitops/render/render.go | 25 +++++++++++++++++++------ pkg/svc/gitops/render/render_test.go | 18 ++++++++++++++---- 3 files changed, 44 insertions(+), 11 deletions(-) diff --git a/pkg/cli/cmd/workload/render.go b/pkg/cli/cmd/workload/render.go index 8a6d436048..15b9204251 100644 --- a/pkg/cli/cmd/workload/render.go +++ b/pkg/cli/cmd/workload/render.go @@ -144,10 +144,20 @@ func warnDegradations(cmd *cobra.Command, degradations []render.Degradation) { } content := "skipped Helm render for HelmRelease %s (validating the resource as-is): %s" - if degradation.Kind == render.DegradationPartialValues { + + switch degradation.Kind { + case render.DegradationPartialValues: content = "rendered HelmRelease %s with incomplete values — valuesFrom %s " + "could not be resolved offline (cluster-managed or in another kustomization); " + "validation/scan may differ from the cluster" + case render.DegradationMissingValuesKey: + // The referent IS in the repo, so pointing at cluster-managed sources + // would misdirect; the actionable cause is the key itself (often a typo). + content = "rendered HelmRelease %s with incomplete values — valuesFrom %s " + + "is in the repo but has no such key (Flux fails reconciliation on a " + + "missing valuesKey, even when the reference is optional); " + + "validation/scan may differ from the cluster" + case render.DegradationSkippedRender: } notify.WriteMessage(notify.Message{ diff --git a/pkg/svc/gitops/render/render.go b/pkg/svc/gitops/render/render.go index 5a9622185d..2e75e2ab69 100644 --- a/pkg/svc/gitops/render/render.go +++ b/pkg/svc/gitops/render/render.go @@ -68,6 +68,13 @@ const ( // offline stream — so the rendered children may differ from what Flux applies // with the real value present. DegradationPartialValues + // DegradationMissingValuesKey marks a HelmRelease that DID render, but whose + // valuesFrom referent IS present in the stream while lacking the requested + // valuesKey — typically a valuesKey typo. It is a distinct kind because the + // remedy differs: the resource is in the repo, so the caller must not suggest + // the value is cluster-managed. Flux fails reconciliation on a missing + // valuesKey even when the reference is marked optional. + DegradationMissingValuesKey ) // Degradation records a way a HelmRelease's offline render fell short of what @@ -247,7 +254,10 @@ func unresolvedValueRefs(helmRelease *helmv2.HelmRelease, sources SourceIndex) [ continue } - degradations = append(degradations, partialValuesDegradation(key, ref)) + degradations = append( + degradations, + valuesDegradation(key, ref, DegradationPartialValues), + ) continue } @@ -255,19 +265,22 @@ func unresolvedValueRefs(helmRelease *helmv2.HelmRelease, sources SourceIndex) [ // The referent is in the stream, so a missing valuesKey is a hard // reconciliation failure in Flux regardless of `optional`. if _, ok := data[ref.GetValuesKey()]; !ok { - degradations = append(degradations, partialValuesDegradation(key, ref)) + degradations = append( + degradations, + valuesDegradation(key, ref, DegradationMissingValuesKey), + ) } } return degradations } -// partialValuesDegradation builds the DegradationPartialValues entry naming the -// valuesFrom reference whose value could not be obtained offline. -func partialValuesDegradation(key string, ref meta.ValuesReference) Degradation { +// valuesDegradation builds the degradation entry naming the valuesFrom reference +// whose value could not be obtained offline, under the given kind. +func valuesDegradation(key string, ref meta.ValuesReference, kind DegradationKind) Degradation { return Degradation{ HelmRelease: key, - Kind: DegradationPartialValues, + Kind: kind, Reason: fmt.Sprintf("%s %q (key %q)", ref.Kind, ref.Name, ref.GetValuesKey()), } } diff --git a/pkg/svc/gitops/render/render_test.go b/pkg/svc/gitops/render/render_test.go index a4d7264892..2f65609fdf 100644 --- a/pkg/svc/gitops/render/render_test.go +++ b/pkg/svc/gitops/render/render_test.go @@ -308,11 +308,13 @@ func TestExpandDegradesOnUnresolvableValuesFrom(t *testing.T) { name string stream []byte wantDegraded bool + wantKind render.DegradationKind }{ { name: "non-optional valuesFrom absent from the stream degrades", stream: joinDocs(helmReleaseValuesFromYAML), wantDegraded: true, + wantKind: render.DegradationPartialValues, }, { name: "optional valuesFrom absent from the stream is tolerated", @@ -323,9 +325,12 @@ func TestExpandDegradesOnUnresolvableValuesFrom(t *testing.T) { // Flux forgives only a not-found optional referent; a present one // missing the requested valuesKey still fails reconciliation, so a // valuesKey typo must not be silently swallowed by `optional: true`. + // It reports as its own kind so the warning can name the real cause + // instead of blaming a cluster-managed source. name: "optional valuesFrom present but missing its valuesKey degrades", stream: joinDocs(helmReleaseValuesFromOptionalMissingKeyYAML, appConfigYAML), wantDegraded: true, + wantKind: render.DegradationMissingValuesKey, }, { name: "optional valuesFrom resolvable from the stream does not degrade", @@ -349,14 +354,19 @@ func TestExpandDegradesOnUnresolvableValuesFrom(t *testing.T) { render.Options{Resolver: resolver}, ) require.NoError(t, err) - assertValuesFromDegradation(t, result, testCase.wantDegraded) + assertValuesFromDegradation(t, result, testCase.wantDegraded, testCase.wantKind) }) } } // assertValuesFromDegradation checks that the chart rendered and that a partial- // values degradation is present exactly when wantDegraded is true. -func assertValuesFromDegradation(t *testing.T, result render.Result, wantDegraded bool) { +func assertValuesFromDegradation( + t *testing.T, + result render.Result, + wantDegraded bool, + wantKind render.DegradationKind, +) { t.Helper() // The chart renders regardless of values coverage. @@ -372,7 +382,7 @@ func assertValuesFromDegradation(t *testing.T, result render.Result, wantDegrade require.Len(t, result.Degradations, 1) degradation := result.Degradations[0] assert.Equal(t, "flux-system/podinfo", degradation.HelmRelease) - assert.Equal(t, render.DegradationPartialValues, degradation.Kind) - assert.False(t, degradation.Silent, "an unresolvable non-optional valuesFrom should warn") + assert.Equal(t, wantKind, degradation.Kind) + assert.False(t, degradation.Silent, "an unresolvable valuesFrom should warn") assert.Contains(t, degradation.Reason, "app-config") } From 5e66ab995391257a9b6fff0a879471da63fb94ff Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Sat, 18 Jul 2026 09:51:25 +0200 Subject: [PATCH 5/5] fix(render): treat a data-less valuesFrom referent as present extractStringData returned nil for a referent carrying no data, so indexValuesData skipped it and an empty-but-present ConfigMap/Secret was indistinguishable from an absent one - letting an optional reference suppress the degradation even though Flux forgives only a missing object. Return the empty map instead, reserving nil for a parse failure, so presence and absence stay distinguishable. Extends the degradation table with the empty-referent case (and extracts it to satisfy funlen). Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/svc/gitops/render/render.go | 9 ++-- pkg/svc/gitops/render/render_test.go | 71 ++++++++++++++++++---------- 2 files changed, 51 insertions(+), 29 deletions(-) diff --git a/pkg/svc/gitops/render/render.go b/pkg/svc/gitops/render/render.go index 2e75e2ab69..d70dcd08ef 100644 --- a/pkg/svc/gitops/render/render.go +++ b/pkg/svc/gitops/render/render.go @@ -461,9 +461,10 @@ func extractStringData(doc []byte, secret bool) map[string]string { maps.Copy(out, obj.StringData) - if len(out) == 0 { - return nil - } - + // An EMPTY (but non-nil) map is returned deliberately for a referent that + // carries no data: the object still exists in the stream, and callers must be + // able to tell "present but has no such key" (a Flux reconciliation failure, + // even for an optional reference) from "absent" (which optional forgives). + // Only a parse failure yields nil, meaning "not a usable referent at all". return out } diff --git a/pkg/svc/gitops/render/render_test.go b/pkg/svc/gitops/render/render_test.go index 2f65609fdf..583f66884b 100644 --- a/pkg/svc/gitops/render/render_test.go +++ b/pkg/svc/gitops/render/render_test.go @@ -288,28 +288,34 @@ metadata: data: values.yaml: | replicaCount: 3` -) -// TestExpandDegradesOnUnresolvableValuesFrom pins the render-fidelity honesty -// contract: a HelmRelease whose non-optional valuesFrom ConfigMap/Secret is -// absent from the offline stream still renders, but records a -// DegradationPartialValues so the shift-left gate does not silently under-report -// its coverage. An optional reference, or a resolvable one, degrades nothing. -func TestExpandDegradesOnUnresolvableValuesFrom(t *testing.T) { - t.Parallel() + // appConfigEmptyYAML is present in the stream but carries no data at all. It + // must still count as a PRESENT referent: Flux forgives only a missing object, + // so an optional reference to this one still fails reconciliation on the + // absent valuesKey. + appConfigEmptyYAML = `apiVersion: v1 +kind: ConfigMap +metadata: + name: app-config + namespace: flux-system` +) - resolver := fakeResolver{ - render: func(_ *helmv2.HelmRelease, _ render.SourceIndex) (string, error) { - return renderedDeployment, nil - }, - } +// valuesFromDegradationCase is one valuesFrom resolution scenario for +// TestExpandDegradesOnUnresolvableValuesFrom. +type valuesFromDegradationCase struct { + name string + stream []byte + wantDegraded bool + wantKind render.DegradationKind +} - tests := []struct { - name string - stream []byte - wantDegraded bool - wantKind render.DegradationKind - }{ +// valuesFromDegradationCases enumerates how each valuesFrom resolution outcome +// must be reported. It mirrors helm-controller's tolerance rule: `optional` +// forgives only a NOT-FOUND referent, so a referent that is present — whether it +// lacks the requested key or carries no data at all — still fails reconciliation +// and must be reported, under its own kind so the warning names the real cause. +func valuesFromDegradationCases() []valuesFromDegradationCase { + return []valuesFromDegradationCase{ { name: "non-optional valuesFrom absent from the stream degrades", stream: joinDocs(helmReleaseValuesFromYAML), @@ -322,16 +328,17 @@ func TestExpandDegradesOnUnresolvableValuesFrom(t *testing.T) { wantDegraded: false, }, { - // Flux forgives only a not-found optional referent; a present one - // missing the requested valuesKey still fails reconciliation, so a - // valuesKey typo must not be silently swallowed by `optional: true`. - // It reports as its own kind so the warning can name the real cause - // instead of blaming a cluster-managed source. name: "optional valuesFrom present but missing its valuesKey degrades", stream: joinDocs(helmReleaseValuesFromOptionalMissingKeyYAML, appConfigYAML), wantDegraded: true, wantKind: render.DegradationMissingValuesKey, }, + { + name: "optional valuesFrom present but empty degrades", + stream: joinDocs(helmReleaseValuesFromOptionalYAML, appConfigEmptyYAML), + wantDegraded: true, + wantKind: render.DegradationMissingValuesKey, + }, { name: "optional valuesFrom resolvable from the stream does not degrade", stream: joinDocs(helmReleaseValuesFromOptionalYAML, appConfigYAML), @@ -343,8 +350,22 @@ func TestExpandDegradesOnUnresolvableValuesFrom(t *testing.T) { wantDegraded: false, }, } +} + +// TestExpandDegradesOnUnresolvableValuesFrom pins the render-fidelity honesty +// contract: a HelmRelease whose valuesFrom ConfigMap/Secret cannot supply its +// value offline still renders, but records a degradation so the shift-left gate +// does not silently under-report its coverage. +func TestExpandDegradesOnUnresolvableValuesFrom(t *testing.T) { + t.Parallel() + + resolver := fakeResolver{ + render: func(_ *helmv2.HelmRelease, _ render.SourceIndex) (string, error) { + return renderedDeployment, nil + }, + } - for _, testCase := range tests { + for _, testCase := range valuesFromDegradationCases() { t.Run(testCase.name, func(t *testing.T) { t.Parallel()