Implement system index access level that includes everything except n…

…et-new indices (elastic#74803)

This PR implements a system index access level that includes all system indices, except net-new system indices.

This new access level is leveraged to fix the error in the Get Alias API described in elastic#74687.

server/src/internalClusterTest/java/org/elasticsearch/aliases/NetNewSystemIndexAliasIT.java

@@ -0,0 +1,116 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+package org.elasticsearch.aliases;
+
+import org.elasticsearch.Version;
+import org.elasticsearch.action.admin.indices.alias.get.GetAliasesRequest;
+import org.elasticsearch.action.admin.indices.alias.get.GetAliasesResponse;
+import org.elasticsearch.action.index.IndexRequest;
+import org.elasticsearch.action.index.IndexResponse;
+import org.elasticsearch.cluster.metadata.IndexMetadata;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.util.CollectionUtils;
+import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.indices.SystemIndexDescriptor;
+import org.elasticsearch.plugins.Plugin;
+import org.elasticsearch.plugins.SystemIndexPlugin;
+import org.elasticsearch.test.ESIntegTestCase;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.util.Collection;
+import java.util.Collections;
+
+import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
+import static org.elasticsearch.index.mapper.MapperService.SINGLE_MAPPING_NAME;
+import static org.hamcrest.Matchers.is;
+
+public class NetNewSystemIndexAliasIT extends ESIntegTestCase {
+    public static final String SYSTEM_INDEX_NAME = ".test-system-idx";
+
+    @Override
+    protected Collection<Class<? extends Plugin>> nodePlugins() {
+        return CollectionUtils.appendToCopy(super.nodePlugins(), NetNewSystemIndexTestPlugin.class);
+    }
+
+    public void testGetAliasWithNetNewSystemIndices() throws Exception {
+        // make sure the net-new system index has been created
+        {
+            final IndexRequest request = new IndexRequest(SYSTEM_INDEX_NAME);
+            request.source("some_field", "some_value");
+            IndexResponse resp = client().index(request).get();
+            assertThat(resp.status().getStatus(), is(201));
+        }
+        ensureGreen();
+
+        GetAliasesRequest getAliasesRequest = new GetAliasesRequest();
+        GetAliasesResponse aliasResponse = client().admin().indices().getAliases(getAliasesRequest).get();
+        assertThat(aliasResponse.getAliases().size(), is(0));
+    }
+
+    public static class NetNewSystemIndexTestPlugin extends Plugin implements SystemIndexPlugin {
+
+        public static final Settings SETTINGS = Settings.builder()
+            .put(IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING.getKey(), 1)
+            .put(IndexMetadata.INDEX_AUTO_EXPAND_REPLICAS_SETTING.getKey(), "0-1")
+            .put(IndexMetadata.SETTING_PRIORITY, Integer.MAX_VALUE)
+            .build();
+
+        @Override
+        public Collection<SystemIndexDescriptor> getSystemIndexDescriptors(Settings settings) {
+            try (XContentBuilder builder = jsonBuilder()) {
+                builder.startObject();
+                {
+                    builder.startObject(SINGLE_MAPPING_NAME);
+                    {
+                        builder.startObject("_meta");
+                        builder.field("version", Version.CURRENT.toString());
+                        builder.endObject();
+
+                        builder.field("dynamic", "strict");
+                        builder.startObject("properties");
+                        {
+                            builder.startObject("some_field");
+                            builder.field("type", "keyword");
+                            builder.endObject();
+                        }
+                        builder.endObject();
+                    }
+                    builder.endObject();
+                }
+                builder.endObject();
+
+                return Collections.singletonList(SystemIndexDescriptor.builder()
+                    .setIndexPattern(SYSTEM_INDEX_NAME + "*")
+                    .setPrimaryIndex(SYSTEM_INDEX_NAME)
+                    .setDescription("Test system index")
+                    .setOrigin(getClass().getName())
+                    .setVersionMetaKey("version")
+                    .setMappings(builder)
+                    .setSettings(SETTINGS)
+                    .setType(SystemIndexDescriptor.Type.INTERNAL_MANAGED)
+                    .setNetNew()
+                    .build()
+                );
+            } catch (IOException e) {
+                throw new UncheckedIOException("Failed to build " + SYSTEM_INDEX_NAME + " index mappings", e);
+            }
+        }
+
+        @Override
+        public String getFeatureName() {
+            return this.getClass().getSimpleName();
+        }
+
+        @Override
+        public String getFeatureDescription() {
+            return "test plugin";
+        }
+    }
+}

server/src/main/java/org/elasticsearch/action/admin/indices/create/TransportCreateIndexAction.java

@@ -83,6 +83,7 @@ protected void masterOperation(final CreateIndexRequest request, final ClusterSt
                         throw systemIndices.netNewSystemIndexAccessException(threadPool.getThreadContext(), List.of(indexName));
                     }
                 } else {
+                    // BACKWARDS_COMPATIBLE_ONLY should never be a possibility here, it cannot be returned from getSystemIndexAccessLevel
                     assert systemIndexAccessLevel == SystemIndexAccessLevel.NONE :
                         "Expected no system index access but level is " + systemIndexAccessLevel;
                     throw systemIndices.netNewSystemIndexAccessException(threadPool.getThreadContext(), List.of(indexName));

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

@@ -135,12 +135,18 @@ public static boolean isIndexVisible(String expression, String index, IndicesOpt
             }
             if (indexAbstraction.isSystem()) {
                 final SystemIndexAccessLevel level = resolver.getSystemIndexAccessLevel();
-                if (level == SystemIndexAccessLevel.ALL) {
-                    return true;
-                } else if (level == SystemIndexAccessLevel.NONE) {
-                    return false;
-                } else if (level == SystemIndexAccessLevel.RESTRICTED) {
-                    return resolver.getSystemIndexAccessPredicate().test(indexAbstraction.getName());
+                switch (level) {
+                    case ALL:
+                        return true;
+                    case NONE:
+                        return false;
+                    case RESTRICTED:
+                        return resolver.getSystemIndexAccessPredicate().test(indexAbstraction.getName());
+                    case BACKWARDS_COMPATIBLE_ONLY:
+                        return resolver.getNetNewSystemIndexPredicate().test(indexAbstraction.getName());
+                    default:
+                        assert false : "unexpected system index access level [" + level + "]";
+                        throw new IllegalStateException("unexpected system index access level [" + level + "]");
                 }
             } else {
                 return isVisible;
@@ -160,12 +166,18 @@ public static boolean isIndexVisible(String expression, String index, IndicesOpt
             // check if it is net new
             if (resolver.getNetNewSystemIndexPredicate().test(indexAbstraction.getName())) {
                 final SystemIndexAccessLevel level = resolver.getSystemIndexAccessLevel();
-                if (level == SystemIndexAccessLevel.ALL) {
-                    return true;
-                } else if (level == SystemIndexAccessLevel.NONE) {
-                    return false;
-                } else if (level == SystemIndexAccessLevel.RESTRICTED) {
-                    return resolver.getSystemIndexAccessPredicate().test(indexAbstraction.getName());
+                switch (level) {
+                    case ALL:
+                        return true;
+                    case NONE:
+                        return false;
+                    case RESTRICTED:
+                        return resolver.getSystemIndexAccessPredicate().test(indexAbstraction.getName());
+                    case BACKWARDS_COMPATIBLE_ONLY:
+                        return resolver.getNetNewSystemIndexPredicate().test(indexAbstraction.getName());
+                    default:
+                        assert false : "unexpected system index access level [" + level + "]";
+                        throw new IllegalStateException("unexpected system index access level [" + level + "]");
                 }
             }
 
@@ -176,12 +188,18 @@ public static boolean isIndexVisible(String expression, String index, IndicesOpt
                     throw new IllegalStateException("system index is part of a data stream that is not a system data stream");
                 }
                 final SystemIndexAccessLevel level = resolver.getSystemIndexAccessLevel();
-                if (level == SystemIndexAccessLevel.ALL) {
-                    return true;
-                } else if (level == SystemIndexAccessLevel.NONE) {
-                    return false;
-                } else if (level == SystemIndexAccessLevel.RESTRICTED) {
-                    return resolver.getSystemIndexAccessPredicate().test(indexAbstraction.getName());
+                switch (level) {
+                    case ALL:
+                        return true;
+                    case NONE:
+                        return false;
+                    case RESTRICTED:
+                        return resolver.getSystemIndexAccessPredicate().test(indexAbstraction.getName());
+                    case BACKWARDS_COMPATIBLE_ONLY:
+                        return resolver.getNetNewSystemIndexPredicate().test(indexAbstraction.getName());
+                    default:
+                        assert false : "unexpected system index access level [" + level + "]";
+                        throw new IllegalStateException("unexpected system index access level [" + level + "]");
                 }
             }
         }

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

@@ -88,7 +88,7 @@ public String[] concreteIndexNames(ClusterState state, IndicesRequest request) {
      */
     public String[] concreteIndexNamesWithSystemIndexAccess(ClusterState state, IndicesRequest request) {
         Context context = new Context(state, request.indicesOptions(), false, false, request.includeDataStreams(),
-            SystemIndexAccessLevel.ALL, name -> true, name -> false);
+            SystemIndexAccessLevel.BACKWARDS_COMPATIBLE_ONLY, name -> true, this.getNetNewSystemIndexPredicate());
         return concreteIndexNames(context, request.indices());
     }
 
@@ -133,11 +133,6 @@ public String[] concreteIndexNames(ClusterState state, IndicesOptions options, I
         return concreteIndexNames(context, request.indices());
     }
 
-    public String[] concreteIndexNamesWithSystemIndexAccess(ClusterState state, IndicesOptions options, String... indexExpressions) {
-        Context context = new Context(state, options, SystemIndexAccessLevel.ALL, name -> true, name -> false);
-        return concreteIndexNames(context, indexExpressions);
-    }
-
     public List<String> dataStreamNames(ClusterState state, IndicesOptions options, String... indexExpressions) {
         Context context = new Context(state, options, false, false, true, true, getSystemIndexAccessLevel(),
             getSystemIndexAccessPredicate(), getNetNewSystemIndexPredicate());
@@ -395,6 +390,11 @@ private void checkSystemIndexAccess(Context context, Metadata metadata, Set<Inde
     }
 
     private static boolean shouldTrackConcreteIndex(Context context, IndicesOptions options, IndexMetadata index) {
+        if (context.systemIndexAccessLevel == SystemIndexAccessLevel.BACKWARDS_COMPATIBLE_ONLY
+            && context.netNewSystemIndexPredicate.test(index.getIndex().getName())) {
+            // Exclude this one as it's a net-new system index, and we explicitly don't want those.
+            return false;
+        }
         if (index.getState() == IndexMetadata.State.CLOSE) {
             if (options.forbidClosedIndices() && options.ignoreUnavailable() == false) {
                 throw new IndexClosedException(index.getIndex());
@@ -774,14 +774,19 @@ boolean isPatternMatchingAllIndices(Metadata metadata, String[] indicesOrAliases
     }
 
     public SystemIndexAccessLevel getSystemIndexAccessLevel() {
-        return systemIndices.getSystemIndexAccessLevel(threadContext);
+        final SystemIndexAccessLevel accessLevel = systemIndices.getSystemIndexAccessLevel(threadContext);
+        assert accessLevel != SystemIndexAccessLevel.BACKWARDS_COMPATIBLE_ONLY
+            : "BACKWARDS_COMPATIBLE_ONLY access level should never be used automatically, it should only be used in known special cases";
+        return accessLevel;
     }
 
     public Predicate<String> getSystemIndexAccessPredicate() {
         final SystemIndexAccessLevel systemIndexAccessLevel = getSystemIndexAccessLevel();
         final Predicate<String> systemIndexAccessLevelPredicate;
         if (systemIndexAccessLevel == SystemIndexAccessLevel.NONE) {
             systemIndexAccessLevelPredicate = s -> false;
+        } else if (systemIndexAccessLevel == SystemIndexAccessLevel.BACKWARDS_COMPATIBLE_ONLY) {
+            systemIndexAccessLevelPredicate = getNetNewSystemIndexPredicate();
         } else if (systemIndexAccessLevel == SystemIndexAccessLevel.ALL) {
             systemIndexAccessLevelPredicate = s -> true;
         } else {
@@ -1183,7 +1188,11 @@ private static List<String> resolveEmptyOrTrivialWildcardWithAllowedSystemIndice
                             assert abstraction != null : "null abstraction for " + name + " but was in array of all indices";
                             if (abstraction.isSystem()) {
                                 if (context.netNewSystemIndexPredicate.test(name)) {
-                                    return context.systemIndexAccessPredicate.test(name);
+                                    if (SystemIndexAccessLevel.BACKWARDS_COMPATIBLE_ONLY.equals(context.systemIndexAccessLevel)) {
+                                        return false;
+                                    } else {
+                                        return context.systemIndexAccessPredicate.test(name);
+                                    }
                                 } else if (abstraction.getType() == Type.DATA_STREAM || abstraction.getParentDataStream() != null) {
                                     return context.systemIndexAccessPredicate.test(name);
                                 }

server/src/main/java/org/elasticsearch/indices/SystemIndices.java

@@ -353,6 +353,7 @@ public SystemDataStreamDescriptor validateDataStreamAccess(String dataStreamName
                 .orElseThrow(() -> new IllegalStateException("system data stream descriptor not found for [" + dataStreamName + "]"));
             if (dataStreamDescriptor.isExternal()) {
                 final SystemIndexAccessLevel accessLevel = getSystemIndexAccessLevel(threadContext);
+                assert accessLevel != SystemIndexAccessLevel.BACKWARDS_COMPATIBLE_ONLY : "BACKWARDS_COMPATIBLE access level is leaking";
                 if (accessLevel == SystemIndexAccessLevel.NONE) {
                     throw dataStreamAccessException(null, dataStreamName);
                 } else if (accessLevel == SystemIndexAccessLevel.RESTRICTED) {
@@ -364,7 +365,7 @@ public SystemDataStreamDescriptor validateDataStreamAccess(String dataStreamName
                         return dataStreamDescriptor;
                     }
                 } else {
-                    assert accessLevel == SystemIndexAccessLevel.ALL;
+                    assert accessLevel == SystemIndexAccessLevel.ALL || accessLevel == SystemIndexAccessLevel.BACKWARDS_COMPATIBLE_ONLY;
                     return dataStreamDescriptor;
                 }
             } else {
@@ -411,6 +412,8 @@ IllegalArgumentException dataStreamAccessException(@Nullable String product, Str
      * {@link SystemIndexAccessLevel#NONE} if no system index access should be allowed.
      */
     public SystemIndexAccessLevel getSystemIndexAccessLevel(ThreadContext threadContext) {
+        // This method intentionally cannot return BACKWARDS_COMPATIBLE_ONLY - that access level should only be used manually
+        // in known special cases.
         final String headerValue = threadContext.getHeader(SYSTEM_INDEX_ACCESS_CONTROL_HEADER_KEY);
         final String productHeaderValue = threadContext.getHeader(EXTERNAL_SYSTEM_INDEX_ACCESS_CONTROL_HEADER_KEY);
 
@@ -429,7 +432,15 @@ public SystemIndexAccessLevel getSystemIndexAccessLevel(ThreadContext threadCont
     public enum SystemIndexAccessLevel {
         ALL,
         NONE,
-        RESTRICTED
+        RESTRICTED,
+        /**
+         * This value exists because there was a desire for "net-new" system indices to opt in to the post-8.0 behavior of having
+         * access blocked in most cases, but this caused problems with certain APIs
+         * (see https://github.com/elastic/elasticsearch/issues/74687), so this access level was added as a workaround. Once we no longer
+         * have to support accessing existing system indices, this can and should be removed, along with the net-new property of
+         * system indices in general.
+         */
+        BACKWARDS_COMPATIBLE_ONLY
     }
 
     /**

server/src/test/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesActionTests.java

@@ -16,9 +16,9 @@
 import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
 import org.elasticsearch.cluster.metadata.Metadata;
 import org.elasticsearch.common.collect.ImmutableOpenMap;
-import org.elasticsearch.core.Tuple;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
+import org.elasticsearch.core.Tuple;
 import org.elasticsearch.indices.EmptySystemIndices;
 import org.elasticsearch.indices.SystemIndexDescriptor;
 import org.elasticsearch.indices.SystemIndices;
@@ -225,6 +225,47 @@ public void testPostProcessDataStreamAliases() {
         assertThat(result.get("logs-foo"), contains(new DataStreamAlias("logs",  Arrays.asList("logs-bar", "logs-foo"), null)));
     }
 
+    public void testNetNewSystemIndicesDontErrorWhenNotRequested() {
+        GetAliasesRequest aliasesRequest = new GetAliasesRequest();
+        // `.b` will be the "net new" system index this test case
+        ClusterState clusterState = systemIndexTestClusterState();
+        String[] concreteIndices;
+
+        SystemIndexDescriptor netNewDescriptor = SystemIndexDescriptor.builder()
+            .setIndexPattern(".b")
+            .setAliasName(".y")
+            .setPrimaryIndex(".b")
+            .setDescription(this.getTestName())
+            .setMappings("{\"_meta\":  {\"version\":  \"1.0.0\"}}")
+            .setSettings(Settings.EMPTY)
+            .setVersionMetaKey("version")
+            .setOrigin(this.getTestName())
+            .setNetNew()
+            .build();
+        SystemIndices systemIndices = new SystemIndices(Collections.singletonMap(
+            this.getTestName(),
+            new SystemIndices.Feature(this.getTestName(), "test feature",
+                Collections.singletonList(netNewDescriptor))));
+
+        final ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
+        final IndexNameExpressionResolver indexNameExpressionResolver = new IndexNameExpressionResolver(threadContext, systemIndices);
+        concreteIndices = indexNameExpressionResolver.concreteIndexNamesWithSystemIndexAccess(clusterState, aliasesRequest);
+
+        ImmutableOpenMap<String, List<AliasMetadata>> initialAliases = ImmutableOpenMap.<String, List<AliasMetadata>>builder().build();
+        ImmutableOpenMap<String, List<AliasMetadata>> finalResponse = TransportGetAliasesAction.postProcess(
+            aliasesRequest,
+            concreteIndices,
+            initialAliases,
+            clusterState,
+            SystemIndexAccessLevel.NONE,
+            threadContext,
+            systemIndices
+        );
+
+        // The real assertion is that the above `postProcess` call doesn't throw
+        assertFalse(finalResponse.containsKey(".b"));
+    }
+
     public ClusterState systemIndexTestClusterState() {
         return ClusterState.builder(ClusterState.EMPTY_STATE)
             .metadata(Metadata.builder()