Move Security to use auto-managed system indices

Backport of elastic#67114. Part of elastic#61656. Change the Security plugin so that its system indices are managed automatically by the system indices infrastructure. Also add an `origin` field to `CreateIndexRequest` and `UpdateSettingsRequest`.
pugnascotia · Feb 2, 2021 · 70a1472 · 70a1472
1 parent 5bce45b
commit 70a1472
Show file tree

Hide file tree

Showing 30 changed files with 926 additions and 849 deletions.
diff --git a/server/src/internalClusterTest/java/org/elasticsearch/indices/TestSystemIndexDescriptor.java b/server/src/internalClusterTest/java/org/elasticsearch/indices/TestSystemIndexDescriptor.java
@@ -51,6 +51,7 @@ public class TestSystemIndexDescriptor extends SystemIndexDescriptor {
         .build();
 
     TestSystemIndexDescriptor() {
+<<<<<<< HEAD
         super(
             INDEX_NAME + "*",
             PRIMARY_INDEX_NAME,
@@ -63,6 +64,9 @@ public class TestSystemIndexDescriptor extends SystemIndexDescriptor {
             "stack",
             MapperService.SINGLE_MAPPING_NAME
         );
+=======
+        super(INDEX_NAME + "*", PRIMARY_INDEX_NAME, "Test system index", null, SETTINGS, INDEX_NAME, 0, "version", "stack", null);
+>>>>>>> 6ffddf8f018... Move Security to use auto-managed system indices (#67114)
     }
 
     @Override

diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/create/AutoCreateAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/create/AutoCreateAction.java
@@ -151,9 +151,24 @@ public ClusterState execute(ClusterState currentState) throws Exception {
                         }
 
                         final SystemIndexDescriptor descriptor = systemIndices.findMatchingDescriptor(indexName);
-                        CreateIndexClusterStateUpdateRequest updateRequest = descriptor != null && descriptor.isAutomaticallyManaged()
-                            ? buildSystemIndexUpdateRequest(descriptor)
-                            : buildUpdateRequest(indexName);
+                        final boolean isSystemIndex = descriptor != null && descriptor.isAutomaticallyManaged();
+
+                        final CreateIndexClusterStateUpdateRequest updateRequest;
+
+                        if (isSystemIndex) {
+                            final String message = descriptor.checkMinimumNodeVersion(
+                                "auto-create index",
+                                state.nodes().getMinNodeVersion()
+                            );
+                            if (message != null) {
+                                logger.warn(message);
+                                throw new IllegalStateException(message);
+                            }
+
+                            updateRequest = buildSystemIndexUpdateRequest(descriptor);
+                        } else {
+                            updateRequest = buildUpdateRequest(indexName);
+                        }
 
                         return createIndexService.applyCreateIndexRequest(currentState, updateRequest, false);
                     }

diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequest.java
@@ -87,6 +87,12 @@ public class CreateIndexRequest extends AcknowledgedRequest<CreateIndexRequest>
 
     private ActiveShardCount waitForActiveShards = ActiveShardCount.DEFAULT;
 
+    private String origin = "";
+
+    /**
+     * Constructs a new request by deserializing an input
+     * @param in the input from which to deserialize
+     */
     public CreateIndexRequest(StreamInput in) throws IOException {
         super(in);
         cause = in.readString();
@@ -118,20 +124,28 @@ public CreateIndexRequest(StreamInput in) throws IOException {
             in.readBoolean(); // updateAllTypes
         }
         waitForActiveShards = ActiveShardCount.readFrom(in);
+        if (in.getVersion().onOrAfter(Version.V_7_12_0)) {
+            origin = in.readString();
+        }
     }
 
     public CreateIndexRequest() {
     }
 
     /**
-     * Constructs a new request to create an index with the specified name.
+     * Constructs a request to create an index.
+     *
+     * @param index the name of the index
      */
     public CreateIndexRequest(String index) {
         this(index, EMPTY_SETTINGS);
     }
 
     /**
-     * Constructs a new request to create an index with the specified name and settings.
+     * Constructs a request to create an index.
+     *
+     * @param index the name of the index
+     * @param settings the settings to apply to the index
      */
     public CreateIndexRequest(String index, Settings settings) {
         this.index = index;
@@ -183,6 +197,15 @@ public String cause() {
         return cause;
     }
 
+    public String origin() {
+        return origin;
+    }
+
+    public CreateIndexRequest origin(String origin) {
+        this.origin = Objects.requireNonNull(origin);
+        return this;
+    }
+
     /**
      * The settings to create the index with.
      */
@@ -276,7 +299,7 @@ public CreateIndexRequest mapping(String type, Map<String, ?> source) {
             throw new IllegalStateException("mappings for type \"" + type + "\" were already defined");
         }
         // wrap it in a type map if its not
-        if (source.size() != 1 || !source.containsKey(type)) {
+        if (source.size() != 1 || source.containsKey(type) == false) {
             source = MapBuilder.<String, Object>newMapBuilder().put(type, source).map();
         }
         try {
@@ -477,6 +500,9 @@ public void writeTo(StreamOutput out) throws IOException {
             out.writeBoolean(true); // updateAllTypes
         }
         waitForActiveShards.writeTo(out);
+        if (out.getVersion().onOrAfter(Version.V_7_12_0)) {
+            out.writeString(origin);
+        }
     }
 
     @Override

diff --git a/...c/main/java/org/elasticsearch/action/admin/indices/create/TransportCreateIndexAction.java b/...c/main/java/org/elasticsearch/action/admin/indices/create/TransportCreateIndexAction.java
@@ -19,6 +19,8 @@
 
 package org.elasticsearch.action.admin.indices.create;
 
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.admin.indices.alias.Alias;
 import org.elasticsearch.action.support.ActionFilters;
@@ -30,6 +32,7 @@
 import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
 import org.elasticsearch.cluster.metadata.MetadataCreateIndexService;
 import org.elasticsearch.cluster.service.ClusterService;
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.indices.SystemIndexDescriptor;
@@ -46,6 +49,7 @@
  * Create index action.
  */
 public class TransportCreateIndexAction extends TransportMasterNodeAction<CreateIndexRequest, CreateIndexResponse> {
+    private static final Logger logger = LogManager.getLogger(TransportCreateIndexAction.class);
 
     private final MetadataCreateIndexService createIndexService;
     private final SystemIndices systemIndices;
@@ -77,9 +81,25 @@ protected void masterOperation(final CreateIndexRequest request, final ClusterSt
         final String indexName = indexNameExpressionResolver.resolveDateMathExpression(request.index());
 
         final SystemIndexDescriptor descriptor = systemIndices.findMatchingDescriptor(indexName);
-        final CreateIndexClusterStateUpdateRequest updateRequest = descriptor != null && descriptor.isAutomaticallyManaged()
-            ? buildSystemIndexUpdateRequest(request, cause, descriptor)
-            : buildUpdateRequest(request, cause, indexName);
+        final boolean isSystemIndex = descriptor != null && descriptor.isAutomaticallyManaged();
+
+        final CreateIndexClusterStateUpdateRequest updateRequest;
+
+        // Requests that a cluster generates itself are permitted to create a system index with
+        // different mappings, settings etc. This is so that rolling upgrade scenarios still work.
+        // We check this via the request's origin. Eventually, `SystemIndexManager` will reconfigure
+        // the index to the latest settings.
+        if (isSystemIndex && Strings.isNullOrEmpty(request.origin())) {
+            final String message = descriptor.checkMinimumNodeVersion("create index", state.nodes().getMinNodeVersion());
+            if (message != null) {
+                logger.warn(message);
+                listener.onFailure(new IllegalStateException(message));
+                return;
+            }
+            updateRequest = buildSystemIndexUpdateRequest(request, cause, descriptor);
+        } else {
+            updateRequest = buildUpdateRequest(request, cause, indexName);
+        }
 
         createIndexService.createIndex(updateRequest, listener.map(response ->
             new CreateIndexResponse(response.isAcknowledged(), response.isShardsAcknowledged(), indexName)));

diff --git a/...ava/org/elasticsearch/action/admin/indices/mapping/put/TransportAutoPutMappingAction.java b/...ava/org/elasticsearch/action/admin/indices/mapping/put/TransportAutoPutMappingAction.java
@@ -18,6 +18,8 @@
  */
 package org.elasticsearch.action.admin.indices.mapping.put;
 
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.support.ActionFilters;
 import org.elasticsearch.action.support.master.AcknowledgedResponse;
@@ -30,6 +32,7 @@
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.index.Index;
+import org.elasticsearch.indices.SystemIndices;
 import org.elasticsearch.tasks.Task;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.TransportService;
@@ -38,7 +41,10 @@
 
 public class TransportAutoPutMappingAction extends AcknowledgedTransportMasterNodeAction<PutMappingRequest> {
 
+    private static final Logger logger = LogManager.getLogger(TransportAutoPutMappingAction.class);
+
     private final MetadataMappingService metadataMappingService;
+    private final SystemIndices systemIndices;
 
     @Inject
     public TransportAutoPutMappingAction(
@@ -47,10 +53,12 @@ public TransportAutoPutMappingAction(
             final ThreadPool threadPool,
             final MetadataMappingService metadataMappingService,
             final ActionFilters actionFilters,
-            final IndexNameExpressionResolver indexNameExpressionResolver) {
+            final IndexNameExpressionResolver indexNameExpressionResolver,
+            final SystemIndices systemIndices) {
         super(AutoPutMappingAction.NAME, transportService, clusterService, threadPool, actionFilters,
             PutMappingRequest::new, indexNameExpressionResolver, ThreadPool.Names.SAME);
         this.metadataMappingService = metadataMappingService;
+        this.systemIndices = systemIndices;
     }
 
     @Override
@@ -72,6 +80,14 @@ protected ClusterBlockException checkBlock(PutMappingRequest request, ClusterSta
     protected void masterOperation(final PutMappingRequest request, final ClusterState state,
                                    final ActionListener<AcknowledgedResponse> listener) {
         final Index[] concreteIndices = new Index[] {request.getConcreteIndex()};
+
+        final String message = TransportPutMappingAction.checkForSystemIndexViolations(systemIndices, concreteIndices, request);
+        if (message != null) {
+            logger.warn(message);
+            listener.onFailure(new IllegalStateException(message));
+            return;
+        }
+
         performMappingUpdate(concreteIndices, request, listener, metadataMappingService);
     }
 

diff --git a/...in/java/org/elasticsearch/action/admin/indices/mapping/put/TransportPutMappingAction.java b/...in/java/org/elasticsearch/action/admin/indices/mapping/put/TransportPutMappingAction.java
@@ -33,6 +33,7 @@
 import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
 import org.elasticsearch.cluster.metadata.MetadataMappingService;
 import org.elasticsearch.cluster.service.ClusterService;
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.index.Index;
 import org.elasticsearch.index.IndexNotFoundException;
@@ -91,22 +92,17 @@ protected void masterOperation(final PutMappingRequest request, final ClusterSta
                                    final ActionListener<AcknowledgedResponse> listener) {
         try {
             final Index[] concreteIndices = resolveIndices(state, request, indexNameExpressionResolver);
-            final String mappingSource = request.source();
 
             final Optional<Exception> maybeValidationException = requestValidators.validateRequest(request, state, concreteIndices);
             if (maybeValidationException.isPresent()) {
                 listener.onFailure(maybeValidationException.get());
                 return;
             }
 
-            final List<String> violations = checkForSystemIndexViolations(concreteIndices, mappingSource);
-            if (violations.isEmpty() == false) {
-                final String message = "Cannot update mappings in "
-                    + violations
-                    + ": system indices can only use mappings from their descriptors, "
-                    + "but the mappings in the request did not match those in the descriptors(s)";
+            final String message = checkForSystemIndexViolations(systemIndices, concreteIndices, request);
+            if (message != null) {
                 logger.warn(message);
-                listener.onFailure(new IllegalArgumentException(message));
+                listener.onFailure(new IllegalStateException(message));
                 return;
             }
 
@@ -160,21 +156,36 @@ public void onFailure(Exception t) {
         });
     }
 
-    private List<String> checkForSystemIndexViolations(Index[] concreteIndices, String requestMappings) {
+    static String checkForSystemIndexViolations(SystemIndices systemIndices, Index[] concreteIndices, PutMappingRequest request) {
+        // Requests that a cluster generates itself are permitted to have a difference in mappings
+        // so that rolling upgrade scenarios still work. We check this via the request's origin.
+        if (Strings.isNullOrEmpty(request.origin()) == false) {
+            return null;
+        }
+
         List<String> violations = new ArrayList<>();
 
+        final String requestMappings = request.source();
+
         for (Index index : concreteIndices) {
             final SystemIndexDescriptor descriptor = systemIndices.findMatchingDescriptor(index.getName());
             if (descriptor != null && descriptor.isAutomaticallyManaged()) {
                 final String descriptorMappings = descriptor.getMappings();
-
                 // Technically we could trip over a difference in whitespace here, but then again nobody should be trying to manually
                 // update a descriptor's mappings.
                 if (descriptorMappings.equals(requestMappings) == false) {
                     violations.add(index.getName());
                 }
             }
         }
-        return violations;
+
+        if (violations.isEmpty() == false) {
+            return "Cannot update mappings in "
+                + violations
+                + ": system indices can only use mappings from their descriptors, "
+                + "but the mappings in the request did not match those in the descriptors(s)";
+        }
+
+        return null;
     }
 }