From 46863e451c022149d603459f071c5f842fd24536 Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Wed, 26 Aug 2026 19:00:45 -0500 Subject: [PATCH] feat: allocate a network's fabric identity A network's identity on the fabric is chosen at random, per location, by whatever places the network there, and checked against nothing. Two locations of one network get unrelated values, so the Route Target derived from each imports neither the other's routes, and the edge VRF device is named differently in each. Two networks that collide do not fail closed: they merge, and tenant traffic crosses between them. The platform now allocates one identity per network, once, and projects it into every location the network reaches. It is drawn from an identifier space that is never routed, so uniqueness, exhaustion accounting, quota and audit come from an allocator that already solves them. It is 32 bits wide because that is what survives into the Route Target; anything wider would be uniqueness the platform believes it has and the fabric does not. A network that holds no identity behaves exactly as it does today, so nothing already running changes. Giving the networks that exist a platform-wide identity renames live VRF devices and moves Route Targets on traffic in flight, and needs its own proposal. Key changes: - Add Network.status.fabricIdentity, immutable once allocated, with an Allocated condition, and project it into NetworkContext.spec beside the address families and MTU already carried there - Allocate a /64 from a platform-scoped identifier pool and read the identity out of the block's index within it; refuse the zero block, since zero is what an unallocated network reads as - Add a platform tenancy to the IPAM config and a ClientForPlatform seam on the client factory, so an identity is never drawn from a consumer's own space or gated on them enabling the address service - Refuse at startup a deployment naming an identifier space with nowhere platform-owned to allocate it from --- api/v1alpha/network_types.go | 50 ++++ api/v1alpha/networkcontext_types.go | 21 ++ ...working.datumapis.com_networkcontexts.yaml | 21 ++ .../networking.datumapis.com_networks.yaml | 29 +++ docs/api/networkcontexts.md | 27 ++- docs/api/networks.md | 27 +++ docs/enhancements/vpc-fabric-identity.md | 228 ++++++++++++++++++ internal/cmd/cell/cell.go | 5 +- internal/cmd/manager/manager.go | 17 +- internal/config/config.go | 53 ++++ internal/config/zz_generated.deepcopy.go | 16 ++ internal/config/zz_generated.defaults.go | 2 + internal/controller/ipam_project_client.go | 46 +++- .../controller/ipam_project_client_test.go | 2 +- internal/controller/network_controller.go | 43 +++- .../controller/network_fabric_identity.go | 228 ++++++++++++++++++ .../network_fabric_identity_test.go | 190 +++++++++++++++ .../networkinterfaceclaim_controller_test.go | 38 ++- .../controller/networkpresence_controller.go | 7 + .../networkpresence_controller_test.go | 67 +++++ 20 files changed, 1097 insertions(+), 20 deletions(-) create mode 100644 docs/enhancements/vpc-fabric-identity.md create mode 100644 internal/controller/network_fabric_identity.go create mode 100644 internal/controller/network_fabric_identity_test.go diff --git a/api/v1alpha/network_types.go b/api/v1alpha/network_types.go index 28d4c675..c781c2f9 100644 --- a/api/v1alpha/network_types.go +++ b/api/v1alpha/network_types.go @@ -98,6 +98,32 @@ const ( NetworkReasonRangeUnsupported = "RangeUnsupported" ) +const ( + // NetworkFabricIdentityAllocated reports whether the network holds the + // identity the fabric knows it by. The type is bare because the fabric + // reads this condition as the answer to "does this network have an + // identity", not as one allocation among several. + NetworkFabricIdentityAllocated = "Allocated" + + // NetworkFabricIdentityReasonAllocated means the network holds an identity. + NetworkFabricIdentityReasonAllocated = "Allocated" + + // NetworkFabricIdentityReasonPending means nothing has been allocated yet + // and the reason is not yet one of the ones below. + NetworkFabricIdentityReasonPending = "Pending" + + // NetworkFabricIdentityReasonIdentitySpaceUnavailable means the identity + // space did not answer, so the network has no identity to carry. It is + // retried. + NetworkFabricIdentityReasonIdentitySpaceUnavailable = "IdentitySpaceUnavailable" + + // NetworkFabricIdentityReasonIdentityUnusable means the identity space + // answered with a block the identifier cannot be read out of. Handing out + // the zero block is the case an operator hits first: zero is what an + // unallocated network reads as, so it can never be an allocation. + NetworkFabricIdentityReasonIdentityUnusable = "IdentityUnusable" +) + // NetworkStatus defines the observed state of Network type NetworkStatus struct { // Represents the observations of a network's current state. @@ -107,6 +133,29 @@ type NetworkStatus struct { // // +kubebuilder:validation:Optional IPAM *NetworkIPAMStatus `json:"ipam,omitempty"` + + // FabricIdentity is the identity the fabric knows this network by, + // allocated once, platform-wide, and the same in every location the network + // reaches. What consumes it derives the network's BGP Route Target from it, + // which is what makes two locations of one network import each other's + // routes rather than behave as two networks that share a name. + // + // It is an integer rather than an encoded string because the consumer + // builds `ASN:`, and it is 32 bits wide because that is what + // survives into the Route Target. A wider value would be uniqueness the + // platform believes it has and the fabric does not. + // + // Zero means unallocated, so an unset field and a real allocation never + // read alike. Once set it never changes: the fabric embeds it in import + // policy in every location the network reaches, so a network that changed + // identity would be a different network to everything already carrying its + // traffic. + // + // +kubebuilder:validation:Optional + // +kubebuilder:validation:Minimum=0 + // +kubebuilder:validation:Maximum=4294967295 + // +kubebuilder:validation:XValidation:rule="oldSelf == 0 || self == oldSelf",message="fabricIdentity is immutable once allocated" + FabricIdentity int64 `json:"fabricIdentity,omitempty"` } // NetworkIPAMStatus reports what IPAM holds for a network. @@ -159,6 +208,7 @@ type NetworkPrefixRef struct { // +kubebuilder:printcolumn:name="IPFamilies",type="string",JSONPath=".spec.ipFamilies",priority=1 // +kubebuilder:printcolumn:name="IPAM",type="string",JSONPath=".spec.ipam.mode",priority=1 // +kubebuilder:printcolumn:name="MTU",type="integer",JSONPath=".spec.mtu",priority=1 +// +kubebuilder:printcolumn:name="FabricIdentity",type="integer",JSONPath=".status.fabricIdentity",priority=1 // Network is the Schema for the networks API type Network struct { diff --git a/api/v1alpha/networkcontext_types.go b/api/v1alpha/networkcontext_types.go index 22459eb6..6e20d53c 100644 --- a/api/v1alpha/networkcontext_types.go +++ b/api/v1alpha/networkcontext_types.go @@ -36,9 +36,30 @@ type NetworkContextSpec struct { // +kubebuilder:validation:Maximum=8856 MTU int32 `json:"mtu,omitempty"` + // FabricIdentity is the network's fabric identity, projected from the + // Network's status. This is where a location reads it: cells cannot reach + // project control planes, and propagation to them carries spec and not + // status. + // + // Zero means the identity has not been projected here yet, either because + // the network does not have one or because this location has not caught up + // with the allocation. A reader that finds it unset must wait rather than + // choose an identity of its own, which is what every location does today + // and is the reason one network is two on the fabric. + // + // +kubebuilder:validation:Optional + // +kubebuilder:validation:Minimum=0 + // +kubebuilder:validation:Maximum=4294967295 + FabricIdentity int64 `json:"fabricIdentity,omitempty"` + // The Network generation the projected fields were read from, so an operator // comparing this to the Network can tell whether this location has caught up. // + // The identity is allocated into the Network's status, so it lands without + // advancing that generation. This still answers whether the location has + // caught up with the network's spec; whether it has caught up with the + // allocation is answered by the identity being present. + // // +kubebuilder:validation:Optional NetworkGeneration int64 `json:"networkGeneration,omitempty"` } diff --git a/config/crd/bases/networking.datumapis.com_networkcontexts.yaml b/config/crd/bases/networking.datumapis.com_networkcontexts.yaml index 5c939214..704a4a4c 100644 --- a/config/crd/bases/networking.datumapis.com_networkcontexts.yaml +++ b/config/crd/bases/networking.datumapis.com_networkcontexts.yaml @@ -52,6 +52,22 @@ spec: spec: description: NetworkContextSpec defines the desired state of NetworkContext properties: + fabricIdentity: + description: |- + FabricIdentity is the network's fabric identity, projected from the + Network's status. This is where a location reads it: cells cannot reach + project control planes, and propagation to them carries spec and not + status. + + Zero means the identity has not been projected here yet, either because + the network does not have one or because this location has not caught up + with the allocation. A reader that finds it unset must wait rather than + choose an identity of its own, which is what every location does today + and is the reason one network is two on the fabric. + format: int64 + maximum: 4294967295 + minimum: 0 + type: integer ipFamilies: description: |- IP families the network carries, projected from the Network. @@ -96,6 +112,11 @@ spec: description: |- The Network generation the projected fields were read from, so an operator comparing this to the Network can tell whether this location has caught up. + + The identity is allocated into the Network's status, so it lands without + advancing that generation. This still answers whether the location has + caught up with the network's spec; whether it has caught up with the + allocation is answered by the identity being present. format: int64 type: integer required: diff --git a/config/crd/bases/networking.datumapis.com_networks.yaml b/config/crd/bases/networking.datumapis.com_networks.yaml index 7bd357d9..c6e056ea 100644 --- a/config/crd/bases/networking.datumapis.com_networks.yaml +++ b/config/crd/bases/networking.datumapis.com_networks.yaml @@ -39,6 +39,10 @@ spec: name: MTU priority: 1 type: integer + - jsonPath: .status.fabricIdentity + name: FabricIdentity + priority: 1 + type: integer name: v1alpha schema: openAPIV3Schema: @@ -171,6 +175,31 @@ spec: - type type: object type: array + fabricIdentity: + description: |- + FabricIdentity is the identity the fabric knows this network by, + allocated once, platform-wide, and the same in every location the network + reaches. What consumes it derives the network's BGP Route Target from it, + which is what makes two locations of one network import each other's + routes rather than behave as two networks that share a name. + + It is an integer rather than an encoded string because the consumer + builds `ASN:`, and it is 32 bits wide because that is what + survives into the Route Target. A wider value would be uniqueness the + platform believes it has and the fabric does not. + + Zero means unallocated, so an unset field and a real allocation never + read alike. Once set it never changes: the fabric embeds it in import + policy in every location the network reaches, so a network that changed + identity would be a different network to everything already carrying its + traffic. + format: int64 + maximum: 4294967295 + minimum: 0 + type: integer + x-kubernetes-validations: + - message: fabricIdentity is immutable once allocated + rule: oldSelf == 0 || self == oldSelf ipam: description: IPAM reports the address space IPAM holds for this network. properties: diff --git a/docs/api/networkcontexts.md b/docs/api/networkcontexts.md index 835a883f..36a63629 100644 --- a/docs/api/networkcontexts.md +++ b/docs/api/networkcontexts.md @@ -99,6 +99,26 @@ NetworkContextSpec defines the desired state of NetworkContext The attached network
true + + fabricIdentity + integer + + FabricIdentity is the network's fabric identity, projected from the +Network's status. This is where a location reads it: cells cannot reach +project control planes, and propagation to them carries spec and not +status. + +Zero means the identity has not been projected here yet, either because +the network does not have one or because this location has not caught up +with the allocation. A reader that finds it unset must wait rather than +choose an identity of its own, which is what every location does today +and is the reason one network is two on the fabric.
+
+ Format: int64
+ Minimum: 0
+ Maximum: 4.294967295e+09
+ + false ipFamilies []enum @@ -128,7 +148,12 @@ the same as a network that carries nothing.
integer The Network generation the projected fields were read from, so an operator -comparing this to the Network can tell whether this location has caught up.
+comparing this to the Network can tell whether this location has caught up. + +The identity is allocated into the Network's status, so it lands without +advancing that generation. This still answers whether the location has +caught up with the network's spec; whether it has caught up with the +allocation is answered by the identity being present.

Format: int64
diff --git a/docs/api/networks.md b/docs/api/networks.md index 5d91c007..b3fb7919 100644 --- a/docs/api/networks.md +++ b/docs/api/networks.md @@ -184,6 +184,33 @@ NetworkStatus defines the observed state of Network Represents the observations of a network's current state.
false + + fabricIdentity + integer + + FabricIdentity is the identity the fabric knows this network by, +allocated once, platform-wide, and the same in every location the network +reaches. What consumes it derives the network's BGP Route Target from it, +which is what makes two locations of one network import each other's +routes rather than behave as two networks that share a name. + +It is an integer rather than an encoded string because the consumer +builds `ASN:`, and it is 32 bits wide because that is what +survives into the Route Target. A wider value would be uniqueness the +platform believes it has and the fabric does not. + +Zero means unallocated, so an unset field and a real allocation never +read alike. Once set it never changes: the fabric embeds it in import +policy in every location the network reaches, so a network that changed +identity would be a different network to everything already carrying its +traffic.
+
+ Validations:
  • oldSelf == 0 || self == oldSelf: fabricIdentity is immutable once allocated
    • + Format: int64
    + Minimum: 0
    + Maximum: 4.294967295e+09
    + + false ipam object diff --git a/docs/enhancements/vpc-fabric-identity.md b/docs/enhancements/vpc-fabric-identity.md new file mode 100644 index 00000000..a0714f4c --- /dev/null +++ b/docs/enhancements/vpc-fabric-identity.md @@ -0,0 +1,228 @@ +--- +status: provisional +stage: alpha +latest-milestone: "v0.x" +--- + +# An identity the fabric knows a network by + +- [Summary](#summary) +- [Motivation](#motivation) + - [Goals](#goals) + - [Non-Goals](#non-goals) +- [Proposal](#proposal) + - [What the fabric consumes](#what-the-fabric-consumes) + - [What it feels like](#what-it-feels-like) +- [Design Details](#design-details) + - [Thirty-two bits, not sixty-four](#thirty-two-bits-not-sixty-four) + - [Allocating an integer from a prefix allocator](#allocating-an-integer-from-a-prefix-allocator) + - [Reaching the data plane](#reaching-the-data-plane) + - [Lifecycle](#lifecycle) +- [What this depends on](#what-this-depends-on) +- [Drawbacks](#drawbacks) +- [Alternatives](#alternatives) +- [Open Questions](#open-questions) + +## Summary + +A network on the fabric exists as one forwarding instance per PoP. What makes those +instances the same network is a **BGP Route Target**, and the Route Target is derived from a +per-network identifier that nothing on the platform allocates. + +This proposes that the platform allocate that identifier, once per network, and surface it +where every PoP can read it. + +This is deliberately not a locator, not a Node-ID, and not a per-PoP VRF instance. Those are +separate allocations with different scopes, and one of them is correctly node-local. This +document covers only the identity that has to be the same everywhere. + +## Motivation + +**Nothing allocates it.** The fabric derives a network's Route Target as +`ASN:`, and takes the identifier from the CNI configuration written for each +attachment. Today that value is a literal typed into a NetworkAttachmentDefinition. The +controller that used to mint one was removed along with the CRDs that held it. There is no +allocator, no registry, and no uniqueness check of any kind. + +**Two networks that collide are one network.** The Route Target is what makes a PoP import +another PoP's routes into the right forwarding instance. Two networks sharing an identifier +do not fail closed. They merge: each imports the other's prefixes, and tenant traffic +crosses between them. + +**The identifier is narrower than it looks.** The fabric truncates it to 32 bits when +building the Route Target. A 48-bit value that is unique across the platform is not +sufficient. What must be unique is the low 32 bits, and nothing today says so. + +### Goals + +- One identifier per network, unique platform-wide in the bits the fabric actually uses, + stable for the network's life, the same in every PoP the network reaches. +- Readable by each PoP from a resource its cell already receives. +- Consumers do nothing. They create a network; they never see an identifier. + +### Non-Goals + +- **The per-PoP uSID locator block and per-node Node-ID.** These are the fabric's routing + anchors and today they are hand-assigned with no registry, which is worth fixing. It is a + different allocation, at a different scope, and it belongs in its own proposal. +- **The per-PoP VRF instance identifier.** The fabric allocates this on the node at + attachment time, and that is correct: a packet only reaches the instance lookup after the + locator has already steered it to that node, so the value is disambiguated upstream. + Centralising it would add coordination that buys nothing. +- **The network's address space.** A network's tenant prefix is a separate per-network + allocation, already named in the API and still unallocated. It is not this. + +## Proposal + +### What the fabric consumes + +Three things identify a network's forwarding state, at three different scopes. Only one of +them has to be the same everywhere. + +| What | Scope | Who should own it | +|---|---|---| +| Locator block, Node-ID | per PoP, per node | an allocator, in a separate proposal | +| VRF instance | per network per node | the fabric, on the node | +| **Route Target identifier** | **per network, platform-wide** | **this proposal** | + +### What it feels like + +A consumer creates a network. Nothing they write mentions the fabric. + +```yaml +apiVersion: networking.datumapis.com/v1alpha +kind: Network +metadata: + name: prod +spec: + ipFamilies: [IPv6] +``` + +The platform allocates an identifier and reports it. + +```yaml +status: + fabricIdentity: 305419896 + conditions: + - type: Allocated + status: "True" +``` + +Place that network in two PoPs and both derive the same Route Target from that number. That +is what makes it one network rather than two that share a name. + +## Design Details + +### Thirty-two bits, not sixty-four + +The identifier is a 32-bit unsigned integer, because that is exactly what survives into the +Route Target. Allocating anything wider invites a value whose uniqueness is real in the API +and absent in the fabric. + +It is surfaced as an integer, not as a prefix or an encoded string. The consumer of this +value builds `ASN:`, and asking it to parse an address to recover a number would +be a worse contract with no upside. + +Value `0` is not allocated, so an unset field and a real allocation never read alike. + +### Allocating an integer from a prefix allocator + +The platform's address management service allocates prefixes, not integers. Rather than +build a second allocator with the same uniqueness and concurrency problems already solved +there, an identifier is allocated as a prefix from a pool that is never routed, and the +integer is the block's index within that pool. + +A `/32` root pool handing out `/64`s yields exactly 2^32 allocations, whose distinguishing +bits are exactly the 32 the fabric uses. The mapping is total and order-preserving in both +directions. + +This buys uniqueness, exhaustion accounting, quota, retention and an audit trail for free, +and costs address space that is never routed and never reachable. The pool must be +described as an identifier space so nobody later reads it as addressing. + +The API surfaces the integer. The prefix is an implementation detail of the allocator and +does not appear in the API. + +### Reaching the data plane + +`Network.status` holds the authoritative allocation. It cannot be what a PoP reads: cells +cannot reach project control planes, and federation strips status on propagation. + +The presence controller projects the identifier into `NetworkContext.spec`, alongside the +address families and MTU already projected there. A cell reads one object and has what it +needs. + +Because the identifier is allocated into status, the network's generation does not advance +when it lands. The context is rewritten when the allocation appears, so the projected +generation still answers whether a PoP has caught up with the network's spec, and the +identifier being present answers whether it has caught up with the allocation. + +### Lifecycle + +The identifier is immutable once allocated. The fabric embeds it in import policy across +every PoP the network reaches, so a network that changed identity would be a different +network to everything already carrying its traffic. + +A released identifier is not reissued. A Route Target still installed in a remote PoP's +import policy would silently merge a new network into a dead one's routes. Retention is the +safe failure, and it is unbounded today because the allocator accepts a retention lease and +does not enforce it. + +## What this depends on + +- **The projection path**, already carrying address families and MTU to cells. +- **The per-project client** for the address management service, already used for endpoint + addressing. +- **A platform-owned tenancy** in that service. The platform allocates this on a consumer's + behalf, so it must not be gated on the consumer enabling the service, and must not consume + their claim budget. Without it, uniqueness is per project, which is not uniqueness. + + The address management service has no platform scope of its own today. Until it does, the + operator addresses one project control plane the platform owns and allocates every + network's identity there. That is one pool and one allocator for the whole platform, which + is what the uniqueness actually rests on; what it does not yet get is a tenancy the service + itself understands as the platform's. The seam is a single method on the client factory, so + when the service grows one, nothing above that line changes. +- **The fabric accepting an assigned identifier.** It has no field for one today. + +## Drawbacks + +Network creation gains a dependency on the allocator. Today it cannot fail this way, because +the identifier is not allocated at all. + +Allocating an integer as a prefix reads as a hack to anyone who finds the pool without the +explanation. + +Thirty-two bits is a ceiling. It is the fabric's ceiling rather than one this introduces, but +it is now a platform-wide one, and exhaustion has no answer beyond widening the Route Target +format. + +## Alternatives + +**Derive it from the network UID.** No allocator and no failure mode, but 32 bits of a UID +collides by birthday at a few tens of thousands of networks, undetectably, with no way to +reserve or repair. + +**Let each PoP keep choosing.** This is the status quo and it does not survive a network +reaching a second PoP. + +**Allocate 48 bits to match the field the fabric parses.** The fabric truncates to 32 on the +way into the Route Target, so the extra bits are uniqueness the platform believes it has and +does not. + +## Open Questions + +- Should the fabric stop truncating instead, and carry a wider identifier in a four-byte + Route Target? That reverses this proposal's central constraint, so it is worth answering + before building. +- What is the exhaustion story at 32 bits, and what reclaims identifiers from deleted + networks given retention is currently permanent? +- Should the network's tenant prefix be allocated in the same change? It is the other + per-network global allocation and the field already exists. +- Does anything other than the Route Target need this identifier, and if so does it need the + same width? +- **How do networks that already exist get one?** Every network on the platform today carries a + per-location identifier chosen where it was placed. Allocating one identity for them is not + a field being filled in: it renames the edge VRF device and moves the Route Target on live + traffic. Nothing here attempts it, and a network that holds no identity behaves exactly as + it does now. The migration needs its own proposal. diff --git a/internal/cmd/cell/cell.go b/internal/cmd/cell/cell.go index b339f5e5..559d61ca 100644 --- a/internal/cmd/cell/cell.go +++ b/internal/cmd/cell/cell.go @@ -330,7 +330,10 @@ func newIPAMClientFactory(serverConfig config.CellControllerManager) (controller return nil, fmt.Errorf("unable to build IPAM scheme: %w", err) } - ipamClients, err := controller.NewIPAMClientFactory(ipamRestConfig, ipamScheme) + ipamClients, err := controller.NewIPAMClientFactory(ipamRestConfig, ipamScheme, + // A cell allocates only on a consumer's behalf, inside their own + // project. Nothing platform-scoped is allocated here. + "") if err != nil { return nil, fmt.Errorf("unable to build IPAM client factory: %w", err) } diff --git a/internal/cmd/manager/manager.go b/internal/cmd/manager/manager.go index f0da9a2c..d5cb6d6e 100644 --- a/internal/cmd/manager/manager.go +++ b/internal/cmd/manager/manager.go @@ -566,6 +566,15 @@ func newIPAMClientFactory(ipamConfig config.IPAMConfig) (controller.IPAMClientFa return nil, nil } + // A network's identity has to be unique across every network on the + // platform, so a deployment that names an identifier space and nowhere + // platform-owned to allocate it in would hand out identities unique only + // within one consumer, which is not unique at all. Say so at startup rather + // than per network. + if ipamConfig.Classes.FabricIdentity != "" && ipamConfig.Platform.Project == "" { + return nil, fmt.Errorf("ipam.classes.fabricIdentity names an identifier space, but ipam.platform.project names nowhere platform-owned to allocate from") + } + restConfig, err := ipamConfig.RestConfig() if err != nil { return nil, fmt.Errorf("unable to load IPAM kubeconfig: %w", err) @@ -576,7 +585,7 @@ func newIPAMClientFactory(ipamConfig config.IPAMConfig) (controller.IPAMClientFa return nil, fmt.Errorf("unable to build IPAM scheme: %w", err) } - return controller.NewIPAMClientFactory(restConfig, ipamScheme) + return controller.NewIPAMClientFactory(restConfig, ipamScheme, ipamConfig.Platform.Project) } // controllerRegistrations lists every controller and the set it belongs to. @@ -593,8 +602,10 @@ func controllerRegistrations( return []namedSetup{ {"network", true, func() error { return (&controller.NetworkReconciler{ - IPAM: deps.ipamClients, - PrefixClass: serverConfig.IPAM.Classes.Network, + IPAM: deps.ipamClients, + PrefixClass: serverConfig.IPAM.Classes.Network, + FabricIdentityClass: serverConfig.IPAM.Classes.FabricIdentity, + FabricIdentityNamespace: serverConfig.IPAM.Platform.Namespace, }).SetupWithManager(mgr) }}, {"networkbinding", true, func() error { diff --git a/internal/config/config.go b/internal/config/config.go index 08e26cb4..ea709e8f 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -173,6 +173,47 @@ type IPAMConfig struct { // Classes names the IPClasses this operator asks IPAM for. Classes IPAMClasses `json:"classes,omitempty"` + + // Platform is the tenancy the operator allocates platform-owned values in, + // as opposed to the ones it allocates on a consumer's behalf inside their + // own project. + // + // Left unset, nothing platform-scoped is allocated and every network is + // reconciled exactly as it was before. + Platform PlatformTenancy `json:"platform,omitempty"` +} + +// +k8s:deepcopy-gen=true + +// PlatformTenancy is where the platform allocates the things it owns rather +// than the things a consumer owns. +// +// A network's fabric identity has to be unique across every network on the +// platform, so it cannot be drawn from the consumer's own space: uniqueness per +// project is not uniqueness. It also must not be gated on that consumer +// enabling the address service, and must not draw on their quota, because they +// never asked for it and cannot see it. +// +// IPAM has no platform tenancy of its own yet, so this names a project control +// plane the platform owns and every network's identity is allocated there. That +// gives one pool and one allocator for the whole platform, which is what the +// uniqueness actually rests on. When IPAM grows a real platform scope, only the +// client factory changes. +type PlatformTenancy struct { + // Project is the control plane platform-owned allocations are addressed at. + // Unset means no platform-scoped allocation is made at all. + Project string `json:"project,omitempty"` + + // Namespace is the namespace inside that control plane the claims are + // written to. Defaults to "default", which is the namespace a project + // control plane is provisioned with. + Namespace string `json:"namespace,omitempty"` +} + +func SetDefaults_PlatformTenancy(obj *PlatformTenancy) { + if obj.Namespace == "" { + obj.Namespace = "default" + } } // +k8s:deepcopy-gen=true @@ -191,6 +232,18 @@ type IPAMClasses struct { // restating something the service already knows. Network string `json:"network,omitempty"` + // FabricIdentity is the class that hands out a network's fabric identity. + // Unset, no identity is allocated and a network is reconciled with none, + // the same as an unset IPAM connection. + // + // The class is an identifier space and not addressing. It roots a /32 that + // is never routed and never reachable, and hands out /64s from it, so the + // 32 bits between the two are the block's index and the identity is that + // index. The pool must not hand out its own zero block: zero is what an + // unallocated network reads as, so a network given it would be + // indistinguishable from one given nothing. + FabricIdentity string `json:"fabricIdentity,omitempty"` + // Subnet is the class that hands out the range a network is addressed from // in one location. Unset, no subnet is claimed and a network context is // reconciled with no address space of its own, the same as an unset IPAM diff --git a/internal/config/zz_generated.deepcopy.go b/internal/config/zz_generated.deepcopy.go index 624b3bbe..b3065df0 100644 --- a/internal/config/zz_generated.deepcopy.go +++ b/internal/config/zz_generated.deepcopy.go @@ -515,6 +515,7 @@ func (in *IPAMConfig) DeepCopyInto(out *IPAMConfig) { *out = *in out.Client = in.Client out.Classes = in.Classes + out.Platform = in.Platform } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IPAMConfig. @@ -723,6 +724,21 @@ func (in *OIDCValidationOptions) DeepCopy() *OIDCValidationOptions { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PlatformTenancy) DeepCopyInto(out *PlatformTenancy) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PlatformTenancy. +func (in *PlatformTenancy) DeepCopy() *PlatformTenancy { + if in == nil { + return nil + } + out := new(PlatformTenancy) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RedisConfig) DeepCopyInto(out *RedisConfig) { *out = *in diff --git a/internal/config/zz_generated.defaults.go b/internal/config/zz_generated.defaults.go index 804374d3..5c8849c6 100644 --- a/internal/config/zz_generated.defaults.go +++ b/internal/config/zz_generated.defaults.go @@ -46,6 +46,7 @@ func SetObjectDefaults_CellControllerManager(in *CellControllerManager) { if in.IPAM.Client.Burst == 0 { in.IPAM.Client.Burst = 100 } + SetDefaults_PlatformTenancy(&in.IPAM.Platform) SetDefaults_ClientConnectionConfig(&in.Federation.Client) if in.Federation.Client.QPS == 0 { in.Federation.Client.QPS = 50 @@ -402,6 +403,7 @@ func SetObjectDefaults_NetworkServicesOperator(in *NetworkServicesOperator) { if in.IPAM.Client.Burst == 0 { in.IPAM.Client.Burst = 100 } + SetDefaults_PlatformTenancy(&in.IPAM.Platform) SetDefaults_LocationPublisherConfig(&in.LocationPublisher) SetDefaults_ClientConnectionConfig(&in.LocationPublisher.Client) if in.LocationPublisher.Client.QPS == 0 { diff --git a/internal/controller/ipam_project_client.go b/internal/controller/ipam_project_client.go index b3278ba1..cbc9b670 100644 --- a/internal/controller/ipam_project_client.go +++ b/internal/controller/ipam_project_client.go @@ -18,33 +18,57 @@ import ( "go.datum.net/network-services-operator/internal/downstreamclient" ) -// IPAMClientFactory returns a client bound to one project. Every IPAM request -// goes through one, so no request can reach IPAM without naming a project. +// IPAMClientFactory returns a client bound to one tenancy. Every IPAM request +// goes through one, so no request can reach IPAM without naming who it is for. type IPAMClientFactory interface { + // ClientForProject reaches IPAM on a consumer's behalf, inside their own + // project. What it allocates is theirs, counts against their quota, and is + // only unique among their own allocations. ClientForProject(project string) (client.Client, error) + + // ClientForPlatform reaches IPAM on the platform's own behalf, for values + // that must be unique across every consumer and must not be gated on one + // enabling the address service or draw on their quota. + // + // IPAM has no platform tenancy of its own, so today this is one project + // control plane the platform owns. The seam is here so that when it gains + // one, nothing above this line changes. + ClientForPlatform() (client.Client, error) } -// NewIPAMClientFactory builds project-scoped clients from one connection. The +// NewIPAMClientFactory builds tenancy-scoped clients from one connection. The // clients are uncached, because a cache would watch every project served. -func NewIPAMClientFactory(base *rest.Config, scheme *runtime.Scheme) (IPAMClientFactory, error) { +// +// platformProject names the control plane platform-owned allocations are made +// in. Empty means the deployment allocates nothing platform-scoped. +func NewIPAMClientFactory(base *rest.Config, scheme *runtime.Scheme, platformProject string) (IPAMClientFactory, error) { if base == nil { return nil, fmt.Errorf("a rest config is required") } return &projectPathIPAMClientFactory{ - base: base, - scheme: scheme, - clients: map[string]client.Client{}, + base: base, + scheme: scheme, + platformProject: platformProject, + clients: map[string]client.Client{}, }, nil } type projectPathIPAMClientFactory struct { - base *rest.Config - scheme *runtime.Scheme + base *rest.Config + scheme *runtime.Scheme + platformProject string mu sync.Mutex clients map[string]client.Client } +func (f *projectPathIPAMClientFactory) ClientForPlatform() (client.Client, error) { + if f.platformProject == "" { + return nil, errNoPlatformTenancy + } + return f.ClientForProject(f.platformProject) +} + func (f *projectPathIPAMClientFactory) ClientForProject(project string) (client.Client, error) { if project == "" { return nil, errNoProject @@ -103,6 +127,10 @@ func IPAMScheme() (*runtime.Scheme, error) { var errNoProject = fmt.Errorf("no project") +// errNoPlatformTenancy says the deployment named no place for the platform to +// allocate what it owns, so it allocates none of it. +var errNoPlatformTenancy = fmt.Errorf("no platform tenancy is configured") + // projectFromNamespace reads the project a namespace belongs to. A namespace // that names no project resolves to nothing, never to a default. func projectFromNamespace(ns *corev1.Namespace) (string, error) { diff --git a/internal/controller/ipam_project_client_test.go b/internal/controller/ipam_project_client_test.go index 74103453..151ab011 100644 --- a/internal/controller/ipam_project_client_test.go +++ b/internal/controller/ipam_project_client_test.go @@ -18,7 +18,7 @@ func newTestIPAMClientFactory(t *testing.T, host string) *projectPathIPAMClientF t.Fatalf("building the IPAM scheme: %v", err) } - f, err := NewIPAMClientFactory(&rest.Config{Host: host}, scheme) + f, err := NewIPAMClientFactory(&rest.Config{Host: host}, scheme, testPlatformProject) if err != nil { t.Fatalf("building the factory: %v", err) } diff --git a/internal/controller/network_controller.go b/internal/controller/network_controller.go index 0915d811..d9b8b7c2 100644 --- a/internal/controller/network_controller.go +++ b/internal/controller/network_controller.go @@ -44,6 +44,15 @@ type NetworkReconciler struct { // addressed from. Empty means the same as a nil IPAM: nothing is claimed. PrefixClass string + // FabricIdentityClass is the IPClass that hands out the identity the fabric + // knows a network by. Empty means the same as a nil IPAM: no identity is + // allocated and a network is reconciled exactly as it was before. + FabricIdentityClass string + + // FabricIdentityNamespace is the namespace in the platform's own tenancy + // that identity claims are written to. + FabricIdentityNamespace string + mgr mcmanager.Manager finalizers finalizer.Finalizers } @@ -129,7 +138,27 @@ func (r *NetworkReconciler) reconcileNetwork( return ctrl.Result{}, nil } - return r.reconcilePrefix(ctx, cl, network) + // The identity comes first and is independent of the address space. A + // network that claims no addresses still spans locations, and the fabric + // still has to know it as one network there. + identityChanged, retryIdentity, err := r.reconcileFabricIdentity(ctx, cl, network) + if err != nil { + return ctrl.Result{}, err + } + if identityChanged { + if retryIdentity { + return ctrl.Result{RequeueAfter: rejectedClaimRetryInterval}, nil + } + // The status write wakes the controller again, and the next pass + // reconciles the address space against what was just recorded. + return ctrl.Result{}, nil + } + + result, err := r.reconcilePrefix(ctx, cl, network) + if err == nil && retryIdentity && result.RequeueAfter == 0 { + result.RequeueAfter = rejectedClaimRetryInterval + } + return result, err } // reconcilePrefix claims the network's IPv6 address space when the network is @@ -328,6 +357,18 @@ func setNetworkReady(network *networkingv1alpha.Network, message string) bool { } } + // A deployment that allocates no identity records no condition for one, so + // its networks read exactly as they did before. Where one is recorded and + // is not held, the network is not ready: it would reach a location the + // fabric cannot tell apart from another network. + if identity := apimeta.FindStatusCondition( + network.Status.Conditions, networkingv1alpha.NetworkFabricIdentityAllocated, + ); identity != nil && identity.Status != metav1.ConditionTrue && ready.Status == metav1.ConditionTrue { + ready.Status = identity.Status + ready.Reason = identity.Reason + ready.Message = identity.Message + } + return apimeta.SetStatusCondition(&network.Status.Conditions, ready) } diff --git a/internal/controller/network_fabric_identity.go b/internal/controller/network_fabric_identity.go new file mode 100644 index 00000000..98507496 --- /dev/null +++ b/internal/controller/network_fabric_identity.go @@ -0,0 +1,228 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package controller + +import ( + "context" + "encoding/binary" + "errors" + "fmt" + "net/netip" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" + + ipamv1alpha1 "go.miloapis.com/ipam/pkg/apis/ipam/v1alpha1" + + networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" +) + +const ( + // fabricIdentityBlockBits is the size of the block an identity is read out + // of. The pool roots a /32 and hands out /64s, so the bits between them are + // the block's index within the pool, and there are exactly 2^32 of them. + fabricIdentityBlockBits = 64 + + // fabricIdentityRootBits is where the pool's own prefix stops and the index + // begins. Reading the index from a fixed offset rather than from the pool's + // CIDR means the identity does not depend on an object this operator would + // otherwise have to read on every allocation. A pool rooted longer than a + // /32 simply leaves the leading bits of every index at zero, which is still + // unique within the one pool the platform allocates from. + fabricIdentityRootBits = 32 +) + +// reconcileFabricIdentity gives the network the identity the fabric knows it +// by, once, and never again. +// +// It reports whether it wrote anything and whether it wants to be called back. +// Allocation is independent of the network's address space: a network that +// claims no addresses still spans locations and still needs one identity there +// rather than a different one per location. +func (r *NetworkReconciler) reconcileFabricIdentity( + ctx context.Context, + cl client.Client, + network *networkingv1alpha.Network, +) (changed bool, retry bool, err error) { + if r.IPAM == nil || r.FabricIdentityClass == "" { + return false, false, nil + } + + // Allocated once. A network that already holds an identity is never asked + // again, which is what makes this idempotent and what makes the identity + // immutable in the only place that writes it. + if network.Status.FabricIdentity != 0 { + return r.reportFabricIdentity(ctx, cl, network, metav1.ConditionTrue, + networkingv1alpha.NetworkFabricIdentityReasonAllocated, + fmt.Sprintf("The fabric knows this network as %d", network.Status.FabricIdentity), + ), false, nil + } + + ipamClient, err := r.IPAM.ClientForPlatform() + if err != nil { + return r.reportFabricIdentity(ctx, cl, network, metav1.ConditionFalse, + networkingv1alpha.NetworkFabricIdentityReasonIdentitySpaceUnavailable, + "No platform identity space is configured, so the network has no identity on the fabric", + ), false, nil + } + + identity, err := r.claimFabricIdentity(ctx, ipamClient, network) + if err != nil { + var unusable *fabricIdentityUnusable + if errors.As(err, &unusable) { + return r.reportFabricIdentity(ctx, cl, network, metav1.ConditionFalse, + networkingv1alpha.NetworkFabricIdentityReasonIdentityUnusable, unusable.Error(), + ), true, nil + } + log.FromContext(ctx).Info("the network's fabric identity cannot be allocated", "error", err.Error()) + return r.reportFabricIdentity(ctx, cl, network, metav1.ConditionFalse, + networkingv1alpha.NetworkFabricIdentityReasonIdentitySpaceUnavailable, err.Error(), + ), true, nil + } + + network.Status.FabricIdentity = identity + apimeta.SetStatusCondition(&network.Status.Conditions, metav1.Condition{ + Type: networkingv1alpha.NetworkFabricIdentityAllocated, + Status: metav1.ConditionTrue, + Reason: networkingv1alpha.NetworkFabricIdentityReasonAllocated, + ObservedGeneration: network.Generation, + Message: fmt.Sprintf("The fabric knows this network as %d", identity), + }) + + if err := cl.Status().Update(ctx, network); err != nil { + return false, false, fmt.Errorf("failed publishing the network's fabric identity: %w", err) + } + return true, false, nil +} + +// claimFabricIdentity holds one block of the identifier space and reads the +// identity out of it. +// +// The claim's name is derived from the network's UID, so a reconcile that lost +// its answer finds the same block again instead of taking a second one, and a +// network deleted and recreated under the same name is a different network with +// a different identity. IPAM binds on create and refuses a duplicate name, so +// the read comes first. +func (r *NetworkReconciler) claimFabricIdentity( + ctx context.Context, + ipamClient client.Client, + network *networkingv1alpha.Network, +) (int64, error) { + ipClaim := &ipamv1alpha1.IPClaim{} + ipClaim.Namespace = r.FabricIdentityNamespace + ipClaim.Name = fabricIdentityClaimName(network) + ipClaim.Spec = ipamv1alpha1.IPClaimSpec{ + ClassName: r.FabricIdentityClass, + Target: ipamv1alpha1.TargetBlock, + PrefixLength: ptr.To(int32(fabricIdentityBlockBits)), + + // An identity is never given back. A Route Target still installed in a + // remote location's import policy would silently merge a new network + // into a dead one's routes, so holding the block forever is the safe + // failure and reissuing it is not a failure anything can see. + ReclaimPolicy: ipamv1alpha1.ReclaimRetain, + } + + existing := &ipamv1alpha1.IPClaim{} + getErr := ipamClient.Get(ctx, client.ObjectKeyFromObject(ipClaim), existing) + if getErr != nil && !apierrors.IsNotFound(getErr) { + return 0, fmt.Errorf("failed reading the identity claim %q: %w", ipClaim.Name, getErr) + } + + if getErr == nil { + ipClaim = existing + } else if createErr := ipamClient.Create(ctx, ipClaim); createErr != nil { + // The create can still lose a race with another writer, so ask again + // before calling this a failure to allocate. + raced := &ipamv1alpha1.IPClaim{} + if err := ipamClient.Get(ctx, client.ObjectKeyFromObject(ipClaim), raced); err != nil { + return 0, fmt.Errorf("failed claiming a fabric identity: %w", createErr) + } + ipClaim = raced + } + + if ipClaim.Status.AllocatedCIDR == "" { + return 0, fmt.Errorf("the identity space allocated nothing for this network (phase %q)", ipClaim.Status.Phase) + } + + return fabricIdentityFromBlock(ipClaim.Status.AllocatedCIDR) +} + +// fabricIdentityFromBlock reads the identity out of the block the identifier +// space handed out. The block's index within the pool is the identity, and the +// index is the 32 bits between the pool's root and the block, which is exactly +// the width that survives into the Route Target. +func fabricIdentityFromBlock(cidr string) (int64, error) { + prefix, err := netip.ParsePrefix(cidr) + if err != nil { + return 0, &fabricIdentityUnusable{message: fmt.Sprintf( + "the identity space answered with %q, which is not a prefix", cidr)} + } + + address := prefix.Addr() + if !address.Is6() || address.Is4In6() { + return 0, &fabricIdentityUnusable{message: fmt.Sprintf( + "the identity space answered with %q; identifiers are read out of an IPv6 space", cidr)} + } + + // Shorter than a /64 and two networks could be handed blocks that share an + // index; longer and one block's index is not the whole of it. + if prefix.Bits() != fabricIdentityBlockBits { + return 0, &fabricIdentityUnusable{message: fmt.Sprintf( + "the identity space answered with %q; identifiers are read out of a /%d", + cidr, fabricIdentityBlockBits)} + } + + octets := address.As16() + identity := int64(binary.BigEndian.Uint32(octets[fabricIdentityRootBits/8 : fabricIdentityBlockBits/8])) + if identity == 0 { + return 0, &fabricIdentityUnusable{message: fmt.Sprintf( + "the identity space answered with %q, whose index is zero; zero is what an unallocated network reads as, so the pool must not hand out its first block", + cidr)} + } + return identity, nil +} + +// fabricIdentityUnusable says the identity space answered, and its answer +// cannot be turned into an identity. Retrying reaches the same block, so this +// is a wait on an operator rather than on the service. +type fabricIdentityUnusable struct { + message string +} + +func (e *fabricIdentityUnusable) Error() string { return e.message } + +// reportFabricIdentity records why the network does or does not carry an +// identity, and reports whether that changed anything. +func (r *NetworkReconciler) reportFabricIdentity( + ctx context.Context, + cl client.Client, + network *networkingv1alpha.Network, + status metav1.ConditionStatus, + reason string, + message string, +) bool { + if !apimeta.SetStatusCondition(&network.Status.Conditions, metav1.Condition{ + Type: networkingv1alpha.NetworkFabricIdentityAllocated, + Status: status, + Reason: reason, + ObservedGeneration: network.Generation, + Message: message, + }) { + return false + } + + if err := cl.Status().Update(ctx, network); err != nil { + log.FromContext(ctx).Error(err, "failed recording the network's fabric identity") + return false + } + return true +} + +func fabricIdentityClaimName(network *networkingv1alpha.Network) string { + return "fabric-identity-" + string(network.UID) +} diff --git a/internal/controller/network_fabric_identity_test.go b/internal/controller/network_fabric_identity_test.go new file mode 100644 index 00000000..41ca471a --- /dev/null +++ b/internal/controller/network_fabric_identity_test.go @@ -0,0 +1,190 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package controller + +import ( + "testing" + + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" +) + +// The class name a deployment configures for the identifier space. It is +// configuration rather than a constant for the same reason the prefix class is. +const testFabricIdentityClass = "datum-fabric-identity" + +func (s *networkScenario) allocatesFabricIdentity() *networkScenario { + s.t.Helper() + s.reconciler.FabricIdentityClass = testFabricIdentityClass + s.reconciler.FabricIdentityNamespace = testPlatformNamespace + return s +} + +func (s *networkScenario) identityCondition() *metav1.Condition { + s.t.Helper() + for _, condition := range s.get().Status.Conditions { + if condition.Type == networkingv1alpha.NetworkFabricIdentityAllocated { + return &condition + } + } + return nil +} + +func TestNetworkIsGivenAFabricIdentityWhenCreated(t *testing.T) { + s := newNetworkScenario(t, newFakeIPAM(t)).allocatesFabricIdentity() + + network := s.createNetwork(networkingv1alpha.IPv6Protocol) + s.reconcile() + + allocated := s.get() + require.NotZero(t, allocated.Status.FabricIdentity, + "the network must carry the identity the fabric knows it by") + + condition := s.identityCondition() + require.NotNil(t, condition) + require.Equal(t, metav1.ConditionTrue, condition.Status) + require.Equal(t, networkingv1alpha.NetworkFabricIdentityReasonAllocated, condition.Reason) + + require.Contains(t, s.ipam.created()[testPlatformProject], fabricIdentityClaimName(network), + "the identity must be allocated in the platform's own tenancy, not the consumer's project") + require.NotContains(t, s.ipam.created()[testProject], fabricIdentityClaimName(network)) +} + +// Reconciling again must find the identity it already has rather than take a +// second one. An identity taken twice is address space burned and, worse, a +// network whose Route Target moved out from under every location carrying it. +func TestFabricIdentityIsAllocatedOnlyOnce(t *testing.T) { + s := newNetworkScenario(t, newFakeIPAM(t)).allocatesFabricIdentity() + + network := s.createNetwork(networkingv1alpha.IPv6Protocol) + s.reconcile() + first := s.get().Status.FabricIdentity + require.NotZero(t, first) + + for range 3 { + s.reconcile() + } + + require.Equal(t, first, s.get().Status.FabricIdentity, "the identity must not change") + require.Equal(t, []string{fabricIdentityClaimName(network)}, + s.ipam.created()[testPlatformProject], + "a network already holding an identity must not ask for another") +} + +// Two networks are two identities. One shared between them would make them one +// network on the fabric: each would import the other's routes. +func TestEachNetworkGetsItsOwnFabricIdentity(t *testing.T) { + ipam := newFakeIPAM(t) + + first := newNetworkScenario(t, ipam).allocatesFabricIdentity() + first.createNetwork(networkingv1alpha.IPv6Protocol) + first.reconcile() + + second := newNetworkScenario(t, ipam).allocatesFabricIdentity() + second.createNetwork(networkingv1alpha.IPv6Protocol) + second.reconcile() + + require.NotEqual(t, first.get().Status.FabricIdentity, second.get().Status.FabricIdentity) +} + +// The API refuses to move an identity that is already allocated, so no writer +// can change it whatever it believes. +func TestFabricIdentityIsImmutableOnceAllocated(t *testing.T) { + s := newNetworkScenario(t, newFakeIPAM(t)).allocatesFabricIdentity() + + s.createNetwork(networkingv1alpha.IPv6Protocol) + s.reconcile() + + allocated := s.get() + require.NotZero(t, allocated.Status.FabricIdentity) + + allocated.Status.FabricIdentity += 1 + err := s.client.Status().Update(s.ctx, allocated) + require.Error(t, err, "the identity must not be movable") + require.Contains(t, err.Error(), "immutable") +} + +// A deployment that names no identifier space reconciles a network exactly as +// it did before one existed: no identity, and no condition claiming one is +// missing. +func TestNetworkWithoutAnIdentitySpaceIsUnchanged(t *testing.T) { + s := newNetworkScenario(t, newFakeIPAM(t)) + + s.createNetwork(networkingv1alpha.IPv6Protocol) + s.reconcile() + + require.Zero(t, s.get().Status.FabricIdentity) + require.Nil(t, s.identityCondition()) + require.Equal(t, metav1.ConditionTrue, s.readyCondition().Status, + "a network the platform allocates no identity for is still ready") +} + +// A network that claims no address space still spans locations, so it still +// needs one identity there rather than a different one per location. +func TestFabricIdentityIsAllocatedForANetworkWithNoAddressSpace(t *testing.T) { + s := newNetworkScenario(t, newFakeIPAM(t)).allocatesFabricIdentity() + + s.createNetwork(networkingv1alpha.IPv4Protocol) + s.reconcile() + + require.NotZero(t, s.get().Status.FabricIdentity) +} + +// Zero is what an unallocated network reads as, so a network handed the pool's +// own first block must be refused rather than published as if it held nothing. +func TestFabricIdentityRefusesTheZeroBlock(t *testing.T) { + ipam := newFakeIPAM(t) + ipam.identityBase = 0 + + s := newNetworkScenario(t, ipam).allocatesFabricIdentity() + s.createNetwork(networkingv1alpha.IPv6Protocol) + s.reconcile() + + require.Zero(t, s.get().Status.FabricIdentity) + + condition := s.identityCondition() + require.NotNil(t, condition) + require.Equal(t, metav1.ConditionFalse, condition.Status) + require.Equal(t, networkingv1alpha.NetworkFabricIdentityReasonIdentityUnusable, condition.Reason) + + require.NotEqual(t, metav1.ConditionTrue, s.readyCondition().Status, + "a network the fabric cannot tell apart from another is not ready") +} + +func TestFabricIdentityIsTheBlocksIndexInThePool(t *testing.T) { + for _, tc := range []struct { + name string + cidr string + want int64 + wants string + }{ + {name: "the first usable block", cidr: "fc00:0:0:1::/64", want: 1}, + {name: "an index spanning both halves", cidr: "fc00:0:1234:5678::/64", want: 0x12345678}, + {name: "the last block in the pool", cidr: "fc00:0:ffff:ffff::/64", want: 0xffffffff}, + {name: "a pool rooted longer than a /32", cidr: "fc00:0:0:beef::/64", want: 0xbeef}, + + {name: "the pool's own zero block", cidr: "fc00::/64", wants: "index is zero"}, + {name: "a block wider than a /64", cidr: "fc00:0:0:1::/48", wants: "read out of a /64"}, + {name: "a block narrower than a /64", cidr: "fc00:0:0:1::/96", wants: "read out of a /64"}, + {name: "an IPv4 block", cidr: "10.0.0.0/24", wants: "read out of an IPv6 space"}, + {name: "not a prefix at all", cidr: "fc00::", wants: "not a prefix"}, + } { + t.Run(tc.name, func(t *testing.T) { + identity, err := fabricIdentityFromBlock(tc.cidr) + if tc.wants != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tc.wants) + return + } + require.NoError(t, err) + require.Equal(t, tc.want, identity) + + // The width the fabric carries is the whole of it. A value that + // needed more than 32 bits would be uniqueness the platform + // believes it has and the Route Target does not. + require.LessOrEqual(t, identity, int64(0xffffffff)) + }) + } +} diff --git a/internal/controller/networkinterfaceclaim_controller_test.go b/internal/controller/networkinterfaceclaim_controller_test.go index 342f6d38..64d461b7 100644 --- a/internal/controller/networkinterfaceclaim_controller_test.go +++ b/internal/controller/networkinterfaceclaim_controller_test.go @@ -52,6 +52,11 @@ const ( testLocationName = "us-central-1" testLocationNS = "datum-locations" testPublicV4Class = "datum-public-v4" + + // The platform's own tenancy, where what it allocates for every consumer is + // allocated rather than inside any one of them. + testPlatformProject = "platform" + testPlatformNamespace = "default" ) // fakeIPAM stands in for the IPAM API server. Allocation is synchronous there, @@ -95,10 +100,15 @@ type fakeIPAM struct { // claim for the same network reads the same range back. prefixRanges map[string]string - nextV4 int - nextV6 int - nextPrefix int - nextSubnet int + nextV4 int + nextV6 int + nextPrefix int + nextSubnet int + nextIdentity int + + // identityBase is the index the identifier pool starts handing out at. Zero + // stands in for a pool that was not told to hold its first block back. + identityBase int } func newFakeIPAM(t *testing.T, classes ...*ipamv1alpha1.IPClass) *fakeIPAM { @@ -117,9 +127,18 @@ func newFakeIPAM(t *testing.T, classes ...*ipamv1alpha1.IPClass) *fakeIPAM { failOn: map[string]error{}, failReleaseOn: map[string]error{}, prefixRanges: map[string]string{}, + + // A correctly provisioned identifier pool holds its own first block + // back, because a network handed index zero reads as one holding no + // identity at all. + identityBase: 1, } } +func (f *fakeIPAM) ClientForPlatform() (client.Client, error) { + return f.ClientForProject(testPlatformProject) +} + func (f *fakeIPAM) ClientForProject(project string) (client.Client, error) { f.mu.Lock() defer f.mu.Unlock() @@ -269,6 +288,17 @@ func (f *fakeIPAM) allocateLocked(project string, ipClaim *ipamv1alpha1.IPClaim) // The real server writes only allocatedCIDR, host prefixes included, and // never status.address. ipClaim.Status.Phase = ipamv1alpha1.ClaimBound + if ipClaim.Spec.PrefixLength != nil { + // A claim naming a block size gets a block of it, carved out of a pool + // rooted at a /32. Only the identifier space asks for one. + index := f.identityBase + f.nextIdentity + f.nextIdentity++ + ipClaim.Status.AllocatedCIDR = fmt.Sprintf("fc00:0:%x:%x::/%d", + index>>16, index&0xffff, *ipClaim.Spec.PrefixLength) + ipClaim.Status.PoolRef = &ipamv1alpha1.LocalRef{Name: "pool-" + project} + ipClaim.Status.BoundAllocationRef = &ipamv1alpha1.LocalRef{Name: allocationNameFor(ipClaim.Name)} + return + } if family == ipamv1alpha1.IPv6 { f.nextV6++ ipClaim.Status.AllocatedCIDR = fmt.Sprintf("2001:db8:a000:%d::/96", f.nextV6) diff --git a/internal/controller/networkpresence_controller.go b/internal/controller/networkpresence_controller.go index 1e307806..47f25638 100644 --- a/internal/controller/networkpresence_controller.go +++ b/internal/controller/networkpresence_controller.go @@ -429,6 +429,13 @@ func (r *NetworkPresenceReconciler) project( networkContext.Spec.Location = pair.Location networkContext.Spec.IPFamilies = append([]networkingv1alpha.IPFamily(nil), network.Spec.IPFamilies...) networkContext.Spec.MTU = network.Spec.MTU + + // Projected from status rather than spec, because that is where it is + // allocated. A network without one projects zero, which is what every + // context written before the identity existed already reads as, so a + // location that has not been given one behaves exactly as it does today. + networkContext.Spec.FabricIdentity = network.Status.FabricIdentity + networkContext.Spec.NetworkGeneration = network.Generation return controllerutil.SetControllerReference(network, networkContext, projectClient.Scheme()) diff --git a/internal/controller/networkpresence_controller_test.go b/internal/controller/networkpresence_controller_test.go index 3ddbcb23..45a95a93 100644 --- a/internal/controller/networkpresence_controller_test.go +++ b/internal/controller/networkpresence_controller_test.go @@ -760,3 +760,70 @@ func TestNetworkPresenceIsTornDownOnceTheGraceExpires(t *testing.T) { _, ok = s.networkContext() require.False(t, ok, "nothing has declared this presence for longer than the wait") } + +// The identity is allocated into the network's status, and a location reads it +// out of the context's spec. Nothing else carries it there: cells cannot reach +// project control planes, and propagation to them strips status. +func TestNetworkPresenceProjectsTheFabricIdentity(t *testing.T) { + s := newPresenceScenario(t, presenceOptions{}) + s.createBinding("consumer-a") + s.reconcile() + + networkContext, ok := s.networkContext() + require.True(t, ok) + require.Zero(t, networkContext.Spec.FabricIdentity, + "a network holding no identity projects none") + + s.network.Status.FabricIdentity = 0x12345678 + require.NoError(t, s.hub.Status().Update(s.ctx, s.network)) + + s.reconcile() + + networkContext, ok = s.networkContext() + require.True(t, ok) + require.Equal(t, int64(0x12345678), networkContext.Spec.FabricIdentity, + "the location must read the same identity every other location reads") +} + +// Every location of one network reads the same identity. That is the whole +// point: two locations that chose their own are two networks on the fabric. +func TestNetworkPresenceProjectsTheSameIdentityIntoEveryLocation(t *testing.T) { + s := newPresenceScenario(t, presenceOptions{}) + + s.network.Status.FabricIdentity = 4242 + require.NoError(t, s.hub.Status().Update(s.ctx, s.network)) + + s.createBinding("consumer-a") + s.createBinding("consumer-b") + s.reconcile() + + networkContext, ok := s.networkContext() + require.True(t, ok) + require.Equal(t, int64(4242), networkContext.Spec.FabricIdentity) + + other := s.inLocation(s.locationName + "-east") + other.createBinding("consumer-c") + other.reconcile() + + otherContext, ok := other.networkContext() + require.True(t, ok) + require.Equal(t, networkContext.Spec.FabricIdentity, otherContext.Spec.FabricIdentity) +} + +// inLocation is the same network in a second location, which is the case the +// per-location identifier fails at: the network is one network and the fabric +// has to agree. +func (s *presenceScenario) inLocation(location string) *presenceScenario { + s.t.Helper() + + locationBinding := &networkingv1alpha.LocationBinding{} + locationBinding.Name = location + locationBinding.Spec.LocationRef = corev1.LocalObjectReference{Name: location} + locationBinding.Spec.LocationClassName = "datum-managed" + require.NoError(s.t, s.hub.Create(s.ctx, locationBinding)) + s.t.Cleanup(func() { _ = s.hub.Delete(s.ctx, locationBinding) }) + + other := *s + other.locationName = location + return &other +}