Skip to content

Commit

Permalink
PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
jack-berg committed Apr 11, 2023
1 parent 7430348 commit 30877e4
Show file tree
Hide file tree
Showing 13 changed files with 45 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@
/**
* Utility class to help with management of TLS related components. TLS config consists {@link
* #keyManager}, {@link #trustManager}, which combine to form {@link #sslContext}. These components
* can be configured via higher level APIs ({@link #createTrustManager(byte[])} and {@link
* #createKeyManager(byte[], byte[])}) which parse keys in PEM format, or the lower level API {@link
* #setSslContext(SSLContext, X509TrustManager)} in which the components are directly set, but NOT
* both. Attempts to reconfigure components which have already been configured throw {@link
* can be configured via higher level APIs ({@link #setTrustManagerFromCerts(byte[])} and {@link
* #setKeyManagerFromCerts(byte[], byte[])}) which parse keys in PEM format, or the lower level API
* {@link #setSslContext(SSLContext, X509TrustManager)} in which the components are directly set,
* but NOT both. Attempts to reconfigure components which have already been configured throw {@link
* IllegalStateException}. Consumers access components via any combination of {@link
* #getKeyManager()}, {@link #getTrustManager()}, and {@link #getSslContext()}.
*
Expand All @@ -44,7 +44,7 @@ public TlsConfigHelper() {}
*
* @param trustedCertsPem Certificate in PEM format.
*/
public void createTrustManager(byte[] trustedCertsPem) {
public void setTrustManagerFromCerts(byte[] trustedCertsPem) {
if (trustManager != null) {
throw new IllegalStateException("trustManager has been previously configured");
}
Expand All @@ -68,7 +68,7 @@ public void createTrustManager(byte[] trustedCertsPem) {
* @param privateKeyPem Private key content in PEM format.
* @param certificatePem Certificate content in PEM format.
*/
public void createKeyManager(byte[] privateKeyPem, byte[] certificatePem) {
public void setKeyManagerFromCerts(byte[] privateKeyPem, byte[] certificatePem) {
if (keyManager != null) {
throw new IllegalStateException("keyManager has been previously configured");
}
Expand All @@ -85,8 +85,8 @@ public void createKeyManager(byte[] privateKeyPem, byte[] certificatePem) {
/**
* Configure the {@link SSLContext} and {@link X509TrustManager}.
*
* <p>Must not be called multiple times, or if {@link #createTrustManager(byte[])} or {@link
* #createKeyManager(byte[], byte[])} has been previously called.
* <p>Must not be called multiple times, or if {@link #setTrustManagerFromCerts(byte[])} or {@link
* #setKeyManagerFromCerts(byte[], byte[])} has been previously called.
*
* @param sslContext the SSL context.
* @param trustManager the trust manager.
Expand Down Expand Up @@ -119,8 +119,7 @@ public SSLContext getSslContext() {
}

try {
SSLContext sslContext;
sslContext = SSLContext.getInstance("TLS");
SSLContext sslContext = SSLContext.getInstance("TLS");
sslContext.init(
keyManager == null ? null : new KeyManager[] {keyManager},
trustManager == null ? null : new TrustManager[] {trustManager},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,13 +100,14 @@ public GrpcExporterBuilder<T> setCompression(String compressionMethod) {
return this;
}

public GrpcExporterBuilder<T> configureTrustManager(byte[] trustedCertificatesPem) {
tlsConfigHelper.createTrustManager(trustedCertificatesPem);
public GrpcExporterBuilder<T> setTrustManagerFromCerts(byte[] trustedCertificatesPem) {
tlsConfigHelper.setTrustManagerFromCerts(trustedCertificatesPem);
return this;
}

public GrpcExporterBuilder<T> configureKeyManager(byte[] privateKeyPem, byte[] certificatePem) {
tlsConfigHelper.createKeyManager(privateKeyPem, certificatePem);
public GrpcExporterBuilder<T> setKeyManagerFromCerts(
byte[] privateKeyPem, byte[] certificatePem) {
tlsConfigHelper.setKeyManagerFromCerts(privateKeyPem, certificatePem);
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,14 @@ public OkHttpExporterBuilder<T> setAuthenticator(Authenticator authenticator) {
return this;
}

public OkHttpExporterBuilder<T> configureTrustManager(byte[] trustedCertificatesPem) {
tlsConfigHelper.createTrustManager(trustedCertificatesPem);
public OkHttpExporterBuilder<T> setTrustManagerFromCerts(byte[] trustedCertificatesPem) {
tlsConfigHelper.setTrustManagerFromCerts(trustedCertificatesPem);
return this;
}

public OkHttpExporterBuilder<T> configureKeyManager(byte[] privateKeyPem, byte[] certificatePem) {
tlsConfigHelper.createKeyManager(privateKeyPem, certificatePem);
public OkHttpExporterBuilder<T> setKeyManagerFromCerts(
byte[] privateKeyPem, byte[] certificatePem) {
tlsConfigHelper.setKeyManagerFromCerts(privateKeyPem, certificatePem);
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,41 +28,41 @@ class TlsConfigHelperTest {

@Test
void createTrustManager() throws CertificateEncodingException {
helper.createTrustManager(serverTls.certificate().getEncoded());
helper.setTrustManagerFromCerts(serverTls.certificate().getEncoded());

assertThat(helper.getTrustManager()).isNotNull();
}

@Test
void createTrustManager_AlreadyExists_Throws() throws Exception {
helper.createTrustManager(serverTls.certificate().getEncoded());
assertThatThrownBy(() -> helper.createTrustManager(serverTls.certificate().getEncoded()))
helper.setTrustManagerFromCerts(serverTls.certificate().getEncoded());
assertThatThrownBy(() -> helper.setTrustManagerFromCerts(serverTls.certificate().getEncoded()))
.isInstanceOf(IllegalStateException.class)
.hasMessageContaining("trustManager has been previously configured");

helper = new TlsConfigHelper();
helper.setSslContext(
SSLContext.getInstance("TLS"), TlsUtil.trustManager(serverTls.certificate().getEncoded()));
assertThatThrownBy(() -> helper.createTrustManager(serverTls.certificate().getEncoded()))
assertThatThrownBy(() -> helper.setTrustManagerFromCerts(serverTls.certificate().getEncoded()))
.isInstanceOf(IllegalStateException.class)
.hasMessageContaining("trustManager has been previously configured");
}

@Test
void createKeyManager() throws CertificateEncodingException {
helper.createKeyManager(
helper.setKeyManagerFromCerts(
serverTls.privateKey().getEncoded(), serverTls.certificate().getEncoded());

assertThat(helper.getKeyManager()).isNotNull();
}

@Test
void createKeyManager_AlreadyExists_Throws() throws Exception {
helper.createKeyManager(
helper.setKeyManagerFromCerts(
serverTls.privateKey().getEncoded(), serverTls.certificate().getEncoded());
assertThatThrownBy(
() ->
helper.createKeyManager(
helper.setKeyManagerFromCerts(
serverTls.privateKey().getEncoded(), serverTls.certificate().getEncoded()))
.isInstanceOf(IllegalStateException.class)
.hasMessageContaining("keyManager has been previously configured");
Expand All @@ -89,7 +89,7 @@ void setSslContext_AlreadyExists_Throws() throws Exception {
.hasMessageContaining("sslContext or trustManager has been previously configured");

helper = new TlsConfigHelper();
helper.createTrustManager(serverTls.certificate().getEncoded());
helper.setTrustManagerFromCerts(serverTls.certificate().getEncoded());
assertThatThrownBy(() -> helper.setSslContext(sslContext, trustManager))
.isInstanceOf(IllegalStateException.class)
.hasMessageContaining("sslContext or trustManager has been previously configured");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,13 +109,13 @@ public JaegerGrpcSpanExporterBuilder setTimeout(Duration timeout) {
* use the system default trusted certificates.
*/
public JaegerGrpcSpanExporterBuilder setTrustedCertificates(byte[] trustedCertificatesPem) {
delegate.configureTrustManager(trustedCertificatesPem);
delegate.setTrustManagerFromCerts(trustedCertificatesPem);
return this;
}

/** Sets the client key and chain to use for verifying servers when mTLS is enabled. */
public JaegerGrpcSpanExporterBuilder setClientTls(byte[] privateKeyPem, byte[] certificatePem) {
delegate.configureKeyManager(privateKeyPem, certificatePem);
delegate.setKeyManagerFromCerts(privateKeyPem, certificatePem);
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public OtlpHttpMetricExporterBuilder addHeader(String key, String value) {
* use the system default trusted certificates.
*/
public OtlpHttpMetricExporterBuilder setTrustedCertificates(byte[] trustedCertificatesPem) {
delegate.configureTrustManager(trustedCertificatesPem);
delegate.setTrustManagerFromCerts(trustedCertificatesPem);
return this;
}

Expand All @@ -108,7 +108,7 @@ public OtlpHttpMetricExporterBuilder setTrustedCertificates(byte[] trustedCertif
* The key must be PKCS8, and both must be in PEM format.
*/
public OtlpHttpMetricExporterBuilder setClientTls(byte[] privateKeyPem, byte[] certificatePem) {
delegate.configureKeyManager(privateKeyPem, certificatePem);
delegate.setKeyManagerFromCerts(privateKeyPem, certificatePem);
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public OtlpHttpSpanExporterBuilder addHeader(String key, String value) {
* use the system default trusted certificates.
*/
public OtlpHttpSpanExporterBuilder setTrustedCertificates(byte[] trustedCertificatesPem) {
delegate.configureTrustManager(trustedCertificatesPem);
delegate.setTrustManagerFromCerts(trustedCertificatesPem);
return this;
}

Expand All @@ -96,7 +96,7 @@ public OtlpHttpSpanExporterBuilder setTrustedCertificates(byte[] trustedCertific
* The key must be PKCS8, and both must be in PEM format.
*/
public OtlpHttpSpanExporterBuilder setClientTls(byte[] privateKeyPem, byte[] certificatePem) {
delegate.configureKeyManager(privateKeyPem, certificatePem);
delegate.setKeyManagerFromCerts(privateKeyPem, certificatePem);
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ public OtlpGrpcMetricExporterBuilder setCompression(String compressionMethod) {
* use the system default trusted certificates.
*/
public OtlpGrpcMetricExporterBuilder setTrustedCertificates(byte[] trustedCertificatesPem) {
delegate.configureTrustManager(trustedCertificatesPem);
delegate.setTrustManagerFromCerts(trustedCertificatesPem);
return this;
}

Expand All @@ -140,7 +140,7 @@ public OtlpGrpcMetricExporterBuilder setTrustedCertificates(byte[] trustedCertif
* The key must be PKCS8, and both must be in PEM format.
*/
public OtlpGrpcMetricExporterBuilder setClientTls(byte[] privateKeyPem, byte[] certificatePem) {
delegate.configureKeyManager(privateKeyPem, certificatePem);
delegate.setKeyManagerFromCerts(privateKeyPem, certificatePem);
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public OtlpGrpcSpanExporterBuilder setCompression(String compressionMethod) {
* use the system default trusted certificates.
*/
public OtlpGrpcSpanExporterBuilder setTrustedCertificates(byte[] trustedCertificatesPem) {
delegate.configureTrustManager(trustedCertificatesPem);
delegate.setTrustManagerFromCerts(trustedCertificatesPem);
return this;
}

Expand All @@ -124,7 +124,7 @@ public OtlpGrpcSpanExporterBuilder setTrustedCertificates(byte[] trustedCertific
* The key must be PKCS8, and both must be in PEM format.
*/
public OtlpGrpcSpanExporterBuilder setClientTls(byte[] privateKeyPem, byte[] certificatePem) {
delegate.configureKeyManager(privateKeyPem, certificatePem);
delegate.setKeyManagerFromCerts(privateKeyPem, certificatePem);
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public OtlpHttpLogRecordExporterBuilder addHeader(String key, String value) {
* use the system default trusted certificates.
*/
public OtlpHttpLogRecordExporterBuilder setTrustedCertificates(byte[] trustedCertificatesPem) {
delegate.configureTrustManager(trustedCertificatesPem);
delegate.setTrustManagerFromCerts(trustedCertificatesPem);
return this;
}

Expand All @@ -93,7 +93,7 @@ public OtlpHttpLogRecordExporterBuilder setTrustedCertificates(byte[] trustedCer
*/
public OtlpHttpLogRecordExporterBuilder setClientTls(
byte[] privateKeyPem, byte[] certificatePem) {
delegate.configureKeyManager(privateKeyPem, certificatePem);
delegate.setKeyManagerFromCerts(privateKeyPem, certificatePem);
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public OtlpGrpcLogRecordExporterBuilder setCompression(String compressionMethod)
* use the system default trusted certificates.
*/
public OtlpGrpcLogRecordExporterBuilder setTrustedCertificates(byte[] trustedCertificatesPem) {
delegate.configureTrustManager(trustedCertificatesPem);
delegate.setTrustManagerFromCerts(trustedCertificatesPem);
return this;
}

Expand All @@ -125,7 +125,7 @@ public OtlpGrpcLogRecordExporterBuilder setTrustedCertificates(byte[] trustedCer
*/
public OtlpGrpcLogRecordExporterBuilder setClientTls(
byte[] privateKeyPem, byte[] certificatePem) {
delegate.configureKeyManager(privateKeyPem, certificatePem);
delegate.setKeyManagerFromCerts(privateKeyPem, certificatePem);
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,13 +93,13 @@ public TelemetryExporterBuilder<T> addHeader(String key, String value) {

@Override
public TelemetryExporterBuilder<T> setTrustedCertificates(byte[] certificates) {
tlsConfigHelper.createTrustManager(certificates);
tlsConfigHelper.setTrustManagerFromCerts(certificates);
return this;
}

@Override
public TelemetryExporterBuilder<T> setClientTls(byte[] privateKeyPem, byte[] certificatePem) {
tlsConfigHelper.createKeyManager(privateKeyPem, certificatePem);
tlsConfigHelper.setKeyManagerFromCerts(privateKeyPem, certificatePem);
return this;
}

Expand Down Expand Up @@ -167,7 +167,6 @@ public CompletableResultCode shutdown() {
private static void setSslContext(
ManagedChannelBuilder<?> managedChannelBuilder, TlsConfigHelper tlsConfigHelper)
throws SSLException {
requireNonNull(managedChannelBuilder, "managedChannelBuilder");
X509TrustManager trustManager = tlsConfigHelper.getTrustManager();
if (trustManager == null) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public JaegerRemoteSamplerBuilder setEndpoint(String endpoint) {
/** Sets trusted certificate. */
public JaegerRemoteSamplerBuilder setTrustedCertificates(byte[] trustedCertificatesPem) {
requireNonNull(trustedCertificatesPem, "trustedCertificatesPem");
tlsConfigHelper.createTrustManager(trustedCertificatesPem);
tlsConfigHelper.setTrustManagerFromCerts(trustedCertificatesPem);
return this;
}

Expand All @@ -88,7 +88,7 @@ public JaegerRemoteSamplerBuilder setTrustedCertificates(byte[] trustedCertifica
public JaegerRemoteSamplerBuilder setClientTls(byte[] privateKeyPem, byte[] certificatePem) {
requireNonNull(privateKeyPem, "privateKeyPem");
requireNonNull(certificatePem, "certificatePem");
tlsConfigHelper.createKeyManager(privateKeyPem, certificatePem);
tlsConfigHelper.setKeyManagerFromCerts(privateKeyPem, certificatePem);
return this;
}

Expand Down

0 comments on commit 30877e4

Please sign in to comment.