Skip to content

#fix: improve replication policy config logging and credential error m…#649

Merged
bupd merged 3 commits into
goharbor:mainfrom
kryptonn36:logs_added
Feb 10, 2026
Merged

#fix: improve replication policy config logging and credential error m…#649
bupd merged 3 commits into
goharbor:mainfrom
kryptonn36:logs_added

Conversation

@kryptonn36

Copy link
Copy Markdown

This PR improves observability and user experience when creating replication policies via CLI.

Changes:

  • Added debug logs for replication policy config loading, YAML parsing, and validation stages
  • Improved credential-related error message to guide users to run harbor login

Testing:

  • Tested with valid replication policy YAML using --verbose flag
  • Verified config load, parse, and validation flow
  • Verified improved error message when credentials are missing

…essage

Signed-off-by: krypton36 <srai325356@gmail.com>

Copilot AI 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.

Pull request overview

This pull request improves observability and user experience when creating replication policies via CLI by adding debug logging at key stages of the configuration loading process and improving the error message when Harbor credentials are missing.

Changes:

  • Added debug logs to track replication policy config file reading, parsing (YAML/JSON), and validation stages
  • Updated credential error message to be more actionable by instructing users to run harbor login

Reviewed changes

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

File Description
pkg/utils/client.go Improved error message to guide users to run harbor login when credentials are not configured
pkg/config/replication/policies.go Added debug logging for config file reading, parsing, and validation to improve observability

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

Comment thread pkg/config/replication/policies.go Outdated
Comment on lines +77 to +78
log.Debug("Replication policy config file read successfully")

Copilot AI Jan 26, 2026

Copy link

Choose a reason for hiding this comment

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

There are extra blank lines after the log.Debug statement that are inconsistent with the codebase style. Similar log statements in the codebase (e.g., in cmd/harbor/root/replication/policies/list.go) typically have only one blank line after them, not two.

Copilot uses AI. Check for mistakes.
Comment on lines +86 to +87
log.Debugf("Parsed %s configuration successfully", fileType)

Copilot AI Jan 26, 2026

Copy link

Choose a reason for hiding this comment

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

There are extra blank lines after the log.Debugf statement that are inconsistent with the codebase style. Similar log statements in the codebase typically have only one blank line after them, not two.

Copilot uses AI. Check for mistakes.
Comment thread pkg/config/replication/policies.go Outdated
Comment on lines +92 to +93
log.Debugf("Parsed %s configuration successfully", fileType)

Copilot AI Jan 26, 2026

Copy link

Choose a reason for hiding this comment

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

There are extra blank lines after the log.Debugf statement that are inconsistent with the codebase style. Similar log statements in the codebase typically have only one blank line after them, not two.

Copilot uses AI. Check for mistakes.
Comment on lines +101 to +102
log.Debug("Replication policy configuration validated successfully")

Copilot AI Jan 26, 2026

Copy link

Choose a reason for hiding this comment

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

There are extra blank lines after the log.Debug statement that are inconsistent with the codebase style. Similar log statements in the codebase typically have only one blank line after them, not two.

Copilot uses AI. Check for mistakes.
@codecov

codecov Bot commented Jan 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 7.17%. Comparing base (60ad0bd) to head (fafad5f).
⚠️ Report is 89 commits behind head on main.

Files with missing lines Patch % Lines
pkg/config/replication/policies.go 0.00% 4 Missing ⚠️
pkg/utils/client.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             main    #649      +/-   ##
=========================================
- Coverage   10.99%   7.17%   -3.82%     
=========================================
  Files         173     260      +87     
  Lines        8671   12851    +4180     
=========================================
- Hits          953     922      -31     
- Misses       7612   11821    +4209     
- Partials      106     108       +2     

☔ 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 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you for your contribution! :) Before merging this PR please make sure to fix the failing lint tasks in the pipeline. Please also make sure to remove compiled binaries.

Otherwise LGTM

Comment thread harbor-cli Outdated
Signed-off-by: krypton36 <srai325356@gmail.com>
@kryptonn36

Copy link
Copy Markdown
Author

Thanks for the review! I’ve fixed the lint issues, removed compiled binaries from the repository as suggested. Please let me know if anything else is needed.

Signed-off-by: krypton36 <srai325356@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 a1ba72e into goharbor:main Feb 10, 2026
6 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.

4 participants