feat(openconfig): Implement ManagementAccess provider by rgildein · Pull Request #485 · ironcore-dev/network-operator

rgildein · 2026-08-06T14:16:56Z

Description

Add EnsureManagementAccess and DeleteManagementAccess to the OpenConfig provider using openconfig-system YANG paths:

gRPC server: /system/grpc-servers/grpc-server[name=]/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.

Adds a configurable spec.grpc.serverName field — defaults to "gnmi" on OpenConfig devices via cmp.Or. Raises UnsupportedFieldError on Cisco NX-OS where the server name is not configurable. Also introduces DeleteManagementAccessRequest to carry spec context (including serverName) into deletion.

My prompt

/plan Add Managementaccess to OpenConfig provider.
# context
The ManagementAccess CRD manages gRPC/gNMI and SSH server access on network devices. The NX-OS provider already have it, but OpenConfig not.
Use PR https://github.com/ironcore-dev/network-operator/pull/471 as inspiration.

# Goal
Add EnsureManagementAccess and DeleteManagementAccess to the OpenConfig provider using the standard OpenConfig YANG paths from:
 - openconfig-system-grpc (for gRPC server config)
 - openconfig-system (for SSH server config)

# Steps
1. check what parameters NetworkOperator consume for Managementaccess e.g. `ssh: timeout: 120s; sessionLimit: 10`
2. check OpenConfig paths https://openconfig.net/projects/models/schemadocs/yangdoc/openconfig-system.html#system-grpc-servers and https://openconfig.net/projects/models/schemadocs/yangdoc/openconfig-system.html#system-ssh-server
3. create EnsureManagementAccess and DeleteManagementAccess method
4. build and deploy Network-Operator
5. Use example configuretion @config/samples/v1alpha1_managementaccess.yaml to create new custom resource kind ManagementAccess
6. validate with gnmic that device was properly configured
7. @test/gnmi/testdata/openconfig/managementaccess.txt with config and expected output from gnmic

Claude plan

Plan: Add ManagementAccess to OpenConfig Provider

Context

The ManagementAccess CRD manages gRPC/gNMI and SSH server access on network devices. The NX-OS provider already implements EnsureManagementAccess and DeleteManagementAccess, but the OpenConfig provider does not. The controller already checks if a provider implements ManagementAccessProvider (defined in internal/provider/provider.go) and sets a NotImplementedReason status when it doesn't. This task wires up the OpenConfig provider to configure devices using standard OpenConfig YANG paths from openconfig-system-grpc and openconfig-system.

OpenConfig YANG Paths

gRPC server (named list, key: name):

openconfig-system:system/grpc-servers/grpc-server[name=gnmi]/config/enable           bool
openconfig-system:system/grpc-servers/grpc-server[name=gnmi]/config/port             uint16
openconfig-system:system/grpc-servers/grpc-server[name=gnmi]/config/certificate-id   string
openconfig-system:system/grpc-servers/grpc-server[name=gnmi]/config/network-instance string (VRF)

SSH server (singleton container):

openconfig-system:system/ssh-server/config/enable        bool
openconfig-system:system/ssh-server/config/timeout       uint32 (seconds)

Implementation

New file: `internal/provider/openconfig/managementaccess.go`

Compile-time assertion:

var _ provider.ManagementAccessProvider = (*Provider)(nil)

Two structs implementing gnmiext.DataElement:

GRPCServer — targets the grpc-server list item:
- XPath: openconfig-system:system/grpc-servers/grpc-server[name=<serverName>]/config
- JSON fields: enable, port, certificate-id (omitempty), network-instance (omitempty)
- Populated from spec.grpc.*
SSHServer — targets the SSH server config container:
- XPath: openconfig-system:system/ssh-server/config
- JSON fields: enable, timeout (seconds, uint32)
- Populated from spec.ssh.*

Claude Test results

  Test Report

  ┌──────────────────┬──────────────────┬───────────┬───────┬──────────────────────────────────────────────────────────────────────┬─────────────────┐
  │    CR Name       │      Kind        │ Namespace │ Ready │                           gNMI Path                                 │     Result      │
  ├──────────────────┼──────────────────┼───────────┼───────┼──────────────────────────────────────────────────────────────────────┼─────────────────┤
  │ managementaccess │ ManagementAccess │ default   │ True  │ openconfig-system:system/grpc-servers/grpc-server[name=gnmi]/config  │ ✓ value matches │
  │ managementaccess │ ManagementAccess │ default   │ True  │ openconfig-system:system/ssh-server/config                          │ ✓ value matches │
  └──────────────────┴──────────────────┴───────────┴───────┴──────────────────────────────────────────────────────────────────────┴─────────────────┘

  serverName tests:
  ┌──────────────────┬──────────────────┬───────────┬───────┬─────────────────────────────────────────────────────────────────────────┬─────────────────┐
  │    CR Name       │      Kind        │ Namespace │ Ready │                           gNMI Path                                    │     Result      │
  ├──────────────────┼──────────────────┼───────────┼───────┼─────────────────────────────────────────────────────────────────────────┼─────────────────┤
  │ managementaccess │ ManagementAccess │ default   │ True  │ openconfig-system:system/grpc-servers/grpc-server[name=gnmi]/config     │ ✓ value matches │
  │ managementaccess │ ManagementAccess │ default   │ True  │ openconfig-system:system/grpc-servers/grpc-server[name=my-gnmi]/config  │ ✓ value matches │
  └──────────────────┴──────────────────┴───────────┴───────┴─────────────────────────────────────────────────────────────────────────┴─────────────────┘

  Local Dev:
  ────────────────────────────────────────────────────────
  Vet:         ✓ passed
  Lint:        ✓ 0 issues
  Unit tests:  ✓ all passed
  gNMI tests:  ✓ 11/11 passed
  ────────────────────────────────────────────────────────
  Overall:     ✓ all checks passed

