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

Read operator privs enabled from Env settings #98246

Merged

Conversation

tvernum
Copy link
Contributor

@tvernum tvernum commented Aug 7, 2023

Security.settings incorrectly caches the node startup settings which does not take into account setting changes that may take place after the node has started (e.g. "addtionalSettings()" from plugins).

This commit fixes the behaviour for the OperatorPrivileges enabled setting, until the bug can be fixed more generally

Security.settings incorrectly caches the node startup settings which
does not take into account setting changes that may take place after
the node has started (e.g. "addtionalSettings()" from plugins).

This commit fixes the behaviour for the OperatorPrivileges enabled
setting, until the bug can be fixed more generally
@tvernum tvernum added >enhancement :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC v8.10.0 labels Aug 7, 2023
@elasticsearchmachine
Copy link
Collaborator

Hi @tvernum, I've created a changelog YAML for you.

@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine elasticsearchmachine added the Team:Security Meta label for security team label Aug 7, 2023
@tvernum
Copy link
Contributor Author

tvernum commented Aug 7, 2023

I'd love to fix the bug more generally - this TODO has been around for far too long, but it required making settings not final, which is a can of worms that I can't get through today.

// TODO This is wrong. Settings can change after this. We should use the settings from createComponents
this.settings = settings;

Copy link
Member

@ywangd ywangd left a comment

Choose a reason for hiding this comment

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

LGTM

FWIW, I had to do something similar for the recent work of supporting overridden builtin roles.

Copy link
Contributor

@jakelandis jakelandis left a comment

Choose a reason for hiding this comment

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

Thanks for fixing !

@tvernum

This comment was marked as outdated.

@tvernum

This comment was marked as outdated.

@tvernum tvernum added the auto-merge Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Aug 8, 2023
@elasticsearchmachine elasticsearchmachine merged commit 2aca985 into elastic:main Aug 8, 2023
15 checks passed
@tvernum tvernum deleted the fix-operator-priv-addn-settings branch August 8, 2023 05:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) >enhancement :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC Team:Security Meta label for security team v8.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants