refactor(robot): unify config loading and permission building functions #531

qcserestipy · 2025-08-18T12:19:08Z

Overview

This is the first of several pull requests that will remove code duplication and enhance maintainability of the robot account functionality of the Harbor CLI. Those PRs will be kept small for easier reviewing purposes.

Goal

SImplify existing code

Non Goal

Add new Features

Current changes

Consolidate loadFromConfigFileForCreate and loadFromConfigFileForUpdate into single loadRobotConfigFromFile function with isUpdate flag
Merge buildMergedPermissions and buildMergedPermissionsForUpdate into unified buildMergedPermissions function using shared rmodel.RobotPermission type
Add backward compatibility wrapper functions for existing callers
Reduce code duplication and improve maintainability

- Consolidate loadFromConfigFileForCreate and loadFromConfigFileForUpdate into single loadRobotConfigFromFile function with isUpdate flag - Merge buildMergedPermissions and buildMergedPermissionsForUpdate into unified buildMergedPermissions function using shared rmodel.RobotPermission type - Add backward compatibility wrapper functions for existing callers - Reduce code duplication and improve maintainability Signed-off-by: Patrick Eschenbach <[email protected]>

codecov · 2025-08-18T12:21:33Z

Codecov Report

❌ Patch coverage is 0% with 593 lines in your changes missing coverage. Please review.
✅ Project coverage is 6.75%. Comparing base (60ad0bd) to head (02c6b61).
⚠️ Report is 53 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/prompt/robot/robot.go	0.00%	112 Missing ⚠️
pkg/robot/permissions_project.go	0.00%	88 Missing ⚠️
pkg/robot/builders.go	0.00%	83 Missing ⚠️
pkg/views/base/multiselect/model.go	0.00%	78 Missing ⚠️
pkg/robot/config_loader.go	0.00%	64 Missing ⚠️
pkg/robot/prompt.go	0.00%	40 Missing ⚠️
pkg/robot/permissions_system.go	0.00%	38 Missing ⚠️
pkg/prompt/prompt.go	0.00%	28 Missing ⚠️
cmd/harbor/root/project/robot/create.go	0.00%	25 Missing ⚠️
pkg/views/project/select/view.go	0.00%	21 Missing ⚠️
... and 4 more

Additional details and impacted files

@@            Coverage Diff            @@
##             main    #531      +/-   ##
=========================================
- Coverage   10.99%   6.75%   -4.24%     
=========================================
  Files         173     264      +91     
  Lines        8671   15265    +6594     
=========================================
+ Hits          953    1031      +78     
- Misses       7612   14127    +6515     
- Partials      106     107       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull Request Overview

This PR consolidates duplicate code in the Harbor CLI robot account functionality by unifying config loading and permission building functions. The refactor reduces code duplication while maintaining backward compatibility.

Key changes:

Consolidates separate config loading functions into a unified loadRobotConfigFromFile function with an isUpdate flag
Merges duplicate permission building logic into a single buildMergedPermissions function using shared types
Creates shared RobotPermission model to eliminate type duplication across create/update views

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
pkg/models/robot/permission.go	Introduces shared `RobotPermission` model to replace duplicate type definitions
pkg/views/robot/create/view.go	Removes duplicate type definitions, uses shared model
pkg/views/robot/update/view.go	Removes duplicate type definitions, uses shared model
cmd/harbor/root/robot/common.go	Consolidates config loading and permission building logic into unified functions
cmd/harbor/root/robot/create.go	Removes duplicate functions, calls unified implementations
cmd/harbor/root/robot/update.go	Removes duplicate functions, calls unified implementations
pkg/views/base/multiselect/model.go	Refactors multiselect UI component with improved list-based implementation
pkg/views/project/select/view.go	Adds `ProjectsList` function using the refactored multiselect component
pkg/prompt/prompt.go	Adds `GetProjectNamesFromUser` function for multi-project selection
pkg/config/robot/robot.go	Updates to use shared `RobotPermission` model
cmd/harbor/root/project/robot/create.go	Updates to use shared `RobotPermission` model
cmd/harbor/root/project/robot/update.go	Updates to use shared `RobotPermission` model

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-08-18T12:52:12Z

pkg/robot/config_loader.go

