fix: adopt pre-existing namespaces with no owner annotation - #331
fix: adopt pre-existing namespaces with no owner annotation#331rckr2710 wants to merge 1 commit into
Conversation
Signed-off-by: rckr2710 <rckr2710@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
This would need significant discussions about the security implications before we can make any changes like this. It would also likely be classified as a "breaking change" and so only be able to be done on a major version. Can you share what your motivation was here so we can understand if there is a better solution? /hold |
|
Yes, this is a feature we're looking for because of one of our enterprise onboarding workflows @thesuperzapper A couple of use cases:
I completely agree that automatically adopting any existing namespace would be a security concern. Instead, what if adoption was explicitly allowed by an administrator? For example, only namespaces with a label like:
would be eligible for adoption. Label present → Kubeflow can adopt the namespace. Label absent → Keep the current behavior and reject adoption. This way, the decision stays with the Kubernetes/Kubeflow administrators, and teams can enable it only for namespaces that are intentionally prepared for Kubeflow. The default behavior remains secure while still supporting enterprise onboarding workflows @rckr2710 , I would help you with the changes. |
|
Was thinking more about this, and the one of the most serious question this feature opens is: Could someone with Profile access take over the |
|
That's a valid concern, and I agree Profile users should never be able to adopt arbitrary namespaces. My thought was that the decision should be entirely in the hands of the Kubernetes/Kubeflow administrator. For example, Kubeflow would only consider adopting a namespace if:
Namespaces like The intention isn't to let users take over existing namespaces, but to support enterprise environments where platform teams pre-provision dedicated namespaces for Kubeflow users and explicitly allow Kubeflow to manage them. |
There was a problem hiding this comment.
Pull request overview
Enables the Profile controller to adopt pre-existing, unclaimed namespaces while retaining ownership-conflict protection.
Changes:
- Adds namespace adoption, labels, Istio injection, and controller ownership.
- Adds unit tests for adoption, conflicts, and label preservation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
profile_controller.go |
Implements namespace adoption and ownership handling. |
profile_controller_test.go |
Tests namespace adoption scenarios. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if err := controllerutil.SetControllerReference(instance, foundNs, r.Scheme); err != nil { | ||
| logger.Info("warning: could not set ControllerReference on adopted namespace", | ||
| "namespace", foundNs.Name, "error", err.Error()) | ||
| } |
| // Check existing namespace ownership before moving forward. | ||
| // | ||
| // A namespace that has no "owner" annotation at all has never been claimed | ||
| // by any Profile (for example, it was created directly via | ||
| // `kubectl create ns <name>` before the Profile CR existed). Such a |
Problem
The Profile controller rejects a namespace that already exists when its
Profile CR is applied, unless the namespace already has a matching
ownerannotation. A namespace created viakubectl create ns(noannotation at all) was always rejected, even with no other claimant.
Fix
Adopt the namespace when it has no
ownerannotation yet (nothing hasclaimed it), and continue to reject only when it's owned by a different
user. On adoption, also set a controller
OwnerReferenceso thenamespace is garbage-collected with its Profile, matching the behavior
of namespaces this controller creates itself.
Testing
Unit Tests
Added unit tests in profile_controller_test.go covering the following scenarios:
All unit tests pass successfully.
make test
Manual Validation
The changes were verified on a local Kind cluster by:
kubectl create namespace test-profile
related: #330
closes: #330