Review remarks

Signed-off-by: Nils Bandener <[email protected]>

Author: nibix

src/integrationTest/java/org/opensearch/security/privileges/int_tests/IndexAuthorizationReadOnlyIntTests.java

@@ -885,6 +885,8 @@ public void search_alias_pattern() throws Exception {
     public void search_alias_pattern_negation() throws Exception {
         try (TestRestClient restClient = cluster.getRestClient(user)) {
             TestRestClient.HttpResponse httpResponse = restClient.get("alias_*,-alias_ab1/_search?size=1000");
+            // Another interesting effect: The negation on alias names does actually have no effect.
+            // This is this time a bug in core. TODO: File issue
             assertThat(
                 httpResponse,
                 containsExactly(index_a1, index_a2, index_a3, index_b1, index_c1).at("hits.hits[*]._index")

src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java

@@ -525,6 +525,11 @@ public void close() {
         // TODO: Is there anything to clean up here?
     }
 
+    /**
+     * Helper method to create very simple dynamic JSON request bodies.
+     * @param attributes Key-value pairs, keys on even indices, values on odd indices.
+     * @return A request body that can be passed to the get(), post(), etc. methods.
+     */
     public static HttpEntity json(Object... attributes) {
         Map<String, Object> map = new HashMap<>();
 

src/integrationTest/java/org/opensearch/test/framework/data/TestIndexOrAliasOrDatastream.java

@@ -28,18 +28,6 @@ public interface TestIndexOrAliasOrDatastream {
 
     void delete(Client client);
 
-    default TestIndexOrAliasOrDatastream intersection(TestIndexOrAliasOrDatastream other) {
-        if (other == this) {
-            return this;
-        }
-
-        if (!this.name().equals(other.name())) {
-            throw new IllegalArgumentException("Cannot intersect different indices: " + this + " vs " + other);
-        }
-
-        return this;
-    }
-
     static void createInitialTestObjects(LocalCluster cluster, TestIndexOrAliasOrDatastream... testIndexLikeArray) {
         try (Client client = cluster.getInternalNodeClient()) {
             for (TestIndexOrAliasOrDatastream testIndexLike : testIndexLikeArray) {

src/integrationTest/java/org/opensearch/test/framework/matcher/RestIndexMatchers.java

@@ -29,6 +29,7 @@
 import org.hamcrest.Matcher;
 
 import org.opensearch.security.DefaultObjectMapper;
+import org.opensearch.security.support.ConfigConstants;
 import org.opensearch.test.framework.cluster.TestRestClient;
 import org.opensearch.test.framework.data.TestIndex;
 import org.opensearch.test.framework.data.TestIndexOrAliasOrDatastream;
@@ -110,17 +111,17 @@ public static OnResponseIndexMatcher containsExactly(TestIndexOrAliasOrDatastrea
 
         public static OnResponseIndexMatcher containsExactly(Collection<TestIndexOrAliasOrDatastream> testIndices) {
             Map<String, TestIndexOrAliasOrDatastream> indexNameMap = new HashMap<>();
-            boolean containsOpenSearchIndices = false;
+            boolean containsOpenSearchSecurityIndex = false;
 
             for (TestIndexOrAliasOrDatastream testIndex : testIndices) {
                 if (testIndex == TestIndex.openSearchSecurityConfigIndex()) {
-                    containsOpenSearchIndices = true;
+                    containsOpenSearchSecurityIndex = true;
                 } else {
                     indexNameMap.put(testIndex.name(), testIndex);
                 }
             }
 
-            return new ContainsExactlyMatcher(indexNameMap, containsOpenSearchIndices);
+            return new ContainsExactlyMatcher(indexNameMap, containsOpenSearchSecurityIndex);
         }
     }
 
@@ -341,7 +342,7 @@ public int size() {
             if (!containsOpenSearchSecurityIndex) {
                 return expectedIndices.size();
             } else {
-                throw new RuntimeException("Size cannot be exactly specified because containsOpenSearchIndices is true");
+                return expectedIndices.size() + 1;
             }
         }
 
@@ -368,7 +369,7 @@ protected Map<String, TestIndexOrAliasOrDatastream> testIndicesIntersection(
                     continue;
                 }
 
-                result.put(key, index1.intersection(index2));
+                result.put(key, index1);
             }
 
             return Collections.unmodifiableMap(result);
@@ -403,17 +404,17 @@ protected static String formatResponse(TestRestClient.HttpResponse response) {
     static class ContainsExactlyMatcher extends AbstractIndexMatcher implements OnResponseIndexMatcher {
         private static final Pattern DS_BACKING_INDEX_PATTERN = Pattern.compile("\\.ds-(.+)-[0-9]+");
 
-        ContainsExactlyMatcher(Map<String, TestIndexOrAliasOrDatastream> indexNameMap, boolean containsOpenSearchIndices) {
-            super(indexNameMap, containsOpenSearchIndices);
+        ContainsExactlyMatcher(Map<String, TestIndexOrAliasOrDatastream> indexNameMap, boolean containsOpenSearchSecurityIndex) {
+            super(indexNameMap, containsOpenSearchSecurityIndex);
         }
 
         ContainsExactlyMatcher(
             Map<String, TestIndexOrAliasOrDatastream> indexNameMap,
-            boolean containsOpenSearchIndices,
+            boolean containsOpenSearchSecurityIndex,
             String jsonPath,
             RestMatchers.HttpResponseMatcher statusCodeWhenEmpty
         ) {
-            super(indexNameMap, containsOpenSearchIndices, jsonPath, statusCodeWhenEmpty);
+            super(indexNameMap, containsOpenSearchSecurityIndex, jsonPath, statusCodeWhenEmpty);
         }
 
         @Override
@@ -454,7 +455,7 @@ protected boolean matchesByIndices(
             for (Object object : collection) {
                 String index = object.toString();
 
-                if (containsOpenSearchSecurityIndex && (index.startsWith(".opendistro"))) {
+                if (containsOpenSearchSecurityIndex && (index.equals(ConfigConstants.OPENDISTRO_SECURITY_DEFAULT_CONFIG_INDEX))) {
                     seenOpenSearchIndicesBuilder.add(index);
                 } else if (index.startsWith(".ds-")) {
                     // We do a special treatment for data stream backing indices. We convert these to the normal data streams if expected
@@ -708,14 +709,14 @@ public OnUserIndexMatcher and(TestIndexOrAliasOrDatastream... testIndices) {
      */
     static class UnlimitedMatcher extends DiagnosingMatcher<Object> implements OnUserIndexMatcher {
 
-        private final boolean containsOpenSearchIndices;
+        private final boolean containsOpenSearchSecurityIndex;
 
         UnlimitedMatcher() {
-            this.containsOpenSearchIndices = false;
+            this.containsOpenSearchSecurityIndex = false;
         }
 
-        UnlimitedMatcher(boolean containsOpenSearchIndices) {
-            this.containsOpenSearchIndices = containsOpenSearchIndices;
+        UnlimitedMatcher(boolean containsOpenSearchSecurityIndex) {
+            this.containsOpenSearchSecurityIndex = containsOpenSearchSecurityIndex;
         }
 
         @Override
@@ -743,7 +744,7 @@ public boolean isEmpty() {
 
         @Override
         public boolean containsOpenSearchSecurityIndex() {
-            return containsOpenSearchIndices;
+            return containsOpenSearchSecurityIndex;
         }
 
         @Override