Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion internal/apistatus/apistatus.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment thread
rgildein marked this conversation as resolved.
Outdated
)

// 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
Expand All @@ -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)
}
Expand Down Expand Up @@ -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) {
Expand Down
7 changes: 7 additions & 0 deletions internal/conditions/conditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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
}

Expand Down
5 changes: 5 additions & 0 deletions internal/controller/core/user_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
94 changes: 94 additions & 0 deletions internal/provider/openconfig/user.go
Original file line number Diff line number Diff line change
@@ -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"`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Config *UserConfig `json:"config,omitempty"`
Config *UserConfig `json:"config"`

This field is always present and should therefore not have an omitempty tag. See

network-operator/AGENTS.md

Lines 156 to 161 in e21328d

**`omitempty` guidelines:**
1. **Safe:** The field's Go zero value matches the platform default or "absent" state. Omitting it from the payload is semantically equivalent to the device's default.
2. **Safe:** The field is a pointer or slice representing "not configured" (nil) vs "configured" (non-nil). Mutually exclusive choices (e.g. `accept`/`drop`) fall into this category.
3. **Dangerous:** The platform default is non-zero (e.g. `admin-state` defaults to `"enable"`, `port` defaults to `49`). Omitting the Go zero value would either misrepresent intent or cause a false diff on subsequent GET responses.
4. **Unnecessary:** The field is unconditionally set to a non-zero value by the provider code. The tag never triggers, but removing it documents intent — the field is always present.

}

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"`
}
46 changes: 46 additions & 0 deletions test/gnmi/testdata/openconfig/user.txt
Original file line number Diff line number Diff line change
@@ -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 --
{}
Loading