From f1cd937b7c121ea22ea7a7fe26f726cf8a218c03 Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Fri, 22 Mar 2024 16:36:36 +0000 Subject: [PATCH 1/2] 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 --- .../action/admin/cluster/state/term/GetTermVersionAction.java | 2 +- .../clustermanager/TransportClusterManagerTermCheckTests.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/state/term/GetTermVersionAction.java b/server/src/main/java/org/opensearch/action/admin/cluster/state/term/GetTermVersionAction.java index 3344fd549b23f..cd3617f939673 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/state/term/GetTermVersionAction.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/state/term/GetTermVersionAction.java @@ -18,7 +18,7 @@ public class GetTermVersionAction extends ActionType { 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/server/src/test/java/org/opensearch/action/support/clustermanager/TransportClusterManagerTermCheckTests.java b/server/src/test/java/org/opensearch/action/support/clustermanager/TransportClusterManagerTermCheckTests.java index 8c7b7a0940c82..40d19dbab84b7 100644 --- a/server/src/test/java/org/opensearch/action/support/clustermanager/TransportClusterManagerTermCheckTests.java +++ b/server/src/test/java/org/opensearch/action/support/clustermanager/TransportClusterManagerTermCheckTests.java @@ -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(), From f4c2e6ce7751a7c02f3b89f920fbc597a7c09397 Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Fri, 22 Mar 2024 18:01:29 +0000 Subject: [PATCH 2/2] Move TermVerson action outside of admin/cluster namespace Signed-off-by: Peter Nied --- .../support/clustermanager/term}/FetchByTermVersionIT.java | 4 +--- .../src/main/java/org/opensearch/action/ActionModule.java | 4 ++-- .../clustermanager/TransportClusterManagerNodeAction.java | 6 +++--- .../clustermanager}/term/GetTermVersionAction.java | 2 +- .../clustermanager}/term/GetTermVersionRequest.java | 2 +- .../clustermanager}/term/GetTermVersionResponse.java | 2 +- .../clustermanager}/term/TransportGetTermVersionAction.java | 2 +- .../state => support/clustermanager}/term/package-info.java | 2 +- .../TransportClusterManagerTermCheckTests.java | 2 +- .../clustermanager}/term/ClusterTermVersionIT.java | 2 +- .../clustermanager}/term/ClusterTermVersionTests.java | 2 +- .../org/opensearch/snapshots/SnapshotResiliencyTests.java | 4 ++-- 12 files changed, 16 insertions(+), 18 deletions(-) rename server/src/internalClusterTest/java/org/opensearch/{cluster/state => action/support/clustermanager/term}/FetchByTermVersionIT.java (97%) rename server/src/main/java/org/opensearch/action/{admin/cluster/state => support/clustermanager}/term/GetTermVersionAction.java (91%) rename server/src/main/java/org/opensearch/action/{admin/cluster/state => support/clustermanager}/term/GetTermVersionRequest.java (93%) rename server/src/main/java/org/opensearch/action/{admin/cluster/state => support/clustermanager}/term/GetTermVersionResponse.java (96%) rename server/src/main/java/org/opensearch/action/{admin/cluster/state => support/clustermanager}/term/TransportGetTermVersionAction.java (97%) rename server/src/main/java/org/opensearch/action/{admin/cluster/state => support/clustermanager}/term/package-info.java (80%) rename server/src/test/java/org/opensearch/action/{admin/cluster/state => support/clustermanager}/term/ClusterTermVersionIT.java (98%) rename server/src/test/java/org/opensearch/action/{admin/cluster/state => support/clustermanager}/term/ClusterTermVersionTests.java (93%) diff --git a/server/src/internalClusterTest/java/org/opensearch/cluster/state/FetchByTermVersionIT.java b/server/src/internalClusterTest/java/org/opensearch/action/support/clustermanager/term/FetchByTermVersionIT.java similarity index 97% rename from server/src/internalClusterTest/java/org/opensearch/cluster/state/FetchByTermVersionIT.java rename to server/src/internalClusterTest/java/org/opensearch/action/support/clustermanager/term/FetchByTermVersionIT.java index cef184b3fddf9..72dcc98dcdc12 100644 --- a/server/src/internalClusterTest/java/org/opensearch/cluster/state/FetchByTermVersionIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/action/support/clustermanager/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 index f827b7f3f0097..7eea1cca10a0c 100644 --- 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/server/src/main/java/org/opensearch/action/support/clustermanager/TransportClusterManagerNodeAction.java b/server/src/main/java/org/opensearch/action/support/clustermanager/TransportClusterManagerNodeAction.java index 5f57658e33924..080b0d607e991 100644 --- a/server/src/main/java/org/opensearch/action/support/clustermanager/TransportClusterManagerNodeAction.java +++ b/server/src/main/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/server/src/main/java/org/opensearch/action/admin/cluster/state/term/GetTermVersionAction.java b/server/src/main/java/org/opensearch/action/support/clustermanager/term/GetTermVersionAction.java similarity index 91% rename from server/src/main/java/org/opensearch/action/admin/cluster/state/term/GetTermVersionAction.java rename to server/src/main/java/org/opensearch/action/support/clustermanager/term/GetTermVersionAction.java index cd3617f939673..2401dddd0cab3 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/state/term/GetTermVersionAction.java +++ b/server/src/main/java/org/opensearch/action/support/clustermanager/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; diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/state/term/GetTermVersionRequest.java b/server/src/main/java/org/opensearch/action/support/clustermanager/term/GetTermVersionRequest.java similarity index 93% rename from server/src/main/java/org/opensearch/action/admin/cluster/state/term/GetTermVersionRequest.java rename to server/src/main/java/org/opensearch/action/support/clustermanager/term/GetTermVersionRequest.java index b099f8087bd15..507997a1f7e7a 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/state/term/GetTermVersionRequest.java +++ b/server/src/main/java/org/opensearch/action/support/clustermanager/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/server/src/main/java/org/opensearch/action/admin/cluster/state/term/GetTermVersionResponse.java b/server/src/main/java/org/opensearch/action/support/clustermanager/term/GetTermVersionResponse.java similarity index 96% rename from server/src/main/java/org/opensearch/action/admin/cluster/state/term/GetTermVersionResponse.java rename to server/src/main/java/org/opensearch/action/support/clustermanager/term/GetTermVersionResponse.java index 16b355a80d1f2..0906abe57d547 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/state/term/GetTermVersionResponse.java +++ b/server/src/main/java/org/opensearch/action/support/clustermanager/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/server/src/main/java/org/opensearch/action/admin/cluster/state/term/TransportGetTermVersionAction.java b/server/src/main/java/org/opensearch/action/support/clustermanager/term/TransportGetTermVersionAction.java similarity index 97% rename from server/src/main/java/org/opensearch/action/admin/cluster/state/term/TransportGetTermVersionAction.java rename to server/src/main/java/org/opensearch/action/support/clustermanager/term/TransportGetTermVersionAction.java index 88305252aa99c..4752a99c910e4 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/state/term/TransportGetTermVersionAction.java +++ b/server/src/main/java/org/opensearch/action/support/clustermanager/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/server/src/main/java/org/opensearch/action/admin/cluster/state/term/package-info.java b/server/src/main/java/org/opensearch/action/support/clustermanager/term/package-info.java similarity index 80% rename from server/src/main/java/org/opensearch/action/admin/cluster/state/term/package-info.java rename to server/src/main/java/org/opensearch/action/support/clustermanager/term/package-info.java index 0ee559c527d7d..229c405df2d7c 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/state/term/package-info.java +++ b/server/src/main/java/org/opensearch/action/support/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/server/src/test/java/org/opensearch/action/support/clustermanager/TransportClusterManagerTermCheckTests.java b/server/src/test/java/org/opensearch/action/support/clustermanager/TransportClusterManagerTermCheckTests.java index 40d19dbab84b7..6d118cbccb42d 100644 --- a/server/src/test/java/org/opensearch/action/support/clustermanager/TransportClusterManagerTermCheckTests.java +++ b/server/src/test/java/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; diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/state/term/ClusterTermVersionIT.java b/server/src/test/java/org/opensearch/action/support/clustermanager/term/ClusterTermVersionIT.java similarity index 98% rename from server/src/test/java/org/opensearch/action/admin/cluster/state/term/ClusterTermVersionIT.java rename to server/src/test/java/org/opensearch/action/support/clustermanager/term/ClusterTermVersionIT.java index fa2a6121af349..7b783e025a575 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/state/term/ClusterTermVersionIT.java +++ b/server/src/test/java/org/opensearch/action/support/clustermanager/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/server/src/test/java/org/opensearch/action/admin/cluster/state/term/ClusterTermVersionTests.java b/server/src/test/java/org/opensearch/action/support/clustermanager/term/ClusterTermVersionTests.java similarity index 93% rename from server/src/test/java/org/opensearch/action/admin/cluster/state/term/ClusterTermVersionTests.java rename to server/src/test/java/org/opensearch/action/support/clustermanager/term/ClusterTermVersionTests.java index 22d9623eebdbe..23ae8c6a58776 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/state/term/ClusterTermVersionTests.java +++ b/server/src/test/java/org/opensearch/action/support/clustermanager/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 index 58315ba031b84..e9bbf3d861408 100644 --- 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;