Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move Security to use auto-managed system indices #67114

Merged
merged 15 commits into from
Feb 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public class TestSystemIndexDescriptor extends SystemIndexDescriptor {
.build();

TestSystemIndexDescriptor() {
super(INDEX_NAME + "*", PRIMARY_INDEX_NAME, "Test system index", null, SETTINGS, INDEX_NAME, 0, "version", "stack");
super(INDEX_NAME + "*", PRIMARY_INDEX_NAME, "Test system index", null, SETTINGS, INDEX_NAME, 0, "version", "stack", null);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,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();
Expand All @@ -107,20 +113,28 @@ public CreateIndexRequest(StreamInput in) throws IOException {
aliases.add(new Alias(in));
}
waitForActiveShards = ActiveShardCount.readFrom(in);
if (in.getVersion().onOrAfter(Version.V_8_0_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;
Expand Down Expand Up @@ -172,6 +186,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.
*/
Expand Down Expand Up @@ -260,7 +283,7 @@ private CreateIndexRequest mapping(BytesReference source, XContentType xContentT

private CreateIndexRequest mapping(String type, Map<String, ?> source) {
// 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 = Map.of(MapperService.SINGLE_MAPPING_NAME, source);
}
else if (MapperService.SINGLE_MAPPING_NAME.equals(type) == false) {
Expand Down Expand Up @@ -462,6 +485,9 @@ public void writeTo(StreamOutput out) throws IOException {
alias.writeTo(out);
}
waitForActiveShards.writeTo(out);
if (out.getVersion().onOrAfter(Version.V_8_0_0)) {
out.writeString(origin);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -45,6 +48,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;
Expand Down Expand Up @@ -76,9 +80,25 @@ protected void masterOperation(Task task, final CreateIndexRequest request, fina
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)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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(
Expand All @@ -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
Expand All @@ -72,6 +80,14 @@ protected ClusterBlockException checkBlock(PutMappingRequest request, ClusterSta
protected void masterOperation(Task task, 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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -92,22 +93,17 @@ protected void masterOperation(Task task, final PutMappingRequest request, final
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;
}

Expand Down Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,6 @@

package org.elasticsearch.action.admin.indices.settings.put;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.stream.Collectors;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessage;
Expand All @@ -40,6 +33,7 @@
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.metadata.MetadataUpdateSettingsService;
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.index.Index;
Expand All @@ -49,6 +43,13 @@
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.TransportService;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.stream.Collectors;

public class TransportUpdateSettingsAction extends AcknowledgedTransportMasterNodeAction<UpdateSettingsRequest> {

private static final Logger logger = LogManager.getLogger(TransportUpdateSettingsAction.class);
Expand Down Expand Up @@ -90,16 +91,15 @@ protected void masterOperation(Task task, final UpdateSettingsRequest request, f
final Index[] concreteIndices = indexNameExpressionResolver.concreteIndices(state, request);
final Settings requestSettings = request.settings();


final Map<String, List<String>> systemIndexViolations = checkForSystemIndexViolations(concreteIndices, requestSettings);
final Map<String, List<String>> systemIndexViolations = checkForSystemIndexViolations(concreteIndices, request);
if (systemIndexViolations.isEmpty() == false) {
final String message = "Cannot override settings on system indices: "
+ systemIndexViolations.entrySet()
.stream()
.map(entry -> "[" + entry.getKey() + "] -> " + entry.getValue())
.collect(Collectors.joining(", "));
logger.warn(message);
listener.onFailure(new IllegalArgumentException(message));
listener.onFailure(new IllegalStateException(message));
return;
}

Expand Down Expand Up @@ -129,11 +129,18 @@ public void onFailure(Exception t) {
* that the system index's descriptor expects.
*
* @param concreteIndices the indices being updated
* @param requestSettings the settings to be applied
* @param request the update request
* @return a mapping from system index pattern to the settings whose values would be overridden. Empty if there are no violations.
*/
private Map<String, List<String>> checkForSystemIndexViolations(Index[] concreteIndices, Settings requestSettings) {
final Map<String, List<String>> violations = new HashMap<>();
private Map<String, List<String>> checkForSystemIndexViolations(Index[] concreteIndices, UpdateSettingsRequest request) {
// Requests that a cluster generates itself are permitted to have a difference in settings
// so that rolling upgrade scenarios still work. We check this via the request's origin.
if (Strings.isNullOrEmpty(request.origin()) == false) {
return Map.of();
}

final Map<String, List<String>> violationsByIndex = new HashMap<>();
final Settings requestSettings = request.settings();

for (Index index : concreteIndices) {
final SystemIndexDescriptor descriptor = systemIndices.findMatchingDescriptor(index.getName());
Expand All @@ -150,10 +157,11 @@ private Map<String, List<String>> checkForSystemIndexViolations(Index[] concrete
}

if (failedKeys.isEmpty() == false) {
violations.put(descriptor.getIndexPattern(), failedKeys);
violationsByIndex.put(descriptor.getIndexPattern(), failedKeys);
}
}
}
return violations;

return violationsByIndex;
}
}
Loading