diff --git a/src/integrationTest/java/org/opensearch/security/privileges/actionlevel/RoleBasedActionPrivilegesTest.java b/src/integrationTest/java/org/opensearch/security/privileges/actionlevel/RoleBasedActionPrivilegesTest.java index 1fc0ef35c6..4d5784b06b 100644 --- a/src/integrationTest/java/org/opensearch/security/privileges/actionlevel/RoleBasedActionPrivilegesTest.java +++ b/src/integrationTest/java/org/opensearch/security/privileges/actionlevel/RoleBasedActionPrivilegesTest.java @@ -24,6 +24,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.JsonNode; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @@ -37,12 +38,15 @@ import org.opensearch.common.settings.Settings; import org.opensearch.core.common.unit.ByteSizeUnit; import org.opensearch.core.common.unit.ByteSizeValue; +import org.opensearch.security.DefaultObjectMapper; import org.opensearch.security.privileges.PrivilegesEvaluationContext; import org.opensearch.security.privileges.PrivilegesEvaluatorResponse; import org.opensearch.security.resolver.IndexResolverReplacer; +import org.opensearch.security.securityconf.DynamicConfigFactory; import org.opensearch.security.securityconf.FlattenedActionGroups; import org.opensearch.security.securityconf.impl.CType; import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; +import org.opensearch.security.securityconf.impl.v7.ActionGroupsV7; import org.opensearch.security.securityconf.impl.v7.RoleV7; import org.opensearch.security.user.User; import org.opensearch.security.util.MockIndexMetadataBuilder; @@ -279,6 +283,138 @@ public void hasAny_wildcard() throws Exception { ); } + /** + * Tests StatefulIndexPrivileges construction performance + *

+ * This test simulates: + * - 3000 roles + * - 6000 indices + 9000 aliases + * - Each role has a unique pattern "role_N*" matching ~3 indices + shared "test-index" + * - DLS queries using user attribute substitution (${attr.internal.should_hide}) + */ + @Test + public void constructionPerformance_sharedPatterns() throws Exception { + final int NUM_ROLES = 3000; + final int NUM_INDICES = 6000; + final int NUM_ALIASES = 9000; + final int INDICES_PER_ROLE = 3; // Each role_N* pattern matches ~3 indices + + List clusterPatterns = Arrays.asList( + "cluster:admin/tasks/cancel", + "cluster:monitor/task", + "cluster:monitor/task/get", + "cluster:monitor/tasks/list", + "cluster:monitor/stats", + "indices:admin/aliases/get", + "indices:admin/exists", + "indices:admin/get", + "indices:admin/mappings/get", + "indices:admin/mapping/put", + "indices:admin/refresh*", + "indices:data*", + "indices:admin/flush*", + "indices:admin/forcemerge", + "indices:monitor/stats" + ); + + // DLS query with user attribute substitution + String dlsQuery = "{\"bool\": {\"must\": {\"match\": {\"should_hide\": \"${attr.internal.should_hide}\"}}}}"; + + // Create roles: each has pattern "role_N*" + shared "test-index" + Map rolesMap = new HashMap<>(); + for (int i = 0; i < NUM_ROLES; i++) { + List indexPatterns = Arrays.asList( + "role_" + i + "*", // unique pattern matching ~3 indices per role + "test-index" // shared index across all roles + ); + + Map indexPermission = new HashMap<>(); + indexPermission.put("index_patterns", indexPatterns); + indexPermission.put("dls", dlsQuery); + indexPermission.put("allowed_actions", Arrays.asList("indices_all")); + + rolesMap.put( + "role_" + i + "_user", + ImmutableMap.of("cluster_permissions", clusterPatterns, "index_permissions", Arrays.asList(indexPermission)) + ); + } + SecurityDynamicConfiguration roles = SecurityDynamicConfiguration.fromMap(rolesMap, CType.ROLES); + + // Create indices: for each role, create INDICES_PER_ROLE indices that match "role_N*" + // e.g., role_0_data, role_0_logs, role_0_metrics + MockIndexMetadataBuilder builder = indices(); + String[] suffixes = { "_data", "_logs", "_metrics" }; + for (int i = 0; i < NUM_ROLES; i++) { + for (int j = 0; j < INDICES_PER_ROLE; j++) { + builder.index("role_" + i + suffixes[j % suffixes.length]); + } + } + // Add the shared index + builder.index("test-index"); + + // Fill remaining indices to reach NUM_INDICES + int createdIndices = NUM_ROLES * INDICES_PER_ROLE + 1; + for (int i = createdIndices; i < NUM_INDICES; i++) { + builder.index("other_index_" + i); + } + + // Create aliases - each pointing to one index + for (int i = 0; i < NUM_ALIASES; i++) { + int indexNum = i % NUM_ROLES; + builder.alias("alias_" + i).of("role_" + indexNum + suffixes[0]); + } + + Metadata indexMetadata = builder.build(); + + // Load static action groups (includes indices_all -> indices:*) + JsonNode staticActionGroupsJsonNode = DefaultObjectMapper.YAML_MAPPER.readTree( + DynamicConfigFactory.class.getResourceAsStream("/static_config/static_action_groups.yml") + ); + SecurityDynamicConfiguration actionGroupsConfig = SecurityDynamicConfiguration.fromNode( + staticActionGroupsJsonNode, + CType.ACTIONGROUPS, + 2, + 0, + 0 + ); + FlattenedActionGroups actionGroups = new FlattenedActionGroups(actionGroupsConfig); + + // Build RoleBasedActionPrivileges and measure construction time + long start = System.nanoTime(); + RoleBasedActionPrivileges subject = new RoleBasedActionPrivileges(roles, actionGroups, Settings.EMPTY); + long constructionMs = java.util.concurrent.TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start); + + long startStateful = System.nanoTime(); + subject.updateStatefulIndexPrivileges(indexMetadata.getIndicesLookup(), 1); + long statefulMs = java.util.concurrent.TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startStateful); + + System.out.println( + "[constructionPerformance_sharedPatterns] Configuration: " + + NUM_ROLES + + " roles, " + + NUM_INDICES + + " indices, " + + NUM_ALIASES + + " aliases" + ); + System.out.println( + "[constructionPerformance_sharedPatterns] RoleBasedActionPrivileges construction: " + constructionMs + "ms" + ); + System.out.println("[constructionPerformance_sharedPatterns] StatefulIndexPrivileges update: " + statefulMs + "ms"); + System.out.println("[constructionPerformance_sharedPatterns] Total: " + (constructionMs + statefulMs) + "ms"); + + // Verify correctness + assertThat(subject.hasClusterPrivilege(ctx().roles("role_0_user").get(), "cluster:monitor/task/get"), isAllowed()); + assertThat(subject.hasClusterPrivilege(ctx().roles("role_2999_user").get(), "indices:data/read/search"), isAllowed()); + + PrivilegesEvaluatorResponse result = subject.hasIndexPrivilege( + ctx().roles("role_0_user").attr("attr.internal.project_id", "123").indexMetadata(indexMetadata).get(), + ImmutableSet.of("indices:data/read/search"), + IndexResolverReplacer.Resolved.ofIndex("role_0_data") + ); + assertThat(result, isAllowed()); + } + } /** diff --git a/src/main/java/org/opensearch/security/privileges/actionlevel/RoleBasedActionPrivileges.java b/src/main/java/org/opensearch/security/privileges/actionlevel/RoleBasedActionPrivileges.java index 3734f340ab..688900c10f 100644 --- a/src/main/java/org/opensearch/security/privileges/actionlevel/RoleBasedActionPrivileges.java +++ b/src/main/java/org/opensearch/security/privileges/actionlevel/RoleBasedActionPrivileges.java @@ -12,6 +12,7 @@ package org.opensearch.security.privileges.actionlevel; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.HashMap; import java.util.List; @@ -651,6 +652,11 @@ static class StatefulIndexPrivileges extends RuntimeOptimizedActionPrivileges.St ByteSizeValue statefulIndexMaxHeapSize ) { long startTime = System.currentTimeMillis(); + long matchingTime = 0; + long innerLoopTime = 0; + long aliasTime = 0; + int aliasHits = 0; + int totalIterations = 0; Map< String, @@ -662,6 +668,10 @@ static class StatefulIndexPrivileges extends RuntimeOptimizedActionPrivileges.St CompactMapGroupBuilder> indexMapBuilder = new CompactMapGroupBuilder<>(indices.keySet(), (k2) -> roleSetBuilder.createSubSetBuilder()); + // Pre-sort index names once for efficient prefix matching + String[] sortedIndexNames = indices.keySet().toArray(new String[0]); + Arrays.sort(sortedIndexNames); + // We iterate here through the present RoleV7 instances and nested through their "index_permissions" sections. // During the loop, the actionToIndexToRoles map is being built. // For that, action patterns from the role will be matched against the "well-known actions" to build @@ -672,7 +682,7 @@ static class StatefulIndexPrivileges extends RuntimeOptimizedActionPrivileges.St // and m is the number of matched indices. This formula does not take the loop through matchedActions in // account, as this is bound by a constant number and thus does not need to be considered in the O() notation. - top: for (Map.Entry entry : roles.getCEntries().entrySet()) { + for (Map.Entry entry : roles.getCEntries().entrySet()) { try { String roleName = entry.getKey(); RoleV7 role = entry.getValue(); @@ -688,15 +698,10 @@ static class StatefulIndexPrivileges extends RuntimeOptimizedActionPrivileges.St continue; } - WildcardMatcher indexMatcher = IndexPattern.from(indexPermissions.getIndex_patterns()).getStaticPattern(); - - if (indexMatcher == WildcardMatcher.NONE) { - // The pattern is likely blank because there are only templated patterns. - // Index patterns with templates are not handled here, but in the static IndexPermissions object - continue; - } - - List matchingIndices = indexMatcher.matching(indices.values(), IndexAbstraction::getName); + long t0 = System.nanoTime(); + List indexPatterns = indexPermissions.getIndex_patterns(); + List matchingIndices = matchIndicesOptimized(indexPatterns, indices, sortedIndexNames); + matchingTime += System.nanoTime() - t0; if (matchingIndices.isEmpty()) { continue; } @@ -708,8 +713,10 @@ static class StatefulIndexPrivileges extends RuntimeOptimizedActionPrivileges.St Collectors.toList() ); + long t1 = System.nanoTime(); for (IndexAbstraction index : matchingIndices) { for (String action : matchedActions) { + totalIterations++; CompactMapGroupBuilder.MapBuilder< String, DeduplicatingCompactSubSetBuilder.SubSetBuilder> indexToRoles = actionToIndexToRoles @@ -718,8 +725,10 @@ static class StatefulIndexPrivileges extends RuntimeOptimizedActionPrivileges.St indexToRoles.get(index.getName()).add(roleName); if (index instanceof IndexAbstraction.Alias) { + long t2 = System.nanoTime(); // For aliases we additionally add the sub-indices to the privilege map for (IndexMetadata subIndex : index.getIndices()) { + aliasHits++; String subIndexName = subIndex.getIndex().getName(); // We need to check whether the subIndex is part of the global indices // metadata map because that map has been filtered by relevantOnly(). @@ -738,22 +747,23 @@ static class StatefulIndexPrivileges extends RuntimeOptimizedActionPrivileges.St ); } } - } - - if (roleSetBuilder.getEstimatedByteSize() + indexMapBuilder - .getEstimatedByteSize() > statefulIndexMaxHeapSize.getBytes()) { - log.info( - "Size of precomputed index privileges exceeds configured limit ({}). Using capped data structure." - + "This might lead to slightly lower performance during privilege evaluation. Consider raising {}.", - statefulIndexMaxHeapSize, - PRECOMPUTED_PRIVILEGES_MAX_HEAP_SIZE.getKey() - ); - break top; + aliasTime += System.nanoTime() - t2; } } } + innerLoopTime += System.nanoTime() - t1; } } + if (roleSetBuilder.getEstimatedByteSize() + indexMapBuilder.getEstimatedByteSize() > statefulIndexMaxHeapSize + .getBytes()) { + log.info( + "Size of precomputed index privileges exceeds configured limit ({}). Using capped data structure." + + "This might lead to slightly lower performance during privilege evaluation. Consider raising {}.", + statefulIndexMaxHeapSize, + PRECOMPUTED_PRIVILEGES_MAX_HEAP_SIZE.getKey() + ); + break; + } } catch (Exception e) { log.error("Unexpected exception while processing role: {}\nIgnoring role.", entry.getKey(), e); } @@ -775,14 +785,58 @@ static class StatefulIndexPrivileges extends RuntimeOptimizedActionPrivileges.St this.indices = ImmutableMap.copyOf(indices); this.metadataVersion = metadataVersion; + } + + /** + * Optimized index matching using pattern categorization: + * - Exact matches: O(1) map lookup + * - Prefix patterns (ending with *): O(log n) binary search + linear scan of matches + * - Complex patterns: O(n) linear scan fallback + */ + private static List matchIndicesOptimized( + List patterns, + Map indices, + String[] sortedIndexNames + ) { + List result = new ArrayList<>(); + List complexPatterns = new ArrayList<>(); - long duration = System.currentTimeMillis() - startTime; + for (String pattern : patterns) { + if (pattern.contains("${")) { + // Templated pattern - skip, handled by static IndexPermissions + continue; + } + if (!pattern.contains("*") && !pattern.contains("?")) { + // Exact match - O(1) + IndexAbstraction idx = indices.get(pattern); + if (idx != null) { + result.add(idx); + } + } else if (pattern.endsWith("*") && !pattern.substring(0, pattern.length() - 1).contains("*") && !pattern.contains("?")) { + // Simple prefix pattern like "role_123*" - use binary search on pre-sorted array + String prefix = pattern.substring(0, pattern.length() - 1); + int idx = Arrays.binarySearch(sortedIndexNames, prefix); + int start = idx >= 0 ? idx : -(idx + 1); + for (int i = start; i < sortedIndexNames.length && sortedIndexNames[i].startsWith(prefix); i++) { + result.add(indices.get(sortedIndexNames[i])); + } + } else { + // Complex pattern - collect for batch processing + complexPatterns.add(pattern); + } + } - if (duration > 30000) { - log.warn("Creation of StatefulIndexPrivileges took {} ms", duration); - } else { - log.debug("Creation of StatefulIndexPrivileges took {} ms", duration); + // Process complex patterns with linear scan + if (!complexPatterns.isEmpty()) { + WildcardMatcher matcher = WildcardMatcher.from(complexPatterns); + for (IndexAbstraction idx : indices.values()) { + if (matcher.test(idx.getName()) && !result.contains(idx)) { + result.add(idx); + } + } } + + return result; } /**