Make GetTermVersionAction internal (#12866)

* Make GetTermVersionAction internal GetTermVersion action has been added to check if cluster state needs to be updated as a performance improvement on all cluster state checks. The Authorization systems in the Security Plugin check all actions that do not start with `internal:` for permissions causing users without `cluster:monitor/*` permissions to start getting 403 exceptions. This change signals that this action does not require an authorization check by any security systems since it cannot be called via REST APIs. Signed-off-by: Peter Nied <[email protected]> * Move TermVerson action outside of admin/cluster namespace Signed-off-by: Peter Nied <[email protected]> --------- Signed-off-by: Peter Nied <[email protected]>
opensearch-project · Mar 23, 2024 · 5b4b4aa · 5b4b4aa
1 parent 8e332b6
commit 5b4b4aa
Show file tree

Hide file tree

Showing 12 changed files with 19 additions and 21 deletions.
diff --git a/...h/cluster/state/FetchByTermVersionIT.java → ...termanager/term/FetchByTermVersionIT.java b/...h/cluster/state/FetchByTermVersionIT.java → ...termanager/term/FetchByTermVersionIT.java
@@ -6,12 +6,10 @@
  * compatible open source license.
  */
 
-package org.opensearch.cluster.state;
+package org.opensearch.action.support.clustermanager.term;
 
 import org.opensearch.action.admin.cluster.state.ClusterStateRequest;
 import org.opensearch.action.admin.cluster.state.ClusterStateResponse;
-import org.opensearch.action.admin.cluster.state.term.GetTermVersionAction;
-import org.opensearch.action.admin.cluster.state.term.GetTermVersionResponse;
 import org.opensearch.action.admin.indices.mapping.put.PutMappingRequest;
 import org.opensearch.cluster.ClusterName;
 import org.opensearch.cluster.coordination.ClusterStateTermVersion;

diff --git a/server/src/main/java/org/opensearch/action/ActionModule.java b/server/src/main/java/org/opensearch/action/ActionModule.java
@@ -107,8 +107,6 @@
 import org.opensearch.action.admin.cluster.snapshots.status.TransportSnapshotsStatusAction;
 import org.opensearch.action.admin.cluster.state.ClusterStateAction;
 import org.opensearch.action.admin.cluster.state.TransportClusterStateAction;
-import org.opensearch.action.admin.cluster.state.term.GetTermVersionAction;
-import org.opensearch.action.admin.cluster.state.term.TransportGetTermVersionAction;
 import org.opensearch.action.admin.cluster.stats.ClusterStatsAction;
 import org.opensearch.action.admin.cluster.stats.TransportClusterStatsAction;
 import org.opensearch.action.admin.cluster.storedscripts.DeleteStoredScriptAction;
@@ -283,6 +281,8 @@
 import org.opensearch.action.support.AutoCreateIndex;
 import org.opensearch.action.support.DestructiveOperations;
 import org.opensearch.action.support.TransportAction;
+import org.opensearch.action.support.clustermanager.term.GetTermVersionAction;
+import org.opensearch.action.support.clustermanager.term.TransportGetTermVersionAction;
 import org.opensearch.action.termvectors.MultiTermVectorsAction;
 import org.opensearch.action.termvectors.TermVectorsAction;
 import org.opensearch.action.termvectors.TransportMultiTermVectorsAction;

diff --git a/.../java/org/opensearch/action/support/clustermanager/TransportClusterManagerNodeAction.java b/.../java/org/opensearch/action/support/clustermanager/TransportClusterManagerNodeAction.java
@@ -37,13 +37,13 @@
 import org.apache.logging.log4j.message.ParameterizedMessage;
 import org.opensearch.action.ActionListenerResponseHandler;
 import org.opensearch.action.ActionRunnable;
-import org.opensearch.action.admin.cluster.state.term.GetTermVersionAction;
-import org.opensearch.action.admin.cluster.state.term.GetTermVersionRequest;
-import org.opensearch.action.admin.cluster.state.term.GetTermVersionResponse;
 import org.opensearch.action.bulk.BackoffPolicy;
 import org.opensearch.action.support.ActionFilters;
 import org.opensearch.action.support.HandledTransportAction;
 import org.opensearch.action.support.RetryableAction;
+import org.opensearch.action.support.clustermanager.term.GetTermVersionAction;
+import org.opensearch.action.support.clustermanager.term.GetTermVersionRequest;
+import org.opensearch.action.support.clustermanager.term.GetTermVersionResponse;
 import org.opensearch.cluster.ClusterManagerNodeChangePredicate;
 import org.opensearch.cluster.ClusterState;
 import org.opensearch.cluster.ClusterStateObserver;

diff --git a/...ster/state/term/GetTermVersionAction.java → ...termanager/term/GetTermVersionAction.java b/...ster/state/term/GetTermVersionAction.java → ...termanager/term/GetTermVersionAction.java
@@ -6,7 +6,7 @@
  * compatible open source license.
  */
 
-package org.opensearch.action.admin.cluster.state.term;
+package org.opensearch.action.support.clustermanager.term;
 
 import org.opensearch.action.ActionType;
 
@@ -18,7 +18,7 @@
 public class GetTermVersionAction extends ActionType<GetTermVersionResponse> {
 
     public static final GetTermVersionAction INSTANCE = new GetTermVersionAction();
-    public static final String NAME = "cluster:monitor/term";
+    public static final String NAME = "internal:monitor/term";
 
     private GetTermVersionAction() {
         super(NAME, GetTermVersionResponse::new);

diff --git a/...ter/state/term/GetTermVersionRequest.java → ...ermanager/term/GetTermVersionRequest.java b/...ter/state/term/GetTermVersionRequest.java → ...ermanager/term/GetTermVersionRequest.java
@@ -6,7 +6,7 @@
  * compatible open source license.
  */
 
-package org.opensearch.action.admin.cluster.state.term;
+package org.opensearch.action.support.clustermanager.term;
 
 import org.opensearch.action.ActionRequestValidationException;
 import org.opensearch.action.support.clustermanager.ClusterManagerNodeReadRequest;

diff --git a/...er/state/term/GetTermVersionResponse.java → ...rmanager/term/GetTermVersionResponse.java b/...er/state/term/GetTermVersionResponse.java → ...rmanager/term/GetTermVersionResponse.java
@@ -6,7 +6,7 @@
  * compatible open source license.
  */
 
-package org.opensearch.action.admin.cluster.state.term;
+package org.opensearch.action.support.clustermanager.term;
 
 import org.opensearch.cluster.ClusterState;
 import org.opensearch.cluster.coordination.ClusterStateTermVersion;

diff --git a/...e/term/TransportGetTermVersionAction.java → ...r/term/TransportGetTermVersionAction.java b/...e/term/TransportGetTermVersionAction.java → ...r/term/TransportGetTermVersionAction.java
@@ -6,7 +6,7 @@
  * compatible open source license.
  */
 
-package org.opensearch.action.admin.cluster.state.term;
+package org.opensearch.action.support.clustermanager.term;
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;

diff --git a/...dmin/cluster/state/term/package-info.java → ...ort/clustermanager/term/package-info.java b/...dmin/cluster/state/term/package-info.java → ...ort/clustermanager/term/package-info.java
@@ -7,4 +7,4 @@
  */
 
 /** Cluster Term transport handler. */
-package org.opensearch.action.admin.cluster.state.term;
+package org.opensearch.action.support.clustermanager.term;
diff --git a/...a/org/opensearch/action/support/clustermanager/TransportClusterManagerTermCheckTests.java b/...a/org/opensearch/action/support/clustermanager/TransportClusterManagerTermCheckTests.java
@@ -33,10 +33,10 @@
 
 import org.opensearch.Version;
 import org.opensearch.action.ActionRequestValidationException;
-import org.opensearch.action.admin.cluster.state.term.GetTermVersionResponse;
 import org.opensearch.action.support.ActionFilters;
 import org.opensearch.action.support.PlainActionFuture;
 import org.opensearch.action.support.ThreadedActionListener;
+import org.opensearch.action.support.clustermanager.term.GetTermVersionResponse;
 import org.opensearch.action.support.replication.ClusterStateCreationUtils;
 import org.opensearch.cluster.ClusterState;
 import org.opensearch.cluster.block.ClusterBlockException;
@@ -223,7 +223,7 @@ public void testTermCheckMatchWithClusterManager() throws ExecutionException, In
         assertThat(transport.capturedRequests().length, equalTo(1));
         CapturingTransport.CapturedRequest capturedRequest = transport.capturedRequests()[0];
         assertTrue(capturedRequest.node.isClusterManagerNode());
-        assertThat(capturedRequest.action, equalTo("cluster:monitor/term"));
+        assertThat(capturedRequest.action, equalTo("internal:monitor/term"));
         GetTermVersionResponse response = new GetTermVersionResponse(
             new ClusterStateTermVersion(
                 clusterService.state().getClusterName(),
@@ -249,7 +249,7 @@ public void testTermCheckNoMatchWithClusterManager() throws ExecutionException,
         assertThat(transport.capturedRequests().length, equalTo(1));
         CapturingTransport.CapturedRequest termCheckRequest = transport.capturedRequests()[0];
         assertTrue(termCheckRequest.node.isClusterManagerNode());
-        assertThat(termCheckRequest.action, equalTo("cluster:monitor/term"));
+        assertThat(termCheckRequest.action, equalTo("internal:monitor/term"));
         GetTermVersionResponse noMatchResponse = new GetTermVersionResponse(
             new ClusterStateTermVersion(
                 clusterService.state().getClusterName(),

diff --git a/...ster/state/term/ClusterTermVersionIT.java → ...termanager/term/ClusterTermVersionIT.java b/...ster/state/term/ClusterTermVersionIT.java → ...termanager/term/ClusterTermVersionIT.java
@@ -6,7 +6,7 @@
  * compatible open source license.
  */
 
-package org.opensearch.action.admin.cluster.state.term;
+package org.opensearch.action.support.clustermanager.term;
 
 import org.opensearch.action.admin.cluster.state.ClusterStateAction;
 import org.opensearch.action.admin.cluster.state.ClusterStateRequest;

diff --git a/...r/state/term/ClusterTermVersionTests.java → ...manager/term/ClusterTermVersionTests.java b/...r/state/term/ClusterTermVersionTests.java → ...manager/term/ClusterTermVersionTests.java
@@ -6,7 +6,7 @@
  * compatible open source license.
  */
 
-package org.opensearch.action.admin.cluster.state.term;
+package org.opensearch.action.support.clustermanager.term;
 
 import org.opensearch.cluster.service.ClusterService;
 import org.opensearch.test.OpenSearchSingleNodeTestCase;

diff --git a/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java b/server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java
@@ -66,8 +66,6 @@
 import org.opensearch.action.admin.cluster.state.ClusterStateRequest;
 import org.opensearch.action.admin.cluster.state.ClusterStateResponse;
 import org.opensearch.action.admin.cluster.state.TransportClusterStateAction;
-import org.opensearch.action.admin.cluster.state.term.GetTermVersionAction;
-import org.opensearch.action.admin.cluster.state.term.TransportGetTermVersionAction;
 import org.opensearch.action.admin.indices.create.CreateIndexAction;
 import org.opensearch.action.admin.indices.create.CreateIndexRequest;
 import org.opensearch.action.admin.indices.create.CreateIndexResponse;
@@ -105,6 +103,8 @@
 import org.opensearch.action.support.PlainActionFuture;
 import org.opensearch.action.support.TransportAction;
 import org.opensearch.action.support.WriteRequest;
+import org.opensearch.action.support.clustermanager.term.GetTermVersionAction;
+import org.opensearch.action.support.clustermanager.term.TransportGetTermVersionAction;
 import org.opensearch.action.support.master.AcknowledgedResponse;
 import org.opensearch.action.update.UpdateHelper;
 import org.opensearch.client.AdminClient;