Skip to content

Commit fdba4b7

Browse files
retareta
committed
Address code review comments, added more tests
Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
1 parent 3632744 commit fdba4b7

File tree

2 files changed

+270
-0
lines changed

2 files changed

+270
-0
lines changed

‎modules/transport-netty4/src/test/java/org/opensearch/http/netty4/ssl/SecureNetty4HttpServerTransportConfigurationTests.java

+242
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,242 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
package org.opensearch.http.netty4.ssl;
10+
11+
import org.opensearch.common.network.NetworkService;
12+
import org.opensearch.common.settings.ClusterSettings;
13+
import org.opensearch.common.settings.Settings;
14+
import org.opensearch.common.util.MockBigArrays;
15+
import org.opensearch.common.util.MockPageCacheRecycler;
16+
import org.opensearch.core.indices.breaker.NoneCircuitBreakerService;
17+
import org.opensearch.http.HttpServerTransport;
18+
import org.opensearch.http.NullDispatcher;
19+
import org.opensearch.plugins.SecureHttpTransportSettingsProvider;
20+
import org.opensearch.plugins.TransportExceptionHandler;
21+
import org.opensearch.telemetry.tracing.noop.NoopTracer;
22+
import org.opensearch.test.OpenSearchTestCase;
23+
import org.opensearch.threadpool.TestThreadPool;
24+
import org.opensearch.threadpool.ThreadPool;
25+
import org.opensearch.transport.SharedGroupFactory;
26+
import org.opensearch.transport.TransportAdapterProvider;
27+
import org.junit.After;
28+
import org.junit.Before;
29+
30+
import javax.net.ssl.SSLEngine;
31+
import javax.net.ssl.SSLException;
32+
33+
import java.util.Collection;
34+
import java.util.Collections;
35+
import java.util.List;
36+
import java.util.Optional;
37+
38+
import io.netty.channel.ChannelInboundHandlerAdapter;
39+
40+
import static org.hamcrest.Matchers.equalTo;
41+
42+
/**
43+
* Tests for the {@link SecureNetty4HttpServerTransport} class.
44+
*/
45+
public class SecureNetty4HttpServerTransportConfigurationTests extends OpenSearchTestCase {
46+
47+
private NetworkService networkService;
48+
private ThreadPool threadPool;
49+
private MockBigArrays bigArrays;
50+
private ClusterSettings clusterSettings;
51+
52+
private static class ConfigurableSecureHttpTransportSettingsProvider implements SecureHttpTransportSettingsProvider {
53+
private final List<TransportAdapterProvider<HttpServerTransport>> transportAdapterProviders;
54+
55+
public ConfigurableSecureHttpTransportSettingsProvider(
56+
List<TransportAdapterProvider<HttpServerTransport>> transportAdapterProviders
57+
) {
58+
this.transportAdapterProviders = transportAdapterProviders;
59+
}
60+
61+
@Override
62+
public Collection<TransportAdapterProvider<HttpServerTransport>> getHttpTransportAdapterProviders(Settings settings) {
63+
return transportAdapterProviders;
64+
}
65+
66+
@Override
67+
public Optional<TransportExceptionHandler> buildHttpServerExceptionHandler(Settings settings, HttpServerTransport transport) {
68+
return Optional.empty();
69+
}
70+
71+
@Override
72+
public Optional<SSLEngine> buildSecureHttpServerEngine(Settings settings, HttpServerTransport transport) throws SSLException {
73+
return Optional.empty();
74+
}
75+
}
76+
77+
@Before
78+
public void setup() throws Exception {
79+
networkService = new NetworkService(Collections.emptyList());
80+
threadPool = new TestThreadPool("test");
81+
bigArrays = new MockBigArrays(new MockPageCacheRecycler(Settings.EMPTY), new NoneCircuitBreakerService());
82+
clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);
83+
}
84+
85+
@After
86+
public void shutdown() throws Exception {
87+
if (threadPool != null) {
88+
threadPool.shutdownNow();
89+
}
90+
threadPool = null;
91+
networkService = null;
92+
bigArrays = null;
93+
clusterSettings = null;
94+
}
95+
96+
public void testRequestHeaderVerifier() throws InterruptedException {
97+
final TransportAdapterProvider<HttpServerTransport> transportAdapterProvider = new TransportAdapterProvider<HttpServerTransport>() {
98+
@Override
99+
public String name() {
100+
return SecureNetty4HttpServerTransport.REQUEST_HEADER_VERIFIER;
101+
}
102+
103+
@SuppressWarnings("unchecked")
104+
@Override
105+
public <C> Optional<C> create(Settings settings, HttpServerTransport transport, Class<C> adapterClass) {
106+
return Optional.of((C) new ChannelInboundHandlerAdapter());
107+
}
108+
109+
};
110+
111+
try (
112+
final SecureNetty4HttpServerTransport transport = new SecureNetty4HttpServerTransport(
113+
Settings.EMPTY,
114+
networkService,
115+
bigArrays,
116+
threadPool,
117+
xContentRegistry(),
118+
new NullDispatcher(),
119+
clusterSettings,
120+
new SharedGroupFactory(Settings.EMPTY),
121+
new ConfigurableSecureHttpTransportSettingsProvider(List.of(transportAdapterProvider)),
122+
NoopTracer.INSTANCE
123+
)
124+
) {
125+
126+
}
127+
}
128+
129+
public void testMultipleRequestHeaderVerifiers() throws InterruptedException {
130+
final TransportAdapterProvider<HttpServerTransport> transportAdapterProvider = new TransportAdapterProvider<HttpServerTransport>() {
131+
@Override
132+
public String name() {
133+
return SecureNetty4HttpServerTransport.REQUEST_HEADER_VERIFIER;
134+
}
135+
136+
@SuppressWarnings("unchecked")
137+
@Override
138+
public <C> Optional<C> create(Settings settings, HttpServerTransport transport, Class<C> adapterClass) {
139+
return Optional.of((C) new ChannelInboundHandlerAdapter());
140+
}
141+
142+
};
143+
144+
final IllegalArgumentException ex = assertThrows(
145+
IllegalArgumentException.class,
146+
() -> new SecureNetty4HttpServerTransport(
147+
Settings.EMPTY,
148+
networkService,
149+
bigArrays,
150+
threadPool,
151+
xContentRegistry(),
152+
new NullDispatcher(),
153+
clusterSettings,
154+
new SharedGroupFactory(Settings.EMPTY),
155+
new ConfigurableSecureHttpTransportSettingsProvider(List.of(transportAdapterProvider, transportAdapterProvider)),
156+
NoopTracer.INSTANCE
157+
)
158+
);
159+
160+
assertThat(ex.getMessage(), equalTo("Cannot have more than one header verifier configured, supplied 2"));
161+
}
162+
163+
public void testRequestDecompressor() throws InterruptedException {
164+
final TransportAdapterProvider<HttpServerTransport> transportAdapterProvider = new TransportAdapterProvider<HttpServerTransport>() {
165+
@Override
166+
public String name() {
167+
return SecureNetty4HttpServerTransport.REQUEST_DECOMPRESSOR;
168+
}
169+
170+
@SuppressWarnings("unchecked")
171+
@Override
172+
public <C> Optional<C> create(Settings settings, HttpServerTransport transport, Class<C> adapterClass) {
173+
return Optional.of((C) new ChannelInboundHandlerAdapter());
174+
}
175+
176+
};
177+
178+
try (
179+
final SecureNetty4HttpServerTransport transport = new SecureNetty4HttpServerTransport(
180+
Settings.EMPTY,
181+
networkService,
182+
bigArrays,
183+
threadPool,
184+
xContentRegistry(),
185+
new NullDispatcher(),
186+
clusterSettings,
187+
new SharedGroupFactory(Settings.EMPTY),
188+
new ConfigurableSecureHttpTransportSettingsProvider(List.of(transportAdapterProvider)),
189+
NoopTracer.INSTANCE
190+
)
191+
) {
192+
193+
}
194+
}
195+
196+
public void testRequestDecompressorAndRequestHeaderVerifier() throws InterruptedException {
197+
final TransportAdapterProvider<HttpServerTransport> requestDecompressor = new TransportAdapterProvider<HttpServerTransport>() {
198+
@Override
199+
public String name() {
200+
return SecureNetty4HttpServerTransport.REQUEST_DECOMPRESSOR;
201+
}
202+
203+
@SuppressWarnings("unchecked")
204+
@Override
205+
public <C> Optional<C> create(Settings settings, HttpServerTransport transport, Class<C> adapterClass) {
206+
return Optional.of((C) new ChannelInboundHandlerAdapter());
207+
}
208+
209+
};
210+
211+
final TransportAdapterProvider<HttpServerTransport> requestHeaderVerifier = new TransportAdapterProvider<HttpServerTransport>() {
212+
@Override
213+
public String name() {
214+
return SecureNetty4HttpServerTransport.REQUEST_HEADER_VERIFIER;
215+
}
216+
217+
@SuppressWarnings("unchecked")
218+
@Override
219+
public <C> Optional<C> create(Settings settings, HttpServerTransport transport, Class<C> adapterClass) {
220+
return Optional.of((C) new ChannelInboundHandlerAdapter());
221+
}
222+
223+
};
224+
225+
try (
226+
final SecureNetty4HttpServerTransport transport = new SecureNetty4HttpServerTransport(
227+
Settings.EMPTY,
228+
networkService,
229+
bigArrays,
230+
threadPool,
231+
xContentRegistry(),
232+
new NullDispatcher(),
233+
clusterSettings,
234+
new SharedGroupFactory(Settings.EMPTY),
235+
new ConfigurableSecureHttpTransportSettingsProvider(List.of(requestDecompressor, requestHeaderVerifier)),
236+
NoopTracer.INSTANCE
237+
)
238+
) {
239+
240+
}
241+
}
242+
}

‎server/src/test/java/org/opensearch/common/network/NetworkModuleTests.java

+28
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,8 @@
7575
import java.util.concurrent.atomic.AtomicInteger;
7676
import java.util.function.Supplier;
7777

78+
import static org.hamcrest.CoreMatchers.startsWith;
79+
7880
public class NetworkModuleTests extends OpenSearchTestCase {
7981
private ThreadPool threadPool;
8082
private SecureSettingsFactory secureSettingsFactory;
@@ -637,4 +639,30 @@ private NetworkModule newNetworkModule(
637639
secureSettingsFactories
638640
);
639641
}
642+
643+
public void testRegisterSecureTransportMultipleProviers() {
644+
Settings settings = Settings.builder().put(NetworkModule.TRANSPORT_TYPE_KEY, "custom-secure").build();
645+
Supplier<Transport> custom = () -> null; // content doesn't matter we check reference equality
646+
NetworkPlugin plugin = new NetworkPlugin() {
647+
@Override
648+
public Map<String, Supplier<Transport>> getSecureTransports(
649+
Settings settings,
650+
ThreadPool threadPool,
651+
PageCacheRecycler pageCacheRecycler,
652+
CircuitBreakerService circuitBreakerService,
653+
NamedWriteableRegistry namedWriteableRegistry,
654+
NetworkService networkService,
655+
SecureTransportSettingsProvider secureTransportSettingsProvider,
656+
Tracer tracer
657+
) {
658+
return Collections.singletonMap("custom-secure", custom);
659+
}
660+
};
661+
662+
final IllegalArgumentException ex = assertThrows(
663+
IllegalArgumentException.class,
664+
() -> newNetworkModule(settings, null, List.of(secureSettingsFactory, secureSettingsFactory), plugin)
665+
);
666+
assertThat(ex.getMessage(), startsWith("there is more than one secure transport settings provider"));
667+
}
640668
}

0 commit comments

Comments
 (0)