Skip to content

Conversation

@danhermann
Copy link
Contributor

@danhermann danhermann commented Sep 29, 2021

The root cause of this bug was the replacement of the wildcard expression in the request's indices member with the actual data streams that expression matched. In the case that the authz code had already replaced it with the *,-* token that means "no authorized data streams", the expression would be evaluated again, would match no data streams, and would set the indices member to an empty array. An empty array is equivalent to *, so all data streams would then be deleted.

Fixes #78422

Comment on lines 88 to 89
}
request.indices(names.toArray(Strings.EMPTY_ARRAY));

Copy link
Contributor Author

@danhermann danhermann Sep 29, 2021

Choose a reason for hiding this comment

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

I'm not sure that it's useful to duplicate the name resolution and system index validation logic in masterOperation and below in removeDataStream but the overwriting of the request's indices is the most narrow root cause of this bug.

Copy link
Member

Choose a reason for hiding this comment

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

I think it's fine to double-check the validation so that we avoid the cluster state task if possible, so this seems reasonable to me.

@danhermann danhermann added the Team:Data Management Meta label for data/management team label Sep 29, 2021
@danhermann
Copy link
Contributor Author

Manually pinging @elastic/es-data-management (Team:Data Management) since GH's webhooks are 🤷

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.

LGTM, though could you please fill out the PR body with an explanation of what happened and how this fixes it for the sake of future Github searchability?

Comment on lines 88 to 89
}
request.indices(names.toArray(Strings.EMPTY_ARRAY));

Copy link
Member

Choose a reason for hiding this comment

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

I think it's fine to double-check the validation so that we avoid the cluster state task if possible, so this seems reasonable to me.

@danhermann
Copy link
Contributor Author

Thanks, @dakrone.

@danhermann danhermann merged commit 9d5395e into elastic:master Sep 30, 2021
@danhermann danhermann deleted the 78422_ds_wildcard_delete branch September 30, 2021 11:47
danhermann added a commit to danhermann/elasticsearch that referenced this pull request Sep 30, 2021
danhermann added a commit to danhermann/elasticsearch that referenced this pull request Sep 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>bug :Data Management/Data streams Data streams and their lifecycles Team:Data Management Meta label for data/management team v7.15.1 v7.16.0 v8.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Non-matching wildcard delete on datastream appears to delete all datastreams

3 participants