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

Include upcoming default values for the session timeouts in the deprecation log. #106673

Merged

Conversation

azasypkin
Copy link
Member

Summary

Include upcoming default values for the session timeouts in the deprecation log.

Follow-up for: #106061

@azasypkin azasypkin added Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! Feature:Security/Authentication Platform Security - Authentication release_note:skip Skip the PR/issue when compiling release notes deprecation_warnings backport:skip This commit does not require backporting v7.15.0 labels Jul 26, 2021
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

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

@azasypkin azasypkin marked this pull request as ready for review July 26, 2021 10:11
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-security (Team:Security)

@azasypkin azasypkin requested a review from legrego July 26, 2021 10:11
Copy link
Member

@legrego legrego left a comment

Choose a reason for hiding this comment

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

LGTM! question do you think we should merge into master as well, so that the config_deprecations file is kept in sync between the branches? At some point master will diverge to support 8.0, but it might make maintaining these deprecations easier in the short term if everything is aligned

@azasypkin
Copy link
Member Author

azasypkin commented Jul 26, 2021

question do you think we should merge into master as well, so that the config_deprecations file is kept in sync between the branches?

I was considering this initially, but the issue is that this deprecation will always be logged in master/8.0 if we keep it (raw session settings will be undefined since we get them before they are validated where we set them to custom defaults), which is confusing and technically not correct. Or what was your idea @legrego ?

@legrego
Copy link
Member

legrego commented Jul 26, 2021

question do you think we should merge into master as well, so that the config_deprecations file is kept in sync between the branches?

I was considering this initially, but the issue is that this deprecation will always be logged in master/8.0 if we keep it (raw session settings will be undefined since we get them before they are validated where we set them to custom defaults), which is confusing and technically not correct. Or what was your idea @legrego ?

Ah good call, I forgot about that the deprecations run beforehand. In that case, I'm happy to leave this as-is, and only merge into 7.x. Thanks!

@azasypkin azasypkin merged commit 2e80797 into elastic:7.x Jul 26, 2021
@azasypkin azasypkin deleted the v7.x-issue-xxx-session-timeout-deprecations branch July 26, 2021 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting deprecation_warnings Feature:Security/Authentication Platform Security - Authentication release_note:skip Skip the PR/issue when compiling release notes Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! v7.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants