Install all Helm chart resources into the release namespace

Merged
GJ
2d ago
+481-75

What type of PR is this?

/kind bug

What this PR does / why we need it:

helm install -n <ns> only moved part of Kelos into <ns>. The controller manager and its supporting resources were hardcoded to kelos-system, and the chart always created Namespace/kelos-system.

This PR makes every namespaced resource follow the release namespace:

  • Hand-written templates: the controller Deployment, kelos-controller ServiceAccount, kelos-webhook Service, Certificate (including dnsNames), Issuer, leader-election Role/RoleBinding, kelos-controller-rolebinding subject, and the telemetry CronJob use {{ .Release.Namespace }}. The control-plane PodMonitor selects only the release namespace.
  • Generated files: hack/update-install-manifest.sh templates the webhook Service namespace and cert-manager.io/inject-ca-from in validating-webhook.yaml and in the subchart CRDs that carry conversion webhooks. The static install-crd.yaml (applied by kelos install) keeps kelos-system.
  • Namespace object: rendered only when the release namespace is kelos-system, so kelos install and existing kelos-system releases keep owning it. Other namespaces come from --create-namespace or the user.
  • Telemetry: the installation ID ConfigMap namespace is injected through a new --telemetry-namespace controller flag, set by the CronJob to the release namespace. It is required with --telemetry-report.
  • kelos install / kelos uninstall: no behavior change; they still render into kelos-system.
  • Conversion guard: with crds.install=false, the chart looks up tasks.kelos.dev and fails if its conversion webhook names a namespace other than the release namespace. This stops an upgrade from moving the kelos-webhook Service away from CRDs the release does not render (CRDs from kelos install, or CRDs kept by crds.keep during phase 1 of a two-phase upgrade). lookup finds nothing under helm template and kelos install, so offline rendering is unaffected; tools that render with helm template (such as Argo CD) skip the check, and crds.install=false releases need get on customresourcedefinitions. The failure message points to "Release Namespace" in the chart README.
  • Docs: chart README ("Release Namespace" section with the guard and a migration guide for releases that own kelos-system from another namespace; the Option 2 CRD adoption loop now lists taskpipelines.kelos.dev and webhookgateways.kelos.dev, which the chart also renders), the podMonitor.enabled row in docs/reference.md, and the troubleshooting skill.
  • Integration suite: test/integration/suite_test.go renders the chart's ValidatingWebhookConfiguration for envtest instead of reading templates/validating-webhook.yaml, which is no longer plain YAML.

Which issue(s) this PR is related to:

Fixes #1816

Special notes for your reviewer:

Not waiting on #1725. The issue marks this as blocked on #1725 because of the CRD conversion webhook. This PR templates the conversion clientConfig.service.namespace and CA annotation in the subchart CRDs, so Helm-rendered CRDs (crds.install=true) work in any namespace. For CRDs the release does not render, the conversion guard refuses an install or upgrade that would leave them pointing at a namespace without the webhook, and the README migration guide covers the one-phase crds.install=true upgrade (adopting outside-managed CRDs first). Once #1725 removes conversion, the guard becomes a no-op (CRDs without a conversion webhook pass) and can be dropped along with the README conversion notes. #1725 will need a small rebase over hack/update-install-manifest.sh, the subchart CRDs, webhook.yaml, render_test.go, and the README.

Decision needed: kelos-system deletion on upgrade. Existing releases installed with -n <other> own Namespace/kelos-system. Upgrading them stops rendering it, so Helm deletes kelos-system and anything in it, and their telemetry installation ID is regenerated. This follows the approach proposed in the issue. Operators who want to keep the namespace can run kubectl annotate namespace kelos-system helm.sh/resource-policy=keep before upgrading; Helm checks the live object's annotation before deleting it (pkg/kube/client.go in Helm v3.20.1). The README and release note document this opt-out.

Verification

  • make verify, make test, and make test-integration pass.
  • New TestRender_ReleaseNamespace renders into kelos-system and kelos and checks namespaced objects, ServiceAccount binding subjects, Certificate dnsNames, webhook/CRD conversion service namespaces, CA annotations, the telemetry flag, the PodMonitor selector, and that the Namespace object is rendered only for kelos-system. Reverting the template changes makes it fail.
  • Helm v3.20 against an envtest API server (Kubernetes 1.31) with cert-manager CRDs:
    • fresh helm install -n team-b --create-namespace --set crds.install=true: deployed; all 11 namespaced objects in team-b; no kelos-system namespace.
    • -n kelos-system release on the previous chart upgraded to this chart: Namespace/kelos-system kept (same UID, Active).
    • -n team-a release on the previous chart upgraded to this chart: controller, Service, and Certificate move to team-a; kelos-system is deleted, as described above.
    • -n team-a release on the previous chart with Helm-owned CRDs: phase 1 (crds.install=false) is refused by the conversion guard with nothing applied; after kubectl annotate namespace kelos-system helm.sh/resource-policy=keep, a single crds.install=true upgrade moves the CRD conversion to team-a and keeps kelos-system; a later phase 1 upgrade passes the guard.
  • New TestRender_CRDConversionNamespaceGuard renders through a fake lookup client and covers the rejection plus no live CRD, a CRD without conversion, matching namespaces, a kelos-system release, and crds.install=true.

Pre-existing, not changed here: on main, a first helm install -n kelos-system --create-namespace fails with Helm v3.20 (namespaces "kelos-system" already exists) because both Helm and the chart create the namespace; a retry succeeds as an upgrade. The behavior for kelos-system releases is unchanged by this PR.

Does this PR introduce a user-facing change?

The Helm chart now installs the controller manager, its webhook Service, Certificate, Issuer, leader-election RBAC, and the telemetry CronJob into the Helm release namespace instead of always using `kelos-system`. The chart renders `Namespace/kelos-system` only when the release namespace is `kelos-system`; other release namespaces must already exist or be created with `--create-namespace`. Releases installed into `kelos-system` and `kelos install` are unaffected.

Action required for releases installed into a namespace other than `kelos-system`: the upgrade moves these resources into the release namespace and deletes the `kelos-system` namespace, including anything else in it, unless you first run `kubectl annotate namespace kelos-system helm.sh/resource-policy=keep`. The telemetry installation ID is regenerated. Upgrade in one step with `--set crds.install=true` instead of the two-phase flow; if the CRDs are managed outside Helm, adopt them into the release first. With `crds.install=false`, installs and upgrades now fail when the existing `tasks.kelos.dev` CRD sends conversion requests to a namespace other than the release namespace. See "Release Namespace" in the chart README.

šŸ¤– Generated with Claude Code

Summary by cubic

Every namespaced Helm chart resource now follows the release namespace instead of hardcoding kelos-system, so helm install -n <ns> no longer splits Kelos across two namespaces.

  • The controller Deployment, ServiceAccount, webhook Service, Certificate, Issuer, leader-election RBAC, and telemetry CronJob use {{ .Release.Namespace }}, as do the webhook Service and CA annotations in the validating webhook and in the subchart CRDs.
  • Namespace/kelos-system is rendered only for kelos-system releases, so kelos install keeps owning it; other namespaces come from --create-namespace or the user.
  • The telemetry CronJob passes its namespace through the new required --telemetry-namespace flag.
  • With crds.install=false, the chart looks up tasks.kelos.dev and fails if its conversion webhook names a namespace other than the release namespace; lookup finds nothing under helm template, so offline rendering is unaffected.
  • The README documents the migration path and the CRD adoption loop now covers every CRD the chart renders.
  • The integration suite renders the chart and hands envtest the resulting ValidatingWebhookConfiguration, since the template is no longer plain YAML.
  • kelos install and kelos uninstall are unchanged.

Migration

  • Upgrading a release in another namespace moves these resources into the release namespace and deletes kelos-system, including anything else in it; the telemetry installation ID is regenerated. Annotate the namespace with helm.sh/resource-policy=keep to retain it.
  • Upgrade such a release once with crds.install=true, adopting outside-managed CRDs into Helm first; the two-phase flow is not supported for this upgrade.