[Transform] Remove node.attr.transform.remote_connect and use new remote cluster client node role (#54217)

With the addition of a formal role for nodes indicating remote cluster connection, the transform specific attribute `node.attr.transform.remote_connect` is no longer necessary.

closes https://github.com/elastic/elasticsearch/issues/54179

x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/Transform.java

@@ -32,7 +32,6 @@ import org.elasticsearch.env.Environment;
 import org.elasticsearch.env.NodeEnvironment;
 import org.elasticsearch.indices.SystemIndexDescriptor;
 import org.elasticsearch.license.XPackLicenseState;
-import org.elasticsearch.node.Node;
 import org.elasticsearch.persistent.PersistentTasksExecutor;
 import org.elasticsearch.plugins.PersistentTaskPlugin;
 import org.elasticsearch.plugins.Plugin;
@@ -148,7 +147,6 @@ public class Transform extends Plugin implements SystemIndexPlugin, PersistentTa
      * These attributes should never be set directly, use the node setting counter parts instead.
      */
     public static final String TRANSFORM_ENABLED_NODE_ATTR = "transform.node";
-    public static final String TRANSFORM_REMOTE_ENABLED_NODE_ATTR = "transform.remote_connect";
 
     /**
      * Setting whether transform (the coordinator task) can run on this node and REST API's are available,
@@ -355,9 +353,8 @@ public class Transform extends Plugin implements SystemIndexPlugin, PersistentTa
     @Override
     public Settings additionalSettings() {
         String transformEnabledNodeAttribute = "node.attr." + TRANSFORM_ENABLED_NODE_ATTR;
-        String transformRemoteEnabledNodeAttribute = "node.attr." + TRANSFORM_REMOTE_ENABLED_NODE_ATTR;
 
-        if (settings.get(transformEnabledNodeAttribute) != null || settings.get(transformRemoteEnabledNodeAttribute) != null) {
+        if (settings.get(transformEnabledNodeAttribute) != null) {
             throw new IllegalArgumentException(
                 "Directly setting transform node attributes is not permitted, please use the documented node settings instead"
             );
@@ -370,7 +367,6 @@ public class Transform extends Plugin implements SystemIndexPlugin, PersistentTa
         Settings.Builder additionalSettings = Settings.builder();
 
         additionalSettings.put(transformEnabledNodeAttribute, TRANSFORM_ENABLED_NODE.get(settings));
-        additionalSettings.put(transformRemoteEnabledNodeAttribute, Node.NODE_REMOTE_CLUSTER_CLIENT.get(settings));
 
         return additionalSettings.build();
     }

x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/transforms/TransformPersistentTasksExecutor.java

@@ -146,7 +146,7 @@ public class TransformPersistentTasksExecutor extends PersistentTasksExecutor<Tr
         }
 
         // does the transform require a remote and remote is enabled?
-        if (params.requiresRemote() && Boolean.parseBoolean(nodeAttributes.get(Transform.TRANSFORM_REMOTE_ENABLED_NODE_ATTR)) == false) {
+        if (params.requiresRemote() && node.isRemoteClusterClient() == false) {
             if (explain != null) {
                 explain.put(node.getId(), "transform requires a remote connection but remote is disabled");
             }

x-pack/plugin/transform/src/test/java/org/elasticsearch/xpack/transform/TransformTests.java

@@ -19,18 +19,12 @@ public class TransformTests extends ESTestCase {
         Settings.Builder builder = Settings.builder();
         boolean transformEnabled = randomBoolean();
         boolean transformPluginEnabled = randomBoolean();
-        boolean remoteClusterClient = randomBoolean();
 
         // randomly use explicit or default setting
         if ((transformEnabled && randomBoolean()) == false) {
             builder.put("node.transform", transformEnabled);
         }
 
-        // randomly use explicit or default setting
-        if ((remoteClusterClient && randomBoolean()) == false) {
-            builder.put("node.remote_cluster_client", remoteClusterClient);
-        }
-
         if (transformPluginEnabled == false) {
             builder.put("xpack.transform.enabled", transformPluginEnabled);
         }
@@ -42,23 +36,14 @@ public class TransformTests extends ESTestCase {
             transformPluginEnabled && transformEnabled,
             Boolean.parseBoolean(transform.additionalSettings().get("node.attr.transform.node"))
         );
-        assertEquals(
-            transformPluginEnabled && remoteClusterClient,
-            Boolean.parseBoolean(transform.additionalSettings().get("node.attr.transform.remote_connect"))
-        );
     }
 
     public void testNodeAttributesDirectlyGiven() {
         Settings.Builder builder = Settings.builder();
-
-        if (randomBoolean()) {
-            builder.put("node.attr.transform.node", randomBoolean());
-        } else {
-            builder.put("node.attr.transform.remote_connect", randomBoolean());
-        }
+        builder.put("node.attr.transform.node", randomBoolean());
 
         Transform transform = createTransform(builder.build());
-        IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> transform.additionalSettings());
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class, transform::additionalSettings);
         assertThat(
             e.getMessage(),
             equalTo("Directly setting transform node attributes is not permitted, please use the documented node settings instead")

x-pack/plugin/transform/src/test/java/org/elasticsearch/xpack/transform/transforms/TransformPersistentTasksExecutorTests.java

@@ -243,19 +243,14 @@ public class TransformPersistentTasksExecutorTests extends ESTestCase {
         boolean dedicatedTransformNode,
         boolean pastDataNode,
         boolean transformRemoteNodes,
-        boolean transformLocanOnlyNodes,
+        boolean transformLocalOnlyNodes,
         boolean currentDataNode
     ) {
 
         Map<String, String> transformNodeAttributes = new HashMap<>();
         transformNodeAttributes.put(Transform.TRANSFORM_ENABLED_NODE_ATTR, "true");
-        transformNodeAttributes.put(Transform.TRANSFORM_REMOTE_ENABLED_NODE_ATTR, "true");
         Map<String, String> transformNodeAttributesDisabled = new HashMap<>();
         transformNodeAttributesDisabled.put(Transform.TRANSFORM_ENABLED_NODE_ATTR, "false");
-        transformNodeAttributesDisabled.put(Transform.TRANSFORM_REMOTE_ENABLED_NODE_ATTR, "true");
-        Map<String, String> transformNodeAttributesNoRemote = new HashMap<>();
-        transformNodeAttributesNoRemote.put(Transform.TRANSFORM_ENABLED_NODE_ATTR, "true");
-        transformNodeAttributesNoRemote.put(Transform.TRANSFORM_REMOTE_ENABLED_NODE_ATTR, "false");
 
         DiscoveryNodes.Builder nodes = DiscoveryNodes.builder();
 
@@ -265,7 +260,7 @@ public class TransformPersistentTasksExecutorTests extends ESTestCase {
                     "dedicated-transform-node",
                     buildNewFakeTransportAddress(),
                     transformNodeAttributes,
-                    Collections.singleton(DiscoveryNodeRole.MASTER_ROLE),
+                    new HashSet<>(Arrays.asList(DiscoveryNodeRole.MASTER_ROLE, DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE)),
                     Version.CURRENT
                 )
             );
@@ -277,7 +272,9 @@ public class TransformPersistentTasksExecutorTests extends ESTestCase {
                     "past-data-node-1",
                     buildNewFakeTransportAddress(),
                     transformNodeAttributes,
-                    new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.MASTER_ROLE)),
+                    new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE,
+                        DiscoveryNodeRole.MASTER_ROLE,
+                        DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE)),
                     Version.V_7_7_0
                 )
             );
@@ -289,7 +286,7 @@ public class TransformPersistentTasksExecutorTests extends ESTestCase {
                     "current-data-node-with-2-tasks",
                     buildNewFakeTransportAddress(),
                     transformNodeAttributes,
-                    new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE)),
+                    new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE)),
                     Version.CURRENT
                 )
             )
@@ -298,18 +295,18 @@ public class TransformPersistentTasksExecutorTests extends ESTestCase {
                         "current-data-node-with-1-tasks",
                         buildNewFakeTransportAddress(),
                         transformNodeAttributes,
-                        new HashSet<>(Arrays.asList(DiscoveryNodeRole.MASTER_ROLE)),
+                        new HashSet<>(Arrays.asList(DiscoveryNodeRole.MASTER_ROLE, DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE)),
                         Version.CURRENT
                     )
                 );
         }
 
-        if (transformLocanOnlyNodes) {
+        if (transformLocalOnlyNodes) {
             nodes.add(
                 new DiscoveryNode(
                     "current-data-node-with-0-tasks-transform-remote-disabled",
                     buildNewFakeTransportAddress(),
-                    transformNodeAttributesNoRemote,
+                    transformNodeAttributes,
                     new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.MASTER_ROLE)),
                     Version.CURRENT
                 )
@@ -322,7 +319,7 @@ public class TransformPersistentTasksExecutorTests extends ESTestCase {
                     "current-data-node-with-transform-disabled",
                     buildNewFakeTransportAddress(),
                     transformNodeAttributesDisabled,
-                    Set.of(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.MASTER_ROLE),
+                    Set.of(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.MASTER_ROLE, DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE),
                     Version.CURRENT
                 )
             );