Fix geoip databases index access after system feature migration (elastic#121196) (elastic#121946)

Co-authored-by: Keith Massey <[email protected]>

Author: joegallo

docs/changelog/121196.yaml

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

modules/ingest-geoip/qa/full-cluster-restart/src/javaRestTest/java/org/elasticsearch/ingest/geoip/FullClusterRestartIT.java

@@ -119,10 +119,16 @@ public void testGeoIpSystemFeaturesMigration() throws Exception {
 
             // before the upgrade, Kibana should work
             assertBusy(() -> testGetStarAsKibana(List.of("my-index-00001"), maybeSecurityIndex));
+
+            // as should a normal get *
+            assertBusy(() -> testGetStar(List.of("my-index-00001"), maybeSecurityIndex));
         } else {
             // after the upgrade, but before the migration, Kibana should work
             assertBusy(() -> testGetStarAsKibana(List.of("my-index-00001"), maybeSecurityIndex));
 
+            // as should a normal get *
+            assertBusy(() -> testGetStar(List.of("my-index-00001"), maybeSecurityIndex));
+
             // migrate the system features and give the cluster a moment to settle
             Request migrateSystemFeatures = new Request("POST", "/_migration/system_features");
             assertOK(client().performRequest(migrateSystemFeatures));
@@ -132,9 +138,10 @@ public void testGeoIpSystemFeaturesMigration() throws Exception {
             assertBusy(() -> testIndexGeoDoc());
 
             // after the migration, Kibana should work
-            if (useSecurity == false) { // BUT IT DOESN'T if security is enabled
-                assertBusy(() -> testGetStarAsKibana(List.of("my-index-00001"), maybeSecurityIndexReindexed));
-            }
+            assertBusy(() -> testGetStarAsKibana(List.of("my-index-00001"), maybeSecurityIndexReindexed));
+
+            // as should a normal get *
+            assertBusy(() -> testGetStar(List.of("my-index-00001"), maybeSecurityIndexReindexed));
 
             Request disableDownloader = new Request("PUT", "/_cluster/settings");
             disableDownloader.setJsonEntity("""
@@ -213,6 +220,23 @@ private void testIndexGeoDoc() throws IOException {
         assertEquals("Sweden", doc.evaluate("_source.geo.country_name"));
     }
 
+    private void testGetStar(List<String> indexNames, @Nullable List<String> additionalIndexNames) throws IOException {
+        Request getStar = new Request("GET", "*?expand_wildcards=all");
+        getStar.setOptions(
+            RequestOptions.DEFAULT.toBuilder().setWarningsHandler(WarningsHandler.PERMISSIVE) // we don't care about warnings, just errors
+        );
+        Response response = client().performRequest(getStar);
+        assertOK(response);
+
+        if (additionalIndexNames != null && additionalIndexNames.isEmpty() == false) {
+            indexNames = new ArrayList<>(indexNames); // recopy into a mutable list
+            indexNames.addAll(additionalIndexNames);
+        }
+
+        Map<String, Object> map = responseAsMap(response);
+        assertThat(map.keySet(), is(new HashSet<>(indexNames)));
+    }
+
     private void testGetStarAsKibana(List<String> indexNames, @Nullable List<String> additionalIndexNames) throws IOException {
         Request getStar = new Request("GET", "*?expand_wildcards=all");
         getStar.setOptions(

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

@@ -168,6 +168,13 @@ 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()) {
+                // check if it is net new
+                if (resolver.getNetNewSystemIndexPredicate().test(indexAbstraction.getName())) {
+                    return isSystemIndexVisible(resolver, indexAbstraction);
+                }
+            }
+
             // it's an alias, ignore expandWildcardsOpen and expandWildcardsClosed.
             // complicated to support those options with aliases pointing to multiple indices...
             isVisible = isVisible && indicesOptions.ignoreAliases() == false;

server/src/test/java/org/elasticsearch/cluster/metadata/IndexAbstractionResolverTests.java

@@ -13,15 +13,24 @@
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
 import org.elasticsearch.core.Tuple;
+import org.elasticsearch.index.IndexVersion;
 import org.elasticsearch.indices.EmptySystemIndices;
 import org.elasticsearch.indices.InvalidIndexNameException;
+import org.elasticsearch.indices.SystemIndexDescriptor;
+import org.elasticsearch.indices.SystemIndices;
 import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.xcontent.XContentBuilder;
 
+import java.io.IOException;
+import java.io.UncheckedIOException;
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Supplier;
 
+import static org.elasticsearch.index.mapper.MapperService.SINGLE_MAPPING_NAME;
+import static org.elasticsearch.indices.SystemIndices.SYSTEM_INDEX_ACCESS_CONTROL_HEADER_KEY;
+import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder;
 import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.either;
@@ -220,13 +229,78 @@ private boolean isIndexVisible(String index, String selector) {
             "*",
             selector,
             index,
-            IndicesOptions.strictExpandOpen(),
+            IndicesOptions.strictExpandHidden(),
             metadata,
             indexNameExpressionResolver,
             true
         );
     }
 
+    public void testIsNetNewSystemIndexVisible() {
+        final Settings settings = Settings.builder()
+            .put("index.number_of_replicas", 0)
+            .put("index.number_of_shards", 1)
+            .put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersion.current())
+            .build();
+
+        final Settings hiddenSettings = Settings.builder().put(settings).put("index.hidden", true).build();
+
+        final IndexMetadata foo = IndexMetadata.builder(".foo").settings(hiddenSettings).system(true).build();
+        final IndexMetadata barReindexed = IndexMetadata.builder(".bar-reindexed")
+            .settings(hiddenSettings)
+            .system(true)
+            .putAlias(AliasMetadata.builder(".bar").isHidden(true).build())
+            .build();
+        final IndexMetadata other = IndexMetadata.builder("other").settings(settings).build();
+
+        final SystemIndexDescriptor fooDescriptor = SystemIndexDescriptor.builder()
+            .setDescription("foo indices")
+            .setOrigin("foo origin")
+            .setPrimaryIndex(".foo")
+            .setIndexPattern(".foo*")
+            .setSettings(settings)
+            .setMappings(mappings())
+            .setNetNew()
+            .build();
+        final SystemIndexDescriptor barDescriptor = SystemIndexDescriptor.builder()
+            .setDescription("bar indices")
+            .setOrigin("bar origin")
+            .setPrimaryIndex(".bar")
+            .setIndexPattern(".bar*")
+            .setSettings(settings)
+            .setMappings(mappings())
+            .setNetNew()
+            .build();
+        final SystemIndices systemIndices = new SystemIndices(
+            List.of(new SystemIndices.Feature("name", "description", List.of(fooDescriptor, barDescriptor)))
+        );
+
+        final ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
+        threadContext.putHeader(SYSTEM_INDEX_ACCESS_CONTROL_HEADER_KEY, "false");
+        indexNameExpressionResolver = new IndexNameExpressionResolver(threadContext, systemIndices);
+        indexAbstractionResolver = new IndexAbstractionResolver(indexNameExpressionResolver);
+
+        metadata = Metadata.builder().put(foo, true).put(barReindexed, true).put(other, true).build();
+
+        assertThat(isIndexVisible("other", "*"), is(true));
+        assertThat(isIndexVisible(".foo", "*"), is(false));
+        assertThat(isIndexVisible(".bar", "*"), is(false));
+    }
+
+    private static XContentBuilder mappings() {
+        try (XContentBuilder builder = jsonBuilder()) {
+            return builder.startObject()
+                .startObject(SINGLE_MAPPING_NAME)
+                .startObject("_meta")
+                .field(SystemIndexDescriptor.VERSION_META_KEY, 0)
+                .endObject()
+                .endObject()
+                .endObject();
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);
+        }
+    }
+
     private List<String> resolveAbstractionsSelectorNotAllowed(List<String> expressions) {
         return resolveAbstractions(expressions, IndicesOptions.strictExpandHiddenNoSelectors(), defaultMask);
     }