Skip to content

Commit 3176053

Browse files
SylvainJugeSylvainJuge
committed
post-review refactor
1 parent 996b573 commit 3176053

File tree

4 files changed

+138
-103
lines changed

4 files changed

+138
-103
lines changed

‎jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectionTest.java‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@
55

66
package io.opentelemetry.contrib.jmxscraper;
77

8-
import static io.opentelemetry.contrib.jmxscraper.TestKeyStoreUtil.addTrustedCertificate;
9-
import static io.opentelemetry.contrib.jmxscraper.TestKeyStoreUtil.createKeyStore;
108
import static org.assertj.core.api.Assertions.assertThat;
119

1210
import java.nio.file.Path;
@@ -94,24 +92,24 @@ private static void testServerSsl(Path tempDir, boolean sslRmiRegistry) {
9492
// server keystore with public/private key pair
9593
// client trust store with certificate from server
9694

97-
Path serverKeystore = tempDir.resolve("server.jks");
98-
Path clientTrustStore = tempDir.resolve("client.jks");
95+
TestKeyStore serverKeystore =
96+
TestKeyStore.newKeyStore(tempDir.resolve("server.jks"), SERVER_PASSWORD);
97+
TestKeyStore clientTrustStore =
98+
TestKeyStore.newKeyStore(tempDir.resolve("client.jks"), CLIENT_PASSWORD);
9999

100-
X509Certificate serverCertificate = createKeyStore(serverKeystore, SERVER_PASSWORD);
101-
102-
createKeyStore(clientTrustStore, CLIENT_PASSWORD);
103-
addTrustedCertificate(clientTrustStore, CLIENT_PASSWORD, serverCertificate);
100+
X509Certificate serverCertificate = serverKeystore.addKeyPair();
101+
clientTrustStore.addTrustedCertificate(serverCertificate);
104102

105103
connectionTest(
106104
app ->
107105
(sslRmiRegistry ? app.withSslRmiRegistry(4242) : app)
108106
.withJmxPort(JMX_PORT)
109107
.withJmxSsl()
110-
.withKeyStore(serverKeystore, SERVER_PASSWORD),
108+
.withKeyStore(serverKeystore),
111109
scraper ->
112110
(sslRmiRegistry ? scraper.withSslRmiRegistry() : scraper)
113111
.withRmiServiceUrl(APP_HOST, JMX_PORT)
114-
.withTrustStore(clientTrustStore, CLIENT_PASSWORD));
112+
.withTrustStore(clientTrustStore));
115113
}
116114

117115
@Test
@@ -125,34 +123,36 @@ void serverSslClientSsl(@TempDir Path tempDir) {
125123
// client key store with public/private key pair
126124
// client trust store with certificate from server
127125

128-
Path serverKeystore = tempDir.resolve("server-keystore.jks");
129-
Path serverTrustStore = tempDir.resolve("server-truststore.jks");
126+
TestKeyStore serverKeystore =
127+
TestKeyStore.newKeyStore(tempDir.resolve("server-keystore.jks"), SERVER_PASSWORD);
128+
TestKeyStore serverTrustStore =
129+
TestKeyStore.newKeyStore(tempDir.resolve("server-truststore.jks"), SERVER_PASSWORD);
130130

131-
X509Certificate serverCertificate = createKeyStore(serverKeystore, SERVER_PASSWORD);
132-
createKeyStore(serverTrustStore, SERVER_PASSWORD);
131+
X509Certificate serverCertificate = serverKeystore.addKeyPair();
133132

134-
Path clientKeystore = tempDir.resolve("client-keystore.jks");
135-
Path clientTrustStore = tempDir.resolve("client-truststore.jks");
133+
TestKeyStore clientKeystore =
134+
TestKeyStore.newKeyStore(tempDir.resolve("client-keystore.jks"), CLIENT_PASSWORD);
135+
TestKeyStore clientTrustStore =
136+
TestKeyStore.newKeyStore(tempDir.resolve("client-truststore.jks"), CLIENT_PASSWORD);
136137

137-
X509Certificate clientCertificate = createKeyStore(clientKeystore, CLIENT_PASSWORD);
138-
createKeyStore(clientTrustStore, CLIENT_PASSWORD);
138+
X509Certificate clientCertificate = clientKeystore.addKeyPair();
139139

140140
// adding certificates in trust stores
141-
addTrustedCertificate(serverTrustStore, SERVER_PASSWORD, clientCertificate);
142-
addTrustedCertificate(clientTrustStore, CLIENT_PASSWORD, serverCertificate);
141+
clientTrustStore.addTrustedCertificate(serverCertificate);
142+
serverTrustStore.addTrustedCertificate(clientCertificate);
143143

144144
connectionTest(
145145
app ->
146146
app.withJmxPort(JMX_PORT)
147147
.withJmxSsl()
148148
.withClientSslCertificate()
149-
.withKeyStore(serverKeystore, SERVER_PASSWORD)
150-
.withTrustStore(serverTrustStore, SERVER_PASSWORD),
149+
.withKeyStore(serverKeystore)
150+
.withTrustStore(serverTrustStore),
151151
scraper ->
152152
scraper
153153
.withRmiServiceUrl(APP_HOST, JMX_PORT)
154-
.withKeyStore(clientKeystore, CLIENT_PASSWORD)
155-
.withTrustStore(clientTrustStore, CLIENT_PASSWORD));
154+
.withKeyStore(clientKeystore)
155+
.withTrustStore(clientTrustStore));
156156
}
157157

158158
private static void connectionTest(

‎jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxScraperContainer.java‎

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,8 @@ public class JmxScraperContainer extends GenericContainer<JmxScraperContainer> {
3232
private String password;
3333
private final List<String> extraJars;
3434
private boolean testJmx;
35-
private Path keyStore;
36-
private String keyStorePassword;
37-
private Path trustStore;
38-
private String trustStorePassword;
35+
private TestKeyStore keyStore;
36+
private TestKeyStore trustStore;
3937
private boolean sslRmiRegistry;
4038

4139
public JmxScraperContainer(String otlpEndpoint, String baseImage) {
@@ -146,28 +144,24 @@ public JmxScraperContainer withTestJmx() {
146144
/**
147145
* Configure key store for the scraper JVM
148146
*
149-
* @param keyStore path to key store
150-
* @param password key store password
147+
* @param keyStore key store
151148
* @return this
152149
*/
153150
@CanIgnoreReturnValue
154-
public JmxScraperContainer withKeyStore(Path keyStore, String password) {
151+
public JmxScraperContainer withKeyStore(TestKeyStore keyStore) {
155152
this.keyStore = keyStore;
156-
this.keyStorePassword = password;
157153
return this;
158154
}
159155

160156
/**
161157
* Configure trust store for the scraper JVM
162158
*
163-
* @param trustStore path to trust store
164-
* @param password trust store password
159+
* @param trustStore trust store
165160
* @return this
166161
*/
167162
@CanIgnoreReturnValue
168-
public JmxScraperContainer withTrustStore(Path trustStore, String password) {
163+
public JmxScraperContainer withTrustStore(TestKeyStore trustStore) {
169164
this.trustStore = trustStore;
170-
this.trustStorePassword = password;
171165
return this;
172166
}
173167

@@ -208,8 +202,8 @@ public void start() {
208202
arguments.add("-Dotel.jmx.password=" + password);
209203
}
210204

211-
arguments.addAll(addKeyStore(keyStore, keyStorePassword, /* keyStore= */ true));
212-
arguments.addAll(addKeyStore(trustStore, trustStorePassword, /* keyStore= */ false));
205+
arguments.addAll(addSecureStore(keyStore, /* isKeyStore= */ true));
206+
arguments.addAll(addSecureStore(trustStore, /* isKeyStore= */ false));
213207

214208
if (sslRmiRegistry) {
215209
arguments.add("-Dotel.jmx.remote.registry.ssl=true");
@@ -249,14 +243,16 @@ public void start() {
249243
super.start();
250244
}
251245

252-
private List<String> addKeyStore(Path path, String password, boolean keyStore) {
253-
if (path == null) {
246+
private List<String> addSecureStore(TestKeyStore keyStore, boolean isKeyStore) {
247+
if (keyStore == null) {
254248
return Collections.emptyList();
255249
}
250+
Path path = keyStore.getPath();
256251
String containerPath = "/" + path.getFileName().toString();
257252
this.withCopyFileToContainer(MountableFile.forHostPath(path), containerPath);
258253

259-
String prefix = String.format("-Djavax.net.ssl.%sStore", keyStore ? "key" : "trust");
260-
return Arrays.asList(prefix + "=" + containerPath, prefix + "Password=" + password);
254+
String prefix = String.format("-Djavax.net.ssl.%sStore", isKeyStore ? "key" : "trust");
255+
return Arrays.asList(
256+
prefix + "=" + containerPath, prefix + "Password=" + keyStore.getPassword());
261257
}
262258
}

‎jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java‎

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,8 @@ public class TestAppContainer extends GenericContainer<TestAppContainer> {
2929
private String pwd;
3030
private boolean jmxSsl;
3131
private boolean jmxSslRegistry;
32-
private Path keyStore;
33-
private String keyStorePassword;
34-
private Path trustStore;
35-
private String trustStorePassword;
32+
private TestKeyStore keyStore;
33+
private TestKeyStore trustStore;
3634
private int jmxPort;
3735
private int jmxRmiPort;
3836
private boolean clientCertificate;
@@ -115,28 +113,24 @@ public TestAppContainer withClientSslCertificate() {
115113
/**
116114
* Configure key store for the remote JVM
117115
*
118-
* @param keyStore path to key store
119-
* @param password key store password
116+
* @param keyStore key store
120117
* @return this
121118
*/
122119
@CanIgnoreReturnValue
123-
public TestAppContainer withKeyStore(Path keyStore, String password) {
120+
public TestAppContainer withKeyStore(TestKeyStore keyStore) {
124121
this.keyStore = keyStore;
125-
this.keyStorePassword = password;
126122
return this;
127123
}
128124

129125
/**
130126
* Configure trust store for the remote JVM
131127
*
132-
* @param trustStore path to trust store
133-
* @param password trust store password
128+
* @param trustStore trust store
134129
* @return this
135130
*/
136131
@CanIgnoreReturnValue
137-
public TestAppContainer withTrustStore(Path trustStore, String password) {
132+
public TestAppContainer withTrustStore(TestKeyStore trustStore) {
138133
this.trustStore = trustStore;
139-
this.trustStorePassword = password;
140134
return this;
141135
}
142136

@@ -176,8 +170,8 @@ public void start() {
176170
}
177171

178172
// add optional key and trust stores
179-
addKeyStore(keyStore, keyStorePassword, /* keyStore= */ true, properties);
180-
addKeyStore(trustStore, trustStorePassword, /* keyStore= */ false, properties);
173+
addSecureStore(keyStore, /* isKeyStore= */ true, properties);
174+
addSecureStore(trustStore, /* isKeyStore= */ false, properties);
181175

182176
String confArgs =
183177
properties.entrySet().stream()
@@ -198,17 +192,18 @@ public void start() {
198192
super.start();
199193
}
200194

201-
private void addKeyStore(
202-
Path path, String password, boolean keyStore, Map<String, String> properties) {
203-
if (path == null) {
195+
private void addSecureStore(
196+
TestKeyStore keyStore, boolean isKeyStore, Map<String, String> properties) {
197+
if (keyStore == null) {
204198
return;
205199
}
200+
Path path = keyStore.getPath();
206201
String containerPath = "/" + path.getFileName().toString();
207202
this.withCopyFileToContainer(MountableFile.forHostPath(path), containerPath);
208203

209-
String prefix = String.format("javax.net.ssl.%sStore", keyStore ? "key" : "trust");
204+
String prefix = String.format("javax.net.ssl.%sStore", isKeyStore ? "key" : "trust");
210205
properties.put(prefix, containerPath);
211-
properties.put(prefix + "Password", password);
206+
properties.put(prefix + "Password", keyStore.getPassword());
212207
}
213208

214209
private static Path createPwdFile(String login, String pwd) {

0 commit comments

Comments
 (0)