fix(core): add warning when config admin password is ignored by thc1006 · Pull Request #22709 · goharbor/harbor

thc1006 · 2026-01-05T13:07:13Z

Summary

Add warning log when harbor_admin_password in config is ignored because admin password already exists in database
Refactor updateInitPassword for testability with dependency injection
Add unit tests for password initialization scenarios

Background

The harbor_admin_password setting only applies during initial installation. When users modify this value after Harbor is running, the change is silently ignored, leading to "username or password is not correct" errors when they try to login with the new password.

This is documented behavior but causes recurring confusion (see related issues below).

Changes

src/core/main.go

Extract updateInitPasswordWithMgr to accept user manager as parameter
Add warning log when config password is set but will not be applied

src/core/main_test.go (new)

Test: new user password initialization
Test: existing user password skip
Test: warning when config password set
Test: no warning when config password empty

Test Plan

Unit tests pass locally
Build passes
Manual verification with Harbor deployment

Related Issues

Fixes #22704

Related: #21981, #17122, #12639, #11600

Copilot

Pull request overview

This PR addresses a common user confusion by adding a warning message when the harbor_admin_password configuration setting is ignored during Harbor startup. The admin password setting only applies during initial installation, and changes after the first run are silently ignored, leading to login errors that are documented but frequently misunderstood.

Key Changes:

Refactored password initialization logic to support dependency injection for testability
Added warning log when config admin password is set but won't be applied due to existing database password
Implemented comprehensive unit tests covering all password initialization scenarios

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/core/main.go	Extracted `updateInitPasswordWithMgr` function with dependency injection and added conditional warning when config password is ignored
src/core/main_test.go	Added four unit tests covering new user password initialization, existing user skip behavior, warning presence/absence scenarios

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

codecov · 2026-01-06T06:37:30Z

Codecov Report

❌ Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 65.94%. Comparing base (c8c11b4) to head (52bd100).
⚠️ Report is 642 commits behind head on main.

Files with missing lines	Patch %	Lines
src/core/main.go	80.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main   #22709       +/-   ##
===========================================
+ Coverage   45.36%   65.94%   +20.57%     
===========================================
  Files         244     1074      +830     
  Lines       13333   116387   +103054     
  Branches     2719     2935      +216     
===========================================
+ Hits         6049    76751    +70702     
- Misses       6983    35385    +28402     
- Partials      301     4251     +3950

