fix(helm): reject mismatched trust-manager TLS namespaces

[krisztian-gajdar]

With bundled trust-manager's Helm certificate mode enabled, a child namespace override puts the Deployment in that namespace while the pinned dependency generates its TLS Secret and certificate DNS in the Helm release namespace. The chart currently renders this inconsistent configuration successfully. Reject that combination during the existing TLS validation, with a clear choice of using the release namespace or cert-manager certificate generation.

Use the enabled child context and its namespace helper. Disabled dependencies, a separate namespace in cert-manager mode, SIE's independent global namespace, the Bundle source namespace, and non-default Helm release names/namespaces retain their supported behavior. Dependency versions, CRDs, certificate contents, and deployment defaults are unchanged. The latest inspected upstream release still has the same split; source evidence is in #781.

Update the earlier credential-separation test to exercise a valid cross-namespace Helm-TLS consumer (SIE workloads in another namespace), while dedicated tests now require the intrinsically inconsistent child-namespace combination to fail. README consistently documents that render failure.

Closes #781.

Validation

• 12 focused real Helm render tests, including actual producer/consumer namespace assertions and custom release namespaces.
• Before the fix, all three invalid-configuration assertions failed and the remaining seven assertions passed.
• Full rebased combined suite: 1,025 passed (537 existing Helm, 117 CI-contract, 359 dependency credential, 12 namespace tests). Public tooling: 611 passed, 9 deselected.
• Helm lint, CI Ruff format/check selections, actionlint, and diff checks passed. Exact-head independent adversarial review: no findings; all 12 namespace tests, four converted-fixture assertions and four mutations passed independently. CI and CodeRabbit must also pass before merge.

Sibling-sweep evidence

Bug class: a chart-generated TLS Secret/certificate DNS uses a different namespace authority from the consuming Deployment.

rg -n 'genCA|genSignedCert|genSelfSignedCert' deploy/helm/sie-cluster --glob '*.yaml' --glob '*.tpl'
# 0 first-party matches (also confirmed by the internal sibling-sweep helper with scope cluster)
rg -n 'genCA|genSignedCert|genSelfSignedCert' /tmp/sie-767-dependencies --glob '*.yaml' --glob '*.tpl'

Full pinned-dependency output (archives extracted from the unchanged Chart.lock):

/tmp/sie-767-dependencies/trust-manager/templates/webhook.yaml:9:genCA has two args - first is cert commonName (CN) and second is validity in days (9125 = ~25 years)
/tmp/sie-767-dependencies/trust-manager/templates/webhook.yaml:17:{{- $ca := genCA (printf "*.%s.svc" .Release.Namespace ) 9125 -}}
/tmp/sie-767-dependencies/trust-manager/templates/webhook.yaml:24:genSignedCert has the following args, in order:
/tmp/sie-767-dependencies/trust-manager/templates/webhook.yaml:31:{{- $cert := genSignedCert $svcName nil (list $svcName) 3650 $ca -}}

All four generator matches belong to the guarded trust-manager Helm mode. I additionally inspected the other pinned certificate producers: KEDA's cert-manager objects and serving Pods consistently use Release.Namespace; Prometheus's certificate objects, certgen Job arguments and serving Deployments use the child namespace helper; cert-manager's dynamic serving uses POD_NAMESPACE from its namespaced Deployment. Trust-manager's cert-manager mode uses its helper consistently and is covered by a rendered producer/consumer test. First-party self-signed/ACME resources use the existing SIE namespace / explicit cert-manager cluster-resource namespace contract; externally configured Issuers, raw provider settings and arbitrary mounts remain bounded operator configuration as assessed in #780. No other matching Helm-generated namespace split was found. No live cluster or Secret content was inspected.

Summary by CodeRabbit

• Bug Fixes

  • Helm now rejects configurations where Helm-generated webhook TLS would be created in a different namespace from the workload.
  • Configurations using cert-manager for TLS can use a separate child namespace.

• Documentation

  • Clarified namespace requirements for webhook TLS and noted that namespace settings for other components do not relocate the TLS producer.

• Tests

  • Added Helm coverage for namespace validation and supported TLS configurations.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Team
• Run ID: 7952c580-81c3-41b3-844b-059bd8846591

📥 Commits

Reviewing files that changed from the base of the PR and between ac600a5 and cecd1c9.

📒 Files selected for processing: 5

• .github/workflows/ci.yml
• deploy/helm/sie-cluster/README.md
• deploy/helm/sie-cluster/templates/_helpers.tpl
• tools/ci/tests/test_helm_dependency_credentials.py
• tools/ci/tests/test_helm_trust_namespace.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.

