Conversation
|
@blueorangutan package |
|
@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #12911 +/- ##
==========================================
Coverage 18.02% 18.02%
- Complexity 16460 16476 +16
==========================================
Files 5968 5973 +5
Lines 537213 537590 +377
Branches 65975 66014 +39
==========================================
+ Hits 96825 96922 +97
- Misses 429469 429734 +265
- Partials 10919 10934 +15
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:
|
|
|
||
| provisionCertificateViaSsh(sshConnection, hostIp, host.getName()); | ||
|
|
||
| SSHCmdHelper.sshExecuteCmd(sshConnection, "sudo service cloudstack-agent restart"); |
There was a problem hiding this comment.
- sudo could be a problem, depends on which user is used on the kvm host. please check.
- may use
systemctlinstead ofservice, please check and keep consistent with other commands.
| null, | ||
| "The ROOT CA certificate.", true); | ||
| "The ROOT CA X.509 certificate in PEM format (must start with '-----BEGIN CERTIFICATE-----'). " + | ||
| "Required when providing a custom CA. Restart management server(s) when changed.", true); |
There was a problem hiding this comment.
test and update description if an intermediate certificate exists ?
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17282 |
|
[SF] Trillian Build Failed (tid-15758) |
There was a problem hiding this comment.
Pull request overview
This PR extends CloudStack’s CA/certificate provisioning to support forced re-provisioning via SSH (for disconnected KVM hosts and SystemVMs) and improves truststore handling so CloudStack CA roots can be trusted by management and SystemVM JVM processes.
Changes:
- Add a
forcedflag toprovisionCertificate(API + manager) and implement SSH-based provisioning paths for KVM hosts and SystemVMs. - Inject configured CA certificate(s) into the management server JVM default truststore on startup (configurable via a new global setting).
- Update SystemVM/scripts truststore import logic to also populate
realhostip.keystorewith CA certs; improve cert-chain splitting robustness.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/src/main/java/org/apache/cloudstack/utils/security/KeyStoreUtils.java | Removes an unused SystemVM import-script constant. |
| utils/src/main/java/com/cloud/utils/nio/Link.java | Adjusts handshake wrap error handling to fail the handshake on SSL wrap exceptions. |
| systemvm/patch-sysvms.sh | Imports CA cert chain into realhostip.keystore during live patch for CPVM/SSVM. |
| server/src/test/java/org/apache/cloudstack/ca/CAManagerImplTest.java | Updates tests for the new provisionCertificate(..., forced) signature. |
| server/src/test/java/org/apache/cloudstack/ca/CABackgroundTaskTest.java | Updates mocks/verifications for the new method signature. |
| server/src/main/java/org/apache/cloudstack/ca/CAManagerImpl.java | Implements forced provisioning via SSH + JVM truststore injection feature and wires new dependencies. |
| server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java | Refactors KVM discovery provisioning to call caManager.provisionCertificateViaSsh(...). |
| scripts/util/keystore-cert-import | Improves cert split loop robustness; imports CA into realhostip.keystore for SystemVMs. |
| plugins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCAProvider.java | Clarifies config docs, improves fallback logging, and fixes startup loading sequence. |
| api/src/main/java/org/apache/cloudstack/ca/CAManager.java | Adds forced support + introduces CaInjectDefaultTruststore config key + adds provisionCertificateViaSsh. |
| api/src/main/java/org/apache/cloudstack/api/command/admin/ca/ProvisionCertificateCmd.java | Adds forced API parameter and passes it through to CA manager. |
Comments suppressed due to low confidence (1)
scripts/util/keystore-cert-import:78
- The certificate-chain splitting uses temp files named
cloudca.*in the current working directory. This can fail or be unsafe if the script runs from an unexpected directory (clobbering existing files or following symlinks). Consider using a dedicated temporary directory viamktemp -dand writing the split certs there, then cleaning up that directory.
awk 'BEGIN{n=0} /-----BEGIN CERTIFICATE-----/{n++}{print > "cloudca." n }' "$CACERT_FILE"
for caChain in $(ls cloudca.* 2>/dev/null); do
keytool -delete -noprompt -alias "$caChain" -keystore "$KS_FILE" -storepass "$KS_PASS" > /dev/null 2>&1 || true
keytool -import -noprompt -storepass "$KS_PASS" -trustcacerts -alias "$caChain" -file "$caChain" -keystore "$KS_FILE" > /dev/null 2>&1
done
rm -f cloudca.*
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
[SF] Trillian Build Failed (tid-15759) |
|
@blueorangutan package |
|
@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✖️ debian ✔️ suse15. SL-JID 17290 |
|
@blueorangutan package |
|
@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17300 |
|
@blueorangutan package |
|
@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17301 |
Description
This PR adds ROOT CAs to the trust store in System VMs & the management
This PR also adds another parameter
forcedtoprovisionCertificatecommand which allows provisioning certs via SSH to KVM hosts & system VMs.Details
This pull request introduces several improvements and new features to the CloudStack Certificate Authority (CA) management system, with a focus on enhancing certificate provisioning flexibility, supporting user-provided CA material, and improving truststore management. The most significant changes add support for forced certificate re-provisioning via SSH for disconnected agents, clarify and enforce requirements for custom CA keys, and enable automatic truststore injection for outgoing HTTPS connections.
Certificate Provisioning Enhancements:
forcedboolean parameter to theProvisionCertificateCmdAPI and theCAManager.provisionCertificatemethod, allowing administrators to re-provision agent certificates via SSH when agents are disconnected (e.g., after a CA change). This is supported for KVM hosts and SystemVMs. The implementation includes a newprovisionCertificateViaSshmethod for KVM hosts. [1] [2] [3] [4] [5] [6]CA Provider and Configuration Improvements:
RootCAProvider, including PEM format requirements and the need to set all three keys together. Enhanced error handling and logging when loading user-provided CA keys/certificates fails, with fallback to auto-generation. [1] [2]ca.framework.provider.pluginconfiguration key to clarify its purpose and the requirements for custom CA material.Truststore Management:
ca.framework.inject.default.truststoreto control whether the CA provider's certificate is injected into the JVM default truststore on management server startup, enabling outgoing HTTPS connections from the management server to trust servers signed by the configured CA.keystore-cert-importscript to also import the CA certificate into therealhostip.keystoreused by the SSVM JVM, ensuring trust for CloudStack CA-signed servers.Script and Utility Fixes:
keystore-cert-importscript to handle certificate splitting and file cleanup more robustly, preventing errors if no certificates are present.Code Cleanup:
LibvirtServerDiscovererand related classes as part of the refactoring for SSH-based certificate provisioning. [1] [2] [3] [4] [5] [6] [7]These changes collectively improve CloudStack's certificate management flexibility, security, and ease of integration with external CA infrastructures.
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?