design-proposal: public IPs as a first-class resource (PublicIP / PublicIPClaim / PublicIPClass)#35
design-proposal: public IPs as a first-class resource (PublicIP / PublicIPClaim / PublicIPClass)#35Timofei Larkin (lllamnyp) wants to merge 1 commit into
Conversation
…cIPClaim/PublicIPClass) A tenant cannot own a public address today: it exists only as a side effect of a Service type: LoadBalancer and evaporates with it. Nothing can be held across a workload rebuild, moved between workloads, quota'd, or enumerated. Applies the PersistentVolume pattern to addresses -- PublicIPClass (the class), PublicIP (the cluster-scoped inventory object), PublicIPClaim (the namespaced request) -- plus a provisioner contract in the shape of CSI, with an explicit Pin capability that the whole model depends on. Records that this is a regression rather than a wishlist: Cozystack has reserve-then-associate today through kube-ovn's OvnEip, and migrating off kube-ovn drops it. Stub: object model and primitives survey are worked out; controller mechanics, provisioner contract details and rollout are sketched, and opened early to settle the shape before code. Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a design proposal to treat public IPs as first-class resources in Cozystack, modeling them after the Kubernetes PersistentVolume pattern with PublicIP, PublicIPClaim, and PublicIPClass resources. The feedback highlights three key areas for improvement: updating the PublicIPClaim status schema to support dual-stack scenarios by using a list of IP objects, configuring MetalLB's IPAddressPool with autoAssign: false to prevent IP conflicts with standard services, and clarifying how the ValidatingAdmissionPolicy will securely identify the controller principal.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| status: | ||
| phase: Bound | ||
| publicIPName: pip-203-0-113-7 | ||
| address: 203.0.113.7 # what the tenant reads, and puts in DNS |
There was a problem hiding this comment.
If a Dual stack claim binds to two separate PublicIP objects (as suggested in Open Question 5), the current status schema for PublicIPClaim is insufficient because publicIPName and address are single scalar strings.
To support dual-stack where one claim maps to multiple PublicIP resources, the status should be designed to accommodate multiple names and addresses (e.g., using a list of IP status objects).
| status: | |
| phase: Bound | |
| publicIPName: pip-203-0-113-7 | |
| address: 203.0.113.7 # what the tenant reads, and puts in DNS | |
| status: | |
| phase: Bound | |
| publicIPs: | |
| - name: pip-203-0-113-7 | |
| address: 203.0.113.7 |
| For MetalLB, forever, for the whole cluster: | ||
|
|
||
| - **one** `IPAddressPool` covering the range, and | ||
| - **one** `L2Advertisement` (or `BGPAdvertisement`) selecting it. | ||
|
|
||
| That is all. No pool per tenant, no `/32`s. The `PublicIPClass` carries the range; the | ||
| provisioner renders the backend objects. |
There was a problem hiding this comment.
There is a potential conflict between the Cozystack provisioner and MetalLB's automatic allocation. If a single IPAddressPool covering the entire range is configured in MetalLB (with autoAssign: true by default), MetalLB will automatically allocate any free IP from this pool to standard Services that do not use claims.
Since MetalLB is unaware of unassociated PublicIP reservations, it could allocate a reserved (but currently unattached) IP to a standard Service, leading to IP conflicts or binding failures when the claim owner later attempts to associate it.
To prevent this, the proposal should specify that the MetalLB IPAddressPool managed by the provisioner must have autoAssign: false configured, or clarify how standard Services are prevented from stealing reserved IPs.
| 3. A `ValidatingAdmissionPolicy` **rejects any principal but the controller** writing | ||
| *any* backend pin annotation (§1.2's list) on a Service. |
There was a problem hiding this comment.
Using a ValidatingAdmissionPolicy to reject any principal except the controller from writing the pin annotations is a robust security measure. However, identifying "the controller" securely in a ValidatingAdmissionPolicy can be challenging and brittle if based solely on hardcoded usernames or namespaces.
Consider specifying how the policy will securely identify the controller principal (e.g., by matching a specific ServiceAccount name and namespace, or utilizing a specific SubjectGroup) to ensure it is both secure and maintainable across different deployment environments.
…esses either Tenet 3 says the CNI does not announce, and increment 3 deleted the announcer. But it left the ALLOCATOR: cozyplane still picks external addresses out of an ExternalPool (firstFreeAddress, for both a VPCGateway's NAT identity and every FloatingIP) and NOTHING ATTRACTS what it allocates. An ExternalPool is a hand-written CIDR list that nothing routes; the e2e admits it -- 'here we do what a CCM would do and simply configure it on one' -- and that CCM does not exist. Allocation without attraction is an address that exists in etcd and nowhere on the wire. Pinning the address onto a Service does not bridge the gap: MetalLB's autoAssign:false does NOT reserve, so MetalLB stays the allocator of record and can hand the address cozyplane chose to some other Service. Two IPAMs, one range, no arbitration. And an LB implementation only ever announces an address that is the live loadBalancer.ingress of a Service it allocated -- so an external address can only be attracted if some OBJECT holds it. Resolved in north-south.md §9: cozyplane does not allocate either. It consumes a platform-allocated, platform-attracted PublicIPClaim (cozystack/community#35). Three consequences, each fixing something currently papered over: - The grant moves onto the claim. It must: a bare Service would let anyone who can create a Service mint a public address, which is the R10 hole. - A pod references a CLAIM, not an address. FloatingIP.spec.target is a VPC IP today -- address-thinking, against tenet 4 -- and a claim reference on the pod makes 'one target, one address' structural instead of a controller tiebreak. - Only a claim can hold an EGRESS identity. A Service is an ingress object; VPCGateway.status.natAddress is many-to-one and has no Service to hang on, yet still needs attracting because the replies come back to it. The VPC case is what proves the claim object has to exist. FloatingIP as a kind is likely superseded by a pod annotation naming a claim. The eBPF does not change at all -- the maps key on an address that arrives from somewhere and never cared who picked it, which is the strongest evidence the boundary was drawn in the right place. Answers north-south.md §7's standing questions on who attracts an EIP and whether its ingress half survives. Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
|
A few reactions grounded in a production Cozystack deployment that is squarely the Strong +1 on the shape, and especially on the security move as a standalone winThe PV/PVC/StorageClass framing is the right altitude, and the Security section The "regression, not a wishlist" framing overstates it for at least one adopterkube-ovn is installed for us and the Suggested additional open question: multiple public addresses per workloadFine to defer the design, but I'd like it captured explicitly, because it
Two API observations that this open question should keep in mind:
None of this blocks the core question — "is an address a resource?" is a clear yes, |
2 participants
What
Adds a design proposal under
design-proposals/public-ip-claims/introducing public addresses as a first-class resource: a cluster-scopedPublicIP, a namespacedPublicIPClaim, aPublicIPClass, and a provisioner contract in the shape of CSI.Status: stub. The problem statement, the primitives survey and the object model are worked out. The controller mechanics, the provisioner contract details and the rollout are deliberately sketched. It is opened early to settle the shape — is an address a resource? — before anyone writes code.
Why
A tenant cannot own a public address. One appears as a side effect of creating a
Service type: LoadBalancer, and evaporates when that Service is deleted. So an address cannot be:The experience we want is the AWS one: allocate an address, keep it, attach it, detach it, attach it to something else, release it when done.
This is a regression, not a wishlist
Cozystack has reserve-then-associate today, via kube-ovn:
OvnEipis created independently and later bound by anOvnFip/OvnSnatRule/OvnDnatRulethat references it by name. Migrating off kube-ovn drops that capability unless something replaces it. The question is not "should Cozystack gain an AWS-style EIP" but "on what terms does it keep the one it already has."The model
The
PersistentVolumepattern, applied to addresses:StorageClassPublicIPClassPersistentVolumePublicIP(cluster-scoped)claimRefand a reclaim policyPersistentVolumeClaimPublicIPClaim(namespaced)Two things fall out of the survey and are worth a reviewer's attention:
fromClassvsproviderRef), directly analogous to PV's volume-source union. Self-allocating backends (MetalLB, Cilium, kube-vip) have no reservation concept anywhere, so our provisioner is the IPAM of record. Cloud backends (AWS, GCP) already hold the reservation with a stable handle and their own IAM, so the provisioner must adopt, not allocate. Surviving a backend where allocation isn't ours is the evidence the shape isn't bent around MetalLB.Pincapability, which the entire model depends on: a backend that cannot be told which address to use can never attach a reserved one, so a class over it must reject claims outright rather than allocate an address it can never bind.The sharp edge (please read the Security section)
The per-backend "pin this IP" annotation (
metallb.io/loadBalancerIPs,lbipam.cilium.io/ips, …) is a privilege-escalation surface. A self-allocating backend has no concept of a reservation, so it will hand a tenant an address that another tenant has reserved but not yet attached. That theft window exists on MetalLB and Cilium alike.The fix is the move Cozystack already makes elsewhere: turn the ungatable field into a reference to an RBAC-gated object. Tenants may only write
network.cozystack.io/public-ip-claim(naming an object in their own namespace); the controller writes the raw pin annotation; aValidatingAdmissionPolicyrejects any other principal writing one. Without that policy the model is advisory, so it belongs in the same release as the CRDs.Notes for reviewers
structured-external-exposure). That proposal restructures how an application requests exposure; this one is about what an address is. They meet at one point — an exposure entry ought to be able to name a claim instead of implicitly minting an address — but neither needs the other to land first, and this does not assume design-proposal: structured, additive external exposure for managed applications #29's shape.ExposureClassis touched on but not depended on. The class idea is right and survives here in some form; binding an address's lifetime to aServiceis the part that structurally cannot serve this use case. Open question 4 defers the merge-or-separate call, since that kind's future is being re-examined independently.ServiceCIDR/IPAddressand explicitly named "any generalization onto something like an IPAM API" a non-goal;loadBalancerIPwas deprecated in 1.24 with no core replacement. Cluster API'sIPAddressClaim/IPAddressis literally this pattern but has only ever been wired to Machine addressing — open question 3 asks whether we adopt it or merely copy the contract.The test that is the proposal
Claim → bind → associate → delete the Service → the address is still held → associate it to a different workload → the same address comes back.
That single integration test is precisely what cannot pass today.