diff --git a/docs/CHANGELOG.asciidoc b/docs/CHANGELOG.asciidoc index 9755acfcb52c2..0e4d0c0f44eac 100644 --- a/docs/CHANGELOG.asciidoc +++ b/docs/CHANGELOG.asciidoc @@ -98,6 +98,19 @@ multi-argument versions. ({pull}29623[#29623]) Do not ignore request analysis/similarity settings on index resize operations when the source index already contains such settings ({pull}30216[#30216]) +== Elasticsearch version 6.3.1 + +=== New Features + +=== Enhancements + +=== Bug Fixes + +Reduce the number of object allocations made by {security} when resolving the indices and aliases for a request ({pull}30180[#30180]) + +=== Regressions + +=== Known Issues //[float] //=== Regressions diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java index 5c0000d430432..4c4d0afc10d86 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java @@ -40,10 +40,10 @@ import java.util.concurrent.CopyOnWriteArraySet; import java.util.stream.Collectors; +import static org.elasticsearch.xpack.core.security.authz.IndicesAndAliasesResolverField.NO_INDEX_PLACEHOLDER; + public class IndicesAndAliasesResolver { - private static final ResolvedIndices NO_INDEX_PLACEHOLDER_RESOLVED = - ResolvedIndices.local(IndicesAndAliasesResolverField.NO_INDEX_PLACEHOLDER); //`*,-*` what we replace indices with if we need Elasticsearch to return empty responses without throwing exception private static final String[] NO_INDICES_ARRAY = new String[] { "*", "-*" }; static final List NO_INDICES_LIST = Arrays.asList(NO_INDICES_ARRAY); @@ -87,12 +87,14 @@ public IndicesAndAliasesResolver(Settings settings, ClusterService clusterServic public ResolvedIndices resolve(TransportRequest request, MetaData metaData, AuthorizedIndices authorizedIndices) { if (request instanceof IndicesAliasesRequest) { - ResolvedIndices indices = ResolvedIndices.empty(); + ResolvedIndices.Builder resolvedIndicesBuilder = new ResolvedIndices.Builder(); IndicesAliasesRequest indicesAliasesRequest = (IndicesAliasesRequest) request; for (IndicesRequest indicesRequest : indicesAliasesRequest.getAliasActions()) { - indices = ResolvedIndices.add(indices, resolveIndicesAndAliases(indicesRequest, metaData, authorizedIndices)); + final ResolvedIndices resolved = resolveIndicesAndAliases(indicesRequest, metaData, authorizedIndices); + resolvedIndicesBuilder.addLocal(resolved.getLocal()); + resolvedIndicesBuilder.addRemote(resolved.getRemote()); } - return indices; + return resolvedIndicesBuilder.build(); } // if for some reason we are missing an action... just for safety we'll reject @@ -102,10 +104,10 @@ public ResolvedIndices resolve(TransportRequest request, MetaData metaData, Auth return resolveIndicesAndAliases((IndicesRequest) request, metaData, authorizedIndices); } - ResolvedIndices resolveIndicesAndAliases(IndicesRequest indicesRequest, MetaData metaData, - AuthorizedIndices authorizedIndices) { + + ResolvedIndices resolveIndicesAndAliases(IndicesRequest indicesRequest, MetaData metaData, AuthorizedIndices authorizedIndices) { + final ResolvedIndices.Builder resolvedIndicesBuilder = new ResolvedIndices.Builder(); boolean indicesReplacedWithNoIndices = false; - final ResolvedIndices indices; if (indicesRequest instanceof PutMappingRequest && ((PutMappingRequest) indicesRequest).getConcreteIndex() != null) { /* * This is a special case since PutMappingRequests from dynamic mapping updates have a concrete index @@ -114,7 +116,7 @@ ResolvedIndices resolveIndicesAndAliases(IndicesRequest indicesRequest, MetaData */ assert indicesRequest.indices() == null || indicesRequest.indices().length == 0 : "indices are: " + Arrays.toString(indicesRequest.indices()); // Arrays.toString() can handle null values - all good - return ResolvedIndices.local(((PutMappingRequest) indicesRequest).getConcreteIndex().getName()); + resolvedIndicesBuilder.addLocal(((PutMappingRequest) indicesRequest).getConcreteIndex().getName()); } else if (indicesRequest instanceof IndicesRequest.Replaceable) { IndicesRequest.Replaceable replaceable = (IndicesRequest.Replaceable) indicesRequest; final boolean replaceWildcards = indicesRequest.indicesOptions().expandWildcardsOpen() @@ -127,13 +129,12 @@ ResolvedIndices resolveIndicesAndAliases(IndicesRequest indicesRequest, MetaData indicesOptions.expandWildcardsOpen(), indicesOptions.expandWildcardsClosed()); } - ResolvedIndices result = ResolvedIndices.empty(); // check for all and return list of authorized indices if (IndexNameExpressionResolver.isAllIndices(indicesList(indicesRequest.indices()))) { if (replaceWildcards) { for (String authorizedIndex : authorizedIndices.get()) { if (isIndexVisible(authorizedIndex, indicesOptions, metaData)) { - result = ResolvedIndices.add(result, ResolvedIndices.local(authorizedIndex)); + resolvedIndicesBuilder.addLocal(authorizedIndex); } } } @@ -144,7 +145,7 @@ ResolvedIndices resolveIndicesAndAliases(IndicesRequest indicesRequest, MetaData if (allowsRemoteIndices(indicesRequest)) { split = remoteClusterResolver.splitLocalAndRemoteIndexNames(indicesRequest.indices()); } else { - split = ResolvedIndices.local(indicesRequest.indices()); + split = new ResolvedIndices(Arrays.asList(indicesRequest.indices()), Collections.emptyList()); } List replaced = replaceWildcardsWithAuthorizedIndices(split.getLocal(), indicesOptions, metaData, authorizedIndices.get(), replaceWildcards); @@ -153,22 +154,23 @@ ResolvedIndices resolveIndicesAndAliases(IndicesRequest indicesRequest, MetaData //remove all the ones that the current user is not authorized for and ignore them replaced = replaced.stream().filter(authorizedIndices.get()::contains).collect(Collectors.toList()); } - result = new ResolvedIndices(new ArrayList<>(replaced), split.getRemote()); + resolvedIndicesBuilder.addLocal(replaced); + resolvedIndicesBuilder.addRemote(split.getRemote()); } - if (result.isEmpty()) { + + if (resolvedIndicesBuilder.isEmpty()) { if (indicesOptions.allowNoIndices()) { //this is how we tell es core to return an empty response, we can let the request through being sure //that the '-*' wildcard expression will be resolved to no indices. We can't let empty indices through //as that would be resolved to _all by es core. replaceable.indices(NO_INDICES_ARRAY); indicesReplacedWithNoIndices = true; - indices = NO_INDEX_PLACEHOLDER_RESOLVED; + resolvedIndicesBuilder.addLocal(NO_INDEX_PLACEHOLDER); } else { throw new IndexNotFoundException(Arrays.toString(indicesRequest.indices())); } } else { - replaceable.indices(result.toArray()); - indices = result; + replaceable.indices(resolvedIndicesBuilder.build().toArray()); } } else { if (containsWildcards(indicesRequest)) { @@ -182,11 +184,9 @@ ResolvedIndices resolveIndicesAndAliases(IndicesRequest indicesRequest, MetaData //That is fine though because they never contain wildcards, as they get replaced as part of the authorization of their //corresponding parent request on the coordinating node. Hence wildcards don't need to get replaced nor exploded for // shard level requests. - List resolvedNames = new ArrayList<>(); for (String name : indicesRequest.indices()) { - resolvedNames.add(nameExpressionResolver.resolveDateMathExpression(name)); + resolvedIndicesBuilder.addLocal(nameExpressionResolver.resolveDateMathExpression(name)); } - indices = new ResolvedIndices(resolvedNames, new ArrayList<>()); } if (indicesRequest instanceof AliasesRequest) { @@ -207,10 +207,10 @@ ResolvedIndices resolveIndicesAndAliases(IndicesRequest indicesRequest, MetaData //if we replaced the indices with '-*' we shouldn't be adding the aliases to the list otherwise the request will //not get authorized. Leave only '-*' and ignore the rest, result will anyway be empty. } else { - return ResolvedIndices.add(indices, ResolvedIndices.local(aliasesRequest.aliases())); + resolvedIndicesBuilder.addLocal(aliasesRequest.aliases()); } } - return indices; + return resolvedIndicesBuilder.build(); } public static boolean allowsRemoteIndices(IndicesRequest request) { @@ -423,24 +423,8 @@ public static class ResolvedIndices { private final List remote; ResolvedIndices(List local, List remote) { - this.local = local; - this.remote = remote; - } - - /** - * Constructs a new instance of this class where both the {@link #getLocal() local} and {@link #getRemote() remote} index lists - * are empty. - */ - private static ResolvedIndices empty() { - return new ResolvedIndices(Collections.emptyList(), Collections.emptyList()); - } - - /** - * Constructs a new instance of this class where both the {@link #getLocal() local} index list is populated with names - * and the {@link #getRemote() remote} index list is empty. - */ - private static ResolvedIndices local(String... names) { - return new ResolvedIndices(Arrays.asList(names), Collections.emptyList()); + this.local = Collections.unmodifiableList(local); + this.remote = Collections.unmodifiableList(remote); } /** @@ -449,14 +433,14 @@ private static ResolvedIndices local(String... names) { * to [ "-a1", "a*" ]. As a consequence, this list may contain duplicates. */ public List getLocal() { - return Collections.unmodifiableList(local); + return local; } /** * Returns the collection of index names that have been stored as "remote" indices. */ public List getRemote() { - return Collections.unmodifiableList(remote); + return remote; } /** @@ -471,7 +455,7 @@ public boolean isEmpty() { * {@link IndicesAndAliasesResolverField#NO_INDEX_PLACEHOLDER no-index-placeholder} and nothing else. */ public boolean isNoIndicesPlaceholder() { - return remote.isEmpty() && local.size() == 1 && local.contains(IndicesAndAliasesResolverField.NO_INDEX_PLACEHOLDER); + return remote.isEmpty() && local.size() == 1 && local.contains(NO_INDEX_PLACEHOLDER); } private String[] toArray() { @@ -487,19 +471,43 @@ private String[] toArray() { } /** - * Returns a new ResolvedIndices contains the {@link #getLocal() local} and {@link #getRemote() remote} - * index lists from b appended to the corresponding lists in a. + * Builder class for ResolvedIndices that allows for the building of a list of indices + * without the need to construct new objects and merging them together */ - private static ResolvedIndices add(ResolvedIndices a, ResolvedIndices b) { - List local = new ArrayList<>(a.local.size() + b.local.size()); - local.addAll(a.local); - local.addAll(b.local); - - List remote = new ArrayList<>(a.remote.size() + b.remote.size()); - remote.addAll(a.remote); - remote.addAll(b.remote); - return new ResolvedIndices(local, remote); - } + private static class Builder { + + private final List local = new ArrayList<>(); + private final List remote = new ArrayList<>(); + /** add a local index name */ + private void addLocal(String index) { + local.add(index); + } + + /** adds the array of local index names */ + private void addLocal(String[] indices) { + local.addAll(Arrays.asList(indices)); + } + + /** adds the list of local index names */ + private void addLocal(List indices) { + local.addAll(indices); + } + + /** adds the list of remote index names */ + private void addRemote(List indices) { + remote.addAll(indices); + } + + /** @return true if both the local and remote index lists are empty. */ + private boolean isEmpty() { + return local.isEmpty() && remote.isEmpty(); + } + + /** @return a immutable ResolvedIndices instance with the local and remote index lists */ + private ResolvedIndices build() { + return new ResolvedIndices(local, remote); + } + } } }