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

[ML] Fixing annotations alias checks #58722

Merged

Conversation

jgowdyelastic
Copy link
Member

Fixes bug caused by the changing of ml-annotations-read and ml-annotations-write to hidden indices (elastic/elasticsearch#52423) which made it impossible to create annotations when security is disabled.

When calling indices.existsAlias we now need to specify index and name.

@jgowdyelastic jgowdyelastic added non-issue Indicates to automation that a pull request should not appear in the release notes :ml Feature:Anomaly Detection ML anomaly detection v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.7.0 labels Feb 27, 2020
@jgowdyelastic jgowdyelastic requested a review from a team as a code owner February 27, 2020 12:52
@jgowdyelastic jgowdyelastic self-assigned this Feb 27, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

LGTM

@droberts195
Copy link
Contributor

when security is disabled

I think this is actually a sign of another bug, alluded to in "We're likely going to have to make some improvements to how Security handles aliases" in elastic/elasticsearch#52304 (comment). Logically the problem being fixed here should have happened both with security enabled and disabled.

/cc @jaymode and @gwbrown to make sure your plans for elastic/elasticsearch#52304 cover this. The discrepancy happened when checking for the existence of an alias by asking if it is an alias of any index:

  • When the alias was an alias on a hidden index and security was disabled the API call said the alias didn't exist
  • When the alias was an alias on a hidden index and security was enabled the API call said the alias did exist

@jgowdyelastic
Copy link
Member Author

@elasticmachine merge upstream

@gwbrown
Copy link

gwbrown commented Feb 27, 2020

Thanks @droberts195, I'll make sure we have integration tests for those scenarios so that they behave the same.

Copy link
Contributor

@darnautov darnautov left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@walterra walterra left a comment

Choose a reason for hiding this comment

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

LGTM

@jgowdyelastic jgowdyelastic force-pushed the fixing-annotations-alias-checks branch from 49a795c to a7aedd8 Compare February 28, 2020 12:35
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

  • 💚 Build #29443 succeeded 49a795cc2e49eff6f6f614e2f496e8bfcc33af72

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Anomaly Detection ML anomaly detection :ml non-issue Indicates to automation that a pull request should not appear in the release notes release_note:skip Skip the PR/issue when compiling release notes v7.7.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants