From 30877e4fc91f4eb06290cd119f86492ee04b759f Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Tue, 11 Apr 2023 14:06:19 -0500 Subject: [PATCH] PR feedback --- .../exporter/internal/TlsConfigHelper.java | 19 +++++++++---------- .../internal/grpc/GrpcExporterBuilder.java | 9 +++++---- .../okhttp/OkHttpExporterBuilder.java | 9 +++++---- .../internal/TlsConfigHelperTest.java | 16 ++++++++-------- .../jaeger/JaegerGrpcSpanExporterBuilder.java | 4 ++-- .../OtlpHttpMetricExporterBuilder.java | 4 ++-- .../trace/OtlpHttpSpanExporterBuilder.java | 4 ++-- .../OtlpGrpcMetricExporterBuilder.java | 4 ++-- .../trace/OtlpGrpcSpanExporterBuilder.java | 4 ++-- .../OtlpHttpLogRecordExporterBuilder.java | 4 ++-- .../OtlpGrpcLogRecordExporterBuilder.java | 4 ++-- ...anagedChannelTelemetryExporterBuilder.java | 5 ++--- .../sampler/JaegerRemoteSamplerBuilder.java | 4 ++-- 13 files changed, 45 insertions(+), 45 deletions(-) diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java index a897f46f688..407dba920ca 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java @@ -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()}. * @@ -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"); } @@ -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"); } @@ -85,8 +85,8 @@ public void createKeyManager(byte[] privateKeyPem, byte[] certificatePem) { /** * Configure the {@link SSLContext} and {@link X509TrustManager}. * - *

Must not be called multiple times, or if {@link #createTrustManager(byte[])} or {@link - * #createKeyManager(byte[], byte[])} has been previously called. + *

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. @@ -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}, diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/GrpcExporterBuilder.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/GrpcExporterBuilder.java index 99fae8b839d..7cdc67832ad 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/GrpcExporterBuilder.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/GrpcExporterBuilder.java @@ -100,13 +100,14 @@ public GrpcExporterBuilder setCompression(String compressionMethod) { return this; } - public GrpcExporterBuilder configureTrustManager(byte[] trustedCertificatesPem) { - tlsConfigHelper.createTrustManager(trustedCertificatesPem); + public GrpcExporterBuilder setTrustManagerFromCerts(byte[] trustedCertificatesPem) { + tlsConfigHelper.setTrustManagerFromCerts(trustedCertificatesPem); return this; } - public GrpcExporterBuilder configureKeyManager(byte[] privateKeyPem, byte[] certificatePem) { - tlsConfigHelper.createKeyManager(privateKeyPem, certificatePem); + public GrpcExporterBuilder setKeyManagerFromCerts( + byte[] privateKeyPem, byte[] certificatePem) { + tlsConfigHelper.setKeyManagerFromCerts(privateKeyPem, certificatePem); return this; } diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/okhttp/OkHttpExporterBuilder.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/okhttp/OkHttpExporterBuilder.java index ab4582722b1..aad41b5c6a3 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/okhttp/OkHttpExporterBuilder.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/okhttp/OkHttpExporterBuilder.java @@ -89,13 +89,14 @@ public OkHttpExporterBuilder setAuthenticator(Authenticator authenticator) { return this; } - public OkHttpExporterBuilder configureTrustManager(byte[] trustedCertificatesPem) { - tlsConfigHelper.createTrustManager(trustedCertificatesPem); + public OkHttpExporterBuilder setTrustManagerFromCerts(byte[] trustedCertificatesPem) { + tlsConfigHelper.setTrustManagerFromCerts(trustedCertificatesPem); return this; } - public OkHttpExporterBuilder configureKeyManager(byte[] privateKeyPem, byte[] certificatePem) { - tlsConfigHelper.createKeyManager(privateKeyPem, certificatePem); + public OkHttpExporterBuilder setKeyManagerFromCerts( + byte[] privateKeyPem, byte[] certificatePem) { + tlsConfigHelper.setKeyManagerFromCerts(privateKeyPem, certificatePem); return this; } diff --git a/exporters/common/src/test/java/io/opentelemetry/exporter/internal/TlsConfigHelperTest.java b/exporters/common/src/test/java/io/opentelemetry/exporter/internal/TlsConfigHelperTest.java index 31af6f6c1c4..83736d5eb06 100644 --- a/exporters/common/src/test/java/io/opentelemetry/exporter/internal/TlsConfigHelperTest.java +++ b/exporters/common/src/test/java/io/opentelemetry/exporter/internal/TlsConfigHelperTest.java @@ -28,29 +28,29 @@ 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(); @@ -58,11 +58,11 @@ void createKeyManager() throws CertificateEncodingException { @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"); @@ -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"); diff --git a/exporters/jaeger/src/main/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterBuilder.java b/exporters/jaeger/src/main/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterBuilder.java index 8c8d9671ea0..e70f9061a7c 100644 --- a/exporters/jaeger/src/main/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterBuilder.java +++ b/exporters/jaeger/src/main/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterBuilder.java @@ -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; } diff --git a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/metrics/OtlpHttpMetricExporterBuilder.java b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/metrics/OtlpHttpMetricExporterBuilder.java index 1ab0f5c8e05..b5c6b47e9ad 100644 --- a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/metrics/OtlpHttpMetricExporterBuilder.java +++ b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/metrics/OtlpHttpMetricExporterBuilder.java @@ -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; } @@ -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; } diff --git a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/trace/OtlpHttpSpanExporterBuilder.java b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/trace/OtlpHttpSpanExporterBuilder.java index 3d8ddbc68df..d49de644da4 100644 --- a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/trace/OtlpHttpSpanExporterBuilder.java +++ b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/trace/OtlpHttpSpanExporterBuilder.java @@ -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; } @@ -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; } diff --git a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/metrics/OtlpGrpcMetricExporterBuilder.java b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/metrics/OtlpGrpcMetricExporterBuilder.java index 5197c17887e..db1a192d255 100644 --- a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/metrics/OtlpGrpcMetricExporterBuilder.java +++ b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/metrics/OtlpGrpcMetricExporterBuilder.java @@ -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; } @@ -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; } diff --git a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/trace/OtlpGrpcSpanExporterBuilder.java b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/trace/OtlpGrpcSpanExporterBuilder.java index 8ead941f91d..5c1f287005f 100644 --- a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/trace/OtlpGrpcSpanExporterBuilder.java +++ b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/trace/OtlpGrpcSpanExporterBuilder.java @@ -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; } @@ -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; } diff --git a/exporters/otlp/logs/src/main/java/io/opentelemetry/exporter/otlp/http/logs/OtlpHttpLogRecordExporterBuilder.java b/exporters/otlp/logs/src/main/java/io/opentelemetry/exporter/otlp/http/logs/OtlpHttpLogRecordExporterBuilder.java index 657b2dd819b..44f0e2c353e 100644 --- a/exporters/otlp/logs/src/main/java/io/opentelemetry/exporter/otlp/http/logs/OtlpHttpLogRecordExporterBuilder.java +++ b/exporters/otlp/logs/src/main/java/io/opentelemetry/exporter/otlp/http/logs/OtlpHttpLogRecordExporterBuilder.java @@ -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; } @@ -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; } diff --git a/exporters/otlp/logs/src/main/java/io/opentelemetry/exporter/otlp/logs/OtlpGrpcLogRecordExporterBuilder.java b/exporters/otlp/logs/src/main/java/io/opentelemetry/exporter/otlp/logs/OtlpGrpcLogRecordExporterBuilder.java index f6ec0e0b908..9e54fed5bcd 100644 --- a/exporters/otlp/logs/src/main/java/io/opentelemetry/exporter/otlp/logs/OtlpGrpcLogRecordExporterBuilder.java +++ b/exporters/otlp/logs/src/main/java/io/opentelemetry/exporter/otlp/logs/OtlpGrpcLogRecordExporterBuilder.java @@ -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; } @@ -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; } diff --git a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/ManagedChannelTelemetryExporterBuilder.java b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/ManagedChannelTelemetryExporterBuilder.java index bedaa381141..1f3ebafe068 100644 --- a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/ManagedChannelTelemetryExporterBuilder.java +++ b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/ManagedChannelTelemetryExporterBuilder.java @@ -93,13 +93,13 @@ public TelemetryExporterBuilder addHeader(String key, String value) { @Override public TelemetryExporterBuilder setTrustedCertificates(byte[] certificates) { - tlsConfigHelper.createTrustManager(certificates); + tlsConfigHelper.setTrustManagerFromCerts(certificates); return this; } @Override public TelemetryExporterBuilder setClientTls(byte[] privateKeyPem, byte[] certificatePem) { - tlsConfigHelper.createKeyManager(privateKeyPem, certificatePem); + tlsConfigHelper.setKeyManagerFromCerts(privateKeyPem, certificatePem); return this; } @@ -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; diff --git a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerBuilder.java b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerBuilder.java index 34a0de74b13..e41c94a2a85 100644 --- a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerBuilder.java +++ b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerBuilder.java @@ -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; } @@ -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; }