From b01f91eeee5edfbe5ceea5f78f5eb90fa22f322c Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Thu, 17 Sep 2026 17:08:52 -0500 Subject: [PATCH 1/3] feat: Bind internet egress into the conflist A network now reaches the internet from a cell when a consumer asked for it. The egress intent projected onto NetworkContext.spec selects the egress shards serving the class, and the shards' SRv6 SIDs are rendered straight into the CNI conflist the node reads. Nothing new lands on VPCAttachment: the node never reads it, and the conflist is written in the same reconcile pass from the same inputs. Intent is a function of (VPC, cell) alone. The kernel VRF is shared by every attachment of a VPC on a node and the datapath route key has no per-attachment component, so divergent intent between two attachments of one VPC is undefined. Every attachment resolves the same NetworkContext and computes the same ordered list, so the install is idempotent by construction. The egress block is absent whenever intent is Disabled, absent, or no shard is bound, and a conflist rendered without it is byte-identical to what the renderer produced before the field existed. Key changes: - Add EgressShardParameters, the cluster-scoped parameters type an InternetEgressClass names in its parametersRef for this controller to serve. It holds the shard namespace and a label selector over the network.datumapis.com/egress-* labels - Select shards in NetworkInterfaceReconciler, requiring the IPv6 family label, in name order, skipping shards that report no SID - Give galactic's BGPPlugin an omitempty egress block carrying an ordered shardSIDs candidate list the node keeps the first resolvable entry from - Watch NetworkContext and EgressShard so intent and a shard's reported SID reach the conflist without waiting out a poll interval - Pin go.datum.net/network and go.datum.net/network-services-operator to local checkouts, because both halves of the contract this reads are on unpushed branches. Lab only; it cannot merge in this state Co-Authored-By: Claude Opus 5 (1M context) --- api/v1alpha1/egressshardparameters_types.go | 106 ++++++++ api/v1alpha1/zz_generated.deepcopy.go | 74 +++++ ...d.datumapis.com_egressshardparameters.yaml | 143 ++++++++++ config/crd/kustomization.yaml | 3 + config/rbac/role.yaml | 9 + docs/api/vpc.md | 50 ++++ go.mod | 4 + go.sum | 4 - .../controller/networkinterface_controller.go | 235 +++++++++++++++- .../networkinterface_controller_test.go | 256 ++++++++++++++++++ internal/galactic/galactic.go | 40 ++- internal/galactic/galactic_test.go | 97 ++++++- 12 files changed, 1006 insertions(+), 15 deletions(-) create mode 100644 api/v1alpha1/egressshardparameters_types.go create mode 100644 config/crd/cloud.datumapis.com_egressshardparameters.yaml diff --git a/api/v1alpha1/egressshardparameters_types.go b/api/v1alpha1/egressshardparameters_types.go new file mode 100644 index 0000000..ee0708a --- /dev/null +++ b/api/v1alpha1/egressshardparameters_types.go @@ -0,0 +1,106 @@ +/* +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 v1alpha1 + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// KindEgressShardParameters is the kind an InternetEgressClass names in its +// parametersRef to be served by this controller. +// +// The reference is opaque to everything that carries it: the class states a +// group, a kind and a name, and no component between the class and this +// controller reads them. This controller answers only for its own group and +// this kind, and ignores a class whose parameters some other implementation +// owns, so two implementations can serve two classes in the same cell. +const KindEgressShardParameters = "EgressShardParameters" + +// EgressShardParametersSpec selects the egress shards serving a class. +type EgressShardParametersSpec struct { + // ShardNamespace is the namespace holding the EgressShard objects this + // selector may match. + // + // It is required and there is no cluster-wide search. A selector evaluated + // over every namespace would match an EgressShard a tenant created in a + // namespace they write to, which is a tenant naming the node their own + // traffic — and everyone else's on the same class — leaves the platform + // through. Naming the one namespace an operator owns keeps that + // unreachable. + // + // +kubebuilder:validation:Required + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=63 + ShardNamespace string `json:"shardNamespace"` + + // ShardSelector selects the EgressShards a network on this class egresses + // through, by the network.datumapis.com/egress-* labels an operator sets + // on them. + // + // An empty selector matches every shard in the namespace, which sends a + // consumer's traffic out of an arbitrary cell. Egress is realized per + // cell, so a selector is expected to pin a cell and a pool. + // + // The selector runs one way, as the only binding between a class and the + // shards serving it: a shard names nothing that selects it, which is what + // keeps the data-plane API group independent of the consumer-facing one. + // + // +kubebuilder:validation:Required + ShardSelector metav1.LabelSelector `json:"shardSelector"` +} + +// +kubebuilder:object:root=true +// +kubebuilder:resource:scope=Cluster +// +kubebuilder:printcolumn:name="Shard Namespace",type="string",JSONPath=".spec.shardNamespace" +// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp" + +// EgressShardParameters is the configuration this controller reads when an +// InternetEgressClass names it, and it holds which egress shards serve the +// networks that class places in this cell. +// +// It is cluster-scoped because the reference that reaches it carries no +// namespace: a class is cluster-scoped and its parametersRef states a group, a +// kind and a name only, so a namespaced parameters object would be +// unresolvable from the class that names it. The content is an operator's +// statement about the cell's own data plane rather than anything belonging to +// one tenant, and every tenant namespace resolves the same answer from it. +// +// This object is written by an operator. No consumer reads or writes one, and +// a consumer names a class, never these parameters. +type EgressShardParameters struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + // Spec is the whole of this object. There is no status: nothing reconciles + // these parameters, and the result of applying them is reported on the + // network context whose egress they served. + Spec EgressShardParametersSpec `json:"spec,omitempty"` +} + +// +kubebuilder:object:root=true + +// EgressShardParametersList contains a list of EgressShardParameters. +type EgressShardParametersList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []EgressShardParameters `json:"items"` +} + +func init() { + SchemeBuilder.Register(&EgressShardParameters{}, &EgressShardParametersList{}) +} diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index b31bf5c..1a1d538 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -26,6 +26,80 @@ import ( runtime "k8s.io/apimachinery/pkg/runtime" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *EgressShardParameters) DeepCopyInto(out *EgressShardParameters) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + in.Spec.DeepCopyInto(&out.Spec) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new EgressShardParameters. +func (in *EgressShardParameters) DeepCopy() *EgressShardParameters { + if in == nil { + return nil + } + out := new(EgressShardParameters) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *EgressShardParameters) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *EgressShardParametersList) DeepCopyInto(out *EgressShardParametersList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]EgressShardParameters, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new EgressShardParametersList. +func (in *EgressShardParametersList) DeepCopy() *EgressShardParametersList { + if in == nil { + return nil + } + out := new(EgressShardParametersList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *EgressShardParametersList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *EgressShardParametersSpec) DeepCopyInto(out *EgressShardParametersSpec) { + *out = *in + in.ShardSelector.DeepCopyInto(&out.ShardSelector) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new EgressShardParametersSpec. +func (in *EgressShardParametersSpec) DeepCopy() *EgressShardParametersSpec { + if in == nil { + return nil + } + out := new(EgressShardParametersSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NetworkFabricIdentity) DeepCopyInto(out *NetworkFabricIdentity) { *out = *in diff --git a/config/crd/cloud.datumapis.com_egressshardparameters.yaml b/config/crd/cloud.datumapis.com_egressshardparameters.yaml new file mode 100644 index 0000000..b486db6 --- /dev/null +++ b/config/crd/cloud.datumapis.com_egressshardparameters.yaml @@ -0,0 +1,143 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.18.0 + name: egressshardparameters.cloud.datumapis.com +spec: + group: cloud.datumapis.com + names: + kind: EgressShardParameters + listKind: EgressShardParametersList + plural: egressshardparameters + singular: egressshardparameters + scope: Cluster + versions: + - additionalPrinterColumns: + - jsonPath: .spec.shardNamespace + name: Shard Namespace + type: string + - jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1alpha1 + schema: + openAPIV3Schema: + description: |- + EgressShardParameters is the configuration this controller reads when an + InternetEgressClass names it, and it holds which egress shards serve the + networks that class places in this cell. + + It is cluster-scoped because the reference that reaches it carries no + namespace: a class is cluster-scoped and its parametersRef states a group, a + kind and a name only, so a namespaced parameters object would be + unresolvable from the class that names it. The content is an operator's + statement about the cell's own data plane rather than anything belonging to + one tenant, and every tenant namespace resolves the same answer from it. + + This object is written by an operator. No consumer reads or writes one, and + a consumer names a class, never these parameters. + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: |- + Spec is the whole of this object. There is no status: nothing reconciles + these parameters, and the result of applying them is reported on the + network context whose egress they served. + properties: + shardNamespace: + description: |- + ShardNamespace is the namespace holding the EgressShard objects this + selector may match. + + It is required and there is no cluster-wide search. A selector evaluated + over every namespace would match an EgressShard a tenant created in a + namespace they write to, which is a tenant naming the node their own + traffic — and everyone else's on the same class — leaves the platform + through. Naming the one namespace an operator owns keeps that + unreachable. + maxLength: 63 + minLength: 1 + type: string + shardSelector: + description: |- + ShardSelector selects the EgressShards a network on this class egresses + through, by the network.datumapis.com/egress-* labels an operator sets + on them. + + An empty selector matches every shard in the namespace, which sends a + consumer's traffic out of an arbitrary cell. Egress is realized per + cell, so a selector is expected to pin a cell and a pool. + + The selector runs one way, as the only binding between a class and the + shards serving it: a shard names nothing that selects it, which is what + keeps the data-plane API group independent of the consumer-facing one. + properties: + matchExpressions: + description: matchExpressions is a list of label selector requirements. + The requirements are ANDed. + items: + description: |- + A label selector requirement is a selector that contains values, a key, and an operator that + relates the key and values. + properties: + key: + description: key is the label key that the selector applies + to. + type: string + operator: + description: |- + operator represents a key's relationship to a set of values. + Valid operators are In, NotIn, Exists and DoesNotExist. + type: string + values: + description: |- + values is an array of string values. If the operator is In or NotIn, + the values array must be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. This array is replaced during a strategic + merge patch. + items: + type: string + type: array + x-kubernetes-list-type: atomic + required: + - key + - operator + type: object + type: array + x-kubernetes-list-type: atomic + matchLabels: + additionalProperties: + type: string + description: |- + matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels + map is equivalent to an element of matchExpressions, whose key field is "key", the + operator is "In", and the values array contains only "value". The requirements are ANDed. + type: object + type: object + x-kubernetes-map-type: atomic + required: + - shardNamespace + - shardSelector + type: object + type: object + served: true + storage: true + subresources: {} diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index bbfc2ec..390e02d 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -6,3 +6,6 @@ resources: # Written centrally, federated to the cells that need it, so it is installed # both places. - cloud.datumapis.com_networkfabricidentities.yaml + # Cell-local: an operator writes one per InternetEgressClass this cell serves, + # and only the controller in the cell reads it. + - cloud.datumapis.com_egressshardparameters.yaml diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 8600ec8..23aaa1c 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -11,6 +11,14 @@ rules: verbs: - create - patch +- apiGroups: + - cloud.datumapis.com + resources: + - egressshardparameters + verbs: + - get + - list + - watch - apiGroups: - cloud.datumapis.com resources: @@ -71,6 +79,7 @@ rules: resources: - bgpadvertisements - bgprouters + - egressshards verbs: - get - list diff --git a/docs/api/vpc.md b/docs/api/vpc.md index 537f5ee..42939f8 100644 --- a/docs/api/vpc.md +++ b/docs/api/vpc.md @@ -9,12 +9,62 @@ Package v1alpha1 contains API Schema definitions for the cloud.datumapis.com/v1alpha1 API group. ### Resource Types +- [EgressShardParameters](#egressshardparameters) - [NetworkFabricIdentity](#networkfabricidentity) - [VPC](#vpc) - [VPCAttachment](#vpcattachment) +#### EgressShardParameters + + + +EgressShardParameters is the configuration this controller reads when an +InternetEgressClass names it, and it holds which egress shards serve the +networks that class places in this cell. + +It is cluster-scoped because the reference that reaches it carries no +namespace: a class is cluster-scoped and its parametersRef states a group, a +kind and a name only, so a namespaced parameters object would be +unresolvable from the class that names it. The content is an operator's +statement about the cell's own data plane rather than anything belonging to +one tenant, and every tenant namespace resolves the same answer from it. + +This object is written by an operator. No consumer reads or writes one, and +a consumer names a class, never these parameters. + + + + + +| Field | Description | Default | Validation | +| --- | --- | --- | --- | +| `apiVersion` _string_ | `cloud.datumapis.com/v1alpha1` | | | +| `kind` _string_ | `EgressShardParameters` | | | +| `kind` _string_ | Kind is a string value representing the REST resource this object represents.
Servers may infer this from the endpoint the client submits requests to.
Cannot be updated.
In CamelCase.
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds | | | +| `apiVersion` _string_ | APIVersion defines the versioned schema of this representation of an object.
Servers should convert recognized schemas to the latest internal value, and
may reject unrecognized values.
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources | | | +| `metadata` _[ObjectMeta](https://kubernetes.io/docs/reference/generated/kubernetes-api/v/#objectmeta-v1-meta)_ | Refer to Kubernetes API documentation for fields of `metadata`. | | | +| `spec` _[EgressShardParametersSpec](#egressshardparametersspec)_ | Spec is the whole of this object. There is no status: nothing reconciles
these parameters, and the result of applying them is reported on the
network context whose egress they served. | | | + + +#### EgressShardParametersSpec + + + +EgressShardParametersSpec selects the egress shards serving a class. + + + +_Appears in:_ +- [EgressShardParameters](#egressshardparameters) + +| Field | Description | Default | Validation | +| --- | --- | --- | --- | +| `shardNamespace` _string_ | ShardNamespace is the namespace holding the EgressShard objects this
selector may match.
It is required and there is no cluster-wide search. A selector evaluated
over every namespace would match an EgressShard a tenant created in a
namespace they write to, which is a tenant naming the node their own
traffic — and everyone else's on the same class — leaves the platform
through. Naming the one namespace an operator owns keeps that
unreachable. | | MaxLength: 63
MinLength: 1
Required: \{\}
| +| `shardSelector` _[LabelSelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v/#labelselector-v1-meta)_ | ShardSelector selects the EgressShards a network on this class egresses
through, by the network.datumapis.com/egress-* labels an operator sets
on them.
An empty selector matches every shard in the namespace, which sends a
consumer's traffic out of an arbitrary cell. Egress is realized per
cell, so a selector is expected to pin a cell and a pool.
The selector runs one way, as the only binding between a class and the
shards serving it: a shard names nothing that selects it, which is what
keeps the data-plane API group independent of the consumer-facing one. | | Required: \{\}
| + + #### IPAddress _Underlying type:_ _string_ diff --git a/go.mod b/go.mod index 0431086..9c4ad41 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 => ../network-egress-spec + +replace go.datum.net/network-services-operator => ../nso-internet-egress diff --git a/go.sum b/go.sum index 6c1b0e7..896395c 100644 --- a/go.sum +++ b/go.sum @@ -121,10 +121,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= diff --git a/internal/controller/networkinterface_controller.go b/internal/controller/networkinterface_controller.go index 5b0a370..79e5d15 100644 --- a/internal/controller/networkinterface_controller.go +++ b/internal/controller/networkinterface_controller.go @@ -18,25 +18,32 @@ along with this program. If not, see . package controller import ( + "cmp" "context" "fmt" + "slices" "time" nadv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/selection" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/handler" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/reconcile" cloudv1alpha1 "go.datum.net/cloud/api/v1alpha1" "go.datum.net/cloud/internal/galactic" "go.datum.net/cloud/internal/identifier" networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" + bgpv1alpha1 "go.datum.net/network/api/v1alpha1" ) const ( @@ -93,7 +100,10 @@ type NetworkInterfaceReconciler struct { // +kubebuilder:rbac:groups=networking.datumapis.com,resources=networkinterfaces/status,verbs=get;update;patch // +kubebuilder:rbac:groups=networking.datumapis.com,resources=networkinterfaceclaims,verbs=get;list;watch // +kubebuilder:rbac:groups=networking.datumapis.com,resources=networkinterfaceclaims/status,verbs=get;update;patch +// +kubebuilder:rbac:groups=networking.datumapis.com,resources=networkcontexts,verbs=get;list;watch // +kubebuilder:rbac:groups=cloud.datumapis.com,resources=vpcs,verbs=get;list;watch +// +kubebuilder:rbac:groups=cloud.datumapis.com,resources=egressshardparameters,verbs=get;list;watch +// +kubebuilder:rbac:groups=network.datumapis.com,resources=egressshards,verbs=get;list;watch // +kubebuilder:rbac:groups=cloud.datumapis.com,resources=vpcattachments,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=cloud.datumapis.com,resources=vpcattachments/status,verbs=get;update;patch // +kubebuilder:rbac:groups=k8s.cni.cncf.io,resources=network-attachment-definitions,verbs=get;list;watch;create;update;patch;delete @@ -110,6 +120,9 @@ func (r *NetworkInterfaceReconciler) Reconcile(ctx context.Context, req ctrl.Req return ctrl.Result{}, nil } + // The VPC is named after the NetworkContext it realizes, so one key reads + // both: the identity the fabric keys on, and the egress intent projected + // onto this location. var vpc cloudv1alpha1.VPC vpcKey := types.NamespacedName{ Namespace: networkInterface.Namespace, @@ -128,11 +141,24 @@ func (r *NetworkInterfaceReconciler) Reconcile(ctx context.Context, req ctrl.Req fmt.Sprintf("VPC %s has no identifier yet", vpc.Name)) } + var networkContext networkingv1alpha.NetworkContext + if err := r.Get(ctx, vpcKey, &networkContext); err != nil { + if apierrors.IsNotFound(err) { + // The VPC exists only because this context did, so a context that + // is gone is a network being withdrawn from the cell rather than a + // race worth rendering through. + return ctrl.Result{RequeueAfter: 10 * time.Second}, r.markPrepared(ctx, &networkInterface, + metav1.ConditionFalse, "AwaitingNetworkContext", + fmt.Sprintf("NetworkContext %s does not exist yet", vpcKey.Name)) + } + return ctrl.Result{}, fmt.Errorf("get NetworkContext %s: %w", vpcKey, err) + } + attachment, err := r.reconcileAttachment(ctx, &networkInterface, &vpc) if err != nil { return ctrl.Result{}, err } - nad, err := r.reconcileNAD(ctx, attachment, &vpc, &networkInterface) + nad, err := r.reconcileNAD(ctx, attachment, &vpc, &networkInterface, &networkContext) if err != nil { return ctrl.Result{}, err } @@ -183,6 +209,7 @@ func (r *NetworkInterfaceReconciler) reconcileNAD( attachment *cloudv1alpha1.VPCAttachment, vpc *cloudv1alpha1.VPC, networkInterface *networkingv1alpha.NetworkInterface, + networkContext *networkingv1alpha.NetworkContext, ) (*nadv1.NetworkAttachmentDefinition, error) { nad := &nadv1.NetworkAttachmentDefinition{ ObjectMeta: metav1.ObjectMeta{Name: attachment.Name, Namespace: attachment.Namespace}, @@ -209,9 +236,13 @@ func (r *NetworkInterfaceReconciler) reconcileNAD( Gateway: address.Gateway, }) } + egress, err := r.resolveInternetEgress(ctx, networkContext) + if err != nil { + return nil, err + } config, err := galactic.ConflistJSON(attachment.Name, masterPlugin(attachment.Spec.Interface.Mode), vpc.Status.VPC, attachmentID, networkInterface.Spec.MTU, addresses, - declaresDevice(attachment.Spec.Interface.Mode)) + declaresDevice(attachment.Spec.Interface.Mode), egress) if err != nil { return nil, err } @@ -281,6 +312,153 @@ func interfaceAddresses(networkInterface *networkingv1alpha.NetworkInterface) [] return addresses } +// resolveInternetEgress turns the egress intent projected onto a NetworkContext +// into the ordered shard candidates a node routes this VPC's VRF toward. A nil +// result renders no egress block, so the VPC reaches nothing outside the +// platform. +// +// Egress intent is a function of (VPC, cell) and of nothing else — not of the +// attachment, the interface, or the claim. The kernel VRF is shared by every +// attachment of a VPC on a node and the datapath's route key has no +// per-attachment component, so two attachments of one VPC asking for different +// egress is undefined: the last ADD wins and silently redirects the traffic of +// every attachment already up. Nothing here can express that divergence, +// because the only input is the NetworkContext, the VPC is named after it, and +// every attachment of a VPC therefore resolves the same context and computes +// the same list. The install is idempotent by construction rather than by a +// check. Keep it that way: an input read off the interface, the claim or the +// attachment, or a selection that is not deterministic over the shards it +// matched, breaks the invariant without breaking a test. +func (r *NetworkInterfaceReconciler) resolveInternetEgress( + ctx context.Context, networkContext *networkingv1alpha.NetworkContext, +) (*galactic.Egress, error) { + log := logf.FromContext(ctx) + + intent := internetEgressIntent(networkContext) + if intent == nil { + // A context written before this field existed carries no intent, which + // is not the same as a network that reaches nothing. Both render no + // egress; only this one is worth saying out loud. + log.V(1).Info("network context carries no projected egress intent", + "networkContext", networkContext.Name) + return nil, nil + } + if intent.Mode != networkingv1alpha.NetworkInternetEgressEnabled { + return nil, nil + } + + ref := intent.ParametersRef + if ref == nil { + log.Info("internet egress is enabled but the serving class names no parameters", + "networkContext", networkContext.Name, "class", intent.ClassName) + return nil, nil + } + // The reference is opaque, so this controller recognizes only its own + // parameters and leaves another implementation's class alone rather than + // guessing at a type it does not own. + if ref.Group != cloudv1alpha1.GroupVersion.Group || ref.Kind != cloudv1alpha1.KindEgressShardParameters { + log.V(1).Info("internet egress class is served by another implementation", + "networkContext", networkContext.Name, "class", intent.ClassName, + "group", ref.Group, "kind", ref.Kind) + return nil, nil + } + + var parameters cloudv1alpha1.EgressShardParameters + if err := r.Get(ctx, client.ObjectKey{Name: ref.Name}, ¶meters); err != nil { + if apierrors.IsNotFound(err) { + log.Info("internet egress parameters do not exist in this cell", + "networkContext", networkContext.Name, "class", intent.ClassName, + "parameters", ref.Name) + return nil, nil + } + return nil, fmt.Errorf("get EgressShardParameters %s: %w", ref.Name, err) + } + + shards, err := r.egressShards(ctx, ¶meters) + if err != nil { + return nil, err + } + + sids := make([]string, 0, len(shards)) + sources := make([]string, 0, len(shards)) + for i := range shards { + // A shard whose SID is unreported has nothing a node can route toward. + // The SID stays in status because nothing allocates one yet. + if shards[i].Status.ShardSID == "" { + continue + } + if slices.Contains(sids, shards[i].Status.ShardSID) { + continue + } + sids = append(sids, shards[i].Status.ShardSID) + if address := shards[i].Status.ShardAddressIPv6; address != "" { + sources = append(sources, address) + } + } + if len(sids) == 0 { + log.Info("internet egress is enabled but no shard serves this network", + "networkContext", networkContext.Name, "class", intent.ClassName, + "parameters", parameters.Name) + return nil, nil + } + + // The source addresses are the answer a consumer reads back. Nothing in + // this cell writes them onto the network context yet, so they are reported + // here and nowhere else. + log.Info("internet egress bound", "networkContext", networkContext.Name, + "class", intent.ClassName, "shardSIDs", sids, "sourceAddressesIPv6", sources) + return &galactic.Egress{ShardSIDs: sids}, nil +} + +// internetEgressIntent reads the internet egress a location was instructed to +// provide. Every field on it is already resolved, so nothing here selects a +// class, picks a default, or interprets the class's parameters reference. +func internetEgressIntent( + networkContext *networkingv1alpha.NetworkContext, +) *networkingv1alpha.NetworkContextInternetEgress { + if networkContext.Spec.Egress == nil { + return nil + } + return networkContext.Spec.Egress.Internet +} + +// egressShards lists the shards the parameters select, in name order. +// +// The order is what makes the candidate list a function of the matched set +// alone: an unordered list would differ between two attachments of one VPC +// reconciled moments apart, which is exactly the divergence the VRF cannot +// represent. +func (r *NetworkInterfaceReconciler) egressShards( + ctx context.Context, parameters *cloudv1alpha1.EgressShardParameters, +) ([]bgpv1alpha1.EgressShard, error) { + selector, err := metav1.LabelSelectorAsSelector(¶meters.Spec.ShardSelector) + if err != nil { + return nil, fmt.Errorf("parse the shard selector on EgressShardParameters %s: %w", + parameters.Name, err) + } + // Only IPv6 is reached, so a shard that translates no IPv6 flow is no + // candidate however an operator wrote the selector. The family label is + // matched on presence: absence, not a false value, means the family is + // unserved, so a shard predating the label never reads as serving one. + servesIPv6, err := labels.NewRequirement(bgpv1alpha1.LabelEgressShardIPv6, selection.Exists, nil) + if err != nil { + return nil, fmt.Errorf("build the IPv6 shard requirement: %w", err) + } + + var shards bgpv1alpha1.EgressShardList + if err := r.List(ctx, &shards, + client.InNamespace(parameters.Spec.ShardNamespace), + client.MatchingLabelsSelector{Selector: selector.Add(*servesIPv6)}, + ); err != nil { + return nil, fmt.Errorf("list egress shards for EgressShardParameters %s: %w", + parameters.Name, err) + } + slices.SortFunc(shards.Items, func(a, b bgpv1alpha1.EgressShard) int { + return cmp.Compare(a.Name, b.Name) + }) + return shards.Items, nil +} + // allocateAttachmentIdentifier draws a random identifier unused within the VPC. // Random rather than lowest-free, so a freed identifier is not immediately // reissued while its BGPAdvertisement is still being garbage collected. @@ -402,6 +580,59 @@ func (r *NetworkInterfaceReconciler) SetupWithManager(mgr ctrl.Manager) error { For(&networkingv1alpha.NetworkInterface{}). Owns(&cloudv1alpha1.VPCAttachment{}). Watches(&nadv1.NetworkAttachmentDefinition{}, handler.EnqueueRequestsFromMapFunc(nadToInterface)). + Watches(&networkingv1alpha.NetworkContext{}, + handler.EnqueueRequestsFromMapFunc(r.interfacesForNetworkContext)). + Watches(&bgpv1alpha1.EgressShard{}, + handler.EnqueueRequestsFromMapFunc(r.interfacesForEgressShard)). Named("networkinterface"). Complete(r) } + +// interfacesForNetworkContext re-renders a location's attachments when the +// egress a consumer declared reaches it, so a network that was enabled does not +// wait out a poll interval it does not have. +func (r *NetworkInterfaceReconciler) interfacesForNetworkContext( + ctx context.Context, object client.Object, +) []reconcile.Request { + var interfaces networkingv1alpha.NetworkInterfaceList + if err := r.List(ctx, &interfaces, client.InNamespace(object.GetNamespace())); err != nil { + return nil + } + + requests := make([]reconcile.Request, 0, len(interfaces.Items)) + for i := range interfaces.Items { + reference := interfaces.Items[i].Status.NetworkContextRef + if reference == nil || reference.Name != object.GetName() { + continue + } + requests = append(requests, reconcile.Request{ + NamespacedName: client.ObjectKeyFromObject(&interfaces.Items[i]), + }) + } + return requests +} + +// interfacesForEgressShard re-renders every attachment in the cell when a shard +// arrives, reports its SID, or leaves. +// +// It enqueues everything rather than working out which networks a shard serves: +// the binding runs the other way, from a class's selector to the shards, so a +// shard cannot name the networks on it. The sweep is affordable because a shard +// is an operator-written object in one namespace and there are a handful of +// them per cell, and re-rendering an unaffected attachment writes nothing. +func (r *NetworkInterfaceReconciler) interfacesForEgressShard( + ctx context.Context, _ client.Object, +) []reconcile.Request { + var interfaces networkingv1alpha.NetworkInterfaceList + if err := r.List(ctx, &interfaces); err != nil { + return nil + } + + requests := make([]reconcile.Request, 0, len(interfaces.Items)) + for i := range interfaces.Items { + requests = append(requests, reconcile.Request{ + NamespacedName: client.ObjectKeyFromObject(&interfaces.Items[i]), + }) + } + return requests +} diff --git a/internal/controller/networkinterface_controller_test.go b/internal/controller/networkinterface_controller_test.go index 6ce4090..f7458fb 100644 --- a/internal/controller/networkinterface_controller_test.go +++ b/internal/controller/networkinterface_controller_test.go @@ -18,13 +18,18 @@ along with this program. If not, see . package controller import ( + "slices" "testing" 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" cloudv1alpha1 "go.datum.net/cloud/api/v1alpha1" "go.datum.net/cloud/internal/galactic" networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" + bgpv1alpha1 "go.datum.net/network/api/v1alpha1" ) func TestMasterPlugin(t *testing.T) { @@ -139,3 +144,254 @@ func TestDeclaresDevice(t *testing.T) { t.Error("a declared hypervisor attachment must ask for a description") } } + +const ( + egressTestNamespace = "project-egress" + egressShardNamespace = "galactic-system" + egressTestParameters = "shared-ipv6" +) + +// newEgressReconciler builds a reconciler over a cell holding the shards and +// the parameters given, so a test states only what it is about. +func newEgressReconciler(t *testing.T, objects ...client.Object) *NetworkInterfaceReconciler { + t.Helper() + + scheme := runtime.NewScheme() + if err := networkingv1alpha.AddToScheme(scheme); err != nil { + t.Fatalf("build the networking scheme: %v", err) + } + if err := cloudv1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("build the cloud scheme: %v", err) + } + if err := bgpv1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("build the fabric scheme: %v", err) + } + + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objects...).Build() + return &NetworkInterfaceReconciler{Client: fakeClient, Scheme: scheme, APIReader: fakeClient} +} + +// newEgressShard is a shard an operator labelled and whose node reported a SID. +func newEgressShard(name, sid, address string, shardLabels map[string]string) *bgpv1alpha1.EgressShard { + shard := &bgpv1alpha1.EgressShard{} + shard.Namespace = egressShardNamespace + shard.Name = name + shard.Labels = shardLabels + shard.Spec.ShardAddressIPv6 = address + shard.Status.ShardSID = sid + shard.Status.ShardAddressIPv6 = address + return shard +} + +// newEgressParameters selects a pool in a cell, which is what an operator +// writes for a class this controller serves. +func newEgressParameters() *cloudv1alpha1.EgressShardParameters { + parameters := &cloudv1alpha1.EgressShardParameters{} + parameters.Name = egressTestParameters + parameters.Spec.ShardNamespace = egressShardNamespace + parameters.Spec.ShardSelector = metav1.LabelSelector{MatchLabels: map[string]string{ + bgpv1alpha1.LabelEgressShardPool: "shared", + bgpv1alpha1.LabelEgressShardCell: "us-central-1", + }} + return parameters +} + +// poolLabels are what an operator sets on a shard serving the selected pool. +func poolLabels() map[string]string { + return map[string]string{ + bgpv1alpha1.LabelEgressShardPool: "shared", + bgpv1alpha1.LabelEgressShardCell: "us-central-1", + bgpv1alpha1.LabelEgressShardIPv6: bgpv1alpha1.LabelValueEgressFamilyServed, + } +} + +// newEgressContext is a location carrying resolved egress intent. Every field +// is written resolved upstream, so nothing under test selects a class or reads +// a default. +func newEgressContext(mode networkingv1alpha.NetworkInternetEgressMode) *networkingv1alpha.NetworkContext { + networkContext := &networkingv1alpha.NetworkContext{} + networkContext.Namespace = egressTestNamespace + networkContext.Name = "default-us-central-1" + networkContext.Spec.Egress = &networkingv1alpha.NetworkContextEgress{ + Internet: &networkingv1alpha.NetworkContextInternetEgress{ + Mode: mode, + Reach: []networkingv1alpha.IPFamily{networkingv1alpha.IPv6Protocol}, + ClassName: "shared", + Sharing: networkingv1alpha.InternetEgressSharingShared, + ParametersRef: &networkingv1alpha.InternetEgressClassParametersRef{ + Group: cloudv1alpha1.GroupVersion.Group, + Kind: cloudv1alpha1.KindEgressShardParameters, + Name: egressTestParameters, + }, + }, + } + return networkContext +} + +func TestResolveInternetEgressSelectsMatchingShards(t *testing.T) { + r := newEgressReconciler(t, + newEgressParameters(), + newEgressShard("shard-b", "2001:db8:ff02::", "2001:db8:1::2", poolLabels()), + newEgressShard("shard-a", "2001:db8:ff01::", "2001:db8:1::1", poolLabels()), + ) + + egress, err := r.resolveInternetEgress(t.Context(), + newEgressContext(networkingv1alpha.NetworkInternetEgressEnabled)) + if err != nil { + t.Fatalf("resolveInternetEgress: %v", err) + } + if egress == nil { + t.Fatal("two matching shards resolved no egress") + } + // Name order, not list order: two attachments of one VPC reconciled moments + // apart have to compute the same list, because the VRF they share cannot + // hold two answers. + want := []string{"2001:db8:ff01::", "2001:db8:ff02::"} + if !slices.Equal(egress.ShardSIDs, want) { + t.Errorf("shard SIDs: got %v, want %v", egress.ShardSIDs, want) + } +} + +// Every reason a network reaches nothing renders the same absent block. A node +// that receives no block installs no route. +func TestResolveInternetEgressYieldsNothingWhenUnbound(t *testing.T) { + otherImplementation := newEgressContext(networkingv1alpha.NetworkInternetEgressEnabled) + otherImplementation.Spec.Egress.Internet.ParametersRef.Kind = "SomeOtherParameters" + + noParameters := newEgressContext(networkingv1alpha.NetworkInternetEgressEnabled) + noParameters.Spec.Egress.Internet.ParametersRef = nil + + missingParameters := newEgressContext(networkingv1alpha.NetworkInternetEgressEnabled) + missingParameters.Spec.Egress.Internet.ParametersRef.Name = "not-in-this-cell" + + unprojected := newEgressContext(networkingv1alpha.NetworkInternetEgressEnabled) + unprojected.Spec.Egress = nil + + noInternet := newEgressContext(networkingv1alpha.NetworkInternetEgressEnabled) + noInternet.Spec.Egress.Internet = nil + + tests := []struct { + name string + networkContext *networkingv1alpha.NetworkContext + objects []client.Object + }{ + { + name: "disabled", + networkContext: newEgressContext(networkingv1alpha.NetworkInternetEgressDisabled), + objects: []client.Object{newEgressParameters(), + newEgressShard("shard-a", "2001:db8:ff01::", "2001:db8:1::1", poolLabels())}, + }, + { + name: "mode never projected", + networkContext: newEgressContext(""), + objects: []client.Object{newEgressParameters(), + newEgressShard("shard-a", "2001:db8:ff01::", "2001:db8:1::1", poolLabels())}, + }, + { + name: "egress never projected", + networkContext: unprojected, + objects: []client.Object{newEgressParameters()}, + }, + { + name: "no internet egress projected", + networkContext: noInternet, + objects: []client.Object{newEgressParameters()}, + }, + { + name: "class names no parameters", + networkContext: noParameters, + objects: []client.Object{newEgressParameters()}, + }, + { + name: "parameters owned by another implementation", + networkContext: otherImplementation, + objects: []client.Object{newEgressParameters()}, + }, + { + name: "parameters absent from this cell", + networkContext: missingParameters, + objects: []client.Object{newEgressParameters()}, + }, + { + name: "no shard matches the selector", + networkContext: newEgressContext(networkingv1alpha.NetworkInternetEgressEnabled), + objects: []client.Object{newEgressParameters(), + newEgressShard("elsewhere", "2001:db8:ff01::", "2001:db8:1::1", map[string]string{ + bgpv1alpha1.LabelEgressShardPool: "shared", + bgpv1alpha1.LabelEgressShardCell: "us-east-1", + bgpv1alpha1.LabelEgressShardIPv6: bgpv1alpha1.LabelValueEgressFamilyServed, + })}, + }, + { + name: "matching shard translates no IPv6", + networkContext: newEgressContext(networkingv1alpha.NetworkInternetEgressEnabled), + objects: []client.Object{newEgressParameters(), + newEgressShard("ipv4-only", "2001:db8:ff01::", "", map[string]string{ + bgpv1alpha1.LabelEgressShardPool: "shared", + bgpv1alpha1.LabelEgressShardCell: "us-central-1", + bgpv1alpha1.LabelEgressShardIPv4: bgpv1alpha1.LabelValueEgressFamilyServed, + })}, + }, + { + name: "matching shard reports no SID", + networkContext: newEgressContext(networkingv1alpha.NetworkInternetEgressEnabled), + objects: []client.Object{newEgressParameters(), + newEgressShard("unprogrammed", "", "2001:db8:1::1", poolLabels())}, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + r := newEgressReconciler(t, test.objects...) + egress, err := r.resolveInternetEgress(t.Context(), test.networkContext) + if err != nil { + t.Fatalf("resolveInternetEgress: %v", err) + } + if egress != nil { + t.Errorf("got %v, want no egress", egress.ShardSIDs) + } + }) + } +} + +// A SID an operator typed onto two shards is one candidate, not two. +func TestResolveInternetEgressDeduplicatesShardSIDs(t *testing.T) { + r := newEgressReconciler(t, + newEgressParameters(), + newEgressShard("shard-a", "2001:db8:ff01::", "2001:db8:1::1", poolLabels()), + newEgressShard("shard-b", "2001:db8:ff01::", "2001:db8:1::2", poolLabels()), + ) + + egress, err := r.resolveInternetEgress(t.Context(), + newEgressContext(networkingv1alpha.NetworkInternetEgressEnabled)) + if err != nil { + t.Fatalf("resolveInternetEgress: %v", err) + } + if want := []string{"2001:db8:ff01::"}; egress == nil || !slices.Equal(egress.ShardSIDs, want) { + t.Errorf("shard SIDs: got %v, want %v", egress, want) + } +} + +// The invariant the datapath depends on: intent is a function of (VPC, cell) +// and nothing else, so every attachment of a VPC computes the same value and +// the install is idempotent. This asserts the property at the only seam where +// it could be broken — the resolver takes the context and nothing else. +func TestResolveInternetEgressIsAFunctionOfTheNetworkContextAlone(t *testing.T) { + r := newEgressReconciler(t, + newEgressParameters(), + newEgressShard("shard-a", "2001:db8:ff01::", "2001:db8:1::1", poolLabels()), + newEgressShard("shard-b", "2001:db8:ff02::", "2001:db8:1::2", poolLabels()), + ) + networkContext := newEgressContext(networkingv1alpha.NetworkInternetEgressEnabled) + + first, err := r.resolveInternetEgress(t.Context(), networkContext) + if err != nil { + t.Fatalf("resolveInternetEgress: %v", err) + } + second, err := r.resolveInternetEgress(t.Context(), networkContext) + if err != nil { + t.Fatalf("resolveInternetEgress: %v", err) + } + if first == nil || second == nil || !slices.Equal(first.ShardSIDs, second.ShardSIDs) { + t.Errorf("two attachments of one VPC resolved %v and %v", first, second) + } +} diff --git a/internal/galactic/galactic.go b/internal/galactic/galactic.go index 4e82cf9..f161767 100644 --- a/internal/galactic/galactic.go +++ b/internal/galactic/galactic.go @@ -98,6 +98,21 @@ type BGPPlugin struct { VPC string `json:"vpc"` VPCAttachment string `json:"vpcattachment"` Namespace string `json:"namespace"` + // Egress asks for an egress route out of this VPC's VRF. It is omitted + // whenever no egress is bound, which is what keeps every conflist rendered + // until now byte-identical — the same property DAN was given. A node that + // receives no block installs no route, so a network reaches nothing + // outside the platform until a consumer asks for it. + Egress *Egress `json:"egress,omitempty"` +} + +// Egress is the outbound path this VPC takes out of the platform. +type Egress struct { + // ShardSIDs are the SRv6 uSIDs of the egress translation shards that may + // serve this VPC, in preference order. It is a candidate list rather than + // one SID because a shard's reachability is a fact only the node knows: + // the node keeps the first entry it can resolve a route toward. + ShardSIDs []string `json:"shardSIDs,omitempty"` } // IPAM is the delegated IPAM block. Presence alone decides whether IPAM runs. @@ -118,8 +133,10 @@ type Address struct { // Conflist renders the conflist for one attachment. Addresses are the addresses // NSO already allocated; an empty list means the guest addresses itself and no // IPAM block is emitted. Set dan for a guest whose hypervisor is handed the -// device rather than discovering it. -func Conflist(name, plugin, vpc, vpcAttachment string, mtu int32, addresses []Address, dan bool) NetConfList { +// device rather than discovering it. A nil egress renders no egress block, so +// the attachment reaches nothing outside the platform. +func Conflist(name, plugin, vpc, vpcAttachment string, mtu int32, addresses []Address, dan bool, + egress *Egress) NetConfList { master := MasterPlugin{ Type: plugin, VPC: vpc, @@ -131,19 +148,32 @@ func Conflist(name, plugin, vpc, vpcAttachment string, mtu int32, addresses []Ad if len(addresses) > 0 { master.IPAM = &IPAM{Type: PluginIPAM, Addresses: addresses} } + // An egress block holding no candidate is one a node can do nothing with, + // and it is not the same instruction as no block: absence is what tells the + // node to install no route. + if egress != nil && len(egress.ShardSIDs) == 0 { + egress = nil + } return NetConfList{ CNIVersion: CNIVersion, Name: name, Plugins: []any{ master, - BGPPlugin{Type: PluginBGP, VPC: vpc, VPCAttachment: vpcAttachment, Namespace: SystemNamespace}, + BGPPlugin{ + Type: PluginBGP, + VPC: vpc, + VPCAttachment: vpcAttachment, + Namespace: SystemNamespace, + Egress: egress, + }, }, } } // ConflistJSON renders the conflist as the string a NAD's spec.config holds. -func ConflistJSON(name, plugin, vpc, vpcAttachment string, mtu int32, addresses []Address, dan bool) (string, error) { - raw, err := json.Marshal(Conflist(name, plugin, vpc, vpcAttachment, mtu, addresses, dan)) +func ConflistJSON(name, plugin, vpc, vpcAttachment string, mtu int32, addresses []Address, dan bool, + egress *Egress) (string, error) { + raw, err := json.Marshal(Conflist(name, plugin, vpc, vpcAttachment, mtu, addresses, dan, egress)) if err != nil { return "", fmt.Errorf("marshal CNI conflist: %w", err) } diff --git a/internal/galactic/galactic_test.go b/internal/galactic/galactic_test.go index d1ac56a..842a66b 100644 --- a/internal/galactic/galactic_test.go +++ b/internal/galactic/galactic_test.go @@ -27,7 +27,7 @@ func TestConflistChainIsComplete(t *testing.T) { []Address{ {Address: "fd00:10:ff01:0:1::1/96", Gateway: "fd00:10:ff01::1"}, {Address: "172.20.1.7/32", Gateway: "172.20.1.1"}, - }, false) + }, false, nil) if conflist.CNIVersion != "1.0.0" { t.Errorf("cniVersion: got %q, want %q", conflist.CNIVersion, "1.0.0") @@ -64,7 +64,7 @@ func TestConflistChainIsComplete(t *testing.T) { } func TestConflistOmitsIPAMForSelfAddressingGuest(t *testing.T) { - raw, err := ConflistJSON("web-eth0", PluginTap, "0000000jU", "01a", 0, nil, false) + raw, err := ConflistJSON("web-eth0", PluginTap, "0000000jU", "01a", 0, nil, false, nil) if err != nil { t.Fatalf("ConflistJSON: %v", err) } @@ -131,11 +131,11 @@ func TestSplitAdvertisementName(t *testing.T) { // to the hypervisor. Every attachment rendered until now leaves it out, so its // absence has to stay the default. func TestConflistCarriesTheDeclaredDeviceRequest(t *testing.T) { - declared, err := ConflistJSON("vm-eth0", PluginTap, "0000000jU", "01a", 1400, nil, true) + declared, err := ConflistJSON("vm-eth0", PluginTap, "0000000jU", "01a", 1400, nil, true, nil) if err != nil { t.Fatalf("ConflistJSON: %v", err) } - discovered, err := ConflistJSON("vm-eth0", PluginTap, "0000000jU", "01a", 1400, nil, false) + discovered, err := ConflistJSON("vm-eth0", PluginTap, "0000000jU", "01a", 1400, nil, false, nil) if err != nil { t.Fatalf("ConflistJSON: %v", err) } @@ -158,3 +158,92 @@ func masterStanza(t *testing.T, raw string) map[string]any { } return decoded.Plugins[0] } + +// The egress block is the only new field in the conflist, and a node that +// receives none installs no egress route. Every conflist rendered before it +// existed has to stay byte-identical, so these are the exact strings the +// renderer produced at the commit that introduced the field. +func TestConflistWithoutEgressIsByteIdentical(t *testing.T) { + tests := []struct { + name string + got func() (string, error) + want string + }{ + { + name: "addressed container", + got: func() (string, error) { + return ConflistJSON("web-eth0", PluginVeth, "0000000jU", "01a", 1400, + []Address{{Address: "fd00:10:ff01:0:1::1/96", Gateway: "fd00:10:ff01::1"}}, false, nil) + }, + want: `{"cniVersion":"1.0.0","name":"web-eth0","plugins":[{"type":"galactic-veth","vpc":"0000000jU","vpcattachment":"01a","namespace":"galactic-system","mtu":1400,"ipam":{"type":"galactic-ipam","addresses":[{"address":"fd00:10:ff01:0:1::1/96","gateway":"fd00:10:ff01::1"}]}},{"type":"galactic-bgp","vpc":"0000000jU","vpcattachment":"01a","namespace":"galactic-system"}]}`, + }, + { + name: "declared guest", + got: func() (string, error) { + return ConflistJSON("web-eth0", PluginVeth, "0000000jU", "01a", 1400, + []Address{{Address: "fd00:10:ff01:0:1::1/96", Gateway: "fd00:10:ff01::1"}}, true, nil) + }, + want: `{"cniVersion":"1.0.0","name":"web-eth0","plugins":[{"type":"galactic-veth","vpc":"0000000jU","vpcattachment":"01a","namespace":"galactic-system","mtu":1400,"dan":true,"ipam":{"type":"galactic-ipam","addresses":[{"address":"fd00:10:ff01:0:1::1/96","gateway":"fd00:10:ff01::1"}]}},{"type":"galactic-bgp","vpc":"0000000jU","vpcattachment":"01a","namespace":"galactic-system"}]}`, + }, + { + name: "self addressing guest", + got: func() (string, error) { + return ConflistJSON("vm-eth0", PluginTap, "0000000jU", "01a", 0, nil, false, nil) + }, + want: `{"cniVersion":"1.0.0","name":"vm-eth0","plugins":[{"type":"galactic-tap","vpc":"0000000jU","vpcattachment":"01a","namespace":"galactic-system"},{"type":"galactic-bgp","vpc":"0000000jU","vpcattachment":"01a","namespace":"galactic-system"}]}`, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got, err := test.got() + if err != nil { + t.Fatalf("ConflistJSON: %v", err) + } + if got != test.want { + t.Errorf("conflist changed:\n got %s\nwant %s", got, test.want) + } + }) + } +} + +// The egress block is the whole contract with the node: it hangs off the +// galactic-bgp stanza, under one key, holding an ordered candidate list the +// node selects the first resolvable entry from. +func TestConflistCarriesTheEgressShardCandidates(t *testing.T) { + const want = `{"cniVersion":"1.0.0","name":"vm-eth0","plugins":[` + + `{"type":"galactic-tap","vpc":"0000000jU","vpcattachment":"01a","namespace":"galactic-system"},` + + `{"type":"galactic-bgp","vpc":"0000000jU","vpcattachment":"01a","namespace":"galactic-system",` + + `"egress":{"shardSIDs":["2001:db8:ff01::","2001:db8:ff02::"]}}]}` + + got, err := ConflistJSON("vm-eth0", PluginTap, "0000000jU", "01a", 0, nil, false, + &Egress{ShardSIDs: []string{"2001:db8:ff01::", "2001:db8:ff02::"}}) + if err != nil { + t.Fatalf("ConflistJSON: %v", err) + } + if got != want { + t.Errorf("conflist:\n got %s\nwant %s", got, want) + } +} + +// An empty candidate list is an egress block a node can do nothing with, so it +// renders as no block at all rather than as an empty one. +func TestConflistOmitsAnEmptyShardCandidateList(t *testing.T) { + raw, err := ConflistJSON("vm-eth0", PluginTap, "0000000jU", "01a", 0, nil, false, &Egress{}) + if err != nil { + t.Fatalf("ConflistJSON: %v", err) + } + if _, present := bgpStanza(t, raw)["egress"]; present { + t.Errorf("egress block present with no candidates: %s", raw) + } +} + +func bgpStanza(t *testing.T, raw string) map[string]any { + t.Helper() + var decoded struct { + Plugins []map[string]any `json:"plugins"` + } + if err := json.Unmarshal([]byte(raw), &decoded); err != nil { + t.Fatalf("unmarshal conflist: %v", err) + } + return decoded.Plugins[1] +} From a578d2e78fed4d15242bf5a188fb14ceb28e55c6 Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Thu, 17 Sep 2026 17:48:55 -0500 Subject: [PATCH 2/3] feat: Report the egress address on the attachment A consumer can now read the address their traffic leaves the platform on, per interface. This controller is the only component that resolved which shard a network bound to, so it publishes the answer on VPCAttachment status, which NSO already reaches through the generic attachmentRef on a network interface and reads without a typed dependency on this module. Stability is derived here from the serving class's sharing rather than left to the consumer: Shared reports None, Dedicated reports Network. Sharing is an operator-side decision about the platform, and a consumer interpreting it would be deciding for themselves whether allow-listing an address is safe. Nothing is published unless the platform can state it. No shard, absent intent, Disabled, a shard that has reported no address, or a class whose sharing never projected all report no egress at all. An absent address is not an empty list and not a placeholder, and one that was published is withdrawn when the path is. Key changes: - Add egress.internet.sourceAddresses to VPCAttachment status, keyed by family, with an IPv6-only family enum while IPv4 reach stays refused - Resolve egress once per reconcile pass, so the conflist the node reads and the address the consumer reads are the same answer - Report the preferred shard's address only. The candidate list is a preference the node resolves to one entry; reporting every candidate would name addresses the traffic does not leave on - Keep whole-object Status().Update rather than moving to server-side apply. The new fields join a writer that already writes this status, so the writer count stays at two over disjoint field sets, and an update carrying a superseded resourceVersion is rejected rather than overwriting the other writer. A status half-written by SSA and half replaced wholesale is worse than either, so converting is a decision for the type, not a side effect of adding three fields - Record that shards live in galactic-system, in the field an operator fills in. shardNamespace stays required: it names the nodes every network on a class leaves through - Replace the committed replace directives with an untracked go.work. CI checks out this repo alone, so a committed replace to a sibling path fails its build outright Co-Authored-By: Claude Opus 5 (1M context) --- api/v1alpha1/egressshardparameters_types.go | 5 + api/v1alpha1/vpcattachment_types.go | 88 +++++++ api/v1alpha1/zz_generated.deepcopy.go | 60 +++++ ...d.datumapis.com_egressshardparameters.yaml | 5 + .../cloud.datumapis.com_vpcattachments.yaml | 63 +++++ docs/api/vpc.md | 98 +++++++- go.mod | 4 - go.sum | 4 + .../controller/networkinterface_controller.go | 173 +++++++++++-- .../networkinterface_controller_test.go | 229 +++++++++++++++++- 10 files changed, 693 insertions(+), 36 deletions(-) diff --git a/api/v1alpha1/egressshardparameters_types.go b/api/v1alpha1/egressshardparameters_types.go index ee0708a..0c8def1 100644 --- a/api/v1alpha1/egressshardparameters_types.go +++ b/api/v1alpha1/egressshardparameters_types.go @@ -43,6 +43,11 @@ type EgressShardParametersSpec struct { // through. Naming the one namespace an operator owns keeps that // unreachable. // + // It carries no default even though every deployment today answers + // galactic-system, which is where the galactic data plane's own objects + // live. The namespace names the nodes that every network on this class + // leaves the platform through, and that is worth an operator stating. + // // +kubebuilder:validation:Required // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=63 diff --git a/api/v1alpha1/vpcattachment_types.go b/api/v1alpha1/vpcattachment_types.go index 13cbe33..674ee0c 100644 --- a/api/v1alpha1/vpcattachment_types.go +++ b/api/v1alpha1/vpcattachment_types.go @@ -114,6 +114,85 @@ type VPCAttachmentInterface struct { Addresses []IPAddress `json:"addresses,omitempty"` } +// InternetEgressAddressFamily is the address family of an egress source +// address. +// +// Only IPv6 is reported. Reaching an IPv4 destination needs a resolver and a +// translator sharing a prefix, which the platform pairs neither of, so the +// value is withheld rather than reported and not delivered. An address written +// today records IPv6, so accepting IPv4 later changes no attachment. +// +// +kubebuilder:validation:Enum=IPv6 +type InternetEgressAddressFamily string + +// InternetEgressAddressFamilyIPv6 is an IPv6 egress source address. +const InternetEgressAddressFamilyIPv6 InternetEgressAddressFamily = "IPv6" + +// InternetEgressAddressStability is how far a consumer may rely on an egress +// source address. It is the consumer-side projection of the serving class's +// sharing, derived here so a consumer never reads a class. +// +// +kubebuilder:validation:Enum=None;Network +type InternetEgressAddressStability string + +const ( + // InternetEgressAddressStabilityNone means the address may change and + // other networks share it. Allow-listing it admits traffic from other + // networks and loses access when the address changes. + InternetEgressAddressStabilityNone InternetEgressAddressStability = "None" + + // InternetEgressAddressStabilityNetwork means the address belongs to this + // network and persists. Allow-listing it is safe. + InternetEgressAddressStabilityNetwork InternetEgressAddressStability = "Network" +) + +// InternetEgressSourceAddress is one address outbound traffic leaves on. +// +// +kubebuilder:validation:XValidation:rule="self.family != 'IPv6' || (isIP(self.address) && ip(self.address).family() == 6)",message="an IPv6 source address must be a valid IPv6 address" +type InternetEgressSourceAddress struct { + // Family is the address family of this source address. + // +required + Family InternetEgressAddressFamily `json:"family"` + + // Address is the source address translation writes, without a prefix + // length. + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=39 + // +required + Address string `json:"address"` + + // Stability states how far a consumer may rely on this address before + // they act on it. + // +required + Stability InternetEgressAddressStability `json:"stability"` +} + +// VPCAttachmentInternetEgressStatus reports the outbound path this attachment +// leaves the platform on. +type VPCAttachmentInternetEgressStatus struct { + // SourceAddresses are the addresses translation writes for this + // attachment, one per family reached. + // + // Absent means this attachment reaches nothing outside the platform, or + // that no address has been reported for a path that does. An absent list + // is never a placeholder: a consumer that allow-listed a guessed address + // would admit the wrong traffic and believe otherwise. + // + // +listType=map + // +listMapKey=family + // +kubebuilder:validation:MaxItems=2 + // +optional + SourceAddresses []InternetEgressSourceAddress `json:"sourceAddresses,omitempty"` +} + +// VPCAttachmentEgressStatus reports what this attachment reaches outside the +// platform. +type VPCAttachmentEgressStatus struct { + // Internet is the internet egress realized for this attachment. + // +optional + Internet *VPCAttachmentInternetEgressStatus `json:"internet,omitempty"` +} + // VPCAttachmentStatus defines the observed state of VPCAttachment. // // Every field but Conditions is optional: an identifier is recorded before a pod @@ -181,6 +260,15 @@ type VPCAttachmentStatus struct { // +kubebuilder:validation:MinLength=1 // +optional NetworkAttachmentDefinition string `json:"networkAttachmentDefinition,omitempty"` + + // Egress reports what this attachment reaches outside the platform. + // + // It is reported per attachment rather than on the network, because the + // interface is what a workload holds and what a consumer reads back + // through. This controller is the only component that resolved which shard + // the network bound to, so it is the only one that can report the answer. + // +optional + Egress *VPCAttachmentEgressStatus `json:"egress,omitempty"` } // +kubebuilder:object:root=true diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 1a1d538..f75c2fd 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -100,6 +100,21 @@ func (in *EgressShardParametersSpec) DeepCopy() *EgressShardParametersSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *InternetEgressSourceAddress) DeepCopyInto(out *InternetEgressSourceAddress) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new InternetEgressSourceAddress. +func (in *InternetEgressSourceAddress) DeepCopy() *InternetEgressSourceAddress { + if in == nil { + return nil + } + out := new(InternetEgressSourceAddress) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NetworkFabricIdentity) DeepCopyInto(out *NetworkFabricIdentity) { *out = *in @@ -258,6 +273,26 @@ func (in *VPCAttachment) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *VPCAttachmentEgressStatus) DeepCopyInto(out *VPCAttachmentEgressStatus) { + *out = *in + if in.Internet != nil { + in, out := &in.Internet, &out.Internet + *out = new(VPCAttachmentInternetEgressStatus) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VPCAttachmentEgressStatus. +func (in *VPCAttachmentEgressStatus) DeepCopy() *VPCAttachmentEgressStatus { + if in == nil { + return nil + } + out := new(VPCAttachmentEgressStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *VPCAttachmentInterface) DeepCopyInto(out *VPCAttachmentInterface) { *out = *in @@ -278,6 +313,26 @@ func (in *VPCAttachmentInterface) DeepCopy() *VPCAttachmentInterface { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *VPCAttachmentInternetEgressStatus) DeepCopyInto(out *VPCAttachmentInternetEgressStatus) { + *out = *in + if in.SourceAddresses != nil { + in, out := &in.SourceAddresses, &out.SourceAddresses + *out = make([]InternetEgressSourceAddress, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VPCAttachmentInternetEgressStatus. +func (in *VPCAttachmentInternetEgressStatus) DeepCopy() *VPCAttachmentInternetEgressStatus { + if in == nil { + return nil + } + out := new(VPCAttachmentInternetEgressStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *VPCAttachmentList) DeepCopyInto(out *VPCAttachmentList) { *out = *in @@ -342,6 +397,11 @@ func (in *VPCAttachmentStatus) DeepCopyInto(out *VPCAttachmentStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.Egress != nil { + in, out := &in.Egress, &out.Egress + *out = new(VPCAttachmentEgressStatus) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VPCAttachmentStatus. diff --git a/config/crd/cloud.datumapis.com_egressshardparameters.yaml b/config/crd/cloud.datumapis.com_egressshardparameters.yaml index b486db6..914ab64 100644 --- a/config/crd/cloud.datumapis.com_egressshardparameters.yaml +++ b/config/crd/cloud.datumapis.com_egressshardparameters.yaml @@ -73,6 +73,11 @@ spec: traffic — and everyone else's on the same class — leaves the platform through. Naming the one namespace an operator owns keeps that unreachable. + + It carries no default even though every deployment today answers + galactic-system, which is where the galactic data plane's own objects + live. The namespace names the nodes that every network on this class + leaves the platform through, and that is worth an operator stating. maxLength: 63 minLength: 1 type: string diff --git a/config/crd/cloud.datumapis.com_vpcattachments.yaml b/config/crd/cloud.datumapis.com_vpcattachments.yaml index d49ec0b..e030fa9 100644 --- a/config/crd/cloud.datumapis.com_vpcattachments.yaml +++ b/config/crd/cloud.datumapis.com_vpcattachments.yaml @@ -164,6 +164,69 @@ spec: maxLength: 46 minLength: 46 type: string + egress: + description: |- + Egress reports what this attachment reaches outside the platform. + + It is reported per attachment rather than on the network, because the + interface is what a workload holds and what a consumer reads back + through. This controller is the only component that resolved which shard + the network bound to, so it is the only one that can report the answer. + properties: + internet: + description: Internet is the internet egress realized for this + attachment. + properties: + sourceAddresses: + description: |- + SourceAddresses are the addresses translation writes for this + attachment, one per family reached. + + Absent means this attachment reaches nothing outside the platform, or + that no address has been reported for a path that does. An absent list + is never a placeholder: a consumer that allow-listed a guessed address + would admit the wrong traffic and believe otherwise. + items: + description: InternetEgressSourceAddress is one address + outbound traffic leaves on. + properties: + address: + description: |- + Address is the source address translation writes, without a prefix + length. + maxLength: 39 + minLength: 1 + type: string + family: + description: Family is the address family of this source + address. + enum: + - IPv6 + type: string + stability: + description: |- + Stability states how far a consumer may rely on this address before + they act on it. + enum: + - None + - Network + type: string + required: + - address + - family + - stability + type: object + x-kubernetes-validations: + - message: an IPv6 source address must be a valid IPv6 address + rule: self.family != 'IPv6' || (isIP(self.address) && + ip(self.address).family() == 6) + maxItems: 2 + type: array + x-kubernetes-list-map-keys: + - family + x-kubernetes-list-type: map + type: object + type: object guestInterface: description: Guest-side veth device name (e.g., "G000000010013G"). minLength: 1 diff --git a/docs/api/vpc.md b/docs/api/vpc.md index 42939f8..191fdad 100644 --- a/docs/api/vpc.md +++ b/docs/api/vpc.md @@ -61,7 +61,7 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `shardNamespace` _string_ | ShardNamespace is the namespace holding the EgressShard objects this
selector may match.
It is required and there is no cluster-wide search. A selector evaluated
over every namespace would match an EgressShard a tenant created in a
namespace they write to, which is a tenant naming the node their own
traffic — and everyone else's on the same class — leaves the platform
through. Naming the one namespace an operator owns keeps that
unreachable. | | MaxLength: 63
MinLength: 1
Required: \{\}
| +| `shardNamespace` _string_ | ShardNamespace is the namespace holding the EgressShard objects this
selector may match.
It is required and there is no cluster-wide search. A selector evaluated
over every namespace would match an EgressShard a tenant created in a
namespace they write to, which is a tenant naming the node their own
traffic — and everyone else's on the same class — leaves the platform
through. Naming the one namespace an operator owns keeps that
unreachable.
It carries no default even though every deployment today answers
galactic-system, which is where the galactic data plane's own objects
live. The namespace names the nodes that every network on this class
leaves the platform through, and that is worth an operator stating. | | MaxLength: 63
MinLength: 1
Required: \{\}
| | `shardSelector` _[LabelSelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v/#labelselector-v1-meta)_ | ShardSelector selects the EgressShards a network on this class egresses
through, by the network.datumapis.com/egress-* labels an operator sets
on them.
An empty selector matches every shard in the namespace, which sends a
consumer's traffic out of an arbitrary cell. Egress is realized per
cell, so a selector is expected to pin a cell and a pool.
The selector runs one way, as the only binding between a class and the
shards serving it: a shard names nothing that selects it, which is what
keeps the data-plane API group independent of the consumer-facing one. | | Required: \{\}
| @@ -79,6 +79,67 @@ _Appears in:_ +#### InternetEgressAddressFamily + +_Underlying type:_ _string_ + +InternetEgressAddressFamily is the address family of an egress source +address. + +Only IPv6 is reported. Reaching an IPv4 destination needs a resolver and a +translator sharing a prefix, which the platform pairs neither of, so the +value is withheld rather than reported and not delivered. An address written +today records IPv6, so accepting IPv4 later changes no attachment. + +_Validation:_ +- Enum: [IPv6] + +_Appears in:_ +- [InternetEgressSourceAddress](#internetegresssourceaddress) + +| Field | Description | +| --- | --- | +| `IPv6` | | + + +#### InternetEgressAddressStability + +_Underlying type:_ _string_ + +InternetEgressAddressStability is how far a consumer may rely on an egress +source address. It is the consumer-side projection of the serving class's +sharing, derived here so a consumer never reads a class. + +_Validation:_ +- Enum: [None Network] + +_Appears in:_ +- [InternetEgressSourceAddress](#internetegresssourceaddress) + +| Field | Description | +| --- | --- | +| `None` | InternetEgressAddressStabilityNone means the address may change and
other networks share it. Allow-listing it admits traffic from other
networks and loses access when the address changes.
| +| `Network` | InternetEgressAddressStabilityNetwork means the address belongs to this
network and persists. Allow-listing it is safe.
| + + +#### InternetEgressSourceAddress + + + +InternetEgressSourceAddress is one address outbound traffic leaves on. + + + +_Appears in:_ +- [VPCAttachmentInternetEgressStatus](#vpcattachmentinternetegressstatus) + +| Field | Description | Default | Validation | +| --- | --- | --- | --- | +| `family` _[InternetEgressAddressFamily](#internetegressaddressfamily)_ | Family is the address family of this source address. | | Enum: [IPv6]
| +| `address` _string_ | Address is the source address translation writes, without a prefix
length. | | MaxLength: 39
MinLength: 1
| +| `stability` _[InternetEgressAddressStability](#internetegressaddressstability)_ | Stability states how far a consumer may rely on this address before
they act on it. | | Enum: [None Network]
| + + #### Network _Underlying type:_ _string_ @@ -222,6 +283,23 @@ VPCAttachment is the Schema for the vpcattachments API | `status` _[VPCAttachmentStatus](#vpcattachmentstatus)_ | status defines the observed state of VPCAttachment | | | +#### VPCAttachmentEgressStatus + + + +VPCAttachmentEgressStatus reports what this attachment reaches outside the +platform. + + + +_Appears in:_ +- [VPCAttachmentStatus](#vpcattachmentstatus) + +| Field | Description | Default | Validation | +| --- | --- | --- | --- | +| `internet` _[VPCAttachmentInternetEgressStatus](#vpcattachmentinternetegressstatus)_ | Internet is the internet egress realized for this attachment. | | | + + #### VPCAttachmentInterface @@ -261,6 +339,23 @@ _Appears in:_ | `HypervisorDeclared` | VPCAttachmentInterfaceModeHypervisorDeclared also hands the interface to a
hypervisor as a device. It differs from Hypervisor in who tells the
hypervisor that the device exists. Under Hypervisor the hypervisor finds
the device from what the node publishes. Under HypervisorDeclared the data
plane states the device, its addresses, and its MTU to the hypervisor
directly, which is what a guest whose hypervisor reads no node state
needs.
| +#### VPCAttachmentInternetEgressStatus + + + +VPCAttachmentInternetEgressStatus reports the outbound path this attachment +leaves the platform on. + + + +_Appears in:_ +- [VPCAttachmentEgressStatus](#vpcattachmentegressstatus) + +| Field | Description | Default | Validation | +| --- | --- | --- | --- | +| `sourceAddresses` _[InternetEgressSourceAddress](#internetegresssourceaddress) array_ | SourceAddresses are the addresses translation writes for this
attachment, one per family reached.
Absent means this attachment reaches nothing outside the platform, or
that no address has been reported for a path that does. An absent list
is never a placeholder: a consumer that allow-listed a guessed address
would admit the wrong traffic and believe otherwise. | | MaxItems: 2
| + + #### VPCAttachmentSpec @@ -307,6 +402,7 @@ _Appears in:_ | `guestInterface` _string_ | Guest-side veth device name (e.g., "G000000010013G"). | | MinLength: 1
| | `podSubnet` _string_ | Allocated subnet in CIDR notation (e.g., "fd00:10:ff01:0:1::/80"). | | MinLength: 1
| | `networkAttachmentDefinition` _string_ | NetworkAttachmentDefinition rendered for this attachment. | | MinLength: 1
| +| `egress` _[VPCAttachmentEgressStatus](#vpcattachmentegressstatus)_ | Egress reports what this attachment reaches outside the platform.
It is reported per attachment rather than on the network, because the
interface is what a workload holds and what a consumer reads back
through. This controller is the only component that resolved which shard
the network bound to, so it is the only one that can report the answer. | | | #### VPCRef diff --git a/go.mod b/go.mod index 9c4ad41..0431086 100644 --- a/go.mod +++ b/go.mod @@ -78,7 +78,3 @@ 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 => ../network-egress-spec - -replace go.datum.net/network-services-operator => ../nso-internet-egress diff --git a/go.sum b/go.sum index 896395c..6c1b0e7 100644 --- a/go.sum +++ b/go.sum @@ -121,6 +121,10 @@ 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= diff --git a/internal/controller/networkinterface_controller.go b/internal/controller/networkinterface_controller.go index 79e5d15..69eb33b 100644 --- a/internal/controller/networkinterface_controller.go +++ b/internal/controller/networkinterface_controller.go @@ -154,15 +154,23 @@ func (r *NetworkInterfaceReconciler) Reconcile(ctx context.Context, req ctrl.Req return ctrl.Result{}, fmt.Errorf("get NetworkContext %s: %w", vpcKey, err) } + // Resolved once for the whole pass. The conflist the node reads and the + // address a consumer reads back have to be the same answer, and resolving + // twice could produce two. + egress, err := r.resolveInternetEgress(ctx, &networkContext) + if err != nil { + return ctrl.Result{}, err + } + attachment, err := r.reconcileAttachment(ctx, &networkInterface, &vpc) if err != nil { return ctrl.Result{}, err } - nad, err := r.reconcileNAD(ctx, attachment, &vpc, &networkInterface, &networkContext) + nad, err := r.reconcileNAD(ctx, attachment, &vpc, &networkInterface, egress) if err != nil { return ctrl.Result{}, err } - if err := r.publishAttachmentStatus(ctx, attachment, &vpc, nad); err != nil { + if err := r.publishAttachmentStatus(ctx, attachment, &vpc, nad, egress); err != nil { return ctrl.Result{}, err } return ctrl.Result{}, r.publishToInterface(ctx, &networkInterface, attachment, &vpc) @@ -209,7 +217,7 @@ func (r *NetworkInterfaceReconciler) reconcileNAD( attachment *cloudv1alpha1.VPCAttachment, vpc *cloudv1alpha1.VPC, networkInterface *networkingv1alpha.NetworkInterface, - networkContext *networkingv1alpha.NetworkContext, + egress *internetEgress, ) (*nadv1.NetworkAttachmentDefinition, error) { nad := &nadv1.NetworkAttachmentDefinition{ ObjectMeta: metav1.ObjectMeta{Name: attachment.Name, Namespace: attachment.Namespace}, @@ -236,13 +244,9 @@ func (r *NetworkInterfaceReconciler) reconcileNAD( Gateway: address.Gateway, }) } - egress, err := r.resolveInternetEgress(ctx, networkContext) - if err != nil { - return nil, err - } config, err := galactic.ConflistJSON(attachment.Name, masterPlugin(attachment.Spec.Interface.Mode), vpc.Status.VPC, attachmentID, networkInterface.Spec.MTU, addresses, - declaresDevice(attachment.Spec.Interface.Mode), egress) + declaresDevice(attachment.Spec.Interface.Mode), egress.conflist()) if err != nil { return nil, err } @@ -312,10 +316,48 @@ func interfaceAddresses(networkInterface *networkingv1alpha.NetworkInterface) [] return addresses } +// internetEgress is what one location's egress intent resolved to: the +// candidates a node routes toward, and the address a consumer reads back. +// +// A nil internetEgress is a network that reaches nothing outside the platform. +// It is not an empty one: absence is the instruction, in the conflist and on +// the attachment alike. +type internetEgress struct { + shardSIDs []string + + // sourceAddress is what translation writes, resolved from the shard the + // node prefers. Empty when no selected shard has reported an address yet, + // or when the class's sharing was never projected and the stability a + // consumer needs before acting cannot be derived. + sourceAddress *cloudv1alpha1.InternetEgressSourceAddress +} + +// conflist renders the block the node reads, or nothing. +func (e *internetEgress) conflist() *galactic.Egress { + if e == nil { + return nil + } + return &galactic.Egress{ShardSIDs: e.shardSIDs} +} + +// status renders what a consumer reads back, or nothing. An address the +// platform cannot state is reported as no egress rather than as a guess: a +// consumer allow-listing the wrong address admits the wrong traffic and has no +// way to tell. +func (e *internetEgress) status() *cloudv1alpha1.VPCAttachmentEgressStatus { + if e == nil || e.sourceAddress == nil { + return nil + } + return &cloudv1alpha1.VPCAttachmentEgressStatus{ + Internet: &cloudv1alpha1.VPCAttachmentInternetEgressStatus{ + SourceAddresses: []cloudv1alpha1.InternetEgressSourceAddress{*e.sourceAddress}, + }, + } +} + // resolveInternetEgress turns the egress intent projected onto a NetworkContext -// into the ordered shard candidates a node routes this VPC's VRF toward. A nil -// result renders no egress block, so the VPC reaches nothing outside the -// platform. +// into the ordered shard candidates a node routes this VPC's VRF toward, and +// the source address those candidates translate to. // // Egress intent is a function of (VPC, cell) and of nothing else — not of the // attachment, the interface, or the claim. The kernel VRF is shared by every @@ -331,7 +373,7 @@ func interfaceAddresses(networkInterface *networkingv1alpha.NetworkInterface) [] // matched, breaks the invariant without breaking a test. func (r *NetworkInterfaceReconciler) resolveInternetEgress( ctx context.Context, networkContext *networkingv1alpha.NetworkContext, -) (*galactic.Egress, error) { +) (*internetEgress, error) { log := logf.FromContext(ctx) intent := internetEgressIntent(networkContext) @@ -379,35 +421,92 @@ func (r *NetworkInterfaceReconciler) resolveInternetEgress( return nil, err } - sids := make([]string, 0, len(shards)) - sources := make([]string, 0, len(shards)) + resolved := &internetEgress{shardSIDs: make([]string, 0, len(shards))} + var preferred *bgpv1alpha1.EgressShard for i := range shards { // A shard whose SID is unreported has nothing a node can route toward. // The SID stays in status because nothing allocates one yet. if shards[i].Status.ShardSID == "" { continue } - if slices.Contains(sids, shards[i].Status.ShardSID) { + if slices.Contains(resolved.shardSIDs, shards[i].Status.ShardSID) { continue } - sids = append(sids, shards[i].Status.ShardSID) - if address := shards[i].Status.ShardAddressIPv6; address != "" { - sources = append(sources, address) + resolved.shardSIDs = append(resolved.shardSIDs, shards[i].Status.ShardSID) + if preferred == nil { + preferred = &shards[i] } } - if len(sids) == 0 { + if len(resolved.shardSIDs) == 0 { log.Info("internet egress is enabled but no shard serves this network", "networkContext", networkContext.Name, "class", intent.ClassName, "parameters", parameters.Name) return nil, nil } - // The source addresses are the answer a consumer reads back. Nothing in - // this cell writes them onto the network context yet, so they are reported - // here and nowhere else. - log.Info("internet egress bound", "networkContext", networkContext.Name, - "class", intent.ClassName, "shardSIDs", sids, "sourceAddressesIPv6", sources) - return &galactic.Egress{ShardSIDs: sids}, nil + resolved.sourceAddress = sourceAddress(preferred, intent.Sharing) + if resolved.sourceAddress == nil { + log.Info("internet egress is bound but no source address can be reported", + "networkContext", networkContext.Name, "shard", preferred.Name, + "sharing", intent.Sharing) + } + log.V(1).Info("internet egress bound", "networkContext", networkContext.Name, + "class", intent.ClassName, "shardSIDs", resolved.shardSIDs, + "sourceAddress", resolved.sourceAddress) + return resolved, nil +} + +// sourceAddress is what a consumer reads back for the shard the node prefers. +// +// The candidate list is a preference the node resolves down to one entry, so +// the first candidate is the shard traffic is intended to leave through and its +// address is the one to report. Reporting every candidate's address would tell +// a consumer their traffic leaves on addresses it does not. +// +// The address itself is write-once and immutable upstream, so a reported value +// that changes means the shard it came from was replaced, not that the platform +// renumbered a live one. +// +// Nothing is reported unless both halves are known. An address without the +// stability that qualifies it invites the allow-listing that stability exists +// to forbid. +func sourceAddress( + shard *bgpv1alpha1.EgressShard, sharing networkingv1alpha.InternetEgressSharing, +) *cloudv1alpha1.InternetEgressSourceAddress { + if shard == nil || shard.Status.ShardAddressIPv6 == "" { + return nil + } + stability, ok := addressStability(sharing) + if !ok { + return nil + } + return &cloudv1alpha1.InternetEgressSourceAddress{ + Family: cloudv1alpha1.InternetEgressAddressFamilyIPv6, + Address: shard.Status.ShardAddressIPv6, + Stability: stability, + } +} + +// addressStability projects the serving class's sharing into the contract a +// consumer acts on. The projection is made here rather than by the consumer: +// sharing is an operator-side decision about the platform, and a consumer that +// had to interpret it would be deciding for themselves whether allow-listing an +// address is safe. +func addressStability( + sharing networkingv1alpha.InternetEgressSharing, +) (cloudv1alpha1.InternetEgressAddressStability, bool) { + switch sharing { + case networkingv1alpha.InternetEgressSharingShared: + return cloudv1alpha1.InternetEgressAddressStabilityNone, true + case networkingv1alpha.InternetEgressSharingDedicated: + return cloudv1alpha1.InternetEgressAddressStabilityNetwork, true + default: + // Sharing is optional upstream, so an unprojected value is an ordinary + // answer. There is no safe default: guessing Shared understates a + // dedicated address, and guessing Dedicated invites an allow-list of a + // shared one. + return "", false + } } // internetEgressIntent reads the internet egress a location was instructed to @@ -487,16 +586,38 @@ func (r *NetworkInterfaceReconciler) allocateAttachmentIdentifier(ctx context.Co vpc, maxIdentifierAttempts) } -// publishAttachmentStatus records the allocated identifiers on the attachment. +// publishAttachmentStatus records the allocated identifiers on the attachment, +// and the egress address a consumer reads back through the interface that holds +// it. +// +// Two reconcilers write this status, over disjoint field sets: this one writes +// the identifiers, the attachment definition and now the egress address, and +// the BGPAdvertisement reconciler writes what the node programmed. Both do a +// whole-object Status().Update, which carries the resourceVersion it was read +// at, so a writer working from a copy the other has since superseded is +// rejected with a conflict and retries — it does not overwrite fields it never +// set. Adding a field set to a reconciler that already writes here keeps the +// writer count at two and that property intact. +// +// Server-side apply was considered and rejected. It would have to convert both +// writers to be coherent: a status subresource written by SSA on one side and +// replaced wholesale on the other is worse than either alone, because the +// wholesale writer drops whatever it did not read. Converting both means +// generated apply configurations this repository does not produce, or the +// deprecated unstructured apply path whose single use here is a foreign object. +// That is a change to make deliberately, for the type as a whole, and not as a +// side effect of adding three fields. func (r *NetworkInterfaceReconciler) publishAttachmentStatus( ctx context.Context, attachment *cloudv1alpha1.VPCAttachment, vpc *cloudv1alpha1.VPC, nad *nadv1.NetworkAttachmentDefinition, + egress *internetEgress, ) error { attachment.Status.VPC = vpc.Status.VPC attachment.Status.VPCAttachment = nad.Labels[LabelVPCAttachment] attachment.Status.NetworkAttachmentDefinition = nad.Name + attachment.Status.Egress = egress.status() attachment.Status.ObservedGeneration = attachment.Generation meta.SetStatusCondition(&attachment.Status.Conditions, metav1.Condition{ Type: cloudv1alpha1.ConditionTypeReady, diff --git a/internal/controller/networkinterface_controller_test.go b/internal/controller/networkinterface_controller_test.go index f7458fb..544ab6d 100644 --- a/internal/controller/networkinterface_controller_test.go +++ b/internal/controller/networkinterface_controller_test.go @@ -21,6 +21,7 @@ import ( "slices" "testing" + nadv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -247,8 +248,8 @@ func TestResolveInternetEgressSelectsMatchingShards(t *testing.T) { // apart have to compute the same list, because the VRF they share cannot // hold two answers. want := []string{"2001:db8:ff01::", "2001:db8:ff02::"} - if !slices.Equal(egress.ShardSIDs, want) { - t.Errorf("shard SIDs: got %v, want %v", egress.ShardSIDs, want) + if !slices.Equal(egress.shardSIDs, want) { + t.Errorf("shard SIDs: got %v, want %v", egress.shardSIDs, want) } } @@ -347,7 +348,16 @@ func TestResolveInternetEgressYieldsNothingWhenUnbound(t *testing.T) { t.Fatalf("resolveInternetEgress: %v", err) } if egress != nil { - t.Errorf("got %v, want no egress", egress.ShardSIDs) + t.Errorf("got %v, want no egress", egress.shardSIDs) + } + // Absence has to reach both sides: a node that receives no block + // installs no route, and a consumer who reads no address has none + // to act on. + if egress.conflist() != nil { + t.Error("no egress resolved but a conflist block rendered") + } + if egress.status() != nil { + t.Error("no egress resolved but an address was published") } }) } @@ -366,7 +376,7 @@ func TestResolveInternetEgressDeduplicatesShardSIDs(t *testing.T) { if err != nil { t.Fatalf("resolveInternetEgress: %v", err) } - if want := []string{"2001:db8:ff01::"}; egress == nil || !slices.Equal(egress.ShardSIDs, want) { + if want := []string{"2001:db8:ff01::"}; egress == nil || !slices.Equal(egress.shardSIDs, want) { t.Errorf("shard SIDs: got %v, want %v", egress, want) } } @@ -391,7 +401,216 @@ func TestResolveInternetEgressIsAFunctionOfTheNetworkContextAlone(t *testing.T) if err != nil { t.Fatalf("resolveInternetEgress: %v", err) } - if first == nil || second == nil || !slices.Equal(first.ShardSIDs, second.ShardSIDs) { + if first == nil || second == nil || !slices.Equal(first.shardSIDs, second.shardSIDs) { t.Errorf("two attachments of one VPC resolved %v and %v", first, second) } } + +// The address a consumer reads back, and the contract that qualifies it. The +// stability is derived here rather than by the consumer, so this is the only +// place the class's sharing is interpreted. +func TestResolveInternetEgressPublishesTheSourceAddress(t *testing.T) { + tests := []struct { + name string + sharing networkingv1alpha.InternetEgressSharing + want cloudv1alpha1.InternetEgressAddressStability + }{ + {"shared", networkingv1alpha.InternetEgressSharingShared, + cloudv1alpha1.InternetEgressAddressStabilityNone}, + {"dedicated", networkingv1alpha.InternetEgressSharingDedicated, + cloudv1alpha1.InternetEgressAddressStabilityNetwork}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + r := newEgressReconciler(t, + newEgressParameters(), + newEgressShard("shard-a", "2001:db8:ff01::", "2001:db8:f00d::100", poolLabels()), + ) + networkContext := newEgressContext(networkingv1alpha.NetworkInternetEgressEnabled) + networkContext.Spec.Egress.Internet.Sharing = test.sharing + + egress, err := r.resolveInternetEgress(t.Context(), networkContext) + if err != nil { + t.Fatalf("resolveInternetEgress: %v", err) + } + status := egress.status() + if status == nil || status.Internet == nil { + t.Fatal("a bound shard reporting an address published nothing") + } + addresses := status.Internet.SourceAddresses + if len(addresses) != 1 { + t.Fatalf("source addresses: got %d, want 1", len(addresses)) + } + if addresses[0].Family != cloudv1alpha1.InternetEgressAddressFamilyIPv6 { + t.Errorf("family: got %q, want IPv6", addresses[0].Family) + } + if addresses[0].Address != "2001:db8:f00d::100" { + t.Errorf("address: got %q, want %q", addresses[0].Address, "2001:db8:f00d::100") + } + if addresses[0].Stability != test.want { + t.Errorf("stability: got %q, want %q", addresses[0].Stability, test.want) + } + }) + } +} + +// Egress that works and an address that cannot yet be stated are different +// facts. The node is told where to route; the consumer is told nothing rather +// than a value they might allow-list. +func TestResolveInternetEgressWithholdsAnAddressItCannotState(t *testing.T) { + tests := []struct { + name string + sharing networkingv1alpha.InternetEgressSharing + address string + }{ + {"shard has reported no address", networkingv1alpha.InternetEgressSharingShared, ""}, + {"sharing was never projected", "", "2001:db8:f00d::100"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + r := newEgressReconciler(t, + newEgressParameters(), + newEgressShard("shard-a", "2001:db8:ff01::", test.address, poolLabels()), + ) + networkContext := newEgressContext(networkingv1alpha.NetworkInternetEgressEnabled) + networkContext.Spec.Egress.Internet.Sharing = test.sharing + + egress, err := r.resolveInternetEgress(t.Context(), networkContext) + if err != nil { + t.Fatalf("resolveInternetEgress: %v", err) + } + if egress == nil || egress.conflist() == nil { + t.Fatal("a bound shard rendered no route for the node") + } + if status := egress.status(); status != nil { + t.Errorf("published %v, want no address", status.Internet.SourceAddresses) + } + }) + } +} + +// One address is reported, for the shard the node prefers. Reporting every +// candidate's address would tell a consumer their traffic leaves on addresses +// it does not. +func TestResolveInternetEgressReportsThePreferredShardsAddress(t *testing.T) { + r := newEgressReconciler(t, + newEgressParameters(), + newEgressShard("shard-b", "2001:db8:ff02::", "2001:db8:f00d::200", poolLabels()), + newEgressShard("shard-a", "2001:db8:ff01::", "2001:db8:f00d::100", poolLabels()), + ) + + egress, err := r.resolveInternetEgress(t.Context(), + newEgressContext(networkingv1alpha.NetworkInternetEgressEnabled)) + if err != nil { + t.Fatalf("resolveInternetEgress: %v", err) + } + addresses := egress.status().Internet.SourceAddresses + if len(addresses) != 1 || addresses[0].Address != "2001:db8:f00d::100" { + t.Errorf("got %v, want only the first candidate's address", addresses) + } +} + +// Egress withdrawn has to be egress unreported. An address left behind on the +// attachment is one a consumer keeps allow-listing after the path is gone. +func TestPublishAttachmentStatusWithdrawsAnUnboundAddress(t *testing.T) { + attachment := &cloudv1alpha1.VPCAttachment{} + attachment.Namespace = egressTestNamespace + attachment.Name = "web-eth0" + attachment.Spec.VPC = cloudv1alpha1.VPCRef{Name: "default-us-central-1"} + attachment.Spec.Interface.Name = "eth0" + attachment.Status.Egress = &cloudv1alpha1.VPCAttachmentEgressStatus{ + Internet: &cloudv1alpha1.VPCAttachmentInternetEgressStatus{ + SourceAddresses: []cloudv1alpha1.InternetEgressSourceAddress{{ + Family: cloudv1alpha1.InternetEgressAddressFamilyIPv6, + Address: "2001:db8:f00d::100", + Stability: cloudv1alpha1.InternetEgressAddressStabilityNone, + }}, + }, + } + + scheme := runtime.NewScheme() + if err := cloudv1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("build the cloud scheme: %v", err) + } + fakeClient := fake.NewClientBuilder().WithScheme(scheme). + WithStatusSubresource(&cloudv1alpha1.VPCAttachment{}). + WithObjects(attachment).Build() + r := &NetworkInterfaceReconciler{Client: fakeClient, Scheme: scheme, APIReader: fakeClient} + + vpc := &cloudv1alpha1.VPC{} + vpc.Status.VPC = "0000000jU" + nad := &nadv1.NetworkAttachmentDefinition{} + nad.Name = attachment.Name + nad.Labels = map[string]string{LabelVPCAttachment: "01a"} + + if err := r.publishAttachmentStatus(t.Context(), attachment, vpc, nad, nil); err != nil { + t.Fatalf("publishAttachmentStatus: %v", err) + } + + stored := &cloudv1alpha1.VPCAttachment{} + if err := fakeClient.Get(t.Context(), client.ObjectKeyFromObject(attachment), stored); err != nil { + t.Fatalf("read the attachment back: %v", err) + } + if stored.Status.Egress != nil { + t.Errorf("egress still reported after it was withdrawn: %v", stored.Status.Egress) + } + // The other field set this reconciler owns still has to land. + if stored.Status.VPC != "0000000jU" || stored.Status.VPCAttachment != "01a" { + t.Errorf("identifiers: got %q/%q", stored.Status.VPC, stored.Status.VPCAttachment) + } +} + +// The BGPAdvertisement reconciler writes a disjoint field set on this same +// status, and both writers do a whole-object update. Neither may drop the +// other's fields, which is the property that makes two writers safe without +// server-side apply. +func TestPublishAttachmentStatusKeepsTheOtherWritersFields(t *testing.T) { + attachment := &cloudv1alpha1.VPCAttachment{} + attachment.Namespace = egressTestNamespace + attachment.Name = "web-eth0" + attachment.Spec.VPC = cloudv1alpha1.VPCRef{Name: "default-us-central-1"} + attachment.Spec.Interface.Name = "eth0" + attachment.Status.Node = "node-1" + attachment.Status.HostInterface = "G0000000jU01aH" + attachment.Status.PodSubnet = "fd00:10:ff01:0:1::/80" + + scheme := runtime.NewScheme() + if err := cloudv1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("build the cloud scheme: %v", err) + } + fakeClient := fake.NewClientBuilder().WithScheme(scheme). + WithStatusSubresource(&cloudv1alpha1.VPCAttachment{}). + WithObjects(attachment).Build() + r := &NetworkInterfaceReconciler{Client: fakeClient, Scheme: scheme, APIReader: fakeClient} + + vpc := &cloudv1alpha1.VPC{} + vpc.Status.VPC = "0000000jU" + nad := &nadv1.NetworkAttachmentDefinition{} + nad.Name = attachment.Name + nad.Labels = map[string]string{LabelVPCAttachment: "01a"} + egress := &internetEgress{ + shardSIDs: []string{"2001:db8:ff01::"}, + sourceAddress: &cloudv1alpha1.InternetEgressSourceAddress{ + Family: cloudv1alpha1.InternetEgressAddressFamilyIPv6, + Address: "2001:db8:f00d::100", + Stability: cloudv1alpha1.InternetEgressAddressStabilityNone, + }, + } + + if err := r.publishAttachmentStatus(t.Context(), attachment, vpc, nad, egress); err != nil { + t.Fatalf("publishAttachmentStatus: %v", err) + } + + stored := &cloudv1alpha1.VPCAttachment{} + if err := fakeClient.Get(t.Context(), client.ObjectKeyFromObject(attachment), stored); err != nil { + t.Fatalf("read the attachment back: %v", err) + } + if stored.Status.Node != "node-1" || stored.Status.HostInterface != "G0000000jU01aH" || + stored.Status.PodSubnet != "fd00:10:ff01:0:1::/80" { + t.Errorf("the data plane's field set was dropped: %+v", stored.Status) + } + if stored.Status.Egress == nil || + stored.Status.Egress.Internet.SourceAddresses[0].Address != "2001:db8:f00d::100" { + t.Errorf("egress address: got %v", stored.Status.Egress) + } +} From 9a206e7944a93a91ca16e294ea22b9f8d47266d7 Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Thu, 17 Sep 2026 20:55:22 -0500 Subject: [PATCH 3/3] 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=