Skip to content

Conversation

@nielsbauman
Copy link
Contributor

@nielsbauman nielsbauman commented Mar 20, 2025

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.
@nielsbauman nielsbauman added >non-issue :Data Management/Indices APIs APIs to create and manage indices and templates Team:Data Management Meta label for data/management team auto-backport Automatically create backport pull requests when merged v8.18.1 v8.19.0 v9.0.1 v9.1.0 labels Mar 20, 2025
@nielsbauman nielsbauman requested a review from masseyke March 20, 2025 21:22
@elasticsearchmachine
Copy link
Collaborator

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

@nielsbauman nielsbauman requested a review from dakrone March 21, 2025 08:00
Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Contributor Author

@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
Copy link
Member

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 [email protected]/org.elasticsearch.action.admin.indices.rollover.TransportRolloverAction.checkBlock(TransportRolloverAction.java:167)
	at [email protected]/org.elasticsearch.action.admin.indices.rollover.TransportRolloverAction.checkBlock(TransportRolloverAction.java:76)
	at [email protected]/org.elasticsearch.action.support.master.TransportMasterNodeAction.checkBlockIfStateRecovered(TransportMasterNodeAction.java:123)
	at [email protected]/org.elasticsearch.action.support.master.TransportMasterNodeAction$AsyncSingleAction.doStart(TransportMasterNodeAction.java:193)
	at [email protected]/org.elasticsearch.action.support.master.TransportMasterNodeAction.doExecute(TransportMasterNodeAction.java:165)
	at [email protected]/org.elasticsearch.action.support.master.TransportMasterNodeAction.doExecute(TransportMasterNodeAction.java:57)
	at [email protected]/org.elasticsearch.action.support.TransportAction$RequestFilterChain.proceed(TransportAction.java:135)
	at [email protected]/org.elasticsearch.action.support.TransportAction.handleExecution(TransportAction.java:96)
	at [email protected]/org.elasticsearch.action.support.TransportAction.execute(TransportAction.java:59)
	at [email protected]/org.elasticsearch.tasks.TaskManager.registerAndExecute(TaskManager.java:197)
	at [email protected]/org.elasticsearch.client.internal.node.NodeClient.executeLocally(NodeClient.java:106)
	at [email protected]/org.elasticsearch.rest.action.RestCancellableNodeClient.doExecute(RestCancellableNodeClient.java:82)
	at [email protected]/org.elasticsearch.client.internal.support.AbstractClient.execute(AbstractClient.java:140)
	at [email protected]/org.elasticsearch.client.internal.IndicesAdminClient.execute(IndicesAdminClient.java:312)
	at [email protected]/org.elasticsearch.client.internal.IndicesAdminClient.rolloverIndex(IndicesAdminClient.java:469)
	at [email protected]/org.elasticsearch.rest.action.admin.indices.RestRolloverIndexAction.lambda$prepareRequest$1(RestRolloverIndexAction.java:65)

@masseyke
Copy link
Member

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

@dakrone
Copy link
Member

dakrone commented Mar 21, 2025

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

@nielsbauman nielsbauman changed the title Fix NPE in rolling over unknown target Fix NPE in rolling over unknown target and return 404 Mar 22, 2025
@elasticsearchmachine
Copy link
Collaborator

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

@nielsbauman nielsbauman enabled auto-merge (squash) March 22, 2025 09:57
@nielsbauman nielsbauman disabled auto-merge March 22, 2025 09:57
@nielsbauman nielsbauman enabled auto-merge (squash) March 22, 2025 09:57
@nielsbauman nielsbauman merged commit fdd4537 into elastic:main Mar 22, 2025
16 of 17 checks passed
@nielsbauman nielsbauman deleted the fix-rollover-not-found branch March 22, 2025 10:59
@elasticsearchmachine
Copy link
Collaborator

💔 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
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
8.x
9.0
8.18

Questions ?

Please refer to the Backport tool documentation

nielsbauman added a commit to nielsbauman/elasticsearch that referenced this pull request Mar 22, 2025
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
nielsbauman added a commit to nielsbauman/elasticsearch that referenced this pull request Mar 22, 2025
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
elasticsearchmachine pushed a commit that referenced this pull request Mar 22, 2025
)

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
elasticsearchmachine pushed a commit that referenced this pull request Mar 22, 2025
)

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
elasticsearchmachine pushed a commit that referenced this pull request Mar 22, 2025
)

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
omricohenn pushed a commit to omricohenn/elasticsearch that referenced this pull request Mar 28, 2025
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged >bug :Data Management/Indices APIs APIs to create and manage indices and templates Team:Data Management Meta label for data/management team v8.18.1 v8.19.0 v9.0.1 v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants