diff --git a/.crd-ref-docs-gateway.yaml b/.crd-ref-docs-gateway.yaml index daa762f..25f5625 100644 --- a/.crd-ref-docs-gateway.yaml +++ b/.crd-ref-docs-gateway.yaml @@ -6,7 +6,6 @@ processor: - "SAFI" - "AddressFamily" - "SRv6Function" - - "RouterRole" - "OriginType" - "RouterRef" - "RouterSelector" diff --git a/api/v1alpha1/router_types.go b/api/v1alpha1/router_types.go index be9e4cf..3a58562 100644 --- a/api/v1alpha1/router_types.go +++ b/api/v1alpha1/router_types.go @@ -22,7 +22,6 @@ const ( // +kubebuilder:subresource:status // +kubebuilder:resource:scope=Namespaced,shortName=bgpr // +kubebuilder:printcolumn:name="TARGET",type="string",JSONPath=".spec.targetRef.name" -// +kubebuilder:printcolumn:name="ROLES",type="string",JSONPath=".spec.roles" // +kubebuilder:printcolumn:name="ASN",type="integer",JSONPath=".spec.localASN" // +kubebuilder:printcolumn:name="ROUTER-ID",type="string",JSONPath=".spec.routerID" // +kubebuilder:printcolumn:name="PHASE",type="string",JSONPath=".status.phase" @@ -41,11 +40,6 @@ type BGPRouterSpec struct { // +kubebuilder:validation:Required TargetRef TargetRef `json:"targetRef"` - // Roles describes the functional roles this router participates in. - // At least one role is required. - // +kubebuilder:validation:MinItems=1 - Roles []RouterRole `json:"roles,omitempty"` - // LocalASN is the BGP Autonomous System Number for this router. // Must be a valid 2-byte or 4-byte ASN per RFC 6793. // +kubebuilder:validation:Required @@ -96,10 +90,6 @@ type BGPRouterStatus struct { // +optional ObservedGeneration int64 `json:"observedGeneration,omitempty"` - // Roles reflects the active roles as observed by the implementation. - // +optional - Roles []RouterRole `json:"roles,omitempty"` - // Peers summarizes peer session counts. // +optional Peers BGPRouterPeerSummary `json:"peers,omitempty"` diff --git a/api/v1alpha1/router_types_test.go b/api/v1alpha1/router_types_test.go index 16f0f9b..f639fd8 100644 --- a/api/v1alpha1/router_types_test.go +++ b/api/v1alpha1/router_types_test.go @@ -7,7 +7,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func newTestRouter(roles ...RouterRole) *BGPRouter { +func newTestRouter() *BGPRouter { return &BGPRouter{ TypeMeta: metav1.TypeMeta{ APIVersion: "network.datumapis.com/v1alpha1", @@ -18,117 +18,10 @@ func newTestRouter(roles ...RouterRole) *BGPRouter { TargetRef: TargetRef{Kind: "Node", Name: "node-1"}, LocalASN: 65000, RouterID: "10.0.0.1", - Roles: roles, }, } } -// TestBGPRouterDeepCopyRoles verifies that mutating Roles on the copy does not -// affect the original. -func TestBGPRouterDeepCopyRoles(t *testing.T) { - orig := newTestRouter(RouterRoleTransit, RouterRoleFabric) - dup := orig.DeepCopy() - - dup.Spec.Roles[0] = RouterRoleTenant - - if orig.Spec.Roles[0] != RouterRoleTransit { - t.Errorf("Roles[0] mutated: got %q, want %q", orig.Spec.Roles[0], RouterRoleTransit) - } - if orig.Spec.Roles[1] != RouterRoleFabric { - t.Errorf("Roles[1] mutated: got %q, want %q", orig.Spec.Roles[1], RouterRoleFabric) - } -} - -// TestBGPRouterDeepCopyNilRoles verifies that a router with no roles deep-copies -// without allocating a non-nil slice. -func TestBGPRouterDeepCopyNilRoles(t *testing.T) { - orig := newTestRouter() - dup := orig.DeepCopy() - - if dup.Spec.Roles != nil { - t.Errorf("expected nil Roles on copy, got %v", dup.Spec.Roles) - } -} - -// TestBGPRouterJSONRoundTripRoles verifies that Roles survives JSON marshal/unmarshal. -func TestBGPRouterJSONRoundTripRoles(t *testing.T) { - cases := []struct { - name string - roles []RouterRole - }{ - {"no roles", nil}, - {"single transit", []RouterRole{RouterRoleTransit}}, - {"all roles", []RouterRole{RouterRoleTransit, RouterRoleFabric, RouterRoleTenant}}, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - orig := newTestRouter(tc.roles...) - - data, err := json.Marshal(orig) - if err != nil { - t.Fatalf("Marshal: %v", err) - } - - var got BGPRouter - if err := json.Unmarshal(data, &got); err != nil { - t.Fatalf("Unmarshal: %v", err) - } - - if len(got.Spec.Roles) != len(tc.roles) { - t.Fatalf("Roles len: got %d, want %d", len(got.Spec.Roles), len(tc.roles)) - } - for i, r := range tc.roles { - if got.Spec.Roles[i] != r { - t.Errorf("Roles[%d]: got %q, want %q", i, got.Spec.Roles[i], r) - } - } - }) - } -} - -// TestBGPRouterJSONRolesFieldName verifies the JSON key is "roles". -func TestBGPRouterJSONRolesFieldName(t *testing.T) { - orig := newTestRouter(RouterRoleFabric) - data, err := json.Marshal(orig.Spec) - if err != nil { - t.Fatalf("Marshal: %v", err) - } - - var m map[string]any - if err := json.Unmarshal(data, &m); err != nil { - t.Fatalf("Unmarshal: %v", err) - } - - raw, ok := m["roles"] - if !ok { - t.Fatal("expected JSON key \"roles\" not found") - } - roles, ok := raw.([]any) - if !ok || len(roles) != 1 || roles[0] != "fabric" { - t.Errorf("unexpected roles value: %v", raw) - } -} - -// TestBGPRouterRolesOmitEmpty verifies that a router with no roles omits the -// "roles" key from JSON output. -func TestBGPRouterRolesOmitEmpty(t *testing.T) { - orig := newTestRouter() - data, err := json.Marshal(orig.Spec) - if err != nil { - t.Fatalf("Marshal: %v", err) - } - - var m map[string]any - if err := json.Unmarshal(data, &m); err != nil { - t.Fatalf("Unmarshal: %v", err) - } - - if _, ok := m["roles"]; ok { - t.Error("expected \"roles\" key to be absent when empty") - } -} - // TestBGPRouterLargeLocalASN verifies that 4-byte ASNs (values above signed int32 max) // survive JSON round-trip correctly. func TestBGPRouterLargeLocalASN(t *testing.T) { @@ -145,7 +38,6 @@ func TestBGPRouterLargeLocalASN(t *testing.T) { TargetRef: TargetRef{Kind: "Node", Name: "node-1"}, LocalASN: maxASN, RouterID: "10.0.0.1", - Roles: []RouterRole{RouterRoleTransit}, AddressFamilies: []AddressFamily{ {AFI: AFIIPv4, SAFI: SAFIUnicast}, }, @@ -183,7 +75,6 @@ func TestBGPRouterLocalASNAboveSignedInt32Max(t *testing.T) { TargetRef: TargetRef{Kind: "Node", Name: "node-1"}, LocalASN: aboveSignedMax, RouterID: "10.0.0.1", - Roles: []RouterRole{RouterRoleTransit}, AddressFamilies: []AddressFamily{ {AFI: AFIIPv4, SAFI: SAFIUnicast}, }, @@ -221,7 +112,7 @@ func TestBGPRouterDeepCopyLocalASN(t *testing.T) { // TestBGPRouterJSONRoundTripSRv6Locator verifies that SRv6Locator survives // JSON marshal/unmarshal. func TestBGPRouterJSONRoundTripSRv6Locator(t *testing.T) { - orig := newTestRouter(RouterRoleFabric) + orig := newTestRouter() orig.Spec.SRv6Locator = "2001:db8:ff01::/48" data, err := json.Marshal(orig) @@ -241,7 +132,7 @@ func TestBGPRouterJSONRoundTripSRv6Locator(t *testing.T) { // TestBGPRouterSRv6LocatorFieldName verifies the JSON key is "srv6Locator". func TestBGPRouterSRv6LocatorFieldName(t *testing.T) { - orig := newTestRouter(RouterRoleFabric) + orig := newTestRouter() orig.Spec.SRv6Locator = "2001:db8:ff01::/48" data, err := json.Marshal(orig.Spec) @@ -262,7 +153,7 @@ func TestBGPRouterSRv6LocatorFieldName(t *testing.T) { // TestBGPRouterSRv6LocatorOmitEmpty verifies that a router without a // SRv6Locator omits the "srv6Locator" key from JSON output. func TestBGPRouterSRv6LocatorOmitEmpty(t *testing.T) { - orig := newTestRouter(RouterRoleFabric) + orig := newTestRouter() data, err := json.Marshal(orig.Spec) if err != nil { @@ -282,7 +173,7 @@ func TestBGPRouterSRv6LocatorOmitEmpty(t *testing.T) { // TestBGPRouterJSONRoundTripNodeID verifies that NodeID survives JSON // marshal/unmarshal. func TestBGPRouterJSONRoundTripNodeID(t *testing.T) { - orig := newTestRouter(RouterRoleFabric) + orig := newTestRouter() orig.Spec.NodeID = 42 data, err := json.Marshal(orig) @@ -302,7 +193,7 @@ func TestBGPRouterJSONRoundTripNodeID(t *testing.T) { // TestBGPRouterNodeIDFieldName verifies the JSON key is "nodeID". func TestBGPRouterNodeIDFieldName(t *testing.T) { - orig := newTestRouter(RouterRoleFabric) + orig := newTestRouter() orig.Spec.NodeID = 42 data, err := json.Marshal(orig.Spec) @@ -323,7 +214,7 @@ func TestBGPRouterNodeIDFieldName(t *testing.T) { // TestBGPRouterNodeIDOmitEmpty verifies that a router without a NodeID omits // the "nodeID" key from JSON output. func TestBGPRouterNodeIDOmitEmpty(t *testing.T) { - orig := newTestRouter(RouterRoleFabric) + orig := newTestRouter() data, err := json.Marshal(orig.Spec) if err != nil { @@ -343,7 +234,7 @@ func TestBGPRouterNodeIDOmitEmpty(t *testing.T) { // TestBGPRouterJSONRoundTripListenPort verifies that ListenPort survives // JSON marshal/unmarshal. func TestBGPRouterJSONRoundTripListenPort(t *testing.T) { - orig := newTestRouter(RouterRoleFabric) + orig := newTestRouter() port := int32(179) orig.Spec.ListenPort = &port @@ -366,7 +257,7 @@ func TestBGPRouterJSONRoundTripListenPort(t *testing.T) { // TestBGPRouterListenPortFieldName verifies the JSON key is "listenPort". func TestBGPRouterListenPortFieldName(t *testing.T) { - orig := newTestRouter(RouterRoleFabric) + orig := newTestRouter() port := int32(179) orig.Spec.ListenPort = &port @@ -388,7 +279,7 @@ func TestBGPRouterListenPortFieldName(t *testing.T) { // TestBGPRouterListenPortOmitEmpty verifies that a router without a // ListenPort omits the "listenPort" key from JSON output. func TestBGPRouterListenPortOmitEmpty(t *testing.T) { - orig := newTestRouter(RouterRoleFabric) + orig := newTestRouter() data, err := json.Marshal(orig.Spec) if err != nil { @@ -418,7 +309,7 @@ func TestBGPRouterListenPortBoundaryValues(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - orig := newTestRouter(RouterRoleFabric) + orig := newTestRouter() orig.Spec.ListenPort = &tc.port data, err := json.Marshal(orig.Spec) diff --git a/api/v1alpha1/shared_types.go b/api/v1alpha1/shared_types.go index 1d3c1b1..3312fbe 100644 --- a/api/v1alpha1/shared_types.go +++ b/api/v1alpha1/shared_types.go @@ -55,17 +55,6 @@ const ( SRv6FunctionEndDT46 SRv6Function = "End.DT46" ) -// RouterRole defines the functional role of a BGPRouter within the network. -// -// +kubebuilder:validation:Enum=fabric;tenant;transit -type RouterRole string - -const ( - RouterRoleFabric RouterRole = "fabric" - RouterRoleTenant RouterRole = "tenant" - RouterRoleTransit RouterRole = "transit" -) - // TargetRef identifies the execution target for a BGPRouter. // Supported values for kind: Node. type TargetRef struct { diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 2063b58..3991830 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -1072,11 +1072,6 @@ func (in *BGPRouterPeerSummary) DeepCopy() *BGPRouterPeerSummary { func (in *BGPRouterSpec) DeepCopyInto(out *BGPRouterSpec) { *out = *in out.TargetRef = in.TargetRef - if in.Roles != nil { - in, out := &in.Roles, &out.Roles - *out = make([]RouterRole, len(*in)) - copy(*out, *in) - } if in.AddressFamilies != nil { in, out := &in.AddressFamilies, &out.AddressFamilies *out = make([]AddressFamily, len(*in)) @@ -1102,11 +1097,6 @@ func (in *BGPRouterSpec) DeepCopy() *BGPRouterSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *BGPRouterStatus) DeepCopyInto(out *BGPRouterStatus) { *out = *in - if in.Roles != nil { - in, out := &in.Roles, &out.Roles - *out = make([]RouterRole, len(*in)) - copy(*out, *in) - } out.Peers = in.Peers if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions diff --git a/config/crd/network.datumapis.com_bgprouters.yaml b/config/crd/network.datumapis.com_bgprouters.yaml index 8a56841..dcad7bd 100644 --- a/config/crd/network.datumapis.com_bgprouters.yaml +++ b/config/crd/network.datumapis.com_bgprouters.yaml @@ -20,9 +20,6 @@ spec: - jsonPath: .spec.targetRef.name name: TARGET type: string - - jsonPath: .spec.roles - name: ROLES - type: string - jsonPath: .spec.localASN name: ASN type: integer @@ -122,20 +119,6 @@ spec: maximum: 254 minimum: 1 type: integer - roles: - description: |- - Roles describes the functional roles this router participates in. - At least one role is required. - items: - description: RouterRole defines the functional role of a BGPRouter - within the network. - enum: - - fabric - - tenant - - transit - type: string - minItems: 1 - type: array routerID: description: |- RouterID is a unique 32-bit identifier expressed in IPv4 dotted-decimal notation. @@ -262,17 +245,6 @@ spec: phase: description: Phase is the high-level lifecycle state of this router. type: string - roles: - description: Roles reflects the active roles as observed by the implementation. - items: - description: RouterRole defines the functional role of a BGPRouter - within the network. - enum: - - fabric - - tenant - - transit - type: string - type: array type: object type: object served: true diff --git a/config/samples/network.datumapis.com_v1alpha1_bgprouter_node.yaml b/config/samples/network.datumapis.com_v1alpha1_bgprouter_node.yaml index 51521f9..4a946b8 100644 --- a/config/samples/network.datumapis.com_v1alpha1_bgprouter_node.yaml +++ b/config/samples/network.datumapis.com_v1alpha1_bgprouter_node.yaml @@ -7,7 +7,5 @@ spec: targetRef: kind: Node name: node-a - roles: - - fabric localASN: 65001 routerID: 10.0.0.1 diff --git a/docs/agents/ARCHITECTURE.md b/docs/agents/ARCHITECTURE.md index f8e5fe6..5a120eb 100644 --- a/docs/agents/ARCHITECTURE.md +++ b/docs/agents/ARCHITECTURE.md @@ -83,7 +83,7 @@ The design follows standard Kubernetes API conventions: resources have a `Spec` | `advertisement_types.go` | `BGPAdvertisement`, `BGPAdvertisementSpec`, `BGPAdvertisementStatus` | | `policy_types.go` | `BGPPolicy`, `BGPPolicySpec`, `BGPPolicyTerm`, `BGPPolicyMatch`, `BGPPolicySetActions`, `CommunitySet` | | `vrf_types.go` | `BGPVRFInstance`, `BGPVRFInstanceSpec`, `BGPVRFInstanceStatus`, `RouteTarget` | -| `shared_types.go` | `RouterTarget`, `RouterRef`, `RouterSelector`, `TargetRef`, `AddressFamily`, `AFI`, `SAFI`, `RouterRole`, `LocalSecretRef`, `RouterStatus`, `ResolvedRouterConfig` | +| `shared_types.go` | `RouterTarget`, `RouterRef`, `RouterSelector`, `TargetRef`, `AddressFamily`, `AFI`, `SAFI`, `LocalSecretRef`, `RouterStatus`, `ResolvedRouterConfig` | | `zz_generated.deepcopy.go` | Generated `DeepCopy*` methods — do not edit | **External dependencies:** @@ -123,7 +123,6 @@ One per routing plane per node. Declares the local ASN, router ID, address famil Key spec fields: - `targetRef.kind/name` — identifies the Kubernetes Node this router runs on -- `roles` — one or more of `fabric`, `tenant`, `transit` (at least one required) - `localASN int64` — 2-byte or 4-byte BGP ASN (1–4294967295) - `routerID string` — IPv4 dotted-decimal notation (logical identifier in IPv6-only underlays) - `addressFamilies []AddressFamily` — AFI/SAFI pairs this router activates diff --git a/docs/agents/CONVENTIONS.md b/docs/agents/CONVENTIONS.md index 2d9f7f9..d103a46 100644 --- a/docs/agents/CONVENTIONS.md +++ b/docs/agents/CONVENTIONS.md @@ -179,7 +179,7 @@ refactor/bgp-api-v3 | Status | `{Resource}Status` | `BGPRouterStatus` | | List | `{Resource}List` | `BGPRouterList` | | Enum type | `{Resource}{Field}` or descriptive noun | `BGPRouterPhase`, `BGPPeerState` | -| Enum constant | `{TypeName}{Value}` | `BGPRouterPhasePending`, `RouterRoleFabric` | +| Enum constant | `{TypeName}{Value}` | `BGPRouterPhasePending`, `AFIIPv4` | | Condition type constant | `ConditionType{Name}` (`string`) | `ConditionTypeReady`, `ConditionTypeAccepted` | | Idle reason constant | `IdleReason{Name}` (`string`) | `IdleReasonBackOff` | | Shared struct | Descriptive PascalCase | `RouterTarget`, `AddressFamily`, `LocalSecretRef` | diff --git a/docs/api/bgp.md b/docs/api/bgp.md index 1210e29..6be5722 100644 --- a/docs/api/bgp.md +++ b/docs/api/bgp.md @@ -777,7 +777,6 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | | `targetRef` _[TargetRef](#targetref)_ | TargetRef identifies the Node this router executes on. | | Required: \{\}
| -| `roles` _[RouterRole](#routerrole) array_ | Roles describes the functional roles this router participates in.
At least one role is required. | | Enum: [fabric tenant transit]
MinItems: 1
| | `localASN` _integer_ | LocalASN is the BGP Autonomous System Number for this router.
Must be a valid 2-byte or 4-byte ASN per RFC 6793. | | Minimum: 1
Required: \{\}
| | `routerID` _string_ | RouterID is a unique 32-bit identifier expressed in IPv4 dotted-decimal notation.
In an IPv6-only underlay this is a logical identifier only. | | Format: ipv4
Required: \{\}
| | `addressFamilies` _[AddressFamily](#addressfamily) array_ | AddressFamilies defines the address families this router activates. | | MinItems: 1
| @@ -801,7 +800,6 @@ _Appears in:_ | --- | --- | --- | --- | | `phase` _[BGPRouterPhase](#bgprouterphase)_ | Phase is the high-level lifecycle state of this router. | | | | `observedGeneration` _integer_ | ObservedGeneration is the .metadata.generation this status was computed from. | | | -| `roles` _[RouterRole](#routerrole) array_ | Roles reflects the active roles as observed by the implementation. | | Enum: [fabric tenant transit]
| | `peers` _[BGPRouterPeerSummary](#bgprouterpeersummary)_ | Peers summarizes peer session counts. | | | | `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v/#condition-v1-meta) array_ | Conditions contains the standard conditions for this resource. | | | @@ -1184,26 +1182,6 @@ _Appears in:_ | `name` _string_ | Name is the name of the BGPRouter. | | MinLength: 1
| -#### RouterRole - -_Underlying type:_ _string_ - -RouterRole defines the functional role of a BGPRouter within the network. - -_Validation:_ -- Enum: [fabric tenant transit] - -_Appears in:_ -- [BGPRouterSpec](#bgprouterspec) -- [BGPRouterStatus](#bgprouterstatus) - -| Field | Description | -| --- | --- | -| `fabric` | | -| `tenant` | | -| `transit` | | - - #### RouterSelector diff --git a/docs/getting-started.md b/docs/getting-started.md index 3f9c98d..23ebf4e 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -63,8 +63,6 @@ spec: targetRef: kind: Node name: node-1 - roles: - - fabric localASN: 65000 routerID: "10.0.0.1" addressFamilies: @@ -102,8 +100,6 @@ spec: targetRef: kind: Node name: node-1 - roles: - - fabric localASN: 65000 routerID: "10.0.0.1" addressFamilies: diff --git a/test/e2e/fixtures/bgprouters.yaml b/test/e2e/fixtures/bgprouters.yaml index 948956a..c2f3c8c 100644 --- a/test/e2e/fixtures/bgprouters.yaml +++ b/test/e2e/fixtures/bgprouters.yaml @@ -8,8 +8,6 @@ spec: targetRef: kind: Node name: bgp-e2e-worker - roles: - - fabric localASN: 65000 routerID: "10.0.0.1" addressFamilies: @@ -25,8 +23,6 @@ spec: targetRef: kind: Node name: bgp-e2e-worker2 - roles: - - fabric localASN: 65000 routerID: "10.0.0.2" addressFamilies: @@ -42,8 +38,6 @@ spec: targetRef: kind: Node name: bgp-e2e-worker3 - roles: - - fabric localASN: 65000 routerID: "10.0.0.3" addressFamilies: