Uh oh!
There was an error while loading. Please reload this page.
config: add error logging when fail to decrypt a encrypted global configuration - #13190
config: add error logging when fail to decrypt a encrypted global configuration#13190weizhouapache wants to merge 1 commit into
Conversation
weizhouapache
commented
May 19, 2026
@blueorangutan package |
blueorangutan
commented
May 19, 2026
@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@## 4.22 #13190 +/- ##
============================================
- Coverage 17.67% 17.67% -0.01% + Complexity 15788 15787 -1
============================================
Files 5922 5922 Lines 533123 533127 +4 Branches 65201 65200 -1 ============================================
- Hits 94242 94237 -5 - Misses 428237 428245 +8 - Partials 10644 10645 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
blueorangutan
commented
May 19, 2026
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17910 |
weizhouapache
commented
May 19, 2026
@blueorangutan test |
blueorangutan
commented
May 19, 2026
@weizhouapache a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
There was a problem hiding this comment.
Pull request overview
This PR improves observability around global configuration retrieval by adding explicit error logging when an encrypted configuration value cannot be decrypted (e.g., wrong/missing management server encryption key), and aligns config retrieval to use the DAO’s getValue(...) path.
Changes:
- Add error logging in
ConfigurationDaoImpl.getValue(...)whenConfigurationVO.getValue()throws during decryption. - Update
ConfigDepotImplto use_configDao.getValue(key)for global-scope string retrieval. - Adjust
ConfigDepotImplTestmocks/verifications to match the new DAO method usage.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| framework/config/src/main/java/org/apache/cloudstack/framework/config/dao/ConfigurationDaoImpl.java | Adds error logging around exceptions when retrieving/decrypting a global config value. |
| framework/config/src/main/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImpl.java | Switches global config retrieval to _configDao.getValue(...) to centralize logging/behavior. |
| framework/config/src/test/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImplTest.java | Updates unit tests to mock/verify _configDao.getValue(...) instead of findById(...) for global retrieval. |
| framework/config/src/main/java/org/apache/cloudstack/framework/config/impl/ConfigurationVO.java | Minor formatting change in getValue() conditional. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| logger.error("Unable to get global configuration {}: {}. " + | ||
| "We expect the value of setting to be encrypted in the database with the Management Server's key, " + | ||
| "but we were unable to decrypt it using this key", name, ex.getMessage()); |
blueorangutan
commented
May 20, 2026
[SF] Trillian test result (tid-16109)
|
Description
This PR addresses #12523 by improving the logging
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?