chore: sync with upstream goharbor/harbor#108
Conversation
Remove the unused function MostMatchSorter, it should not be implemented in golang, should be implement in the db query. Remove the unused function onBoardCommonUserGroup() fixes goharbor#22573 Signed-off-by: stonezdj <stonezdj@gmail.com>
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Comment |
This commit fixes the style issues introduced in e90bf7c according to the output from Gofumpt and Prettier. Details: container-registry/harbor-next#108
|
There was a problem hiding this comment.
1 issue found across 9 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/pkg/user/manager.go">
<violation number="1" location="src/pkg/user/manager.go:32">
P3: Incorrect comment: This is a user manager, not a project manager. The package is `user` and the Manager interface handles user operations.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| // Mgr is the global project manager | ||
| Mgr = New() | ||
| ) | ||
| // Mgr is the global project manager |
There was a problem hiding this comment.
P3: Incorrect comment: This is a user manager, not a project manager. The package is user and the Manager interface handles user operations.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/pkg/user/manager.go, line 32:
<comment>Incorrect comment: This is a user manager, not a project manager. The package is `user` and the Manager interface handles user operations.</comment>
<file context>
@@ -29,10 +29,8 @@ import (
- // Mgr is the global project manager
- Mgr = New()
-)
+// Mgr is the global project manager
+var Mgr = New()
</file context>
| // Mgr is the global project manager | |
| // Mgr is the global user manager |
There was a problem hiding this comment.
3 issues found across 16 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/pkg/user/manager.go">
<violation number="1" location="src/pkg/user/manager.go:250">
P2: Inconsistent handling of default admin exclusion. The `List` and `Count` methods exclude the default admin user (user_id=1) by default, but `SearchByName` doesn't follow this pattern. This could expose the default admin through fuzzy search when other listing methods would hide it. Consider adding the `options ...models.Option` parameter to support `WithDefaultAdmin()` opt-in, similar to the existing `List` method.</violation>
</file>
<file name="src/server/v2.0/handler/user.go">
<violation number="1" location="src/server/v2.0/handler/user.go:281">
P1: Pagination is broken: `SearchByName` only accepts `limitSize` but ignores `params.Page`. Requests for page 2+ will return the same results as page 1, while the response headers (X-Total-Count, Link) still suggest pagination works.</violation>
</file>
<file name="src/server/v2.0/handler/usergroup.go">
<violation number="1" location="src/server/v2.0/handler/usergroup.go:205">
P1: Potential nil pointer dereference: `*params.PageSize` is dereferenced without checking if `PageSize` is nil. Unlike `BuildQuery` which safely handles nil pageSize, this line will panic if the parameter is not provided in the request.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| return operation.NewSearchUsersOK().WithXTotalCount(0).WithPayload([]*models.UserSearchRespItem{}) | ||
| } | ||
| l, err := u.ctl.List(ctx, query) | ||
| l, err := u.ctl.SearchByName(ctx, params.Username, int(*params.PageSize)) |
There was a problem hiding this comment.
P1: Pagination is broken: SearchByName only accepts limitSize but ignores params.Page. Requests for page 2+ will return the same results as page 1, while the response headers (X-Total-Count, Link) still suggest pagination works.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/server/v2.0/handler/user.go, line 281:
<comment>Pagination is broken: `SearchByName` only accepts `limitSize` but ignores `params.Page`. Requests for page 2+ will return the same results as page 1, while the response headers (X-Total-Count, Link) still suggest pagination works.</comment>
<file context>
@@ -279,7 +278,7 @@ func (u *usersAPI) SearchUsers(ctx context.Context, params operation.SearchUsers
return operation.NewSearchUsersOK().WithXTotalCount(0).WithPayload([]*models.UserSearchRespItem{})
}
- l, err := u.ctl.List(ctx, query)
+ l, err := u.ctl.SearchByName(ctx, params.Username, int(*params.PageSize))
if err != nil {
return u.SendError(ctx, err)
</file context>
| return operation.NewSearchUserGroupsOK().WithXTotalCount(0).WithPayload([]*models.UserGroupSearchItem{}) | ||
| } | ||
| ug, err := u.ctl.List(ctx, query) | ||
| ug, err := u.ctl.SearchByName(ctx, params.Groupname, int(*params.PageSize)) |
There was a problem hiding this comment.
P1: Potential nil pointer dereference: *params.PageSize is dereferenced without checking if PageSize is nil. Unlike BuildQuery which safely handles nil pageSize, this line will panic if the parameter is not provided in the request.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/server/v2.0/handler/usergroup.go, line 205:
<comment>Potential nil pointer dereference: `*params.PageSize` is dereferenced without checking if `PageSize` is nil. Unlike `BuildQuery` which safely handles nil pageSize, this line will panic if the parameter is not provided in the request.</comment>
<file context>
@@ -204,14 +202,11 @@ func (u *userGroupAPI) SearchUserGroups(ctx context.Context, params operation.Se
return operation.NewSearchUserGroupsOK().WithXTotalCount(0).WithPayload([]*models.UserGroupSearchItem{})
}
- ug, err := u.ctl.List(ctx, query)
+ ug, err := u.ctl.SearchByName(ctx, params.Groupname, int(*params.PageSize))
if err != nil {
return u.SendError(ctx, err)
</file context>
| u.PasswordVersion = utils.SHA256 | ||
| } | ||
|
|
||
| func (m *manager) SearchByName(ctx context.Context, name string, limitSize int) (commonmodels.Users, error) { |
There was a problem hiding this comment.
P2: Inconsistent handling of default admin exclusion. The List and Count methods exclude the default admin user (user_id=1) by default, but SearchByName doesn't follow this pattern. This could expose the default admin through fuzzy search when other listing methods would hide it. Consider adding the options ...models.Option parameter to support WithDefaultAdmin() opt-in, similar to the existing List method.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/pkg/user/manager.go, line 250:
<comment>Inconsistent handling of default admin exclusion. The `List` and `Count` methods exclude the default admin user (user_id=1) by default, but `SearchByName` doesn't follow this pattern. This could expose the default admin through fuzzy search when other listing methods would hide it. Consider adding the `options ...models.Option` parameter to support `WithDefaultAdmin()` opt-in, similar to the existing `List` method.</comment>
<file context>
@@ -244,3 +246,7 @@ func injectPasswd(u *commonmodels.User, password string) {
u.PasswordVersion = utils.SHA256
}
+
+func (m *manager) SearchByName(ctx context.Context, name string, limitSize int) (commonmodels.Users, error) {
+ return m.dao.SearchByName(ctx, name, limitSize)
+}
</file context>


Automated PR to sync 1 new commit(s) from upstream goharbor/harbor main branch.
Merge strategy: Our changes in
nextare preserved on conflicts (upstream changes are additive only).Note: The
.githubfolder is preserved and not synced from upstream.Summary by cubic
Syncs with upstream Harbor and fixes user and group search by moving matching and sorting into SQL for correct, fast results.
Bug Fixes
Refactors
Written for commit 8f3a1b6. Summary will update on new commits.