From f480dcc86c6fc0a65394ab75f2df27d612abe7a3 Mon Sep 17 00:00:00 2001 From: Robert Gildein Date: Thu, 20 Aug 2026 15:50:49 +0200 Subject: [PATCH 1/2] Add OpenConfig user provider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implement UserProvider for the OpenConfig provider, targeting the standard OpenConfig path: openconfig-system:system/aaa/authentication/users/user[username=X]/config Nokia SR Linux limitation: the OpenConfig user model on SRLinux does not expose password or ssh-public-key as writable config leaves — only username and role are settable, and only a single role is accepted. The provider raises UnsupportedFieldError for spec.password, spec.sshPublicKey, and spec.roles when more than one role is given. Since spec.password is mandatory in the CRD, User CRs will always reach Ready=False (terminal) on Nokia SRL via the OpenConfig provider. Also adds a gnmi testdata file documenting the expected device state for a user created with a single role. Co-authored-by: Claude Signed-off-by: Robert Gildein --- internal/apistatus/apistatus.go | 21 ++++- internal/conditions/conditions.go | 7 ++ internal/controller/core/user_controller.go | 5 ++ internal/provider/openconfig/user.go | 94 +++++++++++++++++++++ test/gnmi/testdata/openconfig/user.txt | 46 ++++++++++ 5 files changed, 172 insertions(+), 1 deletion(-) create mode 100644 internal/provider/openconfig/user.go create mode 100644 test/gnmi/testdata/openconfig/user.txt diff --git a/internal/apistatus/apistatus.go b/internal/apistatus/apistatus.go index 5ab6fa950..c52ce8f53 100644 --- a/internal/apistatus/apistatus.go +++ b/internal/apistatus/apistatus.go @@ -34,12 +34,18 @@ const ( // configured out-of-band). [WrapTerminalError] does not promote these // errors to terminal. CodeFailedPrecondition + + // CodeIgnoredField signals that one or more spec fields were silently + // ignored during realization because the provider does not support them. + // The resource is still considered successfully configured. [WrapTerminalError] + // does not promote these errors to terminal. + CodeIgnoredField ) // Valid reports whether c is a known, non-zero Code. func (c Code) Valid() bool { switch c { - case CodeInvalidArgument, CodeUnsupportedField, CodeFailedPrecondition: + case CodeInvalidArgument, CodeUnsupportedField, CodeFailedPrecondition, CodeIgnoredField: return true default: return false @@ -55,6 +61,8 @@ func (c Code) String() string { return "UnsupportedField" case CodeFailedPrecondition: return "FailedPrecondition" + case CodeIgnoredField: + return "IgnoredField" default: return fmt.Sprintf("Code(%d)", c) } @@ -129,6 +137,17 @@ func NewFailedPreconditionError(message string) *StatusError { } } +// NewIgnoredFieldError returns a [StatusError] with [CodeIgnoredField] for +// one or more spec fields that were silently ignored during realization. +// The resource is still considered successfully configured — the condition +// status will be True with the violation messages as a warning. +func NewIgnoredFieldError(violations ...FieldViolation) *StatusError { + return &StatusError{ + Code: CodeIgnoredField, + FieldViolations: violations, + } +} + // FromError extracts a [*StatusError] from err. // The boolean reports whether the extraction succeeded. func FromError(err error) (*StatusError, bool) { diff --git a/internal/conditions/conditions.go b/internal/conditions/conditions.go index d80e3791b..311a7ecee 100644 --- a/internal/conditions/conditions.go +++ b/internal/conditions/conditions.go @@ -161,6 +161,8 @@ func Sort(conditions []metav1.Condition) { // If the error is nil, it returns a condition indicating success. // If the error is an [apistatus.StatusError], its Code and formatted message // are used to populate the condition's Reason and Message fields. +// [apistatus.CodeIgnoredField] errors are treated as success — the condition +// status is True but the message carries the ignored field warnings. // If the error is a gRPC status error, its code and message are used instead. func FromError(err error) metav1.Condition { cond := metav1.Condition{ @@ -178,6 +180,11 @@ func FromError(err error) metav1.Condition { if statusErr, ok := apistatus.FromError(err); ok { cond.Reason = statusErr.Code.String() cond.Message = statusErr.Error() + // IgnoredField is not a failure — resource was configured successfully + // but some fields were silently skipped by the provider. + if statusErr.Code == apistatus.CodeIgnoredField { + cond.Status = metav1.ConditionTrue + } return cond } diff --git a/internal/controller/core/user_controller.go b/internal/controller/core/user_controller.go index ff16800a3..1d256d44d 100644 --- a/internal/controller/core/user_controller.go +++ b/internal/controller/core/user_controller.go @@ -330,6 +330,11 @@ func (r *UserReconciler) reconcile(ctx context.Context, s *userScope) (reterr er cond.Type = v1alpha1.ReadyCondition conditions.Set(s.User, cond) + // IgnoredField is not a failure — the resource was configured successfully, + // the warning is already captured in the condition message. + if se, ok := apistatus.FromError(err); ok && se.Code == apistatus.CodeIgnoredField { + return nil + } return err } diff --git a/internal/provider/openconfig/user.go b/internal/provider/openconfig/user.go new file mode 100644 index 000000000..c64e7253a --- /dev/null +++ b/internal/provider/openconfig/user.go @@ -0,0 +1,94 @@ +// SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and IronCore contributors +// SPDX-License-Identifier: Apache-2.0 + +package openconfig + +import ( + "context" + "fmt" + + "sigs.k8s.io/controller-runtime/pkg/log" + + "github.com/ironcore-dev/network-operator/internal/apistatus" + "github.com/ironcore-dev/network-operator/internal/provider" + "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" +) + +var _ provider.UserProvider = (*Provider)(nil) + +func (p *Provider) EnsureUser(ctx context.Context, req *provider.EnsureUserRequest) error { + validErr := validateUserRequest(req) + if validErr != nil && !isIgnoredFieldError(validErr) { + return validErr + } + u := &User{ + Username: req.Username, + Config: &UserConfig{ + Username: req.Username, + Role: req.Roles[0], + }, + } + if err := p.client.Update(ctx, u); err != nil { + return err + } + if validErr != nil { + log.FromContext(ctx).Info("User configured with ignored fields", "warning", validErr.Error()) + } + return validErr +} + +func (p *Provider) DeleteUser(ctx context.Context, req *provider.DeleteUserRequest) error { + return p.client.Delete(ctx, &User{Username: req.Username}) +} + +func validateUserRequest(req *provider.EnsureUserRequest) error { + var violations []apistatus.FieldViolation + if req.SSHKey != "" { + violations = append(violations, apistatus.FieldViolation{ + Field: "spec.sshPublicKey", + Description: "sshPublicKey is not supported by the OpenConfig user model on SRLinux", + }) + } + if len(req.Roles) > 1 { + violations = append(violations, apistatus.FieldViolation{ + Field: "spec.roles", + Description: "only one role is supported by the OpenConfig user model on SRLinux; role name must be an OpenConfig AAA identity (e.g. openconfig-aaa-types:SYSTEM_ROLE_ADMIN)", + }) + } + if len(violations) > 0 { + return apistatus.NewUnsupportedFieldError(violations...) + } + if req.Password != "" { + return apistatus.NewIgnoredFieldError(apistatus.FieldViolation{ + Field: "spec.password", + Description: "password is not supported by the OpenConfig user model on SRLinux", + }) + } + return nil +} + +func isIgnoredFieldError(err error) bool { + se, ok := apistatus.FromError(err) + return ok && se.Code == apistatus.CodeIgnoredField +} + +// Compile-time assertion. +var _ gnmiext.DataElement = (*User)(nil) + +// User targets an OpenConfig user entry. +type User struct { + Username string `json:"-"` + Config *UserConfig `json:"config,omitempty"` +} + +func (u *User) XPath() string { + return fmt.Sprintf("openconfig-system:system/aaa/authentication/users/user[username=%s]", u.Username) +} + +// UserConfig holds the user config container leaves. +// Role must be a valid OpenConfig AAA identity string, e.g. "openconfig-aaa-types:SYSTEM_ROLE_ADMIN". +// The device enforces this via a leafref — native role names (e.g. "admin") are rejected. +type UserConfig struct { + Username string `json:"username"` + Role string `json:"role,omitempty"` +} diff --git a/test/gnmi/testdata/openconfig/user.txt b/test/gnmi/testdata/openconfig/user.txt new file mode 100644 index 000000000..627607897 --- /dev/null +++ b/test/gnmi/testdata/openconfig/user.txt @@ -0,0 +1,46 @@ +# User with a single role (password and sshPublicKey are not supported by the OpenConfig user model on SRLinux) +# Role must be specified as an OpenConfig AAA identity string (e.g. openconfig-aaa-types:SYSTEM_ROLE_ADMIN). +# The device enforces this via a leafref — native role names (e.g. "admin") are rejected with FailedPrecondition. +-- users/user -- +apiVersion: networking.metal.ironcore.dev/v1alpha1 +kind: User +metadata: + name: user + namespace: default +spec: + deviceRef: + name: device + username: testplan + password: + secretKeyRef: + name: user-password + key: password + roles: + - name: openconfig-aaa-types:SYSTEM_ROLE_ADMIN + +-- state/preload -- +{} + +-- state/expect -- +{ + "openconfig-system:system": { + "aaa": { + "authentication": { + "users": { + "user": [ + { + "config": { + "role": "SYSTEM_ROLE_ADMIN", + "username": "testplan" + }, + "username": "testplan" + } + ] + } + } + } + } +} + +-- state/delete -- +{} From 523b3bc54a96c5c3a152e7b3550dc722cb6cc98f Mon Sep 17 00:00:00 2001 From: Robert Gildein Date: Wed, 2 Sep 2026 10:30:12 +0200 Subject: [PATCH 2/2] Update OpenConfig user provider to vanilla OpenConfig MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rewrite the user provider to target vanilla OpenConfig rather than Nokia SRLinux-specific behavior: - Support password, ssh-key and role — all standard OpenConfig user config leaves (openconfig-system:system/aaa/authentication/users) - Use gNMI update (Patch) instead of replace (Update) for user creation — Juniper rejects replace for new entries with "statement not found" - Password excluded from UnmarshalJSON to avoid perpetual diffs (device returns hashed value that never matches plaintext) - Remove Nokia-specific UnsupportedFieldError for password/sshPublicKey - Remove CodeIgnoredField from apistatus — not needed for vanilla OpenConfig; revert related changes to conditions.go and user_controller.go - Only retain single-role constraint (OpenConfig role leaf is a single value, not a leaf-list — confirmed on both Juniper and Nokia) - Replace user.txt testdata with proper user.txtar including secrets and full expected gNMI state with ssh-key Tested against Juniper vJunos-Evolved 26.2R1.7 via containerlab. Co-authored-by: Claude Signed-off-by: Robert Gildein --- internal/apistatus/apistatus.go | 21 +----- internal/conditions/conditions.go | 7 -- internal/controller/core/user_controller.go | 5 -- internal/provider/openconfig/user.go | 74 ++++++++------------- test/gnmi/testdata/openconfig/user.txt | 46 ------------- test/gnmi/testdata/openconfig/user.txtar | 69 +++++++++++++++++++ 6 files changed, 99 insertions(+), 123 deletions(-) delete mode 100644 test/gnmi/testdata/openconfig/user.txt create mode 100644 test/gnmi/testdata/openconfig/user.txtar diff --git a/internal/apistatus/apistatus.go b/internal/apistatus/apistatus.go index c52ce8f53..5ab6fa950 100644 --- a/internal/apistatus/apistatus.go +++ b/internal/apistatus/apistatus.go @@ -34,18 +34,12 @@ const ( // configured out-of-band). [WrapTerminalError] does not promote these // errors to terminal. CodeFailedPrecondition - - // CodeIgnoredField signals that one or more spec fields were silently - // ignored during realization because the provider does not support them. - // The resource is still considered successfully configured. [WrapTerminalError] - // does not promote these errors to terminal. - CodeIgnoredField ) // Valid reports whether c is a known, non-zero Code. func (c Code) Valid() bool { switch c { - case CodeInvalidArgument, CodeUnsupportedField, CodeFailedPrecondition, CodeIgnoredField: + case CodeInvalidArgument, CodeUnsupportedField, CodeFailedPrecondition: return true default: return false @@ -61,8 +55,6 @@ func (c Code) String() string { return "UnsupportedField" case CodeFailedPrecondition: return "FailedPrecondition" - case CodeIgnoredField: - return "IgnoredField" default: return fmt.Sprintf("Code(%d)", c) } @@ -137,17 +129,6 @@ func NewFailedPreconditionError(message string) *StatusError { } } -// NewIgnoredFieldError returns a [StatusError] with [CodeIgnoredField] for -// one or more spec fields that were silently ignored during realization. -// The resource is still considered successfully configured — the condition -// status will be True with the violation messages as a warning. -func NewIgnoredFieldError(violations ...FieldViolation) *StatusError { - return &StatusError{ - Code: CodeIgnoredField, - FieldViolations: violations, - } -} - // FromError extracts a [*StatusError] from err. // The boolean reports whether the extraction succeeded. func FromError(err error) (*StatusError, bool) { diff --git a/internal/conditions/conditions.go b/internal/conditions/conditions.go index 311a7ecee..d80e3791b 100644 --- a/internal/conditions/conditions.go +++ b/internal/conditions/conditions.go @@ -161,8 +161,6 @@ func Sort(conditions []metav1.Condition) { // If the error is nil, it returns a condition indicating success. // If the error is an [apistatus.StatusError], its Code and formatted message // are used to populate the condition's Reason and Message fields. -// [apistatus.CodeIgnoredField] errors are treated as success — the condition -// status is True but the message carries the ignored field warnings. // If the error is a gRPC status error, its code and message are used instead. func FromError(err error) metav1.Condition { cond := metav1.Condition{ @@ -180,11 +178,6 @@ func FromError(err error) metav1.Condition { if statusErr, ok := apistatus.FromError(err); ok { cond.Reason = statusErr.Code.String() cond.Message = statusErr.Error() - // IgnoredField is not a failure — resource was configured successfully - // but some fields were silently skipped by the provider. - if statusErr.Code == apistatus.CodeIgnoredField { - cond.Status = metav1.ConditionTrue - } return cond } diff --git a/internal/controller/core/user_controller.go b/internal/controller/core/user_controller.go index 1d256d44d..ff16800a3 100644 --- a/internal/controller/core/user_controller.go +++ b/internal/controller/core/user_controller.go @@ -330,11 +330,6 @@ func (r *UserReconciler) reconcile(ctx context.Context, s *userScope) (reterr er cond.Type = v1alpha1.ReadyCondition conditions.Set(s.User, cond) - // IgnoredField is not a failure — the resource was configured successfully, - // the warning is already captured in the condition message. - if se, ok := apistatus.FromError(err); ok && se.Code == apistatus.CodeIgnoredField { - return nil - } return err } diff --git a/internal/provider/openconfig/user.go b/internal/provider/openconfig/user.go index c64e7253a..7349396e1 100644 --- a/internal/provider/openconfig/user.go +++ b/internal/provider/openconfig/user.go @@ -5,10 +5,9 @@ package openconfig import ( "context" + "encoding/json" "fmt" - "sigs.k8s.io/controller-runtime/pkg/log" - "github.com/ironcore-dev/network-operator/internal/apistatus" "github.com/ironcore-dev/network-operator/internal/provider" "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" @@ -17,61 +16,28 @@ import ( var _ provider.UserProvider = (*Provider)(nil) func (p *Provider) EnsureUser(ctx context.Context, req *provider.EnsureUserRequest) error { - validErr := validateUserRequest(req) - if validErr != nil && !isIgnoredFieldError(validErr) { - return validErr + if len(req.Roles) > 1 { + return apistatus.NewUnsupportedFieldError(apistatus.FieldViolation{ + Field: "spec.roles", + Description: "the OpenConfig user model supports only a single role", + }) } u := &User{ Username: req.Username, Config: &UserConfig{ Username: req.Username, Role: req.Roles[0], + Password: req.Password, + SSHKey: req.SSHKey, }, } - if err := p.client.Update(ctx, u); err != nil { - return err - } - if validErr != nil { - log.FromContext(ctx).Info("User configured with ignored fields", "warning", validErr.Error()) - } - return validErr + return p.client.Patch(ctx, u) } func (p *Provider) DeleteUser(ctx context.Context, req *provider.DeleteUserRequest) error { return p.client.Delete(ctx, &User{Username: req.Username}) } -func validateUserRequest(req *provider.EnsureUserRequest) error { - var violations []apistatus.FieldViolation - if req.SSHKey != "" { - violations = append(violations, apistatus.FieldViolation{ - Field: "spec.sshPublicKey", - Description: "sshPublicKey is not supported by the OpenConfig user model on SRLinux", - }) - } - if len(req.Roles) > 1 { - violations = append(violations, apistatus.FieldViolation{ - Field: "spec.roles", - Description: "only one role is supported by the OpenConfig user model on SRLinux; role name must be an OpenConfig AAA identity (e.g. openconfig-aaa-types:SYSTEM_ROLE_ADMIN)", - }) - } - if len(violations) > 0 { - return apistatus.NewUnsupportedFieldError(violations...) - } - if req.Password != "" { - return apistatus.NewIgnoredFieldError(apistatus.FieldViolation{ - Field: "spec.password", - Description: "password is not supported by the OpenConfig user model on SRLinux", - }) - } - return nil -} - -func isIgnoredFieldError(err error) bool { - se, ok := apistatus.FromError(err) - return ok && se.Code == apistatus.CodeIgnoredField -} - // Compile-time assertion. var _ gnmiext.DataElement = (*User)(nil) @@ -86,9 +52,27 @@ func (u *User) XPath() string { } // UserConfig holds the user config container leaves. -// Role must be a valid OpenConfig AAA identity string, e.g. "openconfig-aaa-types:SYSTEM_ROLE_ADMIN". -// The device enforces this via a leafref — native role names (e.g. "admin") are rejected. +// Password is write-only — the device returns a hashed value that would never match +// the plaintext, so we exclude it from unmarshal to avoid perpetual diffs. type UserConfig struct { Username string `json:"username"` Role string `json:"role,omitempty"` + Password string `json:"password,omitempty"` + SSHKey string `json:"ssh-key,omitempty"` +} + +func (c *UserConfig) UnmarshalJSON(data []byte) error { + type alias struct { + Username string `json:"username"` + Role string `json:"role,omitempty"` + SSHKey string `json:"ssh-key,omitempty"` + } + var a alias + if err := json.Unmarshal(data, &a); err != nil { + return err + } + c.Username = a.Username + c.Role = a.Role + c.SSHKey = a.SSHKey + return nil } diff --git a/test/gnmi/testdata/openconfig/user.txt b/test/gnmi/testdata/openconfig/user.txt deleted file mode 100644 index 627607897..000000000 --- a/test/gnmi/testdata/openconfig/user.txt +++ /dev/null @@ -1,46 +0,0 @@ -# User with a single role (password and sshPublicKey are not supported by the OpenConfig user model on SRLinux) -# Role must be specified as an OpenConfig AAA identity string (e.g. openconfig-aaa-types:SYSTEM_ROLE_ADMIN). -# The device enforces this via a leafref — native role names (e.g. "admin") are rejected with FailedPrecondition. --- users/user -- -apiVersion: networking.metal.ironcore.dev/v1alpha1 -kind: User -metadata: - name: user - namespace: default -spec: - deviceRef: - name: device - username: testplan - password: - secretKeyRef: - name: user-password - key: password - roles: - - name: openconfig-aaa-types:SYSTEM_ROLE_ADMIN - --- state/preload -- -{} - --- state/expect -- -{ - "openconfig-system:system": { - "aaa": { - "authentication": { - "users": { - "user": [ - { - "config": { - "role": "SYSTEM_ROLE_ADMIN", - "username": "testplan" - }, - "username": "testplan" - } - ] - } - } - } - } -} - --- state/delete -- -{} diff --git a/test/gnmi/testdata/openconfig/user.txtar b/test/gnmi/testdata/openconfig/user.txtar new file mode 100644 index 000000000..e2f9fa555 --- /dev/null +++ b/test/gnmi/testdata/openconfig/user.txtar @@ -0,0 +1,69 @@ +# User with password, role and ssh-key +-- secrets/user-password -- +apiVersion: v1 +kind: Secret +metadata: + name: user-password + namespace: default +type: Opaque +stringData: + password: Test1234! + +-- secrets/user-ssh-key -- +apiVersion: v1 +kind: Secret +metadata: + name: user-ssh-key + namespace: default +type: Opaque +stringData: + ssh-publickey: "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAQQDSGgsAKZn/hxPMKyfwKboiOEeuL9bTqW79QfEQ8h0kpGhkFJJEWR1e3BvXpdT9KYQOaKQnNw32atULweSQQNGh6 IronCore Test" + +-- users/user -- +apiVersion: networking.metal.ironcore.dev/v1alpha1 +kind: User +metadata: + name: user + namespace: default +spec: + deviceRef: + name: device + username: testplan + password: + secretKeyRef: + name: user-password + key: password + roles: + - name: superuser + sshPublicKey: + secretKeyRef: + name: user-ssh-key + key: ssh-publickey + +-- state/preload -- +{} + +-- state/expect -- +{ + "openconfig-system:system": { + "aaa": { + "authentication": { + "users": { + "user": [ + { + "config": { + "role": "superuser", + "ssh-key": "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAQQDSGgsAKZn/hxPMKyfwKboiOEeuL9bTqW79QfEQ8h0kpGhkFJJEWR1e3BvXpdT9KYQOaKQnNw32atULweSQQNGh6 IronCore Test", + "username": "testplan" + }, + "username": "testplan" + } + ] + } + } + } + } +} + +-- state/delete -- +{}