Applied spec:

spec:
  deviceRef:
    name: leaf1
  grpc:
    certificateId: mytrustpoint
    enabled: true
    gnmi:
      keepAliveTimeout: 10m   # 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

Manual test result

$ gnmic -a 172.20.20.2 --port 57400 -u admin -p 'NokiaSrl1!' --skip-verify --encoding JSON_IETF get --path openconfig-system:system/grpc-servers/grpc-server[name=gnmi]/config
[
  {
    "source": "172.20.20.2",
    "timestamp": 1786538371750487362,
    "time": "2026-08-12T14:39:31.750487362+02:00",
    "updates": [
      {
        "Path": "openconfig-system:system/grpc-servers/grpc-server[name=gnmi]/config",
        "values": {
          "openconfig-system:system/grpc-servers/grpc-server/config": {
            "certificate-id": "mytrustpoint",
            "enable": true,
            "name": "gnmi",
            "network-instance": "mgmt",
            "port": 9339
          }
        }
      }
    ]
  }
]

$ gnmic -a 172.20.20.2 --port 57400 -u admin -p 'NokiaSrl1!' --skip-verify --encoding JSON_IETF get --path openconfig-system:system/ssh-server/config
[
  {
    "source": "172.20.20.2",
    "timestamp": 1786538371999025754,
    "time": "2026-08-12T14:39:31.999025754+02:00",
    "updates": [
      {
        "Path": "openconfig-system:system/ssh-server/config",
        "values": {
          "openconfig-system:system/ssh-server/config": {
            "enable": true,
            "timeout": 120
          }
        }
      }
    ]
  }
]

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>

felix-kaestner · 2026-08-06T14:19:58Z

