From b2deee16a43a49162b19a9e38010f2f8b35d6a47 Mon Sep 17 00:00:00 2001 From: Peter Sprygada Date: Sun, 19 Jul 2026 16:23:37 -0400 Subject: [PATCH 1/2] feat(api): move VPC/VPCAttachment status to condition-based model Replace the Ready/Identifier bool+string status fields with standard ObservedGeneration + Conditions, add VPCFinalizer, mark VPCSpec immutable after creation, and expand VPCAttachmentStatus with the fields the datapath controller needs to reconcile a single attachment (node, containerID, host/vrf/guest interface names, podSubnet). Co-Authored-By: Claude Sonnet 5 --- api/v1alpha1/vpc_types.go | 22 +++- api/v1alpha1/vpcattachment_types.go | 55 +++++++- api/v1alpha1/zz_generated.deepcopy.go | 19 ++- .../cloud.datumapis.com_vpcattachments.yaml | 121 +++++++++++++++++- config/crd/cloud.datumapis.com_vpcs.yaml | 87 +++++++++++-- docs/api/vpc.md | 18 ++- 6 files changed, 289 insertions(+), 33 deletions(-) diff --git a/api/v1alpha1/vpc_types.go b/api/v1alpha1/vpc_types.go index 72e4b50..b1e3829 100644 --- a/api/v1alpha1/vpc_types.go +++ b/api/v1alpha1/vpc_types.go @@ -21,6 +21,9 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +// VPCFinalizer is the finalizer applied to VPCs to ensure graceful cleanup. +const VPCFinalizer = "cloud.datumapis.com/finalizer" + // Network is an IPv4 or IPv6 CIDR block (e.g., "10.0.0.0/24"). // +kubebuilder:validation:MaxLength=64 type Network string @@ -28,6 +31,7 @@ type Network string // VPCSpec defines the desired state of a VPC. It specifies the CIDR address space. // // +kubebuilder:validation:XValidation:rule="self.networks.all(n, isCIDR(n))",message="each network must be a valid IPv4 or IPv6 CIDR" +// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="spec is immutable" type VPCSpec struct { // CIDR blocks that form the VPC address space. // +kubebuilder:validation:MinItems=1 @@ -37,19 +41,25 @@ type VPCSpec struct { // VPCStatus defines the observed state of a VPC, populated by the controller. type VPCStatus struct { - // True when the VPC is provisioned and ready for attachments. - // +required - // +default:value=false - Ready bool `json:"ready,omitempty"` + // +optional + ObservedGeneration int64 `json:"observedGeneration,omitempty"` - // Opaque controller-assigned identifier for this VPC. + // +listType=map + // +listMapKey=type // +optional - Identifier string `json:"identifier,omitempty"` + Conditions []metav1.Condition `json:"conditions,omitempty"` + + // Base62-encoded VPC identifier. + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=16 + VPC string `json:"vpc"` } // +kubebuilder:object:root=true // +kubebuilder:subresource:status // +kubebuilder:storageversion +// +kubebuilder:printcolumn:name="VPC",type="string",JSONPath=".status.vpc",description="Base62 VPC identifier" +// +kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type==\"Ready\")].status",description="Ready status" // VPC represents a virtual private cloud — an isolated Layer 2 domain backed // by one or more CIDR blocks. diff --git a/api/v1alpha1/vpcattachment_types.go b/api/v1alpha1/vpcattachment_types.go index f4e354f..0c417f6 100644 --- a/api/v1alpha1/vpcattachment_types.go +++ b/api/v1alpha1/vpcattachment_types.go @@ -24,6 +24,8 @@ import ( const VPCAttachmentAnnotation = "k8s.v1alpha1.cloud.datumapis.com/vpc-attachment" // VPCAttachmentSpec defines the desired state of VPCAttachment +// +// +kubebuilder:validation:XValidation:rule="has(self.vpc) && self.vpc.name != ”",message="vpc reference is required" type VPCAttachmentSpec struct { // VPC this attachment belongs to. // +required @@ -63,14 +65,55 @@ type VPCAttachmentInterface struct { // VPCAttachmentStatus defines the observed state of VPCAttachment. type VPCAttachmentStatus struct { - // Indicates whether the VPCAttachment is ready for use - // +required - // +default:value=false - Ready bool `json:"ready,omitempty"` + // +optional + ObservedGeneration int64 `json:"observedGeneration,omitempty"` - // A unique identifier assigned to this VPCAttachment + // +listType=map + // +listMapKey=type // +optional - Identifier string `json:"identifier,omitempty"` + Conditions []metav1.Condition `json:"conditions,omitempty"` + + // Base62-encoded VPC identifier. + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=16 + VPC string `json:"vpc"` + + // Base62-encoded VPCAttachment identifier. + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=16 + VPCAttachment string `json:"vpcAttachment"` + + // Kubernetes node name where the attachment lives. + // +kubebuilder:validation:MinLength=1 + Node string `json:"node"` + + // Full container ID (46 hex characters). + // +kubebuilder:validation:MinLength=46 + // +kubebuilder:validation:MaxLength=46 + ContainerID string `json:"containerID"` + + // Pod name. + // +kubebuilder:validation:MinLength=1 + PodName string `json:"podName"` + + // Host-side veth device name (e.g., "G000000010010H"). + // +kubebuilder:validation:MinLength=1 + HostInterface string `json:"hostInterface"` + + // VRF device name (e.g., "G000000010010V"). + // +kubebuilder:validation:MinLength=1 + VRFInterface string `json:"vrfInterface"` + + // Guest-side veth device name (e.g., "G000000010010G"). + // +kubebuilder:validation:MinLength=1 + // +optional + GuestInterface string `json:"guestInterface,omitempty"` + + // Allocated /80 subnet in CIDR notation (e.g., "fd00:10:ff01:0:1::/80"). + // +kubebuilder:validation:MinLength=1 + // + // +kubebuilder:validation:XValidation:rule="isCIDR(self)",message="podSubnet must be a valid IPv6 CIDR" + PodSubnet string `json:"podSubnet"` } // +kubebuilder:object:root=true diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 2299835..0702675 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -22,6 +22,7 @@ along with this program. If not, see . package v1alpha1 import ( + "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -31,7 +32,7 @@ func (in *VPC) DeepCopyInto(out *VPC) { out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.Spec.DeepCopyInto(&out.Spec) - out.Status = in.Status + in.Status.DeepCopyInto(&out.Status) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VPC. @@ -58,7 +59,7 @@ func (in *VPCAttachment) DeepCopyInto(out *VPCAttachment) { out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.Spec.DeepCopyInto(&out.Spec) - out.Status = in.Status + in.Status.DeepCopyInto(&out.Status) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VPCAttachment. @@ -151,6 +152,13 @@ func (in *VPCAttachmentSpec) DeepCopy() *VPCAttachmentSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *VPCAttachmentStatus) DeepCopyInto(out *VPCAttachmentStatus) { *out = *in + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]v1.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VPCAttachmentStatus. @@ -233,6 +241,13 @@ func (in *VPCSpec) DeepCopy() *VPCSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *VPCStatus) DeepCopyInto(out *VPCStatus) { *out = *in + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]v1.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VPCStatus. diff --git a/config/crd/cloud.datumapis.com_vpcattachments.yaml b/config/crd/cloud.datumapis.com_vpcattachments.yaml index 2a6cb88..d451673 100644 --- a/config/crd/cloud.datumapis.com_vpcattachments.yaml +++ b/config/crd/cloud.datumapis.com_vpcattachments.yaml @@ -78,18 +78,125 @@ spec: - interface - vpc type: object + x-kubernetes-validations: + - message: vpc reference is required + rule: has(self.vpc) && self.vpc.name != '' status: description: status defines the observed state of VPCAttachment properties: - identifier: - description: A unique identifier assigned to this VPCAttachment + conditions: + items: + description: Condition contains details for one aspect of the current + state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map + containerID: + description: Full container ID (46 hex characters). + maxLength: 46 + minLength: 46 + type: string + guestInterface: + description: Guest-side veth device name (e.g., "G000000010010G"). + minLength: 1 + type: string + hostInterface: + description: Host-side veth device name (e.g., "G000000010010H"). + minLength: 1 + type: string + node: + description: Kubernetes node name where the attachment lives. + minLength: 1 + type: string + observedGeneration: + format: int64 + type: integer + podName: + description: Pod name. + minLength: 1 + type: string + podSubnet: + description: Allocated /80 subnet in CIDR notation (e.g., "fd00:10:ff01:0:1::/80"). + minLength: 1 + type: string + x-kubernetes-validations: + - message: podSubnet must be a valid IPv6 CIDR + rule: isCIDR(self) + vpc: + description: Base62-encoded VPC identifier. + maxLength: 16 + minLength: 1 + type: string + vpcAttachment: + description: Base62-encoded VPCAttachment identifier. + maxLength: 16 + minLength: 1 + type: string + vrfInterface: + description: VRF device name (e.g., "G000000010010V"). + minLength: 1 type: string - ready: - default: false - description: Indicates whether the VPCAttachment is ready for use - type: boolean required: - - ready + - containerID + - hostInterface + - node + - podName + - podSubnet + - vpc + - vpcAttachment + - vrfInterface type: object required: - spec diff --git a/config/crd/cloud.datumapis.com_vpcs.yaml b/config/crd/cloud.datumapis.com_vpcs.yaml index e959da6..18283ad 100644 --- a/config/crd/cloud.datumapis.com_vpcs.yaml +++ b/config/crd/cloud.datumapis.com_vpcs.yaml @@ -14,7 +14,16 @@ spec: singular: vpc scope: Namespaced versions: - - name: v1alpha1 + - additionalPrinterColumns: + - description: Base62 VPC identifier + jsonPath: .status.vpc + name: VPC + type: string + - description: Ready status + jsonPath: .status.conditions[?(@.type=="Ready")].status + name: Ready + type: string + name: v1alpha1 schema: openAPIV3Schema: description: |- @@ -56,18 +65,80 @@ spec: x-kubernetes-validations: - message: each network must be a valid IPv4 or IPv6 CIDR rule: self.networks.all(n, isCIDR(n)) + - message: spec is immutable + rule: self == oldSelf status: description: Controller-observed state. properties: - identifier: - description: Opaque controller-assigned identifier for this VPC. + conditions: + items: + description: Condition contains details for one aspect of the current + state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map + observedGeneration: + format: int64 + type: integer + vpc: + description: Base62-encoded VPC identifier. + maxLength: 16 + minLength: 1 type: string - ready: - default: false - description: True when the VPC is provisioned and ready for attachments. - type: boolean required: - - ready + - vpc type: object required: - spec diff --git a/docs/api/vpc.md b/docs/api/vpc.md index 1243091..d0588b6 100644 --- a/docs/api/vpc.md +++ b/docs/api/vpc.md @@ -132,8 +132,17 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `ready` _boolean_ | Indicates whether the VPCAttachment is ready for use | | | -| `identifier` _string_ | A unique identifier assigned to this VPCAttachment | | | +| `observedGeneration` _integer_ | | | | +| `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v/#condition-v1-meta) array_ | | | | +| `vpc` _string_ | Base62-encoded VPC identifier. | | MaxLength: 16
MinLength: 1
| +| `vpcAttachment` _string_ | Base62-encoded VPCAttachment identifier. | | MaxLength: 16
MinLength: 1
| +| `node` _string_ | Kubernetes node name where the attachment lives. | | MinLength: 1
| +| `containerID` _string_ | Full container ID (46 hex characters). | | MaxLength: 46
MinLength: 46
| +| `podName` _string_ | Pod name. | | MinLength: 1
| +| `hostInterface` _string_ | Host-side veth device name (e.g., "G000000010010H"). | | MinLength: 1
| +| `vrfInterface` _string_ | VRF device name (e.g., "G000000010010V"). | | MinLength: 1
| +| `guestInterface` _string_ | Guest-side veth device name (e.g., "G000000010010G"). | | MinLength: 1
| +| `podSubnet` _string_ | Allocated /80 subnet in CIDR notation (e.g., "fd00:10:ff01:0:1::/80"). | | MinLength: 1
| #### VPCRef @@ -181,7 +190,8 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `ready` _boolean_ | True when the VPC is provisioned and ready for attachments. | | | -| `identifier` _string_ | Opaque controller-assigned identifier for this VPC. | | | +| `observedGeneration` _integer_ | | | | +| `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v/#condition-v1-meta) array_ | | | | +| `vpc` _string_ | Base62-encoded VPC identifier. | | MaxLength: 16
MinLength: 1
| From 6cea83fb78a3cdb74da11a1fae5d416b120fe3f0 Mon Sep 17 00:00:00 2001 From: Peter Sprygada Date: Sun, 19 Jul 2026 17:02:15 -0400 Subject: [PATCH 2/2] fix(e2e): update vpc-crd-schema test for immutable VPCSpec This branch added an XValidation rule making VPCSpec immutable after creation (self == oldSelf), but the vpc-networks-update e2e step still patched an existing VPC's spec.networks and asserted the patch succeeded -- which now fails with "spec is immutable", the cause of the failing E2E Tests check on this PR. Replace the step with reject-vpc-spec-update, asserting the patch is rejected, matching the reject-* pattern used for the other schema constraints in this test. Co-Authored-By: Claude Sonnet 5 --- .../tests/vpc-crd-schema/chainsaw-test.yaml | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/test/e2e/tests/vpc-crd-schema/chainsaw-test.yaml b/test/e2e/tests/vpc-crd-schema/chainsaw-test.yaml index c6eda4c..ad13353 100644 --- a/test/e2e/tests/vpc-crd-schema/chainsaw-test.yaml +++ b/test/e2e/tests/vpc-crd-schema/chainsaw-test.yaml @@ -96,23 +96,21 @@ spec: echo "OK: missing networks field correctly rejected" echo "Server response: $OUTPUT" - - name: vpc-networks-update + - name: reject-vpc-spec-update try: - script: content: | + set +e + OUTPUT=$(kubectl patch vpcs.cloud.datumapis.com e2e-valid-vpc -n "$NAMESPACE" \ + --type=merge -p '{"spec":{"networks":["10.0.0.0/8","172.16.0.0/12","192.168.0.0/16"]}}' 2>&1) + EXIT=$? set -e - kubectl patch vpcs.cloud.datumapis.com e2e-valid-vpc -n "$NAMESPACE" \ - --type=merge -p '{"spec":{"networks":["10.0.0.0/8","172.16.0.0/12","192.168.0.0/16"]}}' - COUNT=$(kubectl get vpcs.cloud.datumapis.com e2e-valid-vpc -n "$NAMESPACE" \ - -o jsonpath='{range .spec.networks[*]}{.}{"\n"}{end}' | wc -l | tr -d ' ') - if [ "$COUNT" -ne 3 ]; then - echo "ERROR: expected 3 networks after update but got $COUNT" + if [ "$EXIT" -eq 0 ]; then + echo "ERROR: expected rejection of spec update on an existing VPC but patch succeeded" exit 1 fi - echo "OK: VPC networks updated to $COUNT entries" - # Restore original state for subsequent steps - kubectl patch vpcs.cloud.datumapis.com e2e-valid-vpc -n "$NAMESPACE" \ - --type=merge -p '{"spec":{"networks":["10.0.0.0/8","172.16.0.0/12"]}}' + echo "OK: spec update on an existing VPC correctly rejected (spec is immutable)" + echo "Server response: $OUTPUT" - name: create-valid-vpcattachment try: