Fix geoip databases index access after system feature migration (take 3) (#124604) (#124681)

Author: joegallo

docs/changelog/124604.yaml

@@ -0,0 +1,5 @@
+pr: 124604
+summary: Fix geoip databases index access after system feature migration (take 3)
+area: Ingest Node
+type: bug
+issues: []

modules/ingest-geoip/qa/geoip-reindexed/src/javaRestTest/java/org/elasticsearch/ingest/geoip/GeoIpReindexedIT.java

@@ -18,6 +18,7 @@
 import org.elasticsearch.client.Response;
 import org.elasticsearch.client.WarningsHandler;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.util.CollectionUtils;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
 import org.elasticsearch.core.Nullable;
 import org.elasticsearch.rest.RestStatus;
@@ -34,7 +35,6 @@
 
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
-import java.util.ArrayList;
 import java.util.Base64;
 import java.util.HashSet;
 import java.util.List;
@@ -212,9 +212,8 @@ private void testCatIndices(List<String> indexNames, @Nullable List<String> addi
         String response = EntityUtils.toString(assertOK(client().performRequest(catIndices)).getEntity());
         List<String> indices = List.of(response.trim().split("\\s+"));
 
-        if (additionalIndexNames != null && additionalIndexNames.isEmpty() == false) {
-            indexNames = new ArrayList<>(indexNames); // recopy into a mutable list
-            indexNames.addAll(additionalIndexNames);
+        if (additionalIndexNames != null) {
+            indexNames = CollectionUtils.concatLists(indexNames, additionalIndexNames);
         }
 
         assertThat(new HashSet<>(indices), is(new HashSet<>(indexNames)));
@@ -240,9 +239,8 @@ private void testGetStar(List<String> indexNames, @Nullable List<String> additio
         );
         Response response = assertOK(client().performRequest(getStar));
 
-        if (additionalIndexNames != null && additionalIndexNames.isEmpty() == false) {
-            indexNames = new ArrayList<>(indexNames); // recopy into a mutable list
-            indexNames.addAll(additionalIndexNames);
+        if (additionalIndexNames != null) {
+            indexNames = CollectionUtils.concatLists(indexNames, additionalIndexNames);
         }
 
         Map<String, Object> map = responseAsMap(response);
@@ -258,23 +256,38 @@ private void testGetStarAsKibana(List<String> indexNames, @Nullable List<String>
         );
         Response response = assertOK(client().performRequest(getStar));
 
-        if (additionalIndexNames != null && additionalIndexNames.isEmpty() == false) {
-            indexNames = new ArrayList<>(indexNames); // recopy into a mutable list
-            indexNames.addAll(additionalIndexNames);
+        if (additionalIndexNames != null) {
+            indexNames = CollectionUtils.concatLists(indexNames, additionalIndexNames);
         }
 
         Map<String, Object> map = responseAsMap(response);
         assertThat(map.keySet(), is(new HashSet<>(indexNames)));
     }
 
     private void testGetDatastreams() throws IOException {
-        Request getStar = new Request("GET", "_data_stream");
-        getStar.setOptions(
-            RequestOptions.DEFAULT.toBuilder().setWarningsHandler(WarningsHandler.PERMISSIVE) // we don't care about warnings, just errors
+        final List<List<String>> wildcardOptions = List.of(
+            List.of(), // the default for expand_wildcards (that is, the option is not specified)
+            List.of("all"),
+            List.of("none"),
+            List.of("hidden"),
+            List.of("open"),
+            List.of("closed"),
+            List.of("hidden", "open"),
+            List.of("hidden", "closed"),
+            List.of("open", "closed")
         );
-        Response response = client().performRequest(getStar);
-        assertOK(response);
-
-        // note: we don't actually care about the response, just that there was one and that it didn't error out on us
+        for (List<String> expandWildcards : wildcardOptions) {
+            final Request getStar = new Request(
+                "GET",
+                "_data_stream" + (expandWildcards.isEmpty() ? "" : ("?expand_wildcards=" + String.join(",", expandWildcards)))
+            );
+            getStar.setOptions(
+                RequestOptions.DEFAULT.toBuilder().setWarningsHandler(WarningsHandler.PERMISSIVE) // we only care about errors
+            );
+            Response response = client().performRequest(getStar);
+            assertOK(response);
+
+            // note: we don't actually care about the response, just that there was one and that it didn't error out on us
+        }
     }
 }

server/src/main/java/org/elasticsearch/action/datastreams/DataStreamsActionUtil.java

@@ -10,6 +10,7 @@
 package org.elasticsearch.action.datastreams;
 
 import org.elasticsearch.action.support.IndicesOptions;
+import org.elasticsearch.action.support.IndicesOptions.WildcardOptions;
 import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.metadata.DataStream;
 import org.elasticsearch.cluster.metadata.IndexAbstraction;
@@ -40,9 +41,10 @@ public static List<String> getDataStreamNames(
     }
 
     public static IndicesOptions updateIndicesOptions(IndicesOptions indicesOptions) {
+        // if expandWildcardsOpen=false, then it will be overridden to true
         if (indicesOptions.expandWildcardsOpen() == false) {
             indicesOptions = IndicesOptions.builder(indicesOptions)
-                .wildcardOptions(IndicesOptions.WildcardOptions.builder(indicesOptions.wildcardOptions()).matchOpen(true))
+                .wildcardOptions(WildcardOptions.builder(indicesOptions.wildcardOptions()).matchOpen(true))
                 .build();
         }
         return indicesOptions;

server/src/main/java/org/elasticsearch/cluster/metadata/IndexAbstractionResolver.java

@@ -156,7 +156,11 @@ public static boolean isIndexVisible(
         final boolean isHidden = indexAbstraction.isHidden();
         boolean isVisible = isHidden == false || indicesOptions.expandWildcardsHidden() || isVisibleDueToImplicitHidden(expression, index);
         if (indexAbstraction.getType() == IndexAbstraction.Type.ALIAS) {
-            if (indexAbstraction.isSystem()) {
+            // it's an alias, ignore expandWildcardsOpen and expandWildcardsClosed.
+            // it's complicated to support those options with aliases pointing to multiple indices...
+            isVisible = isVisible && indicesOptions.ignoreAliases() == false;
+
+            if (isVisible && indexAbstraction.isSystem()) {
                 // check if it is net new
                 if (resolver.getNetNewSystemIndexPredicate().test(indexAbstraction.getName())) {
                     // don't give this code any particular credit for being *correct*. it's just trying to resolve a combination of
@@ -182,9 +186,6 @@ public static boolean isIndexVisible(
                 }
             }
 
-            // it's an alias, ignore expandWildcardsOpen and expandWildcardsClosed.
-            // complicated to support those options with aliases pointing to multiple indices...
-            isVisible = isVisible && indicesOptions.ignoreAliases() == false;
             if (isVisible && selectorString != null) {
                 // Check if a selector was present, and if it is, check if this alias is applicable to it
                 IndexComponentSelector selector = IndexComponentSelector.getByKey(selectorString);