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 { 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 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/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" \ 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: