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

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;
@@ -33,7 +34,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;
@@ -210,9 +210,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)));
@@ -238,9 +237,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);
@@ -256,23 +254,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);