jmx-scraper add ssl support #1710
Conversation
SylvainJuge
force-pushed
the
jmx-scraper-server-ssl
branch
from
129ed8b to
057ba99
Compare
| logger.info("Connecting with SSL protected RMI registry to " + url); | ||
| String hostName; | ||
| int port; | ||
|
|
||
| if (url.getURLPath().startsWith("/jndi/")) { | ||
| String[] components = url.getURLPath().split("/", 3); | ||
| URI uri = URI.create(components[2]); | ||
| hostName = uri.getHost(); | ||
| port = uri.getPort(); | ||
| } else { | ||
| hostName = url.getHost(); | ||
| port = url.getPort(); | ||
| } | ||
|
|
||
| try { | ||
| JMXConnector jmxConn = new RMIConnector(getStub(hostName, port), null); | ||
| jmxConn.connect(env); | ||
| return jmxConn; | ||
| } catch (IOException e) { | ||
| throw new IllegalStateException("Unable to connect to " + url, e); | ||
| } |
There was a problem hiding this comment.
[for reviewer] copied almost as-is from jmx gatherer
jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectionTest.java
Outdated
Show resolved
Hide resolved
jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectionTest.java
Outdated
Show resolved
Hide resolved
jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java
Outdated
Show resolved
Hide resolved
| @@ -99,6 +198,19 @@ public void start() { | |||
| super.start(); | |||
| } | |||
|
|
|||
| private void addKeyStore( | |||
| Path path, String password, boolean keyStore, Map<String, String> properties) { | |||
There was a problem hiding this comment.
boolean keyStore could be StoreTypeEnum storeType, where StoreTypeEnum would be KEY_STORE, TRUST_STORE
There was a problem hiding this comment.
Having any kind of strong typing is not really necessary here IMHO as there are only a few use-cases so I think a boolean is fine. We could also have introduced dedicated interfaces for trust and key stores but in practice it's the same file and only the usage we make of it makes it either a trust or a key store, we could even use a single key store for both trusted certificates and storing key pairs.
jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestKeyStoreUtil.java
Outdated
Show resolved
Hide resolved
| @@ -94,24 +92,24 @@ private static void testServerSsl(Path tempDir, boolean sslRmiRegistry) { | |||
| // server keystore with public/private key pair | |||
| // client trust store with certificate from server | |||
|
|
|||
| Path serverKeystore = tempDir.resolve("server.jks"); | |||
| Path clientTrustStore = tempDir.resolve("client.jks"); | |||
| TestKeyStore serverKeystore = | |||
There was a problem hiding this comment.
Something for next PR maybe - to keep consistent name pattern it should be named serverKeyStore
This is based on #1684 which is not merged yet, I'll rebase to minimize the diff once it's merged.Part of #1681
This adds the following features with proper integration tests for JMX with SSL:
Using other advanced JAAS configuration options for authentication is not part of this PR and will be implemented in a separate PR.