From 158b3e9da38f664dc1c728e4bc73d306468a282a Mon Sep 17 00:00:00 2001 From: Ethan Shold Date: Mon, 15 Jun 2026 17:21:45 -0500 Subject: [PATCH 1/4] test(paritycompare): neutralize empty-map and trailing-newline parity false-diffs Both fix sweep-only false diffs in the compare engine's loadFile, not drydock render: - cleanupEmptyMetaMaps: drydock correctly emits argocd.argoproj.io/tracking-id (real Argo CD does too); the raw oracle lacks it; the ignore-rule strips it leaving an empty {} that falsely diffs against the oracle's absent map. ~700 false diffs. - trimStringScalars: the oracle's yq re-serialization appends a trailing newline to block-scalar string values that drydock's faithful helm-Go-engine value lacks. --- internal/paritycompare/compare.go | 47 ++++++++++++++++++++++++ internal/paritycompare/compare_test.go | 51 ++++++++++++++++++++++++++ 2 files changed, 98 insertions(+) diff --git a/internal/paritycompare/compare.go b/internal/paritycompare/compare.go index 8f794e55..b7f0928b 100644 --- a/internal/paritycompare/compare.go +++ b/internal/paritycompare/compare.go @@ -183,6 +183,8 @@ func loadFile(path string, ignores []string) (appResources, error) { return nil, fmt.Errorf("%s %s: %w", path, manifest.IdentityOf(obj), err) } } + cleanupEmptyMetaMaps(obj.Object) + _ = trimStringScalars(obj.Object) key := manifest.IdentityOf(obj).String() body, err := canonicalBody(obj) if err != nil { @@ -193,6 +195,51 @@ func loadFile(path string, ignores []string) (appResources, error) { return out, nil } +// cleanupEmptyMetaMaps removes metadata.annotations and metadata.labels when nil or empty. +// After the ignore rules strip the only entry (e.g. argocd.argoproj.io/tracking-id) an empty +// {} remains and falsely diffs against a side that never had the map. +func cleanupEmptyMetaMaps(root map[string]any) { + meta, ok := root["metadata"].(map[string]any) + if !ok { + return + } + for _, field := range []string{"annotations", "labels"} { + value, present := meta[field] + if !present { + continue + } + if value == nil { + delete(meta, field) + continue + } + if m, ok := value.(map[string]any); ok && len(m) == 0 { + delete(meta, field) + } + } +} + +// trimStringScalars right-trims trailing newlines from every string leaf. The oracle pipes helm +// output through yq, whose block-scalar re-serialization appends a trailing \n that drydock's +// faithful helm-Go-engine value lacks (cert-manager .data.config.yaml, velero .data.global). +func trimStringScalars(value any) any { + switch typed := value.(type) { + case map[string]any: + for k, v := range typed { + typed[k] = trimStringScalars(v) + } + return typed + case []any: + for i, v := range typed { + typed[i] = trimStringScalars(v) + } + return typed + case string: + return strings.TrimRight(typed, "\n") + default: + return value + } +} + func canonicalBody(obj *unstructured.Unstructured) (string, error) { data, err := json.MarshalIndent(obj.Object, "", " ") if err != nil { diff --git a/internal/paritycompare/compare_test.go b/internal/paritycompare/compare_test.go index ee26ef54..b2be4741 100644 --- a/internal/paritycompare/compare_test.go +++ b/internal/paritycompare/compare_test.go @@ -101,6 +101,57 @@ func TestCompareRejectsInvalidJSONPointerEscape(t *testing.T) { } } +func TestCompareStripsEmptyMetaMapsAfterIgnore(t *testing.T) { + root := t.TempDir() + argocdDir := filepath.Join(root, "argocd") + drydockDir := filepath.Join(root, "drydock") + ignoreFile := filepath.Join(root, "ignore.yaml") + writeFile(t, filepath.Join(argocdDir, "demo.yaml"), configMapWithTracking("demo", "argocd")) + writeFile(t, filepath.Join(drydockDir, "demo.yaml"), "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: demo\n annotations: {}\n labels: {}\ndata:\n value: same\n") + writeFile(t, ignoreFile, "jsonPointers:\n - /metadata/annotations/argocd.argoproj.io~1tracking-id\n") + result, err := Compare(Options{ArgoCDDir: argocdDir, DrydockDir: drydockDir, OutDir: filepath.Join(root, "out"), IgnoreFile: ignoreFile}) + if err != nil { + t.Fatalf("Compare() error = %v", err) + } + if result.Differences != 0 { + t.Fatalf("Compare() differences = %d, want 0", result.Differences) + } +} + +func TestCompareStripsNullMetaMaps(t *testing.T) { + root := t.TempDir() + argocdDir := filepath.Join(root, "argocd") + drydockDir := filepath.Join(root, "drydock") + writeFile(t, filepath.Join(argocdDir, "demo.yaml"), "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: demo\n annotations:\n labels:\ndata:\n value: same\n") + writeFile(t, filepath.Join(drydockDir, "demo.yaml"), configMap("demo", "same")) + result, err := Compare(Options{ArgoCDDir: argocdDir, DrydockDir: drydockDir, OutDir: filepath.Join(root, "out")}) + if err != nil { + t.Fatalf("Compare() error = %v", err) + } + if result.Differences != 0 { + t.Fatalf("Compare() differences = %d, want 0", result.Differences) + } +} + +func TestCompareIgnoresTrailingNewlineOnStringScalars(t *testing.T) { + root := t.TempDir() + argocdDir := filepath.Join(root, "argocd") + drydockDir := filepath.Join(root, "drydock") + // oracle side: yq re-serialization adds trailing \n (block scalar with |) + oracleYAML := "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: demo\ndata:\n config.yaml: |\n key: value\n" + // drydock side: helm-Go-engine faithful value lacks trailing \n (block scalar with |-) + drydockYAML := "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: demo\ndata:\n config.yaml: |-\n key: value\n" + writeFile(t, filepath.Join(argocdDir, "demo.yaml"), oracleYAML) + writeFile(t, filepath.Join(drydockDir, "demo.yaml"), drydockYAML) + result, err := Compare(Options{ArgoCDDir: argocdDir, DrydockDir: drydockDir, OutDir: filepath.Join(root, "out")}) + if err != nil { + t.Fatalf("Compare() error = %v", err) + } + if result.Differences != 0 { + t.Fatalf("Compare() differences = %d, want 0", result.Differences) + } +} + func writeFile(t *testing.T, path, body string) { t.Helper() if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { From eb3d81ef55b1a734478dd7dd643d8060496895ff Mon Sep 17 00:00:00 2001 From: Ethan Shold Date: Mon, 15 Jun 2026 17:23:00 -0500 Subject: [PATCH 2/4] test(parity-sweep): drop --discover-kustomize now that default discovery resolves argocd/ home-ops PR #3229 added explicit metadata.namespace: argocd to the apps/argocd Application/ApplicationSet/AppProject manifests, so drydock default discovery resolves apps as argocd/ without rendering the apps/argocd kustomization (which inflated the argo-cd Helm chart on every run). --- scripts/home-ops-live-parity.sh | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/scripts/home-ops-live-parity.sh b/scripts/home-ops-live-parity.sh index 860e57e1..de8c713d 100755 --- a/scripts/home-ops-live-parity.sh +++ b/scripts/home-ops-live-parity.sh @@ -38,10 +38,11 @@ KUBE_CONTEXT="${KUBE_CONTEXT:-default}" ARGOCD_NS="${ARGOCD_NS:-argocd}" HOME_OPS_ROOT="${HOME_OPS_ROOT:-${HOME}/git/home-ops}" HOME_OPS_REPO_URL="${HOME_OPS_REPO_URL:-https://github.com/sholdee/home-ops}" -# Render the apps/argocd kustomization to discover the k3s-apps ApplicationSet (git -# directory generator over apps/*) and literal Applications, so drydock finds every -# app in namespace "argocd" exactly as the live cluster does. -DISCOVER_KUSTOMIZE="${DISCOVER_KUSTOMIZE:-apps/argocd}" +# Default app discovery resolves apps as argocd/ WITHOUT --discover-kustomize because +# home-ops PR #3229 added explicit metadata.namespace: argocd to every Application/ +# ApplicationSet/AppProject. This holds only when the rendered SYNCED_REV includes #3229 +# (the live cluster must be synced at/after #3229 / current master); otherwise apps +# under-resolve and surface as drydock capture failures (not silent wrong results). # Cluster capabilities to match live Argo CD's capability-gated renders (ServiceMonitors, # etc.). DRYDOCK_KUBE_VERSION = cluster server version (e.g. 1.35.5); DRYDOCK_API_VERSIONS_FILE # = file with one group/version or group/version/Kind per line, from @@ -217,10 +218,11 @@ for app in "${APPS[@]}"; do continue fi # drydock render of the same Application, at the synced revision, Secret-free. + # NOTE: --repo-map points the home-ops repoURL at the local detached worktree; mapping a + # repoURL to a local worktree bypasses the git/remote caches and degrades render-cache coverage. if ! "${DRYDOCK_BIN}" build app "argocd/${app}" \ --path "${WT}" \ --repo-map "${HOME_OPS_REPO_URL}=${WT}" \ - --discover-kustomize "${DISCOVER_KUSTOMIZE}" \ --skip-secrets \ "${CAP_ARGS[@]}" \ --git-cache-dir "${CACHE}/git" --chart-cache-dir "${CACHE}/charts" \ From b7b624a46dac81eda9ab16952ba5dda697075096 Mon Sep 17 00:00:00 2001 From: Ethan Shold Date: Mon, 15 Jun 2026 19:10:59 -0500 Subject: [PATCH 3/4] fix(render): preserve null chart defaults so helm-guarded keys match Argo CD Replace CoalesceValues (via ToRenderValuesWithSchemaValidation) with MergeValues + ValidateAgainstSchema + inline top map. CoalesceValues strips null chart defaults via cleanNilValues, causing hasKey guards to evaluate false under drydock while helm v3 (Argo CD's bundled CLI) retains them. Verified against helm v4.2.1 coalesce.go: the two paths differ only in nil handling; subchart/global/schema coalescing is identical. --- internal/render/helm.go | 30 +++++++++++++++++++++++------- internal/render/helm_test.go | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/internal/render/helm.go b/internal/render/helm.go index e433b2e9..961ff0f0 100644 --- a/internal/render/helm.go +++ b/internal/render/helm.go @@ -94,16 +94,32 @@ func (HelmRenderer) Render(ctx context.Context, source ResolvedSource, opts Rend return nil, nil, err } - values, err := chartutil.ToRenderValuesWithSchemaValidation(chart, inputValues, common.ReleaseOptions{ - Name: releaseName, - Namespace: opts.Namespace, - Revision: 1, - IsInstall: true, - IsUpgrade: false, - }, capabilities, opts.SkipSchemaValidation) + chartAccessor, err := helmchart.NewAccessor(chart) if err != nil { return nil, nil, fmt.Errorf("helm render values %s: %w", manifestPath, err) } + // MergeValues (nil-preserving) instead of ToRenderValuesWithSchemaValidation's CoalesceValues, + // which strips null chart defaults. Helm v3 (Argo CD's bundled CLI) retains them so guarded + // keys ({{- if hasKey .Values "x" }}) render under Argo CD but vanish under helm v4. Verified + // vs helm v4.2.1 coalesce.go: the two differ ONLY in nil handling. + mergedValues, err := chartutil.MergeValues(chart, inputValues) + if err != nil { + return nil, nil, fmt.Errorf("helm render values %s: %w", manifestPath, err) + } + if !opts.SkipSchemaValidation { + if err := chartutil.ValidateAgainstSchema(chart, mergedValues); err != nil { + return nil, nil, fmt.Errorf("helm render values %s: values don't meet the chart schema(s): %w", manifestPath, err) + } + } + values := map[string]any{ + "Chart": chartAccessor.MetadataAsMap(), + "Capabilities": capabilities, + "Release": map[string]any{ + "Name": releaseName, "Namespace": opts.Namespace, "IsUpgrade": false, + "IsInstall": true, "Revision": 1, "Service": "Helm", + }, + "Values": mergedValues, + } rendered, err := engine.Render(chart, values) if err != nil { diff --git a/internal/render/helm_test.go b/internal/render/helm_test.go index c360c315..bce7fb7d 100644 --- a/internal/render/helm_test.go +++ b/internal/render/helm_test.go @@ -213,6 +213,38 @@ data: } } +func TestHelmRendererPreservesNullChartDefaultsBehindHasKeyGuard(t *testing.T) { + root := t.TempDir() + writeFile(t, filepath.Join(root, "simple", "Chart.yaml"), "apiVersion: v2\nname: simple\nversion: 0.1.0\n") + writeFile(t, filepath.Join(root, "simple", "values.yaml"), "debugVerbose:\n") + writeFile(t, filepath.Join(root, "simple", "templates", "configmap.yaml"), + "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: {{ .Release.Name }}-config\n namespace: {{ .Release.Namespace }}\ndata:\n{{- if hasKey .Values \"debugVerbose\" }}\n debug-verbose: \"{{ .Values.debugVerbose }}\"\n{{- end }}\n") + result, diags, err := (HelmRenderer{}).Render(context.Background(), + ResolvedSource{RepoRoot: root, Path: "simple", Chart: "simple"}, + RenderOptions{AppName: "demo", Namespace: "demo-ns"}) + if err != nil { + t.Fatalf("Render() error = %v", err) + } + if len(diags) != 0 { + t.Fatalf("diagnostics = %#v", diags) + } + if len(result) != 1 { + t.Fatalf("len(result) = %d, want 1", len(result)) + } + // Cilium's pattern is `key: "{{ .Values.x }}"` (literal quotes): with a null chart default + // the key renders as an empty STRING (matching Argo CD's helm v3), not YAML null. + val, found, err := unstructured.NestedString(result[0].Object.Object, "data", "debug-verbose") + if err != nil { + t.Fatalf("data.debug-verbose accessor error: %v", err) + } + if !found { + t.Fatalf("data.debug-verbose missing: helm stripped the null chart default behind the hasKey guard") + } + if val != "" { + t.Fatalf("data.debug-verbose = %q, want empty string", val) + } +} + func TestHelmRendererSplitsCompactTemplateSeparators(t *testing.T) { for _, tt := range []struct { name string From d8a54052ea7925aa08e972ca0a87c36c32882b3b Mon Sep 17 00:00:00 2001 From: Ethan Shold Date: Mon, 15 Jun 2026 19:21:19 -0500 Subject: [PATCH 4/4] test(parity): add argocd-parity smoke fixture for null chart default preservation --- scripts/argocd-parity-smoke.sh | 1 + .../repo/applications/helm-null-default.yaml | 16 ++++++++++++++++ .../repo/charts/null-default/Chart.yaml | 6 ++++++ .../charts/null-default/templates/configmap.yaml | 14 ++++++++++++++ .../repo/charts/null-default/values.yaml | 7 +++++++ 5 files changed, 44 insertions(+) create mode 100644 testdata/argocd-parity/repo/applications/helm-null-default.yaml create mode 100644 testdata/argocd-parity/repo/charts/null-default/Chart.yaml create mode 100644 testdata/argocd-parity/repo/charts/null-default/templates/configmap.yaml create mode 100644 testdata/argocd-parity/repo/charts/null-default/values.yaml diff --git a/scripts/argocd-parity-smoke.sh b/scripts/argocd-parity-smoke.sh index 7eb25efd..20148079 100755 --- a/scripts/argocd-parity-smoke.sh +++ b/scripts/argocd-parity-smoke.sh @@ -72,6 +72,7 @@ APPLICATIONS=( parity-ft-alpha parity-ft-beta parity-fn-gamma-one + parity-helm-null-default ) TRACKING_APPLICATIONS=( diff --git a/testdata/argocd-parity/repo/applications/helm-null-default.yaml b/testdata/argocd-parity/repo/applications/helm-null-default.yaml new file mode 100644 index 00000000..0a878a5d --- /dev/null +++ b/testdata/argocd-parity/repo/applications/helm-null-default.yaml @@ -0,0 +1,16 @@ +apiVersion: argoproj.io/v1alpha1 +kind: Application +metadata: + name: parity-helm-null-default + namespace: argocd +spec: + project: default + source: + repoURL: git://argocd-parity-git.argocd-parity.svc.cluster.local/repo.git + targetRevision: HEAD + path: charts/null-default + helm: + releaseName: parity-helm-null-default + destination: + name: in-cluster + namespace: parity-helm-null-default diff --git a/testdata/argocd-parity/repo/charts/null-default/Chart.yaml b/testdata/argocd-parity/repo/charts/null-default/Chart.yaml new file mode 100644 index 00000000..2a7faf67 --- /dev/null +++ b/testdata/argocd-parity/repo/charts/null-default/Chart.yaml @@ -0,0 +1,6 @@ +apiVersion: v2 +name: null-default +description: Local Helm chart for Argo CD parity null chart default preservation. +type: application +version: 0.1.0 +appVersion: "1.0.0" diff --git a/testdata/argocd-parity/repo/charts/null-default/templates/configmap.yaml b/testdata/argocd-parity/repo/charts/null-default/templates/configmap.yaml new file mode 100644 index 00000000..2256c056 --- /dev/null +++ b/testdata/argocd-parity/repo/charts/null-default/templates/configmap.yaml @@ -0,0 +1,14 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: {{ .Release.Name }} + namespace: {{ .Release.Namespace }} +data: + {{- if hasKey .Values "debugVerbose" }} + # Cilium-faithful pattern: literal surrounding quotes render a null chart + # default as an empty STRING (not YAML null). Argo CD helm v3 keeps null + # defaults in .Values (hasKey → true, value renders as ""); helm v4 strips + # them (hasKey → false, key absent). This asserts parity with the Argo CD + # helm v3 behaviour. Revisit when Argo CD's repo-server moves to helm v4. + debug-verbose: "{{ .Values.debugVerbose }}" + {{- end }} diff --git a/testdata/argocd-parity/repo/charts/null-default/values.yaml b/testdata/argocd-parity/repo/charts/null-default/values.yaml new file mode 100644 index 00000000..24fd7b95 --- /dev/null +++ b/testdata/argocd-parity/repo/charts/null-default/values.yaml @@ -0,0 +1,7 @@ +# debugVerbose is intentionally null (bare key with no value). +# Argo CD's helm v3 (repo-server) preserves this null in .Values, so hasKey +# returns true and the key renders as an empty string via the literal-quote +# pattern below. Helm v4 strips null chart defaults, so the hasKey guard +# would return false and the key would be absent entirely. +# Revisit / retire this fixture when Argo CD's repo-server moves to helm v4. +debugVerbose: