Skip to content

Commit

Permalink
Fix _alias/<alias> returning non-matching data streams (elastic#104145)
Browse files Browse the repository at this point in the history
This fixes a bug where GET _alias/<alias> would return non-matching data streams
  • Loading branch information
mattc58 committed Jan 29, 2024
1 parent 688628b commit 75c8068
Show file tree
Hide file tree
Showing 9 changed files with 267 additions and 33 deletions.
6 changes: 6 additions & 0 deletions docs/changelog/104145.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 104145
summary: Fix _alias/<alias> returning non-matching data streams
area: Data streams
type: bug
issues:
- 96589
Original file line number Diff line number Diff line change
Expand Up @@ -260,3 +260,50 @@
- match: {test2.aliases: {}}
- match: {test3.aliases: {}}

---
"Test get alias with non-matching data streams":
- skip:
version: " - 8.12.1"
reason: "bugfix fixed from 8.12.1 and later"
features: allowed_warnings

- do:
allowed_warnings:
- "index template [my-template] has index patterns [ds-*] matching patterns from existing older templates [global] with patterns (global => [*]); this template [my-template] will take precedence during new index creation"
indices.put_index_template:
name: my-template
body:
index_patterns: [ ds-* ]
template:
settings:
index.number_of_replicas: 0
data_stream: { }

- do:
indices.create_data_stream:
name: ds-first
- is_true: acknowledged

- do:
indices.create_data_stream:
name: ds-second
- is_true: acknowledged

- do:
indices.update_aliases:
body:
actions:
- add:
index: ds-first
alias: my-alias
- is_true: acknowledged

- do:
indices.get_alias:
name: my-al*
- match: {ds-first.aliases.my-alias: {}}

- do:
indices.get_alias:
name: this-does-not-exist*
- is_false: ds-first.aliases.my-alias
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@
"Add data stream lifecycle":
- skip:
version: " - 8.10.99"
reason: "Data stream lifecycle was GA in 8.11"
reason: "Data stream lifecycle was available from 8.11"

- do:
cluster.put_component_template:
Expand Down Expand Up @@ -146,7 +146,7 @@
"Get data stream lifecycle with default rollover":
- skip:
version: " - 8.10.99"
reason: "Data stream lifecycle was GA in 8.11"
reason: "Data stream lifecycle was available from 8.11"

- do:
cluster.put_component_template:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.logging.DeprecationCategory;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.regex.Regex;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.core.UpdateForV9;
import org.elasticsearch.indices.SystemIndices;
Expand Down Expand Up @@ -147,21 +146,9 @@ static Map<String, List<DataStreamAlias>> postProcess(
ClusterState state
) {
Map<String, List<DataStreamAlias>> result = new HashMap<>();
boolean noAliasesSpecified = request.getOriginalAliases() == null || request.getOriginalAliases().length == 0;
List<String> requestedDataStreams = resolver.dataStreamNames(state, request.indicesOptions(), request.indices());
for (String requestedDataStream : requestedDataStreams) {
List<DataStreamAlias> aliases = state.metadata()
.dataStreamAliases()
.values()
.stream()
.filter(alias -> alias.getDataStreams().contains(requestedDataStream))
.filter(alias -> noAliasesSpecified || Regex.simpleMatch(request.aliases(), alias.getName()))
.toList();
if (aliases.isEmpty() == false) {
result.put(requestedDataStream, aliases);
}
}
return result;

return state.metadata().findDataStreamAliases(request.aliases(), requestedDataStreams.toArray(new String[0]));
}

private static void checkSystemIndexAccess(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

package org.elasticsearch.cluster.metadata;

/**
* Used as a common interface for AliasMetadata and DataStreamAlias
*/
interface AliasInfo {
String getAlias();
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@

import static java.util.Collections.emptySet;

public class AliasMetadata implements SimpleDiffable<AliasMetadata>, ToXContentFragment {
public class AliasMetadata implements SimpleDiffable<AliasMetadata>, ToXContentFragment, AliasInfo {

private final String alias;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
import java.util.function.Predicate;
import java.util.stream.Collectors;

public class DataStreamAlias implements SimpleDiffable<DataStreamAlias>, ToXContentFragment {
public class DataStreamAlias implements SimpleDiffable<DataStreamAlias>, ToXContentFragment, AliasInfo {

public static final ParseField DATA_STREAMS_FIELD = new ParseField("data_streams");
public static final ParseField WRITE_DATA_STREAM_FIELD = new ParseField("write_data_stream");
Expand Down Expand Up @@ -191,6 +191,13 @@ public String getName() {
return name;
}

/**
* Returns the alias name, which is the same value as getName()
*/
public String getAlias() {
return getName();
}

/**
* Returns the data streams that are referenced
*/
Expand Down
110 changes: 96 additions & 14 deletions server/src/main/java/org/elasticsearch/cluster/metadata/Metadata.java
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,16 @@ default boolean isRestorable() {

private final IndexVersion oldestIndexVersion;

// Used in the findAliases and findDataStreamAliases functions
private interface AliasInfoGetter {
List<? extends AliasInfo> get(String entityName);
}

// Used in the findAliases and findDataStreamAliases functions
private interface AliasInfoSetter {
void put(String entityName, List<AliasInfo> aliases);
}

private Metadata(
String clusterUUID,
boolean clusterUUIDCommitted,
Expand Down Expand Up @@ -799,11 +809,63 @@ public Map<String, List<AliasMetadata>> findAllAliases(final String[] concreteIn
* aliases then the result will <b>not</b> include the index's key.
*/
public Map<String, List<AliasMetadata>> findAliases(final String[] aliases, final String[] concreteIndices) {
ImmutableOpenMap.Builder<String, List<AliasMetadata>> mapBuilder = ImmutableOpenMap.builder();

AliasInfoGetter getter = index -> indices.get(index).getAliases().values().stream().toList();

AliasInfoSetter setter = (index, foundAliases) -> {
List<AliasMetadata> d = new ArrayList<>();
foundAliases.forEach(i -> d.add((AliasMetadata) i));
mapBuilder.put(index, d);
};

findAliasInfo(aliases, concreteIndices, getter, setter);

return mapBuilder.build();
}

/**
* Finds the specific data stream aliases that match with the specified aliases directly or partially via wildcards, and
* that point to the specified data streams (directly or matching data streams via wildcards).
*
* @param aliases The aliases to look for. Might contain include or exclude wildcards.
* @param dataStreams The data streams that the aliases must point to in order to be returned
* @return A map of data stream name to the list of DataStreamAlias objects that match. If a data stream does not have matching
* aliases then the result will <b>not</b> include the data stream's key.
*/
public Map<String, List<DataStreamAlias>> findDataStreamAliases(final String[] aliases, final String[] dataStreams) {
ImmutableOpenMap.Builder<String, List<DataStreamAlias>> mapBuilder = ImmutableOpenMap.builder();
Map<String, List<DataStreamAlias>> dataStreamAliases = dataStreamAliasesByDataStream();

AliasInfoGetter getter = dataStream -> dataStreamAliases.getOrDefault(dataStream, Collections.emptyList());

AliasInfoSetter setter = (dataStream, foundAliases) -> {
List<DataStreamAlias> dsAliases = new ArrayList<>();
foundAliases.forEach(alias -> dsAliases.add((DataStreamAlias) alias));
mapBuilder.put(dataStream, dsAliases);
};

findAliasInfo(aliases, dataStreams, getter, setter);

return mapBuilder.build();
}

/**
* Find the aliases that point to the specified data streams or indices. Called from findAliases or findDataStreamAliases.
*
* @param aliases The aliases to look for. Might contain include or exclude wildcards.
* @param possibleMatches The data streams or indices that the aliases must point to in order to be returned
* @param getter A function that is used to get the alises for a given data stream or index
* @param setter A function that is used to keep track of the found aliases
*/
private void findAliasInfo(final String[] aliases, final String[] possibleMatches, AliasInfoGetter getter, AliasInfoSetter setter) {
assert aliases != null;
assert concreteIndices != null;
if (concreteIndices.length == 0) {
return ImmutableOpenMap.of();
assert possibleMatches != null;
if (possibleMatches.length == 0) {
return;
}

// create patterns to use to search for targets
String[] patterns = new String[aliases.length];
boolean[] include = new boolean[aliases.length];
for (int i = 0; i < aliases.length; i++) {
Expand All @@ -816,14 +878,16 @@ public Map<String, List<AliasMetadata>> findAliases(final String[] aliases, fina
include[i] = true;
}
}

boolean matchAllAliases = patterns.length == 0;
ImmutableOpenMap.Builder<String, List<AliasMetadata>> mapBuilder = ImmutableOpenMap.builder();
for (String index : concreteIndices) {
IndexMetadata indexMetadata = indices.get(index);
List<AliasMetadata> filteredValues = new ArrayList<>();
for (AliasMetadata aliasMetadata : indexMetadata.getAliases().values()) {

for (String index : possibleMatches) {
List<AliasInfo> filteredValues = new ArrayList<>();

List<? extends AliasInfo> entities = getter.get(index);
for (AliasInfo aliasInfo : entities) {
boolean matched = matchAllAliases;
String alias = aliasMetadata.alias();
String alias = aliasInfo.getAlias();
for (int i = 0; i < patterns.length; i++) {
if (include[i]) {
if (matched == false) {
Expand All @@ -835,16 +899,15 @@ public Map<String, List<AliasMetadata>> findAliases(final String[] aliases, fina
}
}
if (matched) {
filteredValues.add(aliasMetadata);
filteredValues.add(aliasInfo);
}
}
if (filteredValues.isEmpty() == false) {
// Make the list order deterministic
CollectionUtil.timSort(filteredValues, Comparator.comparing(AliasMetadata::alias));
mapBuilder.put(index, Collections.unmodifiableList(filteredValues));
CollectionUtil.timSort(filteredValues, Comparator.comparing(AliasInfo::getAlias));
setter.put(index, Collections.unmodifiableList(filteredValues));
}
}
return mapBuilder.build();
}

/**
Expand Down Expand Up @@ -1264,6 +1327,25 @@ public Map<String, DataStreamAlias> dataStreamAliases() {
return this.custom(DataStreamMetadata.TYPE, DataStreamMetadata.EMPTY).getDataStreamAliases();
}

/**
* Return a map of DataStreamAlias objects by DataStream name
* @return a map of DataStreamAlias objects by DataStream name
*/
public Map<String, List<DataStreamAlias>> dataStreamAliasesByDataStream() {
Map<String, List<DataStreamAlias>> dataStreamAliases = new HashMap<>();

for (DataStreamAlias dsAlias : dataStreamAliases().values()) {
for (String dataStream : dsAlias.getDataStreams()) {
if (dataStreamAliases.containsKey(dataStream) == false) {
dataStreamAliases.put(dataStream, new ArrayList<>());
}
dataStreamAliases.get(dataStream).add(dsAlias);
}
}

return dataStreamAliases;
}

public NodesShutdownMetadata nodeShutdowns() {
return custom(NodesShutdownMetadata.TYPE, NodesShutdownMetadata.EMPTY);
}
Expand Down Expand Up @@ -2432,7 +2514,7 @@ private static void collectAliasDuplicates(
reported = true;
}
}
// This is for adding an error message for when a data steam alias has the same name as a data stream.
// This is for adding an error message for when a data stream 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
Loading

0 comments on commit 75c8068

Please sign in to comment.