From 3afca438418811007cbe11c8cdce9684fb1e57f7 Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Wed, 26 Aug 2026 18:45:40 -0500 Subject: [PATCH] feat: derive VPC identifier from fabric identity Each cell runs its own copy of the NetworkContext reconciler, and each one drew a random VPC identifier checked only against the VPCs in its own cluster. One network spanning two locations therefore ended up with two unrelated identifiers, which galactic reads as two different networks: the edge VRF device is named from the VPC alone, and the Route Target is derived from it, so neither location imports or exports the other's routes. When the network carries an identity allocated for it, the VPC identifier is now rendered from that value, so every cell reaches the same one. A network without one keeps drawing a random identifier exactly as before, and a VPC that already holds an identifier is never renumbered. Key changes: - Add identifier.VPCBase62 to render a known 48-bit value, alongside the existing random draw - Read the allocated identity from NetworkContext spec and derive the identifier from it when present - Fall back to the existing random allocation when no identity is present --- cmd/main.go | 2 +- .../controller/networkcontext_controller.go | 76 +++++- .../networkcontext_controller_test.go | 223 ++++++++++++++++++ internal/identifier/identifier.go | 11 + 4 files changed, 309 insertions(+), 3 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index 6e55155..27f23ad 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -112,7 +112,7 @@ func main() { } if err := (&controller.NetworkContextReconciler{ - Client: mgr.GetClient(), Scheme: mgr.GetScheme(), + Client: mgr.GetClient(), Scheme: mgr.GetScheme(), APIReader: mgr.GetAPIReader(), }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "NetworkContext") os.Exit(1) diff --git a/internal/controller/networkcontext_controller.go b/internal/controller/networkcontext_controller.go index bddb95f..c872858 100644 --- a/internal/controller/networkcontext_controller.go +++ b/internal/controller/networkcontext_controller.go @@ -25,6 +25,7 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -40,7 +41,8 @@ import ( // identifier the whole galactic fabric keys on. type NetworkContextReconciler struct { client.Client - Scheme *runtime.Scheme + Scheme *runtime.Scheme + APIReader client.Reader } // +kubebuilder:rbac:groups=networking.datumapis.com,resources=networkcontexts,verbs=get;list;watch @@ -83,7 +85,7 @@ func (r *NetworkContextReconciler) Reconcile(ctx context.Context, req ctrl.Reque } if vpc.Status.VPC == "" { - allocated, err := r.allocateVPCIdentifier(ctx) + allocated, err := r.vpcIdentifier(ctx, &networkContext) if err != nil { return ctrl.Result{}, err } @@ -142,6 +144,76 @@ func subnetRange(subnet *networkingv1alpha.Subnet) (string, int32, bool) { return "", 0, false } +// vpcIdentifier resolves the identifier for a VPC that does not have one yet. +// A network whose fabric identity has been allocated for it derives its +// identifier from that value, so every location holding the same network +// arrives at the same one. A network with no allocated identity keeps the +// original behaviour and draws a random identifier for this cell. +func (r *NetworkContextReconciler) vpcIdentifier( + ctx context.Context, networkContext *networkingv1alpha.NetworkContext, +) (string, error) { + fabricIdentity, err := r.fabricIdentity(ctx, networkContext) + if err != nil { + return "", err + } + if fabricIdentity == 0 { + return r.allocateVPCIdentifier(ctx) + } + return vpcIdentifierFor(fabricIdentity) +} + +// vpcIdentifierFor renders an allocated fabric identity as the base62 VPC +// identifier the galactic data plane keys on. The rendering is total and +// deterministic: the same identity yields the same identifier in every cell. +func vpcIdentifierFor(fabricIdentity int64) (string, error) { + if fabricIdentity < 0 { + return "", fmt.Errorf("fabric identity %d is negative", fabricIdentity) + } + rendered, err := identifier.VPCBase62(uint64(fabricIdentity)) + if err != nil { + return "", fmt.Errorf("render fabric identity %d: %w", fabricIdentity, err) + } + return rendered, nil +} + +// fabricIdentity reads the identity allocated for the network this context +// belongs to, or zero when none has been allocated. +// +// The field is read untyped because the Go type in the pinned +// network-services-operator release does not carry it yet, and a typed client +// discards fields its struct does not know: the value would read as absent +// every time. Reading it directly means this cell picks the identity up as +// soon as it is published, with no release ordering between the two repos. +// Once a release carrying the field is pinned, this collapses to a field read. +func (r *NetworkContextReconciler) fabricIdentity( + ctx context.Context, networkContext *networkingv1alpha.NetworkContext, +) (int64, error) { + reader := r.APIReader + if reader == nil { + reader = r.Client + } + + raw := &unstructured.Unstructured{} + raw.SetGroupVersionKind(networkingv1alpha.GroupVersion.WithKind("NetworkContext")) + if err := reader.Get(ctx, client.ObjectKeyFromObject(networkContext), raw); err != nil { + return 0, fmt.Errorf("read NetworkContext %s: %w", networkContext.Name, err) + } + return fabricIdentityFrom(raw.Object) +} + +// fabricIdentityFrom extracts spec.fabricIdentity from an untyped +// NetworkContext, treating an absent field as no allocated identity. +func fabricIdentityFrom(object map[string]any) (int64, error) { + value, found, err := unstructured.NestedInt64(object, "spec", "fabricIdentity") + if err != nil { + return 0, fmt.Errorf("read spec.fabricIdentity: %w", err) + } + if !found { + return 0, nil + } + return value, nil +} + // allocateVPCIdentifier draws a random 48-bit identifier not already in use. // A single leader-elected controller is the only writer, so a list plus a // collision check serializes correctly. diff --git a/internal/controller/networkcontext_controller_test.go b/internal/controller/networkcontext_controller_test.go index 5734222..365d562 100644 --- a/internal/controller/networkcontext_controller_test.go +++ b/internal/controller/networkcontext_controller_test.go @@ -18,8 +18,19 @@ along with this program. If not, see . package controller import ( + "context" "testing" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + cloudv1alpha1 "go.datum.net/cloud/api/v1alpha1" + "go.datum.net/cloud/internal/identifier" networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" ) @@ -50,3 +61,215 @@ func TestSubnetRangePrefersStatusAndFallsBackToSpec(t *testing.T) { t.Fatal("an unallocated subnet should yield nothing") } } + +// A network allocated a fabric identity must resolve to the same VPC +// identifier in every cell that holds it. Two cells reaching different +// identifiers is the defect this replaces: the edge VRF device is named from +// the VPC alone, and the Route Target is derived from it, so a network whose +// locations disagree neither shares a device nor exchanges routes. +func TestFabricIdentityDerivesTheSameVPCIdentifierInEveryCell(t *testing.T) { + const allocated int64 = 0x1A2B3C4D5E6F + + first, err := vpcIdentifierFor(allocated) + if err != nil { + t.Fatalf("derive in the first cell: %v", err) + } + second, err := vpcIdentifierFor(allocated) + if err != nil { + t.Fatalf("derive in the second cell: %v", err) + } + if first != second { + t.Fatalf("two cells derived %q and %q from the same identity", first, second) + } + + other, err := vpcIdentifierFor(allocated + 1) + if err != nil { + t.Fatalf("derive a neighbouring identity: %v", err) + } + if other == first { + t.Fatalf("distinct identities both derived %q", first) + } +} + +// The identifier lands in a nine-character slot in the galactic VRF device +// name, so nothing the allocator can hand out may render wider than that. +func TestDerivedVPCIdentifierFitsTheVRFDeviceName(t *testing.T) { + for _, allocated := range []int64{1, 2, 1000, 1 << 24, int64(identifier.MaxVPC) - 1} { + derived, err := vpcIdentifierFor(allocated) + if err != nil { + t.Fatalf("derive %d: %v", allocated, err) + } + if len(derived) > 9 { + t.Fatalf("identity %d rendered %d characters, wider than the slot holds", allocated, len(derived)) + } + } +} + +// Values the fabric cannot represent are refused rather than folded into +// something that collides with another network's identifier. +func TestUnrepresentableFabricIdentityIsRefused(t *testing.T) { + for _, allocated := range []int64{-1, int64(identifier.MaxVPC), int64(identifier.MaxVPC) + 1} { + if _, err := vpcIdentifierFor(allocated); err == nil { + t.Fatalf("identity %d should have been refused", allocated) + } + } +} + +// Networks that predate the allocator carry no identity, and a context written +// before the field existed carries nothing either. Both read as unallocated. +func TestAbsentFabricIdentityReadsAsUnallocated(t *testing.T) { + absent, err := fabricIdentityFrom(map[string]any{"spec": map[string]any{}}) + if err != nil || absent != 0 { + t.Fatalf("an unset field should read as 0, got %d err=%v", absent, err) + } + + noSpec, err := fabricIdentityFrom(map[string]any{}) + if err != nil || noSpec != 0 { + t.Fatalf("a context with no spec should read as 0, got %d err=%v", noSpec, err) + } + + present, err := fabricIdentityFrom(map[string]any{ + "spec": map[string]any{"fabricIdentity": int64(4242)}, + }) + if err != nil || present != 4242 { + t.Fatalf("a projected identity should read back, got %d err=%v", present, err) + } + + if _, err := fabricIdentityFrom(map[string]any{ + "spec": map[string]any{"fabricIdentity": "not-a-number"}, + }); err == nil { + t.Fatal("a non-integer identity should be refused") + } +} + +func fabricTestScheme(t *testing.T) *runtime.Scheme { + t.Helper() + s := runtime.NewScheme() + if err := cloudv1alpha1.AddToScheme(s); err != nil { + t.Fatalf("register cloud types: %v", err) + } + if err := networkingv1alpha.AddToScheme(s); err != nil { + t.Fatalf("register networking types: %v", err) + } + return s +} + +// projectedIdentityReader serves the NetworkContext as it appears on the wire, +// carrying the projected identity. The fake client stores objects through their +// registered Go type, which discards a field that type does not carry yet — +// the very reason the reconciler reads this field untyped. +type projectedIdentityReader struct { + client.Reader + fabricIdentity int64 +} + +func (r projectedIdentityReader) Get( + ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption, +) error { + raw, ok := obj.(*unstructured.Unstructured) + if !ok { + return r.Reader.Get(ctx, key, obj, opts...) + } + raw.Object = map[string]any{"spec": map[string]any{}} + if r.fabricIdentity != 0 { + raw.Object["spec"] = map[string]any{"fabricIdentity": r.fabricIdentity} + } + return nil +} + +func networkContextWithSubnet() []client.Object { + networkContext := &networkingv1alpha.NetworkContext{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default-us-central-1", + Namespace: "project-a", + UID: "0c1d2e3f-4a5b-6c7d-8e9f-0a1b2c3d4e5f", + }, + Spec: networkingv1alpha.NetworkContextSpec{ + Network: networkingv1alpha.LocalNetworkRef{Name: "default"}, + }, + } + + subnet := &networkingv1alpha.Subnet{ + ObjectMeta: metav1.ObjectMeta{Name: "default-v6", Namespace: "project-a"}, + Spec: networkingv1alpha.SubnetSpec{ + NetworkContext: networkingv1alpha.LocalNetworkContextRef{Name: "default-us-central-1"}, + StartAddress: "fd00::", + PrefixLength: 48, + }, + } + return []client.Object{networkContext, subnet} +} + +func reconcileNetworkContext( + t *testing.T, fabricIdentity int64, objects ...client.Object, +) *cloudv1alpha1.VPC { + t.Helper() + s := fabricTestScheme(t) + c := fake.NewClientBuilder(). + WithScheme(s). + WithObjects(objects...). + WithStatusSubresource(&cloudv1alpha1.VPC{}). + Build() + + r := &NetworkContextReconciler{ + Client: c, Scheme: s, + APIReader: projectedIdentityReader{Reader: c, fabricIdentity: fabricIdentity}, + } + request := ctrl.Request{NamespacedName: types.NamespacedName{ + Name: "default-us-central-1", Namespace: "project-a", + }} + if _, err := r.Reconcile(context.Background(), request); err != nil { + t.Fatalf("reconcile: %v", err) + } + + vpc := &cloudv1alpha1.VPC{} + if err := c.Get(context.Background(), request.NamespacedName, vpc); err != nil { + t.Fatalf("read back the VPC: %v", err) + } + return vpc +} + +// The allocated identity is what the VPC ends up carrying, which is the whole +// point: every cell reconciling this network reaches the same identifier. +func TestReconcileUsesTheAllocatedFabricIdentity(t *testing.T) { + const allocated int64 = 0x1A2B3C4D5E6F + + expected, err := vpcIdentifierFor(allocated) + if err != nil { + t.Fatalf("derive the expected identifier: %v", err) + } + + vpc := reconcileNetworkContext(t, allocated, networkContextWithSubnet()...) + if vpc.Status.VPC != expected { + t.Fatalf("VPC carries %q, want the derived %q", vpc.Status.VPC, expected) + } +} + +// Nothing has allocated identities yet and existing networks have none, so a +// context without one must still get an identifier exactly as it does today. +func TestReconcileFallsBackToARandomIdentifierWithoutAnAllocatedIdentity(t *testing.T) { + vpc := reconcileNetworkContext(t, 0, networkContextWithSubnet()...) + if vpc.Status.VPC == "" { + t.Fatal("a context with no allocated identity should still receive an identifier") + } + if _, err := identifier.Base62ToHex(vpc.Status.VPC); err != nil { + t.Fatalf("identifier %q is not base62: %v", vpc.Status.VPC, err) + } +} + +// A VPC already carrying an identifier keeps it. Rewriting one renames the +// edge VRF device and changes the Route Target under running traffic. +func TestReconcileLeavesAnAlreadyAllocatedVPCAlone(t *testing.T) { + const existing = "3fA2bQ71x" + + objects := append(networkContextWithSubnet(), &cloudv1alpha1.VPC{ + ObjectMeta: metav1.ObjectMeta{Name: "default-us-central-1", Namespace: "project-a"}, + Spec: cloudv1alpha1.VPCSpec{Networks: []cloudv1alpha1.Network{"fd00::/48"}}, + Status: cloudv1alpha1.VPCStatus{VPC: existing}, + }) + + vpc := reconcileNetworkContext(t, 0x1A2B3C4D5E6F, objects...) + if vpc.Status.VPC != existing { + t.Fatalf("a live VPC was renumbered from %q to %q", existing, vpc.Status.VPC) + } +} diff --git a/internal/identifier/identifier.go b/internal/identifier/identifier.go index 5360aa8..ff26c19 100644 --- a/internal/identifier/identifier.go +++ b/internal/identifier/identifier.go @@ -80,6 +80,17 @@ func Base62ToHex(value string) (string, error) { return baseconv.Convert(value, baseconv.Digits62, baseconv.DigitsHex) } +// VPCBase62 renders a specific 48-bit VPC identifier in base62. Callers that +// hold an identifier allocated elsewhere use this instead of drawing a random +// one, so every cell rendering the same value produces the same identifier. +func VPCBase62(value uint64) (string, error) { + hex, err := Hex(value, MaxVPC) + if err != nil { + return "", err + } + return HexToBase62(hex) +} + // RandomVPCBase62 returns a random VPC identifier in base62. func RandomVPCBase62() (string, error) { hex, err := RandomVPC()