Audit fixes: discovery guards + instance file anchoring (#63-#66)

templates/_functions.tpl

@@ -16,7 +16,10 @@ wholesale by helmfile.instance with the values-loader path, and the loader
 runs the progressive merge (SOPS decryption, .yaml.gotmpl rendering, and
 merging in declaration order) at release-evaluation time.
 
-Context: dict with "release", "instance", "templateDir", and "field".
+Context: dict with "release", "instance", "templateDir", "deploymentDir",
+and "field". Template-level entries anchor template-relative,
+instance-level entries anchor deployment-relative (mirroring how the
+respective values: files resolve).
 */ -}}
 {{- define "atlas.applyListOverride" -}}
   {{- $field       := .field }}
@@ -35,12 +38,21 @@ Context: dict with "release", "instance", "templateDir", and "field".
     {{- end }}
   {{- end }}
 
-  {{- /* 2. Append any instance-level overrides */ -}}
+  {{- /* 2. Append any instance-level overrides. String entries are file
+       references authored in deployment.yaml — anchor them relative to
+       the deployment dir (unanchored they would resolve against
+       helmfile's cache dir, effectively undefined in remote
+       consumption). Inline maps pass through untouched. */ -}}
   {{- if hasKey .instance $field }}
     {{- $toAdd := .instance | get $field list }}
     {{- if $toAdd }}
+      {{- $convertedAdd := include "convertPaths" (dict
+        "targetPath" .deploymentDir
+        "values"     (toJson $toAdd)
+        "field"      (printf "%s (instance-level)" $field)
+      ) | fromJson }}
       {{- $current := .release | get $field list }}
-      {{- $_ := set .release $field (concat $current $toAdd) }}
+      {{- $_ := set .release $field (concat $current $convertedAdd) }}
     {{- end }}
   {{- end }}
 {{- end }}

templates/helmfile.all.yaml.gotmpl

@@ -86,21 +86,45 @@
 {{- $clustersRaw := include "glob" $clustersPattern | fromJson }}
 
 {{- $clusters := dict }}
+{{- $leafNamesSeen := dict }}
 
 {{- range $clustersRaw }}
   {{- $clusterPath := . | trimPrefix (printf "%s/" $deploymentsDir) | dir }}
 
-  {{- if ne $clusterPath "" }}
+  {{- /* "" would be an empty match; "." is the global apps/ dir itself
+       (trimPrefix+dir collapse it to the current dir) — in a repo with
+       global deployments but no clusters yet it would otherwise register
+       as a pseudo-cluster and produce a nonsense render target. */ -}}
+  {{- if and (ne $clusterPath "") (ne $clusterPath ".") }}
     {{- $childPattern := printf "%s/%s/*/apps" $deploymentsDir $clusterPath }}
     {{- $hasChildren := include "glob" $childPattern | fromJson }}
     {{- $isLeaf := eq (len $hasChildren) 0 }}
+    {{- if $isLeaf }}
+      {{- /* Leaf-name bookkeeping runs BEFORE the stage-1 filter so a
+           filtered (per-app ArgoCD) render still validates the whole
+           repo — name collisions are a global problem and must not
+           hide behind atlas.filter.cluster. */ -}}
+      {{- $leafName := $clusterPath | base }}
+      {{- $seenAt := $leafNamesSeen | get $leafName list }}
+      {{- $_ := set $leafNamesSeen $leafName (append $seenAt $clusterPath) }}
+    {{- end }}
     {{- $clusterMatches := or (eq $filterCluster "") (eq $clusterPath $filterCluster) }}
     {{- if and $isLeaf $clusterMatches }}
       {{- $_ := set $clusters $clusterPath (list) }}
     {{- end }}
   {{- end }}
 {{- end }}
 
+{{- /* Leaf cluster names must be unique across the repo: clusterName (the
+     leaf) is used to name downstream resources (ArgoCD Applications etc.),
+     so two groups containing same-named clusters silently produce
+     colliding names. Fail with the offending paths instead. */ -}}
+{{- range $leafName, $paths := $leafNamesSeen }}
+  {{- if gt (len $paths) 1 }}
+    {{- fail (printf "cluster discovery: duplicate leaf cluster name %q used by: %s — leaf names must be unique across the repo (clusterName names downstream resources)" $leafName (join ", " $paths)) }}
+  {{- end }}
+{{- end }}
+
 # STEP 2: COLLECT DEPLOYMENTS FOR EACH CLUSTER
 # For each leaf cluster, gather deployments from:
 # 1. Global level (applied to all).
