From a643e236d4086314833d327bf32e3915d8130eed Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Thu, 17 Sep 2026 17:49:39 -0500 Subject: [PATCH 1/6] feat: Claim an egress shard's address from IPAM An egress shard no longer waits for an operator to hand-write the public address it translates to. A controller in the cell claims one address per shard from the addressing service and writes it into the shard's spec, where the data plane already reads it. The shard does not claim its own address. A shard runs on every translating node, including hardware at the edge, and the process that would make the claim is the one serving the datapath -- so the credential would sit on every such node, reachable from the process that also handles tenant packets, and the allocation would sit beside the path that attaches a workload. One controller per cell moves the credential count from the number of translating nodes to the number of cells and takes the allocation off that path: an address is claimed when a node is commissioned, not when a workload arrives. It is a binary of its own rather than a reconciler inside vpc-controller, following the split fabric-identity-controller already establishes for a controller needing an addressing credential. vpc-controller writes the attachment state of every workload in the cell and serves an admission webhook; a credential into the platform's own tenancy widens the blast radius of the one component a cell cannot run without, and one that expired would stop workloads attaching. spec.shardAddressIPv6 is write-once, so nothing here writes a value that is not an address the service has already bound. Every path that cannot produce one leaves the field untouched and retries. Key changes: - Add internal/egressaddress, which holds one address per shard under a claim named for that shard. Attribution rides on the name and on annotations because the service overwrites spec.ownerRef with the requesting project's identity - Add EgressShardAddressReconciler, which assigns the address and the IPv6 family label the selectors placing traffic on a shard match, and releases the claim only for a shard that is actually gone - Read the address from status.allocatedCIDR alone. The API carries a status.address holding the single-address form and no released version of the service writes it - Adopt a retained allocation the service refuses a same-named claim with, so an address held by an earlier claim is read rather than lost behind a conflict that never clears (milo-os/ipam #107) - Claim with reclaimPolicy Delete. Retain would hand a recreated shard the same address back and defeat the only remedy the write-once field leaves for a wrong one, and would never return the address at all - Add config/ipam/egress-shard-ipv6.yaml, the pool and class chain an operator applies. One pool per location, keyed on location rather than cell because two cells already serve one location. IPv6 only: the platform pairs no resolver with a translator, so an IPv4 egress address would be reachability the API reports and the data plane does not deliver - Leave the root pool's cidr empty. The service refuses a root pool without one, so applying the file unedited is rejected rather than seeding a permanent aggregate from a guess This cannot build until go.datum.net/network publishes the EgressShard spec address fields and label keys, which are on an unpushed branch. A local go.work resolves them for development and is gitignored; a committed replace directive would break the build for every checkout and would still need reverting after the dependency ships. Co-Authored-By: Claude Opus 5 (1M context) --- Dockerfile | 2 + cmd/egress-address-controller/main.go | 168 ++++++ .../components/egress-address/deployment.yaml | 118 ++++ .../egress-address/kustomization.yaml | 5 + .../egress-address/metrics_service.yaml | 17 + config/egress-address/kustomization.yaml | 36 ++ config/ipam/egress-shard-ipv6.yaml | 167 ++++++ config/ipam/kustomization.yaml | 6 + config/rbac/role.yaml | 10 + .../egressshardaddress_controller.go | 239 ++++++++ .../egressshardaddress_controller_test.go | 517 ++++++++++++++++++ internal/egressaddress/address.go | 298 ++++++++++ internal/egressaddress/address_test.go | 97 ++++ 13 files changed, 1680 insertions(+) create mode 100644 cmd/egress-address-controller/main.go create mode 100644 config/components/egress-address/deployment.yaml create mode 100644 config/components/egress-address/kustomization.yaml create mode 100644 config/components/egress-address/metrics_service.yaml create mode 100644 config/egress-address/kustomization.yaml create mode 100644 config/ipam/egress-shard-ipv6.yaml create mode 100644 config/ipam/kustomization.yaml create mode 100644 internal/controller/egressshardaddress_controller.go create mode 100644 internal/controller/egressshardaddress_controller_test.go create mode 100644 internal/egressaddress/address.go create mode 100644 internal/egressaddress/address_test.go diff --git a/Dockerfile b/Dockerfile index 96300da..ee62ad8 100644 --- a/Dockerfile +++ b/Dockerfile @@ -14,11 +14,13 @@ COPY internal/ internal/ RUN CGO_ENABLED=0 GOOS=${TARGETOS:-linux} GOARCH=${TARGETARCH} go build -ldflags "-s -w" -o vpc-controller cmd/main.go RUN CGO_ENABLED=0 GOOS=${TARGETOS:-linux} GOARCH=${TARGETARCH} go build -ldflags "-s -w" -o fabric-identity-controller cmd/fabric-identity-controller/main.go +RUN CGO_ENABLED=0 GOOS=${TARGETOS:-linux} GOARCH=${TARGETARCH} go build -ldflags "-s -w" -o egress-address-controller cmd/egress-address-controller/main.go FROM gcr.io/distroless/static-debian12:nonroot WORKDIR / COPY --from=builder /workspace/vpc-controller . COPY --from=builder /workspace/fabric-identity-controller . +COPY --from=builder /workspace/egress-address-controller . USER 65532:65532 ENTRYPOINT ["/vpc-controller"] diff --git a/cmd/egress-address-controller/main.go b/cmd/egress-address-controller/main.go new file mode 100644 index 0000000..7fcdb73 --- /dev/null +++ b/cmd/egress-address-controller/main.go @@ -0,0 +1,168 @@ +/* +Copyright © 2026 Datum Technology, Inc. All rights reserved. + +This program is free software: you can redistribute it and/or modify +it under the terms of the GNU Affero General Public License as +published by the Free Software Foundation, either version 3 of the +License, or (at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Affero General Public License for more details. + +You should have received a copy of the GNU Affero General Public License +along with this program. If not, see . +*/ + +// Command egress-address-controller gives each egress shard in a cell the +// public IPv6 address it translates to. +// +// It runs in the cell, unlike fabric-identity-controller, which allocates +// centrally because a network spans locations and its identity must be the same +// in all of them. A shard is the opposite case: it names the Node it executes +// on, so it exists only where that Node does, and nothing about its address has +// to agree with any other location. +// +// It is a binary of its own rather than a reconciler inside vpc-controller +// because it needs a credential vpc-controller does not have. vpc-controller +// writes the attachment state of every workload in the cell and serves an +// admission webhook; giving that pod a credential into the platform's own +// tenancy widens the blast radius of the one component the cell cannot run +// without, and a missing or expired address credential would stop workloads +// attaching. Split out, an address that cannot be claimed costs new shards +// their addresses and costs nothing else. +package main + +import ( + "flag" + "os" + + "k8s.io/apimachinery/pkg/runtime" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/tools/clientcmd" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/healthz" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" + + "go.datum.net/cloud/internal/controller" + "go.datum.net/cloud/internal/ipam" + bgpv1alpha1 "go.datum.net/network/api/v1alpha1" +) + +var scheme = runtime.NewScheme() + +func init() { + utilruntime.Must(clientgoscheme.AddToScheme(scheme)) + utilruntime.Must(bgpv1alpha1.AddToScheme(scheme)) +} + +func main() { + var metricsAddr, probeAddr string + var addressClass, claimNamespace, location, platformProject, ipamKubeconfig string + var enableLeaderElection bool + + flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "Address the metric endpoint binds to.") + flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "Address the probe endpoint binds to.") + flag.BoolVar(&enableLeaderElection, "leader-elect", true, + "Enable leader election. A single writer is what keeps one shard to one address.") + flag.StringVar(&addressClass, "address-class-ipv6", "", + "Required. The IPClass that hands out shard addresses. It draws from announceable public space shared by every shard in a location.") + flag.StringVar(&claimNamespace, "claim-namespace", "default", + "Namespace in the platform's own tenancy that address claims are written to.") + flag.StringVar(&location, "location", "", + "Required. The location this cell serves. It selects the shared public range addresses come from; two cells serving one location draw from the same range.") + flag.StringVar(&platformProject, "platform-project", "", + "Required. The project control plane the platform allocates its own values in. A shard's address is not a consumer's address and must not be drawn from any one consumer's space or counted against their quota.") + flag.StringVar(&ipamKubeconfig, "ipam-kubeconfig", "", + "Required. Path to a kubeconfig for the cluster serving the IPAM API.") + + opts := zap.Options{Development: false} + opts.BindFlags(flag.CommandLine) + flag.Parse() + + ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) + setupLog := ctrl.Log.WithName("setup") + + // Every one of these is fatal at startup rather than per shard. A shard's + // address cannot be corrected once written, so a deployment that would draw + // from the wrong space, or from no space, must not reach a single shard. + switch { + case addressClass == "": + setupLog.Error(nil, "-address-class-ipv6 is required") + os.Exit(1) + case location == "": + // A claim carrying the wrong location is the dangerous case, not the + // missing one: it succeeds, and hands this cell an address that another + // location's fabric attracts. + setupLog.Error(nil, "-location is required") + os.Exit(1) + case platformProject == "": + setupLog.Error(nil, "-platform-project is required") + os.Exit(1) + case ipamKubeconfig == "": + setupLog.Error(nil, "-ipam-kubeconfig is required") + os.Exit(1) + } + + // The manager runs against the cell this is scheduled on, which is also + // where the shards are. There is no second cluster: an EgressShard names a + // Node, so it is never anywhere but the cell holding that Node. + mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ + Scheme: scheme, + Metrics: metricsserver.Options{BindAddress: metricsAddr}, + HealthProbeBindAddress: probeAddr, + LeaderElection: enableLeaderElection, + LeaderElectionID: "egress-address-controller.cloud.datumapis.com", + }) + if err != nil { + setupLog.Error(err, "unable to start manager") + os.Exit(1) + } + + ipamRestConfig, err := clientcmd.BuildConfigFromFlags("", ipamKubeconfig) + if err != nil { + setupLog.Error(err, "unable to load the IPAM kubeconfig") + os.Exit(1) + } + + ipamScheme, err := ipam.Scheme() + if err != nil { + setupLog.Error(err, "unable to build the IPAM scheme") + os.Exit(1) + } + + ipamClients, err := ipam.NewClientFactory(ipamRestConfig, ipamScheme, platformProject) + if err != nil { + setupLog.Error(err, "unable to build the IPAM client factory") + os.Exit(1) + } + + if err := (&controller.EgressShardAddressReconciler{ + Client: mgr.GetClient(), + IPAM: ipamClients, + AddressClassIPv6: addressClass, + ClaimNamespace: claimNamespace, + Location: location, + }).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "EgressShardAddress") + os.Exit(1) + } + + if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { + setupLog.Error(err, "unable to set up health check") + os.Exit(1) + } + if err := mgr.AddReadyzCheck("readyz", healthz.Ping); err != nil { + setupLog.Error(err, "unable to set up ready check") + os.Exit(1) + } + + setupLog.Info("starting egress address controller", "location", location) + if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil { + setupLog.Error(err, "problem running manager") + os.Exit(1) + } +} diff --git a/config/components/egress-address/deployment.yaml b/config/components/egress-address/deployment.yaml new file mode 100644 index 0000000..6a85004 --- /dev/null +++ b/config/components/egress-address/deployment.yaml @@ -0,0 +1,118 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: egress-address-controller + namespace: system + labels: + app.kubernetes.io/name: egress-address-controller + app.kubernetes.io/component: egress-address-controller + app.kubernetes.io/managed-by: kustomize +spec: + # A shard's address is decided once and cannot be reassigned. Leader election + # is what keeps that true across a rollout; the replica count is not. + replicas: 1 + selector: + matchLabels: + app.kubernetes.io/name: egress-address-controller + template: + metadata: + annotations: + kubectl.kubernetes.io/default-container: manager + labels: + app.kubernetes.io/name: egress-address-controller + app.kubernetes.io/component: egress-address-controller + spec: + serviceAccountName: vpc-controller + securityContext: + runAsNonRoot: true + seccompProfile: + type: RuntimeDefault + containers: + - name: manager + image: ghcr.io/datum-cloud/vpc-controller + command: + - /egress-address-controller + # Args reference env vars so an overlay can retarget any value with a + # strategic-merge patch on env, matched by name, instead of rewriting + # the args list. + args: + - --leader-elect + - --health-probe-bind-address=:8081 + - --metrics-bind-address=:8080 + - --address-class-ipv6=$(ADDRESS_CLASS_IPV6) + - --claim-namespace=$(CLAIM_NAMESPACE) + - --location=$(LOCATION) + - --platform-project=$(PLATFORM_PROJECT) + - --ipam-kubeconfig=/etc/ipam-cluster/kubeconfig + env: + # The class that hands out shard addresses. Required; a deployment + # naming no class refuses to start rather than draw from a default + # that would hand a shard a private address nothing routes. + - name: ADDRESS_CLASS_IPV6 + value: datum-egress-shard-address-ipv6 + - name: CLAIM_NAMESPACE + value: default + # The location this cell serves, which must equal the cell's own + # topology.datum.net/location label. Required and deployment + # specific: a cell claiming under another location's name is handed + # an address that location's fabric attracts, and the assignment + # cannot be taken back. + - name: LOCATION + value: "" + # A shard's address is not a consumer's address, so the claim is + # written in a project the platform owns. Required and + # deployment-specific. + - name: PLATFORM_PROJECT + value: "" + ports: + - name: metrics + containerPort: 8080 + livenessProbe: + httpGet: + path: /healthz + port: 8081 + initialDelaySeconds: 15 + periodSeconds: 20 + readinessProbe: + httpGet: + path: /readyz + port: 8081 + initialDelaySeconds: 5 + periodSeconds: 10 + securityContext: + allowPrivilegeEscalation: false + capabilities: + drop: + - ALL + resources: + requests: + cpu: 10m + memory: 64Mi + limits: + memory: 256Mi + volumeMounts: + - name: ipam-cluster-kubeconfig + mountPath: /etc/ipam-cluster + readOnly: true + volumes: + # Not optional. This component does one thing and cannot do it without + # the address service. A pod waiting in ContainerCreating for a + # credential that has not landed says so plainly; one started against an + # empty dir crashloops until the kubelet's next volume resync, which + # reads as a broken image rather than a missing secret. + # + # There is deliberately no second mount. Unlike the central fabric + # identity controller, everything this reads and writes in the cluster + # is local: an EgressShard names a Node, so it never exists anywhere but + # the cell holding that Node. + # + # This credential authenticates to the platform's own project, which is + # the tenancy holding every platform-owned allocation. It must be scoped + # to this component's job and no more -- "use" on the one address class + # and write access to IPClaims in the one namespace -- because a cell is + # the least trusted cluster in the fleet and there is one of these + # credentials per cell. + - name: ipam-cluster-kubeconfig + secret: + secretName: ipam-cluster-kubeconfig + terminationGracePeriodSeconds: 10 diff --git a/config/components/egress-address/kustomization.yaml b/config/components/egress-address/kustomization.yaml new file mode 100644 index 0000000..82fa7cb --- /dev/null +++ b/config/components/egress-address/kustomization.yaml @@ -0,0 +1,5 @@ +apiVersion: kustomize.config.k8s.io/v1alpha1 +kind: Component +resources: + - deployment.yaml + - metrics_service.yaml diff --git a/config/components/egress-address/metrics_service.yaml b/config/components/egress-address/metrics_service.yaml new file mode 100644 index 0000000..8279a80 --- /dev/null +++ b/config/components/egress-address/metrics_service.yaml @@ -0,0 +1,17 @@ +apiVersion: v1 +kind: Service +metadata: + name: egress-address-metrics + namespace: system + labels: + app.kubernetes.io/name: egress-address-controller + app.kubernetes.io/component: egress-address-controller + app.kubernetes.io/managed-by: kustomize +spec: + ports: + - name: metrics + port: 8080 + protocol: TCP + targetPort: metrics + selector: + app.kubernetes.io/name: egress-address-controller diff --git a/config/egress-address/kustomization.yaml b/config/egress-address/kustomization.yaml new file mode 100644 index 0000000..ebc11be --- /dev/null +++ b/config/egress-address/kustomization.yaml @@ -0,0 +1,36 @@ +# The claimer of an egress shard's public address. It runs in the cell, beside +# the shards it writes, which is what separates it from the fabric identity +# overlay next door: a network spans locations and its identity cannot be +# decided in any one of them, while a shard names the Node it executes on and +# exists only where that Node does. +# +# It is its own overlay rather than a reconciler inside the cell manager because +# it holds a credential the cell manager does not. The cell manager writes the +# attachment state of every workload here and serves an admission webhook; +# giving that pod a credential into the platform's own tenancy widens the blast +# radius of the one component a cell cannot run without, and an address +# credential that expired would stop workloads attaching. Split out, an address +# that cannot be claimed costs new shards their addresses and nothing else. +# +# RBAC comes from ../rbac unchanged. The repo generates one ClusterRole from +# every marker under ./internal/..., so every role this image runs shares a role +# and a ServiceAccount name. +# +# The EgressShard CRD is not included here. It belongs to +# go.datum.net/network and is installed by the data plane that owns it, not by +# a consumer of it. +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +namespace: egress-address-system +resources: + - ../rbac +components: + - ../components/egress-address + +# Pinned here rather than inherited from ../manager: this overlay does not +# include it, and the publish workflow rewrites the tag in every overlay it is +# given. +images: + - name: ghcr.io/datum-cloud/vpc-controller + newName: ghcr.io/datum-cloud/vpc-controller + newTag: latest diff --git a/config/ipam/egress-shard-ipv6.yaml b/config/ipam/egress-shard-ipv6.yaml new file mode 100644 index 0000000..f4081d6 --- /dev/null +++ b/config/ipam/egress-shard-ipv6.yaml @@ -0,0 +1,167 @@ +# The public IPv6 space an egress shard's translation address is drawn from, +# consumed by egress-address-controller. +# +# APPLY DESTINATION. This is platform-project IPAM content. Its sibling — the +# fabric identity space in infra's +# apps/network-services-operator/platform-project/fabric-identity.yaml — is +# applied from infra by the installer scoped to the `datum-cloud` project, and +# this belongs beside it. It is authored here because the controller that reads +# it is here; moving it is an open item, not a second home. +# +# IPv4 IS DELIBERATELY ABSENT. A shard performs NAT64 only when its spec carries +# both an IPv4 address and a NAT64 prefix, and the platform pairs no resolver +# with a translator yet, so an IPv4 egress address would be reachability the API +# reports and the data plane does not deliver. The register that would say which +# public IPv4 prefix is egress space also still reads `status: TODO` throughout +# (infra/network/ipam/ipv4.yaml). Defining the IPv4 half when both are settled +# costs one commit; defining it now is a permanent choice made to avoid a gap. +# +# NOTHING HERE CAN BE CORRECTED IN PLACE. `uniqueWithin` and `poolPer` are +# rejected on update, and a pool's `spec.cidr` is carried by every allocation +# beneath it. Read the note on each field before changing one. + +# --------------------------------------------------------------------------- +# The aggregate. One root pool, carved into one /48 per location. +# --------------------------------------------------------------------------- +apiVersion: ipam.miloapis.com/v1alpha1 +kind: IPPool +metadata: + name: datum-egress-shard-ipv6-root +spec: + # REQUIRED, AND DELIBERATELY EMPTY. The service refuses a root pool with no + # CIDR, so applying this file unedited is rejected rather than seeding a + # permanent aggregate from a plausible-looking guess. That is the same reason + # PLATFORM_PROJECT ships empty in the deployment beside it. + # + # The value must be a /40 inside the platform's public supernet + # 2607:ed40::/32 that overlaps no entry in infra/network/ipam/ipv6.yaml. A /40 + # carves 256 locations at the /48 below; the platform serves 16 today. + # + # Assigning it is a human decision that has not been made. Every entry in that + # register reads `status: TODO` and the file is marked a draft for review, so + # no prefix in it can be read as free. Record the assignment there first, then + # set it here. + cidr: "" + ipFamily: IPv6 + visibility: shared + # The chain ROOT, never the leaf. A pool naming the leaf would hand an + # interface the whole aggregate. + classNames: + - datum-egress-shard-ipv6 + allocation: + # Fixed at the per-location carve. A pool free to hand out other sizes would + # let one location take space sized for several. + minPrefixLength: 48 + maxPrefixLength: 48 + strategy: FirstFit + +# --------------------------------------------------------------------------- +# The per-location range. This class provisions the pools; it binds nothing. +# --------------------------------------------------------------------------- +--- +apiVersion: ipam.miloapis.com/v1alpha1 +kind: IPClass +metadata: + name: datum-egress-shard-ipv6 +spec: + ipFamily: IPv6 + # One pool per location, shared by every cell and every consumer in it. + # + # Keyed on location rather than on cell because two cells already serve one + # location: us-central-1 and us-east-1 each carry two clusters labelled + # topology.datum.net/location with the same value. Keying on the cell would + # give one physical site two announceable aggregates, and would need a `cell` + # scope role that no class in the platform defines, where `location` is + # already the role every other claim carries. + # + # `project` is deliberately absent, which is what makes this shared. A shard's + # address serves every network the class places on it, and per-consumer blocks + # would exhaust a public aggregate after one block per project instead of one + # per location. IMMUTABLE: a class that separated per consumer could not be + # corrected, only replaced. + poolPer: + - location + # Empty: one address space platform-wide. Two shards must never hold one + # address — the datapath claims a reply by exact match against the address it + # translates to, so two holders split each other's return traffic. Emptiness + # makes the space tenant-free, and the constraint that enforces it is a + # database exclusion over (pool, address space, prefix), not Go. + # + # The shard is deliberately NOT in this scope. Naming it would make each shard + # its own address space and let two shards be handed the same address, which + # is the exact failure the field exists to prevent. + uniqueWithin: [] + # The size of the range each location holds. A /48 is the granularity the + # platform's own register already assigns public space at per site, and it is + # the unit that can be announced: prefixes longer than /48 are widely + # filtered, so a smaller carve would hold addresses nothing outside the site + # could route to. The space a /48 wastes on a handful of shards costs nothing + # in IPv6; an unannounceable egress range costs the feature. + allowedPrefixLengths: + min: 48 + max: 48 + defaultPrefixLength: 48 + # The /48 leaves the location as one route; the addresses inside it never + # appear outside it. An aggregate must be originated with a discard route — + # see the dependency note at the foot of this file, because nothing originates + # it today. + routing: + external: Aggregate + # Governs a ScopeRange claim of this class, and nothing makes one: the first + # shard's address claim provisions the location's /48 through the cascade. It + # is stated so that a range later claimed directly is not released by default, + # and it is inert until something claims one. + reclaimPolicy: Retain + +# --------------------------------------------------------------------------- +# The address. This is the class a shard's claim names. +# --------------------------------------------------------------------------- +--- +apiVersion: ipam.miloapis.com/v1alpha1 +kind: IPClass +metadata: + name: datum-egress-shard-address-ipv6 + # No is-default-class marker. This space is reached only by naming it: a claim + # that landed here by default would be handed a public address, and the + # platform's default IPv6 class hands out private endpoint space. +spec: + ipFamily: IPv6 + parentClassName: datum-egress-shard-ipv6 + # No poolPer: this is the leaf. It provisions nothing and binds addresses + # directly inside the /48 its parent carved for the location. + uniqueWithin: [] + # Exactly one address. A shard translates to a single address and the datapath + # matches it exactly, so a block would hand the shard space it cannot use and + # hold the rest out of circulation with nothing reporting the difference. + allowedPrefixLengths: + min: 128 + max: 128 + defaultPrefixLength: 128 + # Freed when the claim goes, which happens only when the shard it is named for + # is gone. + # + # NOT Retain. A shard holding the wrong address is fixed by deleting and + # recreating the shard, because the address in its spec is write-once. Under + # Retain the recreated shard is handed the same address back, so the only + # remedy the API leaves for a wrong address would silently not work. Retain + # also never returns the address at all: the service implements no lease + # expiry, so a retained allocation outlives the decommissioned node forever + # and an announceable aggregate is scarce enough for that to matter. Reissuing + # an address costs nothing a consumer was promised — this class is shared, so + # the address is reported as one nobody may rely on or allow-list. + reclaimPolicy: Delete + # A distinct route inside the location, reached by the shard's own + # advertisement into the fabric. + routing: + internal: Host + +# --------------------------------------------------------------------------- +# WHAT THIS DOES NOT DELIVER +# +# Nothing announces the per-location /48. The shard's own address becomes +# reachable inside its location by the advertisement the shard makes into the +# fabric; the aggregate leaving the location toward the internet is an upstream +# announcement, paired with a discard route, that no component here originates. +# Until it exists, a shard holds a correctly allocated address that the internet +# cannot reply to. +# --------------------------------------------------------------------------- diff --git a/config/ipam/kustomization.yaml b/config/ipam/kustomization.yaml new file mode 100644 index 0000000..bceafc3 --- /dev/null +++ b/config/ipam/kustomization.yaml @@ -0,0 +1,6 @@ +# Operator-applied IPAM content, not part of any deployment overlay. It is +# listed so the file is built and validated rather than only linted. +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: + - egress-shard-ipv6.yaml diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 8600ec8..764aa14 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -75,6 +75,16 @@ rules: - get - list - watch +- apiGroups: + - network.datumapis.com + resources: + - egressshards + verbs: + - get + - list + - patch + - update + - watch - apiGroups: - networking.datumapis.com resources: diff --git a/internal/controller/egressshardaddress_controller.go b/internal/controller/egressshardaddress_controller.go new file mode 100644 index 0000000..b9c5b6d --- /dev/null +++ b/internal/controller/egressshardaddress_controller.go @@ -0,0 +1,239 @@ +/* +Copyright © 2026 Datum Technology, Inc. All rights reserved. + +This program is free software: you can redistribute it and/or modify +it under the terms of the GNU Affero General Public License as +published by the Free Software Foundation, either version 3 of the +License, or (at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Affero General Public License for more details. + +You should have received a copy of the GNU Affero General Public License +along with this program. If not, see . +*/ + +package controller + +import ( + "context" + "errors" + "fmt" + "net/netip" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" + + "go.datum.net/cloud/internal/egressaddress" + "go.datum.net/cloud/internal/ipam" + bgpv1alpha1 "go.datum.net/network/api/v1alpha1" +) + +// EgressShardAddressReconciler gives each egress shard in this cell the public +// IPv6 address it translates to. +// +// It runs in the cell, beside the shards it writes. A shard names the Node it +// executes on, so it exists only where that Node does, and a controller reading +// it from anywhere else would be reading a federated copy of an object whose +// whole purpose is local. That also keeps the write and the object it lands on +// in one cluster, which is what makes the write-once field below safe to +// attempt: there is no copy of the shard that could be carrying a different +// value. +// +// The shard does not claim its own address. A shard runs on every translating +// node, including hardware at the edge of the network, and the process that +// would make the claim is the one serving the datapath -- so an +// address-service credential would sit on every such node, reachable from the +// process that also handles tenant packets, and the allocation request would +// sit beside the path that attaches a workload. One controller per cell moves +// the credential count from the number of translating nodes to the number of +// cells and takes the allocation off that path entirely: an address is claimed +// when a shard object appears, which is when a node is commissioned, not when a +// workload arrives. +type EgressShardAddressReconciler struct { + // Shards reads and writes the EgressShards in this cell. + Client client.Client + + // IPAM reaches the address service. + IPAM ipam.ClientFactory + + // AddressClassIPv6 is the class that hands out shard addresses. + AddressClassIPv6 string + + // ClaimNamespace is the namespace in the platform's own tenancy that + // address claims are written to. + ClaimNamespace string + + // Location is the location this cell serves. It selects the shared public + // range the address comes from, and two cells serving one location draw + // from the same range. + Location string +} + +// Reconcile assigns the shard its address, once. +// +// Reading the shard is what separates "deleted" from "not addressed yet". Only +// a shard that is actually gone releases its claim. +func (r *EgressShardAddressReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + shard := &bgpv1alpha1.EgressShard{} + err := r.Client.Get(ctx, client.ObjectKey{Namespace: req.Namespace, Name: req.Name}, shard) + switch { + case apierrors.IsNotFound(err): + return ctrl.Result{}, r.release(ctx, req.Namespace, req.Name) + case err != nil: + // A read that failed says nothing about whether the shard is still + // there, and releasing on it would put a live shard's address back in + // circulation. + return ctrl.Result{}, err + } + + // A shard on its way out is not given an address it would never program, + // and keeps the one it has until it is actually gone: the flows it is + // translating are still there while it drains. + if !shard.DeletionTimestamp.IsZero() { + return ctrl.Result{}, nil + } + + if shard.Spec.ShardAddressIPv6 != "" { + // Already addressed. The family label restates the assignment for the + // selectors that place traffic on this shard, and cannot be written + // with the address itself: a shard whose label write failed would + // otherwise stay unselectable forever, because the address it would be + // rewritten with cannot be written twice. + return ctrl.Result{}, r.markFamilyServed(ctx, shard) + } + + address, err := r.claim(ctx, shard) + if err != nil { + return ctrl.Result{}, err + } + + // The value comes from a bound claim and from nowhere else. Every path that + // could not produce one returned above, because spec.shardAddressIPv6 is + // write-once: a placeholder written here is not correctable, and the shard + // would have to be deleted and recreated to be rid of it. + shard.Spec.ShardAddressIPv6 = address.String() + if shard.Labels == nil { + shard.Labels = map[string]string{} + } + shard.Labels[bgpv1alpha1.LabelEgressShardIPv6] = bgpv1alpha1.LabelValueEgressFamilyServed + + // An ordinary update on the object just read, not a server-side apply with + // forced ownership. The field's own validation compares against the stored + // value, so a conflicting write has to be refused rather than won: forcing + // ownership of a field that cannot be reassigned is the one thing that must + // not happen quietly here. + if err := r.Client.Update(ctx, shard); err != nil { + return ctrl.Result{}, fmt.Errorf("assign egress shard %q the address %s: %w", + shard.Name, address, err) + } + + log.FromContext(ctx).Info("assigned an egress shard its public IPv6 address", + "shard", shard.Name, "address", address.String(), "location", r.Location) + return ctrl.Result{}, nil +} + +// markFamilyServed records that this shard translates IPv6, for the selectors +// that place traffic on it. A selector matches labels and cannot read a spec +// field, so whoever assigns the address states it here too. +func (r *EgressShardAddressReconciler) markFamilyServed(ctx context.Context, shard *bgpv1alpha1.EgressShard) error { + if shard.Labels[bgpv1alpha1.LabelEgressShardIPv6] == bgpv1alpha1.LabelValueEgressFamilyServed { + return nil + } + if shard.Labels == nil { + shard.Labels = map[string]string{} + } + shard.Labels[bgpv1alpha1.LabelEgressShardIPv6] = bgpv1alpha1.LabelValueEgressFamilyServed + if err := r.Client.Update(ctx, shard); err != nil { + return fmt.Errorf("mark egress shard %q as serving IPv6: %w", shard.Name, err) + } + return nil +} + +func (r *EgressShardAddressReconciler) claim( + ctx context.Context, + shard *bgpv1alpha1.EgressShard, +) (netip.Addr, error) { + ipamClient, err := r.IPAM.ClientForPlatform() + if err != nil { + return netip.Addr{}, fmt.Errorf("reach the public address space: %w", err) + } + + address, err := egressaddress.Claim(ctx, ipamClient, egressaddress.Request{ + ClassName: r.AddressClassIPv6, + Namespace: r.ClaimNamespace, + Location: r.Location, + ShardNamespace: shard.Namespace, + ShardName: shard.Name, + }) + if err != nil { + // An unusable answer is a wait on an operator, not on the service: + // retrying reaches the same allocation. Fail closed either way -- a + // shard given an address it cannot translate to is worse than one given + // none, because the assignment cannot be taken back. + var unusable *egressaddress.UnusableError + if errors.As(err, &unusable) { + log.FromContext(ctx).Error(err, "the public address space handed out something no shard address can be read from", + "shard", shard.Name, "location", r.Location) + } + return netip.Addr{}, fmt.Errorf("claim a public IPv6 address for egress shard %q: %w", shard.Name, err) + } + return address, nil +} + +// release gives back the address of a shard that is gone. +// +// Triggered by the shard's absence rather than by a finalizer. A finalizer would +// close the window in which a missed delete leaks a claim, and would open a +// larger one: a shard whose address could not be released would refuse to +// finish deleting, which is how a node is kept from being decommissioned. A +// leaked claim is an operator deleting one object; a wedged shard is a node +// nobody can retire. +func (r *EgressShardAddressReconciler) release(ctx context.Context, namespace, name string) error { + ipamClient, err := r.IPAM.ClientForPlatform() + if err != nil { + return fmt.Errorf("reach the public address space: %w", err) + } + if err := egressaddress.Release(ctx, ipamClient, r.ClaimNamespace, namespace, name); err != nil { + return err + } + log.FromContext(ctx).Info("released the public IPv6 address of a shard that is gone", + "shard", name, "location", r.Location) + return nil +} + +// This controller writes EgressShards in its own cell and claims addresses on +// the platform's behalf in IPAM, which it reaches under a separate credential +// and which no marker here covers. +// +// +kubebuilder:rbac:groups=coordination.k8s.io,resources=leases,verbs=create;delete;get;list;patch;update;watch +// +kubebuilder:rbac:groups="",resources=events,verbs=create;patch +// +kubebuilder:rbac:groups=network.datumapis.com,resources=egressshards,verbs=get;list;patch;update;watch + +// SetupWithManager registers the reconciler. +func (r *EgressShardAddressReconciler) SetupWithManager(mgr ctrl.Manager) error { + if r.AddressClassIPv6 == "" { + return errors.New("an address class is required") + } + if r.ClaimNamespace == "" { + return errors.New("a namespace to write claims in is required") + } + if r.Location == "" { + // A claim carrying no location is refused by the service, and one + // carrying the wrong location draws from another location's range and + // hands this cell an address nothing routes to it. + return errors.New("the location this cell serves is required") + } + if r.IPAM == nil { + return errors.New("an address space is required") + } + + return ctrl.NewControllerManagedBy(mgr). + Named("egressshardaddress"). + For(&bgpv1alpha1.EgressShard{}). + Complete(r) +} diff --git a/internal/controller/egressshardaddress_controller_test.go b/internal/controller/egressshardaddress_controller_test.go new file mode 100644 index 0000000..49c6716 --- /dev/null +++ b/internal/controller/egressshardaddress_controller_test.go @@ -0,0 +1,517 @@ +/* +Copyright © 2026 Datum Technology, Inc. All rights reserved. + +This program is free software: you can redistribute it and/or modify +it under the terms of the GNU Affero General Public License as +published by the Free Software Foundation, either version 3 of the +License, or (at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Affero General Public License for more details. + +You should have received a copy of the GNU Affero General Public License +along with this program. If not, see . +*/ + +package controller + +import ( + "context" + "errors" + "fmt" + "testing" + + ipamv1alpha1 "go.miloapis.com/ipam/pkg/apis/ipam/v1alpha1" + "go.miloapis.com/ipam/pkg/ipamerrors" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/validation/field" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + + "go.datum.net/cloud/internal/egressaddress" + "go.datum.net/cloud/internal/ipam" + bgpv1alpha1 "go.datum.net/network/api/v1alpha1" +) + +const ( + testShardNamespace = "galactic-system" + testShardName = "worker-8b4e1647-dfw" + testLocation = "us-central-1" + testAddressClass = "datum-egress-shard-address-ipv6" + testClaimNamespace = "default" +) + +// fakeAddressIPAM stands in for the address service. Allocation is synchronous +// there, so the create response already carries the address. +type fakeAddressIPAM struct { + client client.Client + // next is the host index the location's range hands out. + next int + // created records every claim name the service was asked to bind, so a + // second claim for one shard is visible rather than merely harmless. + created []string + // deleted records every claim name released. + deleted []string + // retained maps an allocation name to the address it still holds after its + // claim was deleted under Retain. It is what makes a second claim of the + // same name a conflict rather than a fresh allocation. + retained map[string]string + // unbound holds the allocation back, which is what a claim looks like + // between being accepted and being bound. + unbound bool +} + +func allocationNameForClaim(claimName string) string { return "alloc-" + claimName } + +// refuseWhatTheAddressServerWouldRefuse mirrors the parts of the service's admission +// this depends on. The fake would otherwise bind anything, which is how a claim +// no real server has ever accepted passes every test here. +func refuseWhatTheAddressServerWouldRefuse(ipClaim *ipamv1alpha1.IPClaim) error { + invalid := func(detail string) error { + return apierrors.NewInvalid( + ipamv1alpha1.SchemeGroupVersion.WithKind("IPClaim").GroupKind(), ipClaim.Name, + field.ErrorList{field.Required(field.NewPath("spec"), detail)}) + } + // The server bounds a claim's prefix length by the family stated on the + // claim, before it looks at the class at all. + if p := ipClaim.Spec.PrefixLength; p != nil { + maxLen := int32(32) + if ipClaim.Spec.IPFamily == ipamv1alpha1.IPv6 { + maxLen = 128 + } + if *p > maxLen { + return invalid(fmt.Sprintf("prefixLength %d exceeds %d for family %q", + *p, maxLen, ipClaim.Spec.IPFamily)) + } + } + // The class holding the per-location range names "location" in poolPer, so + // a claim omitting it cannot be resolved to a pool. + if _, ok := ipClaim.Spec.Scope[egressaddress.ScopeRoleLocation]; !ok { + return invalid("scope is missing role \"location\"") + } + return nil +} + +func newFakeAddressIPAM(t *testing.T) *fakeAddressIPAM { + t.Helper() + + scheme := runtime.NewScheme() + if err := ipamv1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("build the IPAM scheme: %v", err) + } + + service := &fakeAddressIPAM{retained: map[string]string{}} + service.client = fake.NewClientBuilder(). + WithScheme(scheme). + WithInterceptorFuncs(interceptor.Funcs{ + Create: func(ctx context.Context, c client.WithWatch, object client.Object, opts ...client.CreateOption) error { + ipClaim, ok := object.(*ipamv1alpha1.IPClaim) + if !ok { + return c.Create(ctx, object, opts...) + } + if err := refuseWhatTheAddressServerWouldRefuse(ipClaim); err != nil { + return err + } + allocationName := allocationNameForClaim(ipClaim.Name) + if cidr, held := service.retained[allocationName]; held { + _ = cidr + return newRetainedAllocationConflict(ipClaim.Name, allocationName) + } + service.created = append(service.created, ipClaim.Name) + if !service.unbound { + service.next++ + ipClaim.Status.Phase = ipamv1alpha1.ClaimPhase("Bound") + ipClaim.Status.AllocatedCIDR = fmt.Sprintf("2001:db8:100::%x/128", service.next) + } else { + ipClaim.Status.Phase = ipamv1alpha1.ClaimPhase("Pending") + } + return c.Create(ctx, ipClaim, opts...) + }, + Delete: func(ctx context.Context, c client.WithWatch, object client.Object, opts ...client.DeleteOption) error { + if ipClaim, ok := object.(*ipamv1alpha1.IPClaim); ok { + service.deleted = append(service.deleted, ipClaim.Name) + } + return c.Delete(ctx, object, opts...) + }, + }). + Build() + return service +} + +func (f *fakeAddressIPAM) ClientForPlatform() (client.Client, error) { return f.client, nil } + +func (f *fakeAddressIPAM) ClientForProject(string) (client.Client, error) { + return nil, errors.New("a shard's address is never drawn from a consumer's project") +} + +var _ ipam.ClientFactory = (*fakeAddressIPAM)(nil) + +// newRetainedAllocationConflict is the refusal the service answers a claim with +// when an allocation under the same identity is still held by a released claim. +// It is built with the service's own constructor so the classifier the +// controller depends on is the one under test, rather than a status this test +// invented and only this test can read. +func newRetainedAllocationConflict(claimName, allocationName string) error { + return ipamerrors.NewRetainedAllocation( + ipamv1alpha1.Resource("ipclaims"), claimName, allocationName, + fmt.Sprintf("an allocation under this identity already exists: IPAllocation %q, retained by an earlier claim of the same name", allocationName)) +} + +func newShardCell(t *testing.T, objects ...client.Object) client.Client { + t.Helper() + + scheme := runtime.NewScheme() + if err := bgpv1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("build the cell scheme: %v", err) + } + return fake.NewClientBuilder().WithScheme(scheme).WithObjects(objects...).Build() +} + +func shard(name string) *bgpv1alpha1.EgressShard { + return &bgpv1alpha1.EgressShard{ + ObjectMeta: metav1.ObjectMeta{Namespace: testShardNamespace, Name: name}, + Spec: bgpv1alpha1.EgressShardSpec{ + TargetRef: bgpv1alpha1.TargetRef{Kind: "Node", Name: name}, + }, + } +} + +func reconcilerFor(cell client.Client, service *fakeAddressIPAM) *EgressShardAddressReconciler { + return &EgressShardAddressReconciler{ + Client: cell, + IPAM: service, + AddressClassIPv6: testAddressClass, + ClaimNamespace: testClaimNamespace, + Location: testLocation, + } +} + +func requestFor(name string) ctrl.Request { + return ctrl.Request{NamespacedName: types.NamespacedName{Namespace: testShardNamespace, Name: name}} +} + +func readShard(t *testing.T, cell client.Client, name string) *bgpv1alpha1.EgressShard { + t.Helper() + got := &bgpv1alpha1.EgressShard{} + if err := cell.Get(context.Background(), client.ObjectKey{Namespace: testShardNamespace, Name: name}, got); err != nil { + t.Fatalf("read the shard back: %v", err) + } + return got +} + +// The whole point: a shard an operator never gave an address to gets one, in +// spec, where galactic reads it. +func TestAShardWithoutAnAddressIsGivenOne(t *testing.T) { + cell := newShardCell(t, shard(testShardName)) + service := newFakeAddressIPAM(t) + + if _, err := reconcilerFor(cell, service).Reconcile(context.Background(), + requestFor(testShardName)); err != nil { + t.Fatalf("reconcile: %v", err) + } + + got := readShard(t, cell, testShardName) + if got.Spec.ShardAddressIPv6 == "" { + t.Fatal("the shard was left with no address to translate to") + } + if got.Labels[bgpv1alpha1.LabelEgressShardIPv6] != bgpv1alpha1.LabelValueEgressFamilyServed { + t.Errorf("the shard is not selectable as serving IPv6: labels = %v", got.Labels) + } +} + +// The claim is named for the shard, which is what makes a second reconcile find +// the address already held rather than draw another one out of a shared public +// range. +func TestReconcilingTwiceClaimsOneAddress(t *testing.T) { + cell := newShardCell(t, shard(testShardName)) + service := newFakeAddressIPAM(t) + reconciler := reconcilerFor(cell, service) + + for i := range 3 { + if _, err := reconciler.Reconcile(context.Background(), + requestFor(testShardName)); err != nil { + t.Fatalf("reconcile %d: %v", i, err) + } + } + + if len(service.created) != 1 { + t.Fatalf("the service was asked to bind %v; one shard holds one address", service.created) + } + want := egressaddress.ClaimName(testShardNamespace, testShardName) + if service.created[0] != want { + t.Errorf("claim name = %q, want the name derived from the shard %q", service.created[0], want) + } +} + +// spec.shardAddressIPv6 cannot be corrected once written, so a claim that holds +// no address yet must leave the field alone rather than write a blank or a +// guess to be replaced later. +func TestAnUnboundClaimWritesNothing(t *testing.T) { + cell := newShardCell(t, shard(testShardName)) + service := newFakeAddressIPAM(t) + service.unbound = true + + _, err := reconcilerFor(cell, service).Reconcile(context.Background(), + requestFor(testShardName)) + if err == nil { + t.Fatal("a claim holding no address reconciled successfully") + } + + got := readShard(t, cell, testShardName) + if got.Spec.ShardAddressIPv6 != "" { + t.Fatalf("a write-once field was written with %q before an address existed", got.Spec.ShardAddressIPv6) + } + if _, marked := got.Labels[bgpv1alpha1.LabelEgressShardIPv6]; marked { + t.Error("the shard was marked as serving IPv6 while holding no address") + } +} + +// An address an operator assigned by hand is what every shard carries today. +// Nothing may claim a second one for it, and nothing may try to rewrite it. +func TestAnOperatorAssignedAddressIsLeftAlone(t *testing.T) { + existing := shard(testShardName) + existing.Spec.ShardAddressIPv6 = "2001:db8:100::dead" + cell := newShardCell(t, existing) + service := newFakeAddressIPAM(t) + + if _, err := reconcilerFor(cell, service).Reconcile(context.Background(), + requestFor(testShardName)); err != nil { + t.Fatalf("reconcile: %v", err) + } + + if len(service.created) != 0 { + t.Errorf("an addressed shard drew %v from the public range", service.created) + } + got := readShard(t, cell, testShardName) + if got.Spec.ShardAddressIPv6 != "2001:db8:100::dead" { + t.Errorf("address = %q, want the operator's own value untouched", got.Spec.ShardAddressIPv6) + } + // The label still has to be caught up: an operator writing the address by + // hand is exactly the case where it is missing, and a shard carrying the + // address without the label is selected by nothing. + if got.Labels[bgpv1alpha1.LabelEgressShardIPv6] != bgpv1alpha1.LabelValueEgressFamilyServed { + t.Errorf("an addressed shard was left unselectable: labels = %v", got.Labels) + } +} + +// A shard draining still carries flows the address is translating, and giving +// one an address it will never program consumes public space for nothing. +func TestAShardOnItsWayOutIsNotAddressed(t *testing.T) { + leaving := shard(testShardName) + leaving.DeletionTimestamp = &metav1.Time{Time: metav1.Now().Time} + leaving.Finalizers = []string{"test.datumapis.com/hold"} + cell := newShardCell(t, leaving) + service := newFakeAddressIPAM(t) + + if _, err := reconcilerFor(cell, service).Reconcile(context.Background(), + requestFor(testShardName)); err != nil { + t.Fatalf("reconcile: %v", err) + } + if len(service.created) != 0 { + t.Errorf("a shard being deleted drew %v from the public range", service.created) + } + if len(service.deleted) != 0 { + t.Errorf("a shard still draining released %v while still translating", service.deleted) + } +} + +// Announceable public space is scarce, so a shard that is actually gone gives +// its address back. The claim carries ReclaimPolicy Delete, so removing it is +// what frees the address. +func TestAShardThatIsGoneReleasesItsAddress(t *testing.T) { + cell := newShardCell(t) + service := newFakeAddressIPAM(t) + + if _, err := reconcilerFor(cell, service).Reconcile(context.Background(), + requestFor(testShardName)); err != nil { + t.Fatalf("reconcile: %v", err) + } + + want := egressaddress.ClaimName(testShardNamespace, testShardName) + if len(service.deleted) != 1 || service.deleted[0] != want { + t.Fatalf("released %v, want the claim named for the departed shard %q", service.deleted, want) + } +} + +// A read that failed says nothing about whether the shard is still there. +// Releasing on it would put a live shard's address back in circulation for +// another shard to be handed while the first is still translating with it. +func TestAFailedReadReleasesNothing(t *testing.T) { + scheme := runtime.NewScheme() + if err := bgpv1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("build the cell scheme: %v", err) + } + cell := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(shard(testShardName)). + WithInterceptorFuncs(interceptor.Funcs{ + Get: func(context.Context, client.WithWatch, client.ObjectKey, client.Object, ...client.GetOption) error { + return errors.New("the cell's API server is unreachable") + }, + }). + Build() + service := newFakeAddressIPAM(t) + + if _, err := reconcilerFor(cell, service).Reconcile(context.Background(), + requestFor(testShardName)); err == nil { + t.Fatal("an unreadable shard reconciled successfully") + } + if len(service.deleted) != 0 { + t.Errorf("an unreadable shard released %v", service.deleted) + } +} + +// Two shards in one cell are two addresses. Sharing one would split each +// other's return traffic, because the datapath claims a reply by exact match +// against the address it translates to. +func TestTwoShardsGetTwoAddresses(t *testing.T) { + cell := newShardCell(t, + shard("worker-a"), + shard("worker-b")) + service := newFakeAddressIPAM(t) + reconciler := reconcilerFor(cell, service) + + for _, name := range []string{"worker-a", "worker-b"} { + if _, err := reconciler.Reconcile(context.Background(), + requestFor(name)); err != nil { + t.Fatalf("reconcile %s: %v", name, err) + } + } + + first := readShard(t, cell, "worker-a").Spec.ShardAddressIPv6 + second := readShard(t, cell, "worker-b").Spec.ShardAddressIPv6 + if first == "" || second == "" { + t.Fatalf("a shard was left unaddressed: %q and %q", first, second) + } + if first == second { + t.Fatalf("two shards were given one address %q", first) + } +} + +// The claim has to carry the location, because the class holding the shared +// per-location range names it in poolPer. A claim without it is refused, and +// one with the wrong value succeeds and hands this cell an address another +// location's fabric attracts. +func TestTheClaimCarriesTheLocationAndTheFamily(t *testing.T) { + cell := newShardCell(t, shard(testShardName)) + service := newFakeAddressIPAM(t) + + if _, err := reconcilerFor(cell, service).Reconcile(context.Background(), + requestFor(testShardName)); err != nil { + t.Fatalf("reconcile: %v", err) + } + + stored := &ipamv1alpha1.IPClaim{} + if err := service.client.Get(context.Background(), client.ObjectKey{ + Namespace: testClaimNamespace, + Name: egressaddress.ClaimName(testShardNamespace, testShardName), + }, stored); err != nil { + t.Fatalf("read the claim back: %v", err) + } + + if got := stored.Spec.Scope[egressaddress.ScopeRoleLocation].Name; got != testLocation { + t.Errorf("claim location = %q, want %q", got, testLocation) + } + if stored.Spec.IPFamily != ipamv1alpha1.IPv6 { + t.Errorf("claim family = %q; without it the server reads a /128 as an IPv4 length", stored.Spec.IPFamily) + } + if stored.Spec.ReclaimPolicy != ipamv1alpha1.ReclaimDelete { + t.Errorf("reclaimPolicy = %q; Retain would hand a recreated shard the same address back and defeat the only remedy for a wrong one", + stored.Spec.ReclaimPolicy) + } + // spec.ownerRef is overwritten by the server with the requesting project's + // identity, so the shard a claim is held for can only be recorded here. + if stored.Annotations[egressaddress.AnnotationShardName] != testShardName { + t.Errorf("the claim records no shard: annotations = %v", stored.Annotations) + } +} + +// A deployment that cannot say which location it serves must not reach a shard +// at all: the address it would write cannot be taken back. +func TestSetupRefusesADeploymentThatCannotClaimCorrectly(t *testing.T) { + for _, tc := range []struct { + name string + reconciler *EgressShardAddressReconciler + }{ + {"no class", &EgressShardAddressReconciler{ClaimNamespace: "default", Location: testLocation, IPAM: &fakeAddressIPAM{}}}, + {"no location", &EgressShardAddressReconciler{AddressClassIPv6: testAddressClass, ClaimNamespace: "default", IPAM: &fakeAddressIPAM{}}}, + {"no namespace", &EgressShardAddressReconciler{AddressClassIPv6: testAddressClass, Location: testLocation, IPAM: &fakeAddressIPAM{}}}, + {"no address space", &EgressShardAddressReconciler{AddressClassIPv6: testAddressClass, ClaimNamespace: "default", Location: testLocation}}, + } { + t.Run(tc.name, func(t *testing.T) { + if err := tc.reconciler.SetupWithManager(nil); err == nil { + t.Fatal("a deployment that cannot claim correctly started anyway") + } + }) + } +} + +// Nothing here writes Retain, but an operator can set it on the class or +// release a claim by hand, and the service then refuses a claim of the same +// name with a 409 rather than handing the address back (milo-os/ipam #107 -- +// no lease expiry and no replacement matching). The refusal names the +// allocation, so the address is one read away: take it rather than leaving the +// shard unaddressed forever behind a conflict that will never clear. +func TestARetainedAddressIsAdoptedRatherThanLost(t *testing.T) { + claimName := egressaddress.ClaimName(testShardNamespace, testShardName) + allocationName := allocationNameForClaim(claimName) + const held = "2001:db8:100::abcd/128" + + retained := &ipamv1alpha1.IPAllocation{ + ObjectMeta: metav1.ObjectMeta{Namespace: testClaimNamespace, Name: allocationName}, + Status: ipamv1alpha1.IPAllocationStatus{AllocatedCIDR: held}, + } + + cell := newShardCell(t, shard(testShardName)) + service := newFakeAddressIPAM(t) + service.retained[allocationName] = held + if err := service.client.Create(context.Background(), retained); err != nil { + t.Fatalf("seed the retained allocation: %v", err) + } + + if _, err := reconcilerFor(cell, service).Reconcile(context.Background(), + requestFor(testShardName)); err != nil { + t.Fatalf("reconcile: %v", err) + } + + got := readShard(t, cell, testShardName).Spec.ShardAddressIPv6 + if got != "2001:db8:100::abcd" { + t.Fatalf("address = %q, want the retained address %q read out of the allocation the refusal named", + got, "2001:db8:100::abcd") + } +} + +// The address is read out of status.allocatedCIDR and nowhere else. The API +// also carries a status.address holding the single-address form, and no +// released version of the service writes it, so a controller reading that +// instead treats every successful allocation as unbound. +func TestTheAddressIsReadFromAllocatedCIDRNotStatusAddress(t *testing.T) { + cell := newShardCell(t, shard(testShardName)) + service := newFakeAddressIPAM(t) + + if _, err := reconcilerFor(cell, service).Reconcile(context.Background(), + requestFor(testShardName)); err != nil { + t.Fatalf("reconcile: %v", err) + } + + stored := &ipamv1alpha1.IPClaim{} + if err := service.client.Get(context.Background(), client.ObjectKey{ + Namespace: testClaimNamespace, + Name: egressaddress.ClaimName(testShardNamespace, testShardName), + }, stored); err != nil { + t.Fatalf("read the claim back: %v", err) + } + if stored.Status.Address != "" { + t.Fatal("the fake set status.address, so this no longer proves the controller ignores it") + } + if readShard(t, cell, testShardName).Spec.ShardAddressIPv6 == "" { + t.Fatal("the shard was left unaddressed by a claim whose allocatedCIDR was set") + } +} diff --git a/internal/egressaddress/address.go b/internal/egressaddress/address.go new file mode 100644 index 0000000..3e93ac2 --- /dev/null +++ b/internal/egressaddress/address.go @@ -0,0 +1,298 @@ +/* +Copyright © 2026 Datum Technology, Inc. All rights reserved. + +This program is free software: you can redistribute it and/or modify +it under the terms of the GNU Affero General Public License as +published by the Free Software Foundation, either version 3 of the +License, or (at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Affero General Public License for more details. + +You should have received a copy of the GNU Affero General Public License +along with this program. If not, see . +*/ + +// Package egressaddress holds the public address an egress shard translates to. +// +// A shard's address is drawn from announceable public space shared by every +// shard in a location, not from a per-consumer prefix: one address serves every +// network the class places on the shard, because per-network blocks exhaust a +// public aggregate long before networks exhaust it. +// +// The claim is the record. Its name is derived from the shard, so the address a +// shard holds is a permanent property of that shard's name in that namespace +// for as long as the claim lives, and nothing here has to store a mapping of +// its own. Attribution rides on the name and on annotations because the service +// overwrites spec.ownerRef with the requesting project's identity, so a claim +// cannot record which shard it is held for that way. +package egressaddress + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "fmt" + "net/netip" + + ipamv1alpha1 "go.miloapis.com/ipam/pkg/apis/ipam/v1alpha1" + "go.miloapis.com/ipam/pkg/ipamerrors" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +const ( + // AddressBits is the prefix length a shard's address is claimed at. A + // shard translates to one address, and the datapath claims a reply by + // exact match against it, so anything shorter would hand the shard space + // it cannot use and would consume announceable public space at a multiple + // of what a shard needs. + AddressBits = 128 + + // ScopeRoleLocation is the scope role naming the location a claim is made + // for. The class holding the per-location pool names it in poolPer, so a + // claim omitting it is refused rather than drawn from another location's + // space. + ScopeRoleLocation = "location" + + // locationAPIGroup and locationKind identify a Location the same way every + // other claim in the platform does. A scope reference is compared, never + // resolved, so the three fields only have to agree with what else claims + // against this space -- and disagreeing would carve a second pool for one + // location rather than return an error. + locationAPIGroup = "networking.datumapis.com" + locationKind = "Location" + + // claimNamePrefix is what makes every claim of this kind recognisable to an + // operator reading the platform project, where claims for several purposes + // share one namespace. + claimNamePrefix = "egress-shard-ipv6" + + // AnnotationShardNamespace and AnnotationShardName record the shard a claim + // is held for. + // + // Annotations rather than labels: a shard is named after the node it runs + // on and may exceed the 63 characters a label value allows, and nothing + // selects these claims -- the name is derived from the shard, so every + // lookup is a Get. A label would buy a selector nobody uses at the cost of + // refusing to record the shards with the longest names. + AnnotationShardNamespace = "cloud.datumapis.com/egress-shard-namespace" + AnnotationShardName = "cloud.datumapis.com/egress-shard-name" + + // maxClaimNameLength is the ceiling on an object name in Kubernetes, which + // is all an IPClaim name is. + maxClaimNameLength = 253 +) + +// Request names one shard's claim on the public address space. +type Request struct { + // ClassName is the class that hands out shard addresses. + ClassName string + + // Namespace is the namespace in the platform's own tenancy the claim is + // written to. + Namespace string + + // Location is the location whose shared public range the address comes + // from. Shards in one location draw from one range, and two cells serving + // the same location share it. + Location string + + // ShardNamespace and ShardName identify the shard the address is for. The + // claim is named from the pair, so a shard reconciled twice finds the + // address it already holds rather than drawing a second one. + ShardNamespace string + ShardName string +} + +// ClaimName is the name the request's claim is held under. +// +// The delimiter is a dot rather than a dash because a namespace is a DNS label +// and cannot contain one, so the first dot after the prefix always ends the +// namespace. A dash would be ambiguous: namespace "a-b" with shard "c" and +// namespace "a" with shard "b-c" would collide, and a collision here is two +// shards sharing one address. +func ClaimName(shardNamespace, shardName string) string { + name := fmt.Sprintf("%s.%s.%s", claimNamePrefix, shardNamespace, shardName) + if len(name) <= maxClaimNameLength { + return name + } + + sum := sha256.Sum256([]byte(shardNamespace + "/" + shardName)) + suffix := "." + hex.EncodeToString(sum[:])[:16] + return name[:maxClaimNameLength-len(suffix)] + suffix +} + +// Claim holds the address this shard translates to. +// +// The service binds on create and refuses a duplicate name, so the read comes +// first. That is what makes the allocation idempotent without this recording +// anything of its own. +func Claim(ctx context.Context, ipamClient client.Client, request Request) (netip.Addr, error) { + ipClaim := &ipamv1alpha1.IPClaim{} + ipClaim.Namespace = request.Namespace + ipClaim.Name = ClaimName(request.ShardNamespace, request.ShardName) + ipClaim.Annotations = map[string]string{ + AnnotationShardNamespace: request.ShardNamespace, + AnnotationShardName: request.ShardName, + } + ipClaim.Spec = ipamv1alpha1.IPClaimSpec{ + ClassName: request.ClassName, + + // The class already fixes the family, but the server bounds a claim's + // prefix length from the family on the claim alone, before it resolves + // the class at all. Left unset, a /128 is read as an IPv4 length and + // refused, so every allocation fails. + IPFamily: ipamv1alpha1.IPv6, + + Target: ipamv1alpha1.TargetBlock, + PrefixLength: ptr.To(int32(AddressBits)), + + Scope: map[string]ipamv1alpha1.ScopeRef{ + ScopeRoleLocation: { + APIGroup: locationAPIGroup, + Kind: locationKind, + Name: request.Location, + }, + }, + + // Stated here rather than left to the class, because the reason for it + // is this controller's own. A shard holding the wrong address is fixed + // by deleting and recreating the shard, and a retained allocation would + // hand the replacement the same address back and silently defeat that + // remedy. Announceable public space is also scarce enough that an + // address held forever by a decommissioned node is a real loss, where + // reissue costs nothing a consumer was promised: the class is shared, + // so the address is reported as one nobody may rely on. + ReclaimPolicy: ipamv1alpha1.ReclaimDelete, + } + + existing := &ipamv1alpha1.IPClaim{} + getErr := ipamClient.Get(ctx, client.ObjectKeyFromObject(ipClaim), existing) + if getErr != nil && !apierrors.IsNotFound(getErr) { + return netip.Addr{}, fmt.Errorf("read the egress address claim %q: %w", ipClaim.Name, getErr) + } + + if getErr == nil { + ipClaim = existing + } else if createErr := ipamClient.Create(ctx, ipClaim); createErr != nil { + // An allocation retained by an earlier claim of this name is this + // shard's own address. The service refuses the create and names the + // allocation, so the address is one read away rather than lost: read it + // rather than treating the refusal as a failure. Nothing here writes + // Retain, so this is reached only where an operator set it on the class + // or released a claim by hand. + if allocationName, retained := ipamerrors.RetainedAllocation(createErr); retained { + return adopt(ctx, ipamClient, request.Namespace, allocationName) + } + + // 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 netip.Addr{}, fmt.Errorf("claim an egress address: %w", createErr) + } + ipClaim = raced + } + + if ipClaim.Status.AllocatedCIDR == "" { + // Not an error about the address: the claim exists and holds nothing + // yet. The caller must write nothing, because the field it would write + // cannot be corrected afterwards. + return netip.Addr{}, &UnboundError{ + claimName: ipClaim.Name, + phase: string(ipClaim.Status.Phase), + } + } + + return FromAllocatedCIDR(ipClaim.Status.AllocatedCIDR) +} + +// Release gives the shard's address back. +// +// Deleting the claim is what frees it, because the claim is written with +// ReclaimPolicy Delete. It is called only for a shard that is gone: releasing +// while a shard still holds the address in its spec would put the address back +// in circulation for another shard to be handed while the first is still +// translating with it, and the two would split each other's return traffic. +func Release(ctx context.Context, ipamClient client.Client, namespace, shardNamespace, shardName string) error { + ipClaim := &ipamv1alpha1.IPClaim{} + ipClaim.Namespace = namespace + ipClaim.Name = ClaimName(shardNamespace, shardName) + + if err := ipamClient.Delete(ctx, ipClaim); err != nil && !apierrors.IsNotFound(err) { + return fmt.Errorf("release the egress address claim %q: %w", ipClaim.Name, err) + } + return nil +} + +// adopt reads the address out of an allocation this shard already holds. The +// allocation outlives the claim that made it, which is what retention is for, +// so the address it names is the one this shard has always had. +func adopt(ctx context.Context, ipamClient client.Client, namespace, allocationName string) (netip.Addr, error) { + allocation := &ipamv1alpha1.IPAllocation{} + if err := ipamClient.Get(ctx, + client.ObjectKey{Namespace: namespace, Name: allocationName}, allocation); err != nil { + return netip.Addr{}, fmt.Errorf("read the retained allocation %q: %w", allocationName, err) + } + if allocation.Status.AllocatedCIDR == "" { + return netip.Addr{}, &UnboundError{claimName: allocationName, phase: string(allocation.Status.Phase)} + } + return FromAllocatedCIDR(allocation.Status.AllocatedCIDR) +} + +// FromAllocatedCIDR reads the shard's address out of what the service handed +// out. +// +// status.allocatedCIDR is the only field read. The API also carries a +// status.address holding the single-address form, and no released version of +// the service writes it, so a controller reading it treats every successful +// allocation as unbound. +func FromAllocatedCIDR(cidr string) (netip.Addr, error) { + prefix, err := netip.ParsePrefix(cidr) + if err != nil { + return netip.Addr{}, &UnusableError{message: fmt.Sprintf( + "the address service answered with %q, which is not a prefix", cidr)} + } + + address := prefix.Addr() + if !address.Is6() || address.Is4In6() { + return netip.Addr{}, &UnusableError{message: fmt.Sprintf( + "the address service answered with %q; a shard's IPv6 address is read out of an IPv6 space", cidr)} + } + + // Anything shorter is a block rather than an address. A shard given one + // would translate to its first address while holding the rest out of + // circulation, and nothing would report the difference. + if prefix.Bits() != AddressBits { + return netip.Addr{}, &UnusableError{message: fmt.Sprintf( + "the address service answered with %q; a shard's address is read out of a /%d", cidr, AddressBits)} + } + + return address, nil +} + +// UnusableError says the address service answered, and its answer cannot be +// used as a shard address. Retrying reaches the same allocation, so this is a +// wait on an operator rather than on the service. +type UnusableError struct { + message string +} + +func (e *UnusableError) Error() string { return e.message } + +// UnboundError says the claim exists and holds no address yet. Unlike +// UnusableError this resolves on its own, so it is worth retrying and is never +// worth writing anything on. +type UnboundError struct { + claimName string + phase string +} + +func (e *UnboundError) Error() string { + return fmt.Sprintf("the address service has allocated nothing for claim %q yet (phase %q)", + e.claimName, e.phase) +} diff --git a/internal/egressaddress/address_test.go b/internal/egressaddress/address_test.go new file mode 100644 index 0000000..2c7178f --- /dev/null +++ b/internal/egressaddress/address_test.go @@ -0,0 +1,97 @@ +/* +Copyright © 2026 Datum Technology, Inc. All rights reserved. + +This program is free software: you can redistribute it and/or modify +it under the terms of the GNU Affero General Public License as +published by the Free Software Foundation, either version 3 of the +License, or (at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Affero General Public License for more details. + +You should have received a copy of the GNU Affero General Public License +along with this program. If not, see . +*/ + +package egressaddress + +import ( + "errors" + "strings" + "testing" +) + +// A claim name collision is two shards sharing one address, so the split +// between namespace and name has to be unambiguous. A dash would not be: the +// two cases below would produce the same name. +func TestClaimNameCannotCollideAcrossTheNamespaceBoundary(t *testing.T) { + first := ClaimName("a-b", "c") + second := ClaimName("a", "b-c") + if first == second { + t.Fatalf("two different shards share the claim name %q", first) + } +} + +func TestClaimNameNamesTheShard(t *testing.T) { + got := ClaimName("galactic-system", "worker-8b4e1647-dfw") + if !strings.Contains(got, "worker-8b4e1647-dfw") { + t.Errorf("claim name %q does not name the shard it is held for", got) + } + if !strings.HasPrefix(got, claimNamePrefix) { + t.Errorf("claim name %q is not recognisable as an egress shard address claim", got) + } +} + +// A shard is named after its node, and the name it produces still has to be a +// name the API server will accept. +func TestALongShardNameStillYieldsAValidClaimName(t *testing.T) { + long := strings.Repeat("n", 300) + got := ClaimName("galactic-system", long) + if len(got) > maxClaimNameLength { + t.Fatalf("claim name is %d characters, over the %d the API server allows", len(got), maxClaimNameLength) + } + // Two long names that share a prefix must not truncate to one claim. + other := ClaimName("galactic-system", long+"x") + if got == other { + t.Fatal("two shards with long names share one claim name") + } +} + +func TestFromAllocatedCIDRReadsTheAddress(t *testing.T) { + got, err := FromAllocatedCIDR("2607:ed40:70::1/128") + if err != nil { + t.Fatalf("read the address: %v", err) + } + if got.String() != "2607:ed40:70::1" { + t.Errorf("address = %q, want the host address without its prefix length", got.String()) + } +} + +// Every one of these is an answer the service gave that cannot be used as a +// shard address. Reading one anyway would write it into a field that cannot be +// corrected. +func TestFromAllocatedCIDRRefusesWhatIsNotAShardAddress(t *testing.T) { + for _, tc := range []struct { + name string + cidr string + }{ + {"not a prefix", "2607:ed40:70::1"}, + {"empty", ""}, + {"a block rather than an address", "2607:ed40:70::/64"}, + {"the wrong family", "198.51.100.7/32"}, + {"an IPv4 address in IPv6 clothing", "::ffff:198.51.100.7/128"}, + } { + t.Run(tc.name, func(t *testing.T) { + got, err := FromAllocatedCIDR(tc.cidr) + if err == nil { + t.Fatalf("%q was accepted as a shard address (%s)", tc.cidr, got) + } + var unusable *UnusableError + if !errors.As(err, &unusable) { + t.Errorf("error = %v; an answer that cannot be used is a wait on an operator, not a retry", err) + } + }) + } +} From 56f2c9ebdf53bd4a83a96dcb22dbfea0b1f9ad73 Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Thu, 17 Sep 2026 20:55:24 -0500 Subject: [PATCH 2/6] chore: Pin egress dependencies to their branches CI checks out this repo alone, so a module replace pointing at a local path cannot resolve. Point both at the pushed commits instead, which CI can fetch, so the build reflects the branches this depends on. Key changes: - Replace the network module with the pushed egress address commit - Replace the operator module with the pushed egress API commit - Drop the local workspace file in favour of resolvable versions Revert both replaces once those modules release the fields. Co-Authored-By: Claude Opus 5 (1M context) --- go.mod | 4 ++++ go.sum | 8 ++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index 0431086..52a4709 100644 --- a/go.mod +++ b/go.mod @@ -78,3 +78,7 @@ require ( sigs.k8s.io/structured-merge-diff/v6 v6.3.3 // indirect sigs.k8s.io/yaml v1.6.0 // indirect ) + +replace go.datum.net/network => github.com/datum-cloud/network v0.1.1-0.20260917203135-45eb71a1eb0c + +replace go.datum.net/network-services-operator => github.com/datum-cloud/network-services-operator v0.27.2-0.20260917225730-eccf0e8922b2 diff --git a/go.sum b/go.sum index 6c1b0e7..32ea3ee 100644 --- a/go.sum +++ b/go.sum @@ -4,6 +4,10 @@ github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= +github.com/datum-cloud/network v0.1.1-0.20260917203135-45eb71a1eb0c h1:NT1bxxVHTzqathHdkh4zKmBqlLLXkX5N39+IjcWeitY= +github.com/datum-cloud/network v0.1.1-0.20260917203135-45eb71a1eb0c/go.mod h1:dqzM8WZczbiZ9bCvsxjkoI10GJqQ24NVWnc9boXgOkE= +github.com/datum-cloud/network-services-operator v0.27.2-0.20260917225730-eccf0e8922b2 h1:2yKJV4XRmoQMNM+VrOJdUgNkP1pM5z6l2P/2qF+K5yI= +github.com/datum-cloud/network-services-operator v0.27.2-0.20260917225730-eccf0e8922b2/go.mod h1:9nuuBWdrkdnIBMaWJsWM3j4CcbKJIF1GDuCmAjdpIHo= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= @@ -121,10 +125,6 @@ github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg= go.datum.net/compute v0.8.0 h1:/v1lni/oO4KwthPeXhhS0+VFRZjWYbU6CXiF3RgCMzY= go.datum.net/compute v0.8.0/go.mod h1:u7YIQX4+Wgts5XJwtvlz5NMs23g7+R/4TkAmv3DRycw= -go.datum.net/network v0.1.0 h1:AmYSwxUWOk26UnK6S6NA7OuucGJniKo/CWqjs+VcSCs= -go.datum.net/network v0.1.0/go.mod h1:dqzM8WZczbiZ9bCvsxjkoI10GJqQ24NVWnc9boXgOkE= -go.datum.net/network-services-operator v0.27.0 h1:LYCUjc6i0/f3c5YXSgisnbV8gB3R8emDwNfiYapdCAo= -go.datum.net/network-services-operator v0.27.0/go.mod h1:9nuuBWdrkdnIBMaWJsWM3j4CcbKJIF1GDuCmAjdpIHo= go.miloapis.com/ipam v0.4.0 h1:U+mg3RMFXj0c2eQ0Lm15CI0bYBPFDqy8IsGLN8v/eGs= go.miloapis.com/ipam v0.4.0/go.mod h1:Jj7xg4lJi9psE0+4PuOg/GQOG8rG13h112xYoM994rc= go.miloapis.com/locations v0.0.1 h1:voJKqBzyLX5x96M3+5y/ga7fgq9/+vypTizd+bBvqUY= From 9fc7b1c398d32a0dc3b62fdf7e7fc371cf8deb36 Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Thu, 17 Sep 2026 21:31:20 -0500 Subject: [PATCH 3/6] feat: Report which record holds a shard's address Claiming an address now reports the record in the addressing service that holds it, alongside the address itself. Nothing is written to the shard yet: the spec field that will carry the reference is not published, so this is the plumbing a correct reference needs and not the reference. The record has to be reported rather than recomputed by whoever wants it. The service overwrites spec.ownerRef on a claim with the requesting project's identity, so a claim cannot say which shard it was made for, and the only place that link can live is on the shard pointing back. The adopt path is why a holder carries its kind. When an allocation retained by an earlier claim already occupies a claim's identity, the service rolls its transaction back before refusing, so the claim it refused is never stored. The address is recovered from the IPAllocation the refusal names, and that name is a hash of the claim's namespace and name rather than the claim name, so it cannot be derived from the shard. A reference built from the shard's own name would, in exactly this case, name an object that does not exist. Key changes: - Return a Holding from egressaddress.Claim carrying the address and the namespace, kind and name of the record holding it, taken from the object actually stored rather than recomputed - Report a retained allocation as held by KindIPAllocation, under the name the refusal carried - Report nothing at all for an unbound claim, so there is no partial holder to write into a field that cannot be rewritten - Assign the address in one update, so a rule pairing it with the record holding it is satisfied by that write rather than by a second one that could fail on its own - Log the holder as a stopgap for auditability. A log line is not queryable and ages out; it is not the record Co-Authored-By: Claude Opus 5 (1M context) --- .../egressshardaddress_controller.go | 31 ++-- internal/egressaddress/address.go | 86 ++++++++-- internal/egressaddress/address_test.go | 152 ++++++++++++++++++ 3 files changed, 249 insertions(+), 20 deletions(-) diff --git a/internal/controller/egressshardaddress_controller.go b/internal/controller/egressshardaddress_controller.go index b9c5b6d..c52110c 100644 --- a/internal/controller/egressshardaddress_controller.go +++ b/internal/controller/egressshardaddress_controller.go @@ -21,7 +21,6 @@ import ( "context" "errors" "fmt" - "net/netip" apierrors "k8s.io/apimachinery/pkg/api/errors" ctrl "sigs.k8s.io/controller-runtime" @@ -107,7 +106,7 @@ func (r *EgressShardAddressReconciler) Reconcile(ctx context.Context, req ctrl.R return ctrl.Result{}, r.markFamilyServed(ctx, shard) } - address, err := r.claim(ctx, shard) + holding, err := r.claim(ctx, shard) if err != nil { return ctrl.Result{}, err } @@ -116,7 +115,11 @@ func (r *EgressShardAddressReconciler) Reconcile(ctx context.Context, req ctrl.R // could not produce one returned above, because spec.shardAddressIPv6 is // write-once: a placeholder written here is not correctable, and the shard // would have to be deleted and recreated to be rid of it. - shard.Spec.ShardAddressIPv6 = address.String() + // + // One update carries everything the shard gains from this allocation, so + // that a rule pairing the address with the record holding it is satisfied + // by the write rather than by a second one that could fail on its own. + shard.Spec.ShardAddressIPv6 = holding.Address.String() if shard.Labels == nil { shard.Labels = map[string]string{} } @@ -128,12 +131,18 @@ func (r *EgressShardAddressReconciler) Reconcile(ctx context.Context, req ctrl.R // ownership of a field that cannot be reassigned is the one thing that must // not happen quietly here. if err := r.Client.Update(ctx, shard); err != nil { - return ctrl.Result{}, fmt.Errorf("assign egress shard %q the address %s: %w", - shard.Name, address, err) + return ctrl.Result{}, fmt.Errorf("assign egress shard %q the address %s held by %s %q: %w", + shard.Name, holding.Address, holding.Kind, holding.Name, err) } + // The holding record is logged rather than recorded on the shard, because + // the field that will carry it does not exist yet. It is what makes the + // address auditable: a claim cannot say which shard it was made for, so + // without this the link exists nowhere at all. A log line is a stopgap -- + // it is not queryable and it ages out. log.FromContext(ctx).Info("assigned an egress shard its public IPv6 address", - "shard", shard.Name, "address", address.String(), "location", r.Location) + "shard", shard.Name, "address", holding.Address.String(), "location", r.Location, + "holderKind", holding.Kind, "holderNamespace", holding.Namespace, "holderName", holding.Name) return ctrl.Result{}, nil } @@ -157,13 +166,13 @@ func (r *EgressShardAddressReconciler) markFamilyServed(ctx context.Context, sha func (r *EgressShardAddressReconciler) claim( ctx context.Context, shard *bgpv1alpha1.EgressShard, -) (netip.Addr, error) { +) (egressaddress.Holding, error) { ipamClient, err := r.IPAM.ClientForPlatform() if err != nil { - return netip.Addr{}, fmt.Errorf("reach the public address space: %w", err) + return egressaddress.Holding{}, fmt.Errorf("reach the public address space: %w", err) } - address, err := egressaddress.Claim(ctx, ipamClient, egressaddress.Request{ + holding, err := egressaddress.Claim(ctx, ipamClient, egressaddress.Request{ ClassName: r.AddressClassIPv6, Namespace: r.ClaimNamespace, Location: r.Location, @@ -180,9 +189,9 @@ func (r *EgressShardAddressReconciler) claim( log.FromContext(ctx).Error(err, "the public address space handed out something no shard address can be read from", "shard", shard.Name, "location", r.Location) } - return netip.Addr{}, fmt.Errorf("claim a public IPv6 address for egress shard %q: %w", shard.Name, err) + return egressaddress.Holding{}, fmt.Errorf("claim a public IPv6 address for egress shard %q: %w", shard.Name, err) } - return address, nil + return holding, nil } // release gives back the address of a shard that is gone. diff --git a/internal/egressaddress/address.go b/internal/egressaddress/address.go index 3e93ac2..805fd34 100644 --- a/internal/egressaddress/address.go +++ b/internal/egressaddress/address.go @@ -87,6 +87,46 @@ const ( maxClaimNameLength = 253 ) +// Holding is a shard's address together with the record in the addressing +// service that holds it. +// +// The record has to be reported, not just the address, because the service +// overwrites spec.ownerRef on a claim with the requesting project's identity. +// A claim therefore cannot say which shard it was made for, and the only place +// that link can live is on the shard, pointing back. +// +// Kind is part of it rather than assumed. An address recovered from a retained +// allocation is held by an IPAllocation and by no claim at all: the service +// rolls the transaction back before refusing, so the claim it refused was +// never stored. A reference that named a claim in that case would name an +// object that does not exist. +type Holding struct { + // Address is the address the shard translates to. + Address netip.Addr + + // Namespace is the namespace in the platform's tenancy holding the record. + Namespace string + + // Kind is KindIPClaim or KindIPAllocation. + Kind string + + // Name is the record's name. For a claim it is derived from the shard; for + // a retained allocation it is the name the service's refusal carried, and + // is never computed here. + Name string +} + +// The kinds a Holding can name. +const ( + KindIPClaim = "IPClaim" + KindIPAllocation = "IPAllocation" +) + +// HeldByClaim reports whether a live claim holds the address. False means the +// address was recovered from an allocation a released claim retained, and no +// claim object exists to point at. +func (h Holding) HeldByClaim() bool { return h.Kind == KindIPClaim } + // Request names one shard's claim on the public address space. type Request struct { // ClassName is the class that hands out shard addresses. @@ -131,7 +171,7 @@ func ClaimName(shardNamespace, shardName string) string { // The service binds on create and refuses a duplicate name, so the read comes // first. That is what makes the allocation idempotent without this recording // anything of its own. -func Claim(ctx context.Context, ipamClient client.Client, request Request) (netip.Addr, error) { +func Claim(ctx context.Context, ipamClient client.Client, request Request) (Holding, error) { ipClaim := &ipamv1alpha1.IPClaim{} ipClaim.Namespace = request.Namespace ipClaim.Name = ClaimName(request.ShardNamespace, request.ShardName) @@ -173,7 +213,7 @@ func Claim(ctx context.Context, ipamClient client.Client, request Request) (neti existing := &ipamv1alpha1.IPClaim{} getErr := ipamClient.Get(ctx, client.ObjectKeyFromObject(ipClaim), existing) if getErr != nil && !apierrors.IsNotFound(getErr) { - return netip.Addr{}, fmt.Errorf("read the egress address claim %q: %w", ipClaim.Name, getErr) + return Holding{}, fmt.Errorf("read the egress address claim %q: %w", ipClaim.Name, getErr) } if getErr == nil { @@ -193,7 +233,7 @@ func Claim(ctx context.Context, ipamClient client.Client, request Request) (neti // before calling this a failure to allocate. raced := &ipamv1alpha1.IPClaim{} if err := ipamClient.Get(ctx, client.ObjectKeyFromObject(ipClaim), raced); err != nil { - return netip.Addr{}, fmt.Errorf("claim an egress address: %w", createErr) + return Holding{}, fmt.Errorf("claim an egress address: %w", createErr) } ipClaim = raced } @@ -202,13 +242,24 @@ func Claim(ctx context.Context, ipamClient client.Client, request Request) (neti // Not an error about the address: the claim exists and holds nothing // yet. The caller must write nothing, because the field it would write // cannot be corrected afterwards. - return netip.Addr{}, &UnboundError{ + return Holding{}, &UnboundError{ claimName: ipClaim.Name, phase: string(ipClaim.Status.Phase), } } - return FromAllocatedCIDR(ipClaim.Status.AllocatedCIDR) + address, err := FromAllocatedCIDR(ipClaim.Status.AllocatedCIDR) + if err != nil { + return Holding{}, err + } + // The claim this address was read out of, by the name it was actually + // stored under -- not the name a caller would recompute. + return Holding{ + Address: address, + Namespace: ipClaim.Namespace, + Kind: KindIPClaim, + Name: ipClaim.Name, + }, nil } // Release gives the shard's address back. @@ -232,16 +283,33 @@ func Release(ctx context.Context, ipamClient client.Client, namespace, shardName // adopt reads the address out of an allocation this shard already holds. The // allocation outlives the claim that made it, which is what retention is for, // so the address it names is the one this shard has always had. -func adopt(ctx context.Context, ipamClient client.Client, namespace, allocationName string) (netip.Addr, error) { +// +// It returns the allocation as the holding record. No claim exists to return: +// the service rolls its transaction back before answering with this refusal. +func adopt(ctx context.Context, ipamClient client.Client, namespace, allocationName string) (Holding, error) { allocation := &ipamv1alpha1.IPAllocation{} if err := ipamClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: allocationName}, allocation); err != nil { - return netip.Addr{}, fmt.Errorf("read the retained allocation %q: %w", allocationName, err) + return Holding{}, fmt.Errorf("read the retained allocation %q: %w", allocationName, err) } if allocation.Status.AllocatedCIDR == "" { - return netip.Addr{}, &UnboundError{claimName: allocationName, phase: string(allocation.Status.Phase)} + return Holding{}, &UnboundError{claimName: allocationName, phase: string(allocation.Status.Phase)} + } + address, err := FromAllocatedCIDR(allocation.Status.AllocatedCIDR) + if err != nil { + return Holding{}, err } - return FromAllocatedCIDR(allocation.Status.AllocatedCIDR) + // The allocation, never the claim. The service rolled its transaction back + // before refusing the create, so the claim whose name this was derived + // from was never stored and pointing at it would point at nothing. The + // name is the one the refusal carried, so it names the object that is + // actually there. + return Holding{ + Address: address, + Namespace: namespace, + Kind: KindIPAllocation, + Name: allocationName, + }, nil } // FromAllocatedCIDR reads the shard's address out of what the service handed diff --git a/internal/egressaddress/address_test.go b/internal/egressaddress/address_test.go index 2c7178f..f8c197f 100644 --- a/internal/egressaddress/address_test.go +++ b/internal/egressaddress/address_test.go @@ -18,9 +18,20 @@ along with this program. If not, see . package egressaddress import ( + "context" "errors" + "fmt" + "net/netip" "strings" "testing" + + ipamv1alpha1 "go.miloapis.com/ipam/pkg/apis/ipam/v1alpha1" + "go.miloapis.com/ipam/pkg/ipamerrors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" ) // A claim name collision is two shards sharing one address, so the split @@ -95,3 +106,144 @@ func TestFromAllocatedCIDRRefusesWhatIsNotAShardAddress(t *testing.T) { }) } } + +const ( + probeNamespace = "default" + probeShardNamespace = "galactic-system" + probeShardName = "worker-8b4e1647-dfw" +) + +func probeRequest() Request { + return Request{ + ClassName: "datum-egress-shard-address-ipv6", + Namespace: probeNamespace, + Location: "us-central-1", + ShardNamespace: probeShardNamespace, + ShardName: probeShardName, + } +} + +func ipamScheme(t *testing.T) *runtime.Scheme { + t.Helper() + scheme := runtime.NewScheme() + if err := ipamv1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("build the IPAM scheme: %v", err) + } + return scheme +} + +// A fresh claim reports itself as the record holding the address, under the +// name it was actually stored as. +func TestAFreshClaimIsReportedAsTheHolder(t *testing.T) { + bind := interceptor.Funcs{ + Create: func(ctx context.Context, c client.WithWatch, object client.Object, opts ...client.CreateOption) error { + ipClaim, ok := object.(*ipamv1alpha1.IPClaim) + if !ok { + return c.Create(ctx, object, opts...) + } + ipClaim.Status.Phase = ipamv1alpha1.ClaimPhase("Bound") + ipClaim.Status.AllocatedCIDR = "2001:db8:100::7/128" + return c.Create(ctx, ipClaim, opts...) + }, + } + ipamClient := fake.NewClientBuilder().WithScheme(ipamScheme(t)).WithInterceptorFuncs(bind).Build() + + holding, err := Claim(context.Background(), ipamClient, probeRequest()) + if err != nil { + t.Fatalf("claim: %v", err) + } + if !holding.HeldByClaim() { + t.Errorf("kind = %q, want the address held by a claim", holding.Kind) + } + if want := ClaimName(probeShardNamespace, probeShardName); holding.Name != want { + t.Errorf("holder name = %q, want the claim named for the shard %q", holding.Name, want) + } + if holding.Namespace != probeNamespace { + t.Errorf("holder namespace = %q, want %q", holding.Namespace, probeNamespace) + } + if holding.Address != netip.MustParseAddr("2001:db8:100::7") { + t.Errorf("address = %s, want the allocated address", holding.Address) + } +} + +// The case most likely to record a reference to something that does not exist. +// +// The service rolls its transaction back before refusing a claim whose identity +// a retained allocation already occupies, so the claim is NEVER stored. The +// holder is the allocation, named as the refusal named it -- and the allocation +// name is a hash of the claim's namespace and name, so it is not the claim name +// and cannot be derived from the shard. +func TestAnAdoptedAddressIsReportedAsHeldByTheAllocation(t *testing.T) { + claimName := ClaimName(probeShardNamespace, probeShardName) + // A hash, as the service computes it -- deliberately unlike the claim name. + const allocationName = "alloc-3f2a1b0c9d8e7f60" + + retained := &ipamv1alpha1.IPAllocation{ + ObjectMeta: metav1.ObjectMeta{Namespace: probeNamespace, Name: allocationName}, + Status: ipamv1alpha1.IPAllocationStatus{AllocatedCIDR: "2001:db8:100::abcd/128"}, + } + + refuse := interceptor.Funcs{ + Create: func(ctx context.Context, c client.WithWatch, object client.Object, opts ...client.CreateOption) error { + if _, ok := object.(*ipamv1alpha1.IPClaim); ok { + return ipamerrors.NewRetainedAllocation( + ipamv1alpha1.Resource("ipclaims"), claimName, allocationName, + fmt.Sprintf("an allocation under this identity already exists: IPAllocation %q", allocationName)) + } + return c.Create(ctx, object, opts...) + }, + } + ipamClient := fake.NewClientBuilder(). + WithScheme(ipamScheme(t)). + WithObjects(retained). + WithInterceptorFuncs(refuse). + Build() + + holding, err := Claim(context.Background(), ipamClient, probeRequest()) + if err != nil { + t.Fatalf("claim: %v", err) + } + + if holding.HeldByClaim() { + t.Fatalf("kind = %q; no claim was stored, so naming one names an object that does not exist", holding.Kind) + } + if holding.Kind != KindIPAllocation { + t.Errorf("kind = %q, want %q", holding.Kind, KindIPAllocation) + } + if holding.Name != allocationName { + t.Fatalf("holder name = %q, want the allocation the refusal named %q", holding.Name, allocationName) + } + if holding.Name == claimName { + t.Fatal("the adopted address was attributed to the claim that was refused and never stored") + } + if holding.Address != netip.MustParseAddr("2001:db8:100::abcd") { + t.Errorf("address = %s, want the retained address", holding.Address) + } +} + +// An unbound claim reports no holder at all, because there is nothing to +// attribute yet and the fields it would be written into cannot be rewritten. +func TestAnUnboundClaimReportsNoHolder(t *testing.T) { + hold := interceptor.Funcs{ + Create: func(ctx context.Context, c client.WithWatch, object client.Object, opts ...client.CreateOption) error { + if ipClaim, ok := object.(*ipamv1alpha1.IPClaim); ok { + ipClaim.Status.Phase = ipamv1alpha1.ClaimPhase("Pending") + return c.Create(ctx, ipClaim, opts...) + } + return c.Create(ctx, object, opts...) + }, + } + ipamClient := fake.NewClientBuilder().WithScheme(ipamScheme(t)).WithInterceptorFuncs(hold).Build() + + holding, err := Claim(context.Background(), ipamClient, probeRequest()) + if err == nil { + t.Fatal("an unbound claim was reported as holding an address") + } + var unbound *UnboundError + if !errors.As(err, &unbound) { + t.Errorf("error = %v, want an unbound claim worth retrying", err) + } + if holding != (Holding{}) { + t.Errorf("holding = %+v, want nothing to attribute", holding) + } +} From fb4e3909d67cf5faedc44dcee5f6ec89f42ac873 Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Thu, 17 Sep 2026 21:37:39 -0500 Subject: [PATCH 4/6] feat: Record the claim behind a shard's address A shard now carries the addressing-service record its public IPv6 address came from, written in the same update as the address itself. The address alone is unattributable: the service overwrites a claim's spec.ownerRef with the requesting project's identity, so a claim cannot say which shard it was made for and the trail has to run from the shard back. The reference names the record that exists, which is not always a claim. An address recovered from a retained allocation is held by an IPAllocation and by no claim at all -- the service rolls its transaction back before refusing the create, so the claim it refused was never stored, and its name is a hash of the claim's namespace and name rather than the claim name. Kind carries that difference instead of taking its default, because both fields are write-once and a reference naming a claim that does not exist would be permanent for the shard's lifetime. Nothing gains a reference it did not earn. An address assigned by hand has no claim behind it and stays unattributable rather than gaining one this controller invented, and a claim holding no address yet writes neither field. Key changes: - Write spec.shardAddressIPv6ClaimRef beside the address in one update. The API pairs neither field with the other, so this is for atomicity rather than to satisfy validation: two writes would leave a window where the address is recorded and its origin is not - Set Kind from the record actually read, IPClaim or IPAllocation, rather than accepting the IPClaim default - Carry the platform project on the reconciler and wire it from the existing -platform-project flag. The reference requires it: a claim is namespaced within a project and this reference is read from outside every project, so a namespace alone does not identify one - Refuse to start without it, alongside the existing required values - Repin go.datum.net/network to affb112, which publishes AddressClaimRef and the field Co-Authored-By: Claude Opus 5 (1M context) --- cmd/egress-address-controller/main.go | 1 + go.mod | 2 +- go.sum | 4 +- .../egressshardaddress_controller.go | 39 +++- .../egressshardaddress_controller_test.go | 189 +++++++++++++++++- 5 files changed, 218 insertions(+), 17 deletions(-) diff --git a/cmd/egress-address-controller/main.go b/cmd/egress-address-controller/main.go index 7fcdb73..4b7e61d 100644 --- a/cmd/egress-address-controller/main.go +++ b/cmd/egress-address-controller/main.go @@ -145,6 +145,7 @@ func main() { IPAM: ipamClients, AddressClassIPv6: addressClass, ClaimNamespace: claimNamespace, + PlatformProject: platformProject, Location: location, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "EgressShardAddress") diff --git a/go.mod b/go.mod index 52a4709..eeb3770 100644 --- a/go.mod +++ b/go.mod @@ -79,6 +79,6 @@ require ( sigs.k8s.io/yaml v1.6.0 // indirect ) -replace go.datum.net/network => github.com/datum-cloud/network v0.1.1-0.20260917203135-45eb71a1eb0c +replace go.datum.net/network => github.com/datum-cloud/network v0.1.1-0.20260918023140-affb112b4eea replace go.datum.net/network-services-operator => github.com/datum-cloud/network-services-operator v0.27.2-0.20260917225730-eccf0e8922b2 diff --git a/go.sum b/go.sum index 32ea3ee..83d1e4b 100644 --- a/go.sum +++ b/go.sum @@ -4,8 +4,8 @@ github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= -github.com/datum-cloud/network v0.1.1-0.20260917203135-45eb71a1eb0c h1:NT1bxxVHTzqathHdkh4zKmBqlLLXkX5N39+IjcWeitY= -github.com/datum-cloud/network v0.1.1-0.20260917203135-45eb71a1eb0c/go.mod h1:dqzM8WZczbiZ9bCvsxjkoI10GJqQ24NVWnc9boXgOkE= +github.com/datum-cloud/network v0.1.1-0.20260918023140-affb112b4eea h1:9E4t6nhRgLwRSlprORVRMz9Hkjez8T93QnZesITnfNA= +github.com/datum-cloud/network v0.1.1-0.20260918023140-affb112b4eea/go.mod h1:dqzM8WZczbiZ9bCvsxjkoI10GJqQ24NVWnc9boXgOkE= github.com/datum-cloud/network-services-operator v0.27.2-0.20260917225730-eccf0e8922b2 h1:2yKJV4XRmoQMNM+VrOJdUgNkP1pM5z6l2P/2qF+K5yI= github.com/datum-cloud/network-services-operator v0.27.2-0.20260917225730-eccf0e8922b2/go.mod h1:9nuuBWdrkdnIBMaWJsWM3j4CcbKJIF1GDuCmAjdpIHo= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= diff --git a/internal/controller/egressshardaddress_controller.go b/internal/controller/egressshardaddress_controller.go index c52110c..f052c6e 100644 --- a/internal/controller/egressshardaddress_controller.go +++ b/internal/controller/egressshardaddress_controller.go @@ -22,6 +22,7 @@ import ( "errors" "fmt" + ipamv1alpha1 "go.miloapis.com/ipam/pkg/apis/ipam/v1alpha1" apierrors "k8s.io/apimachinery/pkg/api/errors" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -67,6 +68,14 @@ type EgressShardAddressReconciler struct { // address claims are written to. ClaimNamespace string + // PlatformProject is the project whose control plane serves the claims. + // + // It is carried here as well as inside the IPAM client factory because the + // reference written onto a shard has to name it: a claim is namespaced + // within a project, and the reference is read from outside every project, + // so a namespace alone does not identify one. + PlatformProject string + // Location is the location this cell serves. It selects the shared public // range the address comes from, and two cells serving one location draw // from the same range. @@ -120,6 +129,7 @@ func (r *EgressShardAddressReconciler) Reconcile(ctx context.Context, req ctrl.R // that a rule pairing the address with the record holding it is satisfied // by the write rather than by a second one that could fail on its own. shard.Spec.ShardAddressIPv6 = holding.Address.String() + shard.Spec.ShardAddressIPv6ClaimRef = r.reference(holding) if shard.Labels == nil { shard.Labels = map[string]string{} } @@ -135,17 +145,30 @@ func (r *EgressShardAddressReconciler) Reconcile(ctx context.Context, req ctrl.R shard.Name, holding.Address, holding.Kind, holding.Name, err) } - // The holding record is logged rather than recorded on the shard, because - // the field that will carry it does not exist yet. It is what makes the - // address auditable: a claim cannot say which shard it was made for, so - // without this the link exists nowhere at all. A log line is a stopgap -- - // it is not queryable and it ages out. log.FromContext(ctx).Info("assigned an egress shard its public IPv6 address", "shard", shard.Name, "address", holding.Address.String(), "location", r.Location, "holderKind", holding.Kind, "holderNamespace", holding.Namespace, "holderName", holding.Name) return ctrl.Result{}, nil } +// reference is the trail from the address back to what holds it. +// +// It names the record that actually exists, which is not always a claim. An +// address recovered from a retained allocation is held by an IPAllocation and +// by no claim: the service rolls its transaction back before refusing, so the +// claim it refused was never stored. Kind carries that difference rather than +// taking its default, because the field is write-once and a reference naming a +// claim that does not exist is permanent for the life of the shard. +func (r *EgressShardAddressReconciler) reference(holding egressaddress.Holding) *bgpv1alpha1.AddressClaimRef { + return &bgpv1alpha1.AddressClaimRef{ + APIGroup: ipamv1alpha1.GroupName, + Kind: holding.Kind, + Project: r.PlatformProject, + Namespace: holding.Namespace, + Name: holding.Name, + } +} + // markFamilyServed records that this shard translates IPv6, for the selectors // that place traffic on it. A selector matches labels and cannot read a spec // field, so whoever assigns the address states it here too. @@ -231,6 +254,12 @@ func (r *EgressShardAddressReconciler) SetupWithManager(mgr ctrl.Manager) error if r.ClaimNamespace == "" { return errors.New("a namespace to write claims in is required") } + if r.PlatformProject == "" { + // Every reference written onto a shard names it, and the field is + // write-once: a reference missing the project is permanent and + // resolves nowhere. + return errors.New("the project serving the claims is required") + } if r.Location == "" { // A claim carrying no location is refused by the service, and one // carrying the wrong location draws from another location's range and diff --git a/internal/controller/egressshardaddress_controller_test.go b/internal/controller/egressshardaddress_controller_test.go index 49c6716..e776735 100644 --- a/internal/controller/egressshardaddress_controller_test.go +++ b/internal/controller/egressshardaddress_controller_test.go @@ -41,11 +41,12 @@ import ( ) const ( - testShardNamespace = "galactic-system" - testShardName = "worker-8b4e1647-dfw" - testLocation = "us-central-1" - testAddressClass = "datum-egress-shard-address-ipv6" - testClaimNamespace = "default" + testShardNamespace = "galactic-system" + testShardName = "worker-8b4e1647-dfw" + testLocation = "us-central-1" + testAddressClass = "datum-egress-shard-address-ipv6" + testClaimNamespace = "default" + testPlatformProject = "datum-cloud" ) // fakeAddressIPAM stands in for the address service. Allocation is synchronous @@ -189,6 +190,7 @@ func reconcilerFor(cell client.Client, service *fakeAddressIPAM) *EgressShardAdd IPAM: service, AddressClassIPv6: testAddressClass, ClaimNamespace: testClaimNamespace, + PlatformProject: testPlatformProject, Location: testLocation, } } @@ -440,10 +442,11 @@ func TestSetupRefusesADeploymentThatCannotClaimCorrectly(t *testing.T) { name string reconciler *EgressShardAddressReconciler }{ - {"no class", &EgressShardAddressReconciler{ClaimNamespace: "default", Location: testLocation, IPAM: &fakeAddressIPAM{}}}, - {"no location", &EgressShardAddressReconciler{AddressClassIPv6: testAddressClass, ClaimNamespace: "default", IPAM: &fakeAddressIPAM{}}}, - {"no namespace", &EgressShardAddressReconciler{AddressClassIPv6: testAddressClass, Location: testLocation, IPAM: &fakeAddressIPAM{}}}, - {"no address space", &EgressShardAddressReconciler{AddressClassIPv6: testAddressClass, ClaimNamespace: "default", Location: testLocation}}, + {"no class", &EgressShardAddressReconciler{ClaimNamespace: "default", PlatformProject: testPlatformProject, Location: testLocation, IPAM: &fakeAddressIPAM{}}}, + {"no location", &EgressShardAddressReconciler{AddressClassIPv6: testAddressClass, ClaimNamespace: "default", PlatformProject: testPlatformProject, IPAM: &fakeAddressIPAM{}}}, + {"no namespace", &EgressShardAddressReconciler{AddressClassIPv6: testAddressClass, PlatformProject: testPlatformProject, Location: testLocation, IPAM: &fakeAddressIPAM{}}}, + {"no project", &EgressShardAddressReconciler{AddressClassIPv6: testAddressClass, ClaimNamespace: "default", Location: testLocation, IPAM: &fakeAddressIPAM{}}}, + {"no address space", &EgressShardAddressReconciler{AddressClassIPv6: testAddressClass, ClaimNamespace: "default", PlatformProject: testPlatformProject, Location: testLocation}}, } { t.Run(tc.name, func(t *testing.T) { if err := tc.reconciler.SetupWithManager(nil); err == nil { @@ -515,3 +518,171 @@ func TestTheAddressIsReadFromAllocatedCIDRNotStatusAddress(t *testing.T) { t.Fatal("the shard was left unaddressed by a claim whose allocatedCIDR was set") } } + +// The address alone is unattributable: the service overwrites a claim's +// ownerRef with the requesting project's identity, so the only trail from a +// translating address to the allocation accountable for it is this reference. +func TestAFreshlyClaimedAddressRecordsItsClaim(t *testing.T) { + cell := newShardCell(t, shard(testShardName)) + service := newFakeAddressIPAM(t) + + if _, err := reconcilerFor(cell, service).Reconcile(context.Background(), + requestFor(testShardName)); err != nil { + t.Fatalf("reconcile: %v", err) + } + + got := readShard(t, cell, testShardName) + ref := got.Spec.ShardAddressIPv6ClaimRef + if ref == nil { + t.Fatal("the address was assigned with no trail back to what holds it") + } + if ref.Kind != egressaddress.KindIPClaim { + t.Errorf("kind = %q, want %q for an address a live claim holds", ref.Kind, egressaddress.KindIPClaim) + } + if want := egressaddress.ClaimName(testShardNamespace, testShardName); ref.Name != want { + t.Errorf("name = %q, want the claim named for the shard %q", ref.Name, want) + } + if ref.Namespace != testClaimNamespace { + t.Errorf("namespace = %q, want %q", ref.Namespace, testClaimNamespace) + } + // Required by the API, and a reference without it resolves nowhere. + if ref.Project != testPlatformProject { + t.Errorf("project = %q, want %q", ref.Project, testPlatformProject) + } + if ref.APIGroup != ipamv1alpha1.GroupName { + t.Errorf("apiGroup = %q, want %q", ref.APIGroup, ipamv1alpha1.GroupName) + } + // The reference must name the claim the address actually came from, which + // is the one the service was asked to bind. + if len(service.created) != 1 || service.created[0] != ref.Name { + t.Errorf("the reference names %q but the claims created were %v", ref.Name, service.created) + } +} + +// The case most likely to record something that does not exist. Adopting an +// address held by a retained allocation means no claim was ever stored, so the +// reference has to name the allocation. Both fields are write-once, so naming +// the refused claim would be permanent for this shard's lifetime. +func TestAnAdoptedAddressRecordsTheAllocationItCameFrom(t *testing.T) { + claimName := egressaddress.ClaimName(testShardNamespace, testShardName) + allocationName := allocationNameForClaim(claimName) + const held = "2001:db8:100::abcd/128" + + retained := &ipamv1alpha1.IPAllocation{ + ObjectMeta: metav1.ObjectMeta{Namespace: testClaimNamespace, Name: allocationName}, + Status: ipamv1alpha1.IPAllocationStatus{AllocatedCIDR: held}, + } + + cell := newShardCell(t, shard(testShardName)) + service := newFakeAddressIPAM(t) + service.retained[allocationName] = held + if err := service.client.Create(context.Background(), retained); err != nil { + t.Fatalf("seed the retained allocation: %v", err) + } + + if _, err := reconcilerFor(cell, service).Reconcile(context.Background(), + requestFor(testShardName)); err != nil { + t.Fatalf("reconcile: %v", err) + } + + got := readShard(t, cell, testShardName) + ref := got.Spec.ShardAddressIPv6ClaimRef + if ref == nil { + t.Fatal("an adopted address was assigned with no trail back to what holds it") + } + if ref.Name == claimName { + t.Fatal("the reference names the claim the service refused and never stored") + } + if ref.Name != allocationName { + t.Errorf("name = %q, want the allocation the refusal named %q", ref.Name, allocationName) + } + if ref.Kind != egressaddress.KindIPAllocation { + t.Errorf("kind = %q, want %q; no claim exists to point at", ref.Kind, egressaddress.KindIPAllocation) + } + if got.Spec.ShardAddressIPv6 != "2001:db8:100::abcd" { + t.Errorf("address = %q, want the retained address", got.Spec.ShardAddressIPv6) + } +} + +// Both fields are write-once, so a shard already carrying them is read and left +// exactly as it is. Reconciling one must not draw a second address, and must +// not attempt a rewrite the API would refuse. +func TestAShardCarryingBothIsLeftUntouched(t *testing.T) { + existing := shard(testShardName) + existing.Spec.ShardAddressIPv6 = "2001:db8:100::dead" + existing.Spec.ShardAddressIPv6ClaimRef = &bgpv1alpha1.AddressClaimRef{ + APIGroup: ipamv1alpha1.GroupName, + Kind: egressaddress.KindIPClaim, + Project: testPlatformProject, + Namespace: testClaimNamespace, + Name: "a-claim-someone-else-made", + } + existing.Labels = map[string]string{ + bgpv1alpha1.LabelEgressShardIPv6: bgpv1alpha1.LabelValueEgressFamilyServed, + } + cell := newShardCell(t, existing) + service := newFakeAddressIPAM(t) + + if _, err := reconcilerFor(cell, service).Reconcile(context.Background(), + requestFor(testShardName)); err != nil { + t.Fatalf("reconcile: %v", err) + } + + if len(service.created) != 0 { + t.Errorf("an addressed shard drew %v from the public range", service.created) + } + got := readShard(t, cell, testShardName) + if got.Spec.ShardAddressIPv6 != "2001:db8:100::dead" { + t.Errorf("address = %q, want it untouched", got.Spec.ShardAddressIPv6) + } + if got.Spec.ShardAddressIPv6ClaimRef == nil || got.Spec.ShardAddressIPv6ClaimRef.Name != "a-claim-someone-else-made" { + t.Errorf("reference = %+v, want it untouched", got.Spec.ShardAddressIPv6ClaimRef) + } +} + +// An address an operator assigned by hand has no claim behind it, so there is +// nothing truthful to reference. It stays unattributable rather than gaining a +// reference this controller invented for an allocation it never made -- which +// would be permanent, and would name a claim that never existed. +func TestAnOperatorAssignedAddressGainsNoInventedReference(t *testing.T) { + existing := shard(testShardName) + existing.Spec.ShardAddressIPv6 = "2001:db8:100::dead" + cell := newShardCell(t, existing) + service := newFakeAddressIPAM(t) + + if _, err := reconcilerFor(cell, service).Reconcile(context.Background(), + requestFor(testShardName)); err != nil { + t.Fatalf("reconcile: %v", err) + } + + got := readShard(t, cell, testShardName) + if got.Spec.ShardAddressIPv6ClaimRef != nil { + t.Fatalf("a hand-assigned address gained the invented reference %+v", + got.Spec.ShardAddressIPv6ClaimRef) + } + if len(service.created) != 0 { + t.Errorf("a hand-assigned address caused %v to be claimed for the sake of a reference", service.created) + } +} + +// Nothing is written at all while the claim holds no address, so a shard never +// gains a reference whose address is still missing -- both fields are +// write-once and a half-written pair cannot be completed. +func TestAnUnboundClaimWritesNeitherAddressNorReference(t *testing.T) { + cell := newShardCell(t, shard(testShardName)) + service := newFakeAddressIPAM(t) + service.unbound = true + + if _, err := reconcilerFor(cell, service).Reconcile(context.Background(), + requestFor(testShardName)); err == nil { + t.Fatal("a claim holding no address reconciled successfully") + } + + got := readShard(t, cell, testShardName) + if got.Spec.ShardAddressIPv6 != "" { + t.Errorf("address = %q, want nothing written", got.Spec.ShardAddressIPv6) + } + if got.Spec.ShardAddressIPv6ClaimRef != nil { + t.Errorf("reference = %+v, want nothing written", got.Spec.ShardAddressIPv6ClaimRef) + } +} From c86e45711f7b04ebb7ca093e68332fc8ed99f1f5 Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Thu, 17 Sep 2026 21:41:01 -0500 Subject: [PATCH 5/6] feat: Grant and bound the claimer's addressing access The claimer mounted a credential that nothing had permitted to create claims. This adds the grant, gives the component an identity of its own, and states on the deployment what holding that credential reaches. The narrowest grant the platform can express is coarser than the one the component wants. Milo carries the project in the caller's user extras and RBAC does not read extras, so verbs are authorized on every project path at once; an iam.miloapis.com PolicyBinding naming the Project is the only way to narrow that, and both existing IPAM grants are wide for the same reason. Per-namespace scoping cannot be expressed at all: IPAM declares Project as the only parent of an IPClaim, so a namespaced Role would authorize that namespace name in every project and narrow nothing. The grant is not part of the cell overlay. It is applied to the Milo control plane, so carrying it here would invite applying it to a cell. Key changes: - Add config/egress-address/milo-grant.yaml: a ClusterRole granting get, create and delete on IPClaims and get on IPAllocations, bound to system:egress-address@cloud.datumapis.com. Deliberately excluded from the overlay's kustomization - Grant no classes and no pools under any verb, and no delete on IPAllocations. Claims carry reclaim policy Delete, so removing a claim frees its address without reaching past it; address space is operator inventory and nothing that allocates from it needs to reshape it - Specify the certificate and per-cell Secret that have to be created with it, including that the Secret must not be the cell controller's ipam-cluster-kubeconfig - Mount egress-address-ipam-kubeconfig rather than reusing that Secret name, which authenticates as system:nso-cell and additionally carries delete on IPAllocations. Sharing the name would have handed this component the broader identity silently - State the blast radius on the volume a reviewer reads to understand the credential: what a compromised holder could do, what it could not, and why the scope stops where it does Co-Authored-By: Claude Opus 5 (1M context) --- .../components/egress-address/deployment.yaml | 51 ++++++-- config/egress-address/kustomization.yaml | 6 + config/egress-address/milo-grant.yaml | 114 ++++++++++++++++++ 3 files changed, 159 insertions(+), 12 deletions(-) create mode 100644 config/egress-address/milo-grant.yaml diff --git a/config/components/egress-address/deployment.yaml b/config/components/egress-address/deployment.yaml index 6a85004..25405a9 100644 --- a/config/components/egress-address/deployment.yaml +++ b/config/components/egress-address/deployment.yaml @@ -43,7 +43,7 @@ spec: - --claim-namespace=$(CLAIM_NAMESPACE) - --location=$(LOCATION) - --platform-project=$(PLATFORM_PROJECT) - - --ipam-kubeconfig=/etc/ipam-cluster/kubeconfig + - --ipam-kubeconfig=/etc/egress-address-ipam/kubeconfig env: # The class that hands out shard addresses. Required; a deployment # naming no class refuses to start rather than draw from a default @@ -91,10 +91,44 @@ spec: limits: memory: 256Mi volumeMounts: - - name: ipam-cluster-kubeconfig - mountPath: /etc/ipam-cluster + - name: egress-address-ipam-kubeconfig + mountPath: /etc/egress-address-ipam readOnly: true volumes: + # BLAST RADIUS OF THIS CREDENTIAL. Read this before widening it. + # + # It is a client certificate authenticating to Milo as + # system:egress-address@cloud.datumapis.com, and it is what lets this + # component claim addresses in the platform's own project -- the tenancy + # holding every platform-owned allocation, including the fabric + # identities a network's forwarding state is keyed on. There is one of + # these per cell, and a cell is the least trusted cluster in the fleet. + # + # The narrowest scope the platform can express is (project) x (resource + # type) x (verb), and RBAC alone cannot even do the project: Milo + # carries the project in the caller's user extras, which RBAC does not + # read. Per-namespace scoping does not exist at all -- IPAM declares + # Project as the only parent of an IPClaim -- so "claims in one + # namespace" is not a grant that can be written. See + # ../../egress-address/milo-grant.yaml for the grant and the reasoning. + # + # What a compromised holder could therefore do: create, read and delete + # IPClaims, and read IPAllocations, in ANY project, under any class it + # can name. Deleting a claim of a class whose reclaim policy is Delete + # releases that address for reissue, so the reachable damage includes + # taking another shard's or another consumer's address out from under + # it. + # + # What it could not do: change a pool or a class, because no grant here + # includes them; take an address already written into a shard's spec, + # because those fields are write-once; or reach any consumer's workload + # or control plane, because nothing else is bound to this identity. + # + # It is deliberately NOT the cell controller's ipam-cluster-kubeconfig, + # which authenticates as system:nso-cell and additionally carries delete + # on IPAllocations. Sharing that Secret name would silently hand this + # component the broader identity. + # # Not optional. This component does one thing and cannot do it without # the address service. A pod waiting in ContainerCreating for a # credential that has not landed says so plainly; one started against an @@ -105,14 +139,7 @@ spec: # identity controller, everything this reads and writes in the cluster # is local: an EgressShard names a Node, so it never exists anywhere but # the cell holding that Node. - # - # This credential authenticates to the platform's own project, which is - # the tenancy holding every platform-owned allocation. It must be scoped - # to this component's job and no more -- "use" on the one address class - # and write access to IPClaims in the one namespace -- because a cell is - # the least trusted cluster in the fleet and there is one of these - # credentials per cell. - - name: ipam-cluster-kubeconfig + - name: egress-address-ipam-kubeconfig secret: - secretName: ipam-cluster-kubeconfig + secretName: egress-address-ipam-kubeconfig terminationGracePeriodSeconds: 10 diff --git a/config/egress-address/kustomization.yaml b/config/egress-address/kustomization.yaml index ebc11be..6383d89 100644 --- a/config/egress-address/kustomization.yaml +++ b/config/egress-address/kustomization.yaml @@ -16,6 +16,12 @@ # every marker under ./internal/..., so every role this image runs shares a role # and a ServiceAccount name. # +# The authorization this component needs is NOT here. It is applied to the Milo +# control plane rather than to a cell, so it would be wrong for this overlay to +# carry it: see milo-grant.yaml beside this file for the grant, the certificate +# and Secret that have to exist with it, and why the scope cannot be narrowed +# to one project by RBAC or to one namespace by anything. +# # The EgressShard CRD is not included here. It belongs to # go.datum.net/network and is installed by the data plane that owns it, not by # a consumer of it. diff --git a/config/egress-address/milo-grant.yaml b/config/egress-address/milo-grant.yaml new file mode 100644 index 0000000..f2686aa --- /dev/null +++ b/config/egress-address/milo-grant.yaml @@ -0,0 +1,114 @@ +# The authorization this component needs, and the ceiling on how narrow it can +# be made. +# +# APPLY DESTINATION: THE MILO CONTROL PLANE, NOT A CELL. This is deliberately +# absent from kustomization.yaml beside it, so applying the cell overlay cannot +# apply this by accident. Its home is the infrastructure repository, next to the +# two grants it mirrors: +# +# apps/vpc-system/control-plane/milo-ipam-rbac/rbac.yaml (central +# fabric +# identity) +# apps/network-services-operator/control-plane/milo-cell-rbac/ (the NSO +# rbac.yaml cell) +# +# It is authored here because the component that needs it is here. Moving it is +# an open item. +# +# --------------------------------------------------------------------------- +# HOW NARROW THIS CAN BE, WHICH IS LESS NARROW THAN IT LOOKS +# +# The grant is cluster-wide across every project, and that is not an oversight. +# Milo carries the project in the caller's user extras, and RBAC does not read +# extras, so these verbs are authorized on every project path at once. Both +# existing IPAM grants say the same thing and are written the same way. +# +# Scoping to the platform project alone requires an iam.miloapis.com +# PolicyBinding whose resourceSelector.resourceRef names that Project, not +# RBAC. That is available and is the tighter form; it is not used here only +# because it would be the first of its kind for an IPAM grant and the two +# existing grants would still be wide. Whether to convert all three together is +# a decision for whoever owns the identity configuration. +# +# Scoping to one NAMESPACE within a project CANNOT BE EXPRESSED AT ALL. IPAM +# declares Project as the only parentResource of an IPClaim +# (config/components/iam/protected-resources/ipclaim.yaml), so the model has no +# namespace dimension to bind against. A namespaced Role and RoleBinding would +# authorize that namespace NAME in every project, which narrows nothing and +# reads as though it does. The narrowest scope the platform can express is +# therefore (project) x (resource type) x (verb). +# +# --------------------------------------------------------------------------- +# THE IDENTITY, WHICH HAS TO BE CREATED TOO +# +# The subject below is the CN of a client certificate that does not exist yet. +# Three objects are needed, mirroring the NSO cell's own: +# +# 1. A Certificate on the control plane issuing CN +# system:egress-address@cloud.datumapis.com, pushed to GCP Secret Manager +# under the key egress-address-ipam-client-cert. Mirror +# apps/network-services-operator/control-plane/staging/ +# cell-ipam-client-certificate.yaml +# +# 2. An ExternalSecret in each cell syncing it into a Secret named +# egress-address-ipam-kubeconfig, with a templated kubeconfig whose server +# is Milo's TLS-passthrough base URL -- NOT a per-project endpoint, because +# the client appends the project path to every request. Mirror +# apps/network-services-operator/cell/ipam-client-cert.yaml, changing the +# Secret name and the user name. +# +# The Secret name must NOT be ipam-cluster-kubeconfig. That name is the NSO +# cell controller's own credential, which authenticates as system:nso-cell +# and additionally carries delete on IPAllocations; reusing it would hand +# this component a broader identity silently. +# +# 3. This grant. +# +# --------------------------------------------------------------------------- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: egress-address-ipam +rules: + # get, to find the address a shard already holds before drawing another -- + # the claim is the record, and reading it first is what makes the allocation + # idempotent. create, to draw one. delete, to give it back once the shard it + # was named for is gone. + - apiGroups: ["ipam.miloapis.com"] + resources: ["ipclaims"] + verbs: ["get", "create", "delete"] + # Read only, and only to recover the address an allocation retained by an + # earlier claim of the same name still holds. The service refuses the create + # and names that allocation, so without this the address is unreachable and + # the shard stays unaddressed behind a conflict that never clears. + # + # Deliberately no delete, unlike the NSO cell's grant. Claims here are written + # with reclaim policy Delete, so removing the claim frees the address; nothing + # this component does needs to reach past it to the allocation. + - apiGroups: ["ipam.miloapis.com"] + resources: ["ipallocations"] + verbs: ["get"] + # No ipclasses rule, unlike both existing grants. This component never reads + # a class: it names one on a claim and the service resolves it server-side. + # The "use" check on a class applies to creating a cross-project class + # reference, which nothing here does. + # + # No pools and no classes under any verb. Address space is operator + # inventory, and a component that allocates from it has no reason to be able + # to change its shape. +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: egress-address-ipam +subjects: + # CN of the certificate described above. Every cell shares this identity; + # nothing here distinguishes one cell from another, so the grant cannot be + # narrowed per location either. + - kind: User + apiGroup: rbac.authorization.k8s.io + name: system:egress-address@cloud.datumapis.com +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: egress-address-ipam From 285abd40ea626b920760230f6156d5a0c44038db Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Fri, 25 Sep 2026 09:03:13 -0500 Subject: [PATCH 6/6] chore: Pin the network API at its merged egress shard spec The address claimer was built against the shard spec while it was still on a branch. Pin the commit that merged it, matching the sibling egress branches so the three stack without a go.mod conflict. Co-Authored-By: Claude Fable 5.1 --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index eeb3770..6f7a160 100644 --- a/go.mod +++ b/go.mod @@ -79,6 +79,6 @@ require ( sigs.k8s.io/yaml v1.6.0 // indirect ) -replace go.datum.net/network => github.com/datum-cloud/network v0.1.1-0.20260918023140-affb112b4eea +replace go.datum.net/network => github.com/datum-cloud/network v0.1.1-0.20260923215140-1ed44c853eea replace go.datum.net/network-services-operator => github.com/datum-cloud/network-services-operator v0.27.2-0.20260917225730-eccf0e8922b2 diff --git a/go.sum b/go.sum index 83d1e4b..961026a 100644 --- a/go.sum +++ b/go.sum @@ -4,8 +4,8 @@ github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= -github.com/datum-cloud/network v0.1.1-0.20260918023140-affb112b4eea h1:9E4t6nhRgLwRSlprORVRMz9Hkjez8T93QnZesITnfNA= -github.com/datum-cloud/network v0.1.1-0.20260918023140-affb112b4eea/go.mod h1:dqzM8WZczbiZ9bCvsxjkoI10GJqQ24NVWnc9boXgOkE= +github.com/datum-cloud/network v0.1.1-0.20260923215140-1ed44c853eea h1:/a67sX+DD7+tRpaQSDAuoFuOfoMz9hC22wi4AGakiuk= +github.com/datum-cloud/network v0.1.1-0.20260923215140-1ed44c853eea/go.mod h1:dqzM8WZczbiZ9bCvsxjkoI10GJqQ24NVWnc9boXgOkE= github.com/datum-cloud/network-services-operator v0.27.2-0.20260917225730-eccf0e8922b2 h1:2yKJV4XRmoQMNM+VrOJdUgNkP1pM5z6l2P/2qF+K5yI= github.com/datum-cloud/network-services-operator v0.27.2-0.20260917225730-eccf0e8922b2/go.mod h1:9nuuBWdrkdnIBMaWJsWM3j4CcbKJIF1GDuCmAjdpIHo= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=