Fix NPE in rolling over unknown target and return 404 by nielsbauman · Pull Request #125352 · elastic/elasticsearch

nielsbauman · 2025-03-20T21:22:12Z

Since #122905 we were throwing NPEs (i.e. 5xxs) when a rollover request has an unknown/non-existent target. Before that, we returned a 400 - illegal argument exception. We now return a 404 which matches "missing target" better. Additionally, to avoid this from happening again, we add a YAML test that asserts the correct exception behavior.

Since elastic#122905 we were throwing NPEs (i.e. 5xxs) when a rollover request has an unknown/non-existent target. This PR restores that to it's original behavior (which is to return a 400 - illegal argument exception). Additionally, to avoid this from happening again, we add a YAML test that asserts the correct exception behavior.

elasticsearchmachine · 2025-03-20T21:22:35Z

Pinging @elastic/es-data-management (Team:Data Management)

rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.rollover/10_basic.yml

dakrone

This LGTM, but I think instead of a >non-issue, we should mark it as >bug and add in the changelog that this fixes an NPE and makes the response a 404.

nielsbauman · 2025-03-21T21:38:23Z

@dakrone I thought about that but figured this wasn't relevant enough for a changelog item. I guess it makes sense to have one (and doesn't hurt). Will update, thanks 👍

masseyke · 2025-03-21T21:47:58Z

Here's the stack trace in case anyone comes across this problem:

java.lang.NullPointerException: Cannot invoke \"org.elasticsearch.cluster.metadata.IndexAbstraction.getType()\" because \"indexAbstraction\" is null
	at org.elasticsearch.server@9.1.0-SNAPSHOT/org.elasticsearch.action.admin.indices.rollover.TransportRolloverAction.checkBlock(TransportRolloverAction.java:167)
	at org.elasticsearch.server@9.1.0-SNAPSHOT/org.elasticsearch.action.admin.indices.rollover.TransportRolloverAction.checkBlock(TransportRolloverAction.java:76)
	at org.elasticsearch.server@9.1.0-SNAPSHOT/org.elasticsearch.action.support.master.TransportMasterNodeAction.checkBlockIfStateRecovered(TransportMasterNodeAction.java:123)
	at org.elasticsearch.server@9.1.0-SNAPSHOT/org.elasticsearch.action.support.master.TransportMasterNodeAction$AsyncSingleAction.doStart(TransportMasterNodeAction.java:193)
	at org.elasticsearch.server@9.1.0-SNAPSHOT/org.elasticsearch.action.support.master.TransportMasterNodeAction.doExecute(TransportMasterNodeAction.java:165)
	at org.elasticsearch.server@9.1.0-SNAPSHOT/org.elasticsearch.action.support.master.TransportMasterNodeAction.doExecute(TransportMasterNodeAction.java:57)
	at org.elasticsearch.server@9.1.0-SNAPSHOT/org.elasticsearch.action.support.TransportAction$RequestFilterChain.proceed(TransportAction.java:135)
	at org.elasticsearch.server@9.1.0-SNAPSHOT/org.elasticsearch.action.support.TransportAction.handleExecution(TransportAction.java:96)
	at org.elasticsearch.server@9.1.0-SNAPSHOT/org.elasticsearch.action.support.TransportAction.execute(TransportAction.java:59)
	at org.elasticsearch.server@9.1.0-SNAPSHOT/org.elasticsearch.tasks.TaskManager.registerAndExecute(TaskManager.java:197)
	at org.elasticsearch.server@9.1.0-SNAPSHOT/org.elasticsearch.client.internal.node.NodeClient.executeLocally(NodeClient.java:106)
	at org.elasticsearch.server@9.1.0-SNAPSHOT/org.elasticsearch.rest.action.RestCancellableNodeClient.doExecute(RestCancellableNodeClient.java:82)
	at org.elasticsearch.server@9.1.0-SNAPSHOT/org.elasticsearch.client.internal.support.AbstractClient.execute(AbstractClient.java:140)
	at org.elasticsearch.server@9.1.0-SNAPSHOT/org.elasticsearch.client.internal.IndicesAdminClient.execute(IndicesAdminClient.java:312)
	at org.elasticsearch.server@9.1.0-SNAPSHOT/org.elasticsearch.client.internal.IndicesAdminClient.rolloverIndex(IndicesAdminClient.java:469)
	at org.elasticsearch.server@9.1.0-SNAPSHOT/org.elasticsearch.rest.action.admin.indices.RestRolloverIndexAction.lambda$prepareRequest$1(RestRolloverIndexAction.java:65)

masseyke · 2025-03-21T21:52:06Z

Nice catch @nielsbauman! Do we need to run by this one by the BCC @dakrone?

dakrone · 2025-03-21T21:57:44Z

I think this fits under the >bug category, so I think we're okay without it.

elasticsearchmachine · 2025-03-22T09:56:19Z

Hi @nielsbauman, I've created a changelog YAML for you.

