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.

Sign up for GitHub

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

Jump to bottom

CORE: Make Pattern Exclusion Work with Aliases #33518

Merged
merged 5 commits into from
Sep 9, 2018
Merged

CORE: Make Pattern Exclusion Work with Aliases #33518

merged 5 commits into from
Sep 9, 2018

Conversation

original-brownbear
Copy link
Member

@original-brownbear
CORE: Make Pattern Exclusion Work with Aliases
576dd2d 
* Adds the pattern exclusion logic to finding aliases
* Closes elastic#33395
@original-brownbear original-brownbear added >bug :Data Management/Indices APIs APIs to create and manage indices and templates v7.0.0 v6.5.0 labels Sep 7, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@jasontedor jasontedor requested a review from bleskes September 8, 2018 02:42
bleskes
bleskes suggested changes Sep 8, 2018
View reviewed changes
Copy link
Contributor

@bleskes bleskes left a comment

Choose a reason for hiding this comment

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

I'm afraid this is not good enough. Think about something like ["a*", "-*b", "b*"] - this should start with adding all the aliases which start with a, remove those who end with a b and then add all aliases that start with a b. This means that an alias bob will be added. I believe the current version of the code doesn't do that.

Another problem I see is when people say ["a*", "-ab"] which should mean - all aliases that start with an by not ab. Note that alias names can't start with a - for this reason. See MetaDataCreateIndexService#validateIndexOrAliasName

@original-brownbear
Copy link
Member Author

@bleskes

I believe the current version of the code doesn't do that.

Yea you're right. Sorry didn't think (or know :)) about that case yet :( => will fix very soon.

Another problem I see is when people say ["a*", "-ab"] which should mean - all aliases that start with an by not ab. Note that alias names can't start with a - for this reason. See MetaDataCreateIndexService#validateIndexOrAliasName

This case works fine already I think (see the added test).

@original-brownbear
Copy link
Member Author

original-brownbear commented Sep 9, 2018 •
edited
Loading

@bleskes from last night's slack conversation: double checked that we disallow indices starting with - starting in 5.0.0 see 6030acb that went into 5.0.0 :)

@original-brownbear
Copy link
Member Author

@bleskes all fixed now (I hope :)) in 8b9e769 -> good for another review :)

bleskes
bleskes approved these changes Sep 9, 2018
View reviewed changes
Copy link
Contributor

@bleskes bleskes 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 for the extra iteration.

@original-brownbear
Copy link
Member Author

@bleskes np + thanks for the review :)

@original-brownbear original-brownbear merged commit d4b212c into elastic:master Sep 9, 2018
@original-brownbear original-brownbear deleted the 33395 branch September 9, 2018 15:31
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Sep 9, 2018
@jasontedor
Merge branch 'master' into upgrade-settings
f071e96 
* master:
  CORE: Make Pattern Exclusion Work with Aliases (elastic#33518)
  Reverse logic for CCR license checks (elastic#33549)
  Add latch countdown on failure in CCR license tests (elastic#33548)
  HLRC: Add put stored script support to high-level rest client (elastic#31323)
  Create temporary directory if needed in CCR test
  Add license checks for auto-follow implementation (elastic#33496)
  Bootstrap a new history_uuid when force allocating a stale primary (elastic#33432)
  INGEST: Remove Outdated TODOs (elastic#33458)
  Logging: Clean up skipping test
  Logging: Skip test if it'd fail
  CRUD: AwaitsFix entire wait_for_refresh close test
  Painless: Add Imported Static Method (elastic#33440)
This was referenced Sep 28, 2018
Merged
Merged
Closed
albertzaharovits added a commit that referenced this pull request Oct 4, 2018
@albertzaharovits
[Security] Multi Index Expression alias wildcard exclusion (#34144)
f817bc5 
The Security plugin authorizes actions on indices. Authorization
happens on a per index/alias basis. Therefore a request with a
Multi Index Expression (containing wildcards) has to be
first evaluated in the authorization layer, before the request is
handled. For authorization purposes, wildcards in expressions will
only be expanded to indices/aliases that are visible by the authenticated
user. However, this "constrained" evaluation has to be compatible with
the expression evaluation that a cluster without the Security plugin
would do. Therefore any change in the evaluation logic
in any of these sites has to be mirrored in the other site.

This commit mirrors the changes in core from #33518 that allowed
for Multi Index Expression in the Get Alias API, loosely speaking.
albertzaharovits added a commit that referenced this pull request Oct 4, 2018
@albertzaharovits
[Security] Multi Index Expression alias wildcard exclusion (#34144)
3a202a8 
The Security plugin authorizes actions on indices. Authorization
happens on a per index/alias basis. Therefore a request with a
Multi Index Expression (containing wildcards) has to be
first evaluated in the authorization layer, before the request is
handled. For authorization purposes, wildcards in expressions will
only be expanded to indices/aliases that are visible by the authenticated
user. However, this "constrained" evaluation has to be compatible with
the expression evaluation that a cluster without the Security plugin
would do. Therefore any change in the evaluation logic
in any of these sites has to be mirrored in the other site.

This commit mirrors the changes in core from #33518 that allowed
for Multi Index Expression in the Get Alias API, loosely speaking.
kcm pushed a commit that referenced this pull request Oct 30, 2018
@albertzaharovits @kcm
[Security] Multi Index Expression alias wildcard exclusion (#34144)
01706ee 
The Security plugin authorizes actions on indices. Authorization
happens on a per index/alias basis. Therefore a request with a
Multi Index Expression (containing wildcards) has to be
first evaluated in the authorization layer, before the request is
handled. For authorization purposes, wildcards in expressions will
only be expanded to indices/aliases that are visible by the authenticated
user. However, this "constrained" evaluation has to be compatible with
the expression evaluation that a cluster without the Security plugin
would do. Therefore any change in the evaluation logic
in any of these sites has to be mirrored in the other site.

This commit mirrors the changes in core from #33518 that allowed
for Multi Index Expression in the Get Alias API, loosely speaking.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bleskes bleskes bleskes approved these changes

Assignees
No one assigned
Labels
>bug :Data Management/Indices APIs APIs to create and manage indices and templates v6.5.0 v7.0.0-beta1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@original-brownbear @elasticmachine @bleskes @colings86