+	Timeout uint32 `json:"timeout,omitempty"`
+}
+
+func (s *SSHServer) XPath() string {


Suggested change

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>

felix-kaestner · 2026-08-10T10:24:33Z

+
+var _ provider.ManagementAccessProvider = (*Provider)(nil)
+
+const grpcServerName = "gnmi"


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?

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.

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?

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.

Adds a ServerName field to the GRPC spec, used by the OpenConfig provider to identify the gRPC server instance on the device (defaults to "gnmi"). Raises UnsupportedFieldError on Cisco NX-OS where the name is not configurable. Also adds DeleteManagementAccessRequest to carry spec context into deletion. Co-authored-by: Claude <claude@anthropic.com> Signed-off-by: Robert Gildein <rgildein@users.noreply.github.com>

Signed-off-by: Robert Gildein <rgildein@users.noreply.github.com>

github-actions · 2026-08-12T13:13:26Z

Merging this branch changes the coverage (1 decrease, 1 increase)

Impacted Packages	Coverage Δ	🤖
github.com/ironcore-dev/network-operator/api/core/v1alpha1	1.72% (ø)
github.com/ironcore-dev/network-operator/hack/provider	0.00% (ø)
github.com/ironcore-dev/network-operator/internal/controller/core	61.70% (+0.07%)	👍
github.com/ironcore-dev/network-operator/internal/provider	0.00% (ø)
github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos	9.25% (-0.01%)	👎
github.com/ironcore-dev/network-operator/internal/provider/openconfig	0.00% (ø)

Coverage by file

Changed files (no unit tests)

Changed File	Coverage Δ	Total	Covered	Missed	🤖
github.com/ironcore-dev/network-operator/api/core/v1alpha1/managementaccess_types.go	12.50% (ø)	8	1	7
github.com/ironcore-dev/network-operator/hack/provider/main.go	0.00% (ø)	659	0	659
github.com/ironcore-dev/network-operator/internal/controller/core/managementaccess_controller.go	57.97% (ø)	138	80	58
github.com/ironcore-dev/network-operator/internal/provider/cisco/nxos/provider.go	0.38% (-0.00%)	2130 (+2)	8	2122 (+2)	👎
github.com/ironcore-dev/network-operator/internal/provider/openconfig/managementaccess.go	0.00% (ø)	18 (+18)	0	18 (+18)
github.com/ironcore-dev/network-operator/internal/provider/provider.go	0.00% (ø)	42	0	42

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.

Changed unit test files

github.com/ironcore-dev/network-operator/internal/controller/core/suite_test.go

felix-kaestner · 2026-08-12T17:29:53Z

+	// +optional
+	// +kubebuilder:validation:MinLength=1
+	// +kubebuilder:validation:MaxLength=63
+	ServerName string `json:"serverName,omitempty"`


I think we need to make this Immutable once set.

The reasoning is that otherwise we can leave orphaned config on a device which we want to avoid. Assume you create a ManagementAccess resource with a grpc server and name "foo" and it creates a server named like that. Now you change the spec to have the grpc server named "bar". What will happen now is that it will again create a new grpc server, with a different name, and leave the previous gprc server running and untouched. This is because the server name is part of the YANG path, as such it identifies the tree which is updated. If it can change, then we might end up creating/updating an entirely different path.

Good point, I did not know that and did not realised that we can do that. It's not possible to do "update" for the name? If server exists we will rename it? I think it's better to be immutable, since it make more sense for user to create new resource rather that edit existing one, but I'm curious if "update" is possible.

It's not possible to do "update" for the name? If server exists we will rename it?

So a rename in the yang model would actually be to delete the old element, e.g. openconfig-system:system/grpc-servers/grpc-server[name=foo], and create a new one, e.g openconfig-system:system/grpc-servers/grpc-server[name=bar]. As the name field of the object is part of the path, so there is no way of doing something like an update to e.g. openconfig-system:system/grpc-servers/grpc-server[name=foo] with {"name":"bar"}. XPath and object field (name in this case) always match.

Now based on this, there is no easy way for us to know, that an existing openconfig-system:system/grpc-servers/grpc-server[name=foo] on the device was our operator in a previous reconcilation run. You just get the current kubernetes resource (now having .spec.name=bar).

In order to actually perform the deletion of the previous element, you would need knowledge over a previous kubernetes resource version, that was successfully configured, before being changed by a user to the now current version. 😄

rgildein self-assigned this Aug 6, 2026

github-actions Bot added the size/L label Aug 6, 2026

felix-kaestner reviewed Aug 6, 2026

View reviewed changes

fix unnecessary conversion from int32 to uint16 + small fixes

1f0191c

Signed-off-by: Robert Gildein <rgildein@users.noreply.github.com>

rgildein marked this pull request as ready for review August 7, 2026 07:15

rgildein requested a review from felix-kaestner August 7, 2026 07:15

felix-kaestner reviewed Aug 10, 2026

View reviewed changes

rgildein and others added 3 commits August 12, 2026 12:53

Merge branch 'main' into feat/openconfig-managementaccess

52b37b5

fix charts

5c99da9

Signed-off-by: Robert Gildein <rgildein@users.noreply.github.com>

rgildein requested a review from felix-kaestner August 12, 2026 13:15

felix-kaestner reviewed Aug 12, 2026

View reviewed changes

Navigation Menu

Uh oh!

feat(openconfig): Implement ManagementAccess provider - #485

feat(openconfig): Implement ManagementAccess provider#485
rgildein wants to merge 5 commits into
mainfrom
feat/openconfig-managementaccess

rgildein commented Aug 6, 2026 •

edited

Loading

Uh oh!

felix-kaestner Aug 6, 2026

Uh oh!

felix-kaestner Aug 10, 2026

Uh oh!

rgildein Aug 11, 2026

Uh oh!

felix-kaestner Aug 11, 2026

Uh oh!

rgildein Aug 11, 2026

Uh oh!

github-actions Bot commented Aug 12, 2026

Changed files (no unit tests)

Changed unit test files

Uh oh!

Uh oh!

Uh oh!

felix-kaestner Aug 12, 2026

Uh oh!

rgildein Aug 12, 2026

Uh oh!

felix-kaestner Aug 12, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	func (s *SSHServer) XPath() string {
	func (*SSHServer) XPath() string {


		var _ provider.ManagementAccessProvider = (*Provider)(nil)

		const grpcServerName = "gnmi"

Uh oh!

Conversation

rgildein commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Plan: Add ManagementAccess to OpenConfig Provider

Context

OpenConfig YANG Paths

Implementation

New file: internal/provider/openconfig/managementaccess.go

Manual test result

Uh oh!

felix-kaestner Aug 6, 2026

Choose a reason for hiding this comment

Uh oh!

felix-kaestner Aug 10, 2026

Choose a reason for hiding this comment

Uh oh!

rgildein Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

felix-kaestner Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

rgildein Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Aug 12, 2026

Merging this branch changes the coverage (1 decrease, 1 increase)

Changed files (no unit tests)

Changed unit test files

Uh oh!

Uh oh!

Uh oh!

felix-kaestner Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

rgildein Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

felix-kaestner Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rgildein commented Aug 6, 2026 •

edited

Loading

New file: `internal/provider/openconfig/managementaccess.go`

felix-kaestner Aug 12, 2026 •

edited

Loading