docs(design): add per-FD BMH namespace placement to fd-datatemplate-selection

[shibaPuppy]

proposing an opt-in bmhNamespace field on each Metal3Cluster.Spec.FailureDomains entry, so each failure domain can be backed by its own BareMetalHost namespace.
composes with the existing per-FD DataTemplate selection.

[metal3-io-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign lentzi90 for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• design/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[metal3-io-bot]

Hi @shibaPuppy. Thanks for your PR.

I'm waiting for a metal3-io member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Tip

We noticed you've done this a few times! Consider joining the org to skip this step and gain /lgtm and other bot rights. We recommend asking approvers on your previous PRs to sponsor you.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[lentzi90]

/ok-to-test

[lentzi90]

Thank you for the proposal!

Unfortunately I do not think this is a good idea. At least not without further adjustments and security/multi-tenancy considerations.
My main objection is that this would make it possible for a user with Metal3 privileges in namespace A to "hijack" BareMetalHosts in any other namespace, even if they have no privileges there.

I think we will need to think about how the HostClaim or CAPM3 multi-tenancy proposal could be extended instead to support failure domains, instead of adding it on separately, on top of the current API directly.

[shibaPuppy]

@lentzi90 thanks for catching this — let me lay out the threat model I have in mind and check whether it matches your concern.

this proposal targets a single-tenant, multi-site deployment. The per-FD namespaces represent physical sites (rack / AZ / power domain), not tenants — one operator owns the Metal3Cluster, the Metal3Machines, and all the per-FD BMH namespaces. In that model, hijacking a BMH from another FD namespace is not a meaningful threat because the same operator already owns it.

HostClaim and the multi-tenancy proposal remain the right answer when namespaces are tenant boundaries. The two designs are complementary, not alternatives: HostClaim provides a cross-tenant API for BMH sharing; this proposal provides FD-aware placement for a single tenant's hardware.

the proposal does not expand the controller's reach. Two existing mechanisms already cover the relevant boundary:

1. CAPM3 already holds cluster-wide get/list/watch permissions on BareMetalHost in the default install and already reconciles BMHs in every namespace it watches. bmhNamespace only lets the operator declare the BMH namespace for an FD; it does not expand what the controller can reach.
2. Deployments that need to constrain CAPM3's reach (e.g. multi-tenant clusters) use the standard WATCH_NAMESPACES (or equivalent manager option) to scope the controller. A bmhNamespace value outside the watched set resolves to NotFound — there is no escalation path through the controller.

in short, WATCH_NAMESPACES is the operative security boundary for this design. The feature is opt-in: clusters that do not set bmhNamespace keep today's same-namespace behavior byte for byte.

the underlying concern — cross-namespace BMH consumption — applies to HostClaim as well, since HostClaim's whole point is to let a tenant consume BMHs from another namespace. HostClaim addresses this with HostDeployPolicy, a per-namespace policy CR that the infrastructure team places alongside the BareMetalHosts to declare which HostClaim namespaces are authorized to bind. That's a more elaborate, tenant-aware policy model than what this proposal needs for the single-tenant case, where WATCH_NAMESPACES plus standard RBAC are sufficient. The two designs target different scopes and, I think, can land independently.

beyond the namespace-A-hijacks-namespace-B scenario you raised, are there other escalation paths you'd want me to consider for this design? I want to make sure the runtime boundary (WATCH_NAMESPACES) is the right answer here before I settle the security model in the doc.

If I've mischaracterized anything above (especially the HostClaim model), please correct me — I'd appreciate it.

[lentzi90]

@shibaPuppy I understand the intended use-case, and in that situation it would be ok. However, there are also use-cases that rely on RBAC and controller behavior for multi-tenancy between namespaces. I.e. without deploying separate BMO instances for each group of WATCH_NAMESPACE.

These use-cases would be broken by the per-FD BMH namespace since there is no way to opt out of it. If we add the field, then any user who can create a Metal3Cluster can also set this field and break the namespace boundary.

You are correct that the controller itself does not get any more privileges with this proposal. However, the expected behavior is that user privileges are controlled through RBAC. We must make sure that they cannot escalate privileges by abusing the controller. One of the most common ways this can happen is exactly the situation in this proposal. We let users access objects in namespaces outside their own reach, through the controller.

One example of what this could lead to is this:

• Create a Metal3Cluster with per-FD BMH namespace
• Configure the Metal3DataTemplate to inject the default ServiceAccount token as metadata
• Add a cloud-init script that sends you all the metadata
• Provision the BMHs and collect the data
• The user now has access to ServiceAccount credentials in all namespaces that have BMHs.

[Copilot]

Pull request overview

This PR updates the FailureDomain-aware DataTemplate selection design to also describe an opt-in per-failure-domain BareMetalHost (BMH) namespace placement mechanism (bmhNamespace) in Metal3Cluster.Spec.FailureDomains, intended to support site/rack/AZ separation via namespaces and to compose with per-FD Metal3DataTemplate selection.

Changes:

• Expands the proposal and motivation to include namespace-based site separation for BMHs via a per-FD bmhNamespace field.
• Documents expected controller behavior/lookup paths that must honor cross-namespace BMH placement, plus an example scenario and backward-compatibility notes.
• Adds related links and an alternatives section discussing HostSelector-based namespace selection.

Comments suppressed due to low confidence (3)

design/fd-datatemplate-selection.md:53

• Terminology/style: per design/bare-metal-style-guide.md, in prose use “bare metal” (noun) or “bare-metal” (adjective). “baremetal infrastructure” should be updated to the appropriate form to match the repo’s convention.

As an operator who has placed their baremetal infrastructure across different
FDs with **different network segments** (subnets, VLANs, IP pools), I would

design/fd-datatemplate-selection.md:559

• Same terminology concern as above: “BMH ownerReference removal” is ambiguous/mismatched with the Kubernetes ownerReferences field. Please align the wording to ownerReferences (or explicitly describe removal of the Metal3Machine ownerReference entry).

- Requires PR [#2506][pr-2506]. The earliest CAPM3 release that can
  carry it is the same release that ships the BMH `ownerReference`
  removal.

design/fd-datatemplate-selection.md:592

• The Kubernetes concept/field is ownerReferences (plural). Consider updating the link text to match (or to the exact title of the referenced issue) to avoid implying a non-existent ownerReference field.

- [PR metal3-io/cluster-api-provider-metal3#2506 — Remove Metal3Machine owner reference from BMH][pr-2506]
- [Kubernetes #94631 — ownerReference cross-namespace](https://github.com/kubernetes/kubernetes/issues/94631)

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[shibaPuppy]

@lentzi90 i understood
I was answering "opt-in" from the user's perspective, but the gap you're pointing at is the lack of an admin-side opt-out. RBAC-based multi-tenancy (single BMO + RBAC for separation) is a legitimate, common deployment, and this proposal breaks the boundary it relies on. the DataTemplate-into-cloud-init exfiltration chain you described is concrete and not theoretical; thanks for spelling it out.

the cleanest opt-out lever that doesn't require new admin machinery is RBAC itself: gate bmhNamespace at admission so the field is effectively usable only by users the admin has already granted BMH access in the target namespace. concretely, the Metal3Cluster validation webhook would issue a SubjectAccessReview against the requesting user's identity for each non-empty bmhNamespace, checking list baremetalhosts.metal3.io in that namespace. If denied, admission rejects the write.

in other words, the admin's existing Role/RoleBinding policy itself becomes the opt-out knob — no new feature gate, no separate CR:

• From the admin's side: the RBAC policy that grants BMH read access is what decides who can use bmhNamespace.
• From the user's side: a user can only declare a bmhNamespace they already have read access to.
• Against the attack chain you described: it stops at step 1 — Team A cannot declare bmhNamespace: team-b unless Team B has explicitly granted them BMH read in team-b.

would the following doc updates work for you?

1. Adding a Security model section that captures this threat model (including your DataTemplate exfiltration example) and the SubjectAccessReview-based admission check as the mitigation.
2. Marking the SubjectAccessReview validation webhook as part of the implementation scope.

Let me know if you'd want a stricter check. I went with list baremetalhosts as the minimum because if a user already has that, they could have read the same data directly without our feature, so the controller is no longer adding reach.

[lentzi90]

I have not come across any such setup before, with a validating webhook that would check RBAC permissions as part of validation. This makes me quite skeptical of it, but I am willing to consider it.
I have two immediate concerns with it:

1. It would require the webhook to make a request to the API server as part of validation. This is something we try hard to avoid. It adds latency and can break validation for other reasons than an invalid object.
2. We would take on a certain authorization burden. This is not something we do anywhere else and it is obviously a sensitive function. Great power and great responsibility and all that. I am basically worried that we make mistakes and it has far reaching consequences.

Given the above, do you think it is worth it? Do you see other ways of achieving the same (e.g. using HostClaims or the multi-tenancy proposal) that would avoid these concerns?

I would also be interested in @tuminoid 's input as security lead.

[shibaPuppy]

would a feature-gate-based controller change work here, without adding any new fields to the CRDs?

concretely:

• New feature gate CrossNamespaceBMH, default off, alpha.
• No fields added to any CRD — Metal3Cluster.Spec.FailureDomains stays exactly as in fd-support-kcp.md.
• When disabled (the default), behavior is byte-for-byte identical to today.
• When enabled, CAPM3's BMH selection (chooseHost and the related lookup paths) widens its search to the namespaces the controller watches, still filtering by the existing FD label.

to clarify the asymmetry: the proposal doesn't move Metal3Machine (or any other CAPI/CAPM3 resource) into a different namespace — they all stay in the cluster's control-plane namespace (e.g. metal3). The only thing that changes is that the BMH a given Metal3Machine consumes can live in a different namespace.

on HostClaim as the long-term alternative: architecturally it lines up well — HostClaimSpec.FailureDomain is literally for this kind of placement. The current state, though: the HostClaim controller in BMO is still a stub, follow-up lifecycle PRs are open and unmerged, and CAPM3 has no HostClaim integration, so redirecting this PR to HostClaim today wouldn't give us a working path. Beyond that, cross-namespace BMH consumption is the underlying capability that both paths (direct, and via HostClaim) need — so even after HostClaim's CAPM3 integration lands, this feature gate is likely still relevant rather than purely a stop-gap.

If this direction works, I'd update the doc in this PR to drop the bmhNamespace field, replace it with the CrossNamespaceBMH feature gate, and add deployment guidance limiting its use to single-tenant deployments (multi-tenant clusters should keep the gate off).

Does this direction work for you? Welcome @tuminoid 's input as well from a security standpoint.

[lentzi90]

Sorry, I don't think this is a good idea. Feature gates are normally expected to graduate and become the default behavior at some point. I don't see this happening for this feature. There is too much risk and we would end up doing double work for something that we can anyway get with HostClaims.

I would much rather focus on implementing HostClaim support in CAPM3. One of the main points with the HostClaim feature is that we reduce the control CAPM3 and CAPM3 users have over the BMHs. We do not want to make CAPM3 pick BMHs across namespaces. Instead it should create HostClaims and be assigned BMHs based on the policy that the BMH owner decides.

There is already agreement about implementing HostClaims. I understand that it may seem like it will take a long time until it is fully implemented, but please understand that getting consensus in the community, deciding what to actually implement, this usually takes more time than actually implementing it. You are very welcome to help with the implementation also. When there is an already accepted design doc it is usually quite easy to get the PR merged.

[tuminoid]

I have not come across any such setup before, with a validating webhook that would check RBAC permissions as part of validation. This makes me quite skeptical of it, but I am willing to consider it. I have two immediate concerns with it:

1. It would require the webhook to make a request to the API server as part of validation. This is something we try hard to avoid. It adds latency and can break validation for other reasons than an invalid object.
2. We would take on a certain authorization burden. This is not something we do anywhere else and it is obviously a sensitive function. Great power and great responsibility and all that. I am basically worried that we make mistakes and it has far reaching consequences.

Given the above, do you think it is worth it? Do you see other ways of achieving the same (e.g. using HostClaims or the multi-tenancy proposal) that would avoid these concerns?

I would also be interested in @tuminoid 's input as security lead.

Let's see if I follow the discussion and whats asked here. I did not check the whole proposal, just the discussion.

I have not come across webhooks doing SAR, the API roundtrip back to API server is problematic as @lentzi90 pointed out. Safeguards are in the controllers, we have implemented few of those more recently. What the controller then checks is some sort of opt-in mechanism, which is often a CR in target namespace, which hence can be created by the namespace owner or the infra admin. As said yourself, this is done in the HostClaim too and we have these safeguards in place all over the code. I see no reason to try reinvent the wheel here.

WATCH_NAMESPACES is deployment level knob, and as we've noticed on the BMO, it is not properly supported on controller-runtime. Using it to fix confused deputy doesn't work, as its all or nothing, making no difference between users or RBAC.

[shibaPuppy]

@lentzi90 @tuminoid understood, thank you.
I'll close this PR and move to contributing on HostClaim and CAPM3.

before I start, could you help me with:

• beyond design/hostclaim-multitenancy.md, are there other docs / past discussions / KEPs I should read to get to a contributor-level understanding?
• which open issues / PRs best show where HostClaim stands today, and is there a project board / tracking issue to follow?
• separate from the HostClaim track, would it be acceptable to keep iterating on FD-related improvements on top of the current BMH model (with the existing FD design)?

once the HostClaim design on the CAPM3 side lands, I'll bring the FD use case back as a separate proposal.

[lentzi90]

Thanks for understanding!

There is a large github discussion about HostClaim implementation details here: https://github.com/orgs/metal3-io/discussions/2795.
Beyond that, I think @pierrecregut is the one who can best tell you what the current status is and what the next steps are. I would suggest coordinating with him any work you do. Currently I think the focus is on metal3-io/baremetal-operator#3152.

I am also open to FD-related improvements in the current API, but of course these must be "reasonable" as discussed above.