+	"github.com/sirupsen/logrus"
+)
+
+func loadRobotConfigFromFile(configFile string, permissions *[]models.Permission, projectPermissionsMap map[string][]models.Permission, isUpdate bool, createOpts *create.CreateView, updateOpts *update.UpdateView) error {


[nitpick] This function has too many parameters (6). Consider using a struct to group related parameters or splitting into separate functions for create and update operations to improve readability and maintainability.

Copilot · 2025-08-18T12:52:13Z

cmd/harbor/root/robot/common.go

+		Namespace: "/",
+		Access:    accessesSystem,
+		Kind:      "system",
+	})


System permissions are always added regardless of whether accessesSystem is empty. This could create empty system permission entries. Consider checking if len(accessesSystem) > 0 before adding system permissions, similar to how it was handled in the original buildMergedPermissionsForUpdate function.

Suggested change

})

if len(accessesSystem) > 0 {

mergedPermissions = append(mergedPermissions, &rmodel.RobotPermission{

Namespace: "/",

Access: accessesSystem,

Kind: "system",

})

}

pkg/views/base/multiselect/model.go

Co-authored-by: Copilot <[email protected]> Signed-off-by: Patrick Eschenbach <[email protected]>

bupd

@qcserestipy

▼ Container.withExec(
  ┆ args: ["go", "build", "-ldflags", "-X github.com/goharbor/harbor-cli/cmd/harbor/internal/version.Version=dev\n\t\t\t\t\t\t  -X github.com/goharbor/harbor-cli/cmd/harbor/internal/version.GoVersion=1.24.4\n\t\t\t\t\t\t  -X github.com/goharbor/harbor-cli/cmd/harbor/internal/version.BuildTime=2025-08-19T11:42:40Z\n\t\t\t\t\t\t  -X github.com/goharbor/harbor-cli/cmd/harbor/internal/version.GitCommit=63a3335\n\n\t\t\t\t", "-o", "/bin/harbor-cli", "/src/cmd/harbor/main.go"]
  ): Container! 3.3s ERROR
