-
Notifications
You must be signed in to change notification settings - Fork 4
DATAGO-116001: event-management-agent: Configure JCSMP to use default JDK truststore for Event Portal connections #282
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
DATAGO-116001: event-management-agent: Configure JCSMP to use default JDK truststore for Event Portal connections #282
Conversation
| boolean isCustomCACertConfigured() { | ||
| String customCaCertsPresent = System.getenv("CUSTOM_CA_CERTS_PRESENT"); | ||
|
|
||
| return ("1".equals(customCaCertsPresent)); | ||
| } |
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.
Package private to aid in testing
| void setDefaultTrustStore(Properties properties) { | ||
| String javaHome = System.getProperty("java.home"); | ||
| if (StringUtils.isBlank(javaHome)) { | ||
| log.warn("java.home system property is not set. Cannot configure default truststore for JCSMP."); | ||
| return; | ||
| } | ||
| Path defaultTrustStorePath = Paths.get(javaHome, "lib", "security", "cacerts"); | ||
| File trustStoreFile = defaultTrustStorePath.toFile(); | ||
|
|
||
| if (!trustStoreFile.exists() || !trustStoreFile.canRead()) { | ||
| log.warn("Default truststore not found or not readable at: {}. JCSMP connection may fail.", defaultTrustStorePath); | ||
| return; | ||
| } | ||
|
|
||
| log.debug("Custom CA certificates present. Explicitly configuring EVMR connection to use default truststore: {}", defaultTrustStorePath); | ||
| properties.setProperty(SolaceProperties.TransportLayerSecurityProperties.TRUST_STORE_PATH, defaultTrustStorePath.toString()); | ||
| } | ||
|
|
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.
package private to help in testing
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 configures the JCSMP (Java Client Solace Messaging Protocol) to use the default JDK truststore when connecting to Event Portal, specifically when custom CA certificates are present in the environment.
Key Changes:
- Added logic to detect custom CA certificates via environment variable and configure JCSMP truststore accordingly
- Implemented truststore path validation and file existence checks
- Added unit tests to verify the truststore configuration behavior
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| VMRProperties.java | Added methods to check for custom CA certs and configure default JDK truststore path for JCSMP connections |
| VMRPropertiesTests.java | Added tests verifying truststore configuration is called/skipped based on custom CA cert presence; fixed package declaration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| properties.setProperty(SolaceProperties.AuthenticationProperties.SCHEME_BASIC_PASSWORD, password); | ||
| properties.setProperty(SolaceProperties.ClientProperties.NAME, clientName); | ||
|
|
||
| //We will always use the default jks truststore for connecting to the EVMR |
Copilot
AI
Nov 4, 2025
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.
The comment states 'We will always use the default jks truststore' but the implementation only configures it when CUSTOM_CA_CERTS_PRESENT=1. Update the comment to accurately reflect the conditional behavior.
| //We will always use the default jks truststore for connecting to the EVMR | |
| // Explicitly configure the default JKS truststore for connecting to the EVMR only when custom CA certificates are present |
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.
I agree with this comment 👍
| boolean isCustomCACertConfigured() { | ||
| String customCaCertsPresent = System.getenv("CUSTOM_CA_CERTS_PRESENT"); | ||
|
|
||
| return ("1".equals(customCaCertsPresent)); | ||
| } |
Copilot
AI
Nov 4, 2025
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.
The method name isCustomCACertConfigured is misleading - it checks whether custom CA certs are present, not whether they are configured. Consider renaming to hasCustomCACerts or isCustomCACertPresent to better reflect what it checks.
| @Test | ||
| @SneakyThrows | ||
| void testSetDefaultTrustStoreCalledWhenCustomCaCertsPresent() { | ||
| // Spy on vmrProperties to mock getCustomCaCertsPresentEnv and verify setDefaultTrustStore is called |
Copilot
AI
Nov 4, 2025
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.
The comment references a non-existent method getCustomCaCertsPresentEnv. It should reference isCustomCACertConfigured instead to match the actual method being mocked.
| @Test | ||
| @SneakyThrows | ||
| void testSetDefaultTrustStoreNotCalledWhenCustomCaCertsNotPresent() { | ||
| // Spy on vmrProperties to mock getCustomCaCertsPresentEnv and verify setDefaultTrustStore is NOT called |
Copilot
AI
Nov 4, 2025
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.
The comment references a non-existent method getCustomCaCertsPresentEnv. It should reference isCustomCACertConfigured instead to match the actual method being mocked.
| // Spy on vmrProperties to mock getCustomCaCertsPresentEnv and verify setDefaultTrustStore is NOT called | |
| // Spy on vmrProperties to mock isCustomCACertConfigured and verify setDefaultTrustStore is NOT called |
...gin/src/main/java/com/solace/maas/ep/event/management/agent/plugin/config/VMRProperties.java
Outdated
Show resolved
Hide resolved
...gin/src/main/java/com/solace/maas/ep/event/management/agent/plugin/config/VMRProperties.java
Outdated
Show resolved
Hide resolved
moodiRealist
left a comment
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.
PR looks good, but we shouldn't merge it until changes to applicatoin.yml file are reverted.
| properties.setProperty(SolaceProperties.AuthenticationProperties.SCHEME_BASIC_PASSWORD, password); | ||
| properties.setProperty(SolaceProperties.ClientProperties.NAME, clientName); | ||
|
|
||
| //We will always use the default jks truststore for connecting to the EVMR |
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.
I agree with this comment 👍
| @Mock | ||
| private ScanStatusPublisher scanStatusPublisher; | ||
|
|
||
| @Mock |
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.
Can we use @MockitoBean in here as well?
|




What is the purpose of this change?
implement asks of https://sol-jira.atlassian.net/browse/DATAGO-116001
How was this change implemented?
Java
How was this change tested?
Is there anything the reviewers should focus on/be aware of?