Correctly ignore system indices when validating dot-prefixed indices

docs/changelog/128868.yaml

@@ -0,0 +1,5 @@
+pr: 128868
+summary: Correctly ignore system indices when validating dot-prefixed indices
+area: Indices APIs
+type: bug
+issues: []

modules/dot-prefix-validation/src/main/java/org/elasticsearch/validation/AutoCreateDotValidator.java

@@ -13,12 +13,13 @@
 import org.elasticsearch.action.admin.indices.create.CreateIndexRequest;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
+import org.elasticsearch.indices.SystemIndices;
 
 import java.util.Set;
 
 public class AutoCreateDotValidator extends DotPrefixValidator<CreateIndexRequest> {
-    public AutoCreateDotValidator(ThreadContext threadContext, ClusterService clusterService) {
-        super(threadContext, clusterService);
+    public AutoCreateDotValidator(ThreadContext threadContext, ClusterService clusterService, SystemIndices systemIndices) {
+        super(threadContext, clusterService, systemIndices);
     }
 
     @Override

modules/dot-prefix-validation/src/main/java/org/elasticsearch/validation/CreateIndexDotValidator.java

@@ -13,12 +13,13 @@
 import org.elasticsearch.action.admin.indices.create.TransportCreateIndexAction;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
+import org.elasticsearch.indices.SystemIndices;
 
 import java.util.Set;
 
 public class CreateIndexDotValidator extends DotPrefixValidator<CreateIndexRequest> {
-    public CreateIndexDotValidator(ThreadContext threadContext, ClusterService clusterService) {
-        super(threadContext, clusterService);
+    public CreateIndexDotValidator(ThreadContext threadContext, ClusterService clusterService, SystemIndices systemIndices) {
+        super(threadContext, clusterService, systemIndices);
     }
 
     @Override

modules/dot-prefix-validation/src/main/java/org/elasticsearch/validation/DotPrefixValidationPlugin.java

@@ -13,6 +13,7 @@
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
+import org.elasticsearch.indices.SystemIndices;
 import org.elasticsearch.plugins.ActionPlugin;
 import org.elasticsearch.plugins.Plugin;
 
@@ -30,12 +31,13 @@ public DotPrefixValidationPlugin() {}
     public Collection<?> createComponents(PluginServices services) {
         ThreadContext context = services.threadPool().getThreadContext();
         ClusterService clusterService = services.clusterService();
+        SystemIndices systemIndices = services.systemIndices();
 
         actionFilters.set(
             List.of(
-                new CreateIndexDotValidator(context, clusterService),
-                new AutoCreateDotValidator(context, clusterService),
-                new IndexTemplateDotValidator(context, clusterService)
+                new CreateIndexDotValidator(context, clusterService, systemIndices),
+                new AutoCreateDotValidator(context, clusterService, systemIndices),
+                new IndexTemplateDotValidator(context, clusterService, systemIndices)
             )
         );
 

modules/dot-prefix-validation/src/main/java/org/elasticsearch/validation/DotPrefixValidator.java

@@ -22,6 +22,7 @@
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
 import org.elasticsearch.core.Nullable;
+import org.elasticsearch.indices.SystemIndices;
 import org.elasticsearch.tasks.Task;
 
 import java.util.List;
@@ -94,12 +95,14 @@ public abstract class DotPrefixValidator<RequestType> implements MappedActionFil
     DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(DotPrefixValidator.class);
 
     private final ThreadContext threadContext;
+    private final SystemIndices systemIndices;
     private final boolean isEnabled;
     private final boolean isStateless;
     private volatile Set<Pattern> ignoredIndexPatterns;
 
-    public DotPrefixValidator(ThreadContext threadContext, ClusterService clusterService) {
+    public DotPrefixValidator(ThreadContext threadContext, ClusterService clusterService, SystemIndices systemIndices) {
         this.threadContext = threadContext;
+        this.systemIndices = systemIndices;
         this.isEnabled = VALIDATE_DOT_PREFIXES.get(clusterService.getSettings());
         this.isStateless = DiscoveryNode.isStateless(clusterService.getSettings());
         this.ignoredIndexPatterns = IGNORED_INDEX_PATTERNS_SETTING.get(clusterService.getSettings())
@@ -139,10 +142,13 @@ void validateIndices(@Nullable Set<String> indices) {
                     if (c == '.') {
                         final String strippedName = stripDateMath(index);
                         if (IGNORED_INDEX_NAMES.contains(strippedName)) {
-                            return;
+                            continue;
+                        }
+                        if (systemIndices.isSystemName(strippedName)) {
+                            continue;
                         }
                         if (this.ignoredIndexPatterns.stream().anyMatch(p -> p.matcher(strippedName).matches())) {
-                            return;
+                            continue;
                         }
                         if (isStateless) {
                             throw new IllegalArgumentException("Index [" + index + "] name beginning with a dot (.) is not allowed");

modules/dot-prefix-validation/src/main/java/org/elasticsearch/validation/IndexTemplateDotValidator.java

@@ -12,14 +12,15 @@
 import org.elasticsearch.action.admin.indices.template.put.TransportPutComposableIndexTemplateAction;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
+import org.elasticsearch.indices.SystemIndices;
 
 import java.util.Arrays;
 import java.util.HashSet;
 import java.util.Set;
 
 public class IndexTemplateDotValidator extends DotPrefixValidator<TransportPutComposableIndexTemplateAction.Request> {
-    public IndexTemplateDotValidator(ThreadContext threadContext, ClusterService clusterService) {
-        super(threadContext, clusterService);
+    public IndexTemplateDotValidator(ThreadContext threadContext, ClusterService clusterService, SystemIndices systemIndices) {
+        super(threadContext, clusterService, systemIndices);
     }
 
     @Override

modules/dot-prefix-validation/src/test/java/org/elasticsearch/validation/DotPrefixValidatorTests.java

@@ -17,6 +17,8 @@
 import org.elasticsearch.common.util.concurrent.ThreadContext;
 import org.elasticsearch.common.util.set.Sets;
 import org.elasticsearch.core.Nullable;
+import org.elasticsearch.indices.SystemIndexDescriptor;
+import org.elasticsearch.indices.SystemIndices;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.junit.BeforeClass;
@@ -26,12 +28,14 @@
 import java.util.Set;
 
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 public class DotPrefixValidatorTests extends ESTestCase {
     private static ClusterService statefulClusterService;
     private static ClusterService statelessClusterService;
+    private static SystemIndices systemIndices;
 
     private final OperatorValidator<?> statefulOpV = new OperatorValidator<>(statefulClusterService, true);
     private final NonOperatorValidator<?> statefulNonOpV = new NonOperatorValidator<>(statefulClusterService, true);
@@ -67,6 +71,23 @@ public static void beforeClass() {
         when(statelessClusterService.getClusterSettings()).thenReturn(statelessClusterSettings);
         when(statelessClusterService.getSettings()).thenReturn(statelessSettings);
         when(statelessClusterService.threadPool()).thenReturn(mock(ThreadPool.class));
+
+        systemIndices = new SystemIndices(
+            List.of(
+                new SystemIndices.Feature(
+                    "test",
+                    "test system indices",
+                    List.of(
+                        SystemIndexDescriptor.builder()
+                            .setIndexPattern(".test-system-index*")
+                            .setDescription("test system index")
+                            .setType(SystemIndexDescriptor.Type.INTERNAL_UNMANAGED)
+                            .setOrigin("test")
+                            .build()
+                    )
+                )
+            )
+        );
     }
 
     public void testValidation() {
@@ -121,6 +142,16 @@ public void testValidation() {
         // Test pattern added to the settings
         assertIgnored(Set.of(".potato5"));
         assertIgnored(Set.of("<.potato5>"));
+
+        // Test system indices are ignored
+        assertIgnored(Set.of(".test-system-index"));
+        assertIgnored(Set.of(".test-system-index-other"));
+        assertIgnored(Set.of(".test-system-index", ".test-system-index-other"));
+        assertIgnored(Set.of("<.test-system-index>"));
+        // Indices that don't match the system index pattern should still fail
+        assertFails(Set.of(".not-a-system-index"));
+        // If we have a mix of system and non-system, we still expect a warning
+        assertSecondIndexFails(List.of(".test-system-index", ".not-a-system-index"));
     }
 
     private void assertFails(Set<String> indices) {
@@ -141,6 +172,31 @@ private void assertFails(Set<String> indices) {
         assertThat(error.getMessage(), containsString("name beginning with a dot (.) is not allowed"));
     }
 
+    /*
+     * This method asserts that the second index in the list is the one that triggers the warning/error.
+     */
+    private void assertSecondIndexFails(List<String> indices) {
+        assertThat(indices.size(), equalTo(2));
+        /*
+         * This method asserts the key difference between stateful and stateless mode -- stateful just logs a deprecation warning, while
+         * stateless throws an exception.
+         */
+        var statefulValidator = new NonOperatorValidator<>(statefulClusterService, false);
+        statefulValidator.validateIndices(Set.copyOf(indices));
+        assertWarnings(
+            "Index ["
+                + indices.get(1)
+                + "] name begins with a dot (.), which is deprecated, and will not be allowed in a future Elasticsearch version."
+        );
+
+        var statelessValidator = new NonOperatorValidator<>(statelessClusterService, false);
+        IllegalArgumentException error = expectThrows(
+            IllegalArgumentException.class,
+            () -> statelessValidator.validateIndices(Set.copyOf(indices))
+        );
+        assertThat(error.getMessage(), containsString("name beginning with a dot (.) is not allowed"));
+    }
+
     private void assertIgnored(Set<String> indices) {
         statefulNonOpV.validateIndices(indices);
         statefulOpV.validateIndices(indices);
@@ -153,7 +209,7 @@ private class NonOperatorValidator<R> extends DotPrefixValidator<R> {
         private final boolean assertNoWarnings;
 
         private NonOperatorValidator(ClusterService clusterService, boolean assertNoWarnings) {
-            super(new ThreadContext(Settings.EMPTY), clusterService);
+            super(new ThreadContext(Settings.EMPTY), clusterService, systemIndices);
             this.assertNoWarnings = assertNoWarnings;
         }
 

modules/dot-prefix-validation/src/yamlRestTest/resources/rest-api-spec/test/dot_prefix/10_basic.yml

@@ -1,8 +1,16 @@
 ---
 teardown:
+  - skip:
+      features: "allowed_warnings"
   - do:
       indices.delete:
         index: .*,-.security-*
+  - do:
+      allowed_warnings:
+        - "this request accesses system indices: [.tasks], but in a future major version, direct access to system indices will be prevented by default"
+      indices.delete:
+        index: .tasks
+        ignore_unavailable: true
 
 ---
 "Index creation with a dot-prefix is deprecated unless x-elastic-product-origin set":
@@ -194,3 +202,34 @@ teardown:
   - do:
       indices.delete_index_template:
         name: my-template2
+
+---
+"System indices do not cause deprecation warnings":
+  - requires:
+      test_runner_features: ["allowed_warnings"]
+
+  - do:
+      allowed_warnings:
+        - "this request accesses system indices: [.tasks], but in a future major version, direct access to system indices will be prevented by default"
+      indices.create:
+        index: .tasks
+
+---
+"Deprecated index template with a system index and a dot prefix index pattern":
+  # This makes sure that we correctly log a deprecation warning for the a dot-prefixed index pattern in an index
+  # template, even if the template also has a system index pattern.
+  - requires:
+      test_runner_features: ["warnings", "headers", "allowed_warnings"]
+
+  - do:
+      warnings:
+        - "Index [.data-*] name begins with a dot (.), which is deprecated, and will not be allowed in a future Elasticsearch version."
+      indices.put_index_template:
+        name: my-template
+        body:
+          index_patterns: [.tasks, .data-*]
+          data_stream: {}
+
+  - do:
+      indices.delete_index_template:
+        name: my-template