Add capabilities list to container specification#2795
Conversation
faf2fef to
993b4bc
Compare
993b4bc to
280d50e
Compare
280d50e to
6124032
Compare
|
@olljanat I removed "WIP' because the moby PR was merged; could you rebase this PR? |
070bb30 to
5c1fec3
Compare
and docker/distribution to 0d3efadf0154c2b8a4e7b6621fff9809655cc580 Signed-off-by: Olli Janatuinen <olli.janatuinen@gmail.com>
5c1fec3 to
d9c62e1
Compare
Codecov Report
@@ Coverage Diff @@
## master #2795 +/- ##
==========================================
- Coverage 62.19% 62.05% -0.15%
==========================================
Files 139 139
Lines 22314 22314
==========================================
- Hits 13879 13846 -33
- Misses 6962 6990 +28
- Partials 1473 1478 +5 |
3e29824 to
a908d18
Compare
Signed-off-by: Olli Janatuinen <olli.janatuinen@gmail.com>
a908d18 to
18d9170
Compare
|
@dperny @wk8 This one is ready for first review. PTAL. /cc @thaJeztah |
wk8
left a comment
There was a problem hiding this comment.
LGTM, might be good to have an integration test in moby once this is merged?
|
@wk8 sure ;) @dperny @anshulpundir PTAL |
|
@dperny ping |
dperny
left a comment
There was a problem hiding this comment.
question about why pids limit changed to a pointer
| pidsLimit := c.spec().PidsLimit | ||
| if pidsLimit > 0 { | ||
| resources.PidsLimit = pidsLimit | ||
| resources.PidsLimit = &pidsLimit |
There was a problem hiding this comment.
is there a reason that this changed?
There was a problem hiding this comment.
It looks to be changed on moby side and it didn't passed build without those (like I said in change log on first message).
EDIT: Most probably it is this one moby/moby#38793 adds need to use pointer here.
|
@justincormack from a security perspective, would this be a safe change to swarmkit? as i understand it, there was a reason we didn't do this from the beginning, which i think you might be familiar with, but i don't remember what it was. |
|
@dperny @justincormack as far I have understood it was this one moby/moby#26849 (comment) why original implementation was not approved. and to be able solve that issue I refactored capabilities handing on moby (with @thaJeztah help) to support for exact list of capabilities on moby/moby#38380 that actually already ships with Docker 19.03. And this PR is needed to get it working with services. |
|
It LGTM after a quick OK from security folks. |
|
@dperny lots of things are not "safe", but deciding that Swarm was the right place to make all safety decisions, and then not actually make them, was a mistake. |
Includes the following changes since last vendoring: moby/swarmkit#2795 - Add capabilities list to container specification moby/swarmkit#2845 - Fix linting error moby/swarmkit#2848 - Bump fernet/fernet-go moby/swarmkit#2856 - Add ListServiceStatuses grpc method moby/swarmkit#2857 - Use Service Placement Constraints in Enforcer Signed-off-by: Drew Erny <drew.erny@docker.com>
Includes the following changes since last vendoring: moby/swarmkit#2795 - Add capabilities list to container specification moby/swarmkit#2845 - Fix linting error moby/swarmkit#2848 - Bump fernet/fernet-go moby/swarmkit#2856 - Add ListServiceStatuses grpc method moby/swarmkit#2857 - Use Service Placement Constraints in Enforcer Signed-off-by: Drew Erny <drew.erny@docker.com> Upstream-commit: 67e25ec5ac568a893e444891a6a583fd2f996f76 Component: engine
- What I did
Add capabilities list to container specification like discussed on moby/moby#26849 (comment)
Second step to solve moby/moby#25885 , moby/moby#24862 and #1030
Replaces #1565
- How to test it
Run test container which prints capabilities to console
Add default capabilities
Remove capabilities:
- Description for the changelog
d9c62e1 bumps docker/docker to same version which docker/cli is using and docker/distribution to same version than is used by moby and docker/cli (and fixes those small incompabilities).
18d9170 contains actual implementation.