From 9bd6d533654b3387c7949171f7aae79f44a9af9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D7=A0=CF=85=CE=B1=CE=B7=20=D7=A0=CF=85=CE=B1=CE=B7=D1=95?= =?UTF-8?q?=CF=83=CE=B7?= Date: Sat, 3 Oct 2026 10:13:36 -0700 Subject: [PATCH] refactor(user): the caller names the resource, so the validator does not validateAccountName took a kind argument purely to put "user" or "group" into its message, and every one of its twelve callers then wrapped the result with that same word. The error read "group: invalid group name %q: ...". Dropping the parameter leaves "group: name %q must match ...", which is the rule the rest of this work settled on: only the outermost frame names the operation. This was left out of #575 because it changes a signature. It is a signature nobody outside the package uses, so the change is contained. Refs #565 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FuKUsHFG1EqZXamffh9M2c --- internal/provider/node/user/debian_group.go | 8 ++++---- .../provider/node/user/debian_public_test.go | 18 +++++++++--------- internal/provider/node/user/debian_ssh_key.go | 6 +++--- .../node/user/debian_ssh_key_public_test.go | 6 +++--- internal/provider/node/user/debian_user.go | 10 +++++----- internal/provider/node/user/validate.go | 8 +++----- 6 files changed, 27 insertions(+), 29 deletions(-) diff --git a/internal/provider/node/user/debian_group.go b/internal/provider/node/user/debian_group.go index 3aec2b499..706b3ebe8 100644 --- a/internal/provider/node/user/debian_group.go +++ b/internal/provider/node/user/debian_group.go @@ -57,7 +57,7 @@ func (d *Debian) GetGroup( ) (*Group, error) { _ = ctx - if err := validateAccountName("group", name); err != nil { + if err := validateAccountName(name); err != nil { return nil, fmt.Errorf("group: %w", err) } @@ -82,7 +82,7 @@ func (d *Debian) CreateGroup( ) (*GroupResult, error) { _ = ctx - if err := validateAccountName("group", opts.Name); err != nil { + if err := validateAccountName(opts.Name); err != nil { return nil, fmt.Errorf("group: %w", err) } @@ -112,7 +112,7 @@ func (d *Debian) UpdateGroup( ) (*GroupResult, error) { _ = ctx - if err := validateAccountName("group", name); err != nil { + if err := validateAccountName(name); err != nil { return nil, fmt.Errorf("group: %w", err) } @@ -141,7 +141,7 @@ func (d *Debian) DeleteGroup( ) (*GroupResult, error) { _ = ctx - if err := validateAccountName("group", name); err != nil { + if err := validateAccountName(name); err != nil { return nil, fmt.Errorf("group: %w", err) } diff --git a/internal/provider/node/user/debian_public_test.go b/internal/provider/node/user/debian_public_test.go index 83b914cb9..61f78c8ea 100644 --- a/internal/provider/node/user/debian_public_test.go +++ b/internal/provider/node/user/debian_public_test.go @@ -351,7 +351,7 @@ func (suite *DebianPublicTestSuite) TestGetUser() { validateFunc: func(result *user.User, err error) { suite.Error(err) suite.Nil(result) - suite.Contains(err.Error(), "invalid user name") + suite.Contains(err.Error(), "must match") }, }, { @@ -540,7 +540,7 @@ func (suite *DebianPublicTestSuite) TestCreateUser() { validateFunc: func(result *user.Result, err error) { suite.Error(err) suite.Nil(result) - suite.Contains(err.Error(), "invalid user name") + suite.Contains(err.Error(), "must match") }, }, } @@ -691,7 +691,7 @@ func (suite *DebianPublicTestSuite) TestUpdateUser() { validateFunc: func(result *user.Result, err error) { suite.Error(err) suite.Nil(result) - suite.Contains(err.Error(), "invalid user name") + suite.Contains(err.Error(), "must match") }, }, } @@ -749,7 +749,7 @@ func (suite *DebianPublicTestSuite) TestDeleteUser() { validateFunc: func(result *user.Result, err error) { suite.Error(err) suite.Nil(result) - suite.Contains(err.Error(), "invalid user name") + suite.Contains(err.Error(), "must match") }, }, } @@ -860,7 +860,7 @@ func (suite *DebianPublicTestSuite) TestChangePassword() { validateFunc: func(result *user.Result, err error) { suite.Error(err) suite.Nil(result) - suite.Contains(err.Error(), "invalid user name") + suite.Contains(err.Error(), "must match") }, }, } @@ -1024,7 +1024,7 @@ func (suite *DebianPublicTestSuite) TestGetGroup() { validateFunc: func(result *user.Group, err error) { suite.Error(err) suite.Nil(result) - suite.Contains(err.Error(), "invalid group name") + suite.Contains(err.Error(), "name \"Invalid\" must match") }, }, } @@ -1126,7 +1126,7 @@ func (suite *DebianPublicTestSuite) TestCreateGroup() { validateFunc: func(result *user.GroupResult, err error) { suite.Error(err) suite.Nil(result) - suite.Contains(err.Error(), "invalid group name") + suite.Contains(err.Error(), "name \"Invalid\" must match") }, }, } @@ -1194,7 +1194,7 @@ func (suite *DebianPublicTestSuite) TestUpdateGroup() { validateFunc: func(result *user.GroupResult, err error) { suite.Error(err) suite.Nil(result) - suite.Contains(err.Error(), "invalid group name") + suite.Contains(err.Error(), "name \"Invalid\" must match") }, }, } @@ -1252,7 +1252,7 @@ func (suite *DebianPublicTestSuite) TestDeleteGroup() { validateFunc: func(result *user.GroupResult, err error) { suite.Error(err) suite.Nil(result) - suite.Contains(err.Error(), "invalid group name") + suite.Contains(err.Error(), "name \"Invalid\" must match") }, }, } diff --git a/internal/provider/node/user/debian_ssh_key.go b/internal/provider/node/user/debian_ssh_key.go index 03d195982..028ffe7e6 100644 --- a/internal/provider/node/user/debian_ssh_key.go +++ b/internal/provider/node/user/debian_ssh_key.go @@ -52,7 +52,7 @@ func (d *Debian) ListKeys( ) ([]SSHKey, error) { _ = ctx - if err := validateAccountName("user", username); err != nil { + if err := validateAccountName(username); err != nil { return nil, fmt.Errorf("ssh key: list: %w", err) } @@ -90,7 +90,7 @@ func (d *Debian) AddKey( ) (*SSHKeyResult, error) { _ = ctx - if err := validateAccountName("user", username); err != nil { + if err := validateAccountName(username); err != nil { return nil, fmt.Errorf("ssh key: add: %w", err) } @@ -181,7 +181,7 @@ func (d *Debian) RemoveKey( ) (*SSHKeyResult, error) { _ = ctx - if err := validateAccountName("user", username); err != nil { + if err := validateAccountName(username); err != nil { return nil, fmt.Errorf("ssh key: remove: %w", err) } diff --git a/internal/provider/node/user/debian_ssh_key_public_test.go b/internal/provider/node/user/debian_ssh_key_public_test.go index bacef83f0..5ab6641e6 100644 --- a/internal/provider/node/user/debian_ssh_key_public_test.go +++ b/internal/provider/node/user/debian_ssh_key_public_test.go @@ -361,7 +361,7 @@ func (suite *DebianSSHKeyPublicTestSuite) TestListKeys() { validateFunc: func(keys []user.SSHKey, err error) { suite.Error(err) suite.Nil(keys) - suite.Contains(err.Error(), "invalid user name") + suite.Contains(err.Error(), "must match") }, }, } @@ -721,7 +721,7 @@ func (suite *DebianSSHKeyPublicTestSuite) TestAddKey() { validateFunc: func(result *user.SSHKeyResult, err error) { suite.Error(err) suite.Nil(result) - suite.Contains(err.Error(), "invalid user name") + suite.Contains(err.Error(), "must match") }, }, } @@ -944,7 +944,7 @@ func (suite *DebianSSHKeyPublicTestSuite) TestRemoveKey() { validateFunc: func(result *user.SSHKeyResult, err error) { suite.Error(err) suite.Nil(result) - suite.Contains(err.Error(), "invalid user name") + suite.Contains(err.Error(), "must match") }, }, } diff --git a/internal/provider/node/user/debian_user.go b/internal/provider/node/user/debian_user.go index f211966a8..29506cbfc 100644 --- a/internal/provider/node/user/debian_user.go +++ b/internal/provider/node/user/debian_user.go @@ -75,7 +75,7 @@ func (d *Debian) GetUser( ) (*User, error) { _ = ctx - if err := validateAccountName("user", name); err != nil { + if err := validateAccountName(name); err != nil { return nil, fmt.Errorf("user: %w", err) } @@ -112,7 +112,7 @@ func (d *Debian) CreateUser( ) (*Result, error) { _ = ctx - if err := validateAccountName("user", opts.Name); err != nil { + if err := validateAccountName(opts.Name); err != nil { return nil, fmt.Errorf("user: %w", err) } @@ -156,7 +156,7 @@ func (d *Debian) UpdateUser( ) (*Result, error) { _ = ctx - if err := validateAccountName("user", name); err != nil { + if err := validateAccountName(name); err != nil { return nil, fmt.Errorf("user: %w", err) } @@ -191,7 +191,7 @@ func (d *Debian) DeleteUser( ) (*Result, error) { _ = ctx - if err := validateAccountName("user", name); err != nil { + if err := validateAccountName(name); err != nil { return nil, fmt.Errorf("user: %w", err) } @@ -219,7 +219,7 @@ func (d *Debian) ChangePassword( ) (*Result, error) { _ = ctx - if err := validateAccountName("user", name); err != nil { + if err := validateAccountName(name); err != nil { return nil, fmt.Errorf("user: %w", err) } diff --git a/internal/provider/node/user/validate.go b/internal/provider/node/user/validate.go index 8ba50b5ce..de4b62831 100644 --- a/internal/provider/node/user/validate.go +++ b/internal/provider/node/user/validate.go @@ -44,16 +44,14 @@ var accountNamePattern = regexp.MustCompile(`^[a-z_][a-z0-9_-]*\$?$`) const accountNameMaxLength = 32 // validateAccountName rejects a user or group name that does not match -// accountNamePattern or exceeds accountNameMaxLength. kind names the -// resource in the error message ("user" or "group"). +// accountNamePattern or exceeds accountNameMaxLength. The caller names the +// resource, so this message does not. func validateAccountName( - kind string, name string, ) error { if len(name) > accountNameMaxLength || !accountNamePattern.MatchString(name) { return fmt.Errorf( - "invalid %s name %q: must match %s and be at most %d characters", - kind, + "name %q must match %s and be at most %d characters", name, accountNamePattern.String(), accountNameMaxLength,