@@ -142,10 +166,15 @@
   {{- $_ := set $clusters $cluster $collectedDeployments -}}
 {{- end }}
 
+{{- /* Discovery dump: the full clusters/deployments dict as comment lines.
+     Real bytes in every rendered state file on large repos, so it is
+     opt-in for debugging only. */ -}}
+{{- if env "ATLAS_DEBUG_DISCOVERY" }}
 # All recognized deployments
 {{- range (toYaml $clusters | splitList "\n") }}
   # {{ . }}
 {{- end }}
+{{- end }}
 
 # OUTPUT
 # Generate YAML helmfiles list, each pointing to the single deployment renderer,

templates/helmfile.instance.yaml.gotmpl

@@ -262,10 +262,11 @@ Purpose:
        resolves entries itself at release-evaluation time. */ -}}
   {{- range $listField := list "strategicMergePatches" "jsonPatches" "transformers" }}
     {{- include "atlas.applyListOverride" (dict
-        "release"     $release
-        "instance"    $thisInstance
-        "templateDir" $templateDir
-        "field"       $listField
+        "release"       $release
+        "instance"      $thisInstance
+        "templateDir"   $templateDir
+        "deploymentDir" (dir $.Values.atlas.deployment.deploymentPath)
+        "field"         $listField
     ) }}
   {{- end }}
 

tests/bats/integration/discovery-guards.bats

@@ -0,0 +1,75 @@
+#!/usr/bin/env bats
+#
+# Scenario: cluster-discovery guard rails (issues #63, #64, #65, #66).
+#
+# Uses dedicated fixture roots via atlas.deploymentDefinitions overrides —
+# same isolation approach as error-paths.bats.
+
+_root() { cd "${BATS_TEST_DIRNAME}/../../.." && pwd; }
+
+_render_fixtures() {
+  local fixtures="$1" root; shift
+  root="$(_root)"
+  helmfile -f "$root/tests/helmfile.yaml.gotmpl" \
+    --state-values-set "atlas.deploymentDefinitions=${fixtures}" \
+    template --skip-schema-validation "$@" 2>&1
+}
+
+# --- duplicate leaf cluster names (issue #64) ---------------------------------
+
+@test "dup leaf names: render fails" {
+  run _render_fixtures fixtures-dup-leaf
+  [ "$status" -ne 0 ]
+}
+
+@test "dup leaf names: error lists the name and both paths" {
+  run _render_fixtures fixtures-dup-leaf
+  echo "$output" | grep -q 'duplicate leaf cluster name "dupe"'
+  echo "$output" | grep -q "group-a/dupe"
+  echo "$output" | grep -q "group-b/dupe"
+}
+
+@test "dup leaf names: stage-1 filter does not hide the collision" {
+  ATLAS_FILTER_CLUSTER=group-a/dupe run _render_fixtures fixtures-dup-leaf
+  [ "$status" -ne 0 ]
+  echo "$output" | grep -q 'duplicate leaf cluster name "dupe"'
+}
+
+# --- zero-cluster repo: no pseudo-cluster "." (issue #65) ---------------------
+
+@test "global-only repo: no pseudo-cluster render target" {
+  run _render_fixtures fixtures-global-only
+  # Correct outcome is zero render targets (helmfile: no releases found),
+  # NOT a render for pseudo-cluster "." (the pre-fix behavior).
+  echo "$output" | grep -q "no releases found"
+  ! echo "$output" | grep -q "cluster: \.$"
+}
+
+# --- discovery debug dump is opt-in (issue #66) -------------------------------
+
+@test "discovery dump: absent by default" {
+  root="$(_root)"
+  run env ATLAS_FILTER_CLUSTER=cluster1 ATLAS_FILTER_DEPLOYMENT_NAME=deployment1 \
+    helmfile build -f "$root/tests/helmfile.yaml.gotmpl" --debug
+  ! echo "$output" | grep -q "All recognized deployments"
+}
+
+@test "discovery dump: present with ATLAS_DEBUG_DISCOVERY" {
+  root="$(_root)"
+  run env ATLAS_DEBUG_DISCOVERY=1 ATLAS_FILTER_CLUSTER=cluster1 ATLAS_FILTER_DEPLOYMENT_NAME=deployment1 \
+    helmfile build -f "$root/tests/helmfile.yaml.gotmpl" --debug
+  echo "$output" | grep -q "All recognized deployments"
+}
+
+# --- instance-level patch file missing (issue #63, negative side) -------------
+
+@test "missing instance patch file: render fails with anchored path" {
+  root="$(_root)"
+  ATLAS_FILTER_CLUSTER=cluster1 ATLAS_FILTER_DEPLOYMENT_NAME=missing-instance-patch \
+  run helmfile -f "$root/tests/helmfile.yaml.gotmpl" \
+    --state-values-set "atlas.deploymentDefinitions=fixtures-negative" \
+    template --skip-schema-validation
+  [ "$status" -ne 0 ]
+  echo "$output" | grep -q "strategicMergePatches (instance-level): file not found"
+  echo "$output" | grep -q "missing-instance-patch/./does-not-exist.yaml"
+}

