Skip to content

Commit

Permalink
Fix data stream alias validation. (elastic#81040)
Browse files Browse the repository at this point in the history 
In case of restoring a snapshot, it is possible to overwrite an existing
data stream with a data stream alias from a snapshot. This change fixes
this by improving the generic duplicate name validation.

On top of this the lack of data stream alias validation in Metadata.Builder#build()
method resulted in cases where data stream aliases could be added for existing
index aliases, data streams or indices with the same name.

Closes elastic#80972
  • Loading branch information
@martijnvg @ywangd
martijnvg authored and ywangd committed Dec 1, 2021
1 parent 8da80cc commit 529e681
Show file tree
Hide file tree
Showing 4 changed files with 305 additions and 4 deletions.
19 changes: 16 additions & 3 deletions server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java
View file
Expand Up @@ -1587,17 +1587,23 @@ public Metadata build(boolean builtIndicesLookupEagerly) {
indexMetadata.getAliases().keysIt().forEachRemaining(allAliases::add);
}

final ArrayList<String> duplicates = new ArrayList<>();
final Set<String> allDataStreams = new HashSet<>();
DataStreamMetadata dataStreamMetadata = (DataStreamMetadata) this.customs.get(DataStreamMetadata.TYPE);
if (dataStreamMetadata != null) {
for (DataStream dataStream : dataStreamMetadata.dataStreams().values()) {
allDataStreams.add(dataStream.getName());
}
// Adding data stream aliases:
for (String dataStreamAlias : dataStreamMetadata.getDataStreamAliases().keySet()) {
if (allAliases.add(dataStreamAlias) == false) {
duplicates.add("data stream alias and indices alias have the same name (" + dataStreamAlias + ")");
}
}
}

final Set<String> aliasDuplicatesWithIndices = new HashSet<>(allAliases);
aliasDuplicatesWithIndices.retainAll(allIndices);
ArrayList<String> duplicates = new ArrayList<>();
if (aliasDuplicatesWithIndices.isEmpty() == false) {
// iterate again and constructs a helpful message
for (ObjectCursor<IndexMetadata> cursor : indices.values()) {
Expand All @@ -1613,12 +1619,19 @@ public Metadata build(boolean builtIndicesLookupEagerly) {
aliasDuplicatesWithDataStreams.retainAll(allDataStreams);
if (aliasDuplicatesWithDataStreams.isEmpty() == false) {
// iterate again and constructs a helpful message
for (ObjectCursor<IndexMetadata> cursor : indices.values()) {
for (String alias : aliasDuplicatesWithDataStreams) {
for (String alias : aliasDuplicatesWithDataStreams) {
// reported var avoids adding a message twice if an index alias has the same name as a data stream.
boolean reported = false;
for (ObjectCursor<IndexMetadata> cursor : indices.values()) {
if (cursor.value.getAliases().containsKey(alias)) {
duplicates.add(alias + " (alias of " + cursor.value.getIndex() + ") conflicts with data stream");
reported = true;
}
}
// This is for adding an error message for when a data steam alias has the same name as a data stream.
if (reported == false && dataStreamMetadata != null && dataStreamMetadata.dataStreams().containsKey(alias)) {
duplicates.add("data stream alias and data stream have the same name (" + alias + ")");
}
}
}

Expand Down
75 changes: 75 additions & 0 deletions server/src/test/java/org/elasticsearch/cluster/metadata/MetadataTests.java
View file
Expand Up @@ -1215,6 +1215,71 @@ public void testBuildIndicesLookupForDataStreamAliases() {
assertThat(value.getAliases(), nullValue());
}

public void testDataStreamAliasValidation() {
Metadata.Builder b = Metadata.builder();
addDataStream("my-alias", b);
b.put("my-alias", "my-alias", null, null);
var e = expectThrows(IllegalStateException.class, b::build);
assertThat(e.getMessage(), containsString("data stream alias and data stream have the same name (my-alias)"));

b = Metadata.builder();
addDataStream("d1", b);
addDataStream("my-alias", b);
b.put("my-alias", "d1", null, null);
e = expectThrows(IllegalStateException.class, b::build);
assertThat(e.getMessage(), containsString("data stream alias and data stream have the same name (my-alias)"));

b = Metadata.builder();
b.put(
IndexMetadata.builder("index1")
.settings(
Settings.builder()
.put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
)
.putAlias(new AliasMetadata.Builder("my-alias"))
);

addDataStream("d1", b);
b.put("my-alias", "d1", null, null);
e = expectThrows(IllegalStateException.class, b::build);
assertThat(e.getMessage(), containsString("data stream alias and indices alias have the same name (my-alias)"));
}

public void testDataStreamAliasValidationRestoreScenario() {
Metadata.Builder b = Metadata.builder();
b.dataStreams(
Map.of("my-alias", createDataStream("my-alias")),
Map.of("my-alias", new DataStreamAlias("my-alias", List.of("my-alias"), null, null))
);
var e = expectThrows(IllegalStateException.class, b::build);
assertThat(e.getMessage(), containsString("data stream alias and data stream have the same name (my-alias)"));

b = Metadata.builder();
b.dataStreams(
Map.of("d1", createDataStream("d1"), "my-alias", createDataStream("my-alias")),
Map.of("my-alias", new DataStreamAlias("my-alias", List.of("d1"), null, null))
);
e = expectThrows(IllegalStateException.class, b::build);
assertThat(e.getMessage(), containsString("data stream alias and data stream have the same name (my-alias)"));

b = Metadata.builder();
b.put(
IndexMetadata.builder("index1")
.settings(
Settings.builder()
.put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
)
.putAlias(new AliasMetadata.Builder("my-alias"))
);
b.dataStreams(Map.of("d1", createDataStream("d1")), Map.of("my-alias", new DataStreamAlias("my-alias", List.of("d1"), null, null)));
e = expectThrows(IllegalStateException.class, b::build);
assertThat(e.getMessage(), containsString("data stream alias and indices alias have the same name (my-alias)"));
}

private void addDataStream(String name, Metadata.Builder b) {
int numBackingIndices = randomIntBetween(1, 4);
List<Index> indices = new ArrayList<>(numBackingIndices);
Expand All @@ -1226,6 +1291,16 @@ private void addDataStream(String name, Metadata.Builder b) {
b.put(new DataStream(name, createTimestampField("@timestamp"), indices));
}

private DataStream createDataStream(String name) {
int numBackingIndices = randomIntBetween(1, 4);
List<Index> indices = new ArrayList<>(numBackingIndices);
for (int j = 1; j <= numBackingIndices; j++) {
IndexMetadata idx = createBackingIndex(name, j).build();
indices.add(idx.getIndex());
}
return new DataStream(name, createTimestampField("@timestamp"), indices);
}

public void testIndicesLookupRecordsDataStreamForBackingIndices() {
final int numIndices = randomIntBetween(2, 5);
final int numBackingIndices = randomIntBetween(2, 5);
Expand Down
184 changes: 183 additions & 1 deletion ...data-streams/src/internalClusterTest/java/org/elasticsearch/datastreams/DataStreamIT.java
View file
Expand Up @@ -14,10 +14,12 @@
import org.elasticsearch.action.ActionRequestBuilder;
import org.elasticsearch.action.DocWriteRequest;
import org.elasticsearch.action.admin.cluster.state.ClusterStateRequest;
import org.elasticsearch.action.admin.indices.alias.Alias;
import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest;
import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest.AliasActions;
import org.elasticsearch.action.admin.indices.alias.get.GetAliasesRequest;
import org.elasticsearch.action.admin.indices.alias.get.GetAliasesResponse;
import org.elasticsearch.action.admin.indices.create.CreateIndexRequest;
import org.elasticsearch.action.admin.indices.get.GetIndexRequest;
import org.elasticsearch.action.admin.indices.get.GetIndexResponse;
import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsResponse;
Expand Down Expand Up @@ -58,6 +60,7 @@
import org.elasticsearch.index.mapper.MapperParsingException;
import org.elasticsearch.index.query.TermQueryBuilder;
import org.elasticsearch.indices.InvalidAliasNameException;
import org.elasticsearch.indices.InvalidIndexNameException;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.search.SearchHit;
Expand Down Expand Up @@ -1531,6 +1534,174 @@ public void testSegmentsSortedOnTimestampDesc() throws Exception {
assertTrue(timestamp2 > timestamp3);
}

public void testCreateDataStreamWithSameNameAsIndexAlias() throws Exception {
CreateIndexRequest createIndexRequest = new CreateIndexRequest("my-index").alias(new Alias("my-alias"));
assertAcked(client().admin().indices().create(createIndexRequest).actionGet());

// Important detail: create template with data stream template after the index has been created
DataStreamIT.putComposableIndexTemplate("my-template", List.of("my-*"));

var request = new CreateDataStreamAction.Request("my-alias");
var e = expectThrows(IllegalStateException.class, () -> client().execute(CreateDataStreamAction.INSTANCE, request).actionGet());
assertThat(e.getMessage(), containsString("[my-alias (alias of ["));
assertThat(e.getMessage(), containsString("]) conflicts with data stream"));
}

public void testCreateDataStreamWithSameNameAsIndex() throws Exception {
CreateIndexRequest createIndexRequest = new CreateIndexRequest("my-index").alias(new Alias("my-alias"));
assertAcked(client().admin().indices().create(createIndexRequest).actionGet());

// Important detail: create template with data stream template after the index has been created
DataStreamIT.putComposableIndexTemplate("my-template", List.of("my-*"));

var request = new CreateDataStreamAction.Request("my-index");
var e = expectThrows(IllegalStateException.class, () -> client().execute(CreateDataStreamAction.INSTANCE, request).actionGet());
assertThat(e.getMessage(), containsString("data stream [my-index] conflicts with index"));
}

public void testCreateDataStreamWithSameNameAsDataStreamAlias() throws Exception {
{
DataStreamIT.putComposableIndexTemplate("my-template", List.of("my-*"));
var request = new CreateDataStreamAction.Request("my-ds");
assertAcked(client().execute(CreateDataStreamAction.INSTANCE, request).actionGet());
var aliasesAddRequest = new IndicesAliasesRequest();
aliasesAddRequest.addAliasAction(new AliasActions(AliasActions.Type.ADD).index("my-ds").aliases("my-alias"));
assertAcked(client().admin().indices().aliases(aliasesAddRequest).actionGet());

var request2 = new CreateDataStreamAction.Request("my-alias");
var e = expectThrows(
IllegalStateException.class,
() -> client().execute(CreateDataStreamAction.INSTANCE, request2).actionGet()
);
assertThat(e.getMessage(), containsString("data stream alias and data stream have the same name (my-alias)"));
}
{
assertAcked(client().execute(DeleteDataStreamAction.INSTANCE, new DeleteDataStreamAction.Request("*")).actionGet());
DataStreamIT.putComposableIndexTemplate(
"my-template",
null,
List.of("my-*"),
null,
null,
Map.of("my-alias", AliasMetadata.builder("my-alias").build())
);
var request = new CreateDataStreamAction.Request("my-ds");
assertAcked(client().execute(CreateDataStreamAction.INSTANCE, request).actionGet());

var request2 = new CreateDataStreamAction.Request("my-alias");
var e = expectThrows(
IllegalStateException.class,
() -> client().execute(CreateDataStreamAction.INSTANCE, request2).actionGet()
);
assertThat(e.getMessage(), containsString("data stream alias and data stream have the same name (my-alias)"));
}
}

public void testCreateDataStreamAliasWithSameNameAsIndexAlias() throws Exception {
{
DataStreamIT.putComposableIndexTemplate("my-template", List.of("logs-*"));
CreateIndexRequest createIndexRequest = new CreateIndexRequest("es-logs").alias(new Alias("logs"));
assertAcked(client().admin().indices().create(createIndexRequest).actionGet());

var request = new CreateDataStreamAction.Request("logs-es");
assertAcked(client().execute(CreateDataStreamAction.INSTANCE, request).actionGet());
IndicesAliasesRequest aliasesAddRequest = new IndicesAliasesRequest();
aliasesAddRequest.addAliasAction(new AliasActions(AliasActions.Type.ADD).index("logs-es").aliases("logs"));
var e = expectThrows(IllegalStateException.class, () -> client().admin().indices().aliases(aliasesAddRequest).actionGet());
assertThat(e.getMessage(), containsString("data stream alias and indices alias have the same name (logs)"));
}
{
assertAcked(client().execute(DeleteDataStreamAction.INSTANCE, new DeleteDataStreamAction.Request("*")).actionGet());
DataStreamIT.putComposableIndexTemplate(
"my-template",
null,
List.of("logs-*"),
null,
null,
Map.of("logs", AliasMetadata.builder("logs").build())
);

var request = new CreateDataStreamAction.Request("logs-es");
var e = expectThrows(IllegalStateException.class, () -> client().execute(CreateDataStreamAction.INSTANCE, request).actionGet());
assertThat(e.getMessage(), containsString("data stream alias and indices alias have the same name (logs)"));
}
}

public void testCreateDataStreamAliasWithSameNameAsIndex() throws Exception {
DataStreamIT.putComposableIndexTemplate("my-template", List.of("logs-*"));

CreateIndexRequest createIndexRequest = new CreateIndexRequest("logs");
assertAcked(client().admin().indices().create(createIndexRequest).actionGet());

{
var request = new CreateDataStreamAction.Request("logs-es");
assertAcked(client().execute(CreateDataStreamAction.INSTANCE, request).actionGet());
IndicesAliasesRequest aliasesAddRequest = new IndicesAliasesRequest();
aliasesAddRequest.addAliasAction(new AliasActions(AliasActions.Type.ADD).index("logs-es").aliases("logs"));
var e = expectThrows(InvalidAliasNameException.class, () -> client().admin().indices().aliases(aliasesAddRequest).actionGet());
assertThat(
e.getMessage(),
equalTo("Invalid alias name [logs]: an index or data stream exists with the same name as the alias")
);
}
{
assertAcked(client().execute(DeleteDataStreamAction.INSTANCE, new DeleteDataStreamAction.Request("*")).actionGet());
var e = expectThrows(
IllegalArgumentException.class,
() -> DataStreamIT.putComposableIndexTemplate(
"my-template",
null,
List.of("logs-*"),
null,
null,
Map.of("logs", AliasMetadata.builder("logs").build())
)
);
assertThat(
e.getCause().getMessage(),
equalTo("Invalid alias name [logs]: an index or data stream exists with the same name as the alias")
);
}
}

public void testCreateIndexWithSameNameAsDataStreamAlias() throws Exception {
DataStreamIT.putComposableIndexTemplate("my-template", List.of("logs-*"));

var request = new CreateDataStreamAction.Request("logs-es");
assertAcked(client().execute(CreateDataStreamAction.INSTANCE, request).actionGet());
IndicesAliasesRequest aliasesAddRequest = new IndicesAliasesRequest();
aliasesAddRequest.addAliasAction(new AliasActions(AliasActions.Type.ADD).index("logs-es").aliases("logs"));
assertAcked(client().admin().indices().aliases(aliasesAddRequest).actionGet());

CreateIndexRequest createIndexRequest = new CreateIndexRequest("logs");
var e = expectThrows(InvalidIndexNameException.class, () -> client().admin().indices().create(createIndexRequest).actionGet());
assertThat(e.getMessage(), equalTo("Invalid index name [logs], already exists as alias"));
}

public void testCreateIndexAliasWithSameNameAsDataStreamAlias() throws Exception {
DataStreamIT.putComposableIndexTemplate("my-template", List.of("logs-*"));

var request = new CreateDataStreamAction.Request("logs-es");
assertAcked(client().execute(CreateDataStreamAction.INSTANCE, request).actionGet());
IndicesAliasesRequest aliasesAddRequest = new IndicesAliasesRequest();
aliasesAddRequest.addAliasAction(new AliasActions(AliasActions.Type.ADD).index("logs-es").aliases("logs"));
assertAcked(client().admin().indices().aliases(aliasesAddRequest).actionGet());

{
CreateIndexRequest createIndexRequest = new CreateIndexRequest("my-index").alias(new Alias("logs"));
var e = expectThrows(IllegalStateException.class, () -> client().admin().indices().create(createIndexRequest).actionGet());
assertThat(e.getMessage(), containsString("data stream alias and indices alias have the same name (logs)"));
}
{
CreateIndexRequest createIndexRequest = new CreateIndexRequest("my-index");
assertAcked(client().admin().indices().create(createIndexRequest).actionGet());
IndicesAliasesRequest addAliasRequest = new IndicesAliasesRequest();
addAliasRequest.addAliasAction(new AliasActions(AliasActions.Type.ADD).index("my-index").aliases("logs"));
var e = expectThrows(IllegalStateException.class, () -> client().admin().indices().aliases(addAliasRequest).actionGet());
assertThat(e.getMessage(), containsString("data stream alias and indices alias have the same name (logs)"));
}
}

private static void verifyResolvability(String dataStream, ActionRequestBuilder<?, ?> requestBuilder, boolean fail) {
verifyResolvability(dataStream, requestBuilder, fail, 0);
}
Expand Down Expand Up @@ -1723,12 +1894,23 @@ static void putComposableIndexTemplate(
List<String> patterns,
@Nullable Settings settings,
@Nullable Map<String, Object> metadata
) throws IOException {
putComposableIndexTemplate(id, mappings, patterns, settings, metadata, null);
}

static void putComposableIndexTemplate(
String id,
@Nullable String mappings,
List<String> patterns,
@Nullable Settings settings,
@Nullable Map<String, Object> metadata,
@Nullable Map<String, AliasMetadata> aliases
) throws IOException {
PutComposableIndexTemplateAction.Request request = new PutComposableIndexTemplateAction.Request(id);
request.indexTemplate(
new ComposableIndexTemplate(
patterns,
new Template(settings, mappings == null ? null : new CompressedXContent(mappings), null),
new Template(settings, mappings == null ? null : new CompressedXContent(mappings), aliases),
null,
null,
null,
Expand Down

0 comments on commit 529e681

Please sign in to comment.