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

System index auto-creation should not be disabled by user settings #62984

Conversation

williamrandolph
Copy link
Contributor

@williamrandolph williamrandolph commented Sep 28, 2020

By default, Elasticsearch auto-creates indices when a document is submitted to a non-existent index. There is a setting that allows users to disable this behavior. However, this setting should not apply to system indices, so that Elasticsearch modules and plugins are able to use auto-create behavior whether or not it is exposed to users.

This commit constructs the AutoCreateIndex object with a reference to the SystemIndices object so that we bypass the check for the user-facing autocreate setting when it's a system index that is being autocreated.

We also modify the logic in TransportBulkAction to make sure that if a system index is included in a bulk request, we don't skip the autocreation step.

Closes #61642

Note that this PR touches the same code as #61858, which should probably be merged first.

Todo:

  • write additional tests?

By default, Elasticsearch auto-creates indices when a document is
submitted to a non-existent index. There is a setting that allows users
to disable this behavior. However, this setting should not apply to
system indices, so that Elasticsearch modules and plugins are able to
use auto-create behavior whether or not it is exposed to users.

This commit constructs the AutoCreateIndex object with a reference to
the SystemIndices object so that we bypass the check for the user-facing
autocreate setting when it's a system index that is being autocreated.

We also modify the logic in TransportBulkAction to make sure that if a
system index is included in a bulk request, we don't skip the
autocreation step.
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (:Core/Infra/Core)

@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Sep 28, 2020
Copy link
Contributor

@gwbrown gwbrown left a comment

Choose a reason for hiding this comment

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

Looking good, I left a couple minor comments and a note about where I think the biggest conflict with #61858 will be.

I do agree it would probably be best to merge #61858 first.

if (needToCheck()) {
final boolean includesSystem = includesSystem(bulkRequest, clusterService.state().metadata().getIndicesLookup(), systemIndices);

if (includesSystem || needToCheck()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You've already noted in this in the PR description, but this is going to conflict with https://github.com/elastic/elasticsearch/pull/61858/files?file-filters%5B%5D=.java#diff-7c2a294c3d25c427072ee6c64983b560L227 specifically. I think this is the only place they'll meaningfully conflict, though.

Copy link
Contributor

@gwbrown gwbrown left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! As we discussed elsewhere, let's go ahead and merge this as it looks like #61858 might be a bit of a wait.

@williamrandolph williamrandolph merged commit 5f7a151 into elastic:master Oct 1, 2020
williamrandolph added a commit to williamrandolph/elasticsearch that referenced this pull request Oct 1, 2020
…lastic#62984)

* Add System Indices check to AutoCreateIndex

By default, Elasticsearch auto-creates indices when a document is
submitted to a non-existent index. There is a setting that allows users
to disable this behavior. However, this setting should not apply to
system indices, so that Elasticsearch modules and plugins are able to
use auto-create behavior whether or not it is exposed to users.

This commit constructs the AutoCreateIndex object with a reference to
the SystemIndices object so that we bypass the check for the user-facing
autocreate setting when it's a system index that is being autocreated.

We also modify the logic in TransportBulkAction to make sure that if a
system index is included in a bulk request, we don't skip the
autocreation step.
williamrandolph added a commit that referenced this pull request Oct 1, 2020
…62984) (#63147)

* Add System Indices check to AutoCreateIndex

By default, Elasticsearch auto-creates indices when a document is
submitted to a non-existent index. There is a setting that allows users
to disable this behavior. However, this setting should not apply to
system indices, so that Elasticsearch modules and plugins are able to
use auto-create behavior whether or not it is exposed to users.

This commit constructs the AutoCreateIndex object with a reference to
the SystemIndices object so that we bypass the check for the user-facing
autocreate setting when it's a system index that is being autocreated.

We also modify the logic in TransportBulkAction to make sure that if a
system index is included in a bulk request, we don't skip the
autocreation step.
@williamrandolph williamrandolph deleted the system-indices-bypass-autocreate-setting branch May 23, 2022 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Core Core issues without another label >enhancement Team:Core/Infra Meta label for core/infra team v7.10.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

System indices should be able to bypass restricted index auto creation
4 participants