Skip to content

Commit 34d1f1f

Browse files
nibixnibix
committed
Removed the setting plugins.security.privileges_evaluation.precomputed_privileges.include_indices
See discussion in opensearch-project#4380 (comment) Signed-off-by: Nils Bandener <[email protected]>
1 parent 9542d48 commit 34d1f1f

File tree

3 files changed

+7
-64
lines changed

3 files changed

+7
-64
lines changed

‎src/integrationTest/java/org/opensearch/security/privileges/ActionPrivilegesTest.java

+3-27
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@
4141
import org.opensearch.security.securityconf.impl.CType;
4242
import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration;
4343
import org.opensearch.security.securityconf.impl.v7.RoleV7;
44-
import org.opensearch.security.support.WildcardMatcher;
4544
import org.opensearch.security.user.User;
4645
import org.opensearch.security.util.MockIndexMetadataBuilder;
4746

@@ -784,7 +783,7 @@ public void relevantOnly_identity() throws Exception {
784783

785784
assertTrue(
786785
"relevantOnly() returned identical object",
787-
ActionPrivileges.StatefulIndexPrivileges.relevantOnly(metadata, null) == metadata
786+
ActionPrivileges.StatefulIndexPrivileges.relevantOnly(metadata) == metadata
788787
);
789788
}
790789

