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 +}