-
Couldn't load subscription status.
- Fork 92
fix: defer keyring initialization to prevent build-time keychain errors #562
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: defer keyring initialization to prevent build-time keychain errors #562
Conversation
Signed-off-by: Rui Chen <[email protected]>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes build-time keychain errors by deferring keyring initialization from package init time to first use through lazy initialization.
- Removed
init()function and replaced withensureKeyringProvider()usingsync.Oncefor thread-safe lazy initialization - Added calls to
ensureKeyringProvider()inGenerateEncryptionKey()andGetEncryptionKey()functions - Prevents keyring system from being accessed during build/package initialization
Comments suppressed due to low confidence (1)
pkg/utils/encryption.go:1
- The
ensureKeyringProvider()call inGetEncryptionKey()lacks test coverage to verify that the keyring provider is properly initialized on first use. Consider adding a test that validates lazy initialization behavior.
// Copyright Project Harbor Authors
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #562 +/- ##
=========================================
- Coverage 10.99% 6.74% -4.25%
=========================================
Files 173 258 +85
Lines 8671 15382 +6711
=========================================
+ Hits 953 1037 +84
- Misses 7612 14239 +6627
Partials 106 106 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
thoughts of merging this change? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
@qcserestipy thanks for the merge, can we cut a new release, this would help with shipping the project in homebrew-core, Homebrew/homebrew-core#250518. |
seeing some build issue with keychain, and patch to do lazy init