@@ -798,7 +797,7 @@ public void relevantOnly_closed() throws Exception {
798797
assertNotNull("Original metadata contains index_open_1", metadata.get("index_open_1"));
799798
assertNotNull("Original metadata contains index_closed", metadata.get("index_closed"));
800799

801-
Map<String, IndexAbstraction> filteredMetadata = ActionPrivileges.StatefulIndexPrivileges.relevantOnly(metadata, null);
800+
Map<String, IndexAbstraction> filteredMetadata = ActionPrivileges.StatefulIndexPrivileges.relevantOnly(metadata);
802801

803802
assertNotNull("Filtered metadata contains index_open_1", filteredMetadata.get("index_open_1"));
804803
assertNull("Filtered metadata does not contain index_closed", filteredMetadata.get("index_closed"));
@@ -811,35 +810,12 @@ public void relevantOnly_dataStreamBackingIndices() throws Exception {
811810
assertNotNull("Original metadata contains backing index", metadata.get(".ds-data_stream_1-000001"));
812811
assertNotNull("Original metadata contains data stream", metadata.get("data_stream_1"));
813812

814-
Map<String, IndexAbstraction> filteredMetadata = ActionPrivileges.StatefulIndexPrivileges.relevantOnly(metadata, null);
813+
Map<String, IndexAbstraction> filteredMetadata = ActionPrivileges.StatefulIndexPrivileges.relevantOnly(metadata);
815814

816815
assertNull("Filtered metadata does not contain backing index", filteredMetadata.get(".ds-data_stream_1-000001"));
817816
assertNotNull("Filtered metadata contains data stream", filteredMetadata.get("data_stream_1"));
818817
}
819818

820-
@Test
821-
public void relevantOnly_includePattern() throws Exception {
822-
Map<String, IndexAbstraction> metadata = //
823-
indices("index_a11", "index_a12", "index_b1")//
824-
.alias("alias_a")
825-
.of("index_a11")//
826-
.build()
827-
.getIndicesLookup();
828-
829-
assertNotNull("Original metadata contains index_a11", metadata.get("index_a11"));
830-
assertNotNull("Original metadata contains index_b1", metadata.get("index_b1"));
831-
assertNotNull("Original metadata contains alias_a", metadata.get("alias_a"));
832-
833-
Map<String, IndexAbstraction> filteredMetadata = ActionPrivileges.StatefulIndexPrivileges.relevantOnly(
834-
metadata,
835-
WildcardMatcher.from("index_a*", "alias_a*")
836-
);
837-
838-
assertNotNull("Filtered metadata contains index_a11", filteredMetadata.get("index_a11"));
839-
assertNull("Filtered metadata does not contain index_b1", filteredMetadata.get("index_b1"));
840-
assertNotNull("Filtered metadata contains alias_a", filteredMetadata.get("alias_a"));
841-
}
842-
843819
@Test
844820
public void backingIndexToDataStream() {
845821
Map<String, IndexAbstraction> metadata = indices("index").dataStream("data_stream").build().getIndicesLookup();

‎src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java

-1
Original file line numberDiff line numberDiff line change
@@ -2040,7 +2040,6 @@ public List<Setting<?>> getSettings() {
20402040

20412041
// Privileges evaluation
20422042
settings.add(ActionPrivileges.PRECOMPUTED_PRIVILEGES_MAX_HEAP_SIZE);
2043-
settings.add(ActionPrivileges.PRECOMPUTED_PRIVILEGES_INCLUDE_INDICES);
20442043
}
20452044

20462045
return settings;

‎src/main/java/org/opensearch/security/privileges/ActionPrivileges.java

+4-36
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import java.util.function.Supplier;
2222
import java.util.stream.Collectors;
2323

24-
import com.google.common.base.Strings;
2524
import com.google.common.collect.ImmutableMap;
2625
import com.google.common.collect.ImmutableSet;
2726
import org.apache.commons.collections4.CollectionUtils;
@@ -64,29 +63,13 @@ public class ActionPrivileges extends ClusterStateMetadataDependentPrivileges {
6463
* This settings defaults to 10 MB. This is a generous limit. Experiments have shown that an example setup with
6564
* 10,000 indices and 1,000 roles requires about 1 MB of heap. 100,000 indices and 100 roles require about 9 MB of heap.
6665
* (Of course, these numbers can vary widely based on the actual role configuration).
67-
* <p>
68-
* The setting plugins.security.privileges_evaluation.precomputed_privileges.include_indices can be used to control
69-
* for which indices the precomputed privileges shall be created. This allows to lower the heap utilization.
7066
*/
7167
public static Setting<ByteSizeValue> PRECOMPUTED_PRIVILEGES_MAX_HEAP_SIZE = Setting.memorySizeSetting(
7268
"plugins.security.privileges_evaluation.precomputed_privileges.max_heap_size",
7369
new ByteSizeValue(10, ByteSizeUnit.MB),
7470
Setting.Property.NodeScope
7571
);
7672

77-
/**
78-
* Determines the indices which shall be included in the precomputed index privileges. Included indices get
79-
* the fasted privilege evaluation.
80-
* <p>
81-
* You can use patterns like "index_*".
82-
* <p>
83-
* Defaults to all indices.
84-
*/
85-
public static Setting<String> PRECOMPUTED_PRIVILEGES_INCLUDE_INDICES = Setting.simpleString(
86-
"plugins.security.privileges_evaluation.precomputed_privileges.include_indices",
87-
Setting.Property.NodeScope
88-
);
89-
9073
private static final Logger log = LogManager.getLogger(ActionPrivileges.class);
9174

9275
private final ClusterPrivileges cluster;
@@ -97,7 +80,6 @@ public class ActionPrivileges extends ClusterStateMetadataDependentPrivileges {
9780
private final ImmutableSet<String> wellKnownIndexActions;
9881
private final Supplier<Map<String, IndexAbstraction>> indexMetadataSupplier;
9982
private final ByteSizeValue statefulIndexMaxHeapSize;
100-
private final WildcardMatcher statefulIndexIncludeIndices;
10183

10284
private final AtomicReference<StatefulIndexPrivileges> statefulIndex = new AtomicReference<>();
10385

@@ -118,10 +100,6 @@ public ActionPrivileges(
118100
this.wellKnownIndexActions = wellKnownIndexActions;
119101
this.indexMetadataSupplier = indexMetadataSupplier;
120102
this.statefulIndexMaxHeapSize = PRECOMPUTED_PRIVILEGES_MAX_HEAP_SIZE.get(settings);
121-
String statefulIndexIncludeIndices = PRECOMPUTED_PRIVILEGES_INCLUDE_INDICES.get(settings);
122-
this.statefulIndexIncludeIndices = Strings.isNullOrEmpty(statefulIndexIncludeIndices)
123-
? null
124-
: WildcardMatcher.from(statefulIndexIncludeIndices);
125103
}
126104

127105
public ActionPrivileges(
@@ -241,7 +219,7 @@ public PrivilegesEvaluatorResponse hasExplicitIndexPrivilege(
241219
void updateStatefulIndexPrivileges(Map<String, IndexAbstraction> indices, long metadataVersion) {
242220
StatefulIndexPrivileges statefulIndex = this.statefulIndex.get();
243221

244-
indices = StatefulIndexPrivileges.relevantOnly(indices, statefulIndexIncludeIndices);
222+
indices = StatefulIndexPrivileges.relevantOnly(indices);
245223

246224
if (statefulIndex == null || !statefulIndex.indices.equals(indices)) {
247225
long start = System.currentTimeMillis();
@@ -1004,10 +982,9 @@ static class StatefulIndexPrivileges {
1004982
.getEstimatedByteSize() > statefulIndexMaxHeapSize.getBytes()) {
1005983
log.info(
1006984
"Size of precomputed index privileges exceeds configured limit ({}). Using capped data structure."
1007-
+ "This might lead to slightly lower performance during privilege evaluation. Consider raising {} or limiting the performance critical indices using {}.",
985+
+ "This might lead to slightly lower performance during privilege evaluation. Consider raising {}.",
1008986
statefulIndexMaxHeapSize,
1009-
PRECOMPUTED_PRIVILEGES_MAX_HEAP_SIZE.getKey(),
1010-
PRECOMPUTED_PRIVILEGES_INCLUDE_INDICES.getKey()
987+
PRECOMPUTED_PRIVILEGES_MAX_HEAP_SIZE.getKey()
1011988
);
1012989
break top;
1013990
}
@@ -1125,16 +1102,11 @@ static String backingIndexToDataStream(String index, Map<String, IndexAbstractio
11251102
* <li>Indices which are not matched by includeIndices
11261103
* </ul>
11271104
*/
1128-
static Map<String, IndexAbstraction> relevantOnly(Map<String, IndexAbstraction> indices, WildcardMatcher includeIndices) {
1105+
static Map<String, IndexAbstraction> relevantOnly(Map<String, IndexAbstraction> indices) {
11291106
// First pass: Check if we need to filter at all
11301107
boolean doFilter = false;
11311108

11321109
for (IndexAbstraction indexAbstraction : indices.values()) {
1133-
if (includeIndices != null && !includeIndices.test(indexAbstraction.getName())) {
1134-
doFilter = true;
1135-
break;
1136-
}
1137-
11381110
if (indexAbstraction instanceof IndexAbstraction.Index) {
11391111
if (indexAbstraction.getParentDataStream() != null
11401112
|| indexAbstraction.getWriteIndex().getState() == IndexMetadata.State.CLOSE) {
@@ -1152,10 +1124,6 @@ static Map<String, IndexAbstraction> relevantOnly(Map<String, IndexAbstraction>
11521124
ImmutableMap.Builder<String, IndexAbstraction> builder = ImmutableMap.builder();
11531125

11541126
for (IndexAbstraction indexAbstraction : indices.values()) {
1155-
if (includeIndices != null && !includeIndices.test(indexAbstraction.getName())) {
1156-
continue;
1157-
}
1158-
11591127
if (indexAbstraction instanceof IndexAbstraction.Index) {
11601128
if (indexAbstraction.getParentDataStream() == null
11611129
&& indexAbstraction.getWriteIndex().getState() != IndexMetadata.State.CLOSE) {

0 commit comments

Comments
 (0)