-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Add exclusive file entitlement for settings #125272
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
Conversation
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
|
||
| @Override | ||
| protected Path filesSetLastModifiedTime(Path path, FileTime time) throws IOException { | ||
| return Files.setLastModifiedTime(path, time); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how this one is working right now since we only have read access. IIRC this is an edge case for when we restore from a snapshot. We should (as a followup) re-consider how restoring from snapshots should interact with file settings because this breaks the rule that we want to use, only allowing read access to the config directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed.
| FileData.ofRelativePath(Path.of(""), SHARED_REPO, READ_WRITE), | ||
|
|
||
| // exclusive settings file | ||
| FileData.ofRelativePath(Path.of("operator/settings.json"), CONFIG, READ).withExclusive(true), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately I think this needs to be READ_WRITE, see my other comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed.
|
@elasticmachine run elasticsearch-ci/part-3 |
|
@elasticmachine run elasticsearch-ci/part-3 |
Adds changes to ensure the correct caller's module is checked for exclusive file settings.
Adds changes to ensure the correct caller's module is checked for exclusive file settings.
ES-11136