Add a flag to set whether a system index is readable on SystemIndexDescriptor

CHANGELOG.md

@@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Introduce a setting to disable download of full cluster state from remote on term mismatch([#16798](https://github.com/opensearch-project/OpenSearch/pull/16798/))
 - Added ability to retrieve value from DocValues in a flat_object filed([#16802](https://github.com/opensearch-project/OpenSearch/pull/16802))
 - Improve performace of NumericTermAggregation by avoiding unnecessary sorting([#17252](https://github.com/opensearch-project/OpenSearch/pull/17252))
+- Add a flag to set whether a system index is readable on SystemIndexDescriptor ([#17296](https://github.com/opensearch-project/OpenSearch/pull/17296))
 
 ### Dependencies
 - Bump `org.awaitility:awaitility` from 4.2.0 to 4.2.2 ([#17230](https://github.com/opensearch-project/OpenSearch/pull/17230))

plugins/examples/rest-handler/src/main/java/org/opensearch/example/resthandler/ExampleRestHandlerPlugin.java

@@ -38,11 +38,15 @@
 import org.opensearch.common.settings.IndexScopedSettings;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.settings.SettingsFilter;
+import org.opensearch.indices.SystemIndexDescriptor;
 import org.opensearch.plugins.ActionPlugin;
+import org.opensearch.plugins.ClusterPlugin;
 import org.opensearch.plugins.Plugin;
+import org.opensearch.plugins.SystemIndexPlugin;
 import org.opensearch.rest.RestController;
 import org.opensearch.rest.RestHandler;
 
+import java.util.Collection;
 import java.util.List;
 import java.util.function.Supplier;
 
@@ -51,7 +55,17 @@
 /**
  * A plugin demonstrating the implementation of a new Rest Handler.
  */
-public class ExampleRestHandlerPlugin extends Plugin implements ActionPlugin {
+public class ExampleRestHandlerPlugin extends Plugin implements ActionPlugin, SystemIndexPlugin, ClusterPlugin {
+
+    /**
+     * The name of the readable system index.
+     */
+    public static final String READABLE_SYSTEM_INDEX_NAME = ".readable-system-index";
+
+    /**
+     * The name of the non-readable system index.
+     */
+    public static final String NONREADABLE_SYSTEM_INDEX_NAME = ".nonreadable-system-index";
 
     /**
      * Instantiate this plugin.
@@ -68,7 +82,14 @@ public List<RestHandler> getRestHandlers(
         final IndexNameExpressionResolver indexNameExpressionResolver,
         final Supplier<DiscoveryNodes> nodesInCluster
     ) {
-
         return singletonList(new ExampleCatAction());
     }
+
+    @Override
+    public Collection<SystemIndexDescriptor> getSystemIndexDescriptors(Settings settings) {
+        return List.of(
+            new SystemIndexDescriptor(READABLE_SYSTEM_INDEX_NAME, "Readable system index for tests", true),
+            new SystemIndexDescriptor(NONREADABLE_SYSTEM_INDEX_NAME, "Non-Readable system index for tests")
+        );
+    }
 }

plugins/examples/rest-handler/src/yamlRestTest/resources/rest-api-spec/test/resthandler/30_readable_system_index.yml

@@ -0,0 +1,39 @@
+---
+setup:
+  - skip:
+      features: allowed_warnings
+  - do:
+      indices.create:
+        index: .readable-system-index
+
+  - do:
+      allowed_warnings:
+        - "this request accesses system indices: [.readable-system-index], but in a future major version, direct access to system indices will be prevented by default"
+      index:
+        index: .readable-system-index
+        id: 1
+        refresh: true
+        body: { "test": 1 }
+
+---
+"Test that .readable-system-index does not output deprecation log on search":
+  - skip:
+      features: [warnings, headers]
+  - do:
+      warnings: []
+      headers: { "X-Opaque-Id": "search-request" }
+      search:
+        rest_total_hits_as_int: true
+        index: .readable-system-index
+        body:
+          query:
+            match_all: { }
+  - match: { hits.total: 1 }
+
+  - do:
+      warnings: [ ]
+      headers: { "X-Opaque-Id": "get-request" }
+      get:
+        index: .readable-system-index
+        id: 1
+  - match: { _source.test: 1 }

plugins/examples/rest-handler/src/yamlRestTest/resources/rest-api-spec/test/resthandler/40_nonreadable_system_index.yml

@@ -0,0 +1,41 @@
+---
+setup:
+  - skip:
+      features: allowed_warnings
+  - do:
+      indices.create:
+        index: .nonreadable-system-index
+
+  - do:
+      allowed_warnings:
+        - "this request accesses system indices: [.nonreadable-system-index], but in a future major version, direct access to system indices will be prevented by default"
+      index:
+        index: .nonreadable-system-index
+        id: 1
+        refresh: true
+        body: { "test": 1 }
+
+---
+"Test that .nonreadable-system-index does output deprecation log on search":
+  - skip:
+      features: [warnings, headers]
+  - do:
+      headers: { "X-Opaque-Id": "search-request" }
+      warnings:
+        - "this request accesses system indices: [.nonreadable-system-index], but in a future major version, direct access to system indices will be prevented by default"
+      search:
+        rest_total_hits_as_int: true
+        index: .nonreadable-system-index
+        body:
+          query:
+            match_all: { }
+  - match: { hits.total: 1 }
+
+  - do:
+      warnings:
+        - "this request accesses system indices: [.nonreadable-system-index], but in a future major version, direct access to system indices will be prevented by default"
+      headers: { "X-Opaque-Id": "get-request" }
+      get:
+        index: .nonreadable-system-index
+        id: 1
+  - match: { _source.test: 1 }

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

@@ -53,6 +53,8 @@
 import org.opensearch.index.IndexNotFoundException;
 import org.opensearch.indices.IndexClosedException;
 import org.opensearch.indices.InvalidIndexNameException;
+import org.opensearch.indices.SystemIndexDescriptor;
+import org.opensearch.indices.SystemIndexRegistry;
 
 import java.time.Instant;
 import java.time.ZoneId;
@@ -223,7 +225,8 @@ public Index[] concreteIndices(ClusterState state, IndicesRequest request, long
             false,
             request.includeDataStreams(),
             false,
-            isSystemIndexAccessAllowed()
+            isSystemIndexAccessAllowed(),
+            true
         );
         return concreteIndices(context, request.indices());
     }
@@ -358,21 +361,28 @@ Index[] concreteIndices(Context context, String... indexExpressions) {
 
     private void checkSystemIndexAccess(Context context, Metadata metadata, Set<Index> concreteIndices, String[] originalPatterns) {
         if (context.isSystemIndexAccessAllowed() == false) {
-            final List<String> resolvedSystemIndices = concreteIndices.stream()
+            final Set<String> resolvedSystemIndices = concreteIndices.stream()
                 .map(metadata::index)
                 .filter(IndexMetadata::isSystem)
                 .map(i -> i.getIndex().getName())
                 .sorted() // reliable order for testing
-                .collect(Collectors.toList());
+                .collect(Collectors.toSet());
             if (resolvedSystemIndices.isEmpty() == false) {
-                resolvedSystemIndices.forEach(
-                    systemIndexName -> deprecationLogger.deprecate(
+                for (String systemIndexName : resolvedSystemIndices) {
+                    SystemIndexDescriptor descriptor = SystemIndexRegistry.matchesSystemIndexDescriptor(resolvedSystemIndices)
+                        .stream()
+                        .findFirst()
+                        .orElse(null);
+                    if (descriptor != null && descriptor.isReadable() && context.isReadRequest()) {
+                        continue;
+                    }
+                    deprecationLogger.deprecate(
                         "open_system_index_access_" + systemIndexName,
                         "this request accesses system indices: [{}], but in a future major version, direct access to system "
                             + "indices will be prevented by default",
                         systemIndexName
-                    )
-                );
+                    );
+                }
             }
         }
     }
@@ -481,7 +491,17 @@ public Index concreteWriteIndex(
             options.ignoreThrottled()
         );
 
-        Context context = new Context(state, combinedOptions, false, true, includeDataStreams, isSystemIndexAccessAllowed());
+        Context context = new Context(
+            state,
+            combinedOptions,
+            System.currentTimeMillis(),
+            false,
+            true,
+            includeDataStreams,
+            false,
+            isSystemIndexAccessAllowed(),
+            false
+        );
         Index[] indices = concreteIndices(context, index);
         if (allowNoIndices && indices.length == 0) {
             return null;
@@ -788,6 +808,7 @@ public static class Context {
         private final boolean includeDataStreams;
         private final boolean preserveDataStreams;
         private final boolean isSystemIndexAccessAllowed;
+        private final boolean isReadRequest;
 
         Context(ClusterState state, IndicesOptions options, boolean isSystemIndexAccessAllowed) {
             this(state, options, System.currentTimeMillis(), isSystemIndexAccessAllowed);
@@ -809,7 +830,8 @@ public static class Context {
                 resolveToWriteIndex,
                 includeDataStreams,
                 false,
-                isSystemIndexAccessAllowed
+                isSystemIndexAccessAllowed,
+                true
             );
         }
 
@@ -830,23 +852,25 @@ public static class Context {
                 resolveToWriteIndex,
                 includeDataStreams,
                 preserveDataStreams,
-                isSystemIndexAccessAllowed
+                isSystemIndexAccessAllowed,
+                true
             );
         }
 
         Context(ClusterState state, IndicesOptions options, long startTime, boolean isSystemIndexAccessAllowed) {
-            this(state, options, startTime, false, false, false, false, isSystemIndexAccessAllowed);
+            this(state, options, startTime, false, false, false, false, isSystemIndexAccessAllowed, true);
         }
 
-        protected Context(
+        Context(
             ClusterState state,
             IndicesOptions options,
             long startTime,
             boolean preserveAliases,
             boolean resolveToWriteIndex,
             boolean includeDataStreams,
             boolean preserveDataStreams,
-            boolean isSystemIndexAccessAllowed
+            boolean isSystemIndexAccessAllowed,
+            boolean isReadRequest
         ) {
             this.state = state;
             this.options = options;
@@ -856,6 +880,7 @@ protected Context(
             this.includeDataStreams = includeDataStreams;
             this.preserveDataStreams = preserveDataStreams;
             this.isSystemIndexAccessAllowed = isSystemIndexAccessAllowed;
+            this.isReadRequest = isReadRequest;
         }
 
         public ClusterState getState() {
@@ -895,6 +920,10 @@ public boolean isPreserveDataStreams() {
             return preserveDataStreams;
         }
 
+        public boolean isReadRequest() {
+            return isReadRequest;
+        }
+
         /**
          * Used to determine if it is allowed to access system indices in this context (e.g. for this request).
          */

server/src/main/java/org/opensearch/indices/SystemIndexDescriptor.java

@@ -50,13 +50,15 @@ public class SystemIndexDescriptor {
     private final String indexPattern;
     private final String description;
     private final CharacterRunAutomaton indexPatternAutomaton;
+    private boolean readable;
 
     /**
      *
      * @param indexPattern The pattern of index names that this descriptor will be used for. Must start with a '.' character.
      * @param description The name of the plugin responsible for this system index.
+     * @param readable Whether this system index is readable. A readable index is one where search and get actions are permitted.
      */
-    public SystemIndexDescriptor(String indexPattern, String description) {
+    public SystemIndexDescriptor(String indexPattern, String description, boolean readable) {
         Objects.requireNonNull(indexPattern, "system index pattern must not be null");
         if (indexPattern.length() < 2) {
             throw new IllegalArgumentException(
@@ -79,6 +81,16 @@ public SystemIndexDescriptor(String indexPattern, String description) {
         Automaton a = Operations.determinize(Regex.simpleMatchToAutomaton(indexPattern), Operations.DEFAULT_DETERMINIZE_WORK_LIMIT);
         this.indexPatternAutomaton = new CharacterRunAutomaton(a);
         this.description = description;
+        this.readable = readable;
+    }
+
+    /**
+     *
+     * @param indexPattern The pattern of index names that this descriptor will be used for. Must start with a '.' character.
+     * @param description The name of the plugin responsible for this system index.
+     */
+    public SystemIndexDescriptor(String indexPattern, String description) {
+        this(indexPattern, description, false);
     }
 
     /**
@@ -104,9 +116,33 @@ public String getDescription() {
         return description;
     }
 
+    /**
+     * @return A boolean corresponding to whether this system index is readable.
+     */
+    public boolean isReadable() {
+        return readable;
+    }
+
     @Override
     public String toString() {
-        return "SystemIndexDescriptor[pattern=[" + indexPattern + "], description=[" + description + "]]";
+        return "SystemIndexDescriptor[pattern=[" + indexPattern + "], description=[" + description + "], readable=[" + readable + "]]";
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
+        if (!(obj instanceof SystemIndexDescriptor)) {
+            return false;
+        }
+        SystemIndexDescriptor other = (SystemIndexDescriptor) obj;
+        return indexPattern.equals(other.indexPattern);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(indexPattern);
     }
 
     // TODO: Index settings and mapping

server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java

@@ -59,6 +59,12 @@ public static Set<String> matchesSystemIndexPattern(Set<String> indexExpressions
         return indexExpressions.stream().filter(pattern -> Regex.simpleMatch(SYSTEM_INDEX_PATTERNS, pattern)).collect(Collectors.toSet());
     }
 
+    public static Set<SystemIndexDescriptor> matchesSystemIndexDescriptor(Set<String> indexExpressions) {
+        return getAllDescriptors().stream()
+            .filter(descriptor -> indexExpressions.stream().anyMatch(pattern -> Regex.simpleMatch(descriptor.getIndexPattern(), pattern)))
+            .collect(Collectors.toSet());
+    }
+
     public static Set<String> matchesPluginSystemIndexPattern(String pluginClassName, Set<String> indexExpressions) {
         if (!SYSTEM_INDEX_DESCRIPTORS_MAP.containsKey(pluginClassName)) {
             return Collections.emptySet();

server/src/test/java/org/opensearch/indices/SystemIndicesTests.java

@@ -46,11 +46,13 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.singletonList;
 import static java.util.Collections.singletonMap;
 import static org.opensearch.tasks.TaskResultsService.TASK_INDEX;
+import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.not;
@@ -189,6 +191,16 @@ public void testSystemIndexMatching() {
             equalTo(Set.of(".system-index1", ".system-index-pattern1"))
         );
         assertThat(SystemIndexRegistry.matchesSystemIndexPattern(Set.of(".not-system")), equalTo(Collections.emptySet()));
+
+        assertThat(
+            SystemIndexRegistry.matchesSystemIndexDescriptor(Set.of(".system-index1", ".system-index2")),
+            containsInAnyOrder(
+                Stream.concat(
+                    plugin1.getSystemIndexDescriptors(Settings.EMPTY).stream(),
+                    plugin2.getSystemIndexDescriptors(Settings.EMPTY).stream()
+                ).toArray()
+            )
+        );
     }
 
     public void testRegisteredSystemIndexGetAllDescriptors() {