operator: own the unbounded-system namespace - #530
Open
Philip Lombardi (plombardi89) wants to merge 6 commits into
Open
operator: own the unbounded-system namespace#530Philip Lombardi (plombardi89) wants to merge 6 commits into
Philip Lombardi (plombardi89) wants to merge 6 commits into
Conversation
The shared unbounded-system namespace was declared by every component's manifest set (net, machina, storage, gantry) and applied by the operator under one server-side-apply field manager with ForceOwnership. Because SSA replaces a manager's owned field set on each apply, the namespace was rewritten several times per reconcile pass and its app.kubernetes.io/name label churned between competing per-component values. There was also no single owner for namespace-level policy, in particular the Pod Security Admission labels the privileged/hostPath workloads here require. Make the operator the sole owner of the namespace object: - internal/unbounded: add the canonical label set (SystemNamespaceLabels) as the single source of truth: app.kubernetes.io/name=unbounded-cloud, managed-by=unbounded-operator, and PSA enforce=privileged (version latest). - component/env: add NamespaceKind and skip Namespace objects centrally in applyManifestData, so no component (present or future) writes the namespace during reconcile. - operator: add BootstrapNamespace, which server-side applies the namespace from a minimal object (name + the operator's own label keys only, no annotations). Granular per-key SSA ownership preserves any labels and annotations placed by another actor while the operator stays authoritative on its own keys (it reasserts enforce=privileged). It refuses legacy namespaces and creates the namespace if absent. - operator: add BootstrapAll (namespace then CRDs) and generalize CRDMaintainer -> BootstrapMaintainer so the periodic maintainer reapplies the namespace alongside the CRDs. main wires both. - deploy: unify the Namespace doc across net/machina/storage/operator to the canonical labels + PSA, and split gantry's Namespace out of serviceaccount.yaml into a standalone 00-namespace.yaml so the templates are uniform. The docs remain in each component's manifests for the standalone kubectl-apply path, exactly like CRDs. Tests: BootstrapNamespace label/idempotency/legacy-refusal and the third-party label+annotation preservation guarantee; applyManifestData skips Namespace; a gantry reconcile applies no Namespace; the namespace drift guard now asserts the operator-managed templates carry exactly SystemNamespaceLabels.
Copilot started reviewing on behalf of
Philip Lombardi (plombardi89)
July 22, 2026 01:03
View session
Contributor
There was a problem hiding this comment.
Pull request overview
This PR consolidates runtime ownership of the shared unbounded-system Kubernetes Namespace under unbounded-operator to eliminate server-side-apply multi-writer churn and to make Pod Security Admission (PSA) labels have a single authoritative owner.
Changes:
- Introduces canonical
SystemNamespaceLabels()(including PSAenforce=privileged) as the single source of truth and enforces template drift-guard parity. - Ensures operator reconcile never applies
kind: Namespacefrom component manifests, and adds an operator bootstrap that SSA-applies only the operator-owned label keys. - Extends bootstrap/maintenance to include the namespace alongside CRDs; aligns deploy templates (including splitting gantry namespace into
00-namespace.yaml.tmpl).
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/unbounded/unbounded.go | Adds canonical system-namespace label constants and SystemNamespaceLabels() helper. |
| internal/unbounded/namespace_drift_test.go | Extends drift guard to assert canonical labels + PSA for operator-managed templates. |
| internal/operator/namespace.go | Adds BootstrapNamespace SSA apply of minimal Namespace object (operator-owned label keys only). |
| internal/operator/namespace_test.go | Adds unit tests for namespace bootstrap, idempotency, legacy refusal, and preservation intent. |
| internal/operator/components/gantry/gantry_test.go | Adds regression assertion that reconcile applies no Namespace objects. |
| internal/operator/component/env.go | Adds NamespaceKind and skips Namespace docs during manifest apply. |
| internal/operator/component/env_test.go | Adds test ensuring operator apply path skips Namespace and still applies other objects. |
| internal/operator/bootstrap.go | Adds BootstrapAll (namespace then CRDs) and generalizes maintainer to reapply both. |
| internal/operator/bootstrap_test.go | Renames/updates maintainer tests for the generalized bootstrap maintainer. |
| deploy/unbounded-storage-supervisor/01-namespace.yaml.tmpl | Unifies Namespace labels to canonical set + PSA. |
| deploy/unbounded-operator/00-namespace.yaml.tmpl | Unifies Namespace labels to canonical set + PSA. |
| deploy/net/00-namespace.yaml.tmpl | Unifies Namespace labels to canonical set + PSA. |
| deploy/machina/01-namespace.yaml.tmpl | Unifies Namespace labels to canonical set + PSA. |
| deploy/gantry/serviceaccount.yaml.tmpl | Removes embedded Namespace doc; points to standalone namespace manifest. |
| deploy/gantry/00-namespace.yaml.tmpl | Adds standalone canonical Namespace manifest for gantry direct-apply path. |
| cmd/unbounded-operator/main.go | Wires BootstrapAll at startup and adds BootstrapMaintainer to manager. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4 tasks
Address review feedback on the namespace-ownership change. - component/env: scope the reconcile-time Namespace skip to the one shared system namespace, matched by name (BuildDefaultNamespace) rather than by Kind. The skip runs before RetargetNamespace, so the embedded object still carries its build-time name. Matching by name keeps a future component that ships a distinct namespace from being silently dropped. Extend the env test to assert a differently-named Namespace is still applied while the system namespace is skipped. - operator: strengthen the fake-client preservation test to apply the foreign label, annotation, and tampered PSA level under a distinct field manager instead of pre-seeding them as unowned metadata. This exercises the real multi-manager server-side-apply path: the operator's ForceOwnership apply must reassert only the keys it declares and leave another manager's keys intact. - e2e/operator: add a kind-based test that validates the SSA preservation contract against a real API server. It stages the namespace under a foreign field manager (foreign label + annotation + enforce=restricted), runs BootstrapNamespace, and asserts the foreign keys survive, PSA is reasserted to privileged, and metadata.managedFields shows the operator owns exactly its four label keys and no annotations. This retires the "confirm against a live cluster" caveat. Unifying the non-operator-managed namespace flows (orca, inventory, machine-ops) that also default into unbounded-system is tracked separately in #531.
Collaborator
Author
|
Pushed 6bfdde8 addressing review feedback:
Legacy-namespace refusal (hard startup failure) confirmed intended; no change. Follow-up for the non-operator-managed flows (orca, inventory, machine-ops) that also default into
|
Copilot started reviewing on behalf of
Philip Lombardi (plombardi89)
July 22, 2026 02:18
View session
TestBootstrapNamespaceDefaultsEmptyNamespace was environment-dependent and tautological: it resolved the expected namespace with SystemNamespace() on both the act and assert sides, so a set POD_NAMESPACE would silently exercise the POD_NAMESPACE branch (or, if it named a legacy namespace, fail with a refusal) instead of the documented fallback to the build default. Pin POD_NAMESPACE to empty and assert the created namespace is exactly DefaultSystemNamespace, so the test validates the empty -> default fallback contract regardless of the test process environment.
Copilot started reviewing on behalf of
Philip Lombardi (plombardi89)
July 22, 2026 02:27
View session
4 tasks
# Conflicts: # internal/operator/components/gantry/gantry_test.go
Copilot started reviewing on behalf of
Philip Lombardi (plombardi89)
July 22, 2026 03:05
View session
Jason Wilder (jwilder)
approved these changes
Jul 22, 2026
Copilot started reviewing on behalf of
Philip Lombardi (plombardi89)
July 22, 2026 05:07
View session
Philip Lombardi (plombardi89)
enabled auto-merge
July 22, 2026 05:25
Copilot started reviewing on behalf of
Philip Lombardi (plombardi89)
July 22, 2026 15:44
View session
github-merge-queue
Bot
removed this pull request from the merge queue due to no response for status checks
Jul 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Makes the unbounded-operator the sole owner of the shared
unbounded-systemNamespace object, fixing the multi-writer label churn and giving Pod Security Admission (PSA) labels a single home. Implements #529.Previously net, machina, storage, and gantry each shipped their own
Namespace/unbounded-systemobject, and the operator applied all of them under one server-side-apply field manager (unbounded-operator) withForceOwnership. Because SSA replaces a manager's owned field set on each apply, the namespace was rewritten several times per reconcile pass and itsapp.kubernetes.io/namelabel churned between competing per-component values (unbounded-net-> stripped ->gantry-> ...). There was also no single owner for the PSA labels the privileged/hostPath workloads here require.What it does
internal/unbounded- adds the canonical label set (SystemNamespaceLabels) as the single source of truth:app.kubernetes.io/name=unbounded-cloud,app.kubernetes.io/managed-by=unbounded-operator, and PSAenforce=privileged(versionlatest).component/env- addsNamespaceKindand skipsNamespaceobjects centrally inapplyManifestData, so no component (present or future) writes the namespace during reconcile.operator(newBootstrapNamespace) - server-side applies the namespace from a minimal object (name + the operator's own label keys only, no annotations). Granular per-key SSA ownership preserves any labels and annotations placed by another actor (admin, GitOps, policy engine) while the operator stays authoritative on its own keys (it reassertsenforce=privileged). Refuses legacy namespaces; creates the namespace if absent.operator- addsBootstrapAll(namespace then CRDs) and generalizesCRDMaintainer->BootstrapMaintainerso the periodic maintainer reapplies the namespace alongside the CRDs.mainwires both at startup + maintenance.deploy- unifies theNamespacedoc across net/machina/storage/operator to the canonical labels + PSA, and splits gantry's Namespace out ofserviceaccount.yamlinto a standalone00-namespace.yaml. The docs stay in each component's manifests for the standalonekubectl apply -f deploy/<x>path, exactly like CRDs.Preservation contract
BootstrapNamespaceis authoritative only for the keys it declares. All other labels and annotations on the namespace are preserved (per-key SSA ownership).ForceOwnershiponly makes the operator win on its own keys, e.g. correcting a third-party PSA downgrade back toprivileged.Testing
BootstrapNamespace: canonical labels, idempotency, legacy-namespace refusal, empty-namespace default, and the third-party label + annotation preservation guarantee (including reasserting a tampered PSA level).applyManifestData/ApplyManifestFSskipsNamespace, applies everything else.Namespace.SystemNamespaceLabels(); gantry entry repointed to00-namespace.yaml.make fmt,make lint(0 issues), andgo test ./internal/operator/... ./internal/unbounded/... ./cmd/unbounded-operator/...all pass.Note
The preservation guarantee relies on real apiserver SSA granular-map semantics. The fake-client tests pass (foreign keys preserved, operator keys reasserted), documenting intent, but this is worth a quick confirmation against a live cluster before merge.
Closes #529