Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to disable destructive operations on all indices #4549

Closed
kimchy opened this issue Dec 26, 2013 · 19 comments
Closed

Allow to disable destructive operations on all indices #4549

kimchy opened this issue Dec 26, 2013 · 19 comments

Comments

@kimchy
Copy link
Member

kimchy commented Dec 26, 2013

Add action.destructive_requires_name in order to control whether wildcards and _all are allowed for destructive operations.

The following APIs are affected by this option and are considered destructive: delete index, close index, open index, delete_by_query and delete mapping.

The action.destructive_requires_name will default to false, meaning that using wildcards and _all in the mentioned APIs is allowed.

In the delete api specifying an index will be made required, so DELETE / will not be allowed at all times regardless of action.destructive_requires_name. (Like is mentioned in #4481)

Also the action.disable_delete_all_indices option which is applicable for the delete index api and the action.disable_close_all_indices which is applicable for the close api will be dropped in favour for action.destructive_requires_name

@s1monw
Copy link
Contributor

s1monw commented Dec 26, 2013

+1

@martijnvg
Copy link
Member

+1 for delete_by_query, maybe also delete mapping? Being invoked from the Java api, doesn't require indices to be set at all.

@kimchy
Copy link
Member Author

kimchy commented Dec 27, 2013

Agreed on delete mapping!

@ghost ghost assigned martijnvg Jan 3, 2014
@martijnvg
Copy link
Member

I updated the the issue description, to better describe what will be changed. Not sure about the naming, so if anyone has a better name for it then please share it :)

@javanna
Copy link
Member

javanna commented Jan 3, 2014

I think we can also replace action.disable_close_all_indices with this new option?

@kimchy
Copy link
Member Author

kimchy commented Jan 3, 2014

Don't have a good name, here is the best I can come up with: action.dirty_all_indices...

@martijnvg
Copy link
Member

@javanna Good point, I've updated the issue description.

Maybe just action.operate_all_indices, since this setting just allows or disallows all indices being operated on.

@javanna
Copy link
Member

javanna commented Jan 3, 2014

allow wouldn't have any effect on delete and close api, as the index is or will be (#4481) required with those apis. The allow option would still be useful for other apis like delete by query and delete mapping when specifying no indices though. Thoughts?

@martijnvg
Copy link
Member

@javanna This issue should also fix #4481, since the default will be disallow, so by default deleting or closing all indices shouldn't be possible. I suggest that this issue will encapsulate #4481.

@martijnvg
Copy link
Member

Perhaps the action.operate_all_indices should be overridable on a per request basis via a query string parameter instead of changing the default for action.operate_all_indices via cluster update settings api.
Something like this: DELETE /_all?operate_all_indices=allow

@javanna
Copy link
Member

javanna commented Jan 6, 2014

Makes sense @martijnvg , I will close #4481 then. Let's also remember that currently curl -XDELETE localhost:9200/ deletes all indices, while curl -XPOST localhost:9200/_close throws an error, as a close index requires an index. Looks like we want to allow closing all indices when specifying no indices, iff action.operate_all_indices=allow.

@martijnvg
Copy link
Member

@javanna Right, the close index api requires you to at least specify one index, alias or wildcard expression, this requirement should be removed.

@martijnvg
Copy link
Member

Changed to option name from action.operate_all_indices to action.destructive_all_indices.
Like is mentioned in PR #4622, perhaps the default should be allow_if_stated instead of disallow?
For now lets not add operate_all_indices to related apis, this can if needed be done later.

@javanna
Copy link
Member

javanna commented Jan 6, 2014

I created #4627 to allow empty index in open/close index api. We might want to consider adding the open index api to the list of potentially dangerous apis when executed on all indices. In fact, if one opens for instance many big indices by mistake, this can result in more resources suddenly used on the cluster and lead to issues.

@clintongormley
Copy link

What reason is there for supporting allow? It just allows people to shoot themselves in the foot.

I think that all of these operations should require a value for {index}, with the default being to accept prefix*, * or _all.

Then this option becomes a true/false choice:

action.destructive_requires_name: true | false

... which defaults to false

@javanna
Copy link
Member

javanna commented Jan 7, 2014

Makes sense, it makes things simpler and easier to understand for users. We don't need any change to open/close index api anymore, thus I closed #4627 and the related PR.

@martijnvg
Copy link
Member

I'll update this issue description. The small difference is that with action.destructive_requires_name we disallow any wildcard expressions (prefix*, * or _all), which make this protection more predictable (you don't really know upfront if an expression is going to match all indices).

Also for the action.destructive_requires_name option the delete index api requires an index, like is described in #4481.

@martijnvg
Copy link
Member

Updated the issue description with what has been discussed.

martijnvg added a commit to martijnvg/elasticsearch that referenced this issue Jan 9, 2014
…rd expressions and `_all` is allowed to be used for destructive operat

Also the delete index api requires always an index to be specified (either concrete index, alias or wildcard expression)

Closes elastic#4549 elastic#4481
brusic pushed a commit to brusic/elasticsearch that referenced this issue Jan 19, 2014
…rd expressions and `_all` is allowed to be used for destructive operat Also the delete index api requires always an index to be specified (either concrete index, alias or wildcard expression)

Closes elastic#4549 elastic#4481
@jeffsteinmetz
Copy link

can this also be configured in elasticsearch.yml?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants