Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
78 changes: 78 additions & 0 deletions internal/provider/openconfig/user.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
// SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and IronCore contributors
// SPDX-License-Identifier: Apache-2.0

package openconfig

import (
"context"
"encoding/json"
"fmt"

"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 {
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,
},
}
return p.client.Patch(ctx, u)

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.

Any particular reason we use a patch over an update here?

}

func (p *Provider) DeleteUser(ctx context.Context, req *provider.DeleteUserRequest) error {
return p.client.Delete(ctx, &User{Username: req.Username})
}

// 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.
// 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.
Comment on lines +53 to +56

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.

Please have a look into what we are doing on the nxos provider in https://github.com/ironcore-dev/network-operator/blob/main/internal/provider/cisco/nxos/provider.go#L2620-L2641

We take the plaintext password as we retrieve it from the kubernetes secret and compute the hash ourselves (which we can do if the hash includes the algorithm and salt value). If the hash we compute from the plaintext value matches, what is stored in hashed form on the device. We can retain that value.

Otherwise, we would end up with a gnmi write on every reconcilation, which we definitely want to avoid.

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