Skip to content

fix: add more stable validation for harbor-cli login#810

Merged
bupd merged 8 commits into
goharbor:mainfrom
qcserestipy:fix/login-validation
May 6, 2026
Merged

fix: add more stable validation for harbor-cli login#810
bupd merged 8 commits into
goharbor:mainfrom
qcserestipy:fix/login-validation

Conversation

@qcserestipy

Copy link
Copy Markdown
Collaborator

Description

Adds a --skip-verify-client flag to harbor login and improves error handling during client validation.

Type of Change

  • New feature

Changes

  • Added --skip-verify-client flag to skip credential validation against the Harbor server during login
  • On 4xx errors, return a clear "authentication failed" message
  • On 5xx errors, check ListProjects and Ping endpoints and report which succeeded/failed

- add a skip-verify-client flag that allows for credendtial addition without validating against the server
- client validation is based on 4xx and 5xx errors.
- 4xx errors mean unauthorized
- 5xx errors lead to more checks of other endpoints

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
Robot accounts get 412 from /users/current (undocumented in
swagger). Old code treated all 4xx as auth failure.

Now only 401/403 = auth failure. Other errors fall through to
ListProjects + Ping checks if both pass, creds are valid.

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
@codecov

codecov Bot commented Apr 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 57.57576% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 9.09%. Comparing base (60ad0bd) to head (80c05ed).
⚠️ Report is 148 commits behind head on main.

Files with missing lines Patch % Lines
cmd/harbor/root/login.go 57.57% 13 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             main    #810      +/-   ##
=========================================
- Coverage   10.99%   9.09%   -1.90%     
=========================================
  Files         173     272      +99     
  Lines        8671   13469    +4798     
=========================================
+ Hits          953    1225     +272     
- Misses       7612   12128    +4516     
- Partials      106     116      +10     

☔ 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.

@qcserestipy qcserestipy requested a review from bupd April 16, 2026 17:55
Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
GetCurrentUserInfo as primary auth check (401/403 = bad creds).
On other errors (412 robot, 5xx), fall back to ListProjects +
Ping. ListProjects fallback also checks for 401/403 to catch
bad creds that ListProjects won't silently pass.

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
Comment thread cmd/harbor/root/login.go Outdated

@NucleoFusion NucleoFusion left a comment

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.

lgtm!

qcserestipy and others added 3 commits May 3, 2026 19:25
Co-authored-by: Nucleo Fusion <148791547+NucleoFusion@users.noreply.github.com>
Signed-off-by: Patrick Eschenbach <45457307+qcserestipy@users.noreply.github.com>
govet's inline check is overly aggressive—readability of named constants
like reflect.Ptr outweighs micro-optimization gains.

Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>
Signed-off-by: Patrick Eschenbach <patrickeschenbach96@gmail.com>

@bupd bupd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/lgtm

@bupd bupd merged commit b783d52 into goharbor:main May 6, 2026
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

login fails with unhelpful swagger error when /users/current is unavailable (credentials are valid)

3 participants