[mashkovd]

Chart prerequisite for #435 (explicit identity linking, #446). It mirrors oidcProvidersSecret from #440.

What

• New value zitadelLinkSecret, empty by default. When set, the chart renders ZITADEL_LINK_CLIENT_ID and ZITADEL_LINK_CLIENT_SECRET from secretKeyRef (same key names, optional: true).
• In production this is mctl-api-oidc-zitadel, which the zitadel-iac Job fills (feat(zitadel): confidential link client for mctl-api identity linking mctl-gitops#1592).
• Absent keys leave both variables unset. cmd/api (feat(principals): explicit ZITADEL identity linking and admin merge #446) then keeps the link browser flow off (503) and logs why. The pod never fails on it.
• With the value unset, the render is byte-identical to before.

Tests

• go test ./helm/ adds three checks:
  • empty value renders unchanged;
  • both env entries render once each, from the secretKeyRef, with no literal value and optional: true;
  • helm lint passes with the value set.
• Mutation check: dropping optional from the secret's ref fails the test.

Production is unaffected until mctl-gitops sets the value, which happens in the wiring PR after #446 is released.

Refs #435

🤖 Generated with Claude Code

https://claude.ai/code/session_01D1kGUJMWeA8hyhQuxKuHd2

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[claude]

Claude finished @mashkovd's task in 1m 25s —— View job

---

Reviewing PR #447

• Fetch pinned diff (70e3ad3...b035c33)
• Check template logic against existing oidcProvidersSecret pattern it mirrors
• Verify test coverage
• Post inline findings
• Submit verdict

Summary: This chart change mirrors the existing oidcProvidersSecret pattern closely (empty-by-default value, secretKeyRef with optional: true, no literal in the manifest), and the new tests correctly exercise byte-identical rendering, env sourcing, and helm lint.

One gap found:

• P2 (helm/templates/deployment.yaml:123): the block it mirrors has a {{- fail ... }} guard (backed by TestDeploymentOIDCProvidersSecretConflictsWithLiteral) that prevents oidcProvidersSecret + a literal env.MCTL_OIDC_PROVIDERS from rendering two env entries with the same name. The new zitadelLinkSecret block has no equivalent guard for env.ZITADEL_LINK_CLIENT_ID / env.ZITADEL_LINK_CLIENT_SECRET, so setting both would silently render a duplicate env key, which server-side apply rejects.

Review verdict: Changes requested (1 P2, 0 P1, 0 P3).

[claude]

Has P1/P2 findings, changes requested: 1 P2 — the new zitadelLinkSecret env block mirrors oidcProvidersSecret's secretKeyRef pattern but omits the matching literal-vs-secret conflict guard (and test) that oidcProvidersSecret has for MCTL_OIDC_PROVIDERS.

[github-actions]

Antigravity (agy) review — informational

Model: gemini-3.8-flash-medium · commit b035c33.

No significant issues found.

[claude]

Claude finished @mashkovd's task in 1m 41s —— View job

---

Reviewing PR #447 (commit 4c68c7d)

• Fetch pinned diff (70e3ad3...4c68c7d)
• Check whether the previously-flagged P2 (missing conflict guard) was addressed
• Verify test coverage for the fix
• Post findings / verdict

Summary: Commit 4c68c7d ("fix(helm): refuse a literal ZITADEL_LINK_* env next to zitadelLinkSecret") resolves the P2 raised on the prior commit (b035c33): helm/templates/deployment.yaml:43-44 now adds a {{- fail ... }} guard when zitadelLinkSecret and a literal env.ZITADEL_LINK_CLIENT_ID/ZITADEL_LINK_CLIENT_SECRET are both set, and the .Values.env range exclusion (deployment.yaml:51-52) now skips those two keys the same way it already skipped MCTL_OIDC_PROVIDERS for oidcProvidersSecret — exactly mirroring the established pattern.

Test coverage added for the fix matches the existing oidcProvidersSecret suite:

• TestDeploymentZitadelLinkSecretConflictsWithLiteral — asserts helm template fails when both the secret and a literal are set, for each key.
• TestDeploymentZitadelLinkSecretSkipsAnEmptyLiteral — confirms an empty literal doesn't trip the guard but also doesn't duplicate the env entry.
• TestDeploymentEmptyZitadelLinkSecretRendersUnchanged, TestDeploymentRendersZitadelLinkSecretEnv, and the helm lint matrix entry carry over from the earlier commits.

No remaining correctness, security, or test-coverage gaps found. Verdict submitted: Approved (0 P1, 0 P2, 0 P3).

[github-actions]

Antigravity (agy) review — informational

Model: gemini-3.8-flash-medium · commit 4c68c7d.

No significant issues found.

[claude]

No P1/P2 findings (0 P3). The prior P2 (missing conflict guard between zitadelLinkSecret and literal env.ZITADEL_LINK_CLIENT_ID/_SECRET) is now fixed: deployment.yaml adds the same fail-guard and range-exclusion pattern used by oidcProvidersSecret, with matching tests (conflict, empty-literal, byte-identical default, env-sourcing, lint). Good to merge.