Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport 2.x] fix rollover alias supports restored searchable snapshot index (#16483) #16568

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Ensure index templates are not applied to system indices ([#16418](https://github.com/opensearch-project/OpenSearch/pull/16418))
- Remove resource usages object from search response headers ([#16532](https://github.com/opensearch-project/OpenSearch/pull/16532))
- Support retrieving doc values of unsigned long field ([#16543](https://github.com/opensearch-project/OpenSearch/pull/16543))
- Fix rollover alias supports restored searchable snapshot index([#16483](https://github.com/opensearch-project/OpenSearch/pull/16483))

### Security

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
import org.opensearch.cluster.ClusterState;
import org.opensearch.cluster.ack.ClusterStateUpdateResponse;
import org.opensearch.cluster.block.ClusterBlockException;
import org.opensearch.cluster.block.ClusterBlockLevel;
import org.opensearch.cluster.block.ClusterBlocks;
import org.opensearch.cluster.metadata.AliasAction;
import org.opensearch.cluster.metadata.AliasMetadata;
import org.opensearch.cluster.metadata.IndexAbstraction;
Expand Down Expand Up @@ -123,7 +123,7 @@ protected ClusterBlockException checkBlock(IndicesAliasesRequest request, Cluste
for (IndicesAliasesRequest.AliasActions aliasAction : request.aliasActions()) {
Collections.addAll(indices, aliasAction.indices());
}
return state.blocks().indicesBlockedException(ClusterBlockLevel.METADATA_WRITE, indices.toArray(new String[0]));
return ClusterBlocks.indicesWithRemoteSnapshotBlockedException(indices, state);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
import org.opensearch.cluster.ClusterState;
import org.opensearch.cluster.ClusterStateUpdateTask;
import org.opensearch.cluster.block.ClusterBlockException;
import org.opensearch.cluster.block.ClusterBlockLevel;
import org.opensearch.cluster.block.ClusterBlocks;
import org.opensearch.cluster.metadata.IndexMetadata;
import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
import org.opensearch.cluster.metadata.Metadata;
Expand All @@ -62,8 +62,10 @@
import org.opensearch.transport.TransportService;

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;
Expand Down Expand Up @@ -127,11 +129,10 @@
request.indicesOptions().expandWildcardsClosed()
);

return state.blocks()
.indicesBlockedException(
ClusterBlockLevel.METADATA_WRITE,
indexNameExpressionResolver.concreteIndexNames(state, indicesOptions, request)
);
return ClusterBlocks.indicesWithRemoteSnapshotBlockedException(
new HashSet<>(Arrays.asList(indexNameExpressionResolver.concreteIndexNames(state, indicesOptions, request))),

Check warning on line 133 in server/src/main/java/org/opensearch/action/admin/indices/rollover/TransportRolloverAction.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/action/admin/indices/rollover/TransportRolloverAction.java#L132-L133

Added lines #L132 - L133 were not covered by tests
state
);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
import org.opensearch.cluster.ack.ClusterStateUpdateResponse;
import org.opensearch.cluster.block.ClusterBlockException;
import org.opensearch.cluster.block.ClusterBlockLevel;
import org.opensearch.cluster.metadata.IndexMetadata;
import org.opensearch.cluster.block.ClusterBlocks;
import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
import org.opensearch.cluster.metadata.MetadataUpdateSettingsService;
import org.opensearch.cluster.service.ClusterService;
Expand Down Expand Up @@ -118,9 +118,8 @@ protected ClusterBlockException checkBlock(UpdateSettingsRequest request, Cluste
return globalBlock;
}
if (request.settings().size() == 1 && // we have to allow resetting these settings otherwise users can't unblock an index
IndexMetadata.INDEX_BLOCKS_METADATA_SETTING.exists(request.settings())
|| IndexMetadata.INDEX_READ_ONLY_SETTING.exists(request.settings())
|| IndexMetadata.INDEX_BLOCKS_READ_ONLY_ALLOW_DELETE_SETTING.exists(request.settings())) {
ClusterBlocks.INDEX_DATA_READ_ONLY_BLOCK_SETTINGS.stream()
.anyMatch(booleanSetting -> booleanSetting.exists(request.settings()))) {
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,22 @@
package org.opensearch.cluster.block;

import org.opensearch.cluster.AbstractDiffable;
import org.opensearch.cluster.ClusterState;
import org.opensearch.cluster.Diff;
import org.opensearch.cluster.metadata.IndexMetadata;
import org.opensearch.cluster.metadata.MetadataIndexStateService;
import org.opensearch.common.Nullable;
import org.opensearch.common.annotation.PublicApi;
import org.opensearch.common.settings.Setting;
import org.opensearch.common.util.set.Sets;
import org.opensearch.core.common.io.stream.StreamInput;
import org.opensearch.core.common.io.stream.StreamOutput;
import org.opensearch.core.rest.RestStatus;
import org.opensearch.index.IndexModule;
import org.opensearch.index.translog.BufferedChecksumStreamOutput;

import java.io.IOException;
import java.util.Collection;
import java.util.Collections;
import java.util.EnumMap;
import java.util.HashMap;
Expand All @@ -56,6 +60,7 @@
import static java.util.Collections.emptySet;
import static java.util.Collections.unmodifiableSet;
import static java.util.stream.Collectors.toSet;
import static org.opensearch.index.IndexModule.INDEX_STORE_TYPE_SETTING;

/**
* Represents current cluster level blocks to block dirty operations done against the cluster.
Expand All @@ -65,7 +70,11 @@
@PublicApi(since = "1.0.0")
public class ClusterBlocks extends AbstractDiffable<ClusterBlocks> {
public static final ClusterBlocks EMPTY_CLUSTER_BLOCK = new ClusterBlocks(emptySet(), Map.of());

public static final Set<Setting<Boolean>> INDEX_DATA_READ_ONLY_BLOCK_SETTINGS = Set.of(
IndexMetadata.INDEX_READ_ONLY_SETTING,
IndexMetadata.INDEX_BLOCKS_METADATA_SETTING,
IndexMetadata.INDEX_BLOCKS_READ_ONLY_ALLOW_DELETE_SETTING
);
private final Set<ClusterBlock> global;

private final Map<String, Set<ClusterBlock>> indicesBlocks;
Expand Down Expand Up @@ -275,6 +284,21 @@ public ClusterBlockException indicesAllowReleaseResources(String[] indices) {
return new ClusterBlockException(indexLevelBlocks);
}

public static ClusterBlockException indicesWithRemoteSnapshotBlockedException(Collection<String> concreteIndices, ClusterState state) {
for (String index : concreteIndices) {
if (state.blocks().indexBlocked(ClusterBlockLevel.METADATA_WRITE, index)) {
IndexMetadata indexMeta = state.metadata().index(index);
if (indexMeta != null
&& (IndexModule.Type.REMOTE_SNAPSHOT.match(indexMeta.getSettings().get(INDEX_STORE_TYPE_SETTING.getKey())) == false
|| ClusterBlocks.INDEX_DATA_READ_ONLY_BLOCK_SETTINGS.stream()
.anyMatch(booleanSetting -> booleanSetting.exists(indexMeta.getSettings())))) {
return state.blocks().indicesBlockedException(ClusterBlockLevel.METADATA_WRITE, concreteIndices.toArray(new String[0]));
}
}
}
return null;
}

@Override
public String toString() {
if (global.isEmpty() && indices().isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,40 @@

package org.opensearch.cluster.block;

import com.carrotsearch.randomizedtesting.RandomizedTest;

import org.opensearch.Version;
import org.opensearch.cluster.ClusterName;
import org.opensearch.cluster.ClusterState;
import org.opensearch.cluster.metadata.AliasMetadata;
import org.opensearch.cluster.metadata.IndexMetadata;
import org.opensearch.cluster.metadata.Metadata;
import org.opensearch.common.io.stream.BytesStreamOutput;
import org.opensearch.common.settings.Setting;
import org.opensearch.common.settings.Settings;
import org.opensearch.core.common.io.stream.StreamInput;
import org.opensearch.index.IndexModule;
import org.opensearch.index.IndexSettings;
import org.opensearch.index.translog.BufferedChecksumStreamOutput;
import org.opensearch.test.OpenSearchTestCase;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;

import static org.opensearch.cluster.block.ClusterBlockTests.randomClusterBlock;
import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_METADATA_BLOCK;
import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_READ_BLOCK;
import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_READ_ONLY_ALLOW_DELETE_BLOCK;
import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_READ_ONLY_BLOCK;
import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_WRITE_BLOCK;
import static org.opensearch.cluster.metadata.IndexMetadata.REMOTE_READ_ONLY_ALLOW_DELETE;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_CREATION_DATE;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_VERSION_CREATED;

public class ClusterBlocksTests extends OpenSearchTestCase {

Expand Down Expand Up @@ -52,4 +80,153 @@ public void testWriteVerifiableTo() throws Exception {
clusterBlocks2.writeVerifiableTo(checksumOut2);
assertEquals(checksumOut.getChecksum(), checksumOut2.getChecksum());
}

public void testGlobalBlock() {
String index = "test-000001";
ClusterState.Builder stateBuilder = ClusterState.builder(new ClusterName("cluster"));
Set<String> indices = new HashSet<>();
indices.add(index);

// no global blocks
{
stateBuilder.blocks(ClusterBlocks.EMPTY_CLUSTER_BLOCK);
ClusterState clusterState = stateBuilder.build();
clusterState.blocks();
assertNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(indices, clusterState));
}

// has global block
{
for (ClusterBlock block : Arrays.asList(
INDEX_READ_ONLY_BLOCK,
INDEX_READ_BLOCK,
INDEX_WRITE_BLOCK,
INDEX_METADATA_BLOCK,
INDEX_READ_ONLY_ALLOW_DELETE_BLOCK,
REMOTE_READ_ONLY_ALLOW_DELETE
)) {
stateBuilder.blocks(ClusterBlocks.builder().addGlobalBlock(block).build());
ClusterState clusterState = stateBuilder.build();
clusterState.blocks();
assertNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(indices, clusterState));
}
}
}

public void testIndexWithBlock() {
String index = "test-000001";
Set<String> indices = new HashSet<>();
indices.add(index);
ClusterState.Builder stateBuilder = ClusterState.builder(new ClusterName("cluster"));
stateBuilder.blocks(ClusterBlocks.builder().addIndexBlock(index, IndexMetadata.INDEX_METADATA_BLOCK));
stateBuilder.metadata(Metadata.builder().put(createIndexMetadata(index, false, null, null), false));
ClusterState clusterState = stateBuilder.build();
clusterState.blocks();
assertNotNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(indices, stateBuilder.build()));
}

public void testRemoteIndexBlock() {
String remoteIndex = "remote_index";
Set<String> indices = new HashSet<>();
indices.add(remoteIndex);
ClusterState.Builder stateBuilder = ClusterState.builder(new ClusterName("cluster"));

{
IndexMetadata remoteSnapshotIndexMetadata = createIndexMetadata(remoteIndex, true, null, null);
stateBuilder.metadata(Metadata.builder().put(remoteSnapshotIndexMetadata, false));
stateBuilder.blocks(ClusterBlocks.builder().addBlocks(remoteSnapshotIndexMetadata));

ClusterState clusterState = stateBuilder.build();
assertTrue(clusterState.blocks().hasIndexBlock(remoteIndex, IndexMetadata.REMOTE_READ_ONLY_ALLOW_DELETE));
clusterState.blocks();
assertNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(indices, clusterState));
}

// searchable snapshot index with block
{
Setting<Boolean> setting = RandomizedTest.randomFrom(new ArrayList<>(ClusterBlocks.INDEX_DATA_READ_ONLY_BLOCK_SETTINGS));
IndexMetadata remoteSnapshotIndexMetadata = createIndexMetadata(remoteIndex, true, null, setting);
stateBuilder.metadata(Metadata.builder().put(remoteSnapshotIndexMetadata, false));
stateBuilder.blocks(ClusterBlocks.builder().addBlocks(remoteSnapshotIndexMetadata));
ClusterState clusterState = stateBuilder.build();
clusterState.blocks();
assertNotNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(indices, clusterState));
}
}

public void testRemoteIndexWithoutBlock() {
String remoteIndex = "remote_index";
ClusterState.Builder stateBuilder = ClusterState.builder(new ClusterName("cluster"));

String alias = "alias1";
IndexMetadata remoteSnapshotIndexMetadata = createIndexMetadata(remoteIndex, true, alias, null);
String index = "test-000001";
IndexMetadata indexMetadata = createIndexMetadata(index, false, alias, null);
stateBuilder.metadata(Metadata.builder().put(remoteSnapshotIndexMetadata, false).put(indexMetadata, false));

Set<String> indices = new HashSet<>();
indices.add(remoteIndex);
ClusterState clusterState = stateBuilder.build();
clusterState.blocks();
assertNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(indices, clusterState));
}

public void testRemoteIndexWithIndexBlock() {
String index = "test-000001";
String remoteIndex = "remote_index";
String alias = "alias1";
{
ClusterState.Builder stateBuilder = ClusterState.builder(new ClusterName("cluster"));
IndexMetadata remoteSnapshotIndexMetadata = createIndexMetadata(remoteIndex, true, alias, null);
IndexMetadata indexMetadata = createIndexMetadata(index, false, alias, null);
stateBuilder.metadata(Metadata.builder().put(remoteSnapshotIndexMetadata, false).put(indexMetadata, false))
.blocks(ClusterBlocks.builder().addBlocks(remoteSnapshotIndexMetadata));
ClusterState clusterState = stateBuilder.build();
clusterState.blocks();
assertNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(Collections.singleton(index), clusterState));
clusterState.blocks();
assertNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(Collections.singleton(remoteIndex), clusterState));
}

{
ClusterState.Builder stateBuilder = ClusterState.builder(new ClusterName("cluster"));
Setting<Boolean> setting = RandomizedTest.randomFrom(new ArrayList<>(ClusterBlocks.INDEX_DATA_READ_ONLY_BLOCK_SETTINGS));
IndexMetadata remoteSnapshotIndexMetadata = createIndexMetadata(remoteIndex, true, alias, setting);
IndexMetadata indexMetadata = createIndexMetadata(index, false, alias, null);
stateBuilder.metadata(Metadata.builder().put(remoteSnapshotIndexMetadata, false).put(indexMetadata, false))
.blocks(ClusterBlocks.builder().addBlocks(remoteSnapshotIndexMetadata));
ClusterState clusterState = stateBuilder.build();
assertNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(Collections.singleton(index), clusterState));
assertNotNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(Collections.singleton(remoteIndex), clusterState));
}
}

private IndexMetadata createIndexMetadata(String index, boolean isRemoteIndex, String alias, Setting<Boolean> blockSetting) {
IndexMetadata.Builder builder = IndexMetadata.builder(index).settings(createIndexSettingBuilder(isRemoteIndex, blockSetting));
if (alias != null) {
AliasMetadata.Builder aliasBuilder = AliasMetadata.builder(alias);
return builder.putAlias(aliasBuilder.build()).build();
}
return builder.build();
}

private Settings.Builder createIndexSettingBuilder(boolean isRemoteIndex, Setting<Boolean> blockSetting) {
Settings.Builder builder = Settings.builder()
.put(IndexMetadata.SETTING_INDEX_UUID, "abc")
.put(SETTING_VERSION_CREATED, Version.CURRENT)
.put(SETTING_CREATION_DATE, System.currentTimeMillis())
.put(SETTING_NUMBER_OF_SHARDS, 1)
.put(SETTING_NUMBER_OF_REPLICAS, 1);

if (isRemoteIndex) {
builder.put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), IndexModule.Type.REMOTE_SNAPSHOT.getSettingsKey())
.put(IndexSettings.SEARCHABLE_SNAPSHOT_REPOSITORY.getKey(), "repo")
.put(IndexSettings.SEARCHABLE_SNAPSHOT_ID_NAME.getKey(), "snapshot");
}
if (blockSetting != null) {
builder.put(blockSetting.getKey(), true);
}

return builder;
}
}
Loading