feat(openconfig): Implement ManagementAccess provider - #485
Conversation
Add EnsureManagementAccess and DeleteManagementAccess to the OpenConfig provider using openconfig-system YANG paths: - gRPC server: /system/grpc-servers/grpc-server[name=gnmi]/config - SSH server: /system/ssh-server/config Unsupported fields (spec.grpc.gnmi, spec.ssh.sessionLimit) are rejected with a terminal UnsupportedFieldError, following the DNS provider pattern. Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Robert Gildein <rgildein@users.noreply.github.com>
| Timeout uint32 `json:"timeout,omitempty"` | ||
| } | ||
|
|
||
| func (s *SSHServer) XPath() string { |
There was a problem hiding this comment.
| func (s *SSHServer) XPath() string { | |
| func (*SSHServer) XPath() string { |
in such cases, we can omit the receiver variable name
Signed-off-by: Robert Gildein <rgildein@users.noreply.github.com>
Merging this branch will not change overall coverage
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. |
|
|
||
| var _ provider.ManagementAccessProvider = (*Provider)(nil) | ||
|
|
||
| const grpcServerName = "gnmi" |
There was a problem hiding this comment.
I don't think this is a universal standard that is set by openconfig. As such, do we want to hardcode this name here, or should we rather allow to set this name from the spec? On a provider such as cisco nx-os which doesn't support multiple servers, this field could then perhaps be ignored?
There was a problem hiding this comment.
I was trying to follow what we have for NX-OS, where we do not set the name. My assumptions was that there is only one resource kind: ManagementAccess per device, so setting generic name (maybe we can go with even something like 'network-operator-gnmi'). Is that true or it's excepted to have multiple resources?
If you think it will be useful, we can do as you say add this to the spec and ignore it for Cisco provider. BTW, I think that Cisco support multiple servers, but without names.
There was a problem hiding this comment.
I think we'll only have one kind: ManagementAccess resource per Device. However, I think some device platforms (like Nokia SRLinux) can have multiple named grpc servers. On Cisco NX-OS I'm only aware that there is a single grpc server without a name. Where did you read that Cisco NX-OS supports multiple grpc servers?
There was a problem hiding this comment.
Here, it's not specifically mentioned how, but that two server can used.
Anyway, I think it makes sense to add name to the spec and raise error if it's used with Cisco provider.
Description
Add EnsureManagementAccess and DeleteManagementAccess to the OpenConfig provider using openconfig-system YANG paths:
Unsupported fields (spec.grpc.gnmi, spec.ssh.sessionLimit) are rejected with a terminal UnsupportedFieldError, following the DNS provider pattern.
My prompt
Claude plan
Plan: Add ManagementAccess to OpenConfig Provider
Context
The
ManagementAccessCRD manages gRPC/gNMI and SSH server access on network devices. The NX-OS provider already implementsEnsureManagementAccessandDeleteManagementAccess, but the OpenConfig provider does not. The controller already checks if a provider implementsManagementAccessProvider(defined ininternal/provider/provider.go) and sets aNotImplementedReasonstatus when it doesn't. This task wires up the OpenConfig provider to configure devices using standard OpenConfig YANG paths fromopenconfig-system-grpcandopenconfig-system.OpenConfig YANG Paths
gRPC server (named list, key:
name):SSH server (singleton container):
Implementation
New file:
internal/provider/openconfig/managementaccess.goCompile-time assertion:
Two structs implementing
gnmiext.DataElement:GRPCServer— targets the grpc-server list item named"gnmi":openconfig-system:system/grpc-servers/grpc-server[name=gnmi]/configenable,port,certificate-id(omitempty),network-instance(omitempty)spec.grpc.*SSHServer— targets the SSH server config container:openconfig-system:system/ssh-server/configenable,timeout(seconds, uint32),session-limit(uint16)spec.ssh.*Duration conversion:
spec.grpc.gnmi.keepAliveTimeoutandspec.ssh.timeoutaremetav1.Duration. Convert to seconds with.Duration.Seconds()→uint32.EnsureManagementAccess:DeleteManagementAccess:Test data file
New file:
test/gnmi/testdata/openconfig/managementaccess.txtFormat mirrors
banner.txt— Kubernetes CR in the first block, expected JSON state in the second block:Critical files
internal/provider/openconfig/managementaccess.goEnsureManagementAccess,DeleteManagementAccess,GRPCServer,SSHServerstructstest/gnmi/testdata/openconfig/managementaccess.txtReference implementations to follow:
internal/provider/openconfig/banner.go— pattern forDataElementstructs andEnsureX/DeleteXmethodsinternal/provider/cisco/nxos/provider.go—EnsureManagementAccesslogic (field mapping)internal/provider/provider.go:388—ManagementAccessProviderinterface definitionVerification
/netop-setupthen/netop-testconfig/samples/v1alpha1_managementaccess.yamlagainst an OpenConfig devicetest/gnmi/testdata/openconfig/managementaccess.txtfixture is exercised by the existing test runner intest/gnmi/main.goManagementAccessCR reachesReady=TrueconditionClaude Test results
--- Test Report — ManagementAccess / OpenConfig Provider ┌──────────────────┬──────────────────┬───────────┬───────┬──────────────────────────────────────────────────────────────┬─────────────────┐ │ CR Name │ Kind │ Namespace │ Ready │ gNMI Path │ Result │ ├──────────────────┼──────────────────┼───────────┼───────┼──────────────────────────────────────────────────────────────┼─────────────────┤ │ managementaccess │ ManagementAccess │ default │ True │ openconfig-system:system/grpc-servers/grpc-server[name=gnmi] │ ✓ value matches │ │ │ │ │ │ /config │ │ ├──────────────────┼──────────────────┼───────────┼───────┼──────────────────────────────────────────────────────────────┼─────────────────┤ │ managementaccess │ ManagementAccess │ default │ True │ openconfig-system:system/ssh-server/config │ ✓ value matches │ └──────────────────┴──────────────────┴───────────┴───────┴──────────────────────────────────────────────────────────────┴─────────────────┘ --- Applied spec (kubectl get managementaccess managementaccess -n default -o yaml): spec: deviceRef: name: leaf1 grpc: certificateId: mytrustpoint enabled: true gnmi: keepAliveTimeout: 10m0s # default — ignored by OpenConfig provider maxConcurrentCall: 8 # default — ignored by OpenConfig provider port: 9339 vrfName: mgmt ssh: enabled: true sessionLimit: 32 # default — ignored by OpenConfig provider timeout: 120s --- gnmic get openconfig-system:system/grpc-servers/grpc-server[name=gnmi]/config ────────────────────────────────────────────────────────────────────── { "certificate-id": "mytrustpoint", "enable": true, "name": "gnmi", "network-instance": "mgmt", "port": 9339 } gnmic get openconfig-system:system/ssh-server/config ────────────────────────────────────────────────────────────────────── { "enable": true, "timeout": 120 } --- Operator logs — no errors on current CR. Previous session errors (now resolved): - leafref destination '.system.grpc-servers.grpc-server{.name=="gnmi"}.config.name' is not set — caused by wrong XPath format [gnmi] instead of [name=gnmi], fixed - schema '/system/ssh-server/config' has no local leaf 'session-limit' — field not in Nokia SRL model, dropped from SSHServer struct - leafref destination '.network-instance{.name=="default"}' does not exist — device only has mgmt network instance; sample updated to use vrfName: mgmt - UnsupportedField: spec.grpc.gnmi / spec.ssh.sessionLimit — validation correctly fires when user sets non-default values for unsupported fieldsManual test result