tests/bats/values/cluster1-deployment48-instance-file-patch.bats

@@ -0,0 +1,30 @@
+#!/usr/bin/env bats
+#
+# Scenario: instance-level strategicMergePatches FILE entry (issue #63).
+#
+# deployment48 references ./instance-patch.yaml next to its deployment.yaml.
+# applyListOverride must anchor the path deployment-relative (unanchored it
+# resolves against helmfile's cache dir and silently never applies) and the
+# patch annotation must land in the rendered ConfigMap.
+
+load '../helpers/render'
+
+CLUSTER=cluster1
+DEPLOYMENT=deployment48
+RELEASE=stage3-patches-release
+
+setup_file() { ensure_rendered; }
+
+@test "d48: release rendered" {
+  instance_rendered_any "$CLUSTER" "$DEPLOYMENT" "$RELEASE"
+}
+
+@test "d48: template-level strategicMergePatches still applied" {
+  run render_contains "$CLUSTER" "$DEPLOYMENT" "$RELEASE" "atlas-test/patched"
+  [ "$status" -eq 0 ]
+}
+
+@test "d48: instance-level file patch applied (deployment-relative anchor)" {
+  run render_contains "$CLUSTER" "$DEPLOYMENT" "$RELEASE" "atlas-test/from-instance-file"
+  [ "$status" -eq 0 ]
+}

tests/deployments/cluster1/apps/deployment48/deployment.yaml

@@ -0,0 +1,10 @@
+# Fixture: instance-level strategicMergePatches FILE entry (issue #63).
+# The path is deployment-relative — applyListOverride anchors it to this
+# directory before appending, so the patch file lives next to this
+# deployment.yaml (unanchored it would resolve against helmfile's cache
+# dir and never apply in remote consumption).
+apps:
+  - template: app-patches
+    namespace: test
+    strategicMergePatches:
+      - ./instance-patch.yaml

tests/deployments/cluster1/apps/deployment48/instance-patch.yaml

@@ -0,0 +1,7 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: stage3-patches-release-chart1
+  namespace: test
+  annotations:
+    atlas-test/from-instance-file: "yes"

tests/fixtures-dup-leaf/group-a/dupe/apps/app1/deployment.yaml

@@ -0,0 +1,5 @@
+# Fixture (issue #64): leaf cluster name "dupe" also exists in group-b —
+# discovery must fail listing both paths.
+apps:
+  - template: app-novals
+    namespace: test

tests/fixtures-dup-leaf/group-b/dupe/apps/app1/deployment.yaml

@@ -0,0 +1,5 @@
+# Fixture (issue #64): leaf cluster name "dupe" also exists in group-a —
+# discovery must fail listing both paths.
+apps:
+  - template: app-novals
+    namespace: test

tests/fixtures-global-only/apps/global-app/deployment.yaml

@@ -0,0 +1,7 @@
+# Fixture for the zero-cluster repo shape (issue #65): global deployments
+# exist but no cluster directories do. Discovery must not register the
+# global apps/ dir itself as pseudo-cluster "." — the correct result is
+# zero render targets, not a nonsense one.
+apps:
+  - template: app-novals
+    namespace: test

tests/fixtures-negative/cluster1/apps/missing-instance-patch/deployment.yaml

@@ -0,0 +1,7 @@
+# NEGATIVE fixture (issue #63): instance-level strategicMergePatches file
+# that does not exist — must fail loudly, not deploy unpatched.
+apps:
+  - template: app-novals
+    namespace: test
+    strategicMergePatches:
+      - ./does-not-exist.yaml