-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
drivers/exec: Fix handling of capabilities for unprivileged tasks #16643
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
```release-note:bug | ||
driver/exec: Fixed a bug where `cap_drop` and `cap_add` would not expand capabilities | ||
``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I was doing a little testing locally without Nomad to compare this approach to what Docker does, and I was a bit surprised to see they seem to have the same problem we do!
When I dug into that, I found that it looks like they decided not to fix this moby/moby#38664 moby/moby#8460 in lieu of telling folks to set the
net.ipv4.ip_unprivileged_port_start
, which is obviously not the only capability that people want (Docker sets it to 0 by default!). I think we're doing the right thing here but I want to pull in @picatz and @shoenig for a consultation on this just to make sure I'm not missing something obvious.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.
Yes, another example of where this issue hits us: we sometimes need to use a
template
to generate a wrapper shell script over anartifact
-downloaded binary. There is no way to specify the desired file mode in theartifact
stanza, so the binary ends up without an executable bit set. Consequently, without a workingcap_add = ["fowner"]
theexec
driver is effectively broken for this use case.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.
I've opened #16692 with some notes about how this same issue impacts the
docker
driver and can't be fixed because the problem is upstream, which will result in some docs updates. I'm going to give this one more pass and make sure it works as I'd expect with a restrictiveallow_caps
.