feat: add cap_add and cap_drop support
#726
Conversation
✅ Deploy Preview for testcontainers-rust ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
DDtKey
changed the title
cap_add and cap_drop to ContainerRequest, ImageExtcap_add and cap_drop support
| fn with_cap_add(self, capabilities: impl IntoIterator<Item = String>) -> ContainerRequest<I> { | ||
| let mut container_req = self.into(); | ||
| container_req | ||
| .cap_add | ||
| .get_or_insert_with(Vec::new) | ||
| .extend(capabilities); | ||
|
|
||
| container_req | ||
| } |
There was a problem hiding this comment.
In general, I'm not sure we need the "extend" semantic. We don't use it for other methods and it might be less expected.
I think it's acceptable to replace if method called again (and perhaps more obvious)
It's possible to get current capabilities, extend and override If users will need to "add" them 🤔
WDYT?
There was a problem hiding this comment.
I see your point. With some with_ functions we do have the additive semantic of extend though, e.g. mounts and environment variables (with the difference that the newly added functions currently support adding multiple values at once instead of one by one, hence the extend instead of push/insert).
Intuitively, I would say it could be the least surprising to the user if it maps to how the docker CLI handles it, since most users are likely to be familiar with it.
- Is it possible to specify the flag multiple times to combine the permissions additively? If yes, extend, if not, replace.
- Is it possible to add/drop multiple permissions with a single instance of the flag? If yes, take
impl IntoIter<Item = String>, if not, takeString.
What do you think of that approach?
Of course I can also add documentation for this behaviour to the functions, regardless of the implementation we go with
There was a problem hiding this comment.
Yeah, that’s all good points.
Only with_cmd actually overwrites the list (and it generally makes sense, I wanted it to be aligned with docker api - additive cmd would be weird, the list is a single value in that case).
AFAIR docker accepts 1 value per flag for cap-add/drop, but they are additive for sure.
So we can go with the way how we expose ENV variables and mounts 🤔 Also, I guess it’s common case when only 1 capability is needed. And we can extend it on demand in general
Correct me if I’m wrong
There was a problem hiding this comment.
Hi @nicmr 👋
Let me know if you can’t (or don’t want to) finalize this last thing, please 🙏
There was a problem hiding this comment.
Sorry, I was on vacation, I'm going to finalize it this week. Thank you for your patience!
Hi 👋 This PR implements #578 and adds tests for the added functionality.
Specifically, this PR adds support for adding and dropping capabilities for containers.
Have a nice weekend!