elasticsearchmachine · 2025-03-22T11:00:38Z

💔 Backport failed

Status	Branch	Result
❌	8.18	Commit could not be cherrypicked due to conflicts
❌	8.x	Commit could not be cherrypicked due to conflicts
❌	9.0	Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 125352

nielsbauman · 2025-03-22T13:07:49Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x
✅	9.0
✅	8.18

Questions ?

Please refer to the Backport tool documentation

Since elastic#122905 we were throwing NPEs (i.e. 5xxs) when a rollover request has an unknown/non-existent target. Before that, we returned a 400 - illegal argument exception. We now return a 404 which matches "missing target" better. Additionally, to avoid this from happening again, we add a YAML test that asserts the correct exception behavior. (cherry picked from commit fdd4537) # Conflicts: # server/src/main/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverService.java

) Since #122905 we were throwing NPEs (i.e. 5xxs) when a rollover request has an unknown/non-existent target. Before that, we returned a 400 - illegal argument exception. We now return a 404 which matches "missing target" better. Additionally, to avoid this from happening again, we add a YAML test that asserts the correct exception behavior. (cherry picked from commit fdd4537) # Conflicts: # server/src/main/java/org/elasticsearch/action/admin/indices/rollover/MetadataRolloverService.java

Since elastic#122905 we were throwing NPEs (i.e. 5xxs) when a rollover request has an unknown/non-existent target. Before that, we returned a 400 - illegal argument exception. We now return a 404 which matches "missing target" better. Additionally, to avoid this from happening again, we add a YAML test that asserts the correct exception behavior.

nielsbauman requested a review from masseyke March 20, 2025 21:22

nielsbauman commented Mar 20, 2025

View reviewed changes

rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.rollover/10_basic.yml Outdated Show resolved Hide resolved

nielsbauman and others added 5 commits March 21, 2025 08:41

Merge branch 'main' into fix-rollover-not-found

1bf5dde

Return 404 instead of 400

9135578

Remove unused parameter

24ddc07

[CI] Auto commit changes from spotless

bf35059

Add test capability

99e9ada

nielsbauman requested a review from dakrone March 21, 2025 08:00

nielsbauman and others added 2 commits March 21, 2025 09:33

Fix test

767b894

Merge branch 'main' into fix-rollover-not-found

72f92f5

dakrone approved these changes Mar 21, 2025

View reviewed changes

masseyke approved these changes Mar 21, 2025

View reviewed changes

nielsbauman changed the title ~~Fix NPE in rolling over unknown target~~ Mar 22, 2025

nielsbauman added >bug and removed >non-issue labels Mar 22, 2025

Update docs/changelog/125352.yaml

08d520f

Merge branch 'main' into fix-rollover-not-found

bb8ace1

nielsbauman enabled auto-merge (squash) March 22, 2025 09:57

nielsbauman disabled auto-merge March 22, 2025 09:57

nielsbauman enabled auto-merge (squash) March 22, 2025 09:57

nielsbauman merged commit fdd4537 into elastic:main Mar 22, 2025
16 of 17 checks passed

nielsbauman deleted the fix-rollover-not-found branch March 22, 2025 10:59

elasticsearchmachine added the backport pending label Mar 22, 2025

This was referenced Mar 22, 2025

[8.x] Fix NPE in rolling over unknown target and return 404 (#125352) #125453

Merged

[9.0] Fix NPE in rolling over unknown target and return 404 (#125352) #125454

Merged

[8.18] Fix NPE in rolling over unknown target and return 404 (#125352) #125455

Merged

nielsbauman removed the backport pending label Apr 23, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix NPE in rolling over unknown target and return 404#125352

Fix NPE in rolling over unknown target and return 404#125352
nielsbauman merged 10 commits intoelastic:mainfrom
nielsbauman:fix-rollover-not-found

nielsbauman commented Mar 20, 2025 •

edited

Loading

elasticsearchmachine commented Mar 20, 2025

Uh oh!

dakrone left a comment

nielsbauman commented Mar 21, 2025

masseyke commented Mar 21, 2025

masseyke commented Mar 21, 2025

dakrone commented Mar 21, 2025

elasticsearchmachine commented Mar 22, 2025

Uh oh!

elasticsearchmachine commented Mar 22, 2025

nielsbauman commented Mar 22, 2025

Labels

4 participants

Conversation

nielsbauman commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

elasticsearchmachine commented Mar 20, 2025

Uh oh!

dakrone left a comment

Choose a reason for hiding this comment

nielsbauman commented Mar 21, 2025

masseyke commented Mar 21, 2025

masseyke commented Mar 21, 2025

dakrone commented Mar 21, 2025

elasticsearchmachine commented Mar 22, 2025

Uh oh!

elasticsearchmachine commented Mar 22, 2025

💔 Backport failed

nielsbauman commented Mar 22, 2025

💚 All backports created successfully

Questions ?

Labels

4 participants

nielsbauman commented Mar 20, 2025 •

edited

Loading