Skip to content

Commit fc793e1

Browse files
original-brownbearoriginal-brownbear
authored
Remove Unused CS Listener from SecurityServerTransportInterceptor (#83556)
This listener only updated the value of an unused field => Removed it.
1 parent f8b3cfd commit fc793e1

File tree

7 files changed

+17
-41
lines changed

7 files changed

+17
-41
lines changed

‎x-pack/plugin/identity-provider/src/test/java/org/elasticsearch/xpack/idp/LocalStateIdentityProviderPlugin.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ public class LocalStateIdentityProviderPlugin extends LocalStateCompositeXPackPl
2020
public LocalStateIdentityProviderPlugin(Settings settings, Path configPath) throws Exception {
2121
super(settings, configPath);
2222
LocalStateIdentityProviderPlugin thisVar = this;
23-
plugins.add(new Security(settings, configPath) {
23+
plugins.add(new Security(settings) {
2424
@Override
2525
protected SSLService getSslService() {
2626
return thisVar.getSslService();

‎x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/LocalStateMachineLearning.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ protected XPackLicenseState getLicenseState() {
6868
return thisVar.getLicenseState();
6969
}
7070
});
71-
plugins.add(new Security(settings, configPath) {
71+
plugins.add(new Security(settings) {
7272
@Override
7373
protected SSLService getSslService() {
7474
return thisVar.getSslService();

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,6 @@
319319
import org.elasticsearch.xpack.security.transport.nio.SecurityNioTransport;
320320

321321
import java.io.IOException;
322-
import java.nio.file.Path;
323322
import java.time.Clock;
324323
import java.util.ArrayList;
325324
import java.util.Arrays;
@@ -471,11 +470,11 @@ public class Security extends Plugin
471470
private final SetOnce<Transport> transportReference = new SetOnce<>();
472471
private final SetOnce<ScriptService> scriptServiceReference = new SetOnce<>();
473472

474-
public Security(Settings settings, final Path configPath) {
475-
this(settings, configPath, Collections.emptyList());
473+
public Security(Settings settings) {
474+
this(settings, Collections.emptyList());
476475
}
477476

478-
Security(Settings settings, final Path configPath, List<SecurityExtension> extensions) {
477+
Security(Settings settings, List<SecurityExtension> extensions) {
479478
// TODO This is wrong. Settings can change after this. We should use the settings from createComponents
480479
this.settings = settings;
481480
// TODO this is wrong, we should only use the environment that is provided to createComponents
@@ -863,8 +862,7 @@ Collection<Object> createComponents(
863862
authzService,
864863
getSslService(),
865864
securityContext.get(),
866-
destructiveOperations,
867-
clusterService
865+
destructiveOperations
868866
)
869867
);
870868

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityServerTransportInterceptor.java

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,12 @@
1111
import org.elasticsearch.Version;
1212
import org.elasticsearch.action.ActionListener;
1313
import org.elasticsearch.action.support.DestructiveOperations;
14-
import org.elasticsearch.cluster.service.ClusterService;
1514
import org.elasticsearch.common.settings.Settings;
1615
import org.elasticsearch.common.ssl.SslConfiguration;
1716
import org.elasticsearch.common.util.Maps;
1817
import org.elasticsearch.common.util.concurrent.AbstractRunnable;
1918
import org.elasticsearch.common.util.concurrent.RunOnce;
2019
import org.elasticsearch.common.util.concurrent.ThreadContext;
21-
import org.elasticsearch.gateway.GatewayService;
2220
import org.elasticsearch.tasks.Task;
2321
import org.elasticsearch.threadpool.ThreadPool;
2422
import org.elasticsearch.transport.SendRequestTransportException;
@@ -57,17 +55,14 @@ public class SecurityServerTransportInterceptor implements TransportInterceptor
5755
private final Settings settings;
5856
private final SecurityContext securityContext;
5957

60-
private volatile boolean isStateNotRecovered = true;
61-
6258
public SecurityServerTransportInterceptor(
6359
Settings settings,
6460
ThreadPool threadPool,
6561
AuthenticationService authcService,
6662
AuthorizationService authzService,
6763
SSLService sslService,
6864
SecurityContext securityContext,
69-
DestructiveOperations destructiveOperations,
70-
ClusterService clusterService
65+
DestructiveOperations destructiveOperations
7166
) {
7267
this.settings = settings;
7368
this.threadPool = threadPool;
@@ -76,7 +71,6 @@ public SecurityServerTransportInterceptor(
7671
this.sslService = sslService;
7772
this.securityContext = securityContext;
7873
this.profileFilters = initializeProfileFilters(destructiveOperations);
79-
clusterService.addListener(e -> isStateNotRecovered = e.state().blocks().hasGlobalBlock(GatewayService.STATE_NOT_RECOVERED_BLOCK));
8074
}
8175

8276
@Override
@@ -176,16 +170,7 @@ public <T extends TransportRequest> TransportRequestHandler<T> interceptHandler(
176170
boolean forceExecution,
177171
TransportRequestHandler<T> actualHandler
178172
) {
179-
return new ProfileSecuredRequestHandler<>(
180-
logger,
181-
action,
182-
forceExecution,
183-
executor,
184-
actualHandler,
185-
profileFilters,
186-
settings,
187-
threadPool
188-
);
173+
return new ProfileSecuredRequestHandler<>(logger, action, forceExecution, executor, actualHandler, profileFilters, threadPool);
189174
}
190175

191176
private Map<String, ServerTransportFilter> initializeProfileFilters(DestructiveOperations destructiveOperations) {
@@ -232,7 +217,6 @@ public static class ProfileSecuredRequestHandler<T extends TransportRequest> imp
232217
String executorName,
233218
TransportRequestHandler<T> handler,
234219
Map<String, ServerTransportFilter> profileFilters,
235-
Settings settings,
236220
ThreadPool threadPool
237221
) {
238222
this.logger = logger;

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/LocalStateSecurity.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ protected XPackLicenseState getLicenseState() {
9797
return thisVar.getLicenseState();
9898
}
9999
});
100-
plugins.add(new Security(settings, configPath) {
100+
plugins.add(new Security(settings) {
101101
@Override
102102
protected SSLService getSslService() {
103103
return thisVar.getSslService();

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ private Collection<Object> createComponentsUtil(Settings settings, SecurityExten
132132
NodeMetadata nodeMetadata = new NodeMetadata(randomAlphaOfLength(8), Version.CURRENT, Version.CURRENT);
133133
licenseState = new TestUtils.UpdatableLicenseState(settings);
134134
SSLService sslService = new SSLService(env);
135-
security = new Security(settings, null, Arrays.asList(extensions)) {
135+
security = new Security(settings, Arrays.asList(extensions)) {
136136
@Override
137137
protected XPackLicenseState getLicenseState() {
138138
return licenseState;
@@ -658,7 +658,7 @@ public void testSecurityPluginInstallsRestHandlerWrapperEvenIfSecurityIsDisabled
658658

659659
try {
660660
UsageService usageService = new UsageService();
661-
Security security = new Security(settings, null);
661+
Security security = new Security(settings);
662662
assertTrue(security.getRestHandlerWrapper(threadPool.getThreadContext()) != null);
663663

664664
} finally {
@@ -680,7 +680,7 @@ public void testSecurityRestHandlerWrapperCanBeInstalled() throws IllegalAccessE
680680

681681
try {
682682
UsageService usageService = new UsageService();
683-
Security security = new Security(settings, null);
683+
Security security = new Security(settings);
684684

685685
// Verify Security rest wrapper is about to be installed
686686
// We will throw later if another wrapper is already installed

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/SecurityServerTransportInterceptorTests.java

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,7 @@ public void testSendAsync() throws Exception {
9696
new DestructiveOperations(
9797
Settings.EMPTY,
9898
new ClusterSettings(Settings.EMPTY, Collections.singleton(DestructiveOperations.REQUIRES_NAME_SETTING))
99-
),
100-
clusterService
99+
)
101100
);
102101
ClusterServiceUtils.setState(clusterService, clusterService.state()); // force state update to trigger listener
103102

@@ -144,8 +143,7 @@ public void testSendAsyncSwitchToSystem() throws Exception {
144143
new DestructiveOperations(
145144
Settings.EMPTY,
146145
new ClusterSettings(Settings.EMPTY, Collections.singleton(DestructiveOperations.REQUIRES_NAME_SETTING))
147-
),
148-
clusterService
146+
)
149147
);
150148
ClusterServiceUtils.setState(clusterService, clusterService.state()); // force state update to trigger listener
151149

@@ -187,8 +185,7 @@ public void testSendWithoutUser() throws Exception {
187185
new DestructiveOperations(
188186
Settings.EMPTY,
189187
new ClusterSettings(Settings.EMPTY, Collections.singleton(DestructiveOperations.REQUIRES_NAME_SETTING))
190-
),
191-
clusterService
188+
)
192189
) {
193190
@Override
194191
void assertNoAuthentication(String action) {}
@@ -236,8 +233,7 @@ public void testSendToNewerVersionSetsCorrectVersion() throws Exception {
236233
new DestructiveOperations(
237234
Settings.EMPTY,
238235
new ClusterSettings(Settings.EMPTY, Collections.singleton(DestructiveOperations.REQUIRES_NAME_SETTING))
239-
),
240-
clusterService
236+
)
241237
);
242238
ClusterServiceUtils.setState(clusterService, clusterService.state()); // force state update to trigger listener
243239

@@ -291,8 +287,7 @@ public void testSendToOlderVersionSetsCorrectVersion() throws Exception {
291287
new DestructiveOperations(
292288
Settings.EMPTY,
293289
new ClusterSettings(Settings.EMPTY, Collections.singleton(DestructiveOperations.REQUIRES_NAME_SETTING))
294-
),
295-
clusterService
290+
)
296291
);
297292
ClusterServiceUtils.setState(clusterService, clusterService.state()); // force state update to trigger listener
298293

@@ -411,7 +406,6 @@ public void testProfileSecuredRequestHandlerDecrementsRefCountOnFailure() throws
411406
profileName,
412407
new ServerTransportFilter(null, null, threadContext, randomBoolean(), destructiveOperations, securityContext)
413408
),
414-
settings,
415409
threadPool
416410
);
417411
final TransportChannel channel = mock(TransportChannel.class);

0 commit comments

Comments
 (0)