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.

Sign up for GitHub

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

Jump to bottom

Security should not reload files that haven't changed #50063

Closed
tvernum opened this issue Dec 11, 2019 · 3 comments · Fixed by #50207
Closed

Security should not reload files that haven't changed #50063

tvernum opened this issue Dec 11, 2019 · 3 comments · Fixed by #50207
Labels
good first issue low hanging fruit help wanted adoptme :Security/Security Security issues without another label

Comments

@tvernum
Copy link
Contributor

tvernum commented Dec 11, 2019

In security we currently monitor a set of files for changes

  • config/role_mapping.yml (or alternative configured path)
  • config/roles.yml
  • config/users
  • config/users_roles

We use the ResourceWatcherService to detect changes and then reload structures and/or clear caches.

In some cases we detect that a "file change" has resulted in no actual changes to the stored content, and do nothing, but there are cases where simply touching a file will clear caches.

It would be preferable to only clear caches if the file change actually caused some internal structure to change (e.g a user was added/removed/modified).
@tvernum tvernum added good first issue low hanging fruit help wanted adoptme :Security/Security Security issues without another label labels Dec 11, 2019
@elasticmachine
Copy link
Collaborator

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

@AntonShuvaev
Copy link
Contributor

Can I start working on this?

@albertzaharovits
Copy link
Contributor

Thank you for your interest in contributing to the project @J-Bean !

Yes, you may start working on it.

@AntonShuvaev AntonShuvaev mentioned this issue Dec 15, 2019
Merged
tvernum pushed a commit that referenced this issue Jan 6, 2020
@AntonShuvaev @tvernum
Security should not reload files that haven't changed (#50207)
3184a89 
In security we currently monitor a set of files for changes:

- config/role_mapping.yml (or alternative configured path)
- config/roles.yml
- config/users
- config/users_roles

This commit prevents unnecessary reloading when the file change actually doesn't change the internal structure.

Closes: #50063

Co-authored-by: Anton Shuvaev <anton.shuvaev91@gmail.com>
SivagurunathanV pushed a commit to SivagurunathanV/elasticsearch that referenced this issue Jan 23, 2020
@AntonShuvaev @SivagurunathanV
Security should not reload files that haven't changed (elastic#50207)
1fbf799 
In security we currently monitor a set of files for changes:

- config/role_mapping.yml (or alternative configured path)
- config/roles.yml
- config/users
- config/users_roles

This commit prevents unnecessary reloading when the file change actually doesn't change the internal structure.

Closes: elastic#50063

Co-authored-by: Anton Shuvaev <anton.shuvaev91@gmail.com>
This was referenced Feb 3, 2020
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue low hanging fruit help wanted adoptme :Security/Security Security issues without another label
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Security should not reload files that haven't changed
4 participants
@tvernum @albertzaharovits @elasticmachine @AntonShuvaev