Add capabilities support to stack/service commands

[akerouanton]

closes #1940
closes #2199

- What I did

This PR supersedes #1940 and #2199. I removed the support of privileged option from the original PR as I believe privileged containers involve more than just having the complete set of capabilities (eg. no user namespace, the unconfined AppArmor profile is used, etc...).

1. docker stack deploy now supports cap_add and cap_drop options ;
2. docker service create|update now support --cap-add and --cap-drop flags. For the update command and unlike most of other options, those flags don't have -add & -rm variants as this would make a poor UX (eg. --cap-add-add & --cap-drop-rm). Instead, updateCapabilities takes care of stripping caps provided to --cap-add from CapabilityDrop (and vice versa) ;
3. docker service inspect --pretty displays the list of added caps and dropped caps ;

Closes moby/moby#25885 #1940 #2199

- How I did it

1. The service converter transforming compose types into API types has been updated to add capabilities to ServiceSpec;
2. A new function with the logic described above and named updateCapabilities() has been introduced and is called by updateService ;
3. The pretty formatter has been updated to include added and dropped caps ;

- How to verify it

docker stack deploy

$ cat docker-compose.yaml
version: "3.7"
services:
  test:
    image: ollijanatuinen/capsh
    cap_add:
      - NET_ADMIN
    cap_drop:
      - CAP_MKNOD
$ build/docker stack deploy --compose-file docker-compose.yaml t1
$ docker inspect t1_test.1.m0jmdufuo93fotyyrhm00xx16
        "CapAdd": [
            "NET_ADMIN"
        ],
        "CapDrop": [
            "CAP_MKNOD"
        ],

docker service create

$ build/docker service create --name test --cap-add NET_ADMIN --cap-drop CAP_MKNOD ollijanatuinen/capsh
$ docker inspect test.1.li7llrgyp8v4mfvo5tvimhzxt
        "CapAdd": [
            "NET_ADMIN"
        ],
        "CapDrop": [
            "CAP_MKNOD"
        ],

docker service update

$ build/docker service update --cap-add CAP_MKNOD t1_test
$ docker inspect t1_test.1.92sd9omniksbenuv6wm17ft78
        "CapAdd": [
            "NET_ADMIN",
            "CAP_MKNOD"
        ],
        "CapDrop": null,

$ build/docker service update --cap-drop CAP_MKNOD --cap-drop NET_ADMIN t1_test
        "CapAdd": null,
        "CapDrop": [
            "CAP_MKNOD",
            "NET_ADMIN"
        ],

docker service inspect

$ build/docker service inspect t1_test
        "CapabilityAdd": [
            "NET_ADMIN"
        ],
        "CapabilityDrop": [
            "CAP_MKNOD"
        ]
$ build/docker service inspect --pretty t1_test
Capabilities:
 Add: NET_ADMIN
 Drop: CAP_MKNOD

- Description for the changelog

Add cap_add and cap_drop support to docker stack deploy and --cap-add and --cap-drop flags to docker service create|update.

- A picture of a cute animal (not mandatory but encouraged)

[codecov-commenter]

Codecov Report

Merging #2663 into master will increase coverage by 0.04%.
The diff coverage is 80.48%.

@@            Coverage Diff             @@
##           master    #2663      +/-   ##
==========================================
+ Coverage   58.14%   58.18%   +0.04%     
==========================================
  Files         295      295              
  Lines       21198    21239      +41     
==========================================
+ Hits        12325    12358      +33     
- Misses       7966     7974       +8     
  Partials      907      907              

[koolay]

Greate job!

[silvin-lubecki]

LGTM 👍

[thaJeztah]

Sorry for the long wait for me to review this one. I kept working on a review, but the combinations of "cap-add" / "cap-drop", in addition to updating is rather complicated. I started work on some updates/suggestions, but they became somewhat to big to leave as review comments, so instead I created a branch with those changes, and opened them as a "carry" pull request; #2687

Given the complexity of the logic, it won't hurt to have more eyes on those changes to verify it's working as expected, so feel free to help reviewing those changes 😅 🤗

[thaJeztah]

We may have to sort CapabilityAdd and CapabilityDrop. Order of these should not matter, and if capabilities are updated and end up in a different order, this will likely cause service-tasks to be updated/re-deployed, causing churn where it's not needed.