Flag	Coverage Δ
unittests	`65.94% <80.00%> (+20.57%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/core/main.go	`6.43% <80.00%> (ø)`

... and 988 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

thc1006 · 2026-01-06T06:42:19Z

TL;DR: Kindly requesting a re-run of the failed APITEST_DB_PROXY_CACHE job and the addition of a release-note/fix label. Both failures appear unrelated to the code changes in this PR.

Dear maintainers,

Thank you very much for taking the time to review this pull request. I have carefully analyzed the CI failures and would like to respectfully share my findings.

Regarding APITEST_DB_PROXY_CACHE failure:

The test failure shows Test Case - Proxy Cache | FAIL | 1 != 0, which appears to be a flaky test or infrastructure-related issue. This PR only modifies the admin password warning logic in src/core/main.go and its corresponding unit tests, and does not touch any Proxy Cache functionality. I have observed that other recent PRs have also experienced similar intermittent failures with this particular test.

I would be most grateful if you could kindly re-run the failed job at your convenience.

Regarding Check release-note label set failure:

This PR requires a release-note/* label to pass the CI check. Since this is a bug fix that addresses issue #22704, I believe release-note/fix would be the appropriate label.

Would you be so kind as to add this label when you have a moment?

I sincerely apologize for any inconvenience this request may cause. Please do not hesitate to let me know if there is anything I should modify or if you have any concerns about the implementation.

Thank you very much for your time and consideration.

Best regards

chlins

lgtm

wy65701436

lgtm

chlins

lgtm

The harbor_admin_password setting in harbor.yml only applies during initial installation. When an admin password already exists in the database, the config value is silently ignored, causing confusion when users try to change the password via configuration. This change adds a warning log when the config password is set but will not be applied because a password already exists. The warning guides users to change the password via Harbor UI or API instead. Refactored updateInitPassword to accept a user manager parameter for testability. Added unit tests covering: - New user password initialization - Existing user password skip - Warning when config password differs - No warning when config password is empty Fixes goharbor#22704 Signed-off-by: thc1006 <[email protected]>

- Remove updateInitPasswordWithMgr wrapper function per reviewer request - Add userID == adminUserID check before showing admin-specific warning - Remove unit tests that depended on the removed DI pattern Signed-off-by: thc1006 <[email protected]>

- Use log.Warning instead of log.Warningf (no format specifier needed) - Combine message into single line to avoid string concatenation Signed-off-by: thc1006 <[email protected]>

- Add updateInitPasswordWithMgr for dependency injection (testability) - Add unit tests with 100% coverage on new warning log code Signed-off-by: thc1006 <[email protected]>

…r#22709) * fix(core): add warning when config admin password is ignored The harbor_admin_password setting in harbor.yml only applies during initial installation. When an admin password already exists in the database, the config value is silently ignored, causing confusion when users try to change the password via configuration. This change adds a warning log when the config password is set but will not be applied because a password already exists. The warning guides users to change the password via Harbor UI or API instead. Refactored updateInitPassword to accept a user manager parameter for testability. Added unit tests covering: - New user password initialization - Existing user password skip - Warning when config password differs - No warning when config password is empty Fixes goharbor#22704 Signed-off-by: thc1006 <[email protected]> * fix(core): simplify admin password warning per review feedback - Remove updateInitPasswordWithMgr wrapper function per reviewer request - Add userID == adminUserID check before showing admin-specific warning - Remove unit tests that depended on the removed DI pattern Signed-off-by: thc1006 <[email protected]> * fix(core): simplify warning log per review feedback - Use log.Warning instead of log.Warningf (no format specifier needed) - Combine message into single line to avoid string concatenation Signed-off-by: thc1006 <[email protected]> * fix(core): add unit tests for admin password warning - Add updateInitPasswordWithMgr for dependency injection (testability) - Add unit tests with 100% coverage on new warning log code Signed-off-by: thc1006 <[email protected]> --------- Signed-off-by: thc1006 <[email protected]> Signed-off-by: sias32 <[email protected]>

Copilot AI review requested due to automatic review settings January 5, 2026 13:07

thc1006 requested a review from a team as a code owner January 5, 2026 13:07

github-actions Bot assigned MinerYang, OrlinVasilev and Vad1mo Jan 5, 2026

Copilot started reviewing on behalf of thc1006 January 5, 2026 13:07 View session

thc1006 mentioned this pull request Jan 5, 2026

change config harbor_admin_password params, start harbor service occur "username or password is not correct" #22704

Closed

Copilot AI reviewed Jan 5, 2026

View reviewed changes

Vad1mo added the release-note/update Update or Fix label Jan 6, 2026

wy65701436 reviewed Jan 11, 2026

View reviewed changes

Comment thread src/core/main.go

wy65701436 reviewed Jan 11, 2026

View reviewed changes

Comment thread src/core/main.go Outdated

thc1006 force-pushed the fix/admin-password-warning-22704 branch 2 times, most recently from c095506 to 8d3b21d Compare January 12, 2026 08:03

thc1006 requested a review from wy65701436 January 12, 2026 08:04

chlins reviewed Jan 22, 2026

View reviewed changes

Comment thread src/core/main.go

thc1006 requested a review from chlins January 22, 2026 18:03

chlins approved these changes Jan 23, 2026

View reviewed changes

chlins force-pushed the fix/admin-password-warning-22704 branch from ffd8565 to e98a612 Compare January 23, 2026 03:01

chlins enabled auto-merge (squash) January 23, 2026 03:02

chlins force-pushed the fix/admin-password-warning-22704 branch from e98a612 to 6b6f159 Compare January 23, 2026 03:53

auto-merge was automatically disabled January 23, 2026 10:50
Head branch was pushed to by a user without write access

thc1006 force-pushed the fix/admin-password-warning-22704 branch from edc916b to e73675a Compare January 23, 2026 10:50

thc1006 requested a review from chlins January 23, 2026 17:21

wy65701436 approved these changes Jan 26, 2026

View reviewed changes

wy65701436 enabled auto-merge (squash) January 26, 2026 06:36

chlins approved these changes Jan 26, 2026

View reviewed changes

thc1006 added 2 commits January 26, 2026 14:41

fix(core): simplify warning log per review feedback

ec1fa43

- Use log.Warning instead of log.Warningf (no format specifier needed) - Combine message into single line to avoid string concatenation Signed-off-by: thc1006 <[email protected]>

fix(core): add unit tests for admin password warning

52bd100

- Add updateInitPasswordWithMgr for dependency injection (testability) - Add unit tests with 100% coverage on new warning log code Signed-off-by: thc1006 <[email protected]>

chlins force-pushed the fix/admin-password-warning-22704 branch from 858191b to 52bd100 Compare January 26, 2026 06:41

wy65701436 merged commit d905a05 into goharbor:main Jan 26, 2026
12 checks passed

thc1006 deleted the fix/admin-password-warning-22704 branch January 26, 2026 07:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(core): add warning when config admin password is ignored#22709

fix(core): add warning when config admin password is ignored#22709
wy65701436 merged 4 commits into
goharbor:mainfrom
thc1006:fix/admin-password-warning-22704

thc1006 commented Jan 5, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

codecov Bot commented Jan 6, 2026 •

edited

Loading

Uh oh!

thc1006 commented Jan 6, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chlins left a comment

Uh oh!

wy65701436 left a comment

Uh oh!

chlins left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Uh oh!

Conversation

thc1006 commented Jan 5, 2026

Summary

Background

Changes

Test Plan

Related Issues

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

codecov Bot commented Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

thc1006 commented Jan 6, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chlins left a comment

Choose a reason for hiding this comment

Uh oh!

wy65701436 left a comment

Choose a reason for hiding this comment

Uh oh!

chlins left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

codecov Bot commented Jan 6, 2026 •

edited

Loading