From ed17105ca1901ea1c94a7ddceba2db9a366fe4ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20K=C3=A4stner?= Date: Thu, 2 Apr 2026 15:32:54 +0200 Subject: [PATCH 1/4] Add terminal error interceptor to prevent retrying non-retryable errors When a gRPC call returns a status code that indicates a permanent failure (e.g. InvalidArgument, NotFound, PermissionDenied, Unauthenticated), retrying the request will not resolve the issue. Without marking these errors as terminal, the controller-runtime reconciler keeps requeuing the reconciliation indefinitely, wasting resources. Introduce a TerminalErrorInterceptor that wraps errors with non-retryable gRPC status codes as reconcile.TerminalError so the reconciler stops retrying immediately. Also fix error wrapping in GetDeviceByName which previously dropped the underlying error, and add documentation to GetDeviceBySerial. --- internal/deviceutil/deviceutil.go | 48 +++++++++++++++++++++++++++---- 1 file changed, 43 insertions(+), 5 deletions(-) diff --git a/internal/deviceutil/deviceutil.go b/internal/deviceutil/deviceutil.go index f764f7906..55ad8d4c3 100644 --- a/internal/deviceutil/deviceutil.go +++ b/internal/deviceutil/deviceutil.go @@ -8,16 +8,20 @@ import ( "crypto/x509" "errors" "fmt" + "slices" "time" "google.golang.org/grpc" + "google.golang.org/grpc/codes" "google.golang.org/grpc/credentials" "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/status" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime/schema" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/ironcore-dev/network-operator/api/core/v1alpha1" "github.com/ironcore-dev/network-operator/internal/clientutil" @@ -58,17 +62,19 @@ func GetOwnerDevice(ctx context.Context, r client.Reader, obj metav1.Object) (*v func GetDeviceByName(ctx context.Context, r client.Reader, namespace, name string) (*v1alpha1.Device, error) { obj := new(v1alpha1.Device) if err := r.Get(ctx, client.ObjectKey{Namespace: namespace, Name: name}, obj); err != nil { - return nil, fmt.Errorf("failed to get %s/%s", v1alpha1.GroupVersion.WithKind(v1alpha1.DeviceKind).String(), name) + return nil, fmt.Errorf("failed to get %s/%s: %w", v1alpha1.GroupVersion.WithKind(v1alpha1.DeviceKind).String(), name, err) } return obj, nil } +// GetDeviceBySerial finds and returns a Device object using the specified serial number. +// It returns an error if no device or multiple devices with the same serial number are found. +// Note: This function assumes that the [v1alpha1.DeviceSerialLabel] is unique across all Device objects in the cluster. func GetDeviceBySerial(ctx context.Context, r client.Reader, namespace, serial string) (*v1alpha1.Device, error) { deviceList := &v1alpha1.DeviceList{} listOpts := &client.ListOptions{ LabelSelector: labels.SelectorFromSet(labels.Set{v1alpha1.DeviceSerialLabel: serial}), } - if err := r.List(ctx, deviceList, listOpts); err != nil { return nil, fmt.Errorf("failed to list %s objects: %w", v1alpha1.GroupVersion.WithKind(v1alpha1.DeviceKind).String(), err) } @@ -82,8 +88,6 @@ func GetDeviceBySerial(ctx context.Context, r client.Reader, namespace, serial s } // Connection holds the necessary information to connect to a device's API. -// -// TODO(felix-kaestner): find a better place for this struct, maybe in a 'connection' package? type Connection struct { // Address is the API address of the device, in the format "host:port". Address string @@ -150,7 +154,7 @@ func NewGrpcClient(ctx context.Context, conn *Connection, o ...Option) (*grpc.Cl creds = credentials.NewTLS(conn.TLS) } - opts := []grpc.DialOption{grpc.WithTransportCredentials(creds)} + opts := []grpc.DialOption{grpc.WithTransportCredentials(creds), grpc.WithUnaryInterceptor(TerminalErrorInterceptor())} if conn.Username != "" && conn.Password != "" { opts = append(opts, grpc.WithPerRPCCredentials(&auth{ Username: conn.Username, @@ -216,3 +220,37 @@ func UnaryDefaultTimeoutInterceptor(timeout time.Duration) grpc.UnaryClientInter return invoker(ctx, method, req, reply, cc, opts...) } } + +// TerminalErrorInterceptor returns a gRPC unary client interceptor that wraps errors returned by the gRPC invoker +// as terminal errors if their gRPC status code is in the set of non-retryable codes defined in [terminalCodes]. +func TerminalErrorInterceptor() grpc.UnaryClientInterceptor { + return func(ctx context.Context, method string, req, reply any, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error { + return WrapTerminalError(invoker(ctx, method, req, reply, cc, opts...)) + } +} + +// WrapTerminalError wraps the given error as a terminal error if its gRPC status error +// with a non-retryable code. +func WrapTerminalError(err error) error { + if statusErr, ok := status.FromError(err); ok && slices.Contains(terminalCodes, statusErr.Code()) { + return reconcile.TerminalError(err) + } + return err +} + +// terminalCodes holds the set of gRPC codes that are considered terminal. +// That is, if an error has one of these codes, retrying the operation +// is not expected to succeed. +// This list is based on the gRPC documentation at https://grpc.io/docs/guides/status-codes. +var terminalCodes = []codes.Code{ + codes.Unknown, + codes.InvalidArgument, + codes.NotFound, + codes.AlreadyExists, + codes.PermissionDenied, + codes.FailedPrecondition, + codes.OutOfRange, + codes.Unimplemented, + codes.DataLoss, + codes.Unauthenticated, +} From ca180ff9a8e52176f3b072f0ac5b6ca0c38a5be8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20K=C3=A4stner?= Date: Thu, 2 Apr 2026 13:31:01 +0200 Subject: [PATCH 2/4] Move `gnmiext` into new `transport` package As we plan to use our `gnmiext` package also for other provider implementations of other hardware vendors than Cisco, we now move it outside of the cisco provider package scope. With this change we are introducing a new `transport` package which will house multiple packages related to transport/protocol specific utilities and clients. --- internal/provider/cisco/iosxr/intf.go | 2 +- internal/provider/cisco/iosxr/provider.go | 2 +- internal/provider/cisco/iosxr/provider_test.go | 2 +- internal/provider/cisco/nxos/acl.go | 2 +- internal/provider/cisco/nxos/banner.go | 2 +- internal/provider/cisco/nxos/bgp.go | 2 +- internal/provider/cisco/nxos/bgw.go | 2 +- internal/provider/cisco/nxos/cert.go | 2 +- internal/provider/cisco/nxos/dhcprelay.go | 2 +- internal/provider/cisco/nxos/dns.go | 2 +- internal/provider/cisco/nxos/evi.go | 2 +- internal/provider/cisco/nxos/feat.go | 2 +- internal/provider/cisco/nxos/grpc.go | 2 +- internal/provider/cisco/nxos/intf.go | 2 +- internal/provider/cisco/nxos/isis.go | 2 +- internal/provider/cisco/nxos/lldp.go | 2 +- internal/provider/cisco/nxos/ntp.go | 2 +- internal/provider/cisco/nxos/nve.go | 2 +- internal/provider/cisco/nxos/ospf.go | 2 +- internal/provider/cisco/nxos/pim.go | 2 +- internal/provider/cisco/nxos/prefix.go | 2 +- internal/provider/cisco/nxos/provider.go | 2 +- internal/provider/cisco/nxos/provider_test.go | 2 +- internal/provider/cisco/nxos/routemap.go | 2 +- internal/provider/cisco/nxos/snmp.go | 2 +- internal/provider/cisco/nxos/syslog.go | 2 +- internal/provider/cisco/nxos/system.go | 2 +- internal/provider/cisco/nxos/term.go | 2 +- internal/provider/cisco/nxos/user.go | 2 +- internal/provider/cisco/nxos/version.go | 2 +- internal/provider/cisco/nxos/vlan.go | 2 +- internal/provider/cisco/nxos/vpc.go | 2 +- internal/provider/cisco/nxos/vrf.go | 2 +- .../cisco/gnmiext/v2 => transport/gnmiext}/client.go | 4 +++- .../cisco/gnmiext/v2 => transport/gnmiext}/client_test.go | 0 .../{provider/cisco/gnmiext/v2 => transport/gnmiext}/doc.go | 0 .../cisco/gnmiext/v2 => transport/gnmiext}/empty.go | 6 +++++- .../cisco/gnmiext/v2 => transport/gnmiext}/empty_test.go | 0 .../cisco/gnmiext/v2 => transport/gnmiext}/list.go | 0 .../cisco/gnmiext/v2 => transport/gnmiext}/list_test.go | 0 40 files changed, 41 insertions(+), 35 deletions(-) rename internal/{provider/cisco/gnmiext/v2 => transport/gnmiext}/client.go (98%) rename internal/{provider/cisco/gnmiext/v2 => transport/gnmiext}/client_test.go (100%) rename internal/{provider/cisco/gnmiext/v2 => transport/gnmiext}/doc.go (100%) rename internal/{provider/cisco/gnmiext/v2 => transport/gnmiext}/empty.go (89%) rename internal/{provider/cisco/gnmiext/v2 => transport/gnmiext}/empty_test.go (100%) rename internal/{provider/cisco/gnmiext/v2 => transport/gnmiext}/list.go (100%) rename internal/{provider/cisco/gnmiext/v2 => transport/gnmiext}/list_test.go (100%) diff --git a/internal/provider/cisco/iosxr/intf.go b/internal/provider/cisco/iosxr/intf.go index 2f45c645a..b5d619fc8 100644 --- a/internal/provider/cisco/iosxr/intf.go +++ b/internal/provider/cisco/iosxr/intf.go @@ -7,7 +7,7 @@ import ( "fmt" "regexp" - "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" + "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) type PhysIf struct { diff --git a/internal/provider/cisco/iosxr/provider.go b/internal/provider/cisco/iosxr/provider.go index 7bc3c4b21..8378e1c87 100644 --- a/internal/provider/cisco/iosxr/provider.go +++ b/internal/provider/cisco/iosxr/provider.go @@ -11,7 +11,7 @@ import ( "github.com/ironcore-dev/network-operator/internal/deviceutil" "github.com/ironcore-dev/network-operator/internal/provider" - "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" + "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" "github.com/ironcore-dev/network-operator/api/core/v1alpha1" diff --git a/internal/provider/cisco/iosxr/provider_test.go b/internal/provider/cisco/iosxr/provider_test.go index 55e90be3b..56ef9f818 100644 --- a/internal/provider/cisco/iosxr/provider_test.go +++ b/internal/provider/cisco/iosxr/provider_test.go @@ -16,7 +16,7 @@ import ( "github.com/ironcore-dev/network-operator/api/core/v1alpha1" "github.com/ironcore-dev/network-operator/internal/provider" - "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" + "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) type TestCase struct { diff --git a/internal/provider/cisco/nxos/acl.go b/internal/provider/cisco/nxos/acl.go index a1f27c15a..9434ab372 100644 --- a/internal/provider/cisco/nxos/acl.go +++ b/internal/provider/cisco/nxos/acl.go @@ -7,7 +7,7 @@ import ( "fmt" "github.com/ironcore-dev/network-operator/api/core/v1alpha1" - "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" + "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) var _ gnmiext.Configurable = (*ACL)(nil) diff --git a/internal/provider/cisco/nxos/banner.go b/internal/provider/cisco/nxos/banner.go index 3a8d1e65b..010fd8ce7 100644 --- a/internal/provider/cisco/nxos/banner.go +++ b/internal/provider/cisco/nxos/banner.go @@ -7,7 +7,7 @@ import ( "fmt" "github.com/ironcore-dev/network-operator/api/core/v1alpha1" - "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" + "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) var ( diff --git a/internal/provider/cisco/nxos/bgp.go b/internal/provider/cisco/nxos/bgp.go index 61ed49429..490b5866c 100644 --- a/internal/provider/cisco/nxos/bgp.go +++ b/internal/provider/cisco/nxos/bgp.go @@ -10,7 +10,7 @@ import ( nxv1alpha1 "github.com/ironcore-dev/network-operator/api/cisco/nx/v1alpha1" "github.com/ironcore-dev/network-operator/api/core/v1alpha1" - "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" + "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) var ( diff --git a/internal/provider/cisco/nxos/bgw.go b/internal/provider/cisco/nxos/bgw.go index 934b19988..ce371c737 100644 --- a/internal/provider/cisco/nxos/bgw.go +++ b/internal/provider/cisco/nxos/bgw.go @@ -4,7 +4,7 @@ package nxos import ( - "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" + "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) var _ gnmiext.Configurable = (*MultisiteItems)(nil) diff --git a/internal/provider/cisco/nxos/cert.go b/internal/provider/cisco/nxos/cert.go index 995ff2c76..357192ee4 100644 --- a/internal/provider/cisco/nxos/cert.go +++ b/internal/provider/cisco/nxos/cert.go @@ -15,7 +15,7 @@ import ( "github.com/openconfig/gnoi/cert" "google.golang.org/grpc" - "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" + "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) // Certificate represents a X.509 certificate and its associated private key. diff --git a/internal/provider/cisco/nxos/dhcprelay.go b/internal/provider/cisco/nxos/dhcprelay.go index 30e67ca75..41cb49bb9 100644 --- a/internal/provider/cisco/nxos/dhcprelay.go +++ b/internal/provider/cisco/nxos/dhcprelay.go @@ -6,7 +6,7 @@ package nxos import ( "net/netip" - "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" + "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) var _ gnmiext.Configurable = (*DHCPRelayConfig)(nil) diff --git a/internal/provider/cisco/nxos/dns.go b/internal/provider/cisco/nxos/dns.go index c1d51b61e..a418b34ab 100644 --- a/internal/provider/cisco/nxos/dns.go +++ b/internal/provider/cisco/nxos/dns.go @@ -3,7 +3,7 @@ package nxos -import "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" +import "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" var _ gnmiext.Configurable = (*DNS)(nil) diff --git a/internal/provider/cisco/nxos/evi.go b/internal/provider/cisco/nxos/evi.go index 9c2acb68d..6a4c1e076 100644 --- a/internal/provider/cisco/nxos/evi.go +++ b/internal/provider/cisco/nxos/evi.go @@ -11,7 +11,7 @@ import ( "strconv" "strings" - "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" + "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) var _ gnmiext.Configurable = (*BDEVI)(nil) diff --git a/internal/provider/cisco/nxos/feat.go b/internal/provider/cisco/nxos/feat.go index ef7752d14..93e83650d 100644 --- a/internal/provider/cisco/nxos/feat.go +++ b/internal/provider/cisco/nxos/feat.go @@ -3,7 +3,7 @@ package nxos -import "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" +import "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" var ( _ gnmiext.Configurable = (*Feature)(nil) diff --git a/internal/provider/cisco/nxos/grpc.go b/internal/provider/cisco/nxos/grpc.go index 31b85c378..34c5557d2 100644 --- a/internal/provider/cisco/nxos/grpc.go +++ b/internal/provider/cisco/nxos/grpc.go @@ -7,7 +7,7 @@ import ( "errors" "fmt" - "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" + "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) var ( diff --git a/internal/provider/cisco/nxos/intf.go b/internal/provider/cisco/nxos/intf.go index a5732cdac..cda069b41 100644 --- a/internal/provider/cisco/nxos/intf.go +++ b/internal/provider/cisco/nxos/intf.go @@ -13,7 +13,7 @@ import ( "strings" nxv1alpha1 "github.com/ironcore-dev/network-operator/api/cisco/nx/v1alpha1" - "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" + "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) var ( diff --git a/internal/provider/cisco/nxos/isis.go b/internal/provider/cisco/nxos/isis.go index 340f6b269..53d320da5 100644 --- a/internal/provider/cisco/nxos/isis.go +++ b/internal/provider/cisco/nxos/isis.go @@ -5,7 +5,7 @@ package nxos import ( "github.com/ironcore-dev/network-operator/api/core/v1alpha1" - "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" + "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) var _ gnmiext.Configurable = (*ISIS)(nil) diff --git a/internal/provider/cisco/nxos/lldp.go b/internal/provider/cisco/nxos/lldp.go index 18a334f43..412f19eca 100644 --- a/internal/provider/cisco/nxos/lldp.go +++ b/internal/provider/cisco/nxos/lldp.go @@ -3,7 +3,7 @@ package nxos -import "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" +import "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" var _ gnmiext.Configurable = (*LLDP)(nil) diff --git a/internal/provider/cisco/nxos/ntp.go b/internal/provider/cisco/nxos/ntp.go index a4b297bf7..99e1d091d 100644 --- a/internal/provider/cisco/nxos/ntp.go +++ b/internal/provider/cisco/nxos/ntp.go @@ -3,7 +3,7 @@ package nxos -import "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" +import "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" var ( _ gnmiext.Configurable = (*NTP)(nil) diff --git a/internal/provider/cisco/nxos/nve.go b/internal/provider/cisco/nxos/nve.go index 8d4ed6b02..3573eacc9 100644 --- a/internal/provider/cisco/nxos/nve.go +++ b/internal/provider/cisco/nxos/nve.go @@ -7,7 +7,7 @@ import ( "encoding/json" "strconv" - "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" + "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) var ( diff --git a/internal/provider/cisco/nxos/ospf.go b/internal/provider/cisco/nxos/ospf.go index db4a12252..f5556ba92 100644 --- a/internal/provider/cisco/nxos/ospf.go +++ b/internal/provider/cisco/nxos/ospf.go @@ -7,7 +7,7 @@ import ( "time" "github.com/ironcore-dev/network-operator/api/core/v1alpha1" - "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" + "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) var _ gnmiext.Configurable = (*OSPF)(nil) diff --git a/internal/provider/cisco/nxos/pim.go b/internal/provider/cisco/nxos/pim.go index 45adfb59f..2bb76b94d 100644 --- a/internal/provider/cisco/nxos/pim.go +++ b/internal/provider/cisco/nxos/pim.go @@ -3,7 +3,7 @@ package nxos -import "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" +import "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" var ( _ gnmiext.Configurable = (*PIM)(nil) diff --git a/internal/provider/cisco/nxos/prefix.go b/internal/provider/cisco/nxos/prefix.go index ea03c3213..6aa56ea8b 100644 --- a/internal/provider/cisco/nxos/prefix.go +++ b/internal/provider/cisco/nxos/prefix.go @@ -4,7 +4,7 @@ package nxos import ( - "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" + "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) var _ gnmiext.Configurable = (*PrefixList)(nil) diff --git a/internal/provider/cisco/nxos/provider.go b/internal/provider/cisco/nxos/provider.go index dee687708..fa1602473 100644 --- a/internal/provider/cisco/nxos/provider.go +++ b/internal/provider/cisco/nxos/provider.go @@ -29,7 +29,7 @@ import ( "github.com/ironcore-dev/network-operator/api/core/v1alpha1" "github.com/ironcore-dev/network-operator/internal/deviceutil" "github.com/ironcore-dev/network-operator/internal/provider" - "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" + "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) var ( diff --git a/internal/provider/cisco/nxos/provider_test.go b/internal/provider/cisco/nxos/provider_test.go index 4209071d2..719662dd1 100644 --- a/internal/provider/cisco/nxos/provider_test.go +++ b/internal/provider/cisco/nxos/provider_test.go @@ -15,7 +15,7 @@ import ( "github.com/google/go-cmp/cmp/cmpopts" "github.com/tidwall/gjson" - "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" + "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) type TestCase struct { diff --git a/internal/provider/cisco/nxos/routemap.go b/internal/provider/cisco/nxos/routemap.go index 70b140801..757ab5486 100644 --- a/internal/provider/cisco/nxos/routemap.go +++ b/internal/provider/cisco/nxos/routemap.go @@ -5,7 +5,7 @@ package nxos import ( "github.com/ironcore-dev/network-operator/api/core/v1alpha1" - "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" + "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) var _ gnmiext.Configurable = (*RouteMap)(nil) diff --git a/internal/provider/cisco/nxos/snmp.go b/internal/provider/cisco/nxos/snmp.go index a5d16733f..f4e97d978 100644 --- a/internal/provider/cisco/nxos/snmp.go +++ b/internal/provider/cisco/nxos/snmp.go @@ -6,7 +6,7 @@ package nxos import ( "strconv" - "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" + "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) var ( diff --git a/internal/provider/cisco/nxos/syslog.go b/internal/provider/cisco/nxos/syslog.go index fc8c0b7aa..b65d83baa 100644 --- a/internal/provider/cisco/nxos/syslog.go +++ b/internal/provider/cisco/nxos/syslog.go @@ -5,7 +5,7 @@ package nxos import ( "github.com/ironcore-dev/network-operator/api/core/v1alpha1" - "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" + "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) var ( diff --git a/internal/provider/cisco/nxos/system.go b/internal/provider/cisco/nxos/system.go index 47b68ba2e..b5cbf0ac9 100644 --- a/internal/provider/cisco/nxos/system.go +++ b/internal/provider/cisco/nxos/system.go @@ -15,7 +15,7 @@ import ( "github.com/openconfig/gnoi/factory_reset" "github.com/openconfig/gnoi/system" - "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" + "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) const Manufacturer = "Cisco" diff --git a/internal/provider/cisco/nxos/term.go b/internal/provider/cisco/nxos/term.go index 91b577952..086d9adba 100644 --- a/internal/provider/cisco/nxos/term.go +++ b/internal/provider/cisco/nxos/term.go @@ -6,7 +6,7 @@ package nxos import ( "fmt" - "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" + "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) var ( diff --git a/internal/provider/cisco/nxos/user.go b/internal/provider/cisco/nxos/user.go index 1bb4a99e7..d0b266ef8 100644 --- a/internal/provider/cisco/nxos/user.go +++ b/internal/provider/cisco/nxos/user.go @@ -15,7 +15,7 @@ import ( "github.com/go-crypt/crypt/algorithm/shacrypt" - "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" + "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) var _ gnmiext.Configurable = (*User)(nil) diff --git a/internal/provider/cisco/nxos/version.go b/internal/provider/cisco/nxos/version.go index c554b1a98..4e21fc9af 100644 --- a/internal/provider/cisco/nxos/version.go +++ b/internal/provider/cisco/nxos/version.go @@ -3,7 +3,7 @@ package nxos -import "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" +import "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" // Version represents the operating system version of the target device. type Version string diff --git a/internal/provider/cisco/nxos/vlan.go b/internal/provider/cisco/nxos/vlan.go index 22343f9fd..45f5aa775 100644 --- a/internal/provider/cisco/nxos/vlan.go +++ b/internal/provider/cisco/nxos/vlan.go @@ -6,7 +6,7 @@ package nxos import ( "encoding/json" - "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" + "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) var ( diff --git a/internal/provider/cisco/nxos/vpc.go b/internal/provider/cisco/nxos/vpc.go index 5307bb2d5..49e912d86 100644 --- a/internal/provider/cisco/nxos/vpc.go +++ b/internal/provider/cisco/nxos/vpc.go @@ -11,7 +11,7 @@ import ( "strings" "time" - "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" + "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) var ( diff --git a/internal/provider/cisco/nxos/vrf.go b/internal/provider/cisco/nxos/vrf.go index d9a7552d2..d6bf04a61 100644 --- a/internal/provider/cisco/nxos/vrf.go +++ b/internal/provider/cisco/nxos/vrf.go @@ -3,7 +3,7 @@ package nxos import ( - "github.com/ironcore-dev/network-operator/internal/provider/cisco/gnmiext/v2" + "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) const ( diff --git a/internal/provider/cisco/gnmiext/v2/client.go b/internal/transport/gnmiext/client.go similarity index 98% rename from internal/provider/cisco/gnmiext/v2/client.go rename to internal/transport/gnmiext/client.go index d66bfe823..3abc2b421 100644 --- a/internal/provider/cisco/gnmiext/v2/client.go +++ b/internal/transport/gnmiext/client.go @@ -18,6 +18,8 @@ import ( "github.com/openconfig/ygot/ygot" "github.com/tidwall/gjson" "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) // Configurable represents a configuration item with a YANG path. @@ -274,7 +276,7 @@ func (c *client) set(ctx context.Context, patch bool, conf ...Configurable) erro } got := cp.Deep(cf) err = c.GetConfig(ctx, got) - if err != nil && !errors.Is(err, ErrNil) { + if err != nil && !errors.Is(err, ErrNil) && status.Code(err) != codes.NotFound { return fmt.Errorf("gnmiext: failed to retrieve current config for %s: %w", cf.XPath(), err) } // If the current configuration is equal to the desired configuration, skip the update. diff --git a/internal/provider/cisco/gnmiext/v2/client_test.go b/internal/transport/gnmiext/client_test.go similarity index 100% rename from internal/provider/cisco/gnmiext/v2/client_test.go rename to internal/transport/gnmiext/client_test.go diff --git a/internal/provider/cisco/gnmiext/v2/doc.go b/internal/transport/gnmiext/doc.go similarity index 100% rename from internal/provider/cisco/gnmiext/v2/doc.go rename to internal/transport/gnmiext/doc.go diff --git a/internal/provider/cisco/gnmiext/v2/empty.go b/internal/transport/gnmiext/empty.go similarity index 89% rename from internal/provider/cisco/gnmiext/v2/empty.go rename to internal/transport/gnmiext/empty.go index 1bcca0843..24c1211a2 100644 --- a/internal/provider/cisco/gnmiext/v2/empty.go +++ b/internal/transport/gnmiext/empty.go @@ -6,6 +6,7 @@ package gnmiext import ( "encoding/json" "fmt" + "regexp" ) // NOTE: Use json.Marshaler and json.Unmarshaler interfaces instead of the @@ -18,6 +19,9 @@ var ( _ json.Unmarshaler = (*Empty)(nil) ) +// Due to some Cisco IOS-XR output we also match "[ \n null \n]" +var nullRe = regexp.MustCompile(`^\[\s*null\s*]$`) + // Empty represents the built-in "empty" type as defined in RFC 7951. // It differentiates between an existing empty value ([null]) and a // non-existing value (null). @@ -39,7 +43,7 @@ func (e *Empty) UnmarshalJSON(b []byte) error { *e = false return nil } - if string(b) != "[null]" { + if !nullRe.MatchString(string(b)) { return fmt.Errorf("gnmiext: invalid empty value: %s", string(b)) } *e = true diff --git a/internal/provider/cisco/gnmiext/v2/empty_test.go b/internal/transport/gnmiext/empty_test.go similarity index 100% rename from internal/provider/cisco/gnmiext/v2/empty_test.go rename to internal/transport/gnmiext/empty_test.go diff --git a/internal/provider/cisco/gnmiext/v2/list.go b/internal/transport/gnmiext/list.go similarity index 100% rename from internal/provider/cisco/gnmiext/v2/list.go rename to internal/transport/gnmiext/list.go diff --git a/internal/provider/cisco/gnmiext/v2/list_test.go b/internal/transport/gnmiext/list_test.go similarity index 100% rename from internal/provider/cisco/gnmiext/v2/list_test.go rename to internal/transport/gnmiext/list_test.go From 125e3a85c6a4a728cdb003d67039e2ea702e7450 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20K=C3=A4stner?= Date: Fri, 3 Apr 2026 20:30:23 +0200 Subject: [PATCH 3/4] Move gRPC transport code into new `transport/grpcext` package Extracts `NewClient`, `Option`, `WithDefaultTimeout`, the interceptors, and the `auth` type from `internal/deviceutil` into a dedicated `internal/transport/grpcext` package, mirroring the existing `internal/transport/gnmiext` sibling. `deviceutil` now only contains device CRD helpers and the `Connection` type. --- internal/deviceutil/deviceutil.go | 118 -------------------- internal/provider/cisco/iosxr/provider.go | 6 +- internal/provider/cisco/nxos/provider.go | 3 +- internal/provider/openconfig/provider.go | 3 +- internal/transport/grpcext/grpcext.go | 130 ++++++++++++++++++++++ 5 files changed, 137 insertions(+), 123 deletions(-) create mode 100644 internal/transport/grpcext/grpcext.go diff --git a/internal/deviceutil/deviceutil.go b/internal/deviceutil/deviceutil.go index 55ad8d4c3..472b6f426 100644 --- a/internal/deviceutil/deviceutil.go +++ b/internal/deviceutil/deviceutil.go @@ -8,20 +8,12 @@ import ( "crypto/x509" "errors" "fmt" - "slices" - "time" - "google.golang.org/grpc" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/credentials" - "google.golang.org/grpc/credentials/insecure" - "google.golang.org/grpc/status" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime/schema" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/ironcore-dev/network-operator/api/core/v1alpha1" "github.com/ironcore-dev/network-operator/internal/clientutil" @@ -144,113 +136,3 @@ func GetDeviceConnection(ctx context.Context, r client.Reader, obj *v1alpha1.Dev TLS: conf, }, nil } - -// NewGrpcClient creates a new gRPC client connection to a specified device using the provided [Connection]. -// The connection will use TLS if the [Connection.TLS] field is set, otherwise it will use an insecure connection. -// If the [Connection.Username] and [Connection.Password] fields are set, basic authentication in the form of metadata will be used. -func NewGrpcClient(ctx context.Context, conn *Connection, o ...Option) (*grpc.ClientConn, error) { - creds := insecure.NewCredentials() - if conn.TLS != nil { - creds = credentials.NewTLS(conn.TLS) - } - - opts := []grpc.DialOption{grpc.WithTransportCredentials(creds), grpc.WithUnaryInterceptor(TerminalErrorInterceptor())} - if conn.Username != "" && conn.Password != "" { - opts = append(opts, grpc.WithPerRPCCredentials(&auth{ - Username: conn.Username, - Password: conn.Password, - })) - } - - for _, opt := range o { - dialOpt, err := opt() - if err != nil { - return nil, err - } - opts = append(opts, dialOpt) - } - - return grpc.NewClient(conn.Address, opts...) -} - -type Option func() (grpc.DialOption, error) - -// WithDefaultTimeout returns a gRPC dial option that sets a default timeout for each RPC. -// If a deadline is already present in the context, it will not be modified. -func WithDefaultTimeout(timeout time.Duration) Option { - return func() (grpc.DialOption, error) { - if timeout <= 0 { - return nil, errors.New("timeout must be greater than zero") - } - return grpc.WithUnaryInterceptor(UnaryDefaultTimeoutInterceptor(timeout)), nil - } -} - -type auth struct { - Username string - Password string // #nosec G117 - SecureTransportCreds bool -} - -var _ credentials.PerRPCCredentials = (*auth)(nil) - -func (a *auth) GetRequestMetadata(_ context.Context, _ ...string) (map[string]string, error) { - return map[string]string{ - "username": a.Username, - "password": a.Password, - }, nil -} - -func (a *auth) RequireTransportSecurity() bool { - // Only called if the transport credentials are insecure. - return false -} - -// UnaryDefaultTimeoutInterceptor returns a gRPC unary client interceptor that sets a default timeout -// for each RPC. If a deadline is already present , it will not be modified. -func UnaryDefaultTimeoutInterceptor(timeout time.Duration) grpc.UnaryClientInterceptor { - return func(ctx context.Context, method string, req, reply any, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error { - if _, ok := ctx.Deadline(); ok { - return invoker(ctx, method, req, reply, cc, opts...) - } - - ctx, cancel := context.WithTimeout(ctx, timeout) - defer cancel() - - return invoker(ctx, method, req, reply, cc, opts...) - } -} - -// TerminalErrorInterceptor returns a gRPC unary client interceptor that wraps errors returned by the gRPC invoker -// as terminal errors if their gRPC status code is in the set of non-retryable codes defined in [terminalCodes]. -func TerminalErrorInterceptor() grpc.UnaryClientInterceptor { - return func(ctx context.Context, method string, req, reply any, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error { - return WrapTerminalError(invoker(ctx, method, req, reply, cc, opts...)) - } -} - -// WrapTerminalError wraps the given error as a terminal error if its gRPC status error -// with a non-retryable code. -func WrapTerminalError(err error) error { - if statusErr, ok := status.FromError(err); ok && slices.Contains(terminalCodes, statusErr.Code()) { - return reconcile.TerminalError(err) - } - return err -} - -// terminalCodes holds the set of gRPC codes that are considered terminal. -// That is, if an error has one of these codes, retrying the operation -// is not expected to succeed. -// This list is based on the gRPC documentation at https://grpc.io/docs/guides/status-codes. -var terminalCodes = []codes.Code{ - codes.Unknown, - codes.InvalidArgument, - codes.NotFound, - codes.AlreadyExists, - codes.PermissionDenied, - codes.FailedPrecondition, - codes.OutOfRange, - codes.Unimplemented, - codes.DataLoss, - codes.Unauthenticated, -} diff --git a/internal/provider/cisco/iosxr/provider.go b/internal/provider/cisco/iosxr/provider.go index 8378e1c87..319eb7fc4 100644 --- a/internal/provider/cisco/iosxr/provider.go +++ b/internal/provider/cisco/iosxr/provider.go @@ -9,11 +9,11 @@ import ( "fmt" "strconv" + "github.com/ironcore-dev/network-operator/api/core/v1alpha1" "github.com/ironcore-dev/network-operator/internal/deviceutil" "github.com/ironcore-dev/network-operator/internal/provider" "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" - - "github.com/ironcore-dev/network-operator/api/core/v1alpha1" + "github.com/ironcore-dev/network-operator/internal/transport/grpcext" "google.golang.org/grpc" ) @@ -33,7 +33,7 @@ func NewProvider() provider.Provider { } func (p *Provider) Connect(ctx context.Context, conn *deviceutil.Connection) (err error) { - p.conn, err = deviceutil.NewGrpcClient(ctx, conn) + p.conn, err = grpcext.NewClient(ctx, conn) if err != nil { return fmt.Errorf("failed to create grpc connection: %w", err) } diff --git a/internal/provider/cisco/nxos/provider.go b/internal/provider/cisco/nxos/provider.go index fa1602473..a615a8a43 100644 --- a/internal/provider/cisco/nxos/provider.go +++ b/internal/provider/cisco/nxos/provider.go @@ -30,6 +30,7 @@ import ( "github.com/ironcore-dev/network-operator/internal/deviceutil" "github.com/ironcore-dev/network-operator/internal/provider" "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" + "github.com/ironcore-dev/network-operator/internal/transport/grpcext" ) var ( @@ -71,7 +72,7 @@ func NewProvider() provider.Provider { } func (p *Provider) Connect(ctx context.Context, conn *deviceutil.Connection) (err error) { - p.conn, err = deviceutil.NewGrpcClient(ctx, conn, deviceutil.WithDefaultTimeout(30*time.Second)) + p.conn, err = grpcext.NewClient(ctx, conn, grpcext.WithDefaultTimeout(30*time.Second)) if err != nil { return fmt.Errorf("failed to create grpc connection: %w", err) } diff --git a/internal/provider/openconfig/provider.go b/internal/provider/openconfig/provider.go index 5f6c21a97..b441c0920 100644 --- a/internal/provider/openconfig/provider.go +++ b/internal/provider/openconfig/provider.go @@ -16,6 +16,7 @@ import ( "github.com/ironcore-dev/network-operator/api/core/v1alpha1" "github.com/ironcore-dev/network-operator/internal/deviceutil" "github.com/ironcore-dev/network-operator/internal/provider" + "github.com/ironcore-dev/network-operator/internal/transport/grpcext" ) var ( @@ -33,7 +34,7 @@ func NewProvider() provider.Provider { } func (p *Provider) Connect(ctx context.Context, conn *deviceutil.Connection) (err error) { - p.conn, err = deviceutil.NewGrpcClient(ctx, conn) + p.conn, err = grpcext.NewClient(ctx, conn) if err != nil { return fmt.Errorf("failed to create grpc connection: %w", err) } diff --git a/internal/transport/grpcext/grpcext.go b/internal/transport/grpcext/grpcext.go new file mode 100644 index 000000000..29d2c94bc --- /dev/null +++ b/internal/transport/grpcext/grpcext.go @@ -0,0 +1,130 @@ +// SPDX-FileCopyrightText: 2025 SAP SE or an SAP affiliate company and IronCore contributors +// SPDX-License-Identifier: Apache-2.0 + +// Package grpcext provides convenience functions and types for working with gRPC clients. +package grpcext + +import ( + "context" + "errors" + "slices" + "time" + + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/credentials" + "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/status" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/ironcore-dev/network-operator/internal/deviceutil" +) + +// NewClient creates a new gRPC client connection to a specified device using the provided [deviceutil.Connection]. +// The connection will use TLS if the [deviceutil.Connection.TLS] field is set, otherwise it will use an insecure connection. +// If the [deviceutil.Connection.Username] and [deviceutil.Connection.Password] fields are set, basic authentication in the form of metadata will be used. +func NewClient(ctx context.Context, conn *deviceutil.Connection, o ...Option) (*grpc.ClientConn, error) { + creds := insecure.NewCredentials() + if conn.TLS != nil { + creds = credentials.NewTLS(conn.TLS) + } + + opts := []grpc.DialOption{grpc.WithTransportCredentials(creds), grpc.WithUnaryInterceptor(TerminalErrorInterceptor())} + if conn.Username != "" && conn.Password != "" { + opts = append(opts, grpc.WithPerRPCCredentials(&auth{ + Username: conn.Username, + Password: conn.Password, + })) + } + + for _, opt := range o { + dialOpt, err := opt() + if err != nil { + return nil, err + } + opts = append(opts, dialOpt) + } + + return grpc.NewClient(conn.Address, opts...) +} + +type Option func() (grpc.DialOption, error) + +// WithDefaultTimeout returns a gRPC dial option that sets a default timeout for each RPC. +// If a deadline is already present in the context, it will not be modified. +func WithDefaultTimeout(timeout time.Duration) Option { + return func() (grpc.DialOption, error) { + if timeout <= 0 { + return nil, errors.New("timeout must be greater than zero") + } + return grpc.WithUnaryInterceptor(UnaryDefaultTimeoutInterceptor(timeout)), nil + } +} + +type auth struct { + Username string + Password string `json:"-"` +} + +var _ credentials.PerRPCCredentials = (*auth)(nil) + +func (a *auth) GetRequestMetadata(_ context.Context, _ ...string) (map[string]string, error) { + return map[string]string{ + "username": a.Username, + "password": a.Password, + }, nil +} + +func (a *auth) RequireTransportSecurity() bool { + // Only called if the transport credentials are insecure. + return false +} + +// UnaryDefaultTimeoutInterceptor returns a gRPC unary client interceptor that sets a default timeout +// for each RPC. If a deadline is already present, it will not be modified. +func UnaryDefaultTimeoutInterceptor(timeout time.Duration) grpc.UnaryClientInterceptor { + return func(ctx context.Context, method string, req, reply any, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error { + if _, ok := ctx.Deadline(); ok { + return invoker(ctx, method, req, reply, cc, opts...) + } + + ctx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + + return invoker(ctx, method, req, reply, cc, opts...) + } +} + +// TerminalErrorInterceptor returns a gRPC unary client interceptor that wraps errors returned by the gRPC invoker +// as terminal errors if their gRPC status code is in the set of non-retryable codes defined in [terminalCodes]. +func TerminalErrorInterceptor() grpc.UnaryClientInterceptor { + return func(ctx context.Context, method string, req, reply any, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error { + return WrapTerminalError(invoker(ctx, method, req, reply, cc, opts...)) + } +} + +// WrapTerminalError wraps the given error as a terminal error if its gRPC status error +// with a non-retryable code. +func WrapTerminalError(err error) error { + if statusErr, ok := status.FromError(err); ok && slices.Contains(terminalCodes, statusErr.Code()) { + return reconcile.TerminalError(err) + } + return err +} + +// terminalCodes holds the set of gRPC codes that are considered terminal. +// That is, if an error has one of these codes, retrying the operation +// is not expected to succeed. +// This list is based on the gRPC documentation at https://grpc.io/docs/guides/status-codes. +var terminalCodes = []codes.Code{ + codes.Unknown, + codes.InvalidArgument, + codes.NotFound, + codes.AlreadyExists, + codes.PermissionDenied, + codes.FailedPrecondition, + codes.OutOfRange, + codes.Unimplemented, + codes.DataLoss, + codes.Unauthenticated, +} From 21bd9165eb677adc8634eeb58f8c1e23841f20a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20K=C3=A4stner?= Date: Wed, 8 Apr 2026 15:40:34 +0200 Subject: [PATCH 4/4] Rename gnmiext.Configurable to DataElement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Configurable interface is used for both configuration and state data, making the old name misleading. Rename it to DataElement, the term used by the gNMI specification for any item addressable by a path — regardless of whether it represents a leaf, container, or entire subtree. Update variable names in the nxos provider package to reflect the intent of each call site: `updates` for Update, `deletes` for Delete, `patches` for Patch, and `el` for generic slices, replacing the ambiguous `conf` that implied configuration-only usage. --- .../provider/cisco/iosxr/provider_test.go | 38 +-- internal/provider/cisco/nxos/acl.go | 2 +- internal/provider/cisco/nxos/banner.go | 4 +- internal/provider/cisco/nxos/bgp.go | 4 +- internal/provider/cisco/nxos/bgw.go | 2 +- internal/provider/cisco/nxos/cert.go | 4 +- internal/provider/cisco/nxos/dhcprelay.go | 2 +- internal/provider/cisco/nxos/dns.go | 2 +- internal/provider/cisco/nxos/evi.go | 2 +- internal/provider/cisco/nxos/feat.go | 4 +- internal/provider/cisco/nxos/grpc.go | 8 +- internal/provider/cisco/nxos/intf.go | 50 ++-- internal/provider/cisco/nxos/isis.go | 2 +- internal/provider/cisco/nxos/lldp.go | 2 +- internal/provider/cisco/nxos/ntp.go | 4 +- internal/provider/cisco/nxos/nve.go | 6 +- internal/provider/cisco/nxos/ospf.go | 2 +- internal/provider/cisco/nxos/pim.go | 14 +- internal/provider/cisco/nxos/prefix.go | 2 +- internal/provider/cisco/nxos/provider.go | 216 +++++++++--------- internal/provider/cisco/nxos/provider_test.go | 4 +- internal/provider/cisco/nxos/routemap.go | 2 +- internal/provider/cisco/nxos/snmp.go | 14 +- internal/provider/cisco/nxos/syslog.go | 10 +- internal/provider/cisco/nxos/system.go | 12 +- internal/provider/cisco/nxos/term.go | 10 +- internal/provider/cisco/nxos/user.go | 2 +- internal/provider/cisco/nxos/vlan.go | 14 +- internal/provider/cisco/nxos/vpc.go | 4 +- internal/provider/cisco/nxos/vrf.go | 2 +- internal/transport/gnmiext/client.go | 89 ++++---- internal/transport/gnmiext/client_test.go | 74 +++--- 32 files changed, 305 insertions(+), 302 deletions(-) diff --git a/internal/provider/cisco/iosxr/provider_test.go b/internal/provider/cisco/iosxr/provider_test.go index 56ef9f818..81a0d18ae 100644 --- a/internal/provider/cisco/iosxr/provider_test.go +++ b/internal/provider/cisco/iosxr/provider_test.go @@ -21,12 +21,12 @@ import ( type TestCase struct { name string - val gnmiext.Configurable + val gnmiext.DataElement } var tests []TestCase -func Register(name string, val gnmiext.Configurable) { +func Register(name string, val gnmiext.DataElement) { tests = append(tests, TestCase{ name: name, val: val, @@ -92,11 +92,11 @@ func Test_Payload(t *testing.T) { type MockClient struct { // Function fields for mocking different methods CapabilitiesFunc func() *gnmiext.Capabilities - GetConfigFunc func(ctx context.Context, conf ...gnmiext.Configurable) error - PatchFunc func(ctx context.Context, conf ...gnmiext.Configurable) error - UpdateFunc func(ctx context.Context, conf ...gnmiext.Configurable) error - DeleteFunc func(ctx context.Context, conf ...gnmiext.Configurable) error - GetStateFunc func(ctx context.Context, conf ...gnmiext.Configurable) error + GetConfigFunc func(ctx context.Context, configs ...gnmiext.DataElement) error + PatchFunc func(ctx context.Context, patches ...gnmiext.DataElement) error + UpdateFunc func(ctx context.Context, updates ...gnmiext.DataElement) error + DeleteFunc func(ctx context.Context, deletes ...gnmiext.DataElement) error + GetStateFunc func(ctx context.Context, states ...gnmiext.DataElement) error } var _ gnmiext.Client = (*MockClient)(nil) @@ -109,37 +109,37 @@ func (m *MockClient) Capabilities() *gnmiext.Capabilities { return nil } -func (m *MockClient) GetConfig(ctx context.Context, conf ...gnmiext.Configurable) error { +func (m *MockClient) GetConfig(ctx context.Context, configs ...gnmiext.DataElement) error { if m.GetConfigFunc != nil { - return m.GetConfigFunc(ctx, conf...) + return m.GetConfigFunc(ctx, configs...) } return nil } -func (m *MockClient) GetState(ctx context.Context, conf ...gnmiext.Configurable) error { +func (m *MockClient) GetState(ctx context.Context, states ...gnmiext.DataElement) error { if m.GetStateFunc != nil { - return m.GetStateFunc(ctx, conf...) + return m.GetStateFunc(ctx, states...) } return nil } -func (m *MockClient) Patch(ctx context.Context, conf ...gnmiext.Configurable) error { +func (m *MockClient) Patch(ctx context.Context, patches ...gnmiext.DataElement) error { if m.PatchFunc != nil { - return m.PatchFunc(ctx, conf...) + return m.PatchFunc(ctx, patches...) } return nil } -func (m *MockClient) Update(ctx context.Context, conf ...gnmiext.Configurable) error { +func (m *MockClient) Update(ctx context.Context, updates ...gnmiext.DataElement) error { if m.UpdateFunc != nil { - return m.UpdateFunc(ctx, conf...) + return m.UpdateFunc(ctx, updates...) } return nil } -func (m *MockClient) Delete(ctx context.Context, conf ...gnmiext.Configurable) error { +func (m *MockClient) Delete(ctx context.Context, deletes ...gnmiext.DataElement) error { if m.DeleteFunc != nil { - return m.DeleteFunc(ctx, conf...) + return m.DeleteFunc(ctx, deletes...) } return nil } @@ -187,8 +187,8 @@ func Test_EnsureInterface(t *testing.T) { func Test_GetState(t *testing.T) { m := &MockClient{ - GetStateFunc: func(ctx context.Context, conf ...gnmiext.Configurable) error { - conf[0].(*PhysIfState).State = "im-state-up" + GetStateFunc: func(ctx context.Context, states ...gnmiext.DataElement) error { + states[0].(*PhysIfState).State = "im-state-up" return nil }, } diff --git a/internal/provider/cisco/nxos/acl.go b/internal/provider/cisco/nxos/acl.go index 9434ab372..b6c67665a 100644 --- a/internal/provider/cisco/nxos/acl.go +++ b/internal/provider/cisco/nxos/acl.go @@ -10,7 +10,7 @@ import ( "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) -var _ gnmiext.Configurable = (*ACL)(nil) +var _ gnmiext.DataElement = (*ACL)(nil) // ACL represents an IPv4 or IPv6 access control list, depending on the rules it contains. // It can only contain either IPv4 or IPv6 rules, never both. It's name must be unique diff --git a/internal/provider/cisco/nxos/banner.go b/internal/provider/cisco/nxos/banner.go index 010fd8ce7..eb058c05e 100644 --- a/internal/provider/cisco/nxos/banner.go +++ b/internal/provider/cisco/nxos/banner.go @@ -11,8 +11,8 @@ import ( ) var ( - _ gnmiext.Configurable = (*Banner)(nil) - _ gnmiext.Defaultable = (*Banner)(nil) + _ gnmiext.DataElement = (*Banner)(nil) + _ gnmiext.Defaultable = (*Banner)(nil) ) // Banner represents the pre-login banner configuration of the device. diff --git a/internal/provider/cisco/nxos/bgp.go b/internal/provider/cisco/nxos/bgp.go index 490b5866c..d688543a7 100644 --- a/internal/provider/cisco/nxos/bgp.go +++ b/internal/provider/cisco/nxos/bgp.go @@ -14,8 +14,8 @@ import ( ) var ( - _ gnmiext.Configurable = (*BGP)(nil) - _ gnmiext.Configurable = (*BGPDom)(nil) + _ gnmiext.DataElement = (*BGP)(nil) + _ gnmiext.DataElement = (*BGPDom)(nil) ) type BGP struct { diff --git a/internal/provider/cisco/nxos/bgw.go b/internal/provider/cisco/nxos/bgw.go index ce371c737..eb314ed70 100644 --- a/internal/provider/cisco/nxos/bgw.go +++ b/internal/provider/cisco/nxos/bgw.go @@ -7,7 +7,7 @@ import ( "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) -var _ gnmiext.Configurable = (*MultisiteItems)(nil) +var _ gnmiext.DataElement = (*MultisiteItems)(nil) type MultisiteItems struct { SiteID string `json:"siteId"` diff --git a/internal/provider/cisco/nxos/cert.go b/internal/provider/cisco/nxos/cert.go index 357192ee4..e9cf67b2d 100644 --- a/internal/provider/cisco/nxos/cert.go +++ b/internal/provider/cisco/nxos/cert.go @@ -91,8 +91,8 @@ func (c *Certificate) EncodeKeyPair() (private, public []byte, err error) { } var ( - _ gnmiext.Configurable = (*Trustpoint)(nil) - _ gnmiext.Configurable = (*KeyPair)(nil) + _ gnmiext.DataElement = (*Trustpoint)(nil) + _ gnmiext.DataElement = (*KeyPair)(nil) ) // Trustpoint represents a PKI trustpoint configuration on a NX-OS device. diff --git a/internal/provider/cisco/nxos/dhcprelay.go b/internal/provider/cisco/nxos/dhcprelay.go index 41cb49bb9..daf5e7df3 100644 --- a/internal/provider/cisco/nxos/dhcprelay.go +++ b/internal/provider/cisco/nxos/dhcprelay.go @@ -9,7 +9,7 @@ import ( "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) -var _ gnmiext.Configurable = (*DHCPRelayConfig)(nil) +var _ gnmiext.DataElement = (*DHCPRelayConfig)(nil) // DHCPRelayConfig represents the complete DHCP relay configuration tree. type DHCPRelayConfig struct { diff --git a/internal/provider/cisco/nxos/dns.go b/internal/provider/cisco/nxos/dns.go index a418b34ab..7347debd2 100644 --- a/internal/provider/cisco/nxos/dns.go +++ b/internal/provider/cisco/nxos/dns.go @@ -5,7 +5,7 @@ package nxos import "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" -var _ gnmiext.Configurable = (*DNS)(nil) +var _ gnmiext.DataElement = (*DNS)(nil) // DNS represents the DNS configuration on a NX-OS device. type DNS struct { diff --git a/internal/provider/cisco/nxos/evi.go b/internal/provider/cisco/nxos/evi.go index 6a4c1e076..5a693183c 100644 --- a/internal/provider/cisco/nxos/evi.go +++ b/internal/provider/cisco/nxos/evi.go @@ -14,7 +14,7 @@ import ( "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) -var _ gnmiext.Configurable = (*BDEVI)(nil) +var _ gnmiext.DataElement = (*BDEVI)(nil) // BDEVI represents a Bridge Domain Ethernet VPN Instance (MAC-VRF). type BDEVI struct { diff --git a/internal/provider/cisco/nxos/feat.go b/internal/provider/cisco/nxos/feat.go index 93e83650d..28cd93dbf 100644 --- a/internal/provider/cisco/nxos/feat.go +++ b/internal/provider/cisco/nxos/feat.go @@ -6,8 +6,8 @@ package nxos import "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" var ( - _ gnmiext.Configurable = (*Feature)(nil) - _ gnmiext.Defaultable = (*Feature)(nil) + _ gnmiext.DataElement = (*Feature)(nil) + _ gnmiext.Defaultable = (*Feature)(nil) ) // Feature represents a dynamic feature configuration on a NX-OS device. diff --git a/internal/provider/cisco/nxos/grpc.go b/internal/provider/cisco/nxos/grpc.go index 34c5557d2..769121d1b 100644 --- a/internal/provider/cisco/nxos/grpc.go +++ b/internal/provider/cisco/nxos/grpc.go @@ -11,10 +11,10 @@ import ( ) var ( - _ gnmiext.Configurable = (*GRPC)(nil) - _ gnmiext.Defaultable = (*GRPC)(nil) - _ gnmiext.Configurable = (*GNMI)(nil) - _ gnmiext.Defaultable = (*GNMI)(nil) + _ gnmiext.DataElement = (*GRPC)(nil) + _ gnmiext.Defaultable = (*GRPC)(nil) + _ gnmiext.DataElement = (*GNMI)(nil) + _ gnmiext.Defaultable = (*GNMI)(nil) ) // GRPC represents the gRPC configuration on a NX-OS device. diff --git a/internal/provider/cisco/nxos/intf.go b/internal/provider/cisco/nxos/intf.go index cda069b41..47c205de7 100644 --- a/internal/provider/cisco/nxos/intf.go +++ b/internal/provider/cisco/nxos/intf.go @@ -17,22 +17,22 @@ import ( ) var ( - _ gnmiext.Configurable = (*Loopback)(nil) - _ gnmiext.Configurable = (*LoopbackOperItems)(nil) - _ gnmiext.Configurable = (*PhysIf)(nil) - _ gnmiext.Defaultable = (*PhysIf)(nil) - _ gnmiext.Configurable = (*PhysIfOperItems)(nil) - _ gnmiext.Configurable = (*VrfMember)(nil) - _ gnmiext.Configurable = (*SpanningTree)(nil) - _ gnmiext.Configurable = (*MultisiteIfTracking)(nil) - _ gnmiext.Configurable = (*BFD)(nil) - _ gnmiext.Configurable = (*ICMPIf)(nil) - _ gnmiext.Configurable = (*PortChannel)(nil) - _ gnmiext.Configurable = (*PortChannelOperItems)(nil) - _ gnmiext.Configurable = (*SwitchVirtualInterface)(nil) - _ gnmiext.Configurable = (*SwitchVirtualInterfaceOperItems)(nil) - _ gnmiext.Configurable = (*AddrItem)(nil) - _ gnmiext.Configurable = (*FabricFwdIf)(nil) + _ gnmiext.DataElement = (*Loopback)(nil) + _ gnmiext.DataElement = (*LoopbackOperItems)(nil) + _ gnmiext.DataElement = (*PhysIf)(nil) + _ gnmiext.Defaultable = (*PhysIf)(nil) + _ gnmiext.DataElement = (*PhysIfOperItems)(nil) + _ gnmiext.DataElement = (*VrfMember)(nil) + _ gnmiext.DataElement = (*SpanningTree)(nil) + _ gnmiext.DataElement = (*MultisiteIfTracking)(nil) + _ gnmiext.DataElement = (*BFD)(nil) + _ gnmiext.DataElement = (*ICMPIf)(nil) + _ gnmiext.DataElement = (*PortChannel)(nil) + _ gnmiext.DataElement = (*PortChannelOperItems)(nil) + _ gnmiext.DataElement = (*SwitchVirtualInterface)(nil) + _ gnmiext.DataElement = (*SwitchVirtualInterfaceOperItems)(nil) + _ gnmiext.DataElement = (*AddrItem)(nil) + _ gnmiext.DataElement = (*FabricFwdIf)(nil) ) // Loopback represents a loopback interface on a NX-OS device. @@ -451,7 +451,7 @@ func Exists(ctx context.Context, client gnmiext.Client, names ...string) (bool, if len(names) == 0 { return false, errors.New("at least one interface name must be provided") } - conf := make([]gnmiext.Configurable, 0, len(names)) + el := make([]gnmiext.DataElement, 0, len(names)) for _, name := range names { if name == "" { return false, errors.New("interface name must not be empty") @@ -460,26 +460,26 @@ func Exists(ctx context.Context, client gnmiext.Client, names ...string) (bool, // mgmt0 is always present continue } - var c gnmiext.Configurable + var e gnmiext.DataElement if matches := ethernetRe.FindStringSubmatch(name); matches != nil { - c = &PhysIf{ID: "eth" + matches[2]} + e = &PhysIf{ID: "eth" + matches[2]} } if matches := loopbackRe.FindStringSubmatch(name); matches != nil { - c = &Loopback{ID: "lo" + matches[2]} + e = &Loopback{ID: "lo" + matches[2]} } if matches := portchannelRe.FindStringSubmatch(name); matches != nil { - c = &PortChannel{ID: "po" + matches[2]} + e = &PortChannel{ID: "po" + matches[2]} } if matches := vlanRe.FindStringSubmatch(name); matches != nil { - c = &SwitchVirtualInterface{ID: "vlan" + matches[2]} + e = &SwitchVirtualInterface{ID: "vlan" + matches[2]} } - if c == nil { + if e == nil { return false, fmt.Errorf("unsupported interface format %q, expected one of: %s, %s, %s, %s, %s", name, mgmtRe.String(), ethernetRe.String(), loopbackRe.String(), portchannelRe.String(), vlanRe.String()) } - conf = append(conf, c) + el = append(el, e) } const batchSize = 10 // On Cisco NX-OS, more than 10 paths per single gNMI request lead to gRPC errors. - for batch := range slices.Chunk(conf, batchSize) { + for batch := range slices.Chunk(el, batchSize) { if err := client.GetConfig(ctx, batch...); err != nil { if errors.Is(err, gnmiext.ErrNil) { return false, nil diff --git a/internal/provider/cisco/nxos/isis.go b/internal/provider/cisco/nxos/isis.go index 53d320da5..3bc0886b8 100644 --- a/internal/provider/cisco/nxos/isis.go +++ b/internal/provider/cisco/nxos/isis.go @@ -8,7 +8,7 @@ import ( "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) -var _ gnmiext.Configurable = (*ISIS)(nil) +var _ gnmiext.DataElement = (*ISIS)(nil) // ISIS represents the IS-IS routing protocol configuration on a NX-OS device. type ISIS struct { diff --git a/internal/provider/cisco/nxos/lldp.go b/internal/provider/cisco/nxos/lldp.go index 412f19eca..a42875729 100644 --- a/internal/provider/cisco/nxos/lldp.go +++ b/internal/provider/cisco/nxos/lldp.go @@ -5,7 +5,7 @@ package nxos import "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" -var _ gnmiext.Configurable = (*LLDP)(nil) +var _ gnmiext.DataElement = (*LLDP)(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. diff --git a/internal/provider/cisco/nxos/ntp.go b/internal/provider/cisco/nxos/ntp.go index 99e1d091d..7f9034203 100644 --- a/internal/provider/cisco/nxos/ntp.go +++ b/internal/provider/cisco/nxos/ntp.go @@ -6,8 +6,8 @@ package nxos import "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" var ( - _ gnmiext.Configurable = (*NTP)(nil) - _ gnmiext.Defaultable = (*NTP)(nil) + _ gnmiext.DataElement = (*NTP)(nil) + _ gnmiext.Defaultable = (*NTP)(nil) ) // NTP represents the NTP configuration on a NX-OS device. diff --git a/internal/provider/cisco/nxos/nve.go b/internal/provider/cisco/nxos/nve.go index 3573eacc9..f3ac1b7e9 100644 --- a/internal/provider/cisco/nxos/nve.go +++ b/internal/provider/cisco/nxos/nve.go @@ -11,9 +11,9 @@ import ( ) var ( - _ gnmiext.Configurable = (*NVE)(nil) - _ gnmiext.Configurable = (*NVEInfraVLANs)(nil) - _ gnmiext.Configurable = (*FabricFwd)(nil) + _ gnmiext.DataElement = (*NVE)(nil) + _ gnmiext.DataElement = (*NVEInfraVLANs)(nil) + _ gnmiext.DataElement = (*FabricFwd)(nil) ) // NVE represents the Network Virtualization Edge interface (nve1). diff --git a/internal/provider/cisco/nxos/ospf.go b/internal/provider/cisco/nxos/ospf.go index f5556ba92..6c276355b 100644 --- a/internal/provider/cisco/nxos/ospf.go +++ b/internal/provider/cisco/nxos/ospf.go @@ -10,7 +10,7 @@ import ( "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) -var _ gnmiext.Configurable = (*OSPF)(nil) +var _ gnmiext.DataElement = (*OSPF)(nil) type OSPF struct { AdminSt AdminSt `json:"adminSt"` diff --git a/internal/provider/cisco/nxos/pim.go b/internal/provider/cisco/nxos/pim.go index 2bb76b94d..2168b2c5e 100644 --- a/internal/provider/cisco/nxos/pim.go +++ b/internal/provider/cisco/nxos/pim.go @@ -6,13 +6,13 @@ package nxos import "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" var ( - _ gnmiext.Configurable = (*PIM)(nil) - _ gnmiext.Configurable = (*PIMDom)(nil) - _ gnmiext.Configurable = (*StaticRPItems)(nil) - _ gnmiext.Configurable = (*StaticRP)(nil) - _ gnmiext.Configurable = (*StaticRPGrp)(nil) - _ gnmiext.Configurable = (*AnycastPeerItems)(nil) - _ gnmiext.Configurable = (*PIMIfItems)(nil) + _ gnmiext.DataElement = (*PIM)(nil) + _ gnmiext.DataElement = (*PIMDom)(nil) + _ gnmiext.DataElement = (*StaticRPItems)(nil) + _ gnmiext.DataElement = (*StaticRP)(nil) + _ gnmiext.DataElement = (*StaticRPGrp)(nil) + _ gnmiext.DataElement = (*AnycastPeerItems)(nil) + _ gnmiext.DataElement = (*PIMIfItems)(nil) ) type PIM struct { diff --git a/internal/provider/cisco/nxos/prefix.go b/internal/provider/cisco/nxos/prefix.go index 6aa56ea8b..f8479ca9c 100644 --- a/internal/provider/cisco/nxos/prefix.go +++ b/internal/provider/cisco/nxos/prefix.go @@ -7,7 +7,7 @@ import ( "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) -var _ gnmiext.Configurable = (*PrefixList)(nil) +var _ gnmiext.DataElement = (*PrefixList)(nil) type PrefixList struct { Name string `json:"name"` diff --git a/internal/provider/cisco/nxos/provider.go b/internal/provider/cisco/nxos/provider.go index a615a8a43..3bb545691 100644 --- a/internal/provider/cisco/nxos/provider.go +++ b/internal/provider/cisco/nxos/provider.go @@ -533,7 +533,7 @@ func (p *Provider) EnsureEVPNInstance(ctx context.Context, req *provider.EVPNIns return err } - conf := make([]gnmiext.Configurable, 0, 3) + updates := make([]gnmiext.DataElement, 0, 3) if req.EVPNInstance.Spec.Type == v1alpha1.EVPNInstanceTypeBridged { v := new(VLAN) v.FabEncap = "vlan-" + strconv.FormatInt(int64(req.VLAN.Spec.ID), 10) @@ -544,7 +544,7 @@ func (p *Provider) EnsureEVPNInstance(ctx context.Context, req *provider.EVPNIns vxlan := new(VXLAN) vxlan.AccEncap = "vxlan-" + strconv.FormatInt(int64(req.EVPNInstance.Spec.VNI), 10) vxlan.FabEncap = v.FabEncap - conf = append(conf, vxlan) + updates = append(updates, vxlan) } vni := new(VNI) @@ -552,7 +552,7 @@ func (p *Provider) EnsureEVPNInstance(ctx context.Context, req *provider.EVPNIns if req.EVPNInstance.Spec.MulticastGroupAddress != "" { vni.McastGroup = NewOption(req.EVPNInstance.Spec.MulticastGroupAddress) } - conf = append(conf, vni) + updates = append(updates, vni) switch req.EVPNInstance.Spec.Type { case v1alpha1.EVPNInstanceTypeBridged: @@ -592,25 +592,25 @@ func (p *Provider) EnsureEVPNInstance(ctx context.Context, req *provider.EVPNIns if exports.EntItems.RttEntryList.Len() > 0 { evi.RttpItems.RttPList.Set(exports) } - conf = append(conf, evi) + updates = append(updates, evi) case v1alpha1.EVPNInstanceTypeRouted: vni.AssociateVrfFlag = true } - return p.Update(ctx, conf...) + return p.Update(ctx, updates...) } func (p *Provider) DeleteEVPNInstance(ctx context.Context, req *provider.EVPNInstanceRequest) error { - conf := make([]gnmiext.Configurable, 0, 3) + deletes := make([]gnmiext.DataElement, 0, 3) evi := new(BDEVI) evi.Encap = "vxlan-" + strconv.FormatInt(int64(req.EVPNInstance.Spec.VNI), 10) - conf = append(conf, evi) + deletes = append(deletes, evi) vni := new(VNI) vni.Vni = req.EVPNInstance.Spec.VNI - conf = append(conf, vni) + deletes = append(deletes, vni) if req.EVPNInstance.Spec.Type == v1alpha1.EVPNInstanceTypeBridged { bd := new(BDItems) @@ -619,11 +619,11 @@ func (p *Provider) DeleteEVPNInstance(ctx context.Context, req *provider.EVPNIns } if v := bd.GetByVXLAN(evi.Encap); v != nil { - conf = append(conf, v) + deletes = append(deletes, v) } } - return p.client.Delete(ctx, conf...) + return p.client.Delete(ctx, deletes...) } // isPointToPoint reports whether the given IPv4 configuration represents a @@ -688,21 +688,21 @@ func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInte } } - del := make([]gnmiext.Configurable, 0, 2) + deletes := make([]gnmiext.DataElement, 0, 2) addrs := new(AddrList) if err := p.client.GetConfig(ctx, addrs); err != nil && !errors.Is(err, gnmiext.ErrNil) { return err } for _, a := range addrs.GetAddrItemsByInterface(name) { if addr == nil || a.Vrf != vrf { - del = append(del, a) + deletes = append(deletes, a) } } - if err := p.client.Delete(ctx, del...); err != nil { + if err := p.client.Delete(ctx, deletes...); err != nil { return err } - conf := make([]gnmiext.Configurable, 0, 4) + updates := make([]gnmiext.DataElement, 0, 4) switch req.Interface.Spec.Type { case v1alpha1.InterfaceTypePhysical: p := new(PhysIf) @@ -774,7 +774,7 @@ func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInte return err } - conf = append(conf, p) + updates = append(updates, p) case v1alpha1.InterfaceTypeLoopback: lb := new(Loopback) @@ -785,13 +785,13 @@ func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInte lb.AdminSt = AdminStUp } lb.RtvrfMbrItems = NewVrfMember(name, vrf) - conf = append(conf, lb) + updates = append(updates, lb) case v1alpha1.InterfaceTypeAggregate: f := new(Feature) f.Name = "lacp" f.AdminSt = AdminStEnabled - conf = append(conf, f) + updates = append(updates, f) pcNum, err := strconv.Atoi(name[2:]) if err != nil { @@ -888,20 +888,20 @@ func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInte pc.AggrExtdItems.BufferBoost = AdminStDisable } - conf = append(conf, pc) + updates = append(updates, pc) if req.MultiChassisID != nil { v := new(VPCIf) v.ID = int(*req.MultiChassisID) v.SetPortChannel(name) - conf = append(conf, v) + updates = append(updates, v) } case v1alpha1.InterfaceTypeRoutedVLAN: f := new(Feature) f.Name = "ifvlan" f.AdminSt = AdminStEnabled - conf = append(conf, f) + updates = append(updates, f) svi := new(SwitchVirtualInterface) svi.ID = name @@ -917,7 +917,7 @@ func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInte } svi.VlanID = req.VLAN.Spec.ID svi.RtvrfMbrItems = NewVrfMember(name, vrf) - conf = append(conf, svi) + updates = append(updates, svi) fwif := new(FabricFwdIf) fwif.ID = name @@ -934,7 +934,7 @@ func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInte fwif.AdminSt = AdminStEnabled fwif.Mode = FwdModeAnycastGateway - conf = append(conf, fwif) + updates = append(updates, fwif) default: if err := p.client.Delete(ctx, fwif); err != nil { return err @@ -973,12 +973,12 @@ func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInte } } } - conf = append(conf, stp) + updates = append(updates, stp) } // Add the address items last, as they depend on the interface being created first. if addr != nil { - conf = append(conf, addr) + updates = append(updates, addr) } switch { @@ -986,14 +986,14 @@ func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInte f := new(Feature) f.Name = "bfd" f.AdminSt = AdminStEnabled - conf = append(conf, f) + updates = append(updates, f) // Disable ICMP redirect messages on BFD-enabled interfaces. // See: https://www.cisco.com/c/en/us/td/docs/dcn/nx-os/nexus9000/106x/configuration/interfaces/cisco-nexus-9000-series-nx-os-interfaces-configuration-guide-release-106x/b-cisco-nexus-9000-nx-os-interfaces-configuration-guide-93x_chapter_01111.html icmp := new(ICMPIf) icmp.ID = name icmp.Ctrl = "port-unreachable" - conf = append(conf, icmp) + updates = append(updates, icmp) bfd := new(BFD) bfd.ID = name @@ -1013,18 +1013,18 @@ func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInte if err := bfd.Validate(); err != nil { return err } - conf = append(conf, bfd) + updates = append(updates, bfd) case req.Interface.Spec.BFD != nil && !req.Interface.Spec.BFD.Enabled: f := new(Feature) f.Name = "bfd" f.AdminSt = AdminStEnabled - conf = append(conf, f) + updates = append(updates, f) bfd := new(BFD) bfd.ID = name bfd.AdminSt = AdminStDisabled - conf = append(conf, bfd) + updates = append(updates, bfd) default: // BFD not specified — clean up any leftover BFD config on the interface. @@ -1047,14 +1047,14 @@ func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInte } case v1alpha1.InterfaceTypeLoopback: icmp.Ctrl = "port-unreachable,redirect" - conf = append(conf, icmp) + updates = append(updates, icmp) case v1alpha1.InterfaceTypeRoutedVLAN: icmp.Ctrl = "port-unreachable" - conf = append(conf, icmp) + updates = append(updates, icmp) } } - return p.Update(ctx, conf...) + return p.Update(ctx, updates...) } func (p *Provider) DeleteInterface(ctx context.Context, req *provider.InterfaceRequest) error { @@ -1063,44 +1063,44 @@ func (p *Provider) DeleteInterface(ctx context.Context, req *provider.InterfaceR return err } - conf := make([]gnmiext.Configurable, 0, 3) + deletes := make([]gnmiext.DataElement, 0, 3) addrs := new(AddrList) if err := p.client.GetConfig(ctx, addrs); err != nil && !errors.Is(err, gnmiext.ErrNil) { return err } for _, addr := range addrs.GetAddrItemsByInterface(name) { - conf = append(conf, addr) + deletes = append(deletes, addr) } bfd := new(BFD) bfd.ID = name - conf = append(conf, bfd) + deletes = append(deletes, bfd) switch req.Interface.Spec.Type { case v1alpha1.InterfaceTypePhysical: i := new(PhysIf) i.ID = name - conf = append(conf, i) + deletes = append(deletes, i) stp := new(SpanningTree) stp.IfName = name if err = p.client.GetConfig(ctx, stp); err == nil { - conf = append(conf, stp) + deletes = append(deletes, stp) } icmp := new(ICMPIf) icmp.ID = name - conf = append(conf, icmp) + deletes = append(deletes, icmp) case v1alpha1.InterfaceTypeLoopback: lb := new(Loopback) lb.ID = name - conf = append(conf, lb) + deletes = append(deletes, lb) case v1alpha1.InterfaceTypeAggregate: pc := new(PortChannel) pc.ID = name - conf = append(conf, pc) + deletes = append(deletes, pc) v := new(VPCIfItems) if err := p.client.GetConfig(ctx, v); err != nil && !errors.Is(err, gnmiext.ErrNil) { @@ -1109,19 +1109,19 @@ func (p *Provider) DeleteInterface(ctx context.Context, req *provider.InterfaceR // Make sure to delete any associated VPC interface. if vpc := v.GetListItemByInterface(name); vpc != nil { - conf = append(conf, vpc) + deletes = append(deletes, vpc) } case v1alpha1.InterfaceTypeRoutedVLAN: svi := new(SwitchVirtualInterface) svi.ID = name - conf = append(conf, svi) + deletes = append(deletes, svi) default: return fmt.Errorf("unsupported interface type: %s", req.Interface.Spec.Type) } - return p.client.Delete(ctx, conf...) + return p.client.Delete(ctx, deletes...) } func (p *Provider) GetInterfaceStatus(ctx context.Context, req *provider.InterfaceRequest) (provider.InterfaceStatus, error) { @@ -1205,7 +1205,7 @@ func (p *Provider) EnsureISIS(ctx context.Context, req *provider.EnsureISISReque f.Name = "isis" f.AdminSt = AdminStEnabled - conf := append(make([]gnmiext.Configurable, 0, 3), f) + updates := append(make([]gnmiext.DataElement, 0, 3), f) if slices.ContainsFunc(req.Interfaces, func(intf *v1alpha1.Interface) bool { return intf.Spec.BFD != nil && intf.Spec.BFD.Enabled @@ -1213,7 +1213,7 @@ func (p *Provider) EnsureISIS(ctx context.Context, req *provider.EnsureISISReque f := new(Feature) f.Name = "bfd" f.AdminSt = AdminStEnabled - conf = append(conf, f) + updates = append(updates, f) } i := new(ISIS) @@ -1286,9 +1286,9 @@ func (p *Provider) EnsureISIS(ctx context.Context, req *provider.EnsureISISReque } dom.IfItems.IfList.Set(intf) } - conf = append(conf, i) + updates = append(updates, i) - return p.Update(ctx, conf...) + return p.Update(ctx, updates...) } func (p *Provider) DeleteISIS(ctx context.Context, req *provider.DeleteISISRequest) error { @@ -1361,13 +1361,13 @@ func (p *Provider) EnsureManagementAccess(ctx context.Context, req *provider.Ens } } - conf := make([]gnmiext.Configurable, 0, 7) - conf = append(conf, gf, sf, g, gn, vty, con) + patches := make([]gnmiext.DataElement, 0, 7) + patches = append(patches, gf, sf, g, gn, vty, con) if acl.Name != "" { - conf = append(conf, acl) + patches = append(patches, acl) } - return p.Patch(ctx, conf...) + return p.Patch(ctx, patches...) } func (p *Provider) DeleteManagementAccess(ctx context.Context) error { @@ -1466,12 +1466,12 @@ func (p *Provider) EnsureOSPF(ctx context.Context, req *provider.EnsureOSPFReque } } - conf := make([]gnmiext.Configurable, 0, 3) + updates := make([]gnmiext.DataElement, 0, 3) f := new(Feature) f.Name = "ospf" f.AdminSt = AdminStEnabled - conf = append(conf, f) + updates = append(updates, f) o := new(OSPF) o.AdminSt = AdminStEnabled @@ -1479,7 +1479,7 @@ func (p *Provider) EnsureOSPF(ctx context.Context, req *provider.EnsureOSPFReque o.AdminSt = AdminStDisabled } o.Name = req.OSPF.Spec.Instance - conf = append(conf, o) + updates = append(updates, o) dom := new(OSPFDom) dom.Name = DefaultVRFName @@ -1543,7 +1543,7 @@ func (p *Provider) EnsureOSPF(ctx context.Context, req *provider.EnsureOSPFReque fb := new(Feature) fb.Name = "bfd" fb.AdminSt = AdminStEnabled - conf = slices.Insert(conf, 1, gnmiext.Configurable(fb)) // insert before OSPF + updates = slices.Insert(updates, 1, gnmiext.DataElement(fb)) // insert before OSPF intf.BFDCtrl = OspfBfdCtrlDisabled if iface.Interface.Spec.BFD.Enabled { @@ -1577,7 +1577,7 @@ func (p *Provider) EnsureOSPF(ctx context.Context, req *provider.EnsureOSPFReque dom.MaxlsapItems.MaxLsa = cfg.MaxLSA } - return p.Update(ctx, conf...) + return p.Update(ctx, updates...) } func (p *Provider) DeleteOSPF(ctx context.Context, req *provider.DeleteOSPFRequest) error { @@ -1702,12 +1702,12 @@ func (p *Provider) EnsurePIM(ctx context.Context, req *provider.EnsurePIMRequest ifItems.IfList.Set(intf) } - conf := make([]gnmiext.Configurable, 0, 3) - del := make([]gnmiext.Configurable, 0, 3) + updates := make([]gnmiext.DataElement, 0, 3) + deletes := make([]gnmiext.DataElement, 0, 3) if len(rpItems.StaticRPList) > 0 { // Diff group-to-RP bindings individually; replacing entire StaticRP entries fails on NX-OS - // with "child (Rn) cannot be added to deleted object Rn=rpgrplist-[...], Commit Failed". + // with "child (Rn) cannot be added to deleteseted object Rn=rpgrplist-[...], Commit Failed". current := new(StaticRPItems) if err := p.client.GetConfig(ctx, current); err != nil && !errors.Is(err, gnmiext.ErrNil) { return err @@ -1716,50 +1716,50 @@ func (p *Provider) EnsurePIM(ctx context.Context, req *provider.EnsurePIMRequest got, ok := current.StaticRPList.Get(rp.Key()) if !ok { // StaticRP does not exist yet — add the entire entry. - conf = append(conf, rp) + updates = append(updates, rp) continue } for _, grp := range rp.RpgrplistItems.RPGrpListList { if gotGrp, ok := got.RpgrplistItems.RPGrpListList.Get(grp.Key()); !ok || !reflect.DeepEqual(gotGrp, grp) { g := *grp g.RpAddr = rp.Addr - conf = append(conf, &g) + updates = append(updates, &g) } } for _, grp := range got.RpgrplistItems.RPGrpListList { if _, ok := rp.RpgrplistItems.RPGrpListList.Get(grp.Key()); !ok { g := *grp g.RpAddr = rp.Addr - del = append(del, &g) + deletes = append(deletes, &g) } } } for _, rp := range current.StaticRPList { if _, ok := rpItems.StaticRPList.Get(rp.Key()); !ok { - del = append(del, rp) + deletes = append(deletes, rp) } } } else { - del = append(del, rpItems) + deletes = append(deletes, rpItems) } if len(apItems.AcastRPPeerList) > 0 { - conf = append(conf, apItems) + updates = append(updates, apItems) } else { - del = append(del, apItems) + deletes = append(deletes, apItems) } if len(ifItems.IfList) > 0 { - conf = append(conf, ifItems) + updates = append(updates, ifItems) } else { - del = append(del, ifItems) + deletes = append(deletes, ifItems) } - if err := p.Update(ctx, conf...); err != nil { + if err := p.Update(ctx, updates...); err != nil { return err } - return p.client.Delete(ctx, del...) + return p.client.Delete(ctx, deletes...) } func (p *Provider) DeletePIM(ctx context.Context, _ *provider.DeletePIMRequest) error { @@ -2515,7 +2515,7 @@ func (p *Provider) EnsureBorderGatewaySettings(ctx context.Context, req *BorderG return err } - conf := make([]gnmiext.Configurable, 0, 3) + updates := make([]gnmiext.DataElement, 0, 3) bg := new(MultisiteItems) bg.AdminSt = AdminStEnabled if req.BorderGateway.Spec.AdminState == v1alpha1.AdminStateDown { @@ -2526,10 +2526,10 @@ func (p *Provider) EnsureBorderGatewaySettings(ctx context.Context, req *BorderG if bg.DelayRestoreSeconds < 30 || bg.DelayRestoreSeconds > 1000 { return fmt.Errorf("border gateway: delay restore time %d seconds is out of range (30-1000)", bg.DelayRestoreSeconds) } - conf = append(conf, bg) + updates = append(updates, bg) bgi := MultisiteBorderGatewayInterface(req.SourceInterface.Spec.Name) - conf = append(conf, &bgi) + updates = append(updates, &bgi) sc := new(StormControlItems) for _, cfg := range req.BorderGateway.Spec.StormControl { @@ -2552,11 +2552,11 @@ func (p *Provider) EnsureBorderGatewaySettings(ctx context.Context, req *BorderG sc.EvpnStormControlList.Set(ctrl) } - del := make([]gnmiext.Configurable, 0, 1) + deletes := make([]gnmiext.DataElement, 0, 1) if sc.EvpnStormControlList.Len() == 0 { - del = append(del, sc) + deletes = append(deletes, sc) } else { - conf = append(conf, sc) + updates = append(updates, sc) } peerItems := new(MultisitePeerItems) @@ -2578,7 +2578,7 @@ func (p *Provider) EnsureBorderGatewaySettings(ctx context.Context, req *BorderG ic, ok := interconnects[intf.ID] if !ok { if intf.MultisiteIfTracking != nil { - del = append(del, intf.MultisiteIfTracking) + deletes = append(deletes, intf.MultisiteIfTracking) } continue } @@ -2588,7 +2588,7 @@ func (p *Provider) EnsureBorderGatewaySettings(ctx context.Context, req *BorderG } intf.MultisiteIfTracking.IfName = intf.ID intf.MultisiteIfTracking.Tracking = MultisiteIfTrackingModeFrom(ic.Tracking) - conf = append(conf, intf.MultisiteIfTracking) + updates = append(updates, intf.MultisiteIfTracking) } for _, peer := range peerItems.PeerList { @@ -2597,23 +2597,23 @@ func (p *Provider) EnsureBorderGatewaySettings(ctx context.Context, req *BorderG }) if idx == -1 { if peer.PeerType != "" { - del = append(del, &MultisitePeer{Addr: peer.Addr}) + deletes = append(deletes, &MultisitePeer{Addr: peer.Addr}) } continue } - conf = append(conf, &MultisitePeer{Addr: peer.Addr, PeerType: BorderGatewayPeerTypeFrom(req.Peers[idx].PeerType)}) + updates = append(updates, &MultisitePeer{Addr: peer.Addr, PeerType: BorderGatewayPeerTypeFrom(req.Peers[idx].PeerType)}) } - if err := p.client.Delete(ctx, del...); err != nil { + if err := p.client.Delete(ctx, deletes...); err != nil { return err } - return p.Update(ctx, conf...) + return p.Update(ctx, updates...) } func (p *Provider) ResetBorderGatewaySettings(ctx context.Context) error { - conf := []gnmiext.Configurable{new(MultisiteItems), new(MultisiteBorderGatewayInterface), new(StormControlItems)} + deletes := []gnmiext.DataElement{new(MultisiteItems), new(MultisiteBorderGatewayInterface), new(StormControlItems)} peerItems := new(MultisitePeerItems) trackingItems := new(MultisiteIfTrackingItems) if err := p.client.GetConfig(ctx, trackingItems, peerItems); err != nil && !errors.Is(err, gnmiext.ErrNil) { @@ -2621,15 +2621,15 @@ func (p *Provider) ResetBorderGatewaySettings(ctx context.Context) error { } for _, intf := range trackingItems.PhysIfList { if intf.MultisiteIfTracking != nil { - conf = append(conf, intf.MultisiteIfTracking) + deletes = append(deletes, intf.MultisiteIfTracking) } } for _, peer := range peerItems.PeerList { if peer.PeerType != "" { - conf = append(conf, &MultisitePeer{Addr: peer.Addr}) + deletes = append(deletes, &MultisitePeer{Addr: peer.Addr}) } } - return p.client.Delete(ctx, conf...) + return p.client.Delete(ctx, deletes...) } // EnsureNVE ensures that the NVE configuration on the device matches the desired state specified in the NVE custom resource. @@ -2692,8 +2692,8 @@ func (p *Provider) EnsureNVE(ctx context.Context, req *provider.NVERequest) erro n.AdvertiseVmac = vc.Spec.AdvertiseVirtualMAC } - conf := make([]gnmiext.Configurable, 0, 3) - conf = append(conf, n) + patches := make([]gnmiext.DataElement, 0, 3) + patches = append(patches, n) iv := new(NVEInfraVLANs) for _, ivList := range vc.Spec.InfraVLANs { @@ -2716,7 +2716,7 @@ func (p *Provider) EnsureNVE(ctx context.Context, req *provider.NVERequest) erro } } } else { - conf = append(conf, iv) + patches = append(patches, iv) } ag := new(FabricFwd) @@ -2724,9 +2724,9 @@ func (p *Provider) EnsureNVE(ctx context.Context, req *provider.NVERequest) erro ag.AdminSt = string(AdminStEnabled) ag.Address = req.NVE.Spec.AnycastGateway.VirtualMAC } - conf = append(conf, ag) + patches = append(patches, ag) - return p.Patch(ctx, conf...) + return p.Patch(ctx, patches...) } func (p *Provider) DeleteNVE(ctx context.Context, req *provider.NVERequest) error { @@ -2864,43 +2864,43 @@ func (p *Provider) GetLLDPStatus(ctx context.Context, req *provider.LLDPRequest) return s, nil } -func (p *Provider) Patch(ctx context.Context, conf ...gnmiext.Configurable) error { +func (p *Provider) Patch(ctx context.Context, patches ...gnmiext.DataElement) error { if NXVersion(p.client.Capabilities()) > VersionNX10_6_2 { - return p.client.Patch(ctx, conf...) + return p.client.Patch(ctx, patches...) } - fa, conf := separateFeatureActivation(conf) + fa, patches := separateFeatureActivation(patches) if err := p.client.Patch(ctx, fa...); err != nil { return err } - return p.client.Patch(ctx, conf...) + return p.client.Patch(ctx, patches...) } -func (p *Provider) Update(ctx context.Context, conf ...gnmiext.Configurable) error { +func (p *Provider) Update(ctx context.Context, updates ...gnmiext.DataElement) error { if NXVersion(p.client.Capabilities()) > VersionNX10_6_2 { - return p.client.Update(ctx, conf...) + return p.client.Update(ctx, updates...) } - fa, conf := separateFeatureActivation(conf) + fa, updates := separateFeatureActivation(updates) if err := p.client.Update(ctx, fa...); err != nil { return err } - return p.client.Update(ctx, conf...) + return p.client.Update(ctx, updates...) } // separateFeatureActivation separates feature activation configurations from other configurations. // This is necessary for NX-OS versions <= 10.6(2) where feature activation must be performed before applying configurations. // For more details, see: https://github.com/ironcore-dev/network-operator/issues/148 -func separateFeatureActivation(conf []gnmiext.Configurable) (features, others []gnmiext.Configurable) { +func separateFeatureActivation(el []gnmiext.DataElement) (features, others []gnmiext.DataElement) { n := 0 - fa := make([]gnmiext.Configurable, 0, len(conf)) - for _, c := range conf { - if f, ok := c.(*Feature); ok { + fa := make([]gnmiext.DataElement, 0, len(el)) + for _, e := range el { + if f, ok := e.(*Feature); ok { fa = append(fa, f) continue } - conf[n] = c + el[n] = e n++ } - return fa, conf[:n:n] + return fa, el[:n:n] } // EnsureDHCPRelay configures DHCP relay on the specified interfaces. @@ -2916,7 +2916,7 @@ func (p *Provider) EnsureDHCPRelay(ctx context.Context, req *provider.DHCPRelayR vrfName = req.VRF.Spec.Name } - conf := new(DHCPRelayConfig) + updates := new(DHCPRelayConfig) for _, intf := range req.Interfaces { ifName, err := ShortName(intf.Spec.Name) if err != nil { @@ -2931,10 +2931,10 @@ func (p *Provider) EnsureDHCPRelay(ctx context.Context, req *provider.DHCPRelayR } relay.AddrItems.AddrList.Set(&DHCPRelayServer{Address: a, Vrf: vrfName}) } - conf.RelayIfList.Set(relay) + updates.RelayIfList.Set(relay) } - return p.Update(ctx, f, conf) + return p.Update(ctx, f, updates) } // DeleteDHCPRelay removes all DHCP relay configurations from the device. diff --git a/internal/provider/cisco/nxos/provider_test.go b/internal/provider/cisco/nxos/provider_test.go index 719662dd1..c0951a169 100644 --- a/internal/provider/cisco/nxos/provider_test.go +++ b/internal/provider/cisco/nxos/provider_test.go @@ -20,12 +20,12 @@ import ( type TestCase struct { name string - val gnmiext.Configurable + val gnmiext.DataElement } var tests []TestCase -func Register(name string, val gnmiext.Configurable) { +func Register(name string, val gnmiext.DataElement) { tests = append(tests, TestCase{ name: name, val: val, diff --git a/internal/provider/cisco/nxos/routemap.go b/internal/provider/cisco/nxos/routemap.go index 757ab5486..50c4cdf7e 100644 --- a/internal/provider/cisco/nxos/routemap.go +++ b/internal/provider/cisco/nxos/routemap.go @@ -8,7 +8,7 @@ import ( "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) -var _ gnmiext.Configurable = (*RouteMap)(nil) +var _ gnmiext.DataElement = (*RouteMap)(nil) type RouteMap struct { Name string `json:"name"` diff --git a/internal/provider/cisco/nxos/snmp.go b/internal/provider/cisco/nxos/snmp.go index f4e97d978..750e98094 100644 --- a/internal/provider/cisco/nxos/snmp.go +++ b/internal/provider/cisco/nxos/snmp.go @@ -10,13 +10,13 @@ import ( ) var ( - _ gnmiext.Configurable = (*SNMPSysInfo)(nil) - _ gnmiext.Configurable = (*SNMPSrcIf)(nil) - _ gnmiext.Configurable = (*SNMPUser)(nil) - _ gnmiext.Configurable = (*SNMPHostItems)(nil) - _ gnmiext.Configurable = (*SNMPHost)(nil) - _ gnmiext.Configurable = (*SNMPCommunityItems)(nil) - _ gnmiext.Configurable = (*SNMPCommunity)(nil) + _ gnmiext.DataElement = (*SNMPSysInfo)(nil) + _ gnmiext.DataElement = (*SNMPSrcIf)(nil) + _ gnmiext.DataElement = (*SNMPUser)(nil) + _ gnmiext.DataElement = (*SNMPHostItems)(nil) + _ gnmiext.DataElement = (*SNMPHost)(nil) + _ gnmiext.DataElement = (*SNMPCommunityItems)(nil) + _ gnmiext.DataElement = (*SNMPCommunity)(nil) ) // SNMPSysInfo represents the SNMP system information configuration on a NX-OS device. diff --git a/internal/provider/cisco/nxos/syslog.go b/internal/provider/cisco/nxos/syslog.go index b65d83baa..5ef25f04d 100644 --- a/internal/provider/cisco/nxos/syslog.go +++ b/internal/provider/cisco/nxos/syslog.go @@ -9,11 +9,11 @@ import ( ) var ( - _ gnmiext.Configurable = (*SyslogOrigin)(nil) - _ gnmiext.Configurable = (*SyslogSrcIf)(nil) - _ gnmiext.Configurable = (*SyslogHistory)(nil) - _ gnmiext.Configurable = (*SyslogRemoteItems)(nil) - _ gnmiext.Configurable = (*SyslogFacilityItems)(nil) + _ gnmiext.DataElement = (*SyslogOrigin)(nil) + _ gnmiext.DataElement = (*SyslogSrcIf)(nil) + _ gnmiext.DataElement = (*SyslogHistory)(nil) + _ gnmiext.DataElement = (*SyslogRemoteItems)(nil) + _ gnmiext.DataElement = (*SyslogFacilityItems)(nil) ) type SyslogOrigin struct { diff --git a/internal/provider/cisco/nxos/system.go b/internal/provider/cisco/nxos/system.go index b5cbf0ac9..5b4acb950 100644 --- a/internal/provider/cisco/nxos/system.go +++ b/internal/provider/cisco/nxos/system.go @@ -21,11 +21,11 @@ import ( const Manufacturer = "Cisco" var ( - _ gnmiext.Configurable = (*SystemJumboMTU)(nil) - _ gnmiext.Defaultable = (*SystemJumboMTU)(nil) - _ gnmiext.Configurable = (*Model)(nil) - _ gnmiext.Configurable = (*SerialNumber)(nil) - _ gnmiext.Configurable = (*FirmwareVersion)(nil) + _ gnmiext.DataElement = (*SystemJumboMTU)(nil) + _ gnmiext.Defaultable = (*SystemJumboMTU)(nil) + _ gnmiext.DataElement = (*Model)(nil) + _ gnmiext.DataElement = (*SerialNumber)(nil) + _ gnmiext.DataElement = (*FirmwareVersion)(nil) ) // SystemJumboMTU represents the jumbo MTU size configured on the system. @@ -61,7 +61,7 @@ func (*FirmwareVersion) XPath() string { return "System/showversion-items/nxosVersion" } -var _ gnmiext.Configurable = (*BootPOAP)(nil) +var _ gnmiext.DataElement = (*BootPOAP)(nil) type BootPOAP string diff --git a/internal/provider/cisco/nxos/term.go b/internal/provider/cisco/nxos/term.go index 086d9adba..7e7cda32e 100644 --- a/internal/provider/cisco/nxos/term.go +++ b/internal/provider/cisco/nxos/term.go @@ -10,11 +10,11 @@ import ( ) var ( - _ gnmiext.Configurable = (*Console)(nil) - _ gnmiext.Defaultable = (*Console)(nil) - _ gnmiext.Configurable = (*VTY)(nil) - _ gnmiext.Defaultable = (*VTY)(nil) - _ gnmiext.Configurable = (*VTYAccessClass)(nil) + _ gnmiext.DataElement = (*Console)(nil) + _ gnmiext.Defaultable = (*Console)(nil) + _ gnmiext.DataElement = (*VTY)(nil) + _ gnmiext.Defaultable = (*VTY)(nil) + _ gnmiext.DataElement = (*VTYAccessClass)(nil) ) // Console represents the primary terminal line configuration. diff --git a/internal/provider/cisco/nxos/user.go b/internal/provider/cisco/nxos/user.go index d0b266ef8..844805121 100644 --- a/internal/provider/cisco/nxos/user.go +++ b/internal/provider/cisco/nxos/user.go @@ -18,7 +18,7 @@ import ( "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) -var _ gnmiext.Configurable = (*User)(nil) +var _ gnmiext.DataElement = (*User)(nil) // User represents a local user on a NX-OS device. type User struct { diff --git a/internal/provider/cisco/nxos/vlan.go b/internal/provider/cisco/nxos/vlan.go index 45f5aa775..a52642589 100644 --- a/internal/provider/cisco/nxos/vlan.go +++ b/internal/provider/cisco/nxos/vlan.go @@ -10,13 +10,13 @@ import ( ) var ( - _ gnmiext.Configurable = (*VLANSystem)(nil) - _ gnmiext.Defaultable = (*VLANSystem)(nil) - _ gnmiext.Configurable = (*VLANReservation)(nil) - _ gnmiext.Defaultable = (*VLANReservation)(nil) - _ gnmiext.Configurable = (*VLAN)(nil) - _ gnmiext.Configurable = (*VLANOperItems)(nil) - _ gnmiext.Configurable = (*VXLAN)(nil) + _ gnmiext.DataElement = (*VLANSystem)(nil) + _ gnmiext.Defaultable = (*VLANSystem)(nil) + _ gnmiext.DataElement = (*VLANReservation)(nil) + _ gnmiext.Defaultable = (*VLANReservation)(nil) + _ gnmiext.DataElement = (*VLAN)(nil) + _ gnmiext.DataElement = (*VLANOperItems)(nil) + _ gnmiext.DataElement = (*VXLAN)(nil) ) // VLANSystem represents the settings shared among all VLANs diff --git a/internal/provider/cisco/nxos/vpc.go b/internal/provider/cisco/nxos/vpc.go index 49e912d86..ca7676e43 100644 --- a/internal/provider/cisco/nxos/vpc.go +++ b/internal/provider/cisco/nxos/vpc.go @@ -15,8 +15,8 @@ import ( ) var ( - _ gnmiext.Configurable = (*VPCDomain)(nil) - _ gnmiext.Configurable = (*VPCIf)(nil) + _ gnmiext.DataElement = (*VPCDomain)(nil) + _ gnmiext.DataElement = (*VPCIf)(nil) ) // VPCDomain represents the domain of a virtual Port Channel (vPC) diff --git a/internal/provider/cisco/nxos/vrf.go b/internal/provider/cisco/nxos/vrf.go index d6bf04a61..f34f5cab2 100644 --- a/internal/provider/cisco/nxos/vrf.go +++ b/internal/provider/cisco/nxos/vrf.go @@ -11,7 +11,7 @@ const ( ManagementVRFName = "management" ) -var _ gnmiext.Configurable = (*VRF)(nil) +var _ gnmiext.DataElement = (*VRF)(nil) type VRF struct { Encap Option[string] `json:"encap"` diff --git a/internal/transport/gnmiext/client.go b/internal/transport/gnmiext/client.go index 3abc2b421..ec7edc718 100644 --- a/internal/transport/gnmiext/client.go +++ b/internal/transport/gnmiext/client.go @@ -22,9 +22,12 @@ import ( "google.golang.org/grpc/status" ) -// Configurable represents a configuration item with a YANG path. -type Configurable interface { - // XPath returns the YANG path for this configuration item. +// DataElement represents a data element addressable by a YANG path. +// A data element can refer to any level of the data tree — a single leaf, +// a container, or an entire subtree — and may carry either configuration +// or state data. +type DataElement interface { + // XPath returns the YANG path for this data element. // It may include an origin prefix (e.g., "openconfig:system/config/hostname"). XPath() string } @@ -60,11 +63,11 @@ type Capabilities struct { type Client interface { Capabilities() *Capabilities - GetConfig(ctx context.Context, conf ...Configurable) error - GetState(ctx context.Context, conf ...Configurable) error - Patch(ctx context.Context, conf ...Configurable) error - Update(ctx context.Context, conf ...Configurable) error - Delete(ctx context.Context, conf ...Configurable) error + GetConfig(context.Context, ...DataElement) error + GetState(context.Context, ...DataElement) error + Patch(context.Context, ...DataElement) error + Update(context.Context, ...DataElement) error + Delete(context.Context, ...DataElement) error } // Client is a gNMI client offering convenience methods for device configuration @@ -136,57 +139,57 @@ func (c *client) Capabilities() *Capabilities { // GetConfig retrieves config and unmarshals it into the provided targets. // If some of the values for the given xpaths are not defined, [ErrNil] is returned. -func (c *client) GetConfig(ctx context.Context, conf ...Configurable) error { - return c.get(ctx, gpb.GetRequest_CONFIG, conf...) +func (c *client) GetConfig(ctx context.Context, el ...DataElement) error { + return c.get(ctx, gpb.GetRequest_CONFIG, el...) } // GetState retrieves state and unmarshals it into the provided targets. // If some of the values for the given xpaths are not defined, [ErrNil] is returned. -func (c *client) GetState(ctx context.Context, conf ...Configurable) error { - return c.get(ctx, gpb.GetRequest_STATE, conf...) +func (c *client) GetState(ctx context.Context, el ...DataElement) error { + return c.get(ctx, gpb.GetRequest_STATE, el...) } // Update replaces the configuration for the given set of items.4c890d // If the current configuration equals the desired configuration, the operation is skipped. // For partial updates that merge changes, use [Client.Patch] instead. -func (c *client) Update(ctx context.Context, conf ...Configurable) error { - return c.set(ctx, false, conf...) +func (c *client) Update(ctx context.Context, el ...DataElement) error { + return c.set(ctx, false, el...) } // Patch merges the configuration for the given set of items. // If the current configuration equals the desired configuration, the operation is skipped. // For full replacement of configuration, use [Client.Update] instead. -func (c *client) Patch(ctx context.Context, conf ...Configurable) error { - return c.set(ctx, true, conf...) +func (c *client) Patch(ctx context.Context, el ...DataElement) error { + return c.set(ctx, true, el...) } // Delete resets the configuration for the given set of items. // If an item implements [Defaultable], it's reset to default value. // Otherwise, the configuration is deleted. -func (c *client) Delete(ctx context.Context, conf ...Configurable) error { - if len(conf) == 0 { +func (c *client) Delete(ctx context.Context, el ...DataElement) error { + if len(el) == 0 { return nil } r := new(gpb.SetRequest) - for _, cf := range conf { - path, err := StringToStructuredPath(cf.XPath()) + for _, e := range el { + path, err := StringToStructuredPath(e.XPath()) if err != nil { return err } - if d, ok := cf.(Defaultable); ok { + if d, ok := e.(Defaultable); ok { d.Default() - b, err := c.Marshal(cf) + b, err := c.Marshal(e) if err != nil { return err } - c.logger.V(1).Info("Resetting to default", "path", cf.XPath(), "payload", string(b)) + c.logger.V(1).Info("Resetting to default", "path", e.XPath(), "payload", string(b)) r.Replace = append(r.Replace, &gpb.Update{ Path: path, Val: c.Encode(b), }) continue } - c.logger.V(1).Info("Deleting", "path", cf.XPath()) + c.logger.V(1).Info("Deleting", "path", e.XPath()) r.Delete = append(r.Delete, path) } if _, err := c.gnmi.Set(ctx, r); err != nil { @@ -198,16 +201,16 @@ func (c *client) Delete(ctx context.Context, conf ...Configurable) error { // get retrieves data of the specified type (CONFIG or STATE) and unmarshals it // into the provided targets. If some of the values for the given xpaths are not // defined, [ErrNil] is returned. -func (c *client) get(ctx context.Context, dt gpb.GetRequest_DataType, conf ...Configurable) error { - if len(conf) == 0 { +func (c *client) get(ctx context.Context, dt gpb.GetRequest_DataType, el ...DataElement) error { + if len(el) == 0 { return nil } r := &gpb.GetRequest{ Type: dt, Encoding: c.encoding, } - for _, cf := range conf { - path, err := StringToStructuredPath(cf.XPath()) + for _, e := range el { + path, err := StringToStructuredPath(e.XPath()) if err != nil { return err } @@ -222,15 +225,15 @@ func (c *client) get(ctx context.Context, dt gpb.GetRequest_DataType, conf ...Co // // [gNMI spec]: https://github.com/openconfig/reference/blob/master/rpc/gnmi/gnmi-specification.md#332-the-getresponse-message notifications := res.GetNotification() - if len(notifications) != len(conf) { + if len(notifications) != len(el) { // This should never happen. If it does, it indicates a bug in the // gNMI server. - return fmt.Errorf("gnmiext: unexpected number of notifications: got %d, want %d", len(notifications), len(conf)) + return fmt.Errorf("gnmiext: unexpected number of notifications: got %d, want %d", len(notifications), len(el)) } // prevent bounds check in for the range loop below // [Bounds Check Elimination]: https://go101.org/optimizations/5-bce.html - _ = notifications[len(conf)-1] - for i, cf := range conf { + _ = notifications[len(el)-1] + for i, e := range el { n := notifications[i] switch len(n.GetUpdate()) { case 0: @@ -249,7 +252,7 @@ func (c *client) get(ctx context.Context, dt gpb.GetRequest_DataType, conf ...Co if len(b) == 0 { return ErrNil } - if err := c.Unmarshal(b, cf); err != nil { + if err := c.Unmarshal(b, e); err != nil { return err } default: @@ -264,32 +267,32 @@ func (c *client) get(ctx context.Context, dt gpb.GetRequest_DataType, conf ...Co // configuration. Otherwise, a full replacement is done. // If the current configuration equals the desired configuration, the operation // is skipped. -func (c *client) set(ctx context.Context, patch bool, conf ...Configurable) error { - if len(conf) == 0 { +func (c *client) set(ctx context.Context, patch bool, el ...DataElement) error { + if len(el) == 0 { return nil } r := new(gpb.SetRequest) - for _, cf := range conf { - path, err := StringToStructuredPath(cf.XPath()) + for _, e := range el { + path, err := StringToStructuredPath(e.XPath()) if err != nil { return err } - got := cp.Deep(cf) + got := cp.Deep(e) err = c.GetConfig(ctx, got) if err != nil && !errors.Is(err, ErrNil) && status.Code(err) != codes.NotFound { - return fmt.Errorf("gnmiext: failed to retrieve current config for %s: %w", cf.XPath(), err) + return fmt.Errorf("gnmiext: failed to retrieve current config for %s: %w", e.XPath(), err) } // If the current configuration is equal to the desired configuration, skip the update. // This avoids unnecessary updates and potential disruptions. - if err == nil && reflect.DeepEqual(cf, got) { - c.logger.V(2).Info("Configuration is already up-to-date", "path", cf.XPath()) + if err == nil && reflect.DeepEqual(e, got) { + c.logger.V(2).Info("Configuration is already up-to-date", "path", e.XPath()) continue } - b, err := c.Marshal(cf) + b, err := c.Marshal(e) if err != nil { return err } - c.logger.V(1).Info("Updating", "path", cf.XPath(), "payload", string(b), "patch", patch) + c.logger.V(1).Info("Updating", "path", e.XPath(), "payload", string(b), "patch", patch) u := &gpb.Update{ Path: path, Val: c.Encode(b), diff --git a/internal/transport/gnmiext/client_test.go b/internal/transport/gnmiext/client_test.go index 6d28f96e8..a34ce142a 100644 --- a/internal/transport/gnmiext/client_test.go +++ b/internal/transport/gnmiext/client_test.go @@ -105,7 +105,7 @@ func TestClient_GetConfig(t *testing.T) { tests := []struct { name string conn grpc.ClientConnInterface - conf []Configurable + configs []DataElement wantErr bool }{ { @@ -156,7 +156,7 @@ func TestClient_GetConfig(t *testing.T) { }, nil }, }, - conf: []Configurable{new(Hostname)}, + configs: []DataElement{new(Hostname)}, wantErr: false, }, { @@ -226,12 +226,12 @@ func TestClient_GetConfig(t *testing.T) { }, nil }, }, - conf: []Configurable{new(Hostname), new(Hostname)}, + configs: []DataElement{new(Hostname), new(Hostname)}, wantErr: false, }, { name: "Empty list", - conf: []Configurable{}, + configs: []DataElement{}, wantErr: false, }, { @@ -260,7 +260,7 @@ func TestClient_GetConfig(t *testing.T) { return nil, errors.New("get rpc failed") }, }, - conf: []Configurable{new(Hostname)}, + configs: []DataElement{new(Hostname)}, wantErr: true, }, { @@ -291,7 +291,7 @@ func TestClient_GetConfig(t *testing.T) { }, nil }, }, - conf: []Configurable{new(Hostname)}, + configs: []DataElement{new(Hostname)}, wantErr: true, }, { @@ -326,7 +326,7 @@ func TestClient_GetConfig(t *testing.T) { }, nil }, }, - conf: []Configurable{new(Hostname)}, + configs: []DataElement{new(Hostname)}, wantErr: true, }, { @@ -377,7 +377,7 @@ func TestClient_GetConfig(t *testing.T) { }, nil }, }, - conf: []Configurable{new(Hostname)}, + configs: []DataElement{new(Hostname)}, wantErr: true, }, { @@ -443,7 +443,7 @@ func TestClient_GetConfig(t *testing.T) { }, nil }, }, - conf: []Configurable{new(Hostname)}, + configs: []DataElement{new(Hostname)}, wantErr: true, }, { @@ -494,7 +494,7 @@ func TestClient_GetConfig(t *testing.T) { }, nil }, }, - conf: []Configurable{new(Hostname)}, + configs: []DataElement{new(Hostname)}, wantErr: true, }, } @@ -506,7 +506,7 @@ func TestClient_GetConfig(t *testing.T) { gnmi: gpb.NewGNMIClient(test.conn), } - err := client.GetConfig(t.Context(), test.conf...) + err := client.GetConfig(t.Context(), test.configs...) if (err != nil) != test.wantErr { t.Errorf("GetConfig() error = %v, wantErr %v", err, test.wantErr) } @@ -518,7 +518,7 @@ func TestClient_GetState(t *testing.T) { tests := []struct { name string conn grpc.ClientConnInterface - conf []Configurable + states []DataElement wantErr bool }{ { @@ -569,12 +569,12 @@ func TestClient_GetState(t *testing.T) { }, nil }, }, - conf: []Configurable{new(HostnameState)}, + states: []DataElement{new(HostnameState)}, wantErr: false, }, { name: "Empty list", - conf: []Configurable{}, + states: []DataElement{}, wantErr: false, }, } @@ -586,7 +586,7 @@ func TestClient_GetState(t *testing.T) { gnmi: gpb.NewGNMIClient(test.conn), } - err := client.GetState(t.Context(), test.conf...) + err := client.GetState(t.Context(), test.states...) if (err != nil) != test.wantErr { t.Errorf("GetState() error = %v, wantErr %v", err, test.wantErr) } @@ -598,7 +598,7 @@ func TestClient_Update(t *testing.T) { tests := []struct { name string conn grpc.ClientConnInterface - conf []Configurable + updates []DataElement wantErr bool }{ { @@ -680,7 +680,7 @@ func TestClient_Update(t *testing.T) { }, nil }, }, - conf: []Configurable{new(Hostname("new-hostname"))}, + updates: []DataElement{new(Hostname("new-hostname"))}, wantErr: false, }, { @@ -731,7 +731,7 @@ func TestClient_Update(t *testing.T) { }, nil }, }, - conf: []Configurable{new(Hostname("test-hostname"))}, + updates: []DataElement{new(Hostname("test-hostname"))}, wantErr: false, }, { @@ -760,7 +760,7 @@ func TestClient_Update(t *testing.T) { return nil, errors.New("get rpc failed") }, }, - conf: []Configurable{new(Hostname("test-hostname"))}, + updates: []DataElement{new(Hostname("test-hostname"))}, wantErr: true, }, { @@ -840,12 +840,12 @@ func TestClient_Update(t *testing.T) { return nil, errors.New("set rpc failed") }, }, - conf: []Configurable{new(Hostname("new-hostname"))}, + updates: []DataElement{new(Hostname("new-hostname"))}, wantErr: true, }, { name: "Empty list", - conf: []Configurable{}, + updates: []DataElement{}, wantErr: false, }, } @@ -857,7 +857,7 @@ func TestClient_Update(t *testing.T) { gnmi: gpb.NewGNMIClient(test.conn), } - err := client.Update(t.Context(), test.conf...) + err := client.Update(t.Context(), test.updates...) if (err != nil) != test.wantErr { t.Errorf("Update() error = %v, wantErr %v", err, test.wantErr) } @@ -869,7 +869,7 @@ func TestClient_Patch(t *testing.T) { tests := []struct { name string conn grpc.ClientConnInterface - conf []Configurable + patches []DataElement wantErr bool }{ { @@ -951,7 +951,7 @@ func TestClient_Patch(t *testing.T) { }, nil }, }, - conf: []Configurable{new(Hostname("new-hostname"))}, + patches: []DataElement{new(Hostname("new-hostname"))}, wantErr: false, }, { @@ -1002,12 +1002,12 @@ func TestClient_Patch(t *testing.T) { }, nil }, }, - conf: []Configurable{new(Hostname("test-hostname"))}, + patches: []DataElement{new(Hostname("test-hostname"))}, wantErr: false, }, { name: "Empty list", - conf: []Configurable{}, + patches: []DataElement{}, wantErr: false, }, } @@ -1019,7 +1019,7 @@ func TestClient_Patch(t *testing.T) { gnmi: gpb.NewGNMIClient(test.conn), } - err := client.Patch(t.Context(), test.conf...) + err := client.Patch(t.Context(), test.patches...) if (err != nil) != test.wantErr { t.Errorf("Update() error = %v, wantErr %v", err, test.wantErr) } @@ -1031,7 +1031,7 @@ func TestClient_Delete(t *testing.T) { tests := []struct { name string conn grpc.ClientConnInterface - conf []Configurable + deletes []DataElement wantErr bool }{ { @@ -1059,7 +1059,7 @@ func TestClient_Delete(t *testing.T) { }, nil }, }, - conf: []Configurable{new(Hostname)}, + deletes: []DataElement{new(Hostname)}, wantErr: false, }, { @@ -1094,7 +1094,7 @@ func TestClient_Delete(t *testing.T) { }, nil }, }, - conf: []Configurable{new(DefaultableHostname)}, + deletes: []DataElement{new(DefaultableHostname)}, wantErr: false, }, { @@ -1120,12 +1120,12 @@ func TestClient_Delete(t *testing.T) { return nil, errors.New("set rpc failed") }, }, - conf: []Configurable{new(Hostname)}, + deletes: []DataElement{new(Hostname)}, wantErr: true, }, { name: "Empty list", - conf: []Configurable{}, + deletes: []DataElement{}, wantErr: false, }, } @@ -1137,7 +1137,7 @@ func TestClient_Delete(t *testing.T) { gnmi: gpb.NewGNMIClient(test.conn), } - err := client.Delete(context.Background(), test.conf...) + err := client.Delete(context.Background(), test.deletes...) if (err != nil) != test.wantErr { t.Errorf("Delete() error = %v, wantErr %v", err, test.wantErr) } @@ -1327,7 +1327,7 @@ func TestClient_Unmarshal(t *testing.T) { type Hostname string -var _ Configurable = (*Hostname)(nil) +var _ DataElement = (*Hostname)(nil) func (*Hostname) XPath() string { return "openconfig:system/config/hostname" } @@ -1335,7 +1335,7 @@ func (*Hostname) XPath() string { return "openconfig:system/config/hostname" } type HostnameState string -var _ Configurable = (*HostnameState)(nil) +var _ DataElement = (*HostnameState)(nil) func (*HostnameState) XPath() string { return "openconfig:system/state/hostname" } @@ -1344,8 +1344,8 @@ func (*HostnameState) XPath() string { return "openconfig:system/state/hostname" type DefaultableHostname string var ( - _ Configurable = (*DefaultableHostname)(nil) - _ Defaultable = (*DefaultableHostname)(nil) + _ DataElement = (*DefaultableHostname)(nil) + _ Defaultable = (*DefaultableHostname)(nil) ) func (*DefaultableHostname) XPath() string { return "openconfig:system/config/hostname" }