From bca10e0dbaea465e772cac98cae659bbe43ca5e4 Mon Sep 17 00:00:00 2001 From: Mikhail Stepura Date: Tue, 26 May 2026 13:19:04 -0700 Subject: [PATCH 1/2] Scope checkBlock to write index only, skipping non-write alias members Signed-off-by: Mikhail Stepura --- .../rollover/TransportRolloverAction.java | 20 +- .../TransportRolloverActionTests.java | 268 ++++++++++++++++++ 2 files changed, 278 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/opensearch/action/admin/indices/rollover/TransportRolloverAction.java b/server/src/main/java/org/opensearch/action/admin/indices/rollover/TransportRolloverAction.java index dc768df1f0c8b..e2055f0c51733 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/rollover/TransportRolloverAction.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/rollover/TransportRolloverAction.java @@ -46,6 +46,7 @@ import org.opensearch.cluster.ClusterStateUpdateTask; import org.opensearch.cluster.block.ClusterBlockException; import org.opensearch.cluster.block.ClusterBlocks; +import org.opensearch.cluster.metadata.IndexAbstraction; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.metadata.IndexNameExpressionResolver; import org.opensearch.cluster.metadata.Metadata; @@ -64,10 +65,8 @@ import org.opensearch.transport.client.Client; import java.io.IOException; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; @@ -128,15 +127,16 @@ protected RolloverResponse read(StreamInput in) throws IOException { @Override protected ClusterBlockException checkBlock(RolloverRequest request, ClusterState state) { - IndicesOptions indicesOptions = IndicesOptions.fromOptions( - true, - true, - request.indicesOptions().expandWildcardsOpen(), - request.indicesOptions().expandWildcardsClosed() - ); - + IndexAbstraction indexAbstraction = state.metadata().getIndicesLookup().get(request.getRolloverTarget()); + if (indexAbstraction == null) { + return null; + } + IndexMetadata writeIndex = indexAbstraction.getWriteIndex(); + if (writeIndex == null) { + return null; + } return ClusterBlocks.indicesWithRemoteSnapshotBlockedException( - new HashSet<>(Arrays.asList(indexNameExpressionResolver.concreteIndexNames(state, indicesOptions, request))), + Collections.singletonList(writeIndex.getIndex().getName()), state ); } diff --git a/server/src/test/java/org/opensearch/action/admin/indices/rollover/TransportRolloverActionTests.java b/server/src/test/java/org/opensearch/action/admin/indices/rollover/TransportRolloverActionTests.java index 2eccfbafb7529..7a06f31885be6 100644 --- a/server/src/test/java/org/opensearch/action/admin/indices/rollover/TransportRolloverActionTests.java +++ b/server/src/test/java/org/opensearch/action/admin/indices/rollover/TransportRolloverActionTests.java @@ -45,6 +45,9 @@ import org.opensearch.action.support.PlainActionFuture; import org.opensearch.cluster.ClusterName; import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.DataStreamTestHelper; +import org.opensearch.cluster.block.ClusterBlockException; +import org.opensearch.cluster.block.ClusterBlocks; import org.opensearch.cluster.metadata.AliasMetadata; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.metadata.IndexNameExpressionResolver; @@ -58,6 +61,7 @@ import org.opensearch.cluster.routing.UnassignedInfo; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.UUIDs; +import org.opensearch.common.collect.Tuple; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; import org.opensearch.common.util.concurrent.ThreadContext; @@ -66,6 +70,7 @@ import org.opensearch.core.common.unit.ByteSizeUnit; import org.opensearch.core.common.unit.ByteSizeValue; import org.opensearch.core.index.shard.ShardId; +import org.opensearch.index.IndexModule; import org.opensearch.index.cache.query.QueryCacheStats; import org.opensearch.index.cache.request.RequestCacheStats; import org.opensearch.index.engine.SegmentsStats; @@ -98,8 +103,11 @@ import static java.util.Collections.emptyList; import static org.opensearch.action.admin.indices.rollover.TransportRolloverAction.evaluateConditions; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; import static org.mockito.Mockito.any; import static org.mockito.Mockito.anyBoolean; import static org.mockito.Mockito.doAnswer; @@ -469,6 +477,266 @@ private static Condition createTestCondition() { return condition; } + private TransportRolloverAction newRolloverActionForCheckBlock(ClusterState state) { + ClusterService clusterService = mock(ClusterService.class); + when(clusterService.state()).thenReturn(state); + ThreadPool threadPool = mock(ThreadPool.class); + when(threadPool.getThreadContext()).thenReturn(new ThreadContext(Settings.EMPTY)); + IndexNameExpressionResolver resolver = new IndexNameExpressionResolver(new ThreadContext(Settings.EMPTY)); + MetadataRolloverService rolloverService = new MetadataRolloverService( + threadPool, + mock(MetadataCreateIndexService.class), + mock(MetadataIndexAliasesService.class), + resolver + ); + return new TransportRolloverAction( + mock(TransportService.class), + clusterService, + threadPool, + mock(ActionFilters.class), + resolver, + rolloverService, + mock(Client.class) + ); + } + + public void testCheckBlockSkipsBlockOnNonWriteAliasMember() { + // Given: an alias whose write index is unblocked, and a non-write + // alias member that carries a METADATA_WRITE block (CCR-style) + String alias = "logs-alias"; + String writeIndexName = "logs-000002"; + String nonWriteMember = "logs-000001"; + + IndexMetadata writeIndex = IndexMetadata.builder(writeIndexName) + .settings(settings(Version.CURRENT)) + .putAlias(AliasMetadata.builder(alias).writeIndex(true).build()) + .numberOfShards(1) + .numberOfReplicas(1) + .build(); + IndexMetadata blockedNonWriteMember = IndexMetadata.builder(nonWriteMember) + .settings(settings(Version.CURRENT)) + .putAlias(AliasMetadata.builder(alias).writeIndex(false).build()) + .numberOfShards(1) + .numberOfReplicas(1) + .build(); + + ClusterState state = ClusterState.builder(ClusterName.DEFAULT) + .metadata(Metadata.builder().put(writeIndex, false).put(blockedNonWriteMember, false)) + // INDEX_WRITE_BLOCK has level WRITE only and would not trip checkBlock's METADATA_WRITE + // probe; INDEX_METADATA_BLOCK is the closest standard constant that includes METADATA_WRITE, + // matching the level CCR's INDEX_REPLICATION_BLOCK uses. + .blocks(ClusterBlocks.builder().addIndexBlock(nonWriteMember, IndexMetadata.INDEX_METADATA_BLOCK).build()) + .build(); + + TransportRolloverAction action = newRolloverActionForCheckBlock(state); + + // When: checkBlock runs for a rollover targeting the alias + ClusterBlockException result = action.checkBlock(new RolloverRequest(alias, null), state); + + // Then: returns null (the bug fix — non-write member is irrelevant) + assertThat(result, is(nullValue())); + } + + public void testCheckBlockAbortsWhenWriteIndexHasMetadataWriteBlock() { + // Given: an alias whose write index has a METADATA_WRITE block applied + String alias = "logs-alias"; + String writeIndexName = "logs-000002"; + + IndexMetadata writeIndex = IndexMetadata.builder(writeIndexName) + .settings(settings(Version.CURRENT)) + .putAlias(AliasMetadata.builder(alias).writeIndex(true).build()) + .numberOfShards(1) + .numberOfReplicas(1) + .build(); + + ClusterState state = ClusterState.builder(ClusterName.DEFAULT) + .metadata(Metadata.builder().put(writeIndex, false)) + // INDEX_METADATA_BLOCK includes METADATA_WRITE level (see sibling test for rationale). + .blocks(ClusterBlocks.builder().addIndexBlock(writeIndexName, IndexMetadata.INDEX_METADATA_BLOCK).build()) + .build(); + + TransportRolloverAction action = newRolloverActionForCheckBlock(state); + + // When: checkBlock runs + ClusterBlockException result = action.checkBlock(new RolloverRequest(alias, null), state); + + // Then: returns a ClusterBlockException naming only the write index + // (regression guard for the still-correct abort path) + assertThat(result, is(notNullValue())); + assertThat(result.getMessage(), containsString(writeIndexName)); + } + + public void testCheckBlockSkipsBlockOnNonWriteDataStreamBacking() { + // Given: a data stream with multiple backing indices; an older backing + // (not the write backing) has a METADATA_WRITE block + String dataStreamName = "logs-ds"; + int generations = 2; + + ClusterState baseState = DataStreamTestHelper.getClusterStateWithDataStreams( + List.of(new Tuple<>(dataStreamName, generations)), + List.of() + ); + + // The write backing is the highest-generation index; older backings come first. + String olderBacking = baseState.metadata().dataStreams().get(dataStreamName).getIndices().getFirst().getName(); + + ClusterState state = ClusterState.builder(baseState) + // INDEX_METADATA_BLOCK includes METADATA_WRITE level (see sibling test for rationale). + .blocks(ClusterBlocks.builder().addIndexBlock(olderBacking, IndexMetadata.INDEX_METADATA_BLOCK).build()) + .build(); + + TransportRolloverAction action = newRolloverActionForCheckBlock(state); + + // When: checkBlock runs for a rollover targeting the data stream + ClusterBlockException result = action.checkBlock(new RolloverRequest(dataStreamName, null), state); + + // Then: returns null (data-stream parity with the alias case) + assertThat(result, is(nullValue())); + } + + public void testCheckBlockSkipsRemoteSnapshotWriteIndexWithoutUserReadOnlyBlock() { + // Given: an alias whose write index is a remote_snapshot. Such indices come with + // an implicit METADATA_WRITE block; without a user-set read-only setting, the + // precheck should defer (parity with ClusterBlocks#indicesWithRemoteSnapshotBlockedException). + String alias = "logs-alias"; + String writeIndexName = "logs-000002"; + + Settings remoteSnapshotSettings = Settings.builder() + .put(settings(Version.CURRENT).build()) + .put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), IndexModule.Type.REMOTE_SNAPSHOT.getSettingsKey()) + .build(); + + IndexMetadata writeIndex = IndexMetadata.builder(writeIndexName) + .settings(remoteSnapshotSettings) + .putAlias(AliasMetadata.builder(alias).writeIndex(true).build()) + .numberOfShards(1) + .numberOfReplicas(1) + .build(); + + ClusterState state = ClusterState.builder(ClusterName.DEFAULT) + .metadata(Metadata.builder().put(writeIndex, false)) + .blocks(ClusterBlocks.builder().addIndexBlock(writeIndexName, IndexMetadata.INDEX_METADATA_BLOCK).build()) + .build(); + + TransportRolloverAction action = newRolloverActionForCheckBlock(state); + + // When: checkBlock runs + ClusterBlockException result = action.checkBlock(new RolloverRequest(alias, null), state); + + // Then: returns null (the implicit remote_snapshot block is exempted) + assertThat(result, is(nullValue())); + } + + public void testCheckBlockAbortsWhenRemoteSnapshotHasUserReadOnlyBlock() { + // Given: a remote_snapshot write index with a user-set INDEX_READ_ONLY_SETTING. + // That's an intentional read-only block, so the precheck must still abort. + String alias = "logs-alias"; + String writeIndexName = "logs-000002"; + + Settings remoteSnapshotSettings = Settings.builder() + .put(settings(Version.CURRENT).build()) + .put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), IndexModule.Type.REMOTE_SNAPSHOT.getSettingsKey()) + .put(IndexMetadata.SETTING_READ_ONLY, true) + .build(); + + IndexMetadata writeIndex = IndexMetadata.builder(writeIndexName) + .settings(remoteSnapshotSettings) + .putAlias(AliasMetadata.builder(alias).writeIndex(true).build()) + .numberOfShards(1) + .numberOfReplicas(1) + .build(); + + ClusterState state = ClusterState.builder(ClusterName.DEFAULT) + .metadata(Metadata.builder().put(writeIndex, false)) + .blocks(ClusterBlocks.builder().addIndexBlock(writeIndexName, IndexMetadata.INDEX_METADATA_BLOCK).build()) + .build(); + + TransportRolloverAction action = newRolloverActionForCheckBlock(state); + + // When: checkBlock runs + ClusterBlockException result = action.checkBlock(new RolloverRequest(alias, null), state); + + // Then: aborts naming the write index (user opted in to read-only) + assertThat(result, is(notNullValue())); + assertThat(result.getMessage(), containsString(writeIndexName)); + } + + public void testCheckBlockReturnsNullWhenRolloverTargetUnresolvable() { + // Given: cluster state with no abstraction matching the rollover target + ClusterState state = ClusterState.builder(ClusterName.DEFAULT).metadata(Metadata.builder()).build(); + + TransportRolloverAction action = newRolloverActionForCheckBlock(state); + + // When: checkBlock runs + ClusterBlockException result = action.checkBlock(new RolloverRequest("does-not-exist", null), state); + + // Then: returns null (defers to main path's canonical "not found" error) + assertThat(result, is(nullValue())); + } + + public void testCheckBlockReturnsNullWhenAliasHasNoWriteMember() { + // Given: an alias with members but none marked is_write_index=true. + // (Multiple non-write members with no write member is the only shape that makes + // Alias#getWriteIndex() return null; a single non-write member is implicitly the write index.) + String alias = "logs-alias"; + String memberA = "logs-000001"; + String memberB = "logs-000002"; + + IndexMetadata indexA = IndexMetadata.builder(memberA) + .settings(settings(Version.CURRENT)) + .putAlias(AliasMetadata.builder(alias).writeIndex(false).build()) + .numberOfShards(1) + .numberOfReplicas(1) + .build(); + IndexMetadata indexB = IndexMetadata.builder(memberB) + .settings(settings(Version.CURRENT)) + .putAlias(AliasMetadata.builder(alias).writeIndex(false).build()) + .numberOfShards(1) + .numberOfReplicas(1) + .build(); + + ClusterState state = ClusterState.builder(ClusterName.DEFAULT) + .metadata(Metadata.builder().put(indexA, false).put(indexB, false)) + .build(); + + TransportRolloverAction action = newRolloverActionForCheckBlock(state); + + // When: checkBlock runs + ClusterBlockException result = action.checkBlock(new RolloverRequest(alias, null), state); + + // Then: returns null — defers to MetadataRolloverService's canonical + // "rollover target [...] does not point to a write index" error. + assertThat(result, is(nullValue())); + } + + public void testCheckBlockAbortsWhenDataStreamWriteBackingBlocked() { + // Given: a data stream where the write (highest-generation) backing carries a METADATA_WRITE block + String dataStreamName = "logs-ds"; + int generations = 2; + + ClusterState baseState = DataStreamTestHelper.getClusterStateWithDataStreams( + List.of(new Tuple<>(dataStreamName, generations)), + List.of() + ); + + // Write backing is the highest-generation index — the last in the indices list. + List backings = baseState.metadata().dataStreams().get(dataStreamName).getIndices(); + String writeBacking = backings.getLast().getName(); + + ClusterState state = ClusterState.builder(baseState) + .blocks(ClusterBlocks.builder().addIndexBlock(writeBacking, IndexMetadata.INDEX_METADATA_BLOCK).build()) + .build(); + + TransportRolloverAction action = newRolloverActionForCheckBlock(state); + + // When: checkBlock runs for a rollover targeting the data stream + ClusterBlockException result = action.checkBlock(new RolloverRequest(dataStreamName, null), state); + + // Then: aborts naming the write backing — symmetry with the alias write-index-blocked case. + assertThat(result, is(notNullValue())); + assertThat(result.getMessage(), containsString(writeBacking)); + } + public static IndicesStatsResponse randomIndicesStatsResponse(final IndexMetadata[] indices) { List shardStats = new ArrayList<>(); for (final IndexMetadata index : indices) { From 78dfd3e6fd3f74bc0c54c49c39106a4b4f4f64fe Mon Sep 17 00:00:00 2001 From: Mikhail Stepura Date: Tue, 26 May 2026 13:37:21 -0700 Subject: [PATCH 2/2] Inline single-line method call in `checkBlock` for rollover action Signed-off-by: Mikhail Stepura --- .../admin/indices/rollover/TransportRolloverAction.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/server/src/main/java/org/opensearch/action/admin/indices/rollover/TransportRolloverAction.java b/server/src/main/java/org/opensearch/action/admin/indices/rollover/TransportRolloverAction.java index e2055f0c51733..e3d4c42b5ad9f 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/rollover/TransportRolloverAction.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/rollover/TransportRolloverAction.java @@ -135,10 +135,7 @@ protected ClusterBlockException checkBlock(RolloverRequest request, ClusterState if (writeIndex == null) { return null; } - return ClusterBlocks.indicesWithRemoteSnapshotBlockedException( - Collections.singletonList(writeIndex.getIndex().getName()), - state - ); + return ClusterBlocks.indicesWithRemoteSnapshotBlockedException(Collections.singletonList(writeIndex.getIndex().getName()), state); } @Override