fix(harbor-rd): carry the release prefix in the resource selectors - #4170

The fix in packages/system/harbor-rd/cozyrds/harbor.yaml is right, and the commit message states the reason well: the lineage webhook evaluates these selectors against a context carrying only {kind, name, namespace}, where name is the custom resource's name and not the Helm release name, so a selector that omits the harbor- prefix names an object that does not exist. Worth guarding too, for exactly the reason the second commit gives — the omission reports nothing, so it is indistinguishable from a selector that legitimately has nothing to match.

Four things on hack/check-rd-release-prefix.bats before this comes out of draft.

1. The test does not run under the repo's BATS runner

make bats-unit-tests executes every hack/*.bats through hack/cozytest.sh, a pure-POSIX-shell reimplementation of a BATS subset rather than the bats binary. It implements no run helper and sets no $status or $output. The result today:

$ hack/cozytest.sh hack/check-rd-release-prefix.bats
hack/cozytest.sh: 33: run: not found
❌ Test failed: every resourceNames selector carries its release prefix (exit 1)

This is the only hack/*.bats file using run. Eight existing files state the constraint in their own headers — hack/clickhouse-update-versions_test.bats puts it as "written in the portable subset hack/cozytest.sh understands (no run/$status/$lines, no setup/teardown, no standalone } inside a test body)", and hack/promote-retag_test.bats, hack/promote-rewrite-tags_test.bats, hack/common-envs_test.bats, hack/migration-56-adopt-path.bats, hack/kubernetes-nodes-unpin-hook.bats, hack/capture-dataplane.bats and hack/cilium-leak-healer_test.bats say the same. The house pattern is a plain $(...) capture asserted with [ ... ], relying on set -e.

2. Please drop the inlined Python for bash + yq

The heredoc'd python3 script also brings a dependency the project does not declare. make build-deps asserts find docker skopeo jq gh helm, mikefarah yq, GNU tar and GNU sed; python3 is not in that list, and the unit-test job in .github/workflows/pull-requests.yaml installs only the helm-unittest plugin. import yaml is a further step beyond that, since PyYAML is not in the standard library, so on a runner with python3 but no PyYAML this fails with a traceback attributed to a harbor selector.

For contrast, 36 of the 105 files in hack/ reach for yq, including the nearest neighbours — check-mongodb-rd-secrets.bats, check-redis-rd-secrets.bats, check-qdrant-rd-secrets.bats, check-nats-rd-secrets.bats, check-mariadb-rd-secrets.bats, check-rabbitmq-rd-secrets.bats — and check-applicationdefinition-crd-ordering.bats, which is structurally the same job as this one: a repo-wide invariant over a glob of YAML, done in yq plus jq with a vacuity guard. Only two files invoke python3 at all, and both do so because the thing under test is itself Python: md-no-hardwrap.bats exercises .claude/hooks/md-no-hardwrap.py and says so in a header that notes "other hack/*.bats deliberately avoid a python3 dependency"; talos-image-cache_test.bats compiles a generated serve.py. There is also hack/check-rd-presets.sh, which already walks packages/system/*-rd/cozyrds/*.yaml in bash with yq and jq and is wired into make unit-tests as rd-presets-check — the closest thing to a template for this.

The rewrite is short. This version runs clean on the tree as of this PR (84 selectors inspected across the 24 prefixed definitions) and reports all three harbor selectors when the fix is reverted:

@test "every resourceNames selector carries its release prefix" {
  command -v yq >/dev/null || { echo "yq (mikefarah v4+) is required" >&2; exit 1; }

  violations=""
  total=0
  for f in "$REPO_ROOT"/packages/system/*-rd/cozyrds/*.yaml; do
    prefix="$(yq -r '.spec.release.prefix // ""' "$f")"
    [ -n "$prefix" ] || continue

    names="$(yq -r '
      (.spec.secrets, .spec.services, .spec.ingresses) | select(. != null)
      | (.include, .exclude) | select(. != null) | .[]
      | .resourceNames | select(. != null) | .[]
    ' "$f")"
    [ -n "$names" ] || continue
    total=$(( total + $(printf '%s\n' "$names" | grep -c .) ))

    bad="$(printf '%s\n' "$names" | grep -vF "${prefix}{{ .name }}" || true)"
    [ -z "$bad" ] || violations="$violations$(printf '%s\n' "$bad" | sed "s|^|${f#"$REPO_ROOT"/}: |")
"
  done

  # A glob that matched nothing, or a yq that stopped returning names, would
  # otherwise pass silently.
  [ "$total" -ge 1 ] || { echo "inspected zero selectors; this test is broken" >&2; exit 1; }

  if [ -n "$violations" ]; then
    echo "resourceNames selectors that can never match the object their chart creates:" >&2
    printf '%s' "$violations" >&2
    echo "Fix: spell the release prefix out, e.g. \"harbor-{{ .name }}-credentials\"." >&2
    exit 1
  fi
}

3. Cut the header comment down to what the code cannot say

The 25-line header is longer than both commit messages put together — 227 words against 67 and 56 — and most of it is those messages retold. The bar in docs/agents/contributing.md (Review Blockers: Messages, Trailers, Comments) is that "a comment says what the code cannot: the reason for this way over the obvious one, a constraint or unit the types do not carry, an external anchor, a trap for the next refactorer". Measured against that, and against what the commits already carry:

• The opening line restates the @test name one line below it.
• The paragraph deriving the prefix from the release name and the webhook's template context is the fix commit's body at three times the length. Someone who wants that reaches for git log or git blame, and there it stays correct.
• The paragraph on the omission being silent is the test commit's body, likewise. The harbor incident that follows it — that the definition shipped this way and the symptom was worked around by stamping a label on the Secret — is the one genuinely new thing in the header, but it is motivation for the change rather than something a future editor of this file needs, and as a comment it goes stale the day that label workaround is removed. Both commit messages are already terse and sufficient as they stand; this is a request to cut, not to move anything into them.

Two things in there are worth keeping, because nothing else in the repo records them:

• Why the assertion is contains rather than a prefix match. Nine live selectors across clickhouse, mongodb, redis and valkey carry their own leading words, so someone tightening this to startsWith would break all nine, and this is the only place that warns them.
• Why an empty prefix is skipped, which otherwise reads as a hole in the check rather than a deliberate exclusion.

Both fit in five lines:

#!/usr/bin/env bats
# "contains <prefix>{{ .name }}" rather than a prefix match: operator-created
# objects carry their own leading words — redis's "rfs-redis-{{ .name }}",
# mongodb's excluded "internal-mongodb-{{ .name }}-users".
#
# An empty spec.release.prefix is skipped: hack/update-crd.sh generates the
# definitions for packages/extra charts with PREFIX="".

One correction while that paragraph is being rewritten: the empty-prefix skip is currently described as covering packages/extra/*, but the glob only reads packages/system/*-rd/cozyrds/*.yaml and there are no definitions under packages/extra at all. What happens is that eight definitions under packages/system — bootbox, computeplane, etcd, external-dns, info, ingress, monitoring, seaweedfs — are generated from charts in packages/extra, and hack/update-crd.sh gives those PREFIX="".

4. Add a per-selector opt-out

A repo-wide name check will eventually meet a selector that is correct and still fails it — an object an operator names on its own terms, or one deliberately matched across releases. Right now the only escape is the blanket empty-prefix skip, which is all-or-nothing per file. Something closer to Go's //nolint, scoped to the one selector and carrying a reason, would keep the guard from turning into a thing people route around.

A YAML line comment is the natural home, since the selector line is the thing being exempted, and yq can read it directly. These files already carry comments inside resourceNames lists — mongodb-rd/cozyrds/mongodb.yaml annotates several of its exclude entries that way — and a comment survives hack/update-crd.sh, which edits the file with yq -i on named fields and round-trips everything else:

  secrets:
    include:
      - resourceNames:
          - "{{ .name }}-credentials" # cozy-lint:ignore rd-release-prefix -- named by the operator, not the release

Honouring it costs one more expression and one more filter. Emit the comment alongside each name and drop the annotated rows before the assertion:

    names="$(yq -r '
      (.spec.secrets, .spec.services, .spec.ingresses) | select(. != null)
      | (.include, .exclude) | select(. != null) | .[]
      | .resourceNames | select(. != null) | .[]
      | [., (line_comment // "")] | @tsv
    ' "$f")"
    ...
    bad="$(printf '%s\n' "$names" \
      | grep -v 'cozy-lint:ignore rd-release-prefix' \
      | grep -vF "${prefix}{{ .name }}" || true)"

Verified against yq v4.45.1: line_comment returns the trailing comment on a sequence entry, and adding the marker to one of the three pre-fix harbor selectors suppresses exactly that one while the other two still fail. Requiring the -- <reason> tail, the way a //nolint directive is expected to explain itself, keeps a bare suppression from being the path of least resistance.

All four points are addressed, and the opt-out went further than requested — enforcing the -- <reason> tail rather than merely conventioning it closes the hole where the directive quietly becomes a mute button.

Checked the rewritten guard against the tree rather than only reading it. It runs under hack/cozytest.sh and inspects 84 selectors across the 24 prefixed definitions, so the port to yq did not narrow what the check sees. Reverting the three harbor selectors reports all three, labelled by section and branch. A marker carrying a reason exempts that one entry and leaves the others failing; a marker with no reason, or with an empty one, is reported as (exempted without a reason) rather than honoured.

The flow-sequence caveat in the header is worth the two lines it takes. Putting the marker on resourceNames: ["{{ .name }}-credentials"] # cozy-lint:ignore ... leaves the entry still reported, since the comment binds to the list rather than to the item — so a directive that does not take makes the test fail rather than pass. That is the right direction for it to break in, and the comment is the only place a reader would learn it.

index($2, needle) is a nicer answer than a fixed-string grep for the containment test, and the [ -f "$f" ] guard covers an unmatched glob expanding to a literal path.

One cosmetic thing, not worth another round: the failure header reads "selectors that can never match the object their chart creates", which does not quite describe a reason-less exemption — though the (exempted without a reason) suffix on the row disambiguates it where it counts.

LGTM.