go: downloading golang.org/x/term v0.34.0
go: downloading golang.org/x/text v0.28.0
go: downloading golang.org/x/sys v0.35.0
# github.com/goharbor/harbor-cli/pkg/views/base/multiselect
pkg/views/base/multiselect/model.go:72:6: syntax error: unexpected name NewModel, expected (
pkg/views/base/multiselect/model.go:93:23: syntax error: unexpected name tea at end of statement
pkg/views/base/multiselect/model.go:139:23: syntax error: unexpected name string at end of statement
! process "go build -ldflags -X github.com/goharbor/harbor-cli/cmd/harbor/internal/version.Version=dev\n\t\t\t\t\t\t  -X github.com/goharbor/harbor-
  cli/cmd/harbor/internal/version.GoVersion=1.24.4\n\t\t\t\t\t\t  -X github.com/goharbor/harbor-cli/cmd/harbor/internal/version.BuildTime=2025-08-19T11:42:40Z\n\t\t\t\t\t\t  -X
  github.com/goharbor/harbor-cli/cmd/harbor/internal/version.GitCommit=63a3335\n\n\t\t\t\t -o /bin/harbor-cli /src/cmd/harbor/main.go" did not complete successfully: exit code: 1

please fix the above error. and fix lint issues

Signed-off-by: patrick.eschenbach <[email protected]>

qcserestipy · 2025-08-19T11:59:32Z

@bupd Sorry should have checked Copilots changes more carefully before comitting them

…ssion flows - Extract robot prompt helpers to pkg/prompt/robot (ChooseProjectPermissionMode, AskMoreProjects, ConfirmReplaceExisting, ChooseModifyMode, SelectProjects, AskUpdateSystemPerms) - Refactor cmd/harbor/root/robot/robot_permissions_project.go: split create/update flows; add small helpers (assignCommonPermissionsToSelectedProjects, assignPerProjectPermissionsInteractive, modifyPerProjectPermissionsInteractive, addProjectsInteractively, clearProjectPermissions, selectExistingProjectsForModify); remove duplication without behavior changes - Refactor cmd/harbor/root/robot/robot_permissions_system.go to use shared prompt helpers - Unify config loading and permission builders; keep public behavior stable - Remove unused splitPermissionsFromRobot and minor dead code Signed-off-by: Patrick Eschenbach <[email protected]>

…ckage Signed-off-by: Patrick Eschenbach <[email protected]>

…rsion error Signed-off-by: Patrick Eschenbach <[email protected]>

Copilot

Pull Request Overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-30T13:11:37Z

pkg/robot/builders.go

+	mergedPermissions = append(mergedPermissions, &rmodel.RobotPermission{
+		Namespace: "/",
+		Access:    accessesSystem,
+		Kind:      "system",
+	})


The system permissions are being added unconditionally, but this could append empty system permissions when accessesSystem is nil or empty. Consider adding a length check: if len(accessesSystem) > 0 { ... }

Suggested change

mergedPermissions = append(mergedPermissions, &rmodel.RobotPermission{

Namespace: "/",

Access: accessesSystem,

Kind: "system",

})

if len(accessesSystem) > 0 {

mergedPermissions = append(mergedPermissions, &rmodel.RobotPermission{

Namespace: "/",

Access: accessesSystem,

Kind: "system",

})

}

Signed-off-by: Prasanth Baskar <[email protected]>

…e central functions in project robot Signed-off-by: Patrick Eschenbach <[email protected]>

Signed-off-by: Patrick Eschenbach <[email protected]>

…to robot_refactor

Signed-off-by: Patrick Eschenbach <[email protected]>

…roject robot create Signed-off-by: Patrick Eschenbach <[email protected]>

qcserestipy requested review from bupd and Copilot and removed request for bupd August 18, 2025 12:47

Copilot AI reviewed Aug 18, 2025

View reviewed changes

qcserestipy and others added 2 commits August 18, 2025 18:50

Update pkg/views/base/multiselect/model.go

c85fe3e

Co-authored-by: Copilot <[email protected]> Signed-off-by: Patrick Eschenbach <[email protected]>

Update pkg/views/base/multiselect/model.go

63a3335

Co-authored-by: Copilot <[email protected]> Signed-off-by: Patrick Eschenbach <[email protected]>

bupd requested changes Aug 19, 2025

View reviewed changes

Fix: revert some changes from copilot review

f653dd0

Signed-off-by: patrick.eschenbach <[email protected]>

qcserestipy added 3 commits September 28, 2025 14:06

fix(merge): merge upstream changes and resolve conflicts in prompt pa…

b0e9633

…ckage Signed-off-by: Patrick Eschenbach <[email protected]>

fix(dagger): update dagger go version to get rid of govulncheck go ve…

bb470d4

…rsion error Signed-off-by: Patrick Eschenbach <[email protected]>

qcserestipy requested a review from bupd September 28, 2025 12:41

Vad1mo requested a review from Copilot September 30, 2025 13:11

Copilot AI reviewed Sep 30, 2025

View reviewed changes

bupd and others added 7 commits October 8, 2025 02:42

Merge branch 'main' into robot_refactor

8dc53ea

Signed-off-by: Prasanth Baskar <[email protected]>

refactor(robot): create robot package and move shared functions; reus…

1240e68

…e central functions in project robot Signed-off-by: Patrick Eschenbach <[email protected]>

fix(conflict): merge upstream and resolve conflicts

be5e5cd

Signed-off-by: Patrick Eschenbach <[email protected]>

Merge branch 'robot_refactor' of github.com:qcserestipy/harbor-cli in…

c6d52eb

…to robot_refactor

fix(lint): remove trailing white space

6ee9ab6

Signed-off-by: Patrick Eschenbach <[email protected]>

fix(lint): export docs

8750fd8

Signed-off-by: Patrick Eschenbach <[email protected]>

refactor(robot): add RobotBuilder for permission handling; clean up p…

02c6b61

…roject robot create Signed-off-by: Patrick Eschenbach <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor(robot): unify config loading and permission building functions #531

refactor(robot): unify config loading and permission building functions #531

Uh oh!

qcserestipy commented Aug 18, 2025

Uh oh!

codecov bot commented Aug 18, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Aug 18, 2025

Uh oh!

Copilot AI Aug 18, 2025

Uh oh!

Uh oh!

Uh oh!

bupd left a comment

Uh oh!

qcserestipy commented Aug 19, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Sep 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

refactor(robot): unify config loading and permission building functions #531

Are you sure you want to change the base?

refactor(robot): unify config loading and permission building functions #531

Uh oh!

Conversation

qcserestipy commented Aug 18, 2025

Overview

Goal

Non Goal

Current changes

Uh oh!

codecov bot commented Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bupd left a comment

Choose a reason for hiding this comment

Uh oh!

qcserestipy commented Aug 19, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Aug 18, 2025 •

edited

Loading