From 471fce0f94cb455e649b4128149215d0c2b27c8f Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Fri, 8 Sep 2023 13:31:41 -0700 Subject: [PATCH 1/4] Add tracer for getHttpTransports Signed-off-by: Ryan Liang --- .../org/opensearch/security/OpenSearchSecurityPlugin.java | 7 +++++-- .../security/ssl/OpenSearchSecuritySSLPlugin.java | 7 +++++-- .../http/netty/SecuritySSLNettyHttpServerTransport.java | 4 +++- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 1803a58fc9..d5f03742ae 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -180,6 +180,7 @@ import org.opensearch.security.user.User; import org.opensearch.security.user.UserService; import org.opensearch.tasks.Task; +import org.opensearch.telemetry.tracing.Tracer; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.RemoteClusterService; import org.opensearch.transport.Transport; @@ -860,7 +861,8 @@ public Map> getHttpTransports( NamedXContentRegistry xContentRegistry, NetworkService networkService, Dispatcher dispatcher, - ClusterSettings clusterSettings + ClusterSettings clusterSettings, + Tracer tracer ) { if (SSLConfig.isSslOnlyMode()) { @@ -873,7 +875,8 @@ public Map> getHttpTransports( xContentRegistry, networkService, dispatcher, - clusterSettings + clusterSettings, + tracer ); } diff --git a/src/main/java/org/opensearch/security/ssl/OpenSearchSecuritySSLPlugin.java b/src/main/java/org/opensearch/security/ssl/OpenSearchSecuritySSLPlugin.java index 711d7ff99e..bff2cf02d5 100644 --- a/src/main/java/org/opensearch/security/ssl/OpenSearchSecuritySSLPlugin.java +++ b/src/main/java/org/opensearch/security/ssl/OpenSearchSecuritySSLPlugin.java @@ -80,6 +80,7 @@ import org.opensearch.security.ssl.transport.SecuritySSLNettyTransport; import org.opensearch.security.ssl.transport.SecuritySSLTransportInterceptor; import org.opensearch.security.ssl.util.SSLConfigConstants; +import org.opensearch.telemetry.tracing.Tracer; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.SharedGroupFactory; import org.opensearch.transport.Transport; @@ -242,7 +243,8 @@ public Map> getHttpTransports( NamedXContentRegistry xContentRegistry, NetworkService networkService, Dispatcher dispatcher, - ClusterSettings clusterSettings + ClusterSettings clusterSettings, + Tracer tracer ) { if (!client && httpSSLEnabled) { @@ -264,7 +266,8 @@ public Map> getHttpTransports( validatingDispatcher, NOOP_SSL_EXCEPTION_HANDLER, clusterSettings, - sharedGroupFactory + sharedGroupFactory, + tracer ); return Collections.singletonMap("org.opensearch.security.ssl.http.netty.SecuritySSLNettyHttpServerTransport", () -> sgsnht); diff --git a/src/main/java/org/opensearch/security/ssl/http/netty/SecuritySSLNettyHttpServerTransport.java b/src/main/java/org/opensearch/security/ssl/http/netty/SecuritySSLNettyHttpServerTransport.java index bfaa060935..8c3b6f0ddc 100644 --- a/src/main/java/org/opensearch/security/ssl/http/netty/SecuritySSLNettyHttpServerTransport.java +++ b/src/main/java/org/opensearch/security/ssl/http/netty/SecuritySSLNettyHttpServerTransport.java @@ -38,6 +38,7 @@ import org.opensearch.http.netty4.Netty4HttpServerTransport; import org.opensearch.security.ssl.SecurityKeyStore; import org.opensearch.security.ssl.SslExceptionHandler; +import org.opensearch.telemetry.tracing.Tracer; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.SharedGroupFactory; @@ -56,7 +57,8 @@ public SecuritySSLNettyHttpServerTransport( final ValidatingDispatcher dispatcher, final SslExceptionHandler errorHandler, ClusterSettings clusterSettings, - SharedGroupFactory sharedGroupFactory + SharedGroupFactory sharedGroupFactory, + Tracer tracer ) { super(settings, networkService, bigArrays, threadPool, namedXContentRegistry, dispatcher, clusterSettings, sharedGroupFactory); this.sks = sks; From c86a610efaf6bf87e8eda7fc58d59d4aabef8777 Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Fri, 8 Sep 2023 13:45:01 -0700 Subject: [PATCH 2/4] Second commit Signed-off-by: Ryan Liang --- .../security/OpenSearchSecurityPlugin.java | 6 ++++-- .../http/SecurityHttpServerTransport.java | 7 +++++-- .../http/SecurityNonSslHttpServerTransport.java | 16 ++++++++++++++-- 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index d5f03742ae..8b1e307172 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -901,7 +901,8 @@ public Map> getHttpTransports( xContentRegistry, validatingDispatcher, clusterSettings, - sharedGroupFactory + sharedGroupFactory, + tracer ); return Collections.singletonMap("org.opensearch.security.http.SecurityHttpServerTransport", () -> odshst); @@ -916,7 +917,8 @@ public Map> getHttpTransports( xContentRegistry, dispatcher, clusterSettings, - sharedGroupFactory + sharedGroupFactory, + tracer ) ); } diff --git a/src/main/java/org/opensearch/security/http/SecurityHttpServerTransport.java b/src/main/java/org/opensearch/security/http/SecurityHttpServerTransport.java index e9487a49a9..fc36e2411b 100644 --- a/src/main/java/org/opensearch/security/http/SecurityHttpServerTransport.java +++ b/src/main/java/org/opensearch/security/http/SecurityHttpServerTransport.java @@ -35,6 +35,7 @@ import org.opensearch.security.ssl.SslExceptionHandler; import org.opensearch.security.ssl.http.netty.SecuritySSLNettyHttpServerTransport; import org.opensearch.security.ssl.http.netty.ValidatingDispatcher; +import org.opensearch.telemetry.tracing.Tracer; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.SharedGroupFactory; @@ -50,7 +51,8 @@ public SecurityHttpServerTransport( final NamedXContentRegistry namedXContentRegistry, final ValidatingDispatcher dispatcher, final ClusterSettings clusterSettings, - SharedGroupFactory sharedGroupFactory + SharedGroupFactory sharedGroupFactory, + Tracer tracer ) { super( settings, @@ -62,7 +64,8 @@ public SecurityHttpServerTransport( dispatcher, sslExceptionHandler, clusterSettings, - sharedGroupFactory + sharedGroupFactory, + tracer ); } } diff --git a/src/main/java/org/opensearch/security/http/SecurityNonSslHttpServerTransport.java b/src/main/java/org/opensearch/security/http/SecurityNonSslHttpServerTransport.java index 1c21f0c4a2..a8e675ec74 100644 --- a/src/main/java/org/opensearch/security/http/SecurityNonSslHttpServerTransport.java +++ b/src/main/java/org/opensearch/security/http/SecurityNonSslHttpServerTransport.java @@ -36,6 +36,7 @@ import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.http.HttpHandlingSettings; import org.opensearch.http.netty4.Netty4HttpServerTransport; +import org.opensearch.telemetry.tracing.Tracer; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.SharedGroupFactory; @@ -49,9 +50,20 @@ public SecurityNonSslHttpServerTransport( final NamedXContentRegistry namedXContentRegistry, final Dispatcher dispatcher, ClusterSettings clusterSettings, - SharedGroupFactory sharedGroupFactory + SharedGroupFactory sharedGroupFactory, + Tracer tracer ) { - super(settings, networkService, bigArrays, threadPool, namedXContentRegistry, dispatcher, clusterSettings, sharedGroupFactory); + super( + settings, + networkService, + bigArrays, + threadPool, + namedXContentRegistry, + dispatcher, + clusterSettings, + sharedGroupFactory, + tracer + ); } @Override From 0407489a67778ceec7e26e1a94de024ec0b9534d Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Fri, 8 Sep 2023 13:48:14 -0700 Subject: [PATCH 3/4] Third commit Signed-off-by: Ryan Liang --- .../ssl/http/netty/SecuritySSLNettyHttpServerTransport.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/opensearch/security/ssl/http/netty/SecuritySSLNettyHttpServerTransport.java b/src/main/java/org/opensearch/security/ssl/http/netty/SecuritySSLNettyHttpServerTransport.java index 8c3b6f0ddc..3e1cf7428e 100644 --- a/src/main/java/org/opensearch/security/ssl/http/netty/SecuritySSLNettyHttpServerTransport.java +++ b/src/main/java/org/opensearch/security/ssl/http/netty/SecuritySSLNettyHttpServerTransport.java @@ -60,7 +60,7 @@ public SecuritySSLNettyHttpServerTransport( SharedGroupFactory sharedGroupFactory, Tracer tracer ) { - super(settings, networkService, bigArrays, threadPool, namedXContentRegistry, dispatcher, clusterSettings, sharedGroupFactory); + super(settings, networkService, bigArrays, threadPool, namedXContentRegistry, dispatcher, clusterSettings, sharedGroupFactory, tracer); this.sks = sks; this.errorHandler = errorHandler; } From a1a86325934a9363172363ffd7115251c41d642d Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Fri, 8 Sep 2023 14:09:40 -0700 Subject: [PATCH 4/4] Fix the SecurityInterceptorTests Signed-off-by: Ryan Liang --- .../SecuritySSLNettyHttpServerTransport.java | 12 +++++++++- .../test/plugin/UserInjectorPlugin.java | 22 +++++++++++++++---- .../transport/SecurityInterceptorTests.java | 4 +++- 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/opensearch/security/ssl/http/netty/SecuritySSLNettyHttpServerTransport.java b/src/main/java/org/opensearch/security/ssl/http/netty/SecuritySSLNettyHttpServerTransport.java index 3e1cf7428e..081cc13f3e 100644 --- a/src/main/java/org/opensearch/security/ssl/http/netty/SecuritySSLNettyHttpServerTransport.java +++ b/src/main/java/org/opensearch/security/ssl/http/netty/SecuritySSLNettyHttpServerTransport.java @@ -60,7 +60,17 @@ public SecuritySSLNettyHttpServerTransport( SharedGroupFactory sharedGroupFactory, Tracer tracer ) { - super(settings, networkService, bigArrays, threadPool, namedXContentRegistry, dispatcher, clusterSettings, sharedGroupFactory, tracer); + super( + settings, + networkService, + bigArrays, + threadPool, + namedXContentRegistry, + dispatcher, + clusterSettings, + sharedGroupFactory, + tracer + ); this.sks = sks; this.errorHandler = errorHandler; } diff --git a/src/test/java/org/opensearch/security/test/plugin/UserInjectorPlugin.java b/src/test/java/org/opensearch/security/test/plugin/UserInjectorPlugin.java index 1046bc81e9..73ede93651 100644 --- a/src/test/java/org/opensearch/security/test/plugin/UserInjectorPlugin.java +++ b/src/test/java/org/opensearch/security/test/plugin/UserInjectorPlugin.java @@ -48,6 +48,7 @@ import org.opensearch.rest.RestChannel; import org.opensearch.rest.RestRequest; import org.opensearch.security.support.ConfigConstants; +import org.opensearch.telemetry.tracing.Tracer; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.SharedGroupFactory; @@ -78,7 +79,8 @@ public Map> getHttpTransports( NamedXContentRegistry xContentRegistry, NetworkService networkService, Dispatcher dispatcher, - ClusterSettings clusterSettings + ClusterSettings clusterSettings, + Tracer tracer ) { final UserInjectingDispatcher validatingDispatcher = new UserInjectingDispatcher(dispatcher); @@ -92,7 +94,8 @@ public Map> getHttpTransports( xContentRegistry, validatingDispatcher, clusterSettings, - sharedGroupFactory + sharedGroupFactory, + tracer ) ); } @@ -107,9 +110,20 @@ public UserInjectingServerTransport( final NamedXContentRegistry namedXContentRegistry, final Dispatcher dispatcher, ClusterSettings clusterSettings, - SharedGroupFactory sharedGroupFactory + SharedGroupFactory sharedGroupFactory, + Tracer tracer ) { - super(settings, networkService, bigArrays, threadPool, namedXContentRegistry, dispatcher, clusterSettings, sharedGroupFactory); + super( + settings, + networkService, + bigArrays, + threadPool, + namedXContentRegistry, + dispatcher, + clusterSettings, + sharedGroupFactory, + tracer + ); } } diff --git a/src/test/java/org/opensearch/security/transport/SecurityInterceptorTests.java b/src/test/java/org/opensearch/security/transport/SecurityInterceptorTests.java index 73c5edd8b0..d3363c54d8 100644 --- a/src/test/java/org/opensearch/security/transport/SecurityInterceptorTests.java +++ b/src/test/java/org/opensearch/security/transport/SecurityInterceptorTests.java @@ -33,6 +33,7 @@ import org.opensearch.security.support.Base64Helper; import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.user.User; +import org.opensearch.telemetry.tracing.noop.NoopTracer; import org.opensearch.test.transport.MockTransport; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.Transport.Connection; @@ -122,7 +123,8 @@ public void testSendRequestDecorate() { TransportService.NOOP_TRANSPORT_INTERCEPTOR, boundTransportAddress -> clusterService.state().nodes().get(SecurityInterceptor.class.getSimpleName()), null, - emptySet() + emptySet(), + NoopTracer.INSTANCE ); // CS-SUPPRESS-SINGLE: RegexpSingleline Extensions manager used for creating a mock