📝 Walkthrough

The chart now rejects Helm-generated trust-manager TLS when the trust-manager namespace differs from the Helm release namespace. New rendering tests cover namespace behavior, and Helm CI runs the test suite separately.

Changes: Trust-manager TLS namespace guard

Layer / File(s)	Summary
Validate Helm TLS namespace deploy/helm/sie-cluster/templates/_helpers.tpl, deploy/helm/sie-cluster/README.md	validateTls rejects a differing trust-manager namespace when Helm-generated TLS is enabled. The README documents the release-namespace constraint and the cert-manager alternative.
Test namespace behavior and run it in Helm CI tools/ci/tests/test_helm_trust_namespace.py, tools/ci/tests/test_helm_dependency_credentials.py, .github/workflows/ci.yml	Rendering tests cover rejected and supported configurations, including custom release namespaces. CI excludes the namespace test from the general test command and runs it in the Helm job. The inactive credential test case now sets global.namespace to workers.

Suggested reviewers: mamayer19

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to cecd1

The chart now fails at render time when Helm-generated trust-manager TLS is combined with a different child namespace, and it points users to the release namespace or cert-manager instead. No merge-blocking risk was identified.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files. (3 skipped: 3 …	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Linked Issues check		The PR addresses #781 with the allowed configuration-guard option. The validateTls helper rejects an installed trust-manager with app.webhook.tls.helmCert.enabled=true when `trust-manager.namespac…
Out of Scope Changes check		The changed helper, README, namespace tests, CI test-lane configuration, and credential-separation fixture update all support #781. The fixture update creates a valid cross-namespace SIE workload case…
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely identifies the main change: rejecting mismatched trust-manager TLS namespaces in Helm.

Full details: Linked Issues check

Explanation

The PR addresses #781 with the allowed configuration-guard option. The validateTls helper rejects an installed trust-manager with app.webhook.tls.helmCert.enabled=true when trust-manager.namespace differs from the Helm release namespace. The error directs users to clear the override or use cert-manager generation. The focused tests cover Helm and cert-manager modes, matching and foreign child namespaces, disabled installation, bundle source namespaces, and custom release namespaces. The README documents the render-time constraint. No dependency upgrade is used, so lock and CRD changes are not required.

Full details: Out of Scope Changes check

Explanation

The changed helper, README, namespace tests, CI test-lane configuration, and credential-separation fixture update all support #781. The fixture update creates a valid cross-namespace SIE workload case and does not add unrelated product behavior. No demonstrated unrelated change appears in the supplied PR evidence.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files. (3 skipped: 3 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

• Fix all pre-merge checks with AI

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[krisztian-gajdar]

Agent-generated adversarial security review

Verified immutable head: cecd1c96f6cfa4fe7f7eb25a37bd2f112da0d927.

Whole-PR range: ac600a51d1c5ea2af17d495b38e896dd3472b3cc...cecd1c96f6cfa4fe7f7eb25a37bd2f112da0d927, five files: the eight-line validation guard, README, CI wiring, the new namespace suite, and one converted credential fixture inherited from #782.

Verdict: no findings. The guard rejects enabled bundled trust-manager in Helm-certificate mode when its effective child namespace differs from the Helm release namespace. It permits cert-manager mode, disabled dependencies, and SIE/Bundle-source namespace settings that do not relocate the TLS producer. The error names both actual namespaces and provides supported remediation. It neither silently moves credentials nor changes the pinned dependency.

I did not author the change. This review independently inspected the full rebased diff and pinned sources, rendered copied charts, tested mutations, and matched all five tested source files to git show at the immutable head. HEAD and source hashes stayed stable during execution. Repository files, refs, PRs, services, and deconflict state were not changed by this reviewer. Probes and mutations used only uniquely prefixed /tmp/sie-781-review-* paths. Credentials were not technically stripped from the environment; read-only restrictions rest on the review instructions and observed actions.

Pinned contract and integration

Chart.lock pins trust-manager v0.13.0. Its templates/webhook.yaml:17-45 generates its Helm certificate DNS and Secret in .Release.Namespace; templates/deployment.yaml:5,140-144 uses include "trust-manager.namespace" for the consumer. That helper follows Values.namespace or .Release.Namespace (_helpers.tpl:54-56). Its cert-manager Certificate uses the helper consistently (certificate.yaml:19-26). The new guard follows those actual rules, including arbitrary release names/namespaces, and does not compare against SIE global.namespace or Bundle app.trust.namespace.

The rebased credential fixture now uses Helm TLS in the release namespace sie while SIE workloads use workers. This is valid, remains a cross-namespace credential-separation control, and is not the inherently inconsistent foreign-child Helm namespace that the new suite rejects. The inventory readers and credential guard implementation from #782 are otherwise unchanged.

The resolved CI diff preserves the existing credential suite and its tooling-lane exclusion, adds the namespace suite/exclusion, and runs both suites after Helm dependency preparation. The README now describes rejection rather than merely advising operators to avoid the mismatch; its remediation matches the error.

Independent verification

PYTHONDONTWRITEBYTECODE=1 .venv/bin/python /tmp/sie-781-review-final-probes.py passed:

• all 12 namespace-suite assertions, including real Secret/Certificate-to-Deployment alignment, disabled dependency, cert-manager mode, Bundle source, SIE workload namespace, and persistent custom Helm release namespace controls;
• all four assertions affected by the converted credential fixture: rendered absence in the SIE namespace and worker/gateway/upstream alias controls;
• CI preservation/order checks and immutable five-file SHA-256 binding.

Four copied-template mutations were caught on the final head:

1. Removing the guard breaks a mismatch regression.
2. Applying it to cert-manager mode breaks a supported separate-namespace control.
3. Comparing against SIE global.namespace breaks a mismatch test when SIE happens to share the foreign child namespace.
4. Replacing .Release.Namespace with literal "sie" breaks the new custom-release test using system-edge.

Preparation review also independently exercised six custom release/name/namespace combinations. Final persistent tests close the preparation coverage recommendation: custom release namespaces are now part of the repository suite. Prior preparation artifacts remain unchanged; this report supersedes their pending-immutable-head qualification.

git diff --check for the final whole-PR range passed. The author separately reports 1,025 combined Helm/policy assertions, 611 tooling tests, lint, Ruff, and actionlint passing; I did not rerun those entire suites and do not present them as this reviewer's executions.

Sibling coverage and limits

An independent genCA|genSignedCert|genSelfSignedCert sweep over extracted pinned YAML/templates returned four lines, all in trust-manager's webhook.yaml (two calls and two explanatory comments); the same sweep found zero first-party template matches. Exact commands/output are saved in /tmp/sie-781-review-sibling-sweep.txt. Producer/consumer source review also confirms KEDA's generated TLS paths use the release namespace consistently and Prometheus certgen/Certificate paths use its child namespace helper consistently. No additional untriaged instance of this mismatched Helm producer/consumer namespace class was identified in the reviewed scope.

This is a render-time validation fix. No live Kubernetes install/upgrade, certificate issuance, TLS handshake, Secret content lookup, hosted endpoint, external service mutation, or registry binary was exercised. Certificate-generation mode and namespaces were verified from the pinned templates and rendered objects. No token or certificate private-key payload was printed or read from a live cluster.

Immutable source record

File	SHA-256
.github/workflows/ci.yml	1ba088e78fb7c2127a0653868df1ae0d6be3a921b115c0790979bfc99a254caf
deploy/helm/sie-cluster/README.md	3fe2150201f458239f47a0ffe65a536d1cbefb093a36b775620e16af03bce9f2
deploy/helm/sie-cluster/templates/_helpers.tpl	71a658684f20b8949e7a9815bf0056fdd9733804546f84ef35415a03a690d76f
tools/ci/tests/test_helm_dependency_credentials.py	97df381066073f9e1e4a82b2adf62926ec4736a48ec9cce951f1d06519403db0
tools/ci/tests/test_helm_trust_namespace.py	4b760dc04adda47b7229400806d77295d922d4de1f5e364c9c0549fa673cf4b4

Blocking findings: 0

[krisztian-gajdar]

Merged as 25bd316f12531f4241fd982cc2c45d819e422a1c after all 42 checks passed on reviewed head cecd1c96f6cfa4fe7f7eb25a37bd2f112da0d927, including CI / Required and Release candidate / Complete. CodeRabbit approved that exact head; there were no unresolved review threads. A fresh fetch and no-op rebase confirmed the branch was current immediately before the normal merge.

Remote validation: 537 existing Helm render tests, 359 dependency-credential tests, all 12 namespace regressions, 15,320 public unit tests, and 611 tooling tests passed. The independent adversarial security review verified this exact head and reported zero blocking findings.

This closes #781. The guard rejects the pinned dependency's mismatched Helm-generated TLS namespace configuration; it preserves supported matching namespaces and cert-manager mode. No deployment or live setting was changed. Arbitrary operator credential layouts remain tracked separately in #780.