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
117 changes: 64 additions & 53 deletions cmd/harbor/root/user/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package user
import (
"fmt"

"github.com/goharbor/go-client/pkg/sdk/v2.0/client/user"
"github.com/goharbor/go-client/pkg/sdk/v2.0/models"
"github.com/goharbor/harbor-cli/pkg/api"
Comment on lines +19 to 21
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

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

In this file’s package user, importing github.com/goharbor/go-client/pkg/sdk/v2.0/client/user as user is very easy to confuse with the current package name and makes types like *user.ListUsersOK hard to read. Consider aliasing the import (e.g., clientuser) and updating the interface return type accordingly.

Copilot uses AI. Check for mistakes.
"github.com/goharbor/harbor-cli/pkg/utils"
Expand All @@ -25,68 +26,78 @@ import (
"github.com/spf13/viper"
)

func UserListCmd() *cobra.Command {
var opts api.ListFlags
func GetUsers(opts api.ListFlags, userListAPI func(opts ...api.ListFlags) (*user.ListUsersOK, error)) ([]*models.UserResp, error) {
var allUsers []*models.UserResp

cmd := &cobra.Command{
Use: "list",
Short: "List users",
Args: cobra.ExactArgs(0),
Aliases: []string{"ls"},
RunE: func(cmd *cobra.Command, args []string) error {
if opts.PageSize < 0 {
return fmt.Errorf("page size must be greater than or equal to 0")
}

if opts.PageSize > 100 {
return fmt.Errorf("page size should be less than or equal to 100")
}
if opts.PageSize > 100 || opts.PageSize < 0 {
return nil, fmt.Errorf("page size should be greater than or equal to 0 and less than or equal to 100")
}

if opts.PageSize == 0 {
opts.PageSize = 100
opts.Page = 1
if opts.PageSize == 0 {
opts.PageSize = 100
opts.Page = 1

for {
response, err := api.ListUsers(opts)
if err != nil {
if isUnauthorizedError(err) {
return fmt.Errorf("Permission denied: Admin privileges are required to execute this command.")
}
return fmt.Errorf("failed to list users: %v", err)
}
for {
response, err := userListAPI(opts)
if err != nil {
if isUnauthorizedError(err) {
return nil, fmt.Errorf("Permission denied: Admin privileges are required to execute this command.")
}
return nil, fmt.Errorf("failed to list users: %w", err)
}
Comment on lines +43 to +47
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

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

This error wrapping uses %v, which prevents callers from unwrapping/inspecting the underlying error. Prefer %w when re-wrapping (e.g., fmt.Errorf("failed to list users: %w", err)).

Copilot uses AI. Check for mistakes.

allUsers = append(allUsers, response.Payload...)
allUsers = append(allUsers, response.Payload...)

if len(response.Payload) < int(opts.PageSize) {
break
}
opts.Page++
}
} else {
response, err := api.ListUsers(opts)
if err != nil {
if isUnauthorizedError(err) {
return fmt.Errorf("Permission denied: Admin privileges are required to execute this command.")
}
return fmt.Errorf("failed to list users: %v", err)
}
allUsers = response.Payload
if len(response.Payload) < int(opts.PageSize) {
break
}
if len(allUsers) == 0 {
log.Info("No users found")
return nil
opts.Page++
}
} else {
response, err := userListAPI(opts)
if err != nil {
if isUnauthorizedError(err) {
return nil, fmt.Errorf("Permission denied: Admin privileges are required to execute this command.")
}
formatFlag := viper.GetString("output-format")
if formatFlag != "" {
err := utils.PrintFormat(allUsers, formatFlag)
if err != nil {
log.Error(err)
}
} else {
list.ListUsers(allUsers)
return nil, fmt.Errorf("failed to list users: %w", err)
}
Comment on lines +59 to +63
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

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

Same as above: use %w when re-wrapping err so it can be unwrapped by callers.

Copilot uses AI. Check for mistakes.
allUsers = response.Payload
}
return allUsers, nil
}
func PrintUsers(allUsers []*models.UserResp) error {
if len(allUsers) == 0 {
log.Info("No users found")
return nil
}
formatFlag := viper.GetString("output-format")
if formatFlag != "" {
err := utils.PrintFormat(allUsers, formatFlag)
if err != nil {
log.Error(err)
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

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

PrintUsers returns an error, but in the formatted-output branch a utils.PrintFormat failure is only logged and the function still returns nil. This makes the command exit successfully even though output failed; return the formatting error instead of swallowing it.

Suggested change
log.Error(err)
log.Error(err)
return err

Copilot uses AI. Check for mistakes.
return err
}
} else {
if err := list.ListUsers(allUsers); err != nil {
return err
}
}
return nil
}
func UserListCmd() *cobra.Command {
var opts api.ListFlags
var userListAPI = api.ListUsers
cmd := &cobra.Command{
Use: "list",
Short: "List users",
Args: cobra.ExactArgs(0),
Aliases: []string{"ls"},
RunE: func(cmd *cobra.Command, args []string) error {
allUsers, err := GetUsers(opts, userListAPI)
if err != nil {
return err
}
return nil
return PrintUsers(allUsers)
},
}

Expand Down
Loading
Loading