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

Deprecation check for Auth realm setting structure #36664

Merged
merged 9 commits into from
Jan 2, 2019

Conversation

gwbrown
Copy link
Contributor

@gwbrown gwbrown commented Dec 14, 2018

Adds a deprecation check for changes to the structure of authentication
realm settings.

Relates to #36024 and #30241

Adds a deprecation check for changes to the structure of authentication
realm settings.
@gwbrown gwbrown added >enhancement :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) v6.6.0 :Core/Features/Features labels Dec 14, 2018
@gwbrown gwbrown requested a review from tvernum December 14, 2018 23:14
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security

@gwbrown
Copy link
Contributor Author

gwbrown commented Dec 15, 2018

Test failure is #36598, which appears to be inconsistent.

@elasticmachine run gradle build tests 1 please

@tvernum
Copy link
Contributor

tvernum commented Dec 17, 2018

Is this the right approach? It might be - I haven't kept up to date with our plan for ugrade checks.

6.x nodes need to use the old realm settings format, and 7.x nodes need to use the new realm format. We don't offer any upgrade option except to change the yml file when performing the upgrade.

So it won't be possible to fix this warning on 6.x, you just need to be aware that there's a step you need to do during the upgrade process.

@gwbrown
Copy link
Contributor Author

gwbrown commented Dec 17, 2018

@tvernum Interesting - most other breaking settings changes you are able to make before upgrading.

I think we still want a warning here, but I'll add a note in the details that this needs to be done as part of the upgrade of each node to 7.x, rather than done before the upgrade like other settings changes.

@gwbrown
Copy link
Contributor Author

gwbrown commented Dec 17, 2018

Oh - I didn't quite process what you were saying. Any and every configured realm will trigger this warning, with no way of resolving it before upgrading. I've simplified the check, as I think this is important to notify cluster administrators of, but there's no need for the complexity that was there.

@jasontedor jasontedor added v6.7.0 and removed v6.6.0 labels Dec 19, 2018
@gwbrown
Copy link
Contributor Author

gwbrown commented Dec 20, 2018

@tvernum How do you feel about this method of checking?

@tvernum
Copy link
Contributor

tvernum commented Dec 21, 2018

This looks right. I'll need to double check it when I'm back at the end of next week, (I'm half asleep now, do I'd probably miss something) but it seems right.

@gwbrown
Copy link
Contributor Author

gwbrown commented Dec 27, 2018

@elasticmachine run gradle build tests 1 please

Copy link
Contributor

@tvernum tvernum left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) v6.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants