Przeglądaj źródła

Use fixed size memory allocation in IndicesPermission (#77748)

This changes the implementation of the Role.authorize method so that
data structures can be constructed with a known size.

Previously, when authorizing a request with a large number of indices,
the HashMaps would need to resize themselves multiple times, at a
noticeable performance cost.

In order to know how large these maps need to be, we now expand all
named resource (indices, aliases, data-streams) upfront and then set
the initial size of the maps accordingly.
Tim Vernum 4 lat temu
rodzic
commit
2cfdadce27

+ 129 - 40
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermission.java

@@ -240,35 +240,125 @@ public final class IndicesPermission {
     }
 
     /**
-     * Authorizes the provided action against the provided indices, given the current cluster metadata
+     * Represents the set of data required about an IndexAbstraction (index/alias/datastream) in order to perform authorization on that
+     * object (including setting up the necessary data structures for Field and Document Level Security).
      */
-    public Map<String, IndicesAccessControl.IndexAccessControl> authorize(String action, Set<String> requestedIndicesOrAliases,
-                                                                          Map<String, IndexAbstraction> lookup,
-                                                                          FieldPermissionsCache fieldPermissionsCache) {
-        // now... every index that is associated with the request, must be granted
-        // by at least one indices permission group
-        Map<String, Set<FieldPermissions>> fieldPermissionsByIndex = new HashMap<>();
-        Map<String, DocumentLevelPermissions> roleQueriesByIndex = new HashMap<>();
-        Map<String, Boolean> grantedBuilder = new HashMap<>();
+    private static class IndexResource {
+        /**
+         * The name of the IndexAbstraction on which authorization is being performed
+         */
+        private final String name;
+
+        /**
+         * The IndexAbstraction on which authorization is being performed, or {@code null} if nothing in the cluster matches the name
+         */
+        @Nullable
+        private final IndexAbstraction indexAbstraction;
+
+        public Collection<String> concreteIndices;
+
+        private IndexResource(String name, @Nullable IndexAbstraction abstraction) {
+            assert name != null : "Resource name cannot be null";
+            assert abstraction == null || abstraction.getName().equals(name) : "Index abstraction has unexpected name ["
+                + abstraction.getName()
+                + "] vs ["
+                + name
+                + "]";
+            this.name = name;
+            this.indexAbstraction = abstraction;
+        }
 
-        final boolean isMappingUpdateAction = isMappingUpdateAction(action);
+        /**
+         * @return {@code true} if-and-only-if this object is related to a data-stream, either by having a
+         * {@link IndexAbstraction#getType()} of {@link IndexAbstraction.Type#DATA_STREAM} or by being the backing index for a
+         * {@link IndexAbstraction#getParentDataStream()}  data-stream}.
+         */
+        public boolean isPartOfDataStream() {
+            if (indexAbstraction == null) {
+                return false;
+            }
+            switch (indexAbstraction.getType()) {
+                case DATA_STREAM:
+                    return true;
+                case CONCRETE_INDEX:
+                    return indexAbstraction.getParentDataStream() != null;
+                default:
+                    return false;
+            }
+        }
 
-        for (String indexOrAlias : requestedIndicesOrAliases) {
-            final boolean isBackingIndex;
-            final boolean isDataStream;
-            final Set<String> concreteIndices = new HashSet<>();
-            final IndexAbstraction indexAbstraction = lookup.get(indexOrAlias);
-            if (indexAbstraction != null) {
-                for (IndexMetadata indexMetadata : indexAbstraction.getIndices()) {
-                    concreteIndices.add(indexMetadata.getIndex().getName());
+        /**
+         * Check whether this object is covered by the provided permission {@link Group}.
+         * For indices that are part of a data-stream, this checks both the index name and the parent data-stream name.
+         * In all other cases, it checks the name of this object only.
+         */
+        public boolean checkIndex(Group group) {
+            final IndexAbstraction.DataStream ds = indexAbstraction == null ? null : indexAbstraction.getParentDataStream();
+            if (ds != null) {
+                if (group.checkIndex(ds.getName())) {
+                    return true;
                 }
-                isBackingIndex = indexAbstraction.getType() == IndexAbstraction.Type.CONCRETE_INDEX &&
-                        indexAbstraction.getParentDataStream() != null;
-                isDataStream = indexAbstraction.getType() == IndexAbstraction.Type.DATA_STREAM;
+            }
+            return group.checkIndex(name);
+        }
+
+        /**
+         * @return the number of distinct objects to which this expansion refers.
+         */
+        public int size() {
+            if (indexAbstraction == null) {
+                return 1;
+            } else if (indexAbstraction.getType() == IndexAbstraction.Type.CONCRETE_INDEX) {
+                return 1;
             } else {
-                isBackingIndex = isDataStream = false;
+                return 1 + indexAbstraction.getIndices().size();
             }
+        }
+
+        public Collection<String> resolveConcreteIndices() {
+            if (indexAbstraction == null) {
+                return List.of();
+            } else if (indexAbstraction.getType() == IndexAbstraction.Type.CONCRETE_INDEX) {
+                return List.of(indexAbstraction.getName());
+            } else {
+                final List<IndexMetadata> indices = indexAbstraction.getIndices();
+                final List<String> concreteIndices = new ArrayList<>(indices.size());
+                for (var idx : indices) {
+                    concreteIndices.add(idx.getIndex().getName());
+                }
+                return concreteIndices;
+            }
+        }
+    }
+
+    /**
+     * Authorizes the provided action against the provided indices, given the current cluster metadata
+     */
+    public Map<String, IndicesAccessControl.IndexAccessControl> authorize(
+        String action,
+        Set<String> requestedIndicesOrAliases,
+        Map<String, IndexAbstraction> lookup,
+        FieldPermissionsCache fieldPermissionsCache
+    ) {
 
+        final List<IndexResource> resources = new ArrayList<>(requestedIndicesOrAliases.size());
+        int totalResourceCount = 0;
+
+        for (String indexOrAlias : requestedIndicesOrAliases) {
+            final IndexResource resource = new IndexResource(indexOrAlias, lookup.get(indexOrAlias));
+            resources.add(resource);
+            totalResourceCount += resource.size();
+        }
+
+        // now... every index that is associated with the request, must be granted
+        // by at least one indices permission group
+        final Map<String, Set<FieldPermissions>> fieldPermissionsByIndex = new HashMap<>(totalResourceCount);
+        final Map<String, DocumentLevelPermissions> roleQueriesByIndex = new HashMap<>(totalResourceCount);
+        final Map<String, Boolean> grantedBuilder = new HashMap<>(totalResourceCount);
+
+        final boolean isMappingUpdateAction = isMappingUpdateAction(action);
+
+        for (IndexResource resource : resources) {
             // true if ANY group covers the given index AND the given action
             boolean granted = false;
             // true if ANY group, which contains certain ingest privileges, covers the given index AND the action is a mapping update for
@@ -276,27 +366,30 @@ public final class IndicesPermission {
             boolean bwcGrantMappingUpdate = false;
             final List<Runnable> bwcDeprecationLogActions = new ArrayList<>();
 
+            final Collection<String> concreteIndices = resource.resolveConcreteIndices();
             for (Group group : groups) {
                 // the group covers the given index OR the given index is a backing index and the group covers the parent data stream
-                final boolean indexCheck = group.checkIndex(indexOrAlias) ||
-                        (isBackingIndex && group.checkIndex(indexAbstraction.getParentDataStream().getName()));
-                if (indexCheck) {
+                if (resource.checkIndex(group)) {
                     boolean actionCheck = group.checkAction(action);
                     granted = granted || actionCheck;
+
                     // mapping updates are allowed for certain privileges on indices and aliases (but not on data streams),
                     // outside of the privilege definition
-                    boolean bwcMappingActionCheck = isMappingUpdateAction && false == isDataStream && false == isBackingIndex &&
-                            containsPrivilegeThatGrantsMappingUpdatesForBwc(group);
+                    boolean bwcMappingActionCheck = isMappingUpdateAction
+                        && false == resource.isPartOfDataStream()
+                        && containsPrivilegeThatGrantsMappingUpdatesForBwc(group);
                     bwcGrantMappingUpdate = bwcGrantMappingUpdate || bwcMappingActionCheck;
+
                     if (actionCheck || bwcMappingActionCheck) {
                         // propagate DLS and FLS permissions over the concrete indices
                         for (String index : concreteIndices) {
                             Set<FieldPermissions> fieldPermissions = fieldPermissionsByIndex.computeIfAbsent(index, (k) -> new HashSet<>());
-                            fieldPermissionsByIndex.put(indexOrAlias, fieldPermissions);
+                            fieldPermissionsByIndex.put(resource.name, fieldPermissions);
                             fieldPermissions.add(group.getFieldPermissions());
+
                             DocumentLevelPermissions permissions =
                                     roleQueriesByIndex.computeIfAbsent(index, (k) -> new DocumentLevelPermissions());
-                            roleQueriesByIndex.putIfAbsent(indexOrAlias, permissions);
+                            roleQueriesByIndex.putIfAbsent(resource.name, permissions);
                             if (group.hasQuery()) {
                                 permissions.addAll(group.getQuery());
                             } else {
@@ -311,9 +404,9 @@ public final class IndicesPermission {
                                 if (PRIVILEGE_NAME_SET_BWC_ALLOW_MAPPING_UPDATE.contains(privilegeName)) {
                                     bwcDeprecationLogActions.add(() ->
                                         deprecationLogger.critical(DeprecationCategory.SECURITY,
-                                            "[" + indexOrAlias + "] mapping update for ingest privilege [" +
+                                            "[" + resource.name + "] mapping update for ingest privilege [" +
                                                 privilegeName + "]", "the index privilege [" + privilegeName + "] allowed the update " +
-                                                "mapping action [" + action + "] on index [" + indexOrAlias + "], this privilege " +
+                                                "mapping action [" + action + "] on index [" + resource.name + "], this privilege " +
                                                 "will not permit mapping updates in the next major release - users who require access " +
                                                 "to update mappings must be granted explicit privileges")
                                     );
@@ -330,17 +423,13 @@ public final class IndicesPermission {
                 bwcDeprecationLogActions.forEach(Runnable::run);
             }
 
-            if (concreteIndices.isEmpty()) {
-                grantedBuilder.put(indexOrAlias, granted);
-            } else {
-                grantedBuilder.put(indexOrAlias, granted);
-                for (String concreteIndex : concreteIndices) {
-                    grantedBuilder.put(concreteIndex, granted);
-                }
+            grantedBuilder.put(resource.name, granted);
+            for (String concreteIndex : concreteIndices) {
+                grantedBuilder.put(concreteIndex, granted);
             }
         }
 
-        Map<String, IndicesAccessControl.IndexAccessControl> indexPermissions = new HashMap<>();
+        Map<String, IndicesAccessControl.IndexAccessControl> indexPermissions = new HashMap<>(grantedBuilder.size());
         for (Map.Entry<String, Boolean> entry : grantedBuilder.entrySet()) {
             String index = entry.getKey();
             DocumentLevelPermissions permissions = roleQueriesByIndex.get(index);
@@ -464,7 +553,7 @@ public final class IndicesPermission {
         private void addAll(Set<BytesReference> query) {
             if (allowAll == false) {
                 if (queries == null) {
-                    queries = new HashSet<>();
+                    queries = new HashSet<>(query.size());
                 }
                 queries.addAll(query);
             }