DSL add update settings,read-only, and downsampling privileges (#99200)

DSL needs the update settings privilege also for system data streams (this was missing). And both regular and system data streams need the add index block and downsampling privileges (as part of the downsampling execution) This is a bug, really, however DSL is not released in 8.10 so marking this as `non-issue`.
elastic · Sep 7, 2023 · 843636a · 843636a
1 parent dcbffb0
commit 843636a
Show file tree

Hide file tree

Showing 8 changed files with 553 additions and 7 deletions.
diff --git a/...ams/src/main/java/org/elasticsearch/datastreams/lifecycle/DataStreamLifecycleService.java b/...ams/src/main/java/org/elasticsearch/datastreams/lifecycle/DataStreamLifecycleService.java
@@ -974,6 +974,7 @@ public void onResponse(AcknowledgedResponse acknowledgedResponse) {
             @Override
             public void onFailure(Exception e) {
                 if (e instanceof IndexNotFoundException) {
+                    logger.trace("Data stream lifecycle did not delete index [{}] as it was already deleted", targetIndex);
                     // index was already deleted, treat this as a success
                     errorStore.clearRecordedError(targetIndex);
                     listener.onResponse(null);

diff --git a/...csearch/datastreams/lifecycle/downsampling/ReplaceBackingWithDownsampleIndexExecutor.java b/...csearch/datastreams/lifecycle/downsampling/ReplaceBackingWithDownsampleIndexExecutor.java
@@ -16,7 +16,7 @@
 import org.elasticsearch.client.internal.Client;
 import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.SimpleBatchedExecutor;
-import org.elasticsearch.core.Strings;
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.core.Tuple;
 import org.elasticsearch.index.IndexNotFoundException;
 import org.elasticsearch.snapshots.SnapshotInProgressException;
@@ -53,6 +53,11 @@ public void taskSucceeded(ReplaceSourceWithDownsampleIndexTask task, Void unused
         );
         task.getListener().onResponse(null);
 
+        LOGGER.trace(
+            "Issuing request to delete index [{}] as it's not part of data stream [{}] anymore",
+            task.getSourceBackingIndex(),
+            task.getDataStreamName()
+        );
         // chain an optimistic delete of the source index call here (if it fails it'll be retried by the data stream lifecycle loop)
         client.admin().indices().delete(new DeleteIndexRequest(task.getSourceBackingIndex()), new ActionListener<>() {
             @Override
@@ -78,6 +83,7 @@ public void onResponse(AcknowledgedResponse acknowledgedResponse) {
             public void onFailure(Exception e) {
                 if (e instanceof IndexNotFoundException) {
                     // index was already deleted, treat this as a success
+                    LOGGER.trace("Did not delete index [{}] as it was already deleted", task.getSourceBackingIndex());
                     return;
                 }
 

diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataDataStreamsService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataDataStreamsService.java
@@ -155,6 +155,15 @@ static ClusterState updateDataLifecycle(
         Metadata.Builder builder = Metadata.builder(metadata);
         for (var dataStreamName : dataStreamNames) {
             var dataStream = validateDataStream(metadata, dataStreamName);
+            if (dataStream.isSystem()) {
+                if (lifecycle != null && lifecycle.getDownsamplingRounds() != null) {
+                    throw new IllegalArgumentException(
+                        "System data streams do not support downsampling as part of their lifecycle configuration. Encountered ["
+                            + dataStream.getName()
+                            + "] in the request"
+                    );
+                }
+            }
             builder.put(
                 new DataStream(
                     dataStream.getName(),

diff --git a/server/src/main/java/org/elasticsearch/indices/SystemDataStreamDescriptor.java b/server/src/main/java/org/elasticsearch/indices/SystemDataStreamDescriptor.java
@@ -12,7 +12,9 @@
 import org.elasticsearch.cluster.metadata.ComponentTemplate;
 import org.elasticsearch.cluster.metadata.ComposableIndexTemplate;
 import org.elasticsearch.cluster.metadata.DataStream;
+import org.elasticsearch.cluster.metadata.DataStreamLifecycle;
 import org.elasticsearch.cluster.metadata.Metadata;
+import org.elasticsearch.cluster.metadata.MetadataIndexTemplateService;
 
 import java.util.List;
 import java.util.Map;
@@ -87,6 +89,7 @@ public SystemDataStreamDescriptor(
         this.type = Objects.requireNonNull(type, "type must be specified");
         this.composableIndexTemplate = Objects.requireNonNull(composableIndexTemplate, "composableIndexTemplate must be provided");
         this.componentTemplates = componentTemplates == null ? Map.of() : Map.copyOf(componentTemplates);
+        validateNoDownsamplingConfigured(composableIndexTemplate, componentTemplates);
         this.allowedElasticProductOrigins = Objects.requireNonNull(
             allowedElasticProductOrigins,
             "allowedElasticProductOrigins must not be null"
@@ -99,6 +102,16 @@ public SystemDataStreamDescriptor(
         this.characterRunAutomaton = new CharacterRunAutomaton(buildAutomaton(backingIndexPatternForDataStream(this.dataStreamName)));
     }
 
+    private void validateNoDownsamplingConfigured(
+        ComposableIndexTemplate composableIndexTemplate,
+        Map<String, ComponentTemplate> componentTemplates
+    ) {
+        DataStreamLifecycle resolvedLifecycle = MetadataIndexTemplateService.resolveLifecycle(composableIndexTemplate, componentTemplates);
+        if (resolvedLifecycle != null && resolvedLifecycle.isEnabled() && resolvedLifecycle.getDownsamplingRounds() != null) {
+            throw new IllegalArgumentException("System data streams do not support downsampling as part of their lifecycle configuration");
+        }
+    }
+
     public String getDataStreamName() {
         return dataStreamName;
     }

diff --git a/...k/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/user/InternalUsers.java b/...k/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/user/InternalUsers.java
@@ -9,10 +9,12 @@
 
 import org.elasticsearch.action.admin.indices.analyze.ReloadAnalyzerAction;
 import org.elasticsearch.action.admin.indices.forcemerge.ForceMergeAction;
+import org.elasticsearch.action.admin.indices.readonly.AddIndexBlockAction;
 import org.elasticsearch.action.admin.indices.refresh.RefreshAction;
 import org.elasticsearch.action.admin.indices.rollover.RolloverAction;
 import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsAction;
 import org.elasticsearch.action.admin.indices.stats.IndicesStatsAction;
+import org.elasticsearch.action.downsample.DownsampleAction;
 import org.elasticsearch.xpack.core.XPackPlugin;
 import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
 import org.elasticsearch.xpack.core.security.support.MetadataUtils;
@@ -144,7 +146,9 @@ public class InternalUsers {
                         ForceMergeAction.NAME + "*",
                         // indices stats is used by rollover, so we need to grant it here
                         IndicesStatsAction.NAME + "*",
-                        UpdateSettingsAction.NAME
+                        UpdateSettingsAction.NAME,
+                        DownsampleAction.NAME,
+                        AddIndexBlockAction.NAME
                     )
                     .allowRestrictedIndices(false)
                     .build(),
@@ -160,7 +164,9 @@ public class InternalUsers {
                         RolloverAction.NAME,
                         ForceMergeAction.NAME + "*",
                         // indices stats is used by rollover, so we need to grant it here
-                        IndicesStatsAction.NAME + "*"
+                        IndicesStatsAction.NAME + "*",
+                        UpdateSettingsAction.NAME
+                        // Down-sampling related actions are not granted here because down-sampling is not supported for system data streams
                     )
                     .allowRestrictedIndices(true)
                     .build() },

diff --git a/...gin/core/src/test/java/org/elasticsearch/xpack/core/security/user/InternalUsersTests.java b/...gin/core/src/test/java/org/elasticsearch/xpack/core/security/user/InternalUsersTests.java
@@ -18,12 +18,15 @@
 import org.elasticsearch.action.admin.indices.delete.DeleteIndexAction;
 import org.elasticsearch.action.admin.indices.forcemerge.ForceMergeAction;
 import org.elasticsearch.action.admin.indices.mapping.put.PutMappingAction;
+import org.elasticsearch.action.admin.indices.readonly.AddIndexBlockAction;
 import org.elasticsearch.action.admin.indices.refresh.RefreshAction;
 import org.elasticsearch.action.admin.indices.refresh.TransportUnpromotableShardRefreshAction;
 import org.elasticsearch.action.admin.indices.rollover.RolloverAction;
+import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsAction;
 import org.elasticsearch.action.admin.indices.stats.IndicesStatsAction;
 import org.elasticsearch.action.admin.indices.template.put.PutComponentTemplateAction;
 import org.elasticsearch.action.bulk.BulkAction;
+import org.elasticsearch.action.downsample.DownsampleAction;
 import org.elasticsearch.action.get.GetAction;
 import org.elasticsearch.cluster.metadata.DataStream;
 import org.elasticsearch.cluster.metadata.IndexAbstraction;
@@ -240,7 +243,18 @@ public void testDataStreamLifecycleUser() {
             RolloverAction.NAME,
             DeleteIndexAction.NAME,
             ForceMergeAction.NAME,
-            IndicesStatsAction.NAME
+            IndicesStatsAction.NAME,
+            UpdateSettingsAction.NAME,
+            DownsampleAction.NAME,
+            AddIndexBlockAction.NAME
+        );
+
+        final List<String> sampleSystemDataStreamActions = List.of(
+            RolloverAction.NAME,
+            DeleteIndexAction.NAME,
+            ForceMergeAction.NAME,
+            IndicesStatsAction.NAME,
+            UpdateSettingsAction.NAME
         );
         final String dataStream = randomAlphaOfLengthBetween(3, 12);
         checkIndexAccess(role, randomFrom(sampleIndexActions), dataStream, true);
@@ -253,16 +267,16 @@ public void testDataStreamLifecycleUser() {
         );
 
         allowedSystemDataStreams.forEach(allowedSystemDataStream -> {
-            checkIndexAccess(role, randomFrom(sampleIndexActions), allowedSystemDataStream, true);
+            checkIndexAccess(role, randomFrom(sampleSystemDataStreamActions), allowedSystemDataStream, true);
             checkIndexAccess(
                 role,
-                randomFrom(sampleIndexActions),
+                randomFrom(sampleSystemDataStreamActions),
                 DataStream.BACKING_INDEX_PREFIX + allowedSystemDataStream + randomAlphaOfLengthBetween(4, 8),
                 true
             );
         });
 
-        checkIndexAccess(role, randomFrom(sampleIndexActions), randomFrom(TestRestrictedIndices.SAMPLE_RESTRICTED_NAMES), false);
+        checkIndexAccess(role, randomFrom(sampleSystemDataStreamActions), randomFrom(TestRestrictedIndices.SAMPLE_RESTRICTED_NAMES), false);
     }
 
     public void testRegularUser() {

diff --git a/x-pack/plugin/security/build.gradle b/x-pack/plugin/security/build.gradle
@@ -20,6 +20,8 @@ dependencies {
   api project(path: ':modules:transport-netty4')
 
   testImplementation project(path: xpackModule('ilm'))
+  testImplementation project(path: xpackModule('downsample'))
+  testImplementation project(path: xpackModule('mapper-aggregate-metric'))
   testImplementation project(path: xpackModule('monitoring'))
   testImplementation project(path: xpackModule('spatial'))
   testImplementation project(path: xpackModule('wildcard'))