Skip to content

Deprecation check for Auth realm setting structure#36664

Merged
AthenaEryma merged 9 commits intoelastic:6.xfrom
AthenaEryma:depcheck/security-realms-types
Jan 2, 2019
Merged

Deprecation check for Auth realm setting structure#36664
AthenaEryma merged 9 commits intoelastic:6.xfrom
AthenaEryma:depcheck/security-realms-types

Conversation

@AthenaEryma
Copy link
Copy Markdown
Contributor

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.
@AthenaEryma AthenaEryma 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
@AthenaEryma AthenaEryma requested a review from tvernum December 14, 2018 23:14
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-core-features

@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-security

@AthenaEryma
Copy link
Copy Markdown
Contributor Author

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

@elasticmachine run gradle build tests 1 please

@tvernum
Copy link
Copy Markdown
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.

@AthenaEryma
Copy link
Copy Markdown
Contributor Author

@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.

@AthenaEryma
Copy link
Copy Markdown
Contributor Author

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
@AthenaEryma
Copy link
Copy Markdown
Contributor Author

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

@tvernum
Copy link
Copy Markdown
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.

@AthenaEryma
Copy link
Copy Markdown
Contributor Author

@elasticmachine run gradle build tests 1 please

Copy link
Copy Markdown
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