From 11b6e7346d8547467db6eed5e0025aaf2686f6a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20K=C3=A4stner?= Date: Wed, 9 Sep 2026 11:41:44 +0200 Subject: [PATCH 1/4] Diff list entries individually on Patch to remove stale config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the provider uses Patch (gNMI update) on a struct containing list fields, the device merges entries into existing config but never removes entries that are no longer desired. Stale list entries survive across reconciliations. Fix this by removing list fields from parent structs and managing each list entry as a standalone DataElement. The provider now fetches current entries from the device, patches each desired entry individually, and deletes any stale entries not present in the desired set. Signed-off-by: Felix Kästner --- internal/provider/cisco/nxos/bgp.go | 28 +++- internal/provider/cisco/nxos/bgp_test.go | 66 ++++---- internal/provider/cisco/nxos/intf.go | 69 ++++++-- internal/provider/cisco/nxos/intf_test.go | 24 +-- internal/provider/cisco/nxos/lldp.go | 27 +++- internal/provider/cisco/nxos/lldp_test.go | 13 +- internal/provider/cisco/nxos/nve.go | 9 +- internal/provider/cisco/nxos/nve_test.go | 12 +- internal/provider/cisco/nxos/provider.go | 147 ++++++++++++++---- .../provider/cisco/nxos/testdata/bgp_dom.json | 13 +- .../cisco/nxos/testdata/bgp_dom_af.json | 22 +++ ...evpn.json => bgp_dom_af_advl2vpnevpn.json} | 5 +- ...n.txt => bgp_dom_af_advl2vpnevpn.json.txt} | 0 ...dom_advpip.json => bgp_dom_af_advpip.json} | 2 - ...ip.json.txt => bgp_dom_af_advpip.json.txt} | 0 .../{bgp_dom_exp.json => bgp_dom_af_exp.json} | 5 +- ...m_exp.json.txt => bgp_dom_af_exp.json.txt} | 0 ...bgp_dom_rdst.json => bgp_dom_af_rdst.json} | 5 +- ...rdst.json.txt => bgp_dom_af_rdst.json.txt} | 0 .../provider/cisco/nxos/testdata/lldp.json | 16 +- .../cisco/nxos/testdata/lldp.json.txt | 5 - .../cisco/nxos/testdata/lldp_if_items.json | 20 +++ .../nxos/testdata/lldp_if_items.json.txt | 6 + internal/provider/cisco/nxos/testdata/pc.json | 9 +- .../provider/cisco/nxos/testdata/pc.json.txt | 3 - .../provider/cisco/nxos/testdata/pc_lacp.json | 9 +- .../cisco/nxos/testdata/pc_member.json | 18 +++ .../cisco/nxos/testdata/pc_member.json.txt | 2 + .../provider/cisco/nxos/testdata/user.json | 14 -- .../cisco/nxos/testdata/user.json.txt | 2 +- .../cisco/nxos/testdata/user_role.json | 25 +++ .../cisco/nxos/testdata/user_role.json.txt | 1 + internal/provider/cisco/nxos/user.go | 31 ++-- internal/provider/cisco/nxos/user_test.go | 5 +- .../vpcdomain.txtar | 7 - 35 files changed, 403 insertions(+), 217 deletions(-) create mode 100644 internal/provider/cisco/nxos/testdata/bgp_dom_af.json rename internal/provider/cisco/nxos/testdata/{bgp_dom_advl2vpnevpn.json => bgp_dom_af_advl2vpnevpn.json} (75%) rename internal/provider/cisco/nxos/testdata/{bgp_dom_advl2vpnevpn.json.txt => bgp_dom_af_advl2vpnevpn.json.txt} (100%) rename internal/provider/cisco/nxos/testdata/{bgp_dom_advpip.json => bgp_dom_af_advpip.json} (89%) rename internal/provider/cisco/nxos/testdata/{bgp_dom_advpip.json.txt => bgp_dom_af_advpip.json.txt} (100%) rename internal/provider/cisco/nxos/testdata/{bgp_dom_exp.json => bgp_dom_af_exp.json} (75%) rename internal/provider/cisco/nxos/testdata/{bgp_dom_exp.json.txt => bgp_dom_af_exp.json.txt} (100%) rename internal/provider/cisco/nxos/testdata/{bgp_dom_rdst.json => bgp_dom_af_rdst.json} (85%) rename internal/provider/cisco/nxos/testdata/{bgp_dom_rdst.json.txt => bgp_dom_af_rdst.json.txt} (100%) create mode 100644 internal/provider/cisco/nxos/testdata/lldp_if_items.json create mode 100644 internal/provider/cisco/nxos/testdata/lldp_if_items.json.txt create mode 100644 internal/provider/cisco/nxos/testdata/pc_member.json create mode 100644 internal/provider/cisco/nxos/testdata/pc_member.json.txt create mode 100644 internal/provider/cisco/nxos/testdata/user_role.json create mode 100644 internal/provider/cisco/nxos/testdata/user_role.json.txt diff --git a/internal/provider/cisco/nxos/bgp.go b/internal/provider/cisco/nxos/bgp.go index 92996735e..c9b423946 100644 --- a/internal/provider/cisco/nxos/bgp.go +++ b/internal/provider/cisco/nxos/bgp.go @@ -18,6 +18,8 @@ var ( _ gnmiext.DataElement = (*BGP)(nil) _ gnmiext.DataElement = (*BGPDom)(nil) _ gnmiext.DataElement = (*BGPDomItems)(nil) + _ gnmiext.DataElement = (*BGPDomAfItems)(nil) + _ gnmiext.DataElement = (*BGPDomAfItem)(nil) _ gnmiext.DataElement = (*BGPPeerGroup)(nil) ) @@ -45,12 +47,9 @@ func (*BGP) XPath() string { } type BGPDom struct { - Name string `json:"name"` - RtrID string `json:"rtrId"` - RtrIDAuto AdminSt `json:"rtrIdAuto"` - AfItems struct { - DomAfList gnmiext.List[AddressFamily, *BGPDomAfItem] `json:"DomAf-list,omitzero"` - } `json:"af-items,omitzero"` + Name string `json:"name"` + RtrID string `json:"rtrId"` + RtrIDAuto AdminSt `json:"rtrIdAuto"` PeerContItems struct { PeerContList gnmiext.List[string, *BGPPeerGroup] `json:"PeerCont-list,omitzero"` } `json:"peercont-items,omitzero"` @@ -87,7 +86,18 @@ func (g *BGPPeerGroup) XPath() string { return "System/bgp-items/inst-items/dom-items/Dom-list[name=" + g.VRFName + "]/peercont-items/PeerCont-list[name=" + g.Name + "]" } +// BGPDomAfItems is the list container for all address families under a BGP domain. +type BGPDomAfItems struct { + Name string `json:"-"` // VRF name, for XPath construction + DomAfList gnmiext.List[AddressFamily, *BGPDomAfItem] `json:"DomAf-list,omitzero"` +} + +func (a *BGPDomAfItems) XPath() string { + return "System/bgp-items/inst-items/dom-items/Dom-list[name=" + a.Name + "]/af-items" +} + type BGPDomAfItem struct { + VRFName string `json:"-"` // for XPath construction // Maximum number of equal-cost paths for iBGP MaxEcmp int8 `json:"maxEcmp,omitempty"` // Maximum number of equal-cost paths for eBGP @@ -163,6 +173,12 @@ func (af *BGPDomAfItem) UnmarshalJSON(v []byte) error { func (af *BGPDomAfItem) Key() AddressFamily { return af.Type } +func (*BGPDomAfItem) IsListItem() {} + +func (af *BGPDomAfItem) XPath() string { + return "System/bgp-items/inst-items/dom-items/Dom-list[name=" + af.VRFName + "]/af-items/DomAf-list[type=" + string(af.Type) + "]" +} + // NewInterLeakPDirect creates an InterLeakP entry for redistributing directly // connected routes into a BGP address family. func NewInterLeakPDirect(rtMap string) *InterLeakP { diff --git a/internal/provider/cisco/nxos/bgp_test.go b/internal/provider/cisco/nxos/bgp_test.go index 41482dcff..3d3055473 100644 --- a/internal/provider/cisco/nxos/bgp_test.go +++ b/internal/provider/cisco/nxos/bgp_test.go @@ -4,26 +4,44 @@ package nxos func init() { - bgpDom := &BGPDom{Name: DefaultVRFName, RtrID: "1.1.1.1", RtrIDAuto: AdminStDisabled} - bgpDom.AfItems.DomAfList.Set(&BGPDomAfItem{ - Type: AddressFamilyL2EVPN, - RetainRttAll: AdminStEnabled, - }) - Register("bgp_dom", bgpDom) + bgp := &BGP{AdminSt: AdminStEnabled, Asn: "65000"} + Register("bgp", bgp) - bgpDomVrf := &BGPDom{Name: "CC-MGMT", RtrID: "1.1.1.1", RtrIDAuto: AdminStDisabled} - Register("bgp_dom_vrf", bgpDomVrf) + Register("bgp_dom", &BGPDom{Name: DefaultVRFName, RtrID: "1.1.1.1", RtrIDAuto: AdminStDisabled}) + + Register("bgp_dom_vrf", &BGPDom{Name: "CC-MGMT", RtrID: "1.1.1.1", RtrIDAuto: AdminStDisabled}) + + Register("bgp_dom_af", &BGPDomAfItem{ + VRFName: DefaultVRFName, + Type: AddressFamilyIPv4Unicast, + ExportGwIP: AdminStDisabled, + AdvertL2vpnEvpn: AdminStDisabled, + }) - bgpDomAdvPip := &BGPDom{Name: DefaultVRFName, RtrID: "1.1.1.1", RtrIDAuto: AdminStDisabled} - bgpDomAdvPip.AfItems.DomAfList.Set(&BGPDomAfItem{ + Register("bgp_dom_af_advpip", &BGPDomAfItem{ + VRFName: DefaultVRFName, Type: AddressFamilyL2EVPN, AdvPip: AdminStEnabled, RetainRttAll: AdminStEnabled, }) - Register("bgp_dom_advpip", bgpDomAdvPip) - bgp := &BGP{AdminSt: AdminStEnabled, Asn: "65000"} - Register("bgp", bgp) + Register("bgp_dom_af_exp", &BGPDomAfItem{ + VRFName: DefaultVRFName, + Type: AddressFamilyIPv4Unicast, + ExportGwIP: AdminStEnabled, + AdvertL2vpnEvpn: AdminStDisabled, + }) + + Register("bgp_dom_af_advl2vpnevpn", &BGPDomAfItem{ + VRFName: DefaultVRFName, + Type: AddressFamilyIPv4Unicast, + ExportGwIP: AdminStDisabled, + AdvertL2vpnEvpn: AdminStEnabled, + }) + + rdstItem := &BGPDomAfItem{VRFName: DefaultVRFName, Type: AddressFamilyIPv4Unicast, ExportGwIP: AdminStDisabled, AdvertL2vpnEvpn: AdminStDisabled} + rdstItem.InterLeakPItems.InterLeakPList.Set(NewInterLeakPDirect("ROUTE_MAP")) + Register("bgp_dom_af_rdst", rdstItem) bgpPeer := &BGPPeer{ VRFName: DefaultVRFName, @@ -62,28 +80,6 @@ func init() { bgpPeerRp.AfItems.PeerAfList.Set(bgpPeerRpAf) Register("bgp_dom_rp", bgpPeerRp) - bgpDomRdst := &BGPDom{Name: "CC-CLOUD01", RtrID: "1.1.1.1", RtrIDAuto: AdminStDisabled} - rdstItem := &BGPDomAfItem{Type: AddressFamilyIPv4Unicast, ExportGwIP: AdminStDisabled, AdvertL2vpnEvpn: AdminStDisabled} - rdstItem.InterLeakPItems.InterLeakPList.Set(NewInterLeakPDirect("ROUTE_MAP")) - bgpDomRdst.AfItems.DomAfList.Set(rdstItem) - Register("bgp_dom_rdst", bgpDomRdst) - - bgpDomExp := &BGPDom{Name: "CC-CLOUD01", RtrID: "1.1.1.1", RtrIDAuto: AdminStDisabled} - bgpDomExp.AfItems.DomAfList.Set(&BGPDomAfItem{ - Type: AddressFamilyIPv4Unicast, - ExportGwIP: AdminStEnabled, - AdvertL2vpnEvpn: AdminStDisabled, - }) - Register("bgp_dom_exp", bgpDomExp) - - bgpDomAdvL2vpnEvpn := &BGPDom{Name: "CC-CLOUD01", RtrID: "1.1.1.1", RtrIDAuto: AdminStDisabled} - bgpDomAdvL2vpnEvpn.AfItems.DomAfList.Set(&BGPDomAfItem{ - Type: AddressFamilyIPv4Unicast, - ExportGwIP: AdminStDisabled, - AdvertL2vpnEvpn: AdminStEnabled, - }) - Register("bgp_dom_advl2vpnevpn", bgpDomAdvL2vpnEvpn) - bgpPeerLocalAs := &BGPPeer{ VRFName: DefaultVRFName, Addr: "1.1.1.1", diff --git a/internal/provider/cisco/nxos/intf.go b/internal/provider/cisco/nxos/intf.go index ba223f47d..c55cb4460 100644 --- a/internal/provider/cisco/nxos/intf.go +++ b/internal/provider/cisco/nxos/intf.go @@ -32,11 +32,15 @@ var ( _ gnmiext.DataElement = (*ICMPIf)(nil) _ gnmiext.DataElement = (*PortChannel)(nil) _ gnmiext.DataElement = (*PortChannelOperItems)(nil) + _ gnmiext.DataElement = (*PortChannelMemberItems)(nil) + _ gnmiext.DataElement = (*PortChannelMember)(nil) _ gnmiext.DataElement = (*SwitchVirtualInterface)(nil) _ gnmiext.DataElement = (*SwitchVirtualInterfaceOperItems)(nil) _ gnmiext.DataElement = (*EncapRoutedInterface)(nil) _ gnmiext.DataElement = (*EncapRoutedInterfaceOperItems)(nil) _ gnmiext.DataElement = (*AddrItem)(nil) + _ gnmiext.DataElement = (*IntfAddrItems)(nil) + _ gnmiext.DataElement = (*IntfAddr)(nil) _ gnmiext.DataElement = (*FabricFwdIf)(nil) ) @@ -294,17 +298,31 @@ type PortChannel struct { SuspIndividual AdminSt4 `json:"suspIndividual"` UserCfgdFlags UserFlags `json:"userCfgdFlags"` RtvrfMbrItems *VrfMember `json:"rtvrfMbr-items,omitempty"` - RsmbrIfsItems struct { - RsMbrIfsList gnmiext.List[string, *PortChannelMember] `json:"RsMbrIfs-list,omitzero"` - } `json:"rsmbrIfs-items,omitzero"` - AggrExtdItems struct { + AggrExtdItems struct { BufferBoost AdminSt4 `json:"bufferBoost,omitempty"` } `json:"aggrExtd-items,omitzero"` } +func (*PortChannel) IsListItem() {} + +func (p *PortChannel) XPath() string { + return "System/intf-items/aggr-items/AggrIf-list[id=" + p.ID + "]" +} + +// PortChannelMemberItems is the list container for fetching port-channel members. +type PortChannelMemberItems struct { + ID string `json:"-"` + RsMbrIfsList gnmiext.List[string, *PortChannelMember] `json:"RsMbrIfs-list,omitzero"` +} + +func (m *PortChannelMemberItems) XPath() string { + return "System/intf-items/aggr-items/AggrIf-list[id=" + m.ID + "]/rsmbrIfs-items" +} + type PortChannelMember struct { - TDn string `json:"tDn"` - Force bool `json:"isMbrForce,omitempty"` + PortChannelID string `json:"-"` + TDn string `json:"tDn"` + Force bool `json:"isMbrForce,omitempty"` } func NewPortChannelMember(name string) *PortChannelMember { @@ -316,10 +334,13 @@ func NewPortChannelMember(name string) *PortChannelMember { func (m *PortChannelMember) Key() string { return m.TDn } -func (*PortChannel) IsListItem() {} +func (*PortChannelMember) IsListItem() {} -func (p *PortChannel) XPath() string { - return "System/intf-items/aggr-items/AggrIf-list[id=" + p.ID + "]" +func (m *PortChannelMember) XPath() string { + // Escape brackets in tDn so ygot.StringToStructuredPath does not + // treat them as key delimiters. + tDn := strings.NewReplacer("[", `\[`, "]", `\]`).Replace(m.TDn) + return "System/intf-items/aggr-items/AggrIf-list[id=" + m.PortChannelID + "]/rsmbrIfs-items/RsMbrIfs-list[tDn=" + tDn + "]" } type PortChannelOperItems struct { @@ -406,9 +427,6 @@ func (d *AddrDom) Key() string { return d.Name } type AddrItem struct { ID string `json:"id"` Unnumbered string `json:"unnumbered,omitempty"` - AddrItems struct { - AddrList gnmiext.List[string, *IntfAddr] `json:"Addr-list,omitzero"` - } `json:"addr-items,omitzero"` // Is6 indicates whether the addresses are IPv6 (true) or IPv4 (false). // This field is not serialized to JSON and is only used internally to @@ -432,7 +450,25 @@ func (a *AddrItem) XPath() string { return "System/ipv4-items/inst-items/dom-items/Dom-list[name=" + a.Vrf + "]/if-items/If-list[id=" + a.ID + "]" } +// IntfAddrItems is the list container for fetching addresses on an interface. +type IntfAddrItems struct { + ID string `json:"-"` + Vrf string `json:"-"` + Is6 bool `json:"-"` + AddrList gnmiext.List[string, *IntfAddr] `json:"Addr-list,omitzero"` +} + +func (a *IntfAddrItems) XPath() string { + if a.Is6 { + return "System/ipv6-items/inst-items/dom-items/Dom-list[name=" + a.Vrf + "]/if-items/If-list[id=" + a.ID + "]/addr-items" + } + return "System/ipv4-items/inst-items/dom-items/Dom-list[name=" + a.Vrf + "]/if-items/If-list[id=" + a.ID + "]/addr-items" +} + type IntfAddr struct { + ID string `json:"-"` + Vrf string `json:"-"` + Is6 bool `json:"-"` Addr string `json:"addr"` Pref int `json:"pref"` Tag int `json:"tag"` @@ -441,6 +477,15 @@ type IntfAddr struct { func (a *IntfAddr) Key() string { return a.Addr } +func (*IntfAddr) IsListItem() {} + +func (a *IntfAddr) XPath() string { + if a.Is6 { + return "System/ipv6-items/inst-items/dom-items/Dom-list[name=" + a.Vrf + "]/if-items/If-list[id=" + a.ID + "]/addr-items/Addr-list[addr=" + a.Addr + "]" + } + return "System/ipv4-items/inst-items/dom-items/Dom-list[name=" + a.Vrf + "]/if-items/If-list[id=" + a.ID + "]/addr-items/Addr-list[addr=" + a.Addr + "]" +} + type IntfAddrType string const ( diff --git a/internal/provider/cisco/nxos/intf_test.go b/internal/provider/cisco/nxos/intf_test.go index 5bb4bc806..9e2829ebd 100644 --- a/internal/provider/cisco/nxos/intf_test.go +++ b/internal/provider/cisco/nxos/intf_test.go @@ -128,16 +128,17 @@ func init() { Descr: NewOption("L3 Subinterface on eth1/1"), }) - intfAddr4 := &AddrItem{ID: "lo0", Vrf: DefaultVRFName} - intfAddr4.AddrItems.AddrList.Set(&IntfAddr{ + Register("intf_addr4", &IntfAddr{ + ID: "lo0", + Vrf: DefaultVRFName, + Is6: false, Addr: "10.0.0.10/32", Pref: 0, Tag: 0, Type: "primary", }) - Register("intf_addr4", intfAddr4) - pc := &PortChannel{ + Register("pc", &PortChannel{ AccessVlan: DefaultVLAN, AdminSt: AdminStUp, Descr: NewOption("vPC Leaf1 to Host1"), @@ -151,9 +152,12 @@ func init() { NativeVlan: DefaultVLAN, SuspIndividual: AdminStEnable, UserCfgdFlags: UserFlagAdminState, - } - pc.RsmbrIfsItems.RsMbrIfsList.Set(NewPortChannelMember("eth1/10")) - Register("pc", pc) + }) + + pcMember := NewPortChannelMember("eth1/10") + pcMember.PortChannelID = "po10" + Register("pc_member", pcMember) + Register("pc_trunk_vlans", &TrunkVlans{IfName: "po10", Vlans: "10"}) Register("pc_rtd", &PortChannel{ @@ -176,7 +180,7 @@ func init() { }{BufferBoost: AdminStEnable}, }) - pcLacp := &PortChannel{ + Register("pc_lacp", &PortChannel{ AccessVlan: DefaultVLAN, AdminSt: AdminStUp, Descr: NewOption("vPC Leaf1 to Host1 (LACP)"), @@ -190,9 +194,7 @@ func init() { NativeVlan: DefaultVLAN, SuspIndividual: AdminStDisable, UserCfgdFlags: UserFlagAdminState, - } - pcLacp.RsmbrIfsItems.RsMbrIfsList.Set(NewPortChannelMember("eth1/1")) - Register("pc_lacp", pcLacp) + }) svi := &SwitchVirtualInterface{ AdminSt: AdminStUp, diff --git a/internal/provider/cisco/nxos/lldp.go b/internal/provider/cisco/nxos/lldp.go index 9502007e5..cb9a37d11 100644 --- a/internal/provider/cisco/nxos/lldp.go +++ b/internal/provider/cisco/nxos/lldp.go @@ -5,24 +5,33 @@ package nxos import "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" -var _ gnmiext.DataElement = (*LLDP)(nil) +var ( + _ gnmiext.DataElement = (*LLDP)(nil) + _ gnmiext.DataElement = (*LLDPIfItems)(nil) + _ gnmiext.DataElement = (*LLDPIfItem)(nil) +) type LLDP struct { // HoldTime is the number of seconds that a receiving device should hold the information sent by another device before discarding it. HoldTime Option[uint16] `json:"holdTime"` // InitDelay is the number of seconds for LLDP to initialize on any interface. InitDelay Option[uint16] `json:"initDelayTime"` - // IfItems contains the per-interface LLDP configuration. - IfItems struct { - IfList gnmiext.List[string, *LLDPIfItem] `json:"If-list,omitzero"` - } `json:"if-items,omitzero"` } +func (*LLDP) IsListItem() {} + func (*LLDP) XPath() string { return "System/lldp-items/inst-items" } -func (*LLDP) IsListItem() {} +// LLDPIfItems is the list container for fetching per-interface LLDP configuration. +type LLDPIfItems struct { + IfList gnmiext.List[string, *LLDPIfItem] `json:"If-list,omitzero"` +} + +func (*LLDPIfItems) XPath() string { + return "System/lldp-items/inst-items/if-items" +} type LLDPIfItem struct { InterfaceName string `json:"id"` @@ -32,6 +41,12 @@ type LLDPIfItem struct { func (i *LLDPIfItem) Key() string { return i.InterfaceName } +func (*LLDPIfItem) IsListItem() {} + +func (i *LLDPIfItem) XPath() string { + return "System/lldp-items/inst-items/if-items/If-list[id=" + i.InterfaceName + "]" +} + type LLDPOper struct { OperSt OperSt `json:"operSt"` } diff --git a/internal/provider/cisco/nxos/lldp_test.go b/internal/provider/cisco/nxos/lldp_test.go index 5128f4a1b..a37b45750 100644 --- a/internal/provider/cisco/nxos/lldp_test.go +++ b/internal/provider/cisco/nxos/lldp_test.go @@ -4,21 +4,20 @@ package nxos func init() { - lldp := &LLDP{ + Register("lldp", &LLDP{ HoldTime: NewOption(uint16(200)), InitDelay: NewOption(uint16(5)), - } + }) - lldp.IfItems.IfList.Set(&LLDPIfItem{ + items := new(LLDPIfItems) + items.IfList.Set(&LLDPIfItem{ InterfaceName: "eth7/1", AdminRxSt: NewOption(AdminStDisabled), AdminTxSt: NewOption(AdminStDisabled), }) - - lldp.IfItems.IfList.Set(&LLDPIfItem{ + items.IfList.Set(&LLDPIfItem{ InterfaceName: "eth8/1", AdminTxSt: NewOption(AdminStDisabled), }) - - Register("lldp", lldp) + Register("lldp_if_items", items) } diff --git a/internal/provider/cisco/nxos/nve.go b/internal/provider/cisco/nxos/nve.go index a1272c1ba..04b2b9a10 100644 --- a/internal/provider/cisco/nxos/nve.go +++ b/internal/provider/cisco/nxos/nve.go @@ -13,6 +13,7 @@ import ( var ( _ gnmiext.DataElement = (*NVE)(nil) _ gnmiext.DataElement = (*NVEInfraVLANs)(nil) + _ gnmiext.DataElement = (*NVEInfraVLAN)(nil) _ gnmiext.DataElement = (*FabricFwd)(nil) ) @@ -93,7 +94,7 @@ const ( ) type NVEInfraVLANs struct { - InfraVLANList []*NVEInfraVLAN `json:"InfraVlan-list,omitempty"` + InfraVLANList gnmiext.List[uint32, *NVEInfraVLAN] `json:"InfraVlan-list,omitzero"` } func (*NVEInfraVLANs) XPath() string { @@ -104,6 +105,12 @@ type NVEInfraVLAN struct { ID uint32 `json:"id"` } +func (v *NVEInfraVLAN) Key() uint32 { return v.ID } + +func (v *NVEInfraVLAN) XPath() string { + return "System/pltfm-items/nve-items/NVE-list[id=1]/infravlan-items/InfraVlan-list[id=" + strconv.FormatUint(uint64(v.ID), 10) + "]" +} + func (*NVEInfraVLAN) IsListItem() {} // NVEOper represents the operational state of the NVE interface. diff --git a/internal/provider/cisco/nxos/nve_test.go b/internal/provider/cisco/nxos/nve_test.go index 30667cb71..e31ba2288 100644 --- a/internal/provider/cisco/nxos/nve_test.go +++ b/internal/provider/cisco/nxos/nve_test.go @@ -22,13 +22,11 @@ func init() { McastGroup: NewOption("239.1.1.100"), } Register("vni", vni) - nveInfraVLANs := &NVEInfraVLANs{ - InfraVLANList: []*NVEInfraVLAN{ - {ID: 4052}, - {ID: 4092}, - }, - } - Register("infra_vlans", nveInfraVLANs) + + infraVLANs := &NVEInfraVLANs{} + infraVLANs.InfraVLANList.Set(&NVEInfraVLAN{ID: 4052}) + infraVLANs.InfraVLANList.Set(&NVEInfraVLAN{ID: 4092}) + Register("infra_vlans", infraVLANs) ffw := &FabricFwd{ AdminSt: "enabled", diff --git a/internal/provider/cisco/nxos/provider.go b/internal/provider/cisco/nxos/provider.go index 8e1a49a42..0890b3484 100644 --- a/internal/provider/cisco/nxos/provider.go +++ b/internal/provider/cisco/nxos/provider.go @@ -637,9 +637,11 @@ func (p *Provider) EnsureBGP(ctx context.Context, req *provider.EnsureBGPRequest marker := &BGPPeerGroup{VRFName: DefaultVRFName, Name: ownershipMarkerName(dom.Name)} sb.Patch(marker) + var safis gnmiext.List[AddressFamily, *BGPDomAfItem] if req.BGP.Spec.AddressFamilies != nil { if af := req.BGP.Spec.AddressFamilies.Ipv4Unicast; af != nil && af.Enabled { item := new(BGPDomAfItem) + item.VRFName = dom.Name item.Type = AddressFamilyIPv4Unicast if err := item.SetMultipath(af.Multipath); err != nil { return err @@ -655,11 +657,12 @@ func (p *Provider) EnsureBGP(ctx context.Context, req *provider.EnsureBGPRequest if cfg.Spec.AddressFamilies.Ipv4UnicastAdvertiseL2vpnEvpn() { item.AdvertL2vpnEvpn = AdminStEnabled } - dom.AfItems.DomAfList.Set(item) + safis.Set(item) } if af := req.BGP.Spec.AddressFamilies.Ipv6Unicast; af != nil && af.Enabled { item := new(BGPDomAfItem) + item.VRFName = dom.Name item.Type = AddressFamilyIPv6Unicast if err := item.SetMultipath(af.Multipath); err != nil { return err @@ -675,7 +678,7 @@ func (p *Provider) EnsureBGP(ctx context.Context, req *provider.EnsureBGPRequest if cfg.Spec.AddressFamilies.Ipv6UnicastAdvertiseL2vpnEvpn() { item.AdvertL2vpnEvpn = AdminStEnabled } - dom.AfItems.DomAfList.Set(item) + safis.Set(item) } if af := req.BGP.Spec.AddressFamilies.L2vpnEvpn; af != nil && af.Enabled { @@ -686,6 +689,7 @@ func (p *Provider) EnsureBGP(ctx context.Context, req *provider.EnsureBGPRequest }) } item := new(BGPDomAfItem) + item.VRFName = dom.Name item.Type = AddressFamilyL2EVPN if err := item.SetMultipath(af.Multipath); err != nil { return err @@ -697,7 +701,21 @@ func (p *Provider) EnsureBGP(ctx context.Context, req *provider.EnsureBGPRequest if cfg.Spec.AddressFamilies != nil && cfg.Spec.AddressFamilies.L2vpnEvpn != nil && cfg.Spec.AddressFamilies.L2vpnEvpn.AdvertisePIP { item.AdvPip = AdminStEnabled } - dom.AfItems.DomAfList.Set(item) + safis.Set(item) + } + } + + current := &BGPDomAfItems{Name: dom.Name} + if err := p.client.GetConfig(ctx, current); err != nil && !errors.Is(err, gnmiext.ErrNil) { + return err + } + for _, item := range safis { + sb.Patch(item) + } + for _, item := range current.DomAfList { + if _, ok := safis.Get(item.Key()); !ok { + item.VRFName = dom.Name + sb.Delete(item) } } @@ -1250,6 +1268,7 @@ func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInte } var addr *AddrItem + var addrs gnmiext.List[string, *IntfAddr] if req.IPv4 != nil { addr = new(AddrItem) addr.ID = name @@ -1263,10 +1282,12 @@ func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInte nth = IntfAddrTypeSecondary } ip := &IntfAddr{ + ID: name, + Vrf: vrf, Addr: p.String(), Type: nth, } - addr.AddrItems.AddrList.Set(ip) + addrs.Set(ip) } case provider.IPv4Unnumbered: @@ -1277,11 +1298,11 @@ func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInte } } - addrs := new(AddrList) - if err := p.client.GetConfig(ctx, addrs); err != nil && !errors.Is(err, gnmiext.ErrNil) { + current := new(AddrList) + if err := p.client.GetConfig(ctx, current); err != nil && !errors.Is(err, gnmiext.ErrNil) { return err } - for _, a := range addrs.GetAddrItemsByInterface(name) { + for _, a := range current.GetAddrItemsByInterface(name) { if addr == nil || a.Vrf != vrf { sb.Delete(a) } @@ -1463,12 +1484,15 @@ func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInte } } + var members gnmiext.List[string, *PortChannelMember] for _, member := range req.Members { n, err := ShortNamePhysicalInterface(member.Spec.Name) if err != nil { return err } - pc.RsmbrIfsItems.RsMbrIfsList.Set(NewPortChannelMember(n)) + m := NewPortChannelMember(n) + m.PortChannelID = name + members.Set(m) } v := new(VPCIfItems) @@ -1500,6 +1524,20 @@ func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInte sb.Patch(pc) + current := &PortChannelMemberItems{ID: name} + if err := p.client.GetConfig(ctx, current); err != nil && !errors.Is(err, gnmiext.ErrNil) { + return err + } + for _, m := range members { + sb.Patch(m) + } + for _, m := range current.RsMbrIfsList { + if _, ok := members.Get(m.Key()); !ok { + m.PortChannelID = name + sb.Delete(m) + } + } + if req.MultiChassisID != nil { v := new(VPCIf) v.ID = int(*req.MultiChassisID) @@ -1631,6 +1669,20 @@ func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInte // Add the address items last, as they depend on the interface being created first. if addr != nil { sb.Patch(addr) + for _, a := range addrs { + sb.Patch(a) + } + currentAddrs := &IntfAddrItems{ID: name, Vrf: vrf} + if err := p.client.GetConfig(ctx, currentAddrs); err != nil && !errors.Is(err, gnmiext.ErrNil) { + return err + } + for _, a := range currentAddrs.AddrList { + if _, ok := addrs.Get(a.Key()); !ok { + a.ID = name + a.Vrf = vrf + sb.Delete(a) + } + } } switch { @@ -2627,14 +2679,10 @@ func (p *Provider) EnsureUser(ctx context.Context, req *provider.EnsureUserReque u.Name = req.Username u.SshauthItems.Data = req.SSHKey - d := new(UserDomain) - d.Name = "all" + var roles gnmiext.List[string, *UserRole] for _, role := range req.Roles { - r := new(UserRole) - r.Name = role - d.RoleItems.UserRoleList.Set(r) + roles.Set(&UserRole{Username: req.Username, Name: role}) } - u.UserdomainItems.UserDomainList.Set(d) // If the user already exists and the password matches, retain the existing // password hash to avoid unnecessary updates. @@ -2665,7 +2713,24 @@ func (p *Provider) EnsureUser(ctx context.Context, req *provider.EnsureUserReque } } - return p.client.Patch(ctx, u) + sb := new(gnmiext.SetBuilder) + sb.Patch(u) + + current := &UserRoleItems{Username: req.Username} + if err := p.client.GetConfig(ctx, current); err != nil && !errors.Is(err, gnmiext.ErrNil) { + return err + } + for _, r := range roles { + sb.Patch(r) + } + for _, r := range current.UserRoleList { + if _, ok := roles.Get(r.Key()); !ok { + r.Username = req.Username + sb.Delete(r) + } + } + + return p.client.Do(ctx, sb) } func (p *Provider) DeleteUser(ctx context.Context, req *provider.DeleteUserRequest) error { @@ -3476,17 +3541,17 @@ func (p *Provider) EnsureNVE(ctx context.Context, req *provider.NVERequest) erro iv := new(NVEInfraVLANs) for _, ivList := range vc.Spec.InfraVLANs { if ivList.ID != 0 { - iv.InfraVLANList = append(iv.InfraVLANList, &NVEInfraVLAN{ID: uint32(ivList.ID)}) // #nosec G115 -- kubebuilder validation + iv.InfraVLANList.Set(&NVEInfraVLAN{ID: uint32(ivList.ID)}) // #nosec G115 -- kubebuilder validation continue } for i := ivList.RangeMin; i <= ivList.RangeMax; i++ { - iv.InfraVLANList = append(iv.InfraVLANList, &NVEInfraVLAN{ID: uint32(i)}) // #nosec G115 -- kubebuilder validation + iv.InfraVLANList.Set(&NVEInfraVLAN{ID: uint32(i)}) // #nosec G115 -- kubebuilder validation } } - infraVLANs := make([]int16, len(iv.InfraVLANList)) - for i := range iv.InfraVLANList { - infraVLANs[i] = int16(iv.InfraVLANList[i].ID) // #nosec G115 -- kubebuilder validation + infraVLANs := make([]int16, 0, iv.InfraVLANList.Len()) + for _, vlan := range iv.InfraVLANList { + infraVLANs = append(infraVLANs, int16(vlan.ID)) // #nosec G115 -- kubebuilder validation } if len(infraVLANs) > 0 { if err := p.ValidateReservedVLANs(ctx, infraVLANs); err != nil { @@ -3494,15 +3559,17 @@ func (p *Provider) EnsureNVE(ctx context.Context, req *provider.NVERequest) erro } } - if len(iv.InfraVLANList) == 0 { - if err := p.client.GetConfig(ctx, iv); err != nil && !errors.Is(err, gnmiext.ErrNil) { - return err - } - if len(iv.InfraVLANList) != 0 { - sb.Delete(iv) + current := new(NVEInfraVLANs) + if err := p.client.GetConfig(ctx, current); err != nil && !errors.Is(err, gnmiext.ErrNil) { + return err + } + for _, vlan := range iv.InfraVLANList { + sb.Patch(vlan) + } + for _, vlan := range current.InfraVLANList { + if _, ok := iv.InfraVLANList.Get(vlan.Key()); !ok { + sb.Delete(vlan) } - } else { - sb.Patch(iv) } ag := new(FabricFwd) @@ -3592,12 +3659,14 @@ func (p *Provider) EnsureLLDP(ctx context.Context, req *provider.LLDPRequest) er l.InitDelay = NewOption(uint16(cfg.Spec.InitDelay)) //nolint:gosec l.HoldTime = NewOption(uint16(cfg.Spec.HoldTime)) //nolint:gosec } + sb.Patch(l) interfaceMap := make(map[string]*v1alpha1.Interface, len(req.Interfaces)) for _, intf := range req.Interfaces { interfaceMap[intf.Name] = intf } + var desired gnmiext.List[string, *LLDPIfItem] for _, ifRef := range req.LLDP.Spec.InterfaceRefs { intf, ok := interfaceMap[ifRef.Name] if !ok { @@ -3621,9 +3690,21 @@ func (p *Provider) EnsureLLDP(ctx context.Context, req *provider.LLDPRequest) er item.AdminTxSt = NewOption(AdminStDisabled) } - l.IfItems.IfList.Set(item) + desired.Set(item) + } + + current := new(LLDPIfItems) + if err := p.client.GetConfig(ctx, current); err != nil && !errors.Is(err, gnmiext.ErrNil) { + return err + } + for _, item := range desired { + sb.Patch(item) + } + for _, item := range current.IfList { + if _, ok := desired.Get(item.Key()); !ok { + sb.Delete(item) + } } - sb.Patch(l) return p.Do(ctx, sb) } @@ -4097,7 +4178,11 @@ func (p *Provider) InterfaceIPAddr(ctx context.Context, name, vrf string, isIPv6 if !isIPv6 && addr.Unnumbered != "" { return p.InterfaceIPAddr(ctx, addr.Unnumbered, vrf, false) } - for _, a := range addr.AddrItems.AddrList { + items := &IntfAddrItems{ID: short, Vrf: vrf, Is6: isIPv6} + if err := p.client.GetConfig(ctx, items); err != nil && !errors.Is(err, gnmiext.ErrNil) { + return "", fmt.Errorf("failed to get IP address for interface %q in VRF %q: %w", name, vrf, err) + } + for _, a := range items.AddrList { if a.Type == IntfAddrTypePrimary { ip, _, _ := strings.Cut(a.Addr, "/") return ip, nil diff --git a/internal/provider/cisco/nxos/testdata/bgp_dom.json b/internal/provider/cisco/nxos/testdata/bgp_dom.json index eb53b134c..e695e9136 100644 --- a/internal/provider/cisco/nxos/testdata/bgp_dom.json +++ b/internal/provider/cisco/nxos/testdata/bgp_dom.json @@ -6,18 +6,7 @@ { "name": "default", "rtrId": "1.1.1.1", - "rtrIdAuto": "disabled", - "af-items": { - "DomAf-list": [ - { - "exportGwIp": "disabled", - "advertL2vpnEvpn": "disabled", - "type": "l2vpn-evpn", - "retainRttAll": "enabled", - "retainRttRtMap": "DME_UNSET_PROPERTY_MARKER" - } - ] - } + "rtrIdAuto": "disabled" } ] } diff --git a/internal/provider/cisco/nxos/testdata/bgp_dom_af.json b/internal/provider/cisco/nxos/testdata/bgp_dom_af.json new file mode 100644 index 000000000..d4c674c1b --- /dev/null +++ b/internal/provider/cisco/nxos/testdata/bgp_dom_af.json @@ -0,0 +1,22 @@ +{ + "bgp-items": { + "inst-items": { + "dom-items": { + "Dom-list": [ + { + "name": "default", + "af-items": { + "DomAf-list": [ + { + "exportGwIp": "disabled", + "advertL2vpnEvpn": "disabled", + "type": "ipv4-ucast" + } + ] + } + } + ] + } + } + } +} diff --git a/internal/provider/cisco/nxos/testdata/bgp_dom_advl2vpnevpn.json b/internal/provider/cisco/nxos/testdata/bgp_dom_af_advl2vpnevpn.json similarity index 75% rename from internal/provider/cisco/nxos/testdata/bgp_dom_advl2vpnevpn.json rename to internal/provider/cisco/nxos/testdata/bgp_dom_af_advl2vpnevpn.json index 5efffb823..06b1a98e2 100644 --- a/internal/provider/cisco/nxos/testdata/bgp_dom_advl2vpnevpn.json +++ b/internal/provider/cisco/nxos/testdata/bgp_dom_af_advl2vpnevpn.json @@ -1,13 +1,10 @@ { "bgp-items": { "inst-items": { - "asn": "65000", "dom-items": { "Dom-list": [ { - "name": "CC-CLOUD01", - "rtrId": "1.1.1.1", - "rtrIdAuto": "disabled", + "name": "default", "af-items": { "DomAf-list": [ { diff --git a/internal/provider/cisco/nxos/testdata/bgp_dom_advl2vpnevpn.json.txt b/internal/provider/cisco/nxos/testdata/bgp_dom_af_advl2vpnevpn.json.txt similarity index 100% rename from internal/provider/cisco/nxos/testdata/bgp_dom_advl2vpnevpn.json.txt rename to internal/provider/cisco/nxos/testdata/bgp_dom_af_advl2vpnevpn.json.txt diff --git a/internal/provider/cisco/nxos/testdata/bgp_dom_advpip.json b/internal/provider/cisco/nxos/testdata/bgp_dom_af_advpip.json similarity index 89% rename from internal/provider/cisco/nxos/testdata/bgp_dom_advpip.json rename to internal/provider/cisco/nxos/testdata/bgp_dom_af_advpip.json index e389a31bb..876e31db8 100644 --- a/internal/provider/cisco/nxos/testdata/bgp_dom_advpip.json +++ b/internal/provider/cisco/nxos/testdata/bgp_dom_af_advpip.json @@ -5,8 +5,6 @@ "Dom-list": [ { "name": "default", - "rtrId": "1.1.1.1", - "rtrIdAuto": "disabled", "af-items": { "DomAf-list": [ { diff --git a/internal/provider/cisco/nxos/testdata/bgp_dom_advpip.json.txt b/internal/provider/cisco/nxos/testdata/bgp_dom_af_advpip.json.txt similarity index 100% rename from internal/provider/cisco/nxos/testdata/bgp_dom_advpip.json.txt rename to internal/provider/cisco/nxos/testdata/bgp_dom_af_advpip.json.txt diff --git a/internal/provider/cisco/nxos/testdata/bgp_dom_exp.json b/internal/provider/cisco/nxos/testdata/bgp_dom_af_exp.json similarity index 75% rename from internal/provider/cisco/nxos/testdata/bgp_dom_exp.json rename to internal/provider/cisco/nxos/testdata/bgp_dom_af_exp.json index 9e3ee120f..75bd73fbd 100644 --- a/internal/provider/cisco/nxos/testdata/bgp_dom_exp.json +++ b/internal/provider/cisco/nxos/testdata/bgp_dom_af_exp.json @@ -1,13 +1,10 @@ { "bgp-items": { "inst-items": { - "asn": "65000", "dom-items": { "Dom-list": [ { - "name": "CC-CLOUD01", - "rtrId": "1.1.1.1", - "rtrIdAuto": "disabled", + "name": "default", "af-items": { "DomAf-list": [ { diff --git a/internal/provider/cisco/nxos/testdata/bgp_dom_exp.json.txt b/internal/provider/cisco/nxos/testdata/bgp_dom_af_exp.json.txt similarity index 100% rename from internal/provider/cisco/nxos/testdata/bgp_dom_exp.json.txt rename to internal/provider/cisco/nxos/testdata/bgp_dom_af_exp.json.txt diff --git a/internal/provider/cisco/nxos/testdata/bgp_dom_rdst.json b/internal/provider/cisco/nxos/testdata/bgp_dom_af_rdst.json similarity index 85% rename from internal/provider/cisco/nxos/testdata/bgp_dom_rdst.json rename to internal/provider/cisco/nxos/testdata/bgp_dom_af_rdst.json index 3ea730d70..68d592265 100644 --- a/internal/provider/cisco/nxos/testdata/bgp_dom_rdst.json +++ b/internal/provider/cisco/nxos/testdata/bgp_dom_af_rdst.json @@ -1,13 +1,10 @@ { "bgp-items": { "inst-items": { - "asn": "65000", "dom-items": { "Dom-list": [ { - "name": "CC-CLOUD01", - "rtrId": "1.1.1.1", - "rtrIdAuto": "disabled", + "name": "default", "af-items": { "DomAf-list": [ { diff --git a/internal/provider/cisco/nxos/testdata/bgp_dom_rdst.json.txt b/internal/provider/cisco/nxos/testdata/bgp_dom_af_rdst.json.txt similarity index 100% rename from internal/provider/cisco/nxos/testdata/bgp_dom_rdst.json.txt rename to internal/provider/cisco/nxos/testdata/bgp_dom_af_rdst.json.txt diff --git a/internal/provider/cisco/nxos/testdata/lldp.json b/internal/provider/cisco/nxos/testdata/lldp.json index 9e659f0a1..4c44f04e5 100644 --- a/internal/provider/cisco/nxos/testdata/lldp.json +++ b/internal/provider/cisco/nxos/testdata/lldp.json @@ -2,21 +2,7 @@ "lldp-items": { "inst-items": { "holdTime": 200, - "initDelayTime": 5, - "if-items": { - "If-list": [ - { - "id": "eth7/1", - "adminRxSt": "disabled", - "adminTxSt": "disabled" - }, - { - "id": "eth8/1", - "adminRxSt": "DME_UNSET_PROPERTY_MARKER", - "adminTxSt": "disabled" - } - ] - } + "initDelayTime": 5 } } } diff --git a/internal/provider/cisco/nxos/testdata/lldp.json.txt b/internal/provider/cisco/nxos/testdata/lldp.json.txt index d9d0a7438..c77c9591d 100644 --- a/internal/provider/cisco/nxos/testdata/lldp.json.txt +++ b/internal/provider/cisco/nxos/testdata/lldp.json.txt @@ -1,7 +1,2 @@ lldp holdtime 200 lldp reinit 5 -interface ethernet 7/1 - no lldp receive - no lldp transmit -interface ethernet 8/1 - no lldp transmit diff --git a/internal/provider/cisco/nxos/testdata/lldp_if_items.json b/internal/provider/cisco/nxos/testdata/lldp_if_items.json new file mode 100644 index 000000000..0565b33bb --- /dev/null +++ b/internal/provider/cisco/nxos/testdata/lldp_if_items.json @@ -0,0 +1,20 @@ +{ + "lldp-items": { + "inst-items": { + "if-items": { + "If-list": [ + { + "id": "eth7/1", + "adminRxSt": "disabled", + "adminTxSt": "disabled" + }, + { + "id": "eth8/1", + "adminRxSt": "DME_UNSET_PROPERTY_MARKER", + "adminTxSt": "disabled" + } + ] + } + } + } +} diff --git a/internal/provider/cisco/nxos/testdata/lldp_if_items.json.txt b/internal/provider/cisco/nxos/testdata/lldp_if_items.json.txt new file mode 100644 index 000000000..eb00dd60d --- /dev/null +++ b/internal/provider/cisco/nxos/testdata/lldp_if_items.json.txt @@ -0,0 +1,6 @@ +interface ethernet 7/1 + no lldp receive + no lldp transmit + +interface ethernet 8/1 + no lldp transmit diff --git a/internal/provider/cisco/nxos/testdata/pc.json b/internal/provider/cisco/nxos/testdata/pc.json index 92c4b3011..4e628cf62 100644 --- a/internal/provider/cisco/nxos/testdata/pc.json +++ b/internal/provider/cisco/nxos/testdata/pc.json @@ -15,14 +15,7 @@ "pcMode": "active", "nativeVlan": "vlan-1", "suspIndividual": "enable", - "userCfgdFlags": "admin_state", - "rsmbrIfs-items": { - "RsMbrIfs-list": [ - { - "tDn": "/System/intf-items/phys-items/PhysIf-list[id='eth1/10']" - } - ] - } + "userCfgdFlags": "admin_state" } ] } diff --git a/internal/provider/cisco/nxos/testdata/pc.json.txt b/internal/provider/cisco/nxos/testdata/pc.json.txt index 95f91362b..1272d9903 100644 --- a/internal/provider/cisco/nxos/testdata/pc.json.txt +++ b/internal/provider/cisco/nxos/testdata/pc.json.txt @@ -1,6 +1,3 @@ -interface Ethernet1/10 - channel-group 10 mode active - interface Port-channel10 description vPC Leaf1 --> Host1 switchport mode trunk diff --git a/internal/provider/cisco/nxos/testdata/pc_lacp.json b/internal/provider/cisco/nxos/testdata/pc_lacp.json index 8ea2e469e..081fec8f1 100644 --- a/internal/provider/cisco/nxos/testdata/pc_lacp.json +++ b/internal/provider/cisco/nxos/testdata/pc_lacp.json @@ -15,14 +15,7 @@ "pcMode": "active", "nativeVlan": "vlan-1", "suspIndividual": "disable", - "userCfgdFlags": "admin_state", - "rsmbrIfs-items": { - "RsMbrIfs-list": [ - { - "tDn": "/System/intf-items/phys-items/PhysIf-list[id='eth1/1']" - } - ] - } + "userCfgdFlags": "admin_state" } ] } diff --git a/internal/provider/cisco/nxos/testdata/pc_member.json b/internal/provider/cisco/nxos/testdata/pc_member.json new file mode 100644 index 000000000..65b7f946d --- /dev/null +++ b/internal/provider/cisco/nxos/testdata/pc_member.json @@ -0,0 +1,18 @@ +{ + "intf-items": { + "aggr-items": { + "AggrIf-list": [ + { + "id": "po10", + "rsmbrIfs-items": { + "RsMbrIfs-list": [ + { + "tDn": "/System/intf-items/phys-items/PhysIf-list[id='eth1/10']" + } + ] + } + } + ] + } + } +} diff --git a/internal/provider/cisco/nxos/testdata/pc_member.json.txt b/internal/provider/cisco/nxos/testdata/pc_member.json.txt new file mode 100644 index 000000000..f3b81ba64 --- /dev/null +++ b/internal/provider/cisco/nxos/testdata/pc_member.json.txt @@ -0,0 +1,2 @@ +interface Ethernet1/10 + channel-group 10 mode active diff --git a/internal/provider/cisco/nxos/testdata/user.json b/internal/provider/cisco/nxos/testdata/user.json index 6909e6290..505fda96f 100644 --- a/internal/provider/cisco/nxos/testdata/user.json +++ b/internal/provider/cisco/nxos/testdata/user.json @@ -11,20 +11,6 @@ "pwdEncryptType": "clear", "sshauth-items": { "data": "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDEgsAKZn/hxPMKyfwKboiOEeuL9bTqW79QfEQ8h0kpGhkFJJEWR1e3BvXpdT9KYQOaKQnNw32atULweSQQNGh6S73FvEIwYViuNCmygDxpiaJLIiYHAfs3NQ8wGG70l+DK6vPhkcO6uvq2XRP+y1W9gMAKlgMPj5BCl2LR6HUO9/Jzvi1yRX4w4E5shpvcVoUUB8ubFJ0IyfMTXb/sQrFvjq4ukH3wAV4CMrsP6fj5FoAQzJw3jlK5GCtK8FqkUkROBexwWGbFFjSbox5KXT2qludLocyQtw10rB6G/3af40tQJLHd0u6LnaCgHGfPod3Z9u2aL6DR1k5hBtGXGWxZ IronCore Test" - }, - "userdomain-items": { - "UserDomain-list": [ - { - "name": "all", - "role-items": { - "UserRole-list": [ - { - "name": "network-admin" - } - ] - } - } - ] } } ] diff --git a/internal/provider/cisco/nxos/testdata/user.json.txt b/internal/provider/cisco/nxos/testdata/user.json.txt index def4becb0..c17621cef 100644 --- a/internal/provider/cisco/nxos/testdata/user.json.txt +++ b/internal/provider/cisco/nxos/testdata/user.json.txt @@ -1,2 +1,2 @@ -username johndoe password 0 Pa$$w0rd pbkdf2 role network-admin +username johndoe password 0 Pa$$w0rd pbkdf2 username johndoe sshkey ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDEgsAKZn/hxPMKyfwKboiOEeuL9bTqW79QfEQ8h0kpGhkFJJEWR1e3BvXpdT9KYQOaKQnNw32atULweSQQNGh6S73FvEIwYViuNCmygDxpiaJLIiYHAfs3NQ8wGG70l+DK6vPhkcO6uvq2XRP+y1W9gMAKlgMPj5BCl2LR6HUO9/Jzvi1yRX4w4E5shpvcVoUUB8ubFJ0IyfMTXb/sQrFvjq4ukH3wAV4CMrsP6fj5FoAQzJw3jlK5GCtK8FqkUkROBexwWGbFFjSbox5KXT2qludLocyQtw10rB6G/3af40tQJLHd0u6LnaCgHGfPod3Z9u2aL6DR1k5hBtGXGWxZ IronCore Test diff --git a/internal/provider/cisco/nxos/testdata/user_role.json b/internal/provider/cisco/nxos/testdata/user_role.json new file mode 100644 index 000000000..f50a9a0fc --- /dev/null +++ b/internal/provider/cisco/nxos/testdata/user_role.json @@ -0,0 +1,25 @@ +{ + "userext-items": { + "user-items": { + "User-list": [ + { + "name": "johndoe", + "userdomain-items": { + "UserDomain-list": [ + { + "name": "all", + "role-items": { + "UserRole-list": [ + { + "name": "network-admin" + } + ] + } + } + ] + } + } + ] + } + } +} diff --git a/internal/provider/cisco/nxos/testdata/user_role.json.txt b/internal/provider/cisco/nxos/testdata/user_role.json.txt new file mode 100644 index 000000000..513cef4aa --- /dev/null +++ b/internal/provider/cisco/nxos/testdata/user_role.json.txt @@ -0,0 +1 @@ +username johndoe role network-admin diff --git a/internal/provider/cisco/nxos/user.go b/internal/provider/cisco/nxos/user.go index 74a7141f7..62559fd9b 100644 --- a/internal/provider/cisco/nxos/user.go +++ b/internal/provider/cisco/nxos/user.go @@ -18,7 +18,11 @@ import ( "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) -var _ gnmiext.DataElement = (*User)(nil) +var ( + _ gnmiext.DataElement = (*User)(nil) + _ gnmiext.DataElement = (*UserRoleItems)(nil) + _ gnmiext.DataElement = (*UserRole)(nil) +) // User represents a local user on a NX-OS device. type User struct { @@ -31,9 +35,6 @@ type User struct { SshauthItems struct { Data string `json:"data,omitempty"` } `json:"sshauth-items,omitzero"` - UserdomainItems struct { - UserDomainList gnmiext.List[string, *UserDomain] `json:"UserDomain-list,omitzero"` - } `json:"userdomain-items,omitzero"` } func (*User) IsListItem() {} @@ -57,21 +58,29 @@ func (u *User) SetPassword(password string, encoder Encoder) error { return nil } -type UserDomain struct { - Name string `json:"name"` - RoleItems struct { - UserRoleList gnmiext.List[string, *UserRole] `json:"UserRole-list,omitzero"` - } `json:"role-items,omitzero"` +// UserRoleItems is the list container for fetching roles under a user domain. +type UserRoleItems struct { + Username string `json:"-"` + UserRoleList gnmiext.List[string, *UserRole] `json:"UserRole-list,omitzero"` } -func (d *UserDomain) Key() string { return d.Name } +func (u *UserRoleItems) XPath() string { + return "System/userext-items/user-items/User-list[name=" + u.Username + "]/userdomain-items/UserDomain-list[name=all]/role-items" +} type UserRole struct { - Name string `json:"name"` + Username string `json:"-"` + Name string `json:"name"` } +func (*UserRole) IsListItem() {} + func (r *UserRole) Key() string { return r.Name } +func (r *UserRole) XPath() string { + return "System/userext-items/user-items/User-list[name=" + r.Username + "]/userdomain-items/UserDomain-list[name=all]/role-items/UserRole-list[name=" + r.Name + "]" +} + type PwdEncryptType string const ( diff --git a/internal/provider/cisco/nxos/user_test.go b/internal/provider/cisco/nxos/user_test.go index a704525f7..9fd57537f 100644 --- a/internal/provider/cisco/nxos/user_test.go +++ b/internal/provider/cisco/nxos/user_test.go @@ -56,8 +56,6 @@ func salt(t *testing.T, saltstr string) [10]byte { } func init() { - dom := &UserDomain{Name: "all"} - dom.RoleItems.UserRoleList.Set(&UserRole{Name: "network-admin"}) user := &User{ AllowExpired: "no", Expiration: "never", @@ -67,6 +65,7 @@ func init() { PwdEncryptType: PwdEncryptTypeClear, } user.SshauthItems.Data = "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDEgsAKZn/hxPMKyfwKboiOEeuL9bTqW79QfEQ8h0kpGhkFJJEWR1e3BvXpdT9KYQOaKQnNw32atULweSQQNGh6S73FvEIwYViuNCmygDxpiaJLIiYHAfs3NQ8wGG70l+DK6vPhkcO6uvq2XRP+y1W9gMAKlgMPj5BCl2LR6HUO9/Jzvi1yRX4w4E5shpvcVoUUB8ubFJ0IyfMTXb/sQrFvjq4ukH3wAV4CMrsP6fj5FoAQzJw3jlK5GCtK8FqkUkROBexwWGbFFjSbox5KXT2qludLocyQtw10rB6G/3af40tQJLHd0u6LnaCgHGfPod3Z9u2aL6DR1k5hBtGXGWxZ IronCore Test" - user.UserdomainItems.UserDomainList.Set(dom) Register("user", user) + + Register("user_role", &UserRole{Username: "johndoe", Name: "network-admin"}) } diff --git a/test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/vpcdomain.txtar b/test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/vpcdomain.txtar index 160f1e330..2e1572b1b 100644 --- a/test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/vpcdomain.txtar +++ b/test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/vpcdomain.txtar @@ -104,13 +104,6 @@ spec: "nativeVlan": "vlan-1", "suspIndividual": "enable", "userCfgdFlags": "admin_layer,admin_state", - "rsmbrIfs-items": { - "RsMbrIfs-list": [ - { - "tDn": "/System/intf-items/phys-items/PhysIf-list[id='eth1/1']" - } - ] - }, "aggrExtd-items": { "bufferBoost": "enable" } From c5dd542b7fbc56087dde7ef644b03ce4ff30fb18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20K=C3=A4stner?= Date: Tue, 15 Sep 2026 11:19:32 +0200 Subject: [PATCH 2/4] Normalize anycast gateway MAC to uppercase in NX-OS NVE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Felix Kästner --- internal/provider/cisco/nxos/provider.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/provider/cisco/nxos/provider.go b/internal/provider/cisco/nxos/provider.go index 0890b3484..f4b12e3f3 100644 --- a/internal/provider/cisco/nxos/provider.go +++ b/internal/provider/cisco/nxos/provider.go @@ -3576,7 +3576,7 @@ func (p *Provider) EnsureNVE(ctx context.Context, req *provider.NVERequest) erro ag.AdminSt = AdminStDisabled if req.NVE.Spec.AnycastGateway != nil { ag.AdminSt = AdminStEnabled - ag.Address = req.NVE.Spec.AnycastGateway.VirtualMAC + ag.Address = strings.ToUpper(req.NVE.Spec.AnycastGateway.VirtualMAC) } sb.Patch(ag) From f42377b5819bc8a1fe7e5697bd7b18fa2a92702c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20K=C3=A4stner?= Date: Tue, 15 Sep 2026 11:19:55 +0200 Subject: [PATCH 3/4] Normalize VPC keepalive addresses to prefix notation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some NX-OS platforms return keepalive source and destination addresses with a /32 prefix (e.g. "10.1.1.1/32") while others return them as plain addresses (e.g. "10.1.1.1"). This inconsistency causes a false diff on every reconcile, triggering unnecessary gNMI Set requests. Introduce a Prefix type that normalizes both formats: UnmarshalText infers /32 for IPv4 (or /128 for IPv6) when the mask is absent, and MarshalText always includes it. Use this type for the VPC keepalive DestIP and SrcIP fields so the provider is idempotent regardless of platform behavior. Signed-off-by: Felix Kästner --- internal/provider/cisco/nxos/provider.go | 4 +- .../cisco/nxos/testdata/vpc_domain.json | 4 +- internal/provider/cisco/nxos/vpc.go | 49 ++++++++++++++++++- internal/provider/cisco/nxos/vpc_test.go | 6 ++- .../vpcdomain.txtar | 4 +- 5 files changed, 57 insertions(+), 10 deletions(-) diff --git a/internal/provider/cisco/nxos/provider.go b/internal/provider/cisco/nxos/provider.go index f4b12e3f3..f70783e6e 100644 --- a/internal/provider/cisco/nxos/provider.go +++ b/internal/provider/cisco/nxos/provider.go @@ -3220,8 +3220,8 @@ func (p *Provider) EnsureVPCDomain(ctx context.Context, vpcdomain *nxv1alpha1.VP v.AutoRecoveryReloadDelay = vpcdomain.Spec.Peer.AutoRecovery.ReloadDelay } - v.KeepAliveItems.DestIP = vpcdomain.Spec.Peer.KeepAlive.Destination - v.KeepAliveItems.SrcIP = vpcdomain.Spec.Peer.KeepAlive.Source + v.KeepAliveItems.DestIP = Prefix(netip.PrefixFrom(netip.MustParseAddr(vpcdomain.Spec.Peer.KeepAlive.Destination), 32)) + v.KeepAliveItems.SrcIP = Prefix(netip.PrefixFrom(netip.MustParseAddr(vpcdomain.Spec.Peer.KeepAlive.Source), 32)) v.KeepAliveItems.VRF = vpcdomain.Spec.Peer.KeepAlive.VrfName if vrf != nil { v.KeepAliveItems.VRF = vrf.Spec.Name diff --git a/internal/provider/cisco/nxos/testdata/vpc_domain.json b/internal/provider/cisco/nxos/testdata/vpc_domain.json index 0310fe28f..56df8f72b 100644 --- a/internal/provider/cisco/nxos/testdata/vpc_domain.json +++ b/internal/provider/cisco/nxos/testdata/vpc_domain.json @@ -15,8 +15,8 @@ "rolePrio": 100, "sysPrio": 10, "keepalive-items": { - "destIp": "10.114.235.156", - "srcIp": "10.114.235.155", + "destIp": "10.114.235.156/32", + "srcIp": "10.114.235.155/32", "vrf": "management", "peerlink-items": { "adminSt": "enabled", diff --git a/internal/provider/cisco/nxos/vpc.go b/internal/provider/cisco/nxos/vpc.go index 4d2206b43..3cea874b7 100644 --- a/internal/provider/cisco/nxos/vpc.go +++ b/internal/provider/cisco/nxos/vpc.go @@ -4,7 +4,10 @@ package nxos import ( + "bytes" + "encoding" "fmt" + "net/netip" "regexp" "slices" "strconv" @@ -34,8 +37,8 @@ type VPCDomain struct { RolePrio int32 `json:"rolePrio"` SysPrio int32 `json:"sysPrio"` KeepAliveItems struct { - DestIP string `json:"destIp"` - SrcIP string `json:"srcIp"` + DestIP Prefix `json:"destIp"` + SrcIP Prefix `json:"srcIp"` VRF string `json:"vrf"` PeerLinkItems struct { AdminSt AdminSt `json:"adminSt"` @@ -136,3 +139,45 @@ func (v *VPCIfItems) GetListItemByInterface(name string) *VPCIf { } return nil } + +var ( + _ encoding.TextMarshaler = Prefix{} + _ encoding.TextUnmarshaler = (*Prefix)(nil) +) + +type Prefix netip.Prefix + +// MarshalText implements the [encoding.TextMarshaler] interface. +func (p Prefix) MarshalText() ([]byte, error) { + return netip.Prefix(p).MarshalText() +} + +// UnmarshalText implements the [encoding.TextUnmarshaler] interface. +func (p *Prefix) UnmarshalText(text []byte) error { + if len(text) == 0 { + *p = Prefix{} + return nil + } + found := bytes.Contains(text, []byte{'/'}) + if !found { + addr, err := netip.ParseAddr(string(text)) + if err != nil { + return fmt.Errorf("failed to parse address: %w", err) + } + if addr.Is4() { + *p = Prefix(netip.PrefixFrom(addr, 32)) + return nil + } + if addr.Is6() { + *p = Prefix(netip.PrefixFrom(addr, 128)) + return nil + } + return fmt.Errorf("failed to infer prefix length from address: %s", addr.String()) + } + np, err := netip.ParsePrefix(string(text)) + if err != nil { + return fmt.Errorf("failed to parse prefix: %w", err) + } + *p = Prefix(np) + return nil +} diff --git a/internal/provider/cisco/nxos/vpc_test.go b/internal/provider/cisco/nxos/vpc_test.go index c7c442901..a88764b1d 100644 --- a/internal/provider/cisco/nxos/vpc_test.go +++ b/internal/provider/cisco/nxos/vpc_test.go @@ -3,6 +3,8 @@ package nxos +import "net/netip" + func init() { vd := &VPCDomain{ AdminSt: AdminStEnabled, @@ -18,8 +20,8 @@ func init() { RolePrio: 100, SysPrio: 10, } - vd.KeepAliveItems.DestIP = "10.114.235.156" - vd.KeepAliveItems.SrcIP = "10.114.235.155" + vd.KeepAliveItems.DestIP = Prefix(netip.MustParsePrefix("10.114.235.156/32")) + vd.KeepAliveItems.SrcIP = Prefix(netip.MustParsePrefix("10.114.235.155/32")) vd.KeepAliveItems.VRF = ManagementVRFName vd.KeepAliveItems.PeerLinkItems.AdminSt = AdminStEnabled vd.KeepAliveItems.PeerLinkItems.ID = "po1" diff --git a/test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/vpcdomain.txtar b/test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/vpcdomain.txtar index 2e1572b1b..218e52063 100644 --- a/test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/vpcdomain.txtar +++ b/test/gnmi/testdata/nx.cisco.networking.metal.ironcore.dev/vpcdomain.txtar @@ -155,8 +155,8 @@ spec: "rolePrio": 32667, "sysPrio": 32667, "keepalive-items": { - "destIp": "10.0.0.2", - "srcIp": "10.0.0.1", + "destIp": "10.0.0.2/32", + "srcIp": "10.0.0.1/32", "vrf": "management", "peerlink-items": { "adminSt": "enabled", From 193795f3162ca0dc055ce5a72e4ce28585f87ef7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20K=C3=A4stner?= Date: Wed, 16 Sep 2026 17:03:03 +0200 Subject: [PATCH 4/4] Document no-list-fields-in-Patch guideline in AGENTS.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Felix Kästner --- AGENTS.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index c31b258dc..24d5b4a03 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -137,6 +137,10 @@ A provider's `EnsureX` methods must be safe to call on every reconciliation with The `gnmiext` package already implements a Get-and-Check approach: it diffs current device state against the desired configuration and only performs a gNMI Set when a real change is needed. This makes it safe for periodic reconciliation. +**No list fields in Patched structs:** + +`Patch` (gNMI update) merges into existing device config — it adds or updates list entries but never removes them. A `DataElement` passed to `Patch` must not contain `gnmiext.List` or slice fields. Instead, manage list entries as individual `DataElement`s: fetch current entries with a container struct via `GetConfig`, `Patch` each desired entry, and `Delete` stale entries not in the desired set. + **Platform default values — critical pitfall:** Optional fields in the API spec that map to optional fields in the provider Go struct require special handling: