Skip to content

Commit 9f5908e

Browse files
JeremyDahlgrenJeremyDahlgren
authored
Enable RemoteClusterService client features on search nodes by default for CPS (#132553)
For CPS in serverless we do not want to support RemoteClusterService client features on non-search nodes that do not explicitly have the remote cluster client role set. This PR checks if stateless is enabled, and if so requires the node to have the search node role to use RemoteClusterService client features if the remote cluster client role is not set. If stateless is disabled the check is only for the remote cluster client role, as is currently done today. Resolves: ES-12597.
1 parent ccb721b commit 9f5908e

File tree

5 files changed

+126
-42
lines changed

5 files changed

+126
-42
lines changed

‎server/src/main/java/org/elasticsearch/action/admin/cluster/remote/TransportRemoteInfoAction.java‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
import org.elasticsearch.action.ActionType;
1414
import org.elasticsearch.action.support.ActionFilters;
1515
import org.elasticsearch.action.support.HandledTransportAction;
16-
import org.elasticsearch.cluster.node.DiscoveryNodeRole;
1716
import org.elasticsearch.common.util.concurrent.EsExecutors;
1817
import org.elasticsearch.injection.guice.Inject;
1918
import org.elasticsearch.tasks.Task;
@@ -35,15 +34,7 @@ public TransportRemoteInfoAction(TransportService transportService, ActionFilter
3534

3635
@Override
3736
protected void doExecute(Task task, RemoteInfoRequest remoteInfoRequest, ActionListener<RemoteInfoResponse> listener) {
38-
if (remoteClusterService.isEnabled() == false) {
39-
throw new IllegalArgumentException(
40-
"node ["
41-
+ remoteClusterService.getLocalNode().getName()
42-
+ "] does not have the ["
43-
+ DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE.roleName()
44-
+ "] role"
45-
);
46-
}
37+
remoteClusterService.ensureClientIsEnabled();
4738
listener.onResponse(new RemoteInfoResponse(remoteClusterService.getRemoteConnectionInfos().collect(toList())));
4839
}
4940
}

‎server/src/main/java/org/elasticsearch/transport/RemoteClusterAware.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@ protected RemoteClusterAware(Settings settings) {
4949
this.isRemoteClusterClientEnabled = DiscoveryNode.isRemoteClusterClient(settings);
5050
}
5151

52+
protected String getNodeName() {
53+
return nodeName;
54+
}
55+
5256
/**
5357
* Returns remote clusters that are enabled in these settings
5458
*/

‎server/src/main/java/org/elasticsearch/transport/RemoteClusterService.java‎

Lines changed: 37 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -147,13 +147,11 @@ public final class RemoteClusterService extends RemoteClusterAware
147147

148148
public static final String REMOTE_CLUSTER_HANDSHAKE_ACTION_NAME = "cluster:internal/remote_cluster/handshake";
149149

150-
private final boolean enabled;
150+
private final boolean isRemoteClusterClient;
151+
private final boolean isSearchNode;
152+
private final boolean isStateless;
151153
private final boolean remoteClusterServerEnabled;
152154

153-
public boolean isEnabled() {
154-
return enabled;
155-
}
156-
157155
public boolean isRemoteClusterServerEnabled() {
158156
return remoteClusterServerEnabled;
159157
}
@@ -166,7 +164,9 @@ public boolean isRemoteClusterServerEnabled() {
166164
@FixForMultiProject(description = "Inject the ProjectResolver instance.")
167165
RemoteClusterService(Settings settings, TransportService transportService) {
168166
super(settings);
169-
this.enabled = DiscoveryNode.isRemoteClusterClient(settings);
167+
this.isRemoteClusterClient = DiscoveryNode.isRemoteClusterClient(settings);
168+
this.isSearchNode = DiscoveryNode.hasRole(settings, DiscoveryNodeRole.SEARCH_ROLE);
169+
this.isStateless = DiscoveryNode.isStateless(settings);
170170
this.remoteClusterServerEnabled = REMOTE_CLUSTER_SERVER_ENABLED.get(settings);
171171
this.transportService = transportService;
172172
this.projectResolver = DefaultProjectResolver.INSTANCE;
@@ -179,10 +179,6 @@ public boolean isRemoteClusterServerEnabled() {
179179
}
180180
}
181181

182-
public DiscoveryNode getLocalNode() {
183-
return transportService.getLocalNode();
184-
}
185-
186182
/**
187183
* Group indices by cluster alias mapped to OriginalIndices for that cluster.
188184
* @param remoteClusterNames Set of configured remote cluster names.
@@ -335,11 +331,7 @@ public void maybeEnsureConnectedAndGetConnection(
335331
}
336332

337333
public RemoteClusterConnection getRemoteClusterConnection(String cluster) {
338-
if (enabled == false) {
339-
throw new IllegalArgumentException(
340-
"this node does not have the " + DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE.roleName() + " role"
341-
);
342-
}
334+
ensureClientIsEnabled();
343335
@FixForMultiProject(description = "Verify all callers will have the proper context set for resolving the origin project ID.")
344336
RemoteClusterConnection connection = getConnectionsMapForCurrentProject().get(cluster);
345337
if (connection == null) {
@@ -595,11 +587,7 @@ public RemoteClusterServerInfo info() {
595587
* function on success.
596588
*/
597589
public void collectNodes(Set<String> clusters, ActionListener<BiFunction<String, String, DiscoveryNode>> listener) {
598-
if (enabled == false) {
599-
throw new IllegalArgumentException(
600-
"this node does not have the " + DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE.roleName() + " role"
601-
);
602-
}
590+
ensureClientIsEnabled();
603591
@FixForMultiProject(description = "Analyze usages and determine if the project ID must be provided.")
604592
final var projectConnectionsMap = getConnectionsMapForCurrentProject();
605593
final var connectionsMap = new HashMap<String, RemoteClusterConnection>();
@@ -662,11 +650,7 @@ public RemoteClusterClient getRemoteClusterClient(
662650
Executor responseExecutor,
663651
DisconnectedStrategy disconnectedStrategy
664652
) {
665-
if (transportService.getRemoteClusterService().isEnabled() == false) {
666-
throw new IllegalArgumentException(
667-
"this node does not have the " + DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE.roleName() + " role"
668-
);
669-
}
653+
ensureClientIsEnabled();
670654
if (transportService.getRemoteClusterService().getRegisteredRemoteClusterNames().contains(clusterAlias) == false) {
671655
throw new NoSuchRemoteClusterException(clusterAlias);
672656
}
@@ -677,6 +661,34 @@ public RemoteClusterClient getRemoteClusterClient(
677661
});
678662
}
679663

664+
/**
665+
* Verifies this node is configured to support linked project client operations.
666+
* @throws IllegalArgumentException If this node is not configured to support client operations.
667+
*/
668+
public void ensureClientIsEnabled() {
669+
if (isRemoteClusterClient) {
670+
return;
671+
}
672+
if (isStateless == false) {
673+
throw new IllegalArgumentException(
674+
"node [" + getNodeName() + "] does not have the [" + DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE.roleName() + "] role"
675+
);
676+
}
677+
// For stateless the remote cluster client is enabled by default for search nodes,
678+
// REMOTE_CLUSTER_CLIENT_ROLE is not explicitly required.
679+
if (isSearchNode == false) {
680+
throw new IllegalArgumentException(
681+
"node ["
682+
+ getNodeName()
683+
+ "] must have the ["
684+
+ DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE.roleName()
685+
+ "] role or the ["
686+
+ DiscoveryNodeRole.SEARCH_ROLE.roleName()
687+
+ "] role in stateless environments to use linked project client features"
688+
);
689+
}
690+
}
691+
680692
static void registerRemoteClusterHandshakeRequestHandler(TransportService transportService) {
681693
transportService.registerRequestHandler(
682694
REMOTE_CLUSTER_HANDSHAKE_ACTION_NAME,

‎server/src/test/java/org/elasticsearch/transport/RemoteClusterClientTests.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.elasticsearch.common.settings.Settings;
2525
import org.elasticsearch.common.util.concurrent.EsExecutors;
2626
import org.elasticsearch.core.TimeValue;
27+
import org.elasticsearch.node.Node;
2728
import org.elasticsearch.test.ESTestCase;
2829
import org.elasticsearch.test.junit.annotations.TestLogging;
2930
import org.elasticsearch.test.transport.MockTransportService;
@@ -215,7 +216,10 @@ public void testEnsureWeReconnect() throws Exception {
215216
}
216217

217218
public void testRemoteClusterServiceNotEnabled() {
218-
final Settings settings = removeRoles(Set.of(DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE));
219+
final Settings settings = Settings.builder()
220+
.put(removeRoles(Set.of(DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE)))
221+
.put(Node.NODE_NAME_SETTING.getKey(), "node-1")
222+
.build();
219223
try (
220224
MockTransportService service = MockTransportService.createNewService(
221225
settings,
@@ -236,7 +240,7 @@ public void testRemoteClusterServiceNotEnabled() {
236240
randomFrom(RemoteClusterService.DisconnectedStrategy.values())
237241
)
238242
);
239-
assertThat(e.getMessage(), equalTo("this node does not have the remote_cluster_client role"));
243+
assertThat(e.getMessage(), equalTo("node [node-1] does not have the [remote_cluster_client] role"));
240244
}
241245
}
242246

‎server/src/test/java/org/elasticsearch/transport/RemoteClusterServiceTests.java‎

Lines changed: 78 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
import static org.elasticsearch.test.MockLog.assertThatLogger;
5353
import static org.elasticsearch.test.NodeRoles.masterOnlyNode;
5454
import static org.elasticsearch.test.NodeRoles.nonMasterNode;
55+
import static org.elasticsearch.test.NodeRoles.onlyRole;
5556
import static org.elasticsearch.test.NodeRoles.onlyRoles;
5657
import static org.elasticsearch.test.NodeRoles.removeRoles;
5758
import static org.hamcrest.Matchers.containsString;
@@ -441,7 +442,7 @@ public void testGroupIndicesWithoutRemoteClusterClientRole() throws Exception {
441442
Sets.newHashSet(DiscoveryNodeRole.DATA_ROLE)
442443
);
443444
try (RemoteClusterService service = new RemoteClusterService(settings, null)) {
444-
assertFalse(service.isEnabled());
445+
expectThrows(IllegalArgumentException.class, service::ensureClientIsEnabled);
445446
assertFalse(hasRegisteredClusters(service));
446447
final IllegalArgumentException error = expectThrows(
447448
IllegalArgumentException.class,
@@ -1383,7 +1384,10 @@ public void testSkipUnavailable() {
13831384
}
13841385

13851386
public void testRemoteClusterServiceNotEnabledGetRemoteClusterConnection() {
1386-
final Settings settings = removeRoles(Set.of(DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE));
1387+
final Settings settings = Settings.builder()
1388+
.put(removeRoles(Set.of(DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE)))
1389+
.put(Node.NODE_NAME_SETTING.getKey(), "node-1")
1390+
.build();
13871391
try (
13881392
MockTransportService service = MockTransportService.createNewService(
13891393
settings,
@@ -1399,12 +1403,81 @@ public void testRemoteClusterServiceNotEnabledGetRemoteClusterConnection() {
13991403
IllegalArgumentException.class,
14001404
() -> service.getRemoteClusterService().getRemoteClusterConnection("test")
14011405
);
1402-
assertThat(e.getMessage(), equalTo("this node does not have the remote_cluster_client role"));
1406+
assertThat(e.getMessage(), equalTo("node [node-1] does not have the [remote_cluster_client] role"));
1407+
}
1408+
}
1409+
1410+
public void testRemoteClusterServiceEnsureClientIsEnabled() throws IOException {
1411+
final var nodeNameSettings = Settings.builder().put(Node.NODE_NAME_SETTING.getKey(), "node-1").build();
1412+
1413+
// Shouldn't throw when the remote cluster client role is enabled.
1414+
final var settingsWithRemoteClusterClientRole = Settings.builder()
1415+
.put(nodeNameSettings)
1416+
.put(onlyRole(DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE))
1417+
.build();
1418+
try (RemoteClusterService service = new RemoteClusterService(settingsWithRemoteClusterClientRole, null)) {
1419+
service.ensureClientIsEnabled();
1420+
}
1421+
1422+
// Expect throws when missing the remote cluster client role.
1423+
final var settingsWithoutRemoteClusterClientRole = Settings.builder()
1424+
.put(nodeNameSettings)
1425+
.put(removeRoles(Set.of(DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE)))
1426+
.build();
1427+
try (RemoteClusterService service = new RemoteClusterService(settingsWithoutRemoteClusterClientRole, null)) {
1428+
final var exception = expectThrows(IllegalArgumentException.class, service::ensureClientIsEnabled);
1429+
assertThat(exception.getMessage(), equalTo("node [node-1] does not have the [remote_cluster_client] role"));
1430+
}
1431+
1432+
// Expect throws when missing both the remote cluster client role and search node role when stateless is enabled.
1433+
final var statelessEnabledSettingsOnNonSearchNode = Settings.builder()
1434+
.put(nodeNameSettings)
1435+
.put(onlyRole(DiscoveryNodeRole.INDEX_ROLE))
1436+
.put(DiscoveryNode.STATELESS_ENABLED_SETTING_NAME, true)
1437+
.build();
1438+
try (RemoteClusterService service = new RemoteClusterService(statelessEnabledSettingsOnNonSearchNode, null)) {
1439+
final var exception = expectThrows(IllegalArgumentException.class, service::ensureClientIsEnabled);
1440+
assertThat(
1441+
exception.getMessage(),
1442+
equalTo(
1443+
"node [node-1] must have the [remote_cluster_client] role or the [search] role "
1444+
+ "in stateless environments to use linked project client features"
1445+
)
1446+
);
1447+
}
1448+
1449+
// Shouldn't throw when stateless is enabled on a search node, or a node with remote cluster client role, or both.
1450+
final var statelessEnabledOnSearchNodeSettings = Settings.builder()
1451+
.put(nodeNameSettings)
1452+
.put(onlyRole(DiscoveryNodeRole.SEARCH_ROLE))
1453+
.put(DiscoveryNode.STATELESS_ENABLED_SETTING_NAME, true)
1454+
.build();
1455+
try (RemoteClusterService service = new RemoteClusterService(statelessEnabledOnSearchNodeSettings, null)) {
1456+
service.ensureClientIsEnabled();
1457+
}
1458+
final var statelessEnabledOnRemoteClusterClientSettings = Settings.builder()
1459+
.put(nodeNameSettings)
1460+
.put(onlyRole(DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE))
1461+
.put(DiscoveryNode.STATELESS_ENABLED_SETTING_NAME, true)
1462+
.build();
1463+
try (RemoteClusterService service = new RemoteClusterService(statelessEnabledOnRemoteClusterClientSettings, null)) {
1464+
service.ensureClientIsEnabled();
1465+
}
1466+
final var statelessEnabledOnSearchNodeAndRemoteClusterClientSettings = Settings.builder()
1467+
.put(nodeNameSettings)
1468+
.put(onlyRoles(Set.of(DiscoveryNodeRole.SEARCH_ROLE, DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE)))
1469+
.put(DiscoveryNode.STATELESS_ENABLED_SETTING_NAME, true)
1470+
.build();
1471+
try (RemoteClusterService service = new RemoteClusterService(statelessEnabledOnSearchNodeAndRemoteClusterClientSettings, null)) {
1472+
service.ensureClientIsEnabled();
14031473
}
14041474
}
14051475

14061476
public void testRemoteClusterServiceNotEnabledGetCollectNodes() {
1407-
final Settings settings = removeRoles(Set.of(DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE));
1477+
final Settings settings = Settings.builder()
1478+
.put(removeRoles(Set.of(DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE)))
1479+
.put(Node.NODE_NAME_SETTING.getKey(), "node-1")
1480+
.build();
14081481
try (
14091482
MockTransportService service = MockTransportService.createNewService(
14101483
settings,
@@ -1420,7 +1493,7 @@ public void testRemoteClusterServiceNotEnabledGetCollectNodes() {
14201493
IllegalArgumentException.class,
14211494
() -> service.getRemoteClusterService().collectNodes(Set.of(), ActionListener.noop())
14221495
);
1423-
assertThat(e.getMessage(), equalTo("this node does not have the remote_cluster_client role"));
1496+
assertThat(e.getMessage(), equalTo("node [node-1] does not have the [remote_cluster_client] role"));
14241497
}
14251498
}
14261499

0 commit comments

Comments
 (0)