Skip to content

Fix data stream alias validation. #81040

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Nov 30, 2021
Merged

Fix data stream alias validation. #81040

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e4793d4
Fix data stream alias validation.
martijnvg Nov 25, 2021
d706428
Merge remote-tracking branch 'es/master' into fix_data_stream_alias_v…
martijnvg Nov 29, 2021
2499026
tweaked test
martijnvg Nov 29, 2021
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
19 changes: 16 additions & 3 deletions server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
Loading