(Possibly the same will be needed for the docker stack deploy code-paths)

For swarm services, we generally want the CLI/client to generate the spec and handle that spec with no (or very little) modifications on the server-side. The SwarmKit reconciliation loop will look for any change in the Service Spec to consider if a service needs updating or not.

Currently, it the CLI does not do any normalization or sorting, which will mean that;

• duplicate values can occur
• capabilities are not sorted; order of capabilities in the list does not matter for the container itself, but a change in order of capabilities will result in all tasks for a service to be re-deployed (in the docker service update case), causing unneeded churn in the service
• capabilities are included without normalization, so both CAP_CHOWN and CHOWN can be added, and preserves case (both cap_chown, CAP_CHOWN and cAp_cHoWn are accepted)

To prevent unneeded re-creation of tasks I think we should both normalize capabilities, remove duplicates, and sort them alphabetically.

Possibly we could re-use NormalizeLegacyCapabilities, but that may need some work, as I don't think it should validate on the client side, if possible, and it's currently not de-duplicating. A similar implementation could be done on the client side though

Note: this is something we should fix for docker run as well (but that's something to handle separately).

I noticed this when trying using the following;

docker service create --detach --tty --cap-drop cAp_cHoWn --cap-drop CAP_CHOWN --cap-drop ChOwN --cap-drop CHOWN --name foo busybox

docker service inspect --format '{{json .Spec.TaskTemplate.ContainerSpec.CapabilityDrop }}' foo | jq .
[
  "cAp_cHoWn",
  "CAP_CHOWN",
  "ChOwN",
  "CHOWN"
]

[thaJeztah]

Probably should be checked if any changed to prevent churn;

Suggested change

	updateCapabilities(flags, cspec)
	if anyChanged(flags, flagCapAdd, flagCapDrop) {
	updateCapabilities(flags, cspec)
	}

[thaJeztah]

Looks like this is inconsistent with how docker run handles the combination of add/drop, where add takes precedence;

# default capabilities
docker run -it --rm busybox sh -c 'touch foo && chown 123:456 foo && echo succesfully chowned'
# succesfully chowned

# dropping CAP_CHOWN
docker run -it --rm --cap-drop CAP_CHOWN busybox sh -c 'touch foo && chown 123:456 foo && echo succesfully chowned'
# chown: foo: Operation not permitted

# dropping *and* adding CAP_CHOWN
docker run -it --rm --cap-add CAP_CHOWN --cap-drop CAP_CHOWN busybox sh -c 'touch foo && chown 123:456 foo && echo succesfully chowned'
# succesfully chowned

note in docker run / docker create, duplicates are not removed (we should fix that for consistency)

docker create --name=test --cap-add CAP_CHOWN --cap-add CAP_CHOWN --cap-drop CAP_CHOWN --cap-drop CAP_CHOWN busybox
docker inspect --format 'CapAdd: {{json .HostConfig.CapAdd}} CapDrop: {{json .HostConfig.CapDrop}}' test
# CapAdd: ["CAP_CHOWN","CAP_CHOWN"] CapDrop: ["CAP_CHOWN","CAP_CHOWN"]

Unfortunately, this is all rather involved, but what I think should happen is that;

• both existing (service spec) and new (values passed through flags or in the compose-file) should be normalized and de-duplicated before use.
• the special "ALL" capability is equivalent to "all capabilities" and taken into account when normalizing capabilities. Combining "ALL" capabilities and other capabilities should be equivalent to just specifying "ALL".
• adding capabilities takes precedence over dropping, which means that if a capability is both set to be "dropped" and to be "added", it should be removed from the list to "drop". This also implies that adding "ALL" capabilities defeats any capability that was added to be "dropped" (and thus equivalent to an empty "drop" list).
• the final lists should be sorted and normalized to reduce service churn
• Except for special handling of the "ALL" value, no validation of capabilities should be handled by the client. Validation should delegated to the daemon/server where possible.

When deploying a service using a docker-compose file, the docker-compose file is mostly handled as being "declarative". However, many of the issues outlined above also apply to compose-files, so similar handling is applied to compose files as well to prevent service churn.