From 66f9ebe9b09dfa4eab2780e8ea249f86a844e0bd Mon Sep 17 00:00:00 2001 From: Maciej Mierzwa Date: Tue, 14 Nov 2023 21:25:18 +0100 Subject: [PATCH 1/3] shrink operation fix + tests Signed-off-by: Maciej Mierzwa --- .../security/SearchOperationTest.java | 63 +++++++++++-------- .../resolver/IndexResolverReplacer.java | 5 ++ 2 files changed, 42 insertions(+), 26 deletions(-) diff --git a/src/integrationTest/java/org/opensearch/security/SearchOperationTest.java b/src/integrationTest/java/org/opensearch/security/SearchOperationTest.java index e39eeeca61..36939ebd2b 100644 --- a/src/integrationTest/java/org/opensearch/security/SearchOperationTest.java +++ b/src/integrationTest/java/org/opensearch/security/SearchOperationTest.java @@ -28,6 +28,8 @@ import org.junit.Test; import org.junit.runner.RunWith; +import org.opensearch.action.admin.cluster.health.ClusterHealthRequest; +import org.opensearch.action.admin.cluster.health.ClusterHealthResponse; import org.opensearch.action.admin.cluster.repositories.delete.DeleteRepositoryRequest; import org.opensearch.action.admin.cluster.repositories.put.PutRepositoryRequest; import org.opensearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse; @@ -83,6 +85,7 @@ import org.opensearch.client.indices.PutMappingRequest; import org.opensearch.client.indices.ResizeRequest; import org.opensearch.client.indices.ResizeResponse; +import org.opensearch.cluster.health.ClusterHealthStatus; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.metadata.IndexTemplateMetadata; import org.opensearch.common.settings.Settings; @@ -335,22 +338,24 @@ public class SearchOperationTest { * indices with names prefixed by the {@link #INDICES_ON_WHICH_USER_CAN_PERFORM_INDEX_OPERATIONS_PREFIX} */ static final User USER_ALLOWED_TO_PERFORM_INDEX_OPERATIONS_ON_SELECTED_INDICES = new User("index-operation-tester").roles( - new Role("index-manager").indexPermissions( - "indices:admin/create", - "indices:admin/get", - "indices:admin/delete", - "indices:admin/close", - "indices:admin/close*", - "indices:admin/open", - "indices:admin/resize", - "indices:monitor/stats", - "indices:monitor/settings/get", - "indices:admin/settings/update", - "indices:admin/mapping/put", - "indices:admin/mappings/get", - "indices:admin/cache/clear", - "indices:admin/aliases" - ).on(INDICES_ON_WHICH_USER_CAN_PERFORM_INDEX_OPERATIONS_PREFIX.concat("*")) + new Role("index-manager").clusterPermissions("cluster:monitor/health") + .indexPermissions( + "indices:admin/create", + "indices:admin/get", + "indices:admin/delete", + "indices:admin/close", + "indices:admin/close*", + "indices:admin/open", + "indices:admin/resize", + "indices:monitor/stats", + "indices:monitor/settings/get", + "indices:admin/settings/update", + "indices:admin/mapping/put", + "indices:admin/mappings/get", + "indices:admin/cache/clear", + "indices:admin/aliases" + ) + .on(INDICES_ON_WHICH_USER_CAN_PERFORM_INDEX_OPERATIONS_PREFIX.concat("*")) ); private static User USER_ALLOWED_TO_CREATE_INDEX = new User("user-allowed-to-create-index").roles( @@ -2272,14 +2277,15 @@ public void openIndex_negative() throws IOException { } @Test - @Ignore // required permissions: "indices:admin/resize", "indices:monitor/stats - // todo even when I assign the `indices:admin/resize` and `indices:monitor/stats` permissions to test user, this test fails. - // Issue: https://github.com/opensearch-project/security/issues/2141 public void shrinkIndex_positive() throws IOException { String sourceIndexName = INDICES_ON_WHICH_USER_CAN_PERFORM_INDEX_OPERATIONS_PREFIX.concat("shrink_index_positive_source"); - Settings sourceIndexSettings = Settings.builder().put("index.blocks.write", true).put("index.number_of_shards", 2).build(); String targetIndexName = INDICES_ON_WHICH_USER_CAN_PERFORM_INDEX_OPERATIONS_PREFIX.concat("shrink_index_positive_target"); + Settings sourceIndexSettings = Settings.builder() + .put("index.number_of_replicas", 1) + .put("index.blocks.write", true) + .put("index.number_of_shards", 4) + .build(); IndexOperationsHelper.createIndex(cluster, sourceIndexName, sourceIndexSettings); try ( @@ -2287,6 +2293,17 @@ public void shrinkIndex_positive() throws IOException { USER_ALLOWED_TO_PERFORM_INDEX_OPERATIONS_ON_SELECTED_INDICES ) ) { + ClusterHealthResponse healthResponse = restHighLevelClient.cluster() + .health( + new ClusterHealthRequest(sourceIndexName).waitForNoRelocatingShards(true) + .waitForActiveShards(4) + .waitForNoInitializingShards(true) + .waitForGreenStatus(), + DEFAULT + ); + + assertThat(healthResponse.getStatus(), is(ClusterHealthStatus.GREEN)); + ResizeRequest resizeRequest = new ResizeRequest(targetIndexName, sourceIndexName); ResizeResponse response = restHighLevelClient.indices().shrink(resizeRequest, DEFAULT); @@ -2329,10 +2346,7 @@ public void shrinkIndex_negative() throws IOException { } @Test - @Ignore // required permissions: "indices:admin/resize", "indices:monitor/stats - // todo even when I assign the `indices:admin/resize` and `indices:monitor/stats` permissions to test user, this test fails. - // Issue: https://github.com/opensearch-project/security/issues/2141 public void cloneIndex_positive() throws IOException { String sourceIndexName = INDICES_ON_WHICH_USER_CAN_PERFORM_INDEX_OPERATIONS_PREFIX.concat("clone_index_positive_source"); Settings sourceIndexSettings = Settings.builder().put("index.blocks.write", true).build(); @@ -2386,10 +2400,7 @@ public void cloneIndex_negative() throws IOException { } @Test - @Ignore // required permissions: "indices:admin/resize", "indices:monitor/stats - // todo even when I assign the `indices:admin/resize` and `indices:monitor/stats` permissions to test user, this test fails. - // Issue: https://github.com/opensearch-project/security/issues/2141 public void splitIndex_positive() throws IOException { String sourceIndexName = INDICES_ON_WHICH_USER_CAN_PERFORM_INDEX_OPERATIONS_PREFIX.concat("split_index_positive_source"); Settings sourceIndexSettings = Settings.builder().put("index.blocks.write", true).build(); diff --git a/src/main/java/org/opensearch/security/resolver/IndexResolverReplacer.java b/src/main/java/org/opensearch/security/resolver/IndexResolverReplacer.java index 3ebfbce29b..d7aeb776c7 100644 --- a/src/main/java/org/opensearch/security/resolver/IndexResolverReplacer.java +++ b/src/main/java/org/opensearch/security/resolver/IndexResolverReplacer.java @@ -56,6 +56,7 @@ import org.opensearch.action.admin.indices.datastream.CreateDataStreamAction; import org.opensearch.action.admin.indices.mapping.put.PutMappingRequest; import org.opensearch.action.admin.indices.resolve.ResolveIndexAction; +import org.opensearch.action.admin.indices.shrink.ResizeRequest; import org.opensearch.action.admin.indices.template.put.PutComponentTemplateAction; import org.opensearch.action.bulk.BulkRequest; import org.opensearch.action.bulk.BulkShardRequest; @@ -777,6 +778,10 @@ private boolean getOrReplaceAllIndices(final Object request, final IndicesProvid return false; } ((CreateIndexRequest) request).index(newIndices.length != 1 ? null : newIndices[0]); + } else if (request instanceof ResizeRequest) { + // clone or shrink operations + provider.provide(((ResizeRequest) request).indices(), request, true); + provider.provide(((ResizeRequest) request).getTargetIndexRequest().indices(), request, true); } else if (request instanceof CreateDataStreamAction.Request) { provider.provide(((CreateDataStreamAction.Request) request).indices(), request, false); } else if (request instanceof ReindexRequest) { From 7a76d1dca3131a8e6a202aa7839678eaea56a431 Mon Sep 17 00:00:00 2001 From: Maciej Mierzwa Date: Wed, 15 Nov 2023 09:24:39 +0100 Subject: [PATCH 2/3] spotless Signed-off-by: Maciej Mierzwa --- .../java/org/opensearch/security/SearchOperationTest.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/integrationTest/java/org/opensearch/security/SearchOperationTest.java b/src/integrationTest/java/org/opensearch/security/SearchOperationTest.java index 36939ebd2b..652b0dcf10 100644 --- a/src/integrationTest/java/org/opensearch/security/SearchOperationTest.java +++ b/src/integrationTest/java/org/opensearch/security/SearchOperationTest.java @@ -122,6 +122,7 @@ import static org.opensearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE; import static org.opensearch.client.RequestOptions.DEFAULT; import static org.opensearch.core.rest.RestStatus.ACCEPTED; +import static org.opensearch.core.rest.RestStatus.BAD_REQUEST; import static org.opensearch.core.rest.RestStatus.FORBIDDEN; import static org.opensearch.core.rest.RestStatus.INTERNAL_SERVER_ERROR; import static org.opensearch.rest.RestRequest.Method.DELETE; @@ -2363,6 +2364,10 @@ public void cloneIndex_positive() throws IOException { assertThat(response, isSuccessfulResizeResponse(targetIndexName)); assertThat(cluster, indexExists(targetIndexName)); + + // can't clone the same index twice, target already exists + ResizeRequest repeatResizeRequest = new ResizeRequest(targetIndexName, sourceIndexName); + assertThatThrownBy(() -> restHighLevelClient.indices().clone(repeatResizeRequest, DEFAULT), statusException(BAD_REQUEST)); } } From 610ba7cc61928f5baaa31ffa03c5138e0a7be10d Mon Sep 17 00:00:00 2001 From: Maciej Mierzwa Date: Wed, 15 Nov 2023 09:50:06 +0100 Subject: [PATCH 3/3] merge and spotless Signed-off-by: Maciej Mierzwa --- .../java/org/opensearch/security/httpclient/HttpClient.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/opensearch/security/httpclient/HttpClient.java b/src/main/java/org/opensearch/security/httpclient/HttpClient.java index 8c31a5f9c9..43b5107b70 100644 --- a/src/main/java/org/opensearch/security/httpclient/HttpClient.java +++ b/src/main/java/org/opensearch/security/httpclient/HttpClient.java @@ -33,6 +33,7 @@ import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLParameters; +import com.google.common.collect.Lists; import org.apache.hc.client5.http.config.RequestConfig; import org.apache.hc.client5.http.impl.async.HttpAsyncClientBuilder; import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManagerBuilder; @@ -52,6 +53,7 @@ import org.apache.hc.core5.ssl.SSLContexts; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; + import org.opensearch.action.index.IndexRequest; import org.opensearch.action.index.IndexResponse; import org.opensearch.action.support.WriteRequest.RefreshPolicy; @@ -62,8 +64,6 @@ import org.opensearch.client.RestHighLevelClient; import org.opensearch.common.xcontent.XContentType; -import com.google.common.collect.Lists; - public class HttpClient implements Closeable { public static class HttpClientBuilder {