diff --git a/docs/changelog/125352.yaml b/docs/changelog/125352.yaml new file mode 100644 index 0000000000000..0895732b0b5bf --- /dev/null +++ b/docs/changelog/125352.yaml @@ -0,0 +1,5 @@ +pr: 125352 +summary: Fix NPE in rolling over unknown target and return 404 +area: Indices APIs +type: bug +issues: [] diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.rollover/10_basic.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.rollover/10_basic.yml index a53365721cf0c..222035174b8bb 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.rollover/10_basic.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.rollover/10_basic.yml @@ -183,3 +183,19 @@ min_age: "0s" min_docs: 1 - match: { error.reason: "Validation Failed: 1: at least one max_* rollover condition must be set when using min_* conditions;" } + +--- +"Rolling over an unknown target should return 404": + - requires: + capabilities: + - method: POST + path: /{index}/_rollover + capabilities: ['return-404-on-missing-target'] + test_runner_features: [capabilities] + reason: Rollover used to return a 400, then it briefly returned a 500 due to an NPE, now it properly returns a 404 + + - do: + catch: missing + indices.rollover: + alias: "non_existent" + - match: {error.reason: "rollover target [non_existent] does not exist"} diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverService.java b/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverService.java index 63a5a792db3c9..eecb0ff354ba5 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverService.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverService.java @@ -11,6 +11,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.elasticsearch.ResourceNotFoundException; import org.elasticsearch.action.admin.indices.create.CreateIndexClusterStateUpdateRequest; import org.elasticsearch.action.admin.indices.create.CreateIndexRequest; import org.elasticsearch.action.datastreams.autosharding.AutoShardingResult; @@ -148,7 +149,7 @@ public RolloverResult rolloverClusterState( @Nullable AutoShardingResult autoShardingResult, boolean isFailureStoreRollover ) throws Exception { - validate(currentState.metadata(), rolloverTarget, newIndexName, createIndexRequest, isFailureStoreRollover); + validate(currentState.metadata(), rolloverTarget, newIndexName, createIndexRequest); final IndexAbstraction indexAbstraction = currentState.metadata().getIndicesLookup().get(rolloverTarget); return switch (indexAbstraction.getType()) { case ALIAS -> rolloverAlias( @@ -190,7 +191,7 @@ public static NameResolution resolveRolloverNames( CreateIndexRequest createIndexRequest, boolean isFailureStoreRollover ) { - validate(currentState.metadata(), rolloverTarget, newIndexName, createIndexRequest, isFailureStoreRollover); + validate(currentState.metadata(), rolloverTarget, newIndexName, createIndexRequest); final IndexAbstraction indexAbstraction = currentState.metadata().getIndicesLookup().get(rolloverTarget); return switch (indexAbstraction.getType()) { case ALIAS -> resolveAliasRolloverNames(currentState.metadata(), indexAbstraction, newIndexName); @@ -627,16 +628,10 @@ static void checkNoDuplicatedAliasInIndexTemplate( } } - static void validate( - Metadata metadata, - String rolloverTarget, - String newIndexName, - CreateIndexRequest request, - boolean isFailureStoreRollover - ) { + static void validate(Metadata metadata, String rolloverTarget, String newIndexName, CreateIndexRequest request) { final IndexAbstraction indexAbstraction = metadata.getIndicesLookup().get(rolloverTarget); if (indexAbstraction == null) { - throw new IllegalArgumentException("rollover target [" + rolloverTarget + "] does not exist"); + throw new ResourceNotFoundException("rollover target [" + rolloverTarget + "] does not exist"); } if (VALID_ROLLOVER_TARGETS.contains(indexAbstraction.getType()) == false) { throw new IllegalArgumentException( diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java index e7a3752ca4b32..59feb9b7e0bbb 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/TransportRolloverAction.java @@ -156,7 +156,7 @@ protected ClusterBlockException checkBlock(RolloverRequest request, ClusterState ResolvedExpression resolvedRolloverTarget = SelectorResolver.parseExpression(request.getRolloverTarget(), request.indicesOptions()); final IndexAbstraction indexAbstraction = state.metadata().getIndicesLookup().get(resolvedRolloverTarget.resource()); final String[] indicesToCheck; - if (indexAbstraction.getType().equals(IndexAbstraction.Type.DATA_STREAM)) { + if (indexAbstraction != null && indexAbstraction.getType().equals(IndexAbstraction.Type.DATA_STREAM)) { DataStream dataStream = (DataStream) indexAbstraction; boolean targetFailureStore = resolvedRolloverTarget.selector() != null && resolvedRolloverTarget.selector().shouldIncludeFailures(); diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestRolloverIndexAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestRolloverIndexAction.java index 18f7cc1222d5f..83abc0555b0b8 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestRolloverIndexAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestRolloverIndexAction.java @@ -50,9 +50,9 @@ public String getName() { @Override public Set supportedCapabilities() { if (DataStream.isFailureStoreFeatureFlagEnabled()) { - return Set.of("lazy-rollover-failure-store", "index-expression-selectors"); + return Set.of("return-404-on-missing-target", "lazy-rollover-failure-store", "index-expression-selectors"); } else { - return Set.of(); + return Set.of("return-404-on-missing-target"); } } diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverServiceTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverServiceTests.java index 57750bb02bb14..729b1734fea22 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverServiceTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverServiceTests.java @@ -9,6 +9,7 @@ package org.elasticsearch.action.admin.indices.rollover; +import org.elasticsearch.ResourceNotFoundException; import org.elasticsearch.action.admin.indices.alias.Alias; import org.elasticsearch.action.admin.indices.create.CreateIndexClusterStateUpdateRequest; import org.elasticsearch.action.admin.indices.create.CreateIndexRequest; @@ -203,23 +204,23 @@ public void testAliasValidation() { Metadata metadata = metadataBuilder.build(); CreateIndexRequest req = new CreateIndexRequest(); - IllegalArgumentException exception = expectThrows( + Exception exception = expectThrows( IllegalArgumentException.class, - () -> MetadataRolloverService.validate(metadata, aliasWithNoWriteIndex, randomAlphaOfLength(5), req, false) + () -> MetadataRolloverService.validate(metadata, aliasWithNoWriteIndex, randomAlphaOfLength(5), req) ); assertThat(exception.getMessage(), equalTo("rollover target [" + aliasWithNoWriteIndex + "] does not point to a write index")); exception = expectThrows( IllegalArgumentException.class, - () -> MetadataRolloverService.validate(metadata, randomFrom(index1, index2), randomAlphaOfLength(5), req, false) + () -> MetadataRolloverService.validate(metadata, randomFrom(index1, index2), randomAlphaOfLength(5), req) ); assertThat(exception.getMessage(), equalTo("rollover target is a [concrete index] but one of [alias,data_stream] was expected")); final String aliasName = randomAlphaOfLength(5); exception = expectThrows( - IllegalArgumentException.class, - () -> MetadataRolloverService.validate(metadata, aliasName, randomAlphaOfLength(5), req, false) + ResourceNotFoundException.class, + () -> MetadataRolloverService.validate(metadata, aliasName, randomAlphaOfLength(5), req) ); assertThat(exception.getMessage(), equalTo("rollover target [" + aliasName + "] does not exist")); - MetadataRolloverService.validate(metadata, aliasWithWriteIndex, randomAlphaOfLength(5), req, false); + MetadataRolloverService.validate(metadata, aliasWithWriteIndex, randomAlphaOfLength(5), req); } public void testDataStreamValidation() throws IOException { @@ -232,18 +233,18 @@ public void testDataStreamValidation() throws IOException { Metadata metadata = md.build(); CreateIndexRequest req = new CreateIndexRequest(); - MetadataRolloverService.validate(metadata, randomDataStream.getName(), null, req, false); + MetadataRolloverService.validate(metadata, randomDataStream.getName(), null, req); IllegalArgumentException exception = expectThrows( IllegalArgumentException.class, - () -> MetadataRolloverService.validate(metadata, randomDataStream.getName(), randomAlphaOfLength(5), req, false) + () -> MetadataRolloverService.validate(metadata, randomDataStream.getName(), randomAlphaOfLength(5), req) ); assertThat(exception.getMessage(), equalTo("new index name may not be specified when rolling over a data stream")); CreateIndexRequest aliasReq = new CreateIndexRequest().alias(new Alias("no_aliases_permitted")); exception = expectThrows( IllegalArgumentException.class, - () -> MetadataRolloverService.validate(metadata, randomDataStream.getName(), null, aliasReq, false) + () -> MetadataRolloverService.validate(metadata, randomDataStream.getName(), null, aliasReq) ); assertThat( exception.getMessage(), @@ -254,7 +255,7 @@ public void testDataStreamValidation() throws IOException { CreateIndexRequest mappingReq = new CreateIndexRequest().mapping(mapping); exception = expectThrows( IllegalArgumentException.class, - () -> MetadataRolloverService.validate(metadata, randomDataStream.getName(), null, mappingReq, false) + () -> MetadataRolloverService.validate(metadata, randomDataStream.getName(), null, mappingReq) ); assertThat( exception.getMessage(), @@ -264,7 +265,7 @@ public void testDataStreamValidation() throws IOException { CreateIndexRequest settingReq = new CreateIndexRequest().settings(Settings.builder().put("foo", "bar")); exception = expectThrows( IllegalArgumentException.class, - () -> MetadataRolloverService.validate(metadata, randomDataStream.getName(), null, settingReq, false) + () -> MetadataRolloverService.validate(metadata, randomDataStream.getName(), null, settingReq) ); assertThat( exception.getMessage(),