From 3760e8cb62d89f3c1e48f48901b0269d7c2b5361 Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Tue, 19 Mar 2024 15:41:52 -0500 Subject: [PATCH 01/25] Add granular error list to alias action response This commit does the following: * When an alias action list succeeds partially a list of results for each action are returned. This will only occur is must_exist==false * Use same exception for removal of missing aliases whether must_exist is true or false. Closes #94478 --- docs/reference/alias.asciidoc | 60 ++++++++ docs/reference/indices/aliases.asciidoc | 10 +- .../data_stream/140_data_stream_aliases.yml | 81 +++++++++++ .../indices.update_aliases/40_must_exist.yml | 83 +++++++++++ ...dicesAliasesClusterStateUpdateRequest.java | 10 +- .../indices/alias/IndicesAliasesRequest.java | 4 + .../alias/IndicesAliasesRequestBuilder.java | 3 +- .../indices/alias/IndicesAliasesResponse.java | 136 ++++++++++++++++++ .../alias/TransportIndicesAliasesAction.java | 44 ++++-- .../client/internal/IndicesAdminClient.java | 5 +- .../internal/support/AbstractClient.java | 5 +- .../cluster/metadata/AliasAction.java | 4 +- .../metadata/MetadataIndexAliasesService.java | 15 +- .../MetadataIndexAliasesServiceTests.java | 14 +- .../core/ml/annotations/AnnotationIndex.java | 6 +- .../xpack/core/ml/utils/MlIndexAndAlias.java | 3 +- .../search/SearchApplicationIndexService.java | 5 +- .../xpack/ml/MlInitializationService.java | 5 +- .../ml/job/persistence/JobDataDeleter.java | 9 +- .../job/persistence/JobResultsProvider.java | 3 +- .../TransformClusterStateListener.java | 4 +- 21 files changed, 462 insertions(+), 47 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java diff --git a/docs/reference/alias.asciidoc b/docs/reference/alias.asciidoc index 6ddd3602e1467..6873d5a763b90 100644 --- a/docs/reference/alias.asciidoc +++ b/docs/reference/alias.asciidoc @@ -121,6 +121,66 @@ POST _aliases // TEST[s/^/PUT _data_stream\/logs-nginx.access-prod\nPUT _data_stream\/logs-my_app-default\n/] // end::alias-multiple-actions-example[] +[discrete] +[[Multiple action results]] +=== Multiple action results + +When using multiple actions, if some succeed and some fail, a list of per-action results will be returned. + +// tag::alias-multiple-actions-result-example[] +Consider a similar action list to the previous example, but now with an alias `log-non-existing`, which does not yet exist. +In this case, the `remove` action will fail, but the `add` action will succeed. +The response will contain the list `action_results`, with a result for every requested action. + +[source,console] +---- +POST _aliases +{ + "actions": [ + { + "remove": { + "index": "index1", + "alias": "logs-non-existing" + } + }, + { + "add": { + "index": "index2", + "alias": "logs-non-existing" + } + } + ] +} +---- +// TEST[s/^/PUT \/index1\nPUT \/index2\n/] + +The API returns the following result: + +[source,console-result] +-------------------------------------------------- +{ + "acknowledged": true, + "errors": true, + "action_results": [ + { + "action": "remove", + "success": false, + "error": "aliases_not_found_exception" + }, + { + "action": "add", + "success": true + } + ] +} +-------------------------------------------------- + +Allowing the action list to succeed partially may not provide the desired result. +It may be more appropriate to use set `must_exist` to `true`, which will cause the entire action +list to fail if a single action fails. + +// end::alias-multiple-actions-example[] + [discrete] [[add-alias-at-creation]] === Add an alias at index creation diff --git a/docs/reference/indices/aliases.asciidoc b/docs/reference/indices/aliases.asciidoc index 76698501fd416..096cb9d611900 100644 --- a/docs/reference/indices/aliases.asciidoc +++ b/docs/reference/indices/aliases.asciidoc @@ -145,10 +145,16 @@ the alias points to one data stream. + Only the `add` action supports this parameter. +// tag::alias-options[] `must_exist`:: (Optional, Boolean) -If `true`, the alias must exist to perform the action. Defaults to `false`. Only -the `remove` action supports this parameter. +Affects the behavior when attempting to remove an alias which does not exist. +If `true`, removing an alias which does not exist will cause all actions to fail. +If `false`, removing an alias which does not exist will only cause that removal to fail. +Defaults to `false`. +// end::alias-options[] ++ +Only the `remove` action supports this parameter. // tag::alias-options[] `routing`:: diff --git a/modules/data-streams/src/yamlRestTest/resources/rest-api-spec/test/data_stream/140_data_stream_aliases.yml b/modules/data-streams/src/yamlRestTest/resources/rest-api-spec/test/data_stream/140_data_stream_aliases.yml index 70c563d1d4510..35c70475d376d 100644 --- a/modules/data-streams/src/yamlRestTest/resources/rest-api-spec/test/data_stream/140_data_stream_aliases.yml +++ b/modules/data-streams/src/yamlRestTest/resources/rest-api-spec/test/data_stream/140_data_stream_aliases.yml @@ -307,3 +307,84 @@ indices.get_alias: name: this-does-not-exist* - is_false: ds-first.aliases.my-alias +--- +"Action Results with multiple matching aliases": + - skip: + version: " - 7.13.99" + reason: "data streams aliases are available from 7.14.0" + features: allowed_warnings + - do: + allowed_warnings: + - "index template [my-template] has index patterns [log-*] matching patterns from existing older templates [global] with patterns (global => [*]); this template [my-template] will take precedence during new index creation" + indices.put_index_template: + name: my-template + body: + index_patterns: [ log-* ] + template: + settings: + index.number_of_replicas: 0 + data_stream: { } + - do: + indices.create_data_stream: + name: log-foobar + - is_true: acknowledged + - do: + indices.update_aliases: + body: + actions: + - add: + index: log-foobar + aliases: test_alias1 + - remove: + index: log-foobar + aliases: test_non_existing + must_exist: false + - is_true: errors + - length: { action_results: 2 } + - match: { action_results.0.success: true } + - match: { action_results.0.action: add } + - match: { action_results.1.success: false } + - match: { action_results.1.action: remove } + - match: { action_results.1.error: aliases_not_found_exception } +--- +"Single action result per action": + - skip: + version: " - 7.13.99" + reason: "data streams aliases are available from 7.14.0" + features: allowed_warnings + - do: + allowed_warnings: + - "index template [my-template] has index patterns [log-*] matching patterns from existing older templates [global] with patterns (global => [*]); this template [my-template] will take precedence during new index creation" + indices.put_index_template: + name: my-template + body: + index_patterns: [ log-* ] + template: + settings: + index.number_of_replicas: 0 + data_stream: { } + - do: + indices.create_data_stream: + name: log-test-1 + - do: + indices.create_data_stream: + name: log-test-2 + - is_true: acknowledged + - do: + indices.update_aliases: + body: + actions: + - add: + index: log-test-* + aliases: test_alias1 + - remove: + index: log-test-* + aliases: test_non_existing + must_exist: false + - is_true: errors + - length: { action_results: 2 } + - match: { action_results.0.success: true } + - match: { action_results.0.action: add } + - match: { action_results.1.success: false } + - match: { action_results.1.action: remove } + - match: { action_results.1.error: aliases_not_found_exception } diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.update_aliases/40_must_exist.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.update_aliases/40_must_exist.yml index dbe167608e576..e0da7803bb0e7 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.update_aliases/40_must_exist.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.update_aliases/40_must_exist.yml @@ -82,3 +82,86 @@ - remove_index: index: test_index must_exist: true +--- +"Partial success with must_exist == false": + - do: + indices.create: + index: test_index + - do: + indices.update_aliases: + body: + actions: + - add: + index: test_index + aliases: test_alias1 + - remove: + index: test_index + aliases: test_non_existing + must_exist: false + - is_true: errors + - match: { action_results.0.success: true } + - match: { action_results.0.action: add } + - match: { action_results.1.success: false } + - match: { action_results.1.action: remove } + - match: { action_results.1.error: aliases_not_found_exception } +--- +"Partial success with must_exist == null (default)": + - do: + indices.create: + index: test_index + - do: + indices.update_aliases: + body: + actions: + - add: + index: test_index + aliases: test_alias1 + - remove: + index: test_index + aliases: test_non_existing + - is_true: errors + - match: { action_results.0.success: true } + - match: { action_results.0.action: add } + - match: { action_results.1.success: false } + - match: { action_results.1.action: remove } + - match: { action_results.1.error: aliases_not_found_exception } +--- +"No action_results field if all actions successful": + - do: + indices.create: + index: test_index + - do: + indices.update_aliases: + body: + actions: + - add: + index: test_index + aliases: test_alias1 + - is_false: errors + - match: { action_results: null } +--- +"Single result per input action": + - do: + indices.create: + index: test_index1 + - do: + indices.create: + index: test_index2 + - do: + indices.update_aliases: + body: + actions: + - add: + index: test_index* + aliases: test_alias1 + - remove: + index: test_index* + aliases: test_non_existing + - length: { action_results: 2 } + - is_true: errors + - match: { action_results.0.success: true } + - match: { action_results.0.action: add } + - match: { action_results.1.success: false } + - match: { action_results.1.action: remove } + - match: { action_results.1.error: aliases_not_found_exception } + diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesClusterStateUpdateRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesClusterStateUpdateRequest.java index b52098a49c002..1f87cf618dfcf 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesClusterStateUpdateRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesClusterStateUpdateRequest.java @@ -7,6 +7,7 @@ */ package org.elasticsearch.action.admin.indices.alias; +import org.elasticsearch.action.admin.indices.alias.IndicesAliasesResponse.AliasActionResult; import org.elasticsearch.cluster.ack.ClusterStateUpdateRequest; import org.elasticsearch.cluster.metadata.AliasAction; @@ -18,8 +19,11 @@ public class IndicesAliasesClusterStateUpdateRequest extends ClusterStateUpdateRequest { private final List actions; - public IndicesAliasesClusterStateUpdateRequest(List actions) { + private final List actionResults; + + public IndicesAliasesClusterStateUpdateRequest(List actions, List actionResults) { this.actions = actions; + this.actionResults = actionResults; } /** @@ -28,4 +32,8 @@ public IndicesAliasesClusterStateUpdateRequest(List actions) { public List actions() { return actions; } + + public List getActionResults() { + return actionResults; + } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java index a4f5ee9eb672b..628f5220cc22c 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java @@ -105,6 +105,10 @@ public byte value() { return value; } + public String getFieldName() { + return fieldName; + } + public static Type fromValue(byte value) { return switch (value) { case 0 -> ADD; diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequestBuilder.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequestBuilder.java index 4e49a5fe8d400..1462e36ea7895 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequestBuilder.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequestBuilder.java @@ -10,7 +10,6 @@ import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest.AliasActions; import org.elasticsearch.action.support.master.AcknowledgedRequestBuilder; -import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.client.internal.ElasticsearchClient; import org.elasticsearch.index.query.QueryBuilder; @@ -21,7 +20,7 @@ */ public class IndicesAliasesRequestBuilder extends AcknowledgedRequestBuilder< IndicesAliasesRequest, - AcknowledgedResponse, + IndicesAliasesResponse, IndicesAliasesRequestBuilder> { public IndicesAliasesRequestBuilder(ElasticsearchClient client) { diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java new file mode 100644 index 0000000000000..2d4bb6b6870c7 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java @@ -0,0 +1,136 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.action.admin.indices.alias; + +import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest.AliasActions; +import org.elasticsearch.action.support.master.AcknowledgedResponse; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.xcontent.ToXContentObject; +import org.elasticsearch.xcontent.XContentBuilder; + +import java.io.IOException; +import java.util.List; + +public class IndicesAliasesResponse extends AcknowledgedResponse { + + public static final IndicesAliasesResponse NOT_ACKNOWLEDGED = new IndicesAliasesResponse(false, false, List.of()); + public static final IndicesAliasesResponse ACKNOWLEDGED_NO_ERRORS = new IndicesAliasesResponse(true, false, List.of()); + + private static final String ACTION_RESULTS_FIELD = "action_results"; + private static final String ERRORS_FIELD = "errors"; + + private final List actionResults; + private final boolean errors; + + protected IndicesAliasesResponse(StreamInput in) throws IOException { + super(in); + this.errors = in.readBoolean(); + this.actionResults = in.readCollectionAsList(AliasActionResult::new); + } + + private IndicesAliasesResponse(boolean acknowledged, boolean errors, final List actionResults) { + super(acknowledged); + this.errors = errors; + this.actionResults = actionResults; + } + + public static IndicesAliasesResponse build(final List actionResults) { + final boolean errors = actionResults.stream().anyMatch(a -> a.success == false); + return new IndicesAliasesResponse(true, errors, actionResults); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + super.writeTo(out); + out.writeBoolean(errors); + out.writeCollection(actionResults); + } + + @Override + protected void addCustomFields(XContentBuilder builder, Params params) throws IOException { + builder.field(ERRORS_FIELD, errors); + // if there are no errors, don't provide granular list of results + if (errors) { + builder.field(ACTION_RESULTS_FIELD, actionResults); + } + } + + public static class AliasActionResult implements Writeable, ToXContentObject { + public static AliasActionResult REMOVE_MISSING = new AliasActionResult( + AliasActions.Type.REMOVE, + false, + "aliases_not_found_exception" + ); + + private final AliasActions.Type actionType; + private final boolean success; + private final String error; + + boolean getSuccess() { + return success; + } + + public static AliasActionResult merge(AliasActionResult currResult, AliasActionResult newResult) { + // there is not currently a result value for this action, so return new value + if (currResult == null) { + return newResult; + } + + // current value is error due to removal of missing alias, replace with newResult + if (currResult.success == false) { + return newResult; + } + + // return current value the action has been successful + return currResult; + } + + public static AliasActionResult buildSuccess(AliasActions action) { + return new AliasActionResult(action.actionType(), true, null); + } + + private AliasActionResult(AliasActions.Type actionType, boolean success, String error) { + assert error != null ^ success : "AliasActionResult should contain error message if and only if action not successful"; + this.actionType = actionType; + this.success = success; + this.error = error; + } + + private AliasActionResult(StreamInput in) throws IOException { + this.actionType = AliasActions.Type.fromValue(in.readByte()); + this.success = in.readBoolean(); + this.error = in.readOptionalString(); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeByte(actionType.value()); + out.writeBoolean(success); + out.writeOptionalString(error); + } + + public static final String ACTION_FIELD = "action"; + public static final String SUCCESS_FIELD = "success"; + public static final String ERROR_FIELD = "error"; + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(); + builder.field(ACTION_FIELD, actionType.getFieldName()); + builder.field(SUCCESS_FIELD, success); + if (error != null) { + builder.field(ERROR_FIELD, error); + } + builder.endObject(); + return builder; + } + } +} diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/TransportIndicesAliasesAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/TransportIndicesAliasesAction.java index e56be8852e7df..02553ac147d5c 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/TransportIndicesAliasesAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/TransportIndicesAliasesAction.java @@ -14,9 +14,9 @@ import org.elasticsearch.action.ActionType; import org.elasticsearch.action.RequestValidators; import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest.AliasActions; +import org.elasticsearch.action.admin.indices.alias.IndicesAliasesResponse.AliasActionResult; import org.elasticsearch.action.support.ActionFilters; -import org.elasticsearch.action.support.master.AcknowledgedResponse; -import org.elasticsearch.action.support.master.AcknowledgedTransportMasterNodeAction; +import org.elasticsearch.action.support.master.TransportMasterNodeAction; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.block.ClusterBlockException; import org.elasticsearch.cluster.block.ClusterBlockLevel; @@ -56,10 +56,10 @@ /** * Add/remove aliases action */ -public class TransportIndicesAliasesAction extends AcknowledgedTransportMasterNodeAction { +public class TransportIndicesAliasesAction extends TransportMasterNodeAction { public static final String NAME = "indices:admin/aliases"; - public static final ActionType TYPE = new ActionType<>(NAME); + public static final ActionType TYPE = new ActionType<>(NAME); private static final Logger logger = LogManager.getLogger(TransportIndicesAliasesAction.class); private final MetadataIndexAliasesService indexAliasesService; @@ -85,6 +85,7 @@ public TransportIndicesAliasesAction( actionFilters, IndicesAliasesRequest::new, indexNameExpressionResolver, + IndicesAliasesResponse::new, EsExecutors.DIRECT_EXECUTOR_SERVICE ); this.indexAliasesService = indexAliasesService; @@ -106,15 +107,19 @@ protected void masterOperation( Task task, final IndicesAliasesRequest request, final ClusterState state, - final ActionListener listener + final ActionListener listener ) { // Expand the indices names List actions = request.aliasActions(); List finalActions = new ArrayList<>(); + List actionResults = new ArrayList<>(); // Resolve all the AliasActions into AliasAction instances and gather all the aliases Set aliases = new HashSet<>(); + for (AliasActions action : actions) { + + AliasActionResult actionResult = null; List concreteDataStreams = indexNameExpressionResolver.dataStreamNames( state, request.indicesOptions(), @@ -161,18 +166,28 @@ protected void masterOperation( finalActions.add(new AddDataStreamAlias(alias, dataStreamName, action.writeIndex(), action.filter())); } } + actionResults.add(AliasActionResult.buildSuccess(action)); continue; } case REMOVE -> { for (String dataStreamName : concreteDataStreams) { - for (String alias : concreteDataStreamAliases(action, state.metadata(), dataStreamName)) { + var dataStreamAliases = concreteDataStreamAliases(action, state.metadata(), dataStreamName); + for (String alias : dataStreamAliases) { finalActions.add(new AliasAction.RemoveDataStreamAlias(alias, dataStreamName, action.mustExist())); } + + AliasActionResult newResult = dataStreamAliases.length == 0 + ? AliasActionResult.REMOVE_MISSING + : AliasActionResult.buildSuccess(action); + actionResult = AliasActionResult.merge(actionResult, newResult); } + if (nonBackingIndices.isEmpty() == false) { // Regular aliases/indices match as well with the provided expression. // (Only when adding new aliases, matching both data streams and indices is disallowed) } else { + // there are no additional regular indices, so add result directly + actionResults.add(actionResult); continue; } } @@ -206,6 +221,7 @@ protected void masterOperation( for (final Index index : concreteIndices) { switch (action.actionType()) { case ADD: + actionResult = AliasActionResult.buildSuccess(action); for (String alias : concreteAliases(action, state.metadata(), index.getName())) { String resolvedName = IndexNameExpressionResolver.resolveDateMathExpression(alias, now); finalActions.add( @@ -222,25 +238,33 @@ protected void masterOperation( } break; case REMOVE: - for (String alias : concreteAliases(action, state.metadata(), index.getName())) { + String[] matchingAliases = concreteAliases(action, state.metadata(), index.getName()); + AliasActionResult newResult = matchingAliases.length == 0 + ? AliasActionResult.REMOVE_MISSING + : AliasActionResult.buildSuccess(action); + actionResult = AliasActionResult.merge(actionResult, newResult); + for (String alias : matchingAliases) { finalActions.add(new AliasAction.Remove(index.getName(), alias, action.mustExist())); } break; case REMOVE_INDEX: + actionResult = AliasActionResult.buildSuccess(action); finalActions.add(new AliasAction.RemoveIndex(index.getName())); break; default: throw new IllegalArgumentException("Unsupported action [" + action.actionType() + "]"); } } + actionResults.add(actionResult); } if (finalActions.isEmpty() && false == actions.isEmpty()) { throw new AliasesNotFoundException(aliases.toArray(new String[aliases.size()])); } request.aliasActions().clear(); - IndicesAliasesClusterStateUpdateRequest updateRequest = new IndicesAliasesClusterStateUpdateRequest(unmodifiableList(finalActions)) - .ackTimeout(request.timeout()) - .masterNodeTimeout(request.masterNodeTimeout()); + IndicesAliasesClusterStateUpdateRequest updateRequest = new IndicesAliasesClusterStateUpdateRequest( + unmodifiableList(finalActions), + unmodifiableList(actionResults) + ).ackTimeout(request.timeout()).masterNodeTimeout(request.masterNodeTimeout()); indexAliasesService.indicesAliases(updateRequest, listener.delegateResponse((l, e) -> { logger.debug("failed to perform aliases", e); diff --git a/server/src/main/java/org/elasticsearch/client/internal/IndicesAdminClient.java b/server/src/main/java/org/elasticsearch/client/internal/IndicesAdminClient.java index d931302740f19..8821a50c66fd0 100644 --- a/server/src/main/java/org/elasticsearch/client/internal/IndicesAdminClient.java +++ b/server/src/main/java/org/elasticsearch/client/internal/IndicesAdminClient.java @@ -12,6 +12,7 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest; import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequestBuilder; +import org.elasticsearch.action.admin.indices.alias.IndicesAliasesResponse; import org.elasticsearch.action.admin.indices.alias.get.GetAliasesRequest; import org.elasticsearch.action.admin.indices.alias.get.GetAliasesRequestBuilder; import org.elasticsearch.action.admin.indices.alias.get.GetAliasesResponse; @@ -371,7 +372,7 @@ public interface IndicesAdminClient extends ElasticsearchClient { * @param request The index aliases request * @return The result future */ - ActionFuture aliases(IndicesAliasesRequest request); + ActionFuture aliases(IndicesAliasesRequest request); /** * Allows to add/remove aliases from indices. @@ -379,7 +380,7 @@ public interface IndicesAdminClient extends ElasticsearchClient { * @param request The index aliases request * @param listener A listener to be notified with a result */ - void aliases(IndicesAliasesRequest request, ActionListener listener); + void aliases(IndicesAliasesRequest request, ActionListener listener); /** * Allows to add/remove aliases from indices. diff --git a/server/src/main/java/org/elasticsearch/client/internal/support/AbstractClient.java b/server/src/main/java/org/elasticsearch/client/internal/support/AbstractClient.java index c6d9c3a8f3563..b68b52031baa2 100644 --- a/server/src/main/java/org/elasticsearch/client/internal/support/AbstractClient.java +++ b/server/src/main/java/org/elasticsearch/client/internal/support/AbstractClient.java @@ -118,6 +118,7 @@ import org.elasticsearch.action.admin.cluster.storedscripts.TransportPutStoredScriptAction; import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest; import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequestBuilder; +import org.elasticsearch.action.admin.indices.alias.IndicesAliasesResponse; import org.elasticsearch.action.admin.indices.alias.TransportIndicesAliasesAction; import org.elasticsearch.action.admin.indices.alias.get.GetAliasesAction; import org.elasticsearch.action.admin.indices.alias.get.GetAliasesRequest; @@ -1083,12 +1084,12 @@ public ThreadPool threadPool() { } @Override - public ActionFuture aliases(final IndicesAliasesRequest request) { + public ActionFuture aliases(final IndicesAliasesRequest request) { return execute(TransportIndicesAliasesAction.TYPE, request); } @Override - public void aliases(final IndicesAliasesRequest request, final ActionListener listener) { + public void aliases(final IndicesAliasesRequest request, final ActionListener listener) { execute(TransportIndicesAliasesAction.TYPE, request, listener); } diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/AliasAction.java b/server/src/main/java/org/elasticsearch/cluster/metadata/AliasAction.java index 63647e53619fe..533ae3a3ad50d 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/AliasAction.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/AliasAction.java @@ -8,10 +8,10 @@ package org.elasticsearch.cluster.metadata; -import org.elasticsearch.ResourceNotFoundException; import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest; import org.elasticsearch.common.Strings; import org.elasticsearch.core.Nullable; +import org.elasticsearch.rest.action.admin.indices.AliasesNotFoundException; /** * Individual operation to perform on the cluster state as part of an {@link IndicesAliasesRequest}. @@ -189,7 +189,7 @@ boolean removeIndex() { boolean apply(NewAliasValidator aliasValidator, Metadata.Builder metadata, IndexMetadata index) { if (false == index.getAliases().containsKey(alias)) { if (mustExist != null && mustExist) { - throw new ResourceNotFoundException("required alias [" + alias + "] does not exist"); + throw new AliasesNotFoundException(alias); } return false; } diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesService.java index fb5acbdd2ac49..d9cd1a7725ca8 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesService.java @@ -11,7 +11,7 @@ import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.admin.indices.alias.IndicesAliasesClusterStateUpdateRequest; -import org.elasticsearch.action.support.master.AcknowledgedResponse; +import org.elasticsearch.action.admin.indices.alias.IndicesAliasesResponse; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.ClusterStateAckListener; import org.elasticsearch.cluster.ClusterStateTaskExecutor; @@ -79,7 +79,10 @@ public Tuple executeTask(ApplyAliasesTask this.taskQueue = clusterService.createTaskQueue("index-aliases", Priority.URGENT, this.executor); } - public void indicesAliases(final IndicesAliasesClusterStateUpdateRequest request, final ActionListener listener) { + public void indicesAliases( + final IndicesAliasesClusterStateUpdateRequest request, + final ActionListener listener + ) { taskQueue.submitTask("index-aliases", new ApplyAliasesTask(request, listener), null); // TODO use request.masterNodeTimeout() here? } @@ -254,7 +257,7 @@ private static void validateAliasTargetIsNotDSBackingIndex(ClusterState currentS /** * A cluster state update task that consists of the cluster state request and the listeners that need to be notified upon completion. */ - record ApplyAliasesTask(IndicesAliasesClusterStateUpdateRequest request, ActionListener listener) + record ApplyAliasesTask(IndicesAliasesClusterStateUpdateRequest request, ActionListener listener) implements ClusterStateTaskListener, ClusterStateAckListener { @@ -271,17 +274,17 @@ public boolean mustAck(DiscoveryNode discoveryNode) { @Override public void onAllNodesAcked() { - listener.onResponse(AcknowledgedResponse.TRUE); + listener.onResponse(IndicesAliasesResponse.build(request.getActionResults())); } @Override public void onAckFailure(Exception e) { - listener.onResponse(AcknowledgedResponse.FALSE); + listener.onResponse(IndicesAliasesResponse.NOT_ACKNOWLEDGED); } @Override public void onAckTimeout() { - listener.onResponse(AcknowledgedResponse.FALSE); + listener.onResponse(IndicesAliasesResponse.NOT_ACKNOWLEDGED); } @Override diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesServiceTests.java index 0901b1190cfc0..f5665d2538cbf 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesServiceTests.java @@ -8,7 +8,6 @@ package org.elasticsearch.cluster.metadata; -import org.elasticsearch.ResourceNotFoundException; import org.elasticsearch.action.admin.indices.alias.IndicesAliasesClusterStateUpdateRequest; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; @@ -19,6 +18,7 @@ import org.elasticsearch.core.Tuple; import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.index.IndexVersion; +import org.elasticsearch.rest.action.admin.indices.AliasesNotFoundException; import org.elasticsearch.test.ClusterServiceUtils; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.index.IndexVersionUtils; @@ -156,11 +156,11 @@ public void testMustExist() { // Show that removing non-existing alias with mustExist == true fails final ClusterState finalCS = after; - final ResourceNotFoundException iae = expectThrows( - ResourceNotFoundException.class, + final AliasesNotFoundException iae = expectThrows( + AliasesNotFoundException.class, () -> service.applyAliasActions(finalCS, singletonList(new AliasAction.Remove(index, "test_2", true))) ); - assertThat(iae.getMessage(), containsString("required alias [test_2] does not exist")); + assertThat(iae.getMessage(), containsString("aliases [test_2] missing")); } public void testMultipleIndices() { @@ -690,10 +690,12 @@ public void testAddAndRemoveAliasClusterStateUpdate() throws Exception { String index = randomAlphaOfLength(5); ClusterState before = createIndex(ClusterState.builder(ClusterName.DEFAULT).build(), index); IndicesAliasesClusterStateUpdateRequest addAliasRequest = new IndicesAliasesClusterStateUpdateRequest( - List.of(new AliasAction.Add(index, "test", null, null, null, null, null)) + List.of(new AliasAction.Add(index, "test", null, null, null, null, null)), + List.of() ); IndicesAliasesClusterStateUpdateRequest removeAliasRequest = new IndicesAliasesClusterStateUpdateRequest( - List.of(new AliasAction.Remove(index, "test", true)) + List.of(new AliasAction.Remove(index, "test", true)), + List.of() ); ClusterState after = ClusterStateTaskExecutorUtils.executeAndAssertSuccessful( diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/annotations/AnnotationIndex.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/annotations/AnnotationIndex.java index d3a20235e3a38..07be597c7024e 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/annotations/AnnotationIndex.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/annotations/AnnotationIndex.java @@ -14,9 +14,9 @@ import org.elasticsearch.action.admin.cluster.health.TransportClusterHealthAction; import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest; import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequestBuilder; +import org.elasticsearch.action.admin.indices.alias.IndicesAliasesResponse; import org.elasticsearch.action.admin.indices.create.CreateIndexRequest; import org.elasticsearch.action.admin.indices.create.CreateIndexResponse; -import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.client.internal.Client; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.IndexAbstraction; @@ -130,7 +130,9 @@ public static void createAnnotationsIndexIfNecessary( client.threadPool().getThreadContext(), ML_ORIGIN, requestBuilder.request(), - finalDelegate.delegateFailureAndWrap((l, r) -> checkMappingsListener.onResponse(r.isAcknowledged())), + finalDelegate.delegateFailureAndWrap( + (l, r) -> checkMappingsListener.onResponse(r.isAcknowledged()) + ), client.admin().indices()::aliases ); }); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/utils/MlIndexAndAlias.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/utils/MlIndexAndAlias.java index 016540815fb0a..d4ec7563b868b 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/utils/MlIndexAndAlias.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/utils/MlIndexAndAlias.java @@ -16,6 +16,7 @@ import org.elasticsearch.action.admin.indices.alias.Alias; import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest; import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequestBuilder; +import org.elasticsearch.action.admin.indices.alias.IndicesAliasesResponse; import org.elasticsearch.action.admin.indices.create.CreateIndexRequest; import org.elasticsearch.action.admin.indices.create.CreateIndexRequestBuilder; import org.elasticsearch.action.admin.indices.create.CreateIndexResponse; @@ -295,7 +296,7 @@ private static void updateWriteAlias( client.threadPool().getThreadContext(), ML_ORIGIN, request, - listener.delegateFailureAndWrap((l, resp) -> l.onResponse(resp.isAcknowledged())), + listener.delegateFailureAndWrap((l, resp) -> l.onResponse(resp.isAcknowledged())), client.admin().indices()::aliases ); } diff --git a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/search/SearchApplicationIndexService.java b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/search/SearchApplicationIndexService.java index 46e4d45f2c146..d7c83ae62cec7 100644 --- a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/search/SearchApplicationIndexService.java +++ b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/search/SearchApplicationIndexService.java @@ -18,6 +18,7 @@ import org.elasticsearch.action.DocWriteResponse; import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest; import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequestBuilder; +import org.elasticsearch.action.admin.indices.alias.IndicesAliasesResponse; import org.elasticsearch.action.admin.indices.alias.get.GetAliasesRequest; import org.elasticsearch.action.admin.indices.alias.get.GetAliasesResponse; import org.elasticsearch.action.delete.DeleteRequest; @@ -223,7 +224,7 @@ private static String getSearchAliasName(SearchApplication app) { public void putSearchApplication(SearchApplication app, boolean create, ActionListener listener) { createOrUpdateAlias(app, new ActionListener<>() { @Override - public void onResponse(AcknowledgedResponse acknowledgedResponse) { + public void onResponse(IndicesAliasesResponse response) { updateSearchApplication(app, create, listener); } @@ -240,7 +241,7 @@ public void onFailure(Exception e) { }); } - private void createOrUpdateAlias(SearchApplication app, ActionListener listener) { + private void createOrUpdateAlias(SearchApplication app, ActionListener listener) { final Metadata metadata = clusterService.state().metadata(); final String searchAliasName = getSearchAliasName(app); diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlInitializationService.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlInitializationService.java index dab2010035b66..c849e69c780bd 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlInitializationService.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlInitializationService.java @@ -10,6 +10,7 @@ import org.apache.logging.log4j.Logger; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest; +import org.elasticsearch.action.admin.indices.alias.IndicesAliasesResponse; import org.elasticsearch.action.admin.indices.alias.TransportIndicesAliasesAction; import org.elasticsearch.action.admin.indices.alias.get.GetAliasesAction; import org.elasticsearch.action.admin.indices.alias.get.GetAliasesRequest; @@ -173,7 +174,7 @@ private void makeMlInternalIndicesHidden() { String[] mlHiddenIndexPatterns = MachineLearning.getMlHiddenIndexPatterns(); // Step 5: Handle errors encountered on the way. - ActionListener finalListener = ActionListener.wrap(updateAliasesResponse -> { + ActionListener finalListener = ActionListener.wrap(updateAliasesResponse -> { if (updateAliasesResponse.isAcknowledged() == false) { logger.warn("One or more of the ML internal aliases could not be made hidden."); return; @@ -194,7 +195,7 @@ private void makeMlInternalIndicesHidden() { } if (indicesAliasesRequest.getAliasActions().isEmpty()) { logger.debug("There are no ML internal aliases that need to be made hidden, [{}]", getAliasesResponse.getAliases()); - finalListener.onResponse(AcknowledgedResponse.TRUE); + finalListener.onResponse(IndicesAliasesResponse.ACKNOWLEDGED_NO_ERRORS); return; } String indicesWithNonHiddenAliasesString = indicesAliasesRequest.getAliasActions() diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobDataDeleter.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobDataDeleter.java index 577bbe3dac6ce..b9cc1902b7ab6 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobDataDeleter.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobDataDeleter.java @@ -10,6 +10,7 @@ import org.apache.logging.log4j.Logger; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest; +import org.elasticsearch.action.admin.indices.alias.IndicesAliasesResponse; import org.elasticsearch.action.admin.indices.alias.get.GetAliasesRequest; import org.elasticsearch.action.admin.indices.alias.get.GetAliasesResponse; import org.elasticsearch.action.admin.indices.delete.DeleteIndexRequest; @@ -287,7 +288,7 @@ public void deleteJobDocuments( AtomicReference indexNames = new AtomicReference<>(); - final ActionListener completionHandler = ActionListener.wrap( + final ActionListener completionHandler = ActionListener.wrap( response -> finishedHandler.accept(response.isAcknowledged()), failureHandler ); @@ -295,7 +296,7 @@ public void deleteJobDocuments( // Step 9. If we did not drop the indices and after DBQ state done, we delete the aliases ActionListener dbqHandler = ActionListener.wrap(bulkByScrollResponse -> { if (bulkByScrollResponse == null) { // no action was taken by DBQ, assume indices were deleted - completionHandler.onResponse(AcknowledgedResponse.TRUE); + completionHandler.onResponse(IndicesAliasesResponse.ACKNOWLEDGED_NO_ERRORS); } else { if (bulkByScrollResponse.isTimedOut()) { logger.warn("[{}] DeleteByQuery for indices [{}] timed out.", jobId, String.join(", ", indexNames.get())); @@ -469,7 +470,7 @@ private void deleteResultsByQuery( executeAsyncWithOrigin(client, ML_ORIGIN, RefreshAction.INSTANCE, refreshRequest, refreshListener); } - private void deleteAliases(@SuppressWarnings("HiddenField") String jobId, ActionListener finishedHandler) { + private void deleteAliases(@SuppressWarnings("HiddenField") String jobId, ActionListener finishedHandler) { final String readAliasName = AnomalyDetectorsIndex.jobResultsAliasedName(jobId); final String writeAliasName = AnomalyDetectorsIndex.resultsWriteAlias(jobId); @@ -486,7 +487,7 @@ private void deleteAliases(@SuppressWarnings("HiddenField") String jobId, Action if (removeRequest == null) { // don't error if the job's aliases have already been deleted - carry on and delete the // rest of the job's data - finishedHandler.onResponse(AcknowledgedResponse.TRUE); + finishedHandler.onResponse(IndicesAliasesResponse.ACKNOWLEDGED_NO_ERRORS); return; } executeAsyncWithOrigin( diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProvider.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProvider.java index f8f1e95fecd2e..35ee88360d602 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProvider.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobResultsProvider.java @@ -16,6 +16,7 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.DelegatingActionListener; import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest; +import org.elasticsearch.action.admin.indices.alias.IndicesAliasesResponse; import org.elasticsearch.action.admin.indices.create.CreateIndexRequest; import org.elasticsearch.action.admin.indices.create.CreateIndexResponse; import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsRequest; @@ -344,7 +345,7 @@ public void createJobResultIndex(Job job, ClusterState state, final ActionListen client.threadPool().getThreadContext(), ML_ORIGIN, request, - ActionListener.wrap(r -> finalListener.onResponse(true), finalListener::onFailure), + ActionListener.wrap(r -> finalListener.onResponse(true), finalListener::onFailure), client.admin().indices()::aliases ); }, finalListener::onFailure); diff --git a/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/TransformClusterStateListener.java b/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/TransformClusterStateListener.java index e2f66fe914bc2..970403e49c5a3 100644 --- a/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/TransformClusterStateListener.java +++ b/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/TransformClusterStateListener.java @@ -11,7 +11,7 @@ import org.apache.logging.log4j.Logger; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest; -import org.elasticsearch.action.support.master.AcknowledgedResponse; +import org.elasticsearch.action.admin.indices.alias.IndicesAliasesResponse; import org.elasticsearch.client.internal.Client; import org.elasticsearch.cluster.ClusterChangedEvent; import org.elasticsearch.cluster.ClusterState; @@ -97,7 +97,7 @@ private static void createAuditAliasForDataFrameBWC(ClusterState state, Client c client.threadPool().getThreadContext(), TRANSFORM_ORIGIN, request, - ActionListener.wrap(r -> finalListener.onResponse(r.isAcknowledged()), finalListener::onFailure), + ActionListener.wrap(r -> finalListener.onResponse(r.isAcknowledged()), finalListener::onFailure), client.admin().indices()::aliases ); } From dbb590a366ed212b784f6667fcdbc77589b211ed Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Wed, 20 Mar 2024 13:21:41 -0500 Subject: [PATCH 02/25] Add response body description to aliases API docs --- docs/reference/alias.asciidoc | 6 ++--- docs/reference/indices/aliases.asciidoc | 35 +++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/docs/reference/alias.asciidoc b/docs/reference/alias.asciidoc index 6873d5a763b90..044ef210cc7e8 100644 --- a/docs/reference/alias.asciidoc +++ b/docs/reference/alias.asciidoc @@ -122,12 +122,11 @@ POST _aliases // end::alias-multiple-actions-example[] [discrete] -[[Multiple action results]] +[[multiple-action-results]] === Multiple action results When using multiple actions, if some succeed and some fail, a list of per-action results will be returned. -// tag::alias-multiple-actions-result-example[] Consider a similar action list to the previous example, but now with an alias `log-non-existing`, which does not yet exist. In this case, the `remove` action will fail, but the `add` action will succeed. The response will contain the list `action_results`, with a result for every requested action. @@ -176,10 +175,9 @@ The API returns the following result: -------------------------------------------------- Allowing the action list to succeed partially may not provide the desired result. -It may be more appropriate to use set `must_exist` to `true`, which will cause the entire action +It may be more appropriate to set `must_exist` to `true`, which will cause the entire action list to fail if a single action fails. -// end::alias-multiple-actions-example[] [discrete] [[add-alias-at-creation]] diff --git a/docs/reference/indices/aliases.asciidoc b/docs/reference/indices/aliases.asciidoc index 096cb9d611900..08293722b5bd9 100644 --- a/docs/reference/indices/aliases.asciidoc +++ b/docs/reference/indices/aliases.asciidoc @@ -174,3 +174,38 @@ stream aliases don't support this parameter. Only the `add` action supports this parameter. ===== ==== + + + +[role="child_attributes"] +[[indices-aliases-api-response-body]] +==== {api-response-body-title} + +`acknowledged`:: +(Boolean) +If `true`, the request received a response from the master node within the +`timeout` period. + +`errors`:: +(Boolean) +If `true`, at least one of the requested actions failed. + +`action_results`:: +(array of objects) Results for each requested action. ++ +.Properties of `action_results` objects +[%collapsible%open] +==== + +`action`:: +(string) +The type of the associated action, the values will be one of `add`, `remove`, or `remove_index`. + +`success`:: +(boolean) +If true, the requested action was successfully applied at least once. + +`error`:: +(Optional, string) +Only present if `success` is `false`. Consists of a message describing why the action failed +==== From e701c5e8ce71a84d0b93851e0b785c3171cb63f0 Mon Sep 17 00:00:00 2001 From: Parker Timmins <45302127+parkertimmins@users.noreply.github.com> Date: Wed, 20 Mar 2024 13:33:53 -0500 Subject: [PATCH 03/25] Update docs/changelog/106514.yaml --- docs/changelog/106514.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 docs/changelog/106514.yaml diff --git a/docs/changelog/106514.yaml b/docs/changelog/106514.yaml new file mode 100644 index 0000000000000..5b25f40db2742 --- /dev/null +++ b/docs/changelog/106514.yaml @@ -0,0 +1,6 @@ +pr: 106514 +summary: Add granular error list to alias action response +area: Indices APIs +type: feature +issues: + - 94478 From c001f79746c8e7d142d4c762fe59449355bb7d21 Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Wed, 20 Mar 2024 16:03:32 -0500 Subject: [PATCH 04/25] Add new transport version and fix broken tests --- .../org/elasticsearch/TransportVersions.java | 1 + .../indices/alias/IndicesAliasesResponse.java | 17 +++++++++++++---- .../xpack/core/ilm/ShrinkSetAliasStepTests.java | 8 ++++---- .../core/ml/utils/MlIndexAndAliasTests.java | 10 +++++----- 4 files changed, 23 insertions(+), 13 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/TransportVersions.java b/server/src/main/java/org/elasticsearch/TransportVersions.java index 418720284eda8..b61633de1eb51 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersions.java +++ b/server/src/main/java/org/elasticsearch/TransportVersions.java @@ -142,6 +142,7 @@ static TransportVersion def(int id) { public static final TransportVersion ESQL_SERIALIZE_ARRAY_BLOCK = def(8_602_00_0); public static final TransportVersion ADD_DATA_STREAM_GLOBAL_RETENTION = def(8_603_00_0); public static final TransportVersion ALLOCATION_STATS = def(8_604_00_0); + public static final TransportVersion ALIAS_ACTION_RESULTS = def(8_605_00_0); /* * STOP! READ THIS FIRST! No, really, diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java index 2d4bb6b6870c7..72445bc640c68 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java @@ -8,6 +8,7 @@ package org.elasticsearch.action.admin.indices.alias; +import org.elasticsearch.TransportVersions; import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest.AliasActions; import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.common.io.stream.StreamInput; @@ -32,8 +33,14 @@ public class IndicesAliasesResponse extends AcknowledgedResponse { protected IndicesAliasesResponse(StreamInput in) throws IOException { super(in); - this.errors = in.readBoolean(); - this.actionResults = in.readCollectionAsList(AliasActionResult::new); + + if (in.getTransportVersion().onOrAfter(TransportVersions.ALIAS_ACTION_RESULTS)) { + this.errors = in.readBoolean(); + this.actionResults = in.readCollectionAsList(AliasActionResult::new); + } else { + this.errors = false; + this.actionResults = List.of(); + } } private IndicesAliasesResponse(boolean acknowledged, boolean errors, final List actionResults) { @@ -50,8 +57,10 @@ public static IndicesAliasesResponse build(final List actionR @Override public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); - out.writeBoolean(errors); - out.writeCollection(actionResults); + if (out.getTransportVersion().onOrAfter(TransportVersions.ALIAS_ACTION_RESULTS)) { + out.writeBoolean(errors); + out.writeCollection(actionResults); + } } @Override diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/ShrinkSetAliasStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/ShrinkSetAliasStepTests.java index 15e1539570e28..d12cd17d957d4 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/ShrinkSetAliasStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/ShrinkSetAliasStepTests.java @@ -9,8 +9,8 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest; import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest.AliasActions; +import org.elasticsearch.action.admin.indices.alias.IndicesAliasesResponse; import org.elasticsearch.action.support.PlainActionFuture; -import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.cluster.metadata.AliasMetadata; import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.index.IndexVersion; @@ -90,8 +90,8 @@ public void testPerformAction() throws Exception { IndicesAliasesRequest request = (IndicesAliasesRequest) invocation.getArguments()[0]; assertThat(request.getAliasActions(), equalTo(expectedAliasActions)); @SuppressWarnings("unchecked") - ActionListener listener = (ActionListener) invocation.getArguments()[1]; - listener.onResponse(AcknowledgedResponse.TRUE); + ActionListener listener = (ActionListener) invocation.getArguments()[1]; + listener.onResponse(IndicesAliasesResponse.ACKNOWLEDGED_NO_ERRORS); return null; }).when(indicesClient).aliases(Mockito.any(), Mockito.any()); @@ -113,7 +113,7 @@ public void testPerformActionFailure() { Mockito.doAnswer((Answer) invocation -> { @SuppressWarnings("unchecked") - ActionListener listener = (ActionListener) invocation.getArguments()[1]; + ActionListener listener = (ActionListener) invocation.getArguments()[1]; listener.onFailure(exception); return null; }).when(indicesClient).aliases(Mockito.any(), Mockito.any()); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/utils/MlIndexAndAliasTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/utils/MlIndexAndAliasTests.java index e7dcc6b441a31..5e0de77a60bc1 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/utils/MlIndexAndAliasTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/utils/MlIndexAndAliasTests.java @@ -13,11 +13,11 @@ import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest; import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest.AliasActions; import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequestBuilder; +import org.elasticsearch.action.admin.indices.alias.IndicesAliasesResponse; import org.elasticsearch.action.admin.indices.create.CreateIndexRequest; import org.elasticsearch.action.admin.indices.create.CreateIndexRequestBuilder; import org.elasticsearch.action.admin.indices.create.CreateIndexResponse; import org.elasticsearch.action.admin.indices.template.put.TransportPutComposableIndexTemplateAction; -import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.action.support.master.MasterNodeRequest; import org.elasticsearch.client.internal.AdminClient; import org.elasticsearch.client.internal.Client; @@ -97,8 +97,8 @@ public void setUpMocks() { ); doAnswer(withResponse(new CreateIndexResponse(true, true, FIRST_CONCRETE_INDEX))).when(indicesAdminClient).create(any(), any()); when(indicesAdminClient.prepareAliases()).thenReturn(new IndicesAliasesRequestBuilder(client)); - doAnswer(withResponse(AcknowledgedResponse.TRUE)).when(indicesAdminClient).aliases(any(), any()); - doAnswer(withResponse(AcknowledgedResponse.TRUE)).when(indicesAdminClient).putTemplate(any(), any()); + doAnswer(withResponse(IndicesAliasesResponse.ACKNOWLEDGED_NO_ERRORS)).when(indicesAdminClient).aliases(any(), any()); + doAnswer(withResponse(IndicesAliasesResponse.ACKNOWLEDGED_NO_ERRORS)).when(indicesAdminClient).putTemplate(any(), any()); clusterAdminClient = mock(ClusterAdminClient.class); doAnswer(invocationOnMock -> { @@ -116,8 +116,8 @@ public void setUpMocks() { when(client.threadPool()).thenReturn(threadPool); when(client.admin()).thenReturn(adminClient); doAnswer(invocationOnMock -> { - ActionListener actionListener = (ActionListener) invocationOnMock.getArguments()[2]; - actionListener.onResponse(AcknowledgedResponse.TRUE); + ActionListener actionListener = (ActionListener) invocationOnMock.getArguments()[2]; + actionListener.onResponse(IndicesAliasesResponse.ACKNOWLEDGED_NO_ERRORS); return null; }).when(client) .execute( From 818e397b791da75676efc2fe959b9a714a710fb7 Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Wed, 20 Mar 2024 16:13:40 -0500 Subject: [PATCH 05/25] Formatting - need to call spotlessApply --- .../xpack/core/ml/utils/MlIndexAndAliasTests.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/utils/MlIndexAndAliasTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/utils/MlIndexAndAliasTests.java index 5e0de77a60bc1..f9fdc0c8362e5 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/utils/MlIndexAndAliasTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/utils/MlIndexAndAliasTests.java @@ -116,7 +116,8 @@ public void setUpMocks() { when(client.threadPool()).thenReturn(threadPool); when(client.admin()).thenReturn(adminClient); doAnswer(invocationOnMock -> { - ActionListener actionListener = (ActionListener) invocationOnMock.getArguments()[2]; + ActionListener actionListener = (ActionListener) invocationOnMock + .getArguments()[2]; actionListener.onResponse(IndicesAliasesResponse.ACKNOWLEDGED_NO_ERRORS); return null; }).when(client) From 1e77a38cdcfe7c44daa84b25b2fb24df1be1d8e8 Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Thu, 21 Mar 2024 09:12:03 -0500 Subject: [PATCH 06/25] Skip AliasActionResult yaml rest tests before 8.14 --- .../test/data_stream/140_data_stream_aliases.yml | 8 ++++---- .../test/indices.update_aliases/40_must_exist.yml | 12 ++++++++++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/modules/data-streams/src/yamlRestTest/resources/rest-api-spec/test/data_stream/140_data_stream_aliases.yml b/modules/data-streams/src/yamlRestTest/resources/rest-api-spec/test/data_stream/140_data_stream_aliases.yml index 35c70475d376d..eb91e26a6f079 100644 --- a/modules/data-streams/src/yamlRestTest/resources/rest-api-spec/test/data_stream/140_data_stream_aliases.yml +++ b/modules/data-streams/src/yamlRestTest/resources/rest-api-spec/test/data_stream/140_data_stream_aliases.yml @@ -310,8 +310,8 @@ --- "Action Results with multiple matching aliases": - skip: - version: " - 7.13.99" - reason: "data streams aliases are available from 7.14.0" + version: " - 8.13.99" + reason: "alias action results do not work until 8.14" features: allowed_warnings - do: allowed_warnings: @@ -349,8 +349,8 @@ --- "Single action result per action": - skip: - version: " - 7.13.99" - reason: "data streams aliases are available from 7.14.0" + version: " - 8.13.99" + reason: "alias action results do not work until 8.14" features: allowed_warnings - do: allowed_warnings: diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.update_aliases/40_must_exist.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.update_aliases/40_must_exist.yml index e0da7803bb0e7..c3fcb0ead18d7 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.update_aliases/40_must_exist.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.update_aliases/40_must_exist.yml @@ -84,6 +84,9 @@ must_exist: true --- "Partial success with must_exist == false": + - skip: + version: " - 8.13.99" + reason: "alias action results do not work until 8.14" - do: indices.create: index: test_index @@ -106,6 +109,9 @@ - match: { action_results.1.error: aliases_not_found_exception } --- "Partial success with must_exist == null (default)": + - skip: + version: " - 8.13.99" + reason: "alias action results do not work until 8.14" - do: indices.create: index: test_index @@ -127,6 +133,9 @@ - match: { action_results.1.error: aliases_not_found_exception } --- "No action_results field if all actions successful": + - skip: + version: " - 8.13.99" + reason: "alias action results do not work until 8.14" - do: indices.create: index: test_index @@ -141,6 +150,9 @@ - match: { action_results: null } --- "Single result per input action": + - skip: + version: " - 8.13.99" + reason: "alias action results do not work until 8.14" - do: indices.create: index: test_index1 From f541cbf537b44fb034cf4629e2c1157b388ce657 Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Thu, 21 Mar 2024 16:04:08 -0500 Subject: [PATCH 07/25] Fix broken compilation after merge from main --- .../application/search/SearchApplicationIndexService.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/search/SearchApplicationIndexService.java b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/search/SearchApplicationIndexService.java index eb6fb7e0c188f..8e3dd553cece5 100644 --- a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/search/SearchApplicationIndexService.java +++ b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/search/SearchApplicationIndexService.java @@ -333,14 +333,14 @@ private void removeAlias(String searchAliasName, ActionListener() { @Override - public void onResponse(AcknowledgedResponse acknowledgedResponse) { - listener.onResponse(AcknowledgedResponse.TRUE); + public void onResponse(IndicesAliasesResponse acknowledgedResponse) { + listener.onResponse(IndicesAliasesResponse.ACKNOWLEDGED_NO_ERRORS); } @Override public void onFailure(Exception e) { if (e instanceof ResourceNotFoundException) { - listener.onResponse(AcknowledgedResponse.TRUE); + listener.onResponse(IndicesAliasesResponse.ACKNOWLEDGED_NO_ERRORS); } else { listener.onFailure(e); } From 6e6aba0e7f81b6d540fbd603316e421bd7711461 Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Fri, 22 Mar 2024 10:59:09 -0500 Subject: [PATCH 08/25] Pass on actual response rather than default value --- .../application/search/SearchApplicationIndexService.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/search/SearchApplicationIndexService.java b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/search/SearchApplicationIndexService.java index 8e3dd553cece5..0ccef9acba088 100644 --- a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/search/SearchApplicationIndexService.java +++ b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/search/SearchApplicationIndexService.java @@ -333,8 +333,8 @@ private void removeAlias(String searchAliasName, ActionListener() { @Override - public void onResponse(IndicesAliasesResponse acknowledgedResponse) { - listener.onResponse(IndicesAliasesResponse.ACKNOWLEDGED_NO_ERRORS); + public void onResponse(IndicesAliasesResponse response) { + listener.onResponse(response); } @Override From d15732d0801735a373213382235fd2dc15125028 Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Fri, 22 Mar 2024 11:00:09 -0500 Subject: [PATCH 09/25] Clarify confusing phrase in docs --- docs/reference/indices/aliases.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reference/indices/aliases.asciidoc b/docs/reference/indices/aliases.asciidoc index 08293722b5bd9..46782991fdb89 100644 --- a/docs/reference/indices/aliases.asciidoc +++ b/docs/reference/indices/aliases.asciidoc @@ -203,7 +203,7 @@ The type of the associated action, the values will be one of `add`, `remove`, or `success`:: (boolean) -If true, the requested action was successfully applied at least once. +If true, the requested action was successfully applied to at least one alias. `error`:: (Optional, string) From c481858614527c47a8a7e8229ad605b3b52df07d Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Fri, 22 Mar 2024 11:01:03 -0500 Subject: [PATCH 10/25] Refactor main alias action result loop The current logic in this main loop is really hard to track. It is not obvious that the action results are guaranteed to not be null. It would also be easy to introduce a change that does make them null. Hopefully, this changes makes to more clear that the alias action results lists is built safely. --- .../indices/alias/IndicesAliasesResponse.java | 25 +++------------ .../alias/TransportIndicesAliasesAction.java | 31 ++++++------------- 2 files changed, 14 insertions(+), 42 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java index 72445bc640c68..62d5cf84fe180 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java @@ -83,27 +83,12 @@ public static class AliasActionResult implements Writeable, ToXContentObject { private final boolean success; private final String error; - boolean getSuccess() { - return success; - } - - public static AliasActionResult merge(AliasActionResult currResult, AliasActionResult newResult) { - // there is not currently a result value for this action, so return new value - if (currResult == null) { - return newResult; - } - - // current value is error due to removal of missing alias, replace with newResult - if (currResult.success == false) { - return newResult; + public static AliasActionResult build(AliasActions action, int numAliasesRemoved) { + if (action.actionType() == AliasActions.Type.REMOVE && numAliasesRemoved == 0) { + return REMOVE_MISSING; + } else { + return new AliasActionResult(action.actionType(), true, null); } - - // return current value the action has been successful - return currResult; - } - - public static AliasActionResult buildSuccess(AliasActions action) { - return new AliasActionResult(action.actionType(), true, null); } private AliasActionResult(AliasActions.Type actionType, boolean success, String error) { diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/TransportIndicesAliasesAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/TransportIndicesAliasesAction.java index 02553ac147d5c..6a72c2999ed43 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/TransportIndicesAliasesAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/TransportIndicesAliasesAction.java @@ -116,10 +116,9 @@ protected void masterOperation( List actionResults = new ArrayList<>(); // Resolve all the AliasActions into AliasAction instances and gather all the aliases Set aliases = new HashSet<>(); - for (AliasActions action : actions) { + int numAliasesRemoved = 0; - AliasActionResult actionResult = null; List concreteDataStreams = indexNameExpressionResolver.dataStreamNames( state, request.indicesOptions(), @@ -166,28 +165,21 @@ protected void masterOperation( finalActions.add(new AddDataStreamAlias(alias, dataStreamName, action.writeIndex(), action.filter())); } } - actionResults.add(AliasActionResult.buildSuccess(action)); + actionResults.add(AliasActionResult.build(action, numAliasesRemoved)); continue; } case REMOVE -> { for (String dataStreamName : concreteDataStreams) { - var dataStreamAliases = concreteDataStreamAliases(action, state.metadata(), dataStreamName); - for (String alias : dataStreamAliases) { + for (String alias : concreteDataStreamAliases(action, state.metadata(), dataStreamName)) { finalActions.add(new AliasAction.RemoveDataStreamAlias(alias, dataStreamName, action.mustExist())); + numAliasesRemoved++; } - - AliasActionResult newResult = dataStreamAliases.length == 0 - ? AliasActionResult.REMOVE_MISSING - : AliasActionResult.buildSuccess(action); - actionResult = AliasActionResult.merge(actionResult, newResult); } - if (nonBackingIndices.isEmpty() == false) { // Regular aliases/indices match as well with the provided expression. // (Only when adding new aliases, matching both data streams and indices is disallowed) } else { - // there are no additional regular indices, so add result directly - actionResults.add(actionResult); + actionResults.add(AliasActionResult.build(action, numAliasesRemoved)); continue; } } @@ -221,7 +213,6 @@ protected void masterOperation( for (final Index index : concreteIndices) { switch (action.actionType()) { case ADD: - actionResult = AliasActionResult.buildSuccess(action); for (String alias : concreteAliases(action, state.metadata(), index.getName())) { String resolvedName = IndexNameExpressionResolver.resolveDateMathExpression(alias, now); finalActions.add( @@ -238,24 +229,20 @@ protected void masterOperation( } break; case REMOVE: - String[] matchingAliases = concreteAliases(action, state.metadata(), index.getName()); - AliasActionResult newResult = matchingAliases.length == 0 - ? AliasActionResult.REMOVE_MISSING - : AliasActionResult.buildSuccess(action); - actionResult = AliasActionResult.merge(actionResult, newResult); - for (String alias : matchingAliases) { + for (String alias : concreteAliases(action, state.metadata(), index.getName())) { finalActions.add(new AliasAction.Remove(index.getName(), alias, action.mustExist())); + numAliasesRemoved++; } break; case REMOVE_INDEX: - actionResult = AliasActionResult.buildSuccess(action); finalActions.add(new AliasAction.RemoveIndex(index.getName())); break; default: throw new IllegalArgumentException("Unsupported action [" + action.actionType() + "]"); } } - actionResults.add(actionResult); + + actionResults.add(AliasActionResult.build(action, numAliasesRemoved)); } if (finalActions.isEmpty() && false == actions.isEmpty()) { throw new AliasesNotFoundException(aliases.toArray(new String[aliases.size()])); From f189e0c547910ef480e28274109e93b33b9059ac Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Tue, 26 Mar 2024 12:17:38 -0500 Subject: [PATCH 11/25] Add info from request to alias action response Ideally we would return the AliasActions request directly to the users. But the AliasActions object contains logic which we probably don't want to expose in an api. Also, index patterns are resolved in the security layer (if its on), meaning the indices field of the request does not contain the pattern provided by the user. To solve this we store the original indices and do not update this field when the security update the indices field --- .../data_stream/140_data_stream_aliases.yml | 8 ++-- .../indices.update_aliases/40_must_exist.yml | 13 +++--- .../indices/alias/IndicesAliasesRequest.java | 20 +++++++++ .../indices/alias/IndicesAliasesResponse.java | 43 ++++++++++++------- .../alias/TransportIndicesAliasesAction.java | 2 +- 5 files changed, 59 insertions(+), 27 deletions(-) diff --git a/modules/data-streams/src/yamlRestTest/resources/rest-api-spec/test/data_stream/140_data_stream_aliases.yml b/modules/data-streams/src/yamlRestTest/resources/rest-api-spec/test/data_stream/140_data_stream_aliases.yml index eb91e26a6f079..0deb3736be9f3 100644 --- a/modules/data-streams/src/yamlRestTest/resources/rest-api-spec/test/data_stream/140_data_stream_aliases.yml +++ b/modules/data-streams/src/yamlRestTest/resources/rest-api-spec/test/data_stream/140_data_stream_aliases.yml @@ -342,9 +342,9 @@ - is_true: errors - length: { action_results: 2 } - match: { action_results.0.success: true } - - match: { action_results.0.action: add } + - match: { action_results.0.action: { 'type': 'add', 'indices': ['log-foobar'], 'aliases': ['test_alias1'] } } - match: { action_results.1.success: false } - - match: { action_results.1.action: remove } + - match: { action_results.1.action: { 'type': 'remove', 'indices': ['log-foobar'], 'aliases': ['test_non_existing'] } } - match: { action_results.1.error: aliases_not_found_exception } --- "Single action result per action": @@ -384,7 +384,7 @@ - is_true: errors - length: { action_results: 2 } - match: { action_results.0.success: true } - - match: { action_results.0.action: add } + - match: { action_results.0.action: { 'type': 'add', 'indices': ['log-test-*'], 'aliases': ['test_alias1'] } } - match: { action_results.1.success: false } - - match: { action_results.1.action: remove } + - match: { action_results.1.action: { 'type': 'remove', 'indices': ['log-test-*'], 'aliases': ['test_non_existing'] } } - match: { action_results.1.error: aliases_not_found_exception } diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.update_aliases/40_must_exist.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.update_aliases/40_must_exist.yml index c3fcb0ead18d7..f630263db67c0 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.update_aliases/40_must_exist.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.update_aliases/40_must_exist.yml @@ -103,9 +103,9 @@ must_exist: false - is_true: errors - match: { action_results.0.success: true } - - match: { action_results.0.action: add } + - match: { action_results.0.action: { 'type': 'add', 'indices': ['test_index'], 'aliases': ['test_alias1'] } } - match: { action_results.1.success: false } - - match: { action_results.1.action: remove } + - match: { action_results.1.action: { 'type': 'remove', 'indices': ['test_index'], 'aliases': ['test_non_existing'] } } - match: { action_results.1.error: aliases_not_found_exception } --- "Partial success with must_exist == null (default)": @@ -127,9 +127,9 @@ aliases: test_non_existing - is_true: errors - match: { action_results.0.success: true } - - match: { action_results.0.action: add } + - match: { action_results.0.action: { 'type': 'add', 'indices': ['test_index'], 'aliases': ['test_alias1'] } } - match: { action_results.1.success: false } - - match: { action_results.1.action: remove } + - match: { action_results.1.action: { 'type': 'remove', 'indices': ['test_index'], 'aliases': ['test_non_existing'] } } - match: { action_results.1.error: aliases_not_found_exception } --- "No action_results field if all actions successful": @@ -172,8 +172,7 @@ - length: { action_results: 2 } - is_true: errors - match: { action_results.0.success: true } - - match: { action_results.0.action: add } + - match: { action_results.0.action: { 'type': 'add', 'indices': ['test_index*'], 'aliases': ['test_alias1'] } } - match: { action_results.1.success: false } - - match: { action_results.1.action: remove } + - match: { action_results.1.action: { 'type': 'remove', 'indices': ['test_index*'], 'aliases': ['test_non_existing'] } } - match: { action_results.1.error: aliases_not_found_exception } - diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java index 628f5220cc22c..96f09fbc408c9 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java @@ -9,6 +9,7 @@ package org.elasticsearch.action.admin.indices.alias; import org.elasticsearch.ElasticsearchGenerationException; +import org.elasticsearch.TransportVersions; import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.AliasesRequest; import org.elasticsearch.action.IndicesRequest; @@ -83,6 +84,7 @@ public static class AliasActions implements AliasesRequest, Writeable, ToXConten private static final ParseField IS_WRITE_INDEX = new ParseField("is_write_index"); private static final ParseField IS_HIDDEN = new ParseField("is_hidden"); private static final ParseField MUST_EXIST = new ParseField("must_exist"); + private static final ParseField ORIGINAL_INDICES = new ParseField("original_indices"); private static final ParseField ADD = new ParseField("add"); private static final ParseField REMOVE = new ParseField("remove"); @@ -217,6 +219,7 @@ private static ObjectParser parser(String name, Supplier { From 824ef6997a77e3f9e81fe38323a98eb97121cda2 Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Tue, 26 Mar 2024 13:32:56 -0500 Subject: [PATCH 12/25] Update docs with action info added in last commmit --- docs/reference/alias.asciidoc | 12 ++++++++++-- docs/reference/indices/aliases.asciidoc | 19 ++++++++++++++++--- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/docs/reference/alias.asciidoc b/docs/reference/alias.asciidoc index 044ef210cc7e8..5dc2d455418f1 100644 --- a/docs/reference/alias.asciidoc +++ b/docs/reference/alias.asciidoc @@ -162,12 +162,20 @@ The API returns the following result: "errors": true, "action_results": [ { - "action": "remove", + "action": { + "type": "remove", + "indices": [ "index1" ], + "aliases": [ "logs-non-existing" ], + }, "success": false, "error": "aliases_not_found_exception" }, { - "action": "add", + "action": { + "type": "add", + "indices": [ "index2" ], + "aliases": [ "logs-non-existing" ], + }, "success": true } ] diff --git a/docs/reference/indices/aliases.asciidoc b/docs/reference/indices/aliases.asciidoc index 46782991fdb89..c5e7a34b27195 100644 --- a/docs/reference/indices/aliases.asciidoc +++ b/docs/reference/indices/aliases.asciidoc @@ -191,15 +191,28 @@ If `true`, the request received a response from the master node within the If `true`, at least one of the requested actions failed. `action_results`:: -(array of objects) Results for each requested action. +(Optional, array of objects) Results for each requested action. + .Properties of `action_results` objects [%collapsible%open] ==== `action`:: -(string) -The type of the associated action, the values will be one of `add`, `remove`, or `remove_index`. +(object) +Description of the associated action request. ++ +.Properties of `action` object +[%collapsible%open] +===== +`type`:: +(string) The type of the associated action, one of `add`, `remove`, or `remove_index`. + +`indices`:: +(array of strings) List of indices in the associated action. + +`aliases`:: +(array of strings) List of aliases in the associated action. +===== `success`:: (boolean) From 26b9896f68583b5b148bc0fe3ab3d2ec9c881bc3 Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Mon, 1 Apr 2024 11:28:04 -0500 Subject: [PATCH 13/25] Change success+error msg to status+error object --- .../indices/alias/IndicesAliasesResponse.java | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java index 572085f805ed0..0ba7d2141dd08 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java @@ -8,12 +8,14 @@ package org.elasticsearch.action.admin.indices.alias; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.TransportVersions; import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest.AliasActions; import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.rest.action.admin.indices.AliasesNotFoundException; import org.elasticsearch.xcontent.ToXContentObject; import org.elasticsearch.xcontent.XContentBuilder; @@ -50,7 +52,7 @@ private IndicesAliasesResponse(boolean acknowledged, boolean errors, final List< } public static IndicesAliasesResponse build(final List actionResults) { - final boolean errors = actionResults.stream().anyMatch(a -> a.success == false); + final boolean errors = actionResults.stream().anyMatch(a -> a.error != null); return new IndicesAliasesResponse(true, errors, actionResults); } @@ -73,10 +75,9 @@ protected void addCustomFields(XContentBuilder builder, Params params) throws IO } public static class AliasActionResult implements Writeable, ToXContentObject { - private static final String ALIAS_MISSING_ERROR = "aliases_not_found_exception"; + private final AliasActions action; - private final boolean success; - private final String error; + private final ElasticsearchException error; public static AliasActionResult build(AliasActions action, int numAliasesRemoved) { if (action.actionType() == AliasActions.Type.REMOVE && numAliasesRemoved == 0) { @@ -86,38 +87,38 @@ public static AliasActionResult build(AliasActions action, int numAliasesRemoved } private static AliasActionResult buildRemoveError(AliasActions action) { - return new AliasActionResult(action, false, ALIAS_MISSING_ERROR); + return new AliasActionResult(action, new AliasesNotFoundException((action.getOriginalAliases()))); } public static AliasActionResult buildSuccess(AliasActions action) { - return new AliasActionResult(action, true, null); + return new AliasActionResult(action, null); + } + + private int getStatus() { + return error == null ? 200 : error.status().getStatus(); } - private AliasActionResult(AliasActions action, boolean success, String error) { - assert error != null ^ success : "AliasActionResult should contain error message if and only if action not successful"; + private AliasActionResult(AliasActions action, ElasticsearchException error) { this.action = action; - this.success = success; this.error = error; } private AliasActionResult(StreamInput in) throws IOException { this.action = new AliasActions(in); - this.success = in.readBoolean(); - this.error = in.readOptionalString(); + this.error = in.readException(); } @Override public void writeTo(StreamOutput out) throws IOException { action.writeTo(out); - out.writeBoolean(success); - out.writeOptionalString(error); + out.writeException(error); } public static final String ACTION_FIELD = "action"; public static final String ACTION_TYPE_FIELD = "type"; public static final String ACTION_INDICES_FIELD = "indices"; public static final String ACTION_ALIASES_FIELD = "aliases"; - public static final String SUCCESS_FIELD = "success"; + public static final String STATUS_FIELD = "status"; public static final String ERROR_FIELD = "error"; @Override @@ -132,9 +133,12 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.array(ACTION_ALIASES_FIELD, action.getOriginalAliases()); builder.endObject(); - builder.field(SUCCESS_FIELD, success); + builder.field(STATUS_FIELD, getStatus()); + if (error != null) { - builder.field(ERROR_FIELD, error); + builder.startObject(ERROR_FIELD); + error.toXContent(builder, params); + builder.endObject(); } builder.endObject(); return builder; From 1d41b67c8798bbd895e669b263b56a4b5fc967a6 Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Mon, 1 Apr 2024 11:41:16 -0500 Subject: [PATCH 14/25] Fix yaml rest tests --- .../data_stream/140_data_stream_aliases.yml | 14 +++++++------ .../indices.update_aliases/40_must_exist.yml | 21 +++++++++++-------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/modules/data-streams/src/yamlRestTest/resources/rest-api-spec/test/data_stream/140_data_stream_aliases.yml b/modules/data-streams/src/yamlRestTest/resources/rest-api-spec/test/data_stream/140_data_stream_aliases.yml index 0deb3736be9f3..e9c4ed172da75 100644 --- a/modules/data-streams/src/yamlRestTest/resources/rest-api-spec/test/data_stream/140_data_stream_aliases.yml +++ b/modules/data-streams/src/yamlRestTest/resources/rest-api-spec/test/data_stream/140_data_stream_aliases.yml @@ -341,11 +341,12 @@ must_exist: false - is_true: errors - length: { action_results: 2 } - - match: { action_results.0.success: true } + - match: { action_results.0.status: 200 } - match: { action_results.0.action: { 'type': 'add', 'indices': ['log-foobar'], 'aliases': ['test_alias1'] } } - - match: { action_results.1.success: false } + - match: { action_results.0.error: null } + - match: { action_results.1.status: 404 } - match: { action_results.1.action: { 'type': 'remove', 'indices': ['log-foobar'], 'aliases': ['test_non_existing'] } } - - match: { action_results.1.error: aliases_not_found_exception } + - match: { action_results.1.error.type: aliases_not_found_exception } --- "Single action result per action": - skip: @@ -383,8 +384,9 @@ must_exist: false - is_true: errors - length: { action_results: 2 } - - match: { action_results.0.success: true } + - match: { action_results.0.status: 200} - match: { action_results.0.action: { 'type': 'add', 'indices': ['log-test-*'], 'aliases': ['test_alias1'] } } - - match: { action_results.1.success: false } + - match: { action_results.0.error: null } + - match: { action_results.1.status: 404 } - match: { action_results.1.action: { 'type': 'remove', 'indices': ['log-test-*'], 'aliases': ['test_non_existing'] } } - - match: { action_results.1.error: aliases_not_found_exception } + - match: { action_results.1.error.type: aliases_not_found_exception } diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.update_aliases/40_must_exist.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.update_aliases/40_must_exist.yml index f630263db67c0..0e9d88255e249 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.update_aliases/40_must_exist.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.update_aliases/40_must_exist.yml @@ -102,11 +102,12 @@ aliases: test_non_existing must_exist: false - is_true: errors - - match: { action_results.0.success: true } + - match: { action_results.0.status: 200 } - match: { action_results.0.action: { 'type': 'add', 'indices': ['test_index'], 'aliases': ['test_alias1'] } } - - match: { action_results.1.success: false } + - match: { action_results.0.error: null } + - match: { action_results.1.status: 404 } - match: { action_results.1.action: { 'type': 'remove', 'indices': ['test_index'], 'aliases': ['test_non_existing'] } } - - match: { action_results.1.error: aliases_not_found_exception } + - match: { action_results.1.error.type: aliases_not_found_exception } --- "Partial success with must_exist == null (default)": - skip: @@ -126,11 +127,12 @@ index: test_index aliases: test_non_existing - is_true: errors - - match: { action_results.0.success: true } + - match: { action_results.0.status: 200} - match: { action_results.0.action: { 'type': 'add', 'indices': ['test_index'], 'aliases': ['test_alias1'] } } - - match: { action_results.1.success: false } + - match: { action_results.0.error: null } + - match: { action_results.1.status: 404} - match: { action_results.1.action: { 'type': 'remove', 'indices': ['test_index'], 'aliases': ['test_non_existing'] } } - - match: { action_results.1.error: aliases_not_found_exception } + - match: { action_results.1.error.type: aliases_not_found_exception } --- "No action_results field if all actions successful": - skip: @@ -171,8 +173,9 @@ aliases: test_non_existing - length: { action_results: 2 } - is_true: errors - - match: { action_results.0.success: true } + - match: { action_results.0.status: 200} - match: { action_results.0.action: { 'type': 'add', 'indices': ['test_index*'], 'aliases': ['test_alias1'] } } - - match: { action_results.1.success: false } + - match: { action_results.0.error: null } + - match: { action_results.1.status: 404} - match: { action_results.1.action: { 'type': 'remove', 'indices': ['test_index*'], 'aliases': ['test_non_existing'] } } - - match: { action_results.1.error: aliases_not_found_exception } + - match: { action_results.1.error.type: aliases_not_found_exception } From b1451cea90befd12d086365975bd31d3386fcf93 Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Mon, 1 Apr 2024 11:56:15 -0500 Subject: [PATCH 15/25] update docs with status + error obj --- docs/reference/alias.asciidoc | 11 ++++++++--- docs/reference/indices/aliases.asciidoc | 10 +++++----- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/docs/reference/alias.asciidoc b/docs/reference/alias.asciidoc index 5dc2d455418f1..5b30501ed7c9d 100644 --- a/docs/reference/alias.asciidoc +++ b/docs/reference/alias.asciidoc @@ -167,8 +167,13 @@ The API returns the following result: "indices": [ "index1" ], "aliases": [ "logs-non-existing" ], }, - "success": false, - "error": "aliases_not_found_exception" + "status": 404, + "error": { + "type": "aliases_not_found_exception", + "reason": "aliases [logs-non-existing] missing", + "resource.type": "aliases", + "resource.id": "logs-non-existing" + } }, { "action": { @@ -176,7 +181,7 @@ The API returns the following result: "indices": [ "index2" ], "aliases": [ "logs-non-existing" ], }, - "success": true + "status": 200 } ] } diff --git a/docs/reference/indices/aliases.asciidoc b/docs/reference/indices/aliases.asciidoc index c5e7a34b27195..34248cc5f98d3 100644 --- a/docs/reference/indices/aliases.asciidoc +++ b/docs/reference/indices/aliases.asciidoc @@ -214,11 +214,11 @@ Description of the associated action request. (array of strings) List of aliases in the associated action. ===== -`success`:: -(boolean) -If true, the requested action was successfully applied to at least one alias. +`status`:: +(integer) HTTP status code returned for the action. `error`:: -(Optional, string) -Only present if `success` is `false`. Consists of a message describing why the action failed +(Optional, object) Contains additional information about the failed action. ++ +Only present if the action failed. ==== From 91472a4e753bdbdb4e8143104e932ea1917459b4 Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Tue, 2 Apr 2024 16:56:58 -0500 Subject: [PATCH 16/25] Revert originalIndices logic to use indices If a request is reused, the originalIndices field will be incorrect. Instead, use indices field. It will contain the indices resolved by the security layer. --- .../indices/alias/IndicesAliasesRequest.java | 20 ------------------- .../indices/alias/IndicesAliasesResponse.java | 2 +- 2 files changed, 1 insertion(+), 21 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java index 96f09fbc408c9..45fb80589978d 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java @@ -84,8 +84,6 @@ public static class AliasActions implements AliasesRequest, Writeable, ToXConten private static final ParseField IS_WRITE_INDEX = new ParseField("is_write_index"); private static final ParseField IS_HIDDEN = new ParseField("is_hidden"); private static final ParseField MUST_EXIST = new ParseField("must_exist"); - private static final ParseField ORIGINAL_INDICES = new ParseField("original_indices"); - private static final ParseField ADD = new ParseField("add"); private static final ParseField REMOVE = new ParseField("remove"); private static final ParseField REMOVE_INDEX = new ParseField("remove_index"); @@ -219,7 +217,6 @@ private static ObjectParser parser(String name, Supplier Date: Tue, 2 Apr 2024 21:38:01 -0500 Subject: [PATCH 17/25] Add resolved index names to alias action result Resolved index names are already provided when security layer is used. This adds resolved indices to alias action result in all cases. --- .../indices.update_aliases/40_must_exist.yml | 4 +-- .../indices/alias/IndicesAliasesRequest.java | 1 - .../indices/alias/IndicesAliasesResponse.java | 27 +++++++++++-------- .../alias/TransportIndicesAliasesAction.java | 11 +++++--- 4 files changed, 26 insertions(+), 17 deletions(-) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.update_aliases/40_must_exist.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.update_aliases/40_must_exist.yml index 0e9d88255e249..fa3c740612872 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.update_aliases/40_must_exist.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.update_aliases/40_must_exist.yml @@ -174,8 +174,8 @@ - length: { action_results: 2 } - is_true: errors - match: { action_results.0.status: 200} - - match: { action_results.0.action: { 'type': 'add', 'indices': ['test_index*'], 'aliases': ['test_alias1'] } } + - match: { action_results.0.action: { 'type': 'add', 'indices': ['test_index1', 'test_index2'], 'aliases': ['test_alias1'] } } - match: { action_results.0.error: null } - match: { action_results.1.status: 404} - - match: { action_results.1.action: { 'type': 'remove', 'indices': ['test_index*'], 'aliases': ['test_non_existing'] } } + - match: { action_results.1.action: { 'type': 'remove', 'indices': ['test_index1', 'test_index2'], 'aliases': ['test_non_existing'] } } - match: { action_results.1.error.type: aliases_not_found_exception } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java index 45fb80589978d..fac2006b68814 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java @@ -9,7 +9,6 @@ package org.elasticsearch.action.admin.indices.alias; import org.elasticsearch.ElasticsearchGenerationException; -import org.elasticsearch.TransportVersions; import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.AliasesRequest; import org.elasticsearch.action.IndicesRequest; diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java index d420c47f28b4e..bc8dd1e66434f 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.util.List; +import java.util.stream.Collectors; public class IndicesAliasesResponse extends AcknowledgedResponse { @@ -45,7 +46,7 @@ protected IndicesAliasesResponse(StreamInput in) throws IOException { } } - private IndicesAliasesResponse(boolean acknowledged, boolean errors, final List actionResults) { + public IndicesAliasesResponse(boolean acknowledged, boolean errors, final List actionResults) { super(acknowledged); this.errors = errors; this.actionResults = actionResults; @@ -75,41 +76,45 @@ protected void addCustomFields(XContentBuilder builder, Params params) throws IO } public static class AliasActionResult implements Writeable, ToXContentObject { - + private final List indices; private final AliasActions action; private final ElasticsearchException error; - public static AliasActionResult build(AliasActions action, int numAliasesRemoved) { + public static AliasActionResult build(List indices, AliasActions action, int numAliasesRemoved) { if (action.actionType() == AliasActions.Type.REMOVE && numAliasesRemoved == 0) { - return buildRemoveError(action); + return buildRemoveError(indices, action); } - return buildSuccess(action); + return buildSuccess(indices, action); } - private static AliasActionResult buildRemoveError(AliasActions action) { - return new AliasActionResult(action, new AliasesNotFoundException((action.getOriginalAliases()))); + private static AliasActionResult buildRemoveError(List indices, AliasActions action) { + return new AliasActionResult(indices, action, new AliasesNotFoundException((action.getOriginalAliases()))); } - public static AliasActionResult buildSuccess(AliasActions action) { - return new AliasActionResult(action, null); + public static AliasActionResult buildSuccess(List indices, AliasActions action) { + return new AliasActionResult(indices, action, null); } private int getStatus() { return error == null ? 200 : error.status().getStatus(); } - private AliasActionResult(AliasActions action, ElasticsearchException error) { + private AliasActionResult(List indices, AliasActions action, ElasticsearchException error) { + assert indices.isEmpty() == false : "Alias action result must be instantiated with at least one index"; + this.indices = indices; this.action = action; this.error = error; } private AliasActionResult(StreamInput in) throws IOException { + this.indices = in.readStringCollectionAsList(); this.action = new AliasActions(in); this.error = in.readException(); } @Override public void writeTo(StreamOutput out) throws IOException { + out.writeStringCollection(indices); action.writeTo(out); out.writeException(error); } @@ -129,7 +134,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.field(ACTION_FIELD); builder.startObject(); builder.field(ACTION_TYPE_FIELD, action.actionType().getFieldName()); - builder.array(ACTION_INDICES_FIELD, action.indices()); + builder.field(ACTION_INDICES_FIELD, indices.stream().sorted().collect(Collectors.toList())); builder.array(ACTION_ALIASES_FIELD, action.getOriginalAliases()); builder.endObject(); diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/TransportIndicesAliasesAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/TransportIndicesAliasesAction.java index 818e8a7c1579a..2e231b398af72 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/TransportIndicesAliasesAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/TransportIndicesAliasesAction.java @@ -118,6 +118,7 @@ protected void masterOperation( Set aliases = new HashSet<>(); for (AliasActions action : actions) { int numAliasesRemoved = 0; + List resolvedIndices = new ArrayList<>(); List concreteDataStreams = indexNameExpressionResolver.dataStreamNames( state, @@ -165,7 +166,8 @@ protected void masterOperation( finalActions.add(new AddDataStreamAlias(alias, dataStreamName, action.writeIndex(), action.filter())); } } - actionResults.add(AliasActionResult.buildSuccess(action)); + + actionResults.add(AliasActionResult.buildSuccess(concreteDataStreams, action)); continue; } case REMOVE -> { @@ -175,11 +177,13 @@ protected void masterOperation( numAliasesRemoved++; } } + if (nonBackingIndices.isEmpty() == false) { // Regular aliases/indices match as well with the provided expression. // (Only when adding new aliases, matching both data streams and indices is disallowed) + resolvedIndices.addAll(concreteDataStreams); } else { - actionResults.add(AliasActionResult.build(action, numAliasesRemoved)); + actionResults.add(AliasActionResult.build(concreteDataStreams, action, numAliasesRemoved)); continue; } } @@ -242,7 +246,8 @@ protected void masterOperation( } } - actionResults.add(AliasActionResult.build(action, numAliasesRemoved)); + Arrays.stream(concreteIndices).map(Index::getName).forEach(resolvedIndices::add); + actionResults.add(AliasActionResult.build(resolvedIndices, action, numAliasesRemoved)); } if (finalActions.isEmpty() && false == actions.isEmpty()) { throw new AliasesNotFoundException(aliases.toArray(new String[aliases.size()])); From 481ee9d8cd64812b96a7fff5d7d8f64ce4161a01 Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Wed, 3 Apr 2024 11:52:41 -0500 Subject: [PATCH 18/25] Fix tests to expect index name resolution --- .../test/data_stream/140_data_stream_aliases.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/data-streams/src/yamlRestTest/resources/rest-api-spec/test/data_stream/140_data_stream_aliases.yml b/modules/data-streams/src/yamlRestTest/resources/rest-api-spec/test/data_stream/140_data_stream_aliases.yml index e9c4ed172da75..1050d6e01a95f 100644 --- a/modules/data-streams/src/yamlRestTest/resources/rest-api-spec/test/data_stream/140_data_stream_aliases.yml +++ b/modules/data-streams/src/yamlRestTest/resources/rest-api-spec/test/data_stream/140_data_stream_aliases.yml @@ -385,8 +385,8 @@ - is_true: errors - length: { action_results: 2 } - match: { action_results.0.status: 200} - - match: { action_results.0.action: { 'type': 'add', 'indices': ['log-test-*'], 'aliases': ['test_alias1'] } } + - match: { action_results.0.action: { 'type': 'add', 'indices': ['log-test-1', 'log-test-2'], 'aliases': ['test_alias1'] } } - match: { action_results.0.error: null } - match: { action_results.1.status: 404 } - - match: { action_results.1.action: { 'type': 'remove', 'indices': ['log-test-*'], 'aliases': ['test_non_existing'] } } + - match: { action_results.1.action: { 'type': 'remove', 'indices': ['log-test-1', 'log-test-2'], 'aliases': ['test_non_existing'] } } - match: { action_results.1.error.type: aliases_not_found_exception } From eba3361667e459a0706eef5a2820fe29db8a4eff Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Wed, 3 Apr 2024 13:58:26 -0500 Subject: [PATCH 19/25] Add serialization tests for IndicesAliasesResponse --- .../indices/alias/IndicesAliasesResponse.java | 49 +++++++++++++- .../alias/IndicesAliasesResponseTests.java | 64 +++++++++++++++++++ 2 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 server/src/test/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponseTests.java diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java index bc8dd1e66434f..cc76f9f943096 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.util.List; +import java.util.Objects; import java.util.stream.Collectors; public class IndicesAliasesResponse extends AcknowledgedResponse { @@ -39,7 +40,7 @@ protected IndicesAliasesResponse(StreamInput in) throws IOException { if (in.getTransportVersion().onOrAfter(TransportVersions.ALIAS_ACTION_RESULTS)) { this.errors = in.readBoolean(); - this.actionResults = in.readCollectionAsList(AliasActionResult::new); + this.actionResults = in.readCollectionAsImmutableList(AliasActionResult::new); } else { this.errors = false; this.actionResults = List.of(); @@ -52,6 +53,14 @@ public IndicesAliasesResponse(boolean acknowledged, boolean errors, final List getActionResults() { + return actionResults; + } + + public boolean hasErrors() { + return errors; + } + public static IndicesAliasesResponse build(final List actionResults) { final boolean errors = actionResults.stream().anyMatch(a -> a.error != null); return new IndicesAliasesResponse(true, errors, actionResults); @@ -75,6 +84,22 @@ protected void addCustomFields(XContentBuilder builder, Params params) throws IO } } + @Override + // Only used equals in tests + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (super.equals(o) == false) return false; + IndicesAliasesResponse response = (IndicesAliasesResponse) o; + return errors == response.errors && Objects.equals(actionResults, response.actionResults); + } + + @Override + // Only used hashCode in tests + public int hashCode() { + return Objects.hash(super.hashCode(), actionResults, errors); + } + public static class AliasActionResult implements Writeable, ToXContentObject { private final List indices; private final AliasActions action; @@ -148,5 +173,27 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.endObject(); return builder; } + + @Override + // Only used equals in tests + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + AliasActionResult that = (AliasActionResult) o; + return Objects.equals(indices, that.indices) && Objects.equals(action, that.action) + // ElasticsearchException does not have equals() so assume errors are equal iff messages are equal + && Objects.equals(error == null ? null : error.getMessage(), that.error == null ? null : that.error.getMessage()); + } + + @Override + // Only used hashCode in tests + public int hashCode() { + return Objects.hash( + indices, + action, + // ElasticsearchException does not have hashCode() so assume errors are equal iff messages are equal + error == null ? null : error.getMessage() + ); + } } } diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponseTests.java new file mode 100644 index 0000000000000..3f3e15c419cce --- /dev/null +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponseTests.java @@ -0,0 +1,64 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.action.admin.indices.alias; + +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.index.alias.RandomAliasActionsGenerator; +import org.elasticsearch.test.AbstractWireSerializingTestCase; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +public class IndicesAliasesResponseTests extends AbstractWireSerializingTestCase { + @Override + protected Writeable.Reader instanceReader() { + return IndicesAliasesResponse::new; + } + + @Override + protected IndicesAliasesResponse createTestInstance() { + int numActions = between(0, 5); + List results = new ArrayList<>(); + for (int i = 0; i < numActions; ++i) { + results.add(randomIndicesAliasesResult()); + } + return new IndicesAliasesResponse(randomBoolean(), randomBoolean(), results); + } + + @Override + protected IndicesAliasesResponse mutateInstance(IndicesAliasesResponse instance) throws IOException { + switch (between(0, 2)) { + case 0: { + boolean acknowledged = instance.isAcknowledged() == false; + return new IndicesAliasesResponse(acknowledged, instance.hasErrors(), instance.getActionResults()); + } + case 1: { + boolean errors = instance.hasErrors() == false; + return new IndicesAliasesResponse(instance.isAcknowledged(), errors, instance.getActionResults()); + } + default: { + var results = new ArrayList<>(instance.getActionResults()); + if (results.isEmpty()) { + results.add(randomIndicesAliasesResult()); + } else { + results.remove(between(0, results.size() - 1)); + } + return new IndicesAliasesResponse(instance.isAcknowledged(), instance.hasErrors(), results); + } + } + } + + private static IndicesAliasesResponse.AliasActionResult randomIndicesAliasesResult() { + var action = RandomAliasActionsGenerator.randomAliasAction(); + var indices = Arrays.asList(generateRandomStringArray(10, 5, false, false)); + return IndicesAliasesResponse.AliasActionResult.build(indices, action, randomIntBetween(0, 3)); + } +} From 02e225f4fe3f77d3749ce18180a5a39fc6084fe2 Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Wed, 3 Apr 2024 17:31:47 -0500 Subject: [PATCH 20/25] Test serialization Ack <-> IndicesAliasesResponse --- .../alias/IndicesAliasesResponseTests.java | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponseTests.java index 3f3e15c419cce..75a1bf8732a4f 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponseTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponseTests.java @@ -8,6 +8,10 @@ package org.elasticsearch.action.admin.indices.alias; +import org.elasticsearch.TransportVersions; +import org.elasticsearch.action.support.master.AcknowledgedResponse; +import org.elasticsearch.common.io.stream.BytesStreamOutput; +import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.index.alias.RandomAliasActionsGenerator; import org.elasticsearch.test.AbstractWireSerializingTestCase; @@ -18,6 +22,42 @@ import java.util.List; public class IndicesAliasesResponseTests extends AbstractWireSerializingTestCase { + public void testMixedModeSerialization() throws IOException { + + // AcknowledgedResponse to IndicesAliasesResponse + // in version before TransportVersions.ALIAS_ACTION_RESULTS + { + var ack = AcknowledgedResponse.of(randomBoolean()); + try (BytesStreamOutput output = new BytesStreamOutput()) { + ack.writeTo(output); + try (StreamInput in = output.bytes().streamInput()) { + in.setTransportVersion(TransportVersions.V_8_12_0); + + var indicesAliasesResponse = new IndicesAliasesResponse(in); + + assertEquals(ack.isAcknowledged(), indicesAliasesResponse.isAcknowledged()); + assertTrue(indicesAliasesResponse.getActionResults().isEmpty()); + assertFalse(indicesAliasesResponse.hasErrors()); + } + } + } + + // IndicesAliasesResponse to AcknowledgedResponse + // out version before TransportVersions.ALIAS_ACTION_RESULTS + { + var indicesAliasesResponse = randomIndicesAliasesResponse(); + try (BytesStreamOutput output = new BytesStreamOutput()) { + output.setTransportVersion(TransportVersions.V_8_12_0); + + indicesAliasesResponse.writeTo(output); + try (StreamInput in = output.bytes().streamInput()) { + var ack = AcknowledgedResponse.readFrom(in); + assertEquals(ack.isAcknowledged(), indicesAliasesResponse.isAcknowledged()); + } + } + } + } + @Override protected Writeable.Reader instanceReader() { return IndicesAliasesResponse::new; @@ -25,6 +65,10 @@ protected Writeable.Reader instanceReader() { @Override protected IndicesAliasesResponse createTestInstance() { + return randomIndicesAliasesResponse(); + } + + private static IndicesAliasesResponse randomIndicesAliasesResponse() { int numActions = between(0, 5); List results = new ArrayList<>(); for (int i = 0; i < numActions; ++i) { From fea757b8d21a97fb45e9c8608b148e0bced1580b Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Wed, 3 Apr 2024 17:36:51 -0500 Subject: [PATCH 21/25] add javadoc --- .../indices/alias/IndicesAliasesResponse.java | 45 ++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java index cc76f9f943096..da33a668f1167 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java @@ -24,9 +24,19 @@ import java.util.Objects; import java.util.stream.Collectors; +/** + * Response with error information for a request to add/remove aliases for one or more indices. + * Contains an acknowledged boolean, an errors boolean, and a list of results. + * The result list is only present if there are errors, and contains a result for every input action. + * This response replaces AcknowledgedResponse, and knows how to de/serialize from/to AcknowledgedResponse + * in case of mixed version clusters. + */ public class IndicesAliasesResponse extends AcknowledgedResponse { + // Response without any error information, analogous to AcknowledgedResponse.FALSE public static final IndicesAliasesResponse NOT_ACKNOWLEDGED = new IndicesAliasesResponse(false, false, List.of()); + + // Response without any error information, analogous to AcknowledgedResponse.TRUE public static final IndicesAliasesResponse ACKNOWLEDGED_NO_ERRORS = new IndicesAliasesResponse(true, false, List.of()); private static final String ACTION_RESULTS_FIELD = "action_results"; @@ -47,7 +57,12 @@ protected IndicesAliasesResponse(StreamInput in) throws IOException { } } - public IndicesAliasesResponse(boolean acknowledged, boolean errors, final List actionResults) { + /** + * @param acknowledged whether the update was acknowledged by all the relevant nodes in the cluster + * @param errors true if any of the requested actions failed + * @param actionResults the list of results for each input action, only present if there are errors + */ + IndicesAliasesResponse(boolean acknowledged, boolean errors, final List actionResults) { super(acknowledged); this.errors = errors; this.actionResults = actionResults; @@ -61,6 +76,12 @@ public boolean hasErrors() { return errors; } + /** + * Build a response from a list of action results. Sets the errors boolean based + * on whether an of the individual results contain an error. + * @param actionResults an action result for each of the requested alias actions + * @return response containing all action results + */ public static IndicesAliasesResponse build(final List actionResults) { final boolean errors = actionResults.stream().anyMatch(a -> a.error != null); return new IndicesAliasesResponse(true, errors, actionResults); @@ -100,11 +121,27 @@ public int hashCode() { return Objects.hash(super.hashCode(), actionResults, errors); } + /** + * Result for a single alias add/remove action + */ public static class AliasActionResult implements Writeable, ToXContentObject { + + /** + * Resolved indices to which the action applies. This duplicates information + * which exists in the action, but is included because the action indices may + * or may not be resolved depending on if the security layer is used or not. + */ private final List indices; private final AliasActions action; private final ElasticsearchException error; + /** + * Build result that could be either a success or failure + * @param indices the resolved indices to which the associated action applies + * @param action the alias action consisting of add/remove, aliases, and indices + * @param numAliasesRemoved the number of aliases remove, if any + * @return the action result + */ public static AliasActionResult build(List indices, AliasActions action, int numAliasesRemoved) { if (action.actionType() == AliasActions.Type.REMOVE && numAliasesRemoved == 0) { return buildRemoveError(indices, action); @@ -112,10 +149,16 @@ public static AliasActionResult build(List indices, AliasActions action, return buildSuccess(indices, action); } + /** + * Build an error result for a failed remove action. + */ private static AliasActionResult buildRemoveError(List indices, AliasActions action) { return new AliasActionResult(indices, action, new AliasesNotFoundException((action.getOriginalAliases()))); } + /** + * Build a success action result with no errors. + */ public static AliasActionResult buildSuccess(List indices, AliasActions action) { return new AliasActionResult(indices, action, null); } From 0dd10ba48072e27c9f0172a1bb8c83ce9e2c5c05 Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Sat, 6 Apr 2024 20:51:24 -0500 Subject: [PATCH 22/25] Asserts on IndiceAliasesResponse construction IndicesAliasesResponse could be instantiated with an empty error list and set the errors field to true. Add some validation to make sure is not instantiated this way. --- .../alias/IndicesAliasesClusterStateUpdateRequest.java | 1 + .../action/admin/indices/alias/IndicesAliasesResponse.java | 1 + .../cluster/metadata/MetadataIndexAliasesServiceTests.java | 6 ++++-- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesClusterStateUpdateRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesClusterStateUpdateRequest.java index 1f87cf618dfcf..c3a0416784b02 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesClusterStateUpdateRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesClusterStateUpdateRequest.java @@ -22,6 +22,7 @@ public class IndicesAliasesClusterStateUpdateRequest extends ClusterStateUpdateR private final List actionResults; public IndicesAliasesClusterStateUpdateRequest(List actions, List actionResults) { + assert actions.size() == actionResults.size() : "There must be a result for every alias action"; this.actions = actions; this.actionResults = actionResults; } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java index da33a668f1167..114cd5facbc86 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java @@ -83,6 +83,7 @@ public boolean hasErrors() { * @return response containing all action results */ public static IndicesAliasesResponse build(final List actionResults) { + assert actionResults.isEmpty() == false : "IndicesAliasesResponse must be instantiated with at least one action result."; final boolean errors = actionResults.stream().anyMatch(a -> a.error != null); return new IndicesAliasesResponse(true, errors, actionResults); } diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesServiceTests.java index f5665d2538cbf..3f63875bfc216 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetadataIndexAliasesServiceTests.java @@ -9,6 +9,8 @@ package org.elasticsearch.cluster.metadata; import org.elasticsearch.action.admin.indices.alias.IndicesAliasesClusterStateUpdateRequest; +import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest.AliasActions; +import org.elasticsearch.action.admin.indices.alias.IndicesAliasesResponse.AliasActionResult; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.service.ClusterService; @@ -691,11 +693,11 @@ public void testAddAndRemoveAliasClusterStateUpdate() throws Exception { ClusterState before = createIndex(ClusterState.builder(ClusterName.DEFAULT).build(), index); IndicesAliasesClusterStateUpdateRequest addAliasRequest = new IndicesAliasesClusterStateUpdateRequest( List.of(new AliasAction.Add(index, "test", null, null, null, null, null)), - List.of() + List.of(AliasActionResult.buildSuccess(List.of(index), AliasActions.add().aliases("test").indices(index))) ); IndicesAliasesClusterStateUpdateRequest removeAliasRequest = new IndicesAliasesClusterStateUpdateRequest( List.of(new AliasAction.Remove(index, "test", true)), - List.of() + List.of(AliasActionResult.buildSuccess(List.of(index), AliasActions.remove().aliases("test").indices(index))) ); ClusterState after = ClusterStateTaskExecutorUtils.executeAndAssertSuccessful( From a9a1931f0c68bb994a114fa0253ef2b4aea4e354 Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Sat, 6 Apr 2024 21:02:59 -0500 Subject: [PATCH 23/25] Add exception class to equality/hashcode Rather than checking exception equality exactly, just make sure exception class and message are equal --- .../admin/indices/alias/IndicesAliasesResponse.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java index 114cd5facbc86..7a64abeeabde8 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java @@ -225,8 +225,9 @@ public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) return false; AliasActionResult that = (AliasActionResult) o; return Objects.equals(indices, that.indices) && Objects.equals(action, that.action) - // ElasticsearchException does not have equals() so assume errors are equal iff messages are equal - && Objects.equals(error == null ? null : error.getMessage(), that.error == null ? null : that.error.getMessage()); + // ElasticsearchException does not have hashCode() so assume errors are equal iff class and message are equal + && Objects.equals(error == null ? null : error.getMessage(), that.error == null ? null : that.error.getMessage()) + && Objects.equals(error == null ? null : error.getClass(), that.error == null ? null : that.error.getClass()); } @Override @@ -235,8 +236,9 @@ public int hashCode() { return Objects.hash( indices, action, - // ElasticsearchException does not have hashCode() so assume errors are equal iff messages are equal - error == null ? null : error.getMessage() + // ElasticsearchException does not have hashCode() so assume errors are equal iff class and message are equal + error == null ? null : error.getMessage(), + error == null ? null : error.getClass() ); } } From 386078936d0162c5d9511590bfa16499976d1abb Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Sat, 6 Apr 2024 21:07:27 -0500 Subject: [PATCH 24/25] spotlessApply --- .../action/admin/indices/alias/IndicesAliasesResponse.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java index 7a64abeeabde8..b4f483e6f8161 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java @@ -226,8 +226,8 @@ public boolean equals(Object o) { AliasActionResult that = (AliasActionResult) o; return Objects.equals(indices, that.indices) && Objects.equals(action, that.action) // ElasticsearchException does not have hashCode() so assume errors are equal iff class and message are equal - && Objects.equals(error == null ? null : error.getMessage(), that.error == null ? null : that.error.getMessage()) - && Objects.equals(error == null ? null : error.getClass(), that.error == null ? null : that.error.getClass()); + && Objects.equals(error == null ? null : error.getMessage(), that.error == null ? null : that.error.getMessage()) + && Objects.equals(error == null ? null : error.getClass(), that.error == null ? null : that.error.getClass()); } @Override From 6a86c40602f566f373773091ef229f50c9164bd5 Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Sat, 6 Apr 2024 22:33:48 -0500 Subject: [PATCH 25/25] Remove incorrect assert There should be one AliasActionResult for every AliasActions, not for every AliasAction. The former is the requested action on the unresolved alias pattern, which may map to multiple of the latter each of which is for a single specific alias --- .../indices/alias/IndicesAliasesClusterStateUpdateRequest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesClusterStateUpdateRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesClusterStateUpdateRequest.java index c3a0416784b02..1f87cf618dfcf 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesClusterStateUpdateRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesClusterStateUpdateRequest.java @@ -22,7 +22,6 @@ public class IndicesAliasesClusterStateUpdateRequest extends ClusterStateUpdateR private final List actionResults; public IndicesAliasesClusterStateUpdateRequest(List actions, List actionResults) { - assert actions.size() == actionResults.size() : "There must be a result for every alias action"; this.actions = actions; this.actionResults = actionResults; }