Replace service "Capabilities" w/ add/drop API

After dicussing with maintainers, it was decided putting the burden of
providing the full cap list on the client is not a good design.
Instead we decided to follow along with the container API and use cap
add/drop.

This brings in the changes already merged into swarmkit.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>

api/server/router/container/container_routes.go

@@ -489,9 +489,6 @@ func (s *containerRouter) postContainersCreate(ctx context.Context, w http.Respo
 		// Ignore KernelMemoryTCP because it was added in API 1.40.
 		hostConfig.KernelMemoryTCP = 0
 
-		// Ignore Capabilities because it was added in API 1.40.
-		hostConfig.Capabilities = nil
-
 		// Older clients (API < 1.40) expects the default to be shareable, make them happy
 		if hostConfig.IpcMode.IsEmpty() {
 			hostConfig.IpcMode = container.IpcMode("shareable")

api/server/router/swarm/helpers.go

@@ -99,7 +99,8 @@ func adjustForAPIVersion(cliVersion string, service *swarm.ServiceSpec) {
 		if service.TaskTemplate.ContainerSpec != nil {
 			// Capabilities for docker swarm services weren't
 			// supported before API version 1.41
-			service.TaskTemplate.ContainerSpec.Capabilities = nil
+			service.TaskTemplate.ContainerSpec.CapabilityAdd = nil
+			service.TaskTemplate.ContainerSpec.CapabilityDrop = nil
 		}
 		if service.TaskTemplate.Resources != nil && service.TaskTemplate.Resources.Limits != nil {
 			// Limits.Pids  not supported before API version 1.41

api/swagger.yaml

@@ -906,15 +906,6 @@ definitions:
               $ref: "#/definitions/Mount"
 
           # Applicable to UNIX platforms
-          Capabilities:
-            type: "array"
-            description: |
-              A list of kernel capabilities to be available for container (this
-              overrides the default set).
-
-              Conflicts with options 'CapAdd' and 'CapDrop'"
-            items:
-              type: "string"
           CapAdd:
             type: "array"
             description: |
@@ -3276,11 +3267,11 @@ definitions:
             additionalProperties:
               type: "string"
           # This option is not used by Windows containers
-          Capabilities:
+          CapabilityAdd:
             type: "array"
             description: |
-              A list of kernel capabilities to be available for container (this
-              overrides the default set).
+              A list of kernel capabilities to add to the default set
+              for the container.
             items:
               type: "string"
             example:
@@ -3288,6 +3279,15 @@ definitions:
               - "CAP_SYS_ADMIN"
               - "CAP_SYS_CHROOT"
               - "CAP_SYSLOG"
+          CapabilityDrop:
+            type: "array"
+            description: |
+              A list of kernel capabilities to drop from the default set
+              for the container.
+            items:
+              type: "string"
+            example:
+              - "CAP_NET_RAW"
       NetworkAttachmentSpec:
         description: |
           Read-only spec type for non-swarm containers attached to swarm overlay

api/types/container/host_config.go

@@ -403,7 +403,6 @@ type HostConfig struct {
 	// Applicable to UNIX platforms
 	CapAdd          strslice.StrSlice // List of kernel capabilities to add to the container
 	CapDrop         strslice.StrSlice // List of kernel capabilities to remove from the container
-	Capabilities    []string          `json:"Capabilities"` // List of kernel capabilities to be available for container (this overrides the default set)
 	CgroupnsMode    CgroupnsMode      // Cgroup namespace mode to use for the container
 	DNS             []string          `json:"Dns"`        // List of DNS server to lookup
 	DNSOptions      []string          `json:"DnsOptions"` // List of DNSOption to look for

api/types/swarm/container.go

@@ -67,11 +67,12 @@ type ContainerSpec struct {
 	// The format of extra hosts on swarmkit is specified in:
 	// http://man7.org/linux/man-pages/man5/hosts.5.html
 	//    IP_address canonical_hostname [aliases...]
-	Hosts        []string            `json:",omitempty"`
-	DNSConfig    *DNSConfig          `json:",omitempty"`
-	Secrets      []*SecretReference  `json:",omitempty"`
-	Configs      []*ConfigReference  `json:",omitempty"`
-	Isolation    container.Isolation `json:",omitempty"`
-	Sysctls      map[string]string   `json:",omitempty"`
-	Capabilities []string            `json:",omitempty"`
+	Hosts          []string            `json:",omitempty"`
+	DNSConfig      *DNSConfig          `json:",omitempty"`
+	Secrets        []*SecretReference  `json:",omitempty"`
+	Configs        []*ConfigReference  `json:",omitempty"`
+	Isolation      container.Isolation `json:",omitempty"`
+	Sysctls        map[string]string   `json:",omitempty"`
+	CapabilityAdd  []string            `json:",omitempty"`
+	CapabilityDrop []string            `json:",omitempty"`
 }

daemon/cluster/convert/container.go

@@ -18,26 +18,27 @@ func containerSpecFromGRPC(c *swarmapi.ContainerSpec) *types.ContainerSpec {
 		return nil
 	}
 	containerSpec := &types.ContainerSpec{
-		Image:        c.Image,
-		Labels:       c.Labels,
-		Command:      c.Command,
-		Args:         c.Args,
-		Hostname:     c.Hostname,
-		Env:          c.Env,
-		Dir:          c.Dir,
-		User:         c.User,
-		Groups:       c.Groups,
-		StopSignal:   c.StopSignal,
-		TTY:          c.TTY,
-		OpenStdin:    c.OpenStdin,
-		ReadOnly:     c.ReadOnly,
-		Hosts:        c.Hosts,
-		Secrets:      secretReferencesFromGRPC(c.Secrets),
-		Configs:      configReferencesFromGRPC(c.Configs),
-		Isolation:    IsolationFromGRPC(c.Isolation),
-		Init:         initFromGRPC(c.Init),
-		Sysctls:      c.Sysctls,
-		Capabilities: c.Capabilities,
+		Image:          c.Image,
+		Labels:         c.Labels,
+		Command:        c.Command,
+		Args:           c.Args,
+		Hostname:       c.Hostname,
+		Env:            c.Env,
+		Dir:            c.Dir,
+		User:           c.User,
+		Groups:         c.Groups,
+		StopSignal:     c.StopSignal,
+		TTY:            c.TTY,
+		OpenStdin:      c.OpenStdin,
+		ReadOnly:       c.ReadOnly,
+		Hosts:          c.Hosts,
+		Secrets:        secretReferencesFromGRPC(c.Secrets),
+		Configs:        configReferencesFromGRPC(c.Configs),
+		Isolation:      IsolationFromGRPC(c.Isolation),
+		Init:           initFromGRPC(c.Init),
+		Sysctls:        c.Sysctls,
+		CapabilityAdd:  c.CapabilityAdd,
+		CapabilityDrop: c.CapabilityDrop,
 	}
 
 	if c.DNSConfig != nil {
@@ -246,25 +247,26 @@ func configReferencesFromGRPC(sr []*swarmapi.ConfigReference) []*types.ConfigRef
 
 func containerToGRPC(c *types.ContainerSpec) (*swarmapi.ContainerSpec, error) {
 	containerSpec := &swarmapi.ContainerSpec{
-		Image:        c.Image,
-		Labels:       c.Labels,
-		Command:      c.Command,
-		Args:         c.Args,
-		Hostname:     c.Hostname,
-		Env:          c.Env,
-		Dir:          c.Dir,
-		User:         c.User,
-		Groups:       c.Groups,
-		StopSignal:   c.StopSignal,
-		TTY:          c.TTY,
-		OpenStdin:    c.OpenStdin,
-		ReadOnly:     c.ReadOnly,
-		Hosts:        c.Hosts,
-		Secrets:      secretReferencesToGRPC(c.Secrets),
-		Isolation:    isolationToGRPC(c.Isolation),
-		Init:         initToGRPC(c.Init),
-		Sysctls:      c.Sysctls,
-		Capabilities: c.Capabilities,
+		Image:          c.Image,
+		Labels:         c.Labels,
+		Command:        c.Command,
+		Args:           c.Args,
+		Hostname:       c.Hostname,
+		Env:            c.Env,
+		Dir:            c.Dir,
+		User:           c.User,
+		Groups:         c.Groups,
+		StopSignal:     c.StopSignal,
+		TTY:            c.TTY,
+		OpenStdin:      c.OpenStdin,
+		ReadOnly:       c.ReadOnly,
+		Hosts:          c.Hosts,
+		Secrets:        secretReferencesToGRPC(c.Secrets),
+		Isolation:      isolationToGRPC(c.Isolation),
+		Init:           initToGRPC(c.Init),
+		Sysctls:        c.Sysctls,
+		CapabilityAdd:  c.CapabilityAdd,
+		CapabilityDrop: c.CapabilityDrop,
 	}
 
 	if c.DNSConfig != nil {

daemon/cluster/executor/container/container.go

@@ -360,7 +360,8 @@ func (c *containerConfig) hostConfig() *enginecontainer.HostConfig {
 		Isolation:      c.isolation(),
 		Init:           c.init(),
 		Sysctls:        c.spec().Sysctls,
-		Capabilities:   c.spec().Capabilities,
+		CapAdd:         c.spec().CapabilityAdd,
+		CapDrop:        c.spec().CapabilityDrop,
 	}
 
 	if c.spec().DNSConfig != nil {

daemon/container.go

@@ -305,21 +305,12 @@ func validateHostConfig(hostConfig *containertypes.HostConfig, platform string)
 }
 
 func validateCapabilities(hostConfig *containertypes.HostConfig) error {
-	if len(hostConfig.CapAdd) > 0 && hostConfig.Capabilities != nil {
-		return errdefs.InvalidParameter(errors.Errorf("conflicting options: Capabilities and CapAdd"))
-	}
-	if len(hostConfig.CapDrop) > 0 && hostConfig.Capabilities != nil {
-		return errdefs.InvalidParameter(errors.Errorf("conflicting options: Capabilities and CapDrop"))
-	}
 	if _, err := caps.NormalizeLegacyCapabilities(hostConfig.CapAdd); err != nil {
 		return errors.Wrap(err, "invalid CapAdd")
 	}
 	if _, err := caps.NormalizeLegacyCapabilities(hostConfig.CapDrop); err != nil {
 		return errors.Wrap(err, "invalid CapDrop")
 	}
-	if err := caps.ValidateCapabilities(hostConfig.Capabilities); err != nil {
-		return errors.Wrap(err, "invalid Capabilities")
-	}
 	// TODO consider returning warnings if "Privileged" is combined with Capabilities, CapAdd and/or CapDrop
 	return nil
 }

daemon/oci_linux.go

@@ -162,7 +162,6 @@ func WithCapabilities(c *container.Container) coci.SpecOpts {
 			caps.DefaultCapabilities(),
 			c.HostConfig.CapAdd,
 			c.HostConfig.CapDrop,
-			c.HostConfig.Capabilities,
 			c.HostConfig.Privileged,
 		)
 		if err != nil {

daemon/oci_windows.go

@@ -390,7 +390,7 @@ func (daemon *Daemon) createSpecLinuxFields(c *container.Container, s *specs.Spe
 	// Note these are against the UVM.
 	setResourcesInSpec(c, s, true) // LCOW is Hyper-V only
 
-	capabilities, err := caps.TweakCapabilities(caps.DefaultCapabilities(), c.HostConfig.CapAdd, c.HostConfig.CapDrop, c.HostConfig.Capabilities, c.HostConfig.Privileged)
+	capabilities, err := caps.TweakCapabilities(caps.DefaultCapabilities(), c.HostConfig.CapAdd, c.HostConfig.CapDrop, c.HostConfig.Privileged)
 	if err != nil {
 		return fmt.Errorf("linux spec capabilities: %v", err)
 	}

docs/api/version-history.md

@@ -28,12 +28,12 @@ keywords: "API, Docker, rcli, REST, documentation"
 * The `filter` (singular) query parameter, which was deprecated in favor of the
   `filters` option in Docker 1.13, has now been removed from the `GET /images/json`
   endpoint. The parameter remains available when using API version 1.40 or below.
-* `GET /services` now returns `Capabilities` as part of the `ContainerSpec`.
-* `GET /services/{id}` now returns `Capabilities` as part of the `ContainerSpec`.
-* `POST /services/create` now accepts `Capabilities` as part of the `ContainerSpec`.
-* `POST /services/{id}/update` now accepts `Capabilities` as part of the `ContainerSpec`.
-* `GET /tasks` now  returns `Capabilities` as part of the `ContainerSpec`.
-* `GET /tasks/{id}` now  returns `Capabilities` as part of the `ContainerSpec`.
+* `GET /services` now returns `CappAdd` and `CapDrop` as part of the `ContainerSpec`.
+* `GET /services/{id}` now returns `CapAdd` and `CapDrop` as part of the `ContainerSpec`.
+* `POST /services/create` now accepts `CapAdd` and `CapDrop` as part of the `ContainerSpec`.
+* `POST /services/{id}/update` now accepts `CapAdd` and `CapDrop` as part of the `ContainerSpec`.
+* `GET /tasks` now  returns `CapAdd` and `CapDrop` as part of the `ContainerSpec`.
+* `GET /tasks/{id}` now  returns `CapAdd` and `CapDrop` as part of the `ContainerSpec`.
 * `GET /services` now returns `Pids` in `TaskTemplate.Resources.Limits`.
 * `GET /services/{id}` now returns `Pids` in `TaskTemplate.Resources.Limits`.
 * `POST /services/create` now accepts `Pids` in `TaskTemplate.Resources.Limits`.
@@ -135,11 +135,6 @@ keywords: "API, Docker, rcli, REST, documentation"
 * `GET /service/{id}` now  returns `MaxReplicas` as part of the `Placement`.
 * `POST /service/create` and `POST /services/(id or name)/update` now take the field `MaxReplicas`
   as part of the service `Placement`, allowing to specify maximum replicas per node for the service.
-* `GET /containers` now returns `Capabilities` field as part of the `HostConfig`.
-* `GET /containers/{id}/json` now returns a `Capabilities` field as part of the `HostConfig`.
-* `POST /containers/create` now takes a `Capabilities` field to set the list of
-  kernel capabilities to be available for the container (this overrides the default
-  set).
 * `POST /containers/create` on Linux now creates a container with `HostConfig.IpcMode=private`
   by default, if IpcMode is not explicitly specified. The per-daemon default can be changed
   back to `shareable` by using `DefaultIpcMode` daemon configuration parameter.

integration/container/create_test.go

@@ -17,7 +17,6 @@ import (
 	"github.com/docker/docker/errdefs"
 	ctr "github.com/docker/docker/integration/internal/container"
 	"github.com/docker/docker/oci"
-	"github.com/docker/docker/testutil/request"
 	specs "github.com/opencontainers/image-spec/specs-go/v1"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
@@ -258,133 +257,6 @@ func TestCreateWithCustomMaskedPaths(t *testing.T) {
 	}
 }
 
-func TestCreateWithCapabilities(t *testing.T) {
-	skip.If(t, testEnv.DaemonInfo.OSType == "windows", "FIXME: test should be able to run on LCOW")
-	skip.If(t, versions.LessThan(testEnv.DaemonAPIVersion(), "1.40"), "Capabilities was added in API v1.40")
-
-	defer setupTest(t)()
-	ctx := context.Background()
-	clientNew := request.NewAPIClient(t)
-	clientOld := request.NewAPIClient(t, client.WithVersion("1.39"))
-
-	testCases := []struct {
-		doc           string
-		hostConfig    container.HostConfig
-		expected      []string
-		expectedError string
-		oldClient     bool
-	}{
-		{
-			doc:        "no capabilities",
-			hostConfig: container.HostConfig{},
-		},
-		{
-			doc: "empty capabilities",
-			hostConfig: container.HostConfig{
-				Capabilities: []string{},
-			},
-			expected: []string{},
-		},
-		{
-			doc: "valid capabilities",
-			hostConfig: container.HostConfig{
-				Capabilities: []string{"CAP_NET_RAW", "CAP_SYS_CHROOT"},
-			},
-			expected: []string{"CAP_NET_RAW", "CAP_SYS_CHROOT"},
-		},
-		{
-			doc: "invalid capabilities",
-			hostConfig: container.HostConfig{
-				Capabilities: []string{"NET_RAW"},
-			},
-			expectedError: `invalid Capabilities: unknown capability: "NET_RAW"`,
-		},
-		{
-			doc: "duplicate capabilities",
-			hostConfig: container.HostConfig{
-				Capabilities: []string{"CAP_SYS_NICE", "CAP_SYS_NICE"},
-			},
-			expected: []string{"CAP_SYS_NICE", "CAP_SYS_NICE"},
-		},
-		{
-			doc: "capabilities API v1.39",
-			hostConfig: container.HostConfig{
-				Capabilities: []string{"CAP_NET_RAW", "CAP_SYS_CHROOT"},
-			},
-			expected:  nil,
-			oldClient: true,
-		},
-		{
-			doc: "empty capadd",
-			hostConfig: container.HostConfig{
-				Capabilities: []string{"CAP_NET_ADMIN"},
-				CapAdd:       []string{},
-			},
-			expected: []string{"CAP_NET_ADMIN"},
-		},
-		{
-			doc: "empty capdrop",
-			hostConfig: container.HostConfig{
-				Capabilities: []string{"CAP_NET_ADMIN"},
-				CapDrop:      []string{},
-			},
-			expected: []string{"CAP_NET_ADMIN"},
-		},
-		{
-			doc: "capadd capdrop",
-			hostConfig: container.HostConfig{
-				CapAdd:  []string{"SYS_NICE", "CAP_SYS_NICE"},
-				CapDrop: []string{"SYS_NICE", "CAP_SYS_NICE"},
-			},
-		},
-		{
-			doc: "conflict with capadd",
-			hostConfig: container.HostConfig{
-				Capabilities: []string{"CAP_NET_ADMIN"},
-				CapAdd:       []string{"SYS_NICE"},
-			},
-			expectedError: `conflicting options: Capabilities and CapAdd`,
-		},
-		{
-			doc: "conflict with capdrop",
-			hostConfig: container.HostConfig{
-				Capabilities: []string{"CAP_NET_ADMIN"},
-				CapDrop:      []string{"NET_RAW"},
-			},
-			expectedError: `conflicting options: Capabilities and CapDrop`,
-		},
-	}
-
-	for _, tc := range testCases {
-		tc := tc
-		t.Run(tc.doc, func(t *testing.T) {
-			t.Parallel()
-			client := clientNew
-			if tc.oldClient {
-				client = clientOld
-			}
-
-			c, err := client.ContainerCreate(context.Background(),
-				&container.Config{Image: "busybox"},
-				&tc.hostConfig,
-				&network.NetworkingConfig{},
-				nil,
-				"",
-			)
-			if tc.expectedError == "" {
-				assert.NilError(t, err)
-				ci, err := client.ContainerInspect(ctx, c.ID)
-				assert.NilError(t, err)
-				assert.Check(t, ci.HostConfig != nil)
-				assert.DeepEqual(t, tc.expected, ci.HostConfig.Capabilities)
-			} else {
-				assert.ErrorContains(t, err, tc.expectedError)
-				assert.Check(t, errdefs.IsInvalidParameter(err))
-			}
-		})
-	}
-}
-
 func TestCreateWithCustomReadonlyPaths(t *testing.T) {
 	skip.If(t, testEnv.DaemonInfo.OSType != "linux")
 

integration/internal/swarm/service.go

@@ -189,10 +189,11 @@ func ServiceWithSysctls(sysctls map[string]string) ServiceSpecOpt {
 }
 
 // ServiceWithCapabilities sets the Capabilities option of the service's ContainerSpec.
-func ServiceWithCapabilities(Capabilities []string) ServiceSpecOpt {
+func ServiceWithCapabilities(add []string, drop []string) ServiceSpecOpt {
 	return func(spec *swarmtypes.ServiceSpec) {
 		ensureContainerSpec(spec)
-		spec.TaskTemplate.ContainerSpec.Capabilities = Capabilities
+		spec.TaskTemplate.ContainerSpec.CapabilityAdd = add
+		spec.TaskTemplate.ContainerSpec.CapabilityDrop = drop
 	}
 }
 

integration/service/create_test.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/filters"
+	"github.com/docker/docker/api/types/strslice"
 	swarmtypes "github.com/docker/docker/api/types/swarm"
 	"github.com/docker/docker/api/types/versions"
 	"github.com/docker/docker/client"
@@ -492,12 +493,13 @@ func TestCreateServiceCapabilities(t *testing.T) {
 	ctx := context.Background()
 
 	// store the map we're going to be using everywhere.
-	expectedCapabilities := []string{"CAP_NET_RAW", "CAP_SYS_CHROOT"}
+	capAdd := []string{"CAP_SYS_CHROOT"}
+	capDrop := []string{"CAP_NET_RAW"}
 
 	// Create the service with the capabilities options
 	var instances uint64 = 1
 	serviceID := swarm.CreateService(t, d,
-		swarm.ServiceWithCapabilities(expectedCapabilities),
+		swarm.ServiceWithCapabilities(capAdd, capDrop),
 	)
 
 	// wait for the service to converge to 1 running task as expected
@@ -529,15 +531,16 @@ func TestCreateServiceCapabilities(t *testing.T) {
 	// verify that the container has the capabilities option set
 	ctnr, err := client.ContainerInspect(ctx, tasks[0].Status.ContainerStatus.ContainerID)
 	assert.NilError(t, err)
-	assert.DeepEqual(t, ctnr.HostConfig.Capabilities, expectedCapabilities)
+	assert.DeepEqual(t, ctnr.HostConfig.CapAdd, strslice.StrSlice(capAdd))
+	assert.DeepEqual(t, ctnr.HostConfig.CapDrop, strslice.StrSlice(capDrop))
 
 	// verify that the task has the capabilities option set in the task object
-	assert.DeepEqual(t, tasks[0].Spec.ContainerSpec.Capabilities, expectedCapabilities)
+	assert.DeepEqual(t, tasks[0].Spec.ContainerSpec.CapabilityAdd, capAdd)
+	assert.DeepEqual(t, tasks[0].Spec.ContainerSpec.CapabilityDrop, capDrop)
 
 	// verify that the service also has the capabilities set in the spec.
 	service, _, err := client.ServiceInspectWithRaw(ctx, serviceID, types.ServiceInspectOptions{})
 	assert.NilError(t, err)
-	assert.DeepEqual(t,
-		service.Spec.TaskTemplate.ContainerSpec.Capabilities, expectedCapabilities,
-	)
+	assert.DeepEqual(t, service.Spec.TaskTemplate.ContainerSpec.CapabilityAdd, capAdd)
+	assert.DeepEqual(t, service.Spec.TaskTemplate.ContainerSpec.CapabilityDrop, capDrop)
 }

oci/caps/utils.go

@@ -117,17 +117,11 @@ func ValidateCapabilities(caps []string) error {
 
 // TweakCapabilities tweaks capabilities by adding, dropping, or overriding
 // capabilities in the basics capabilities list.
-func TweakCapabilities(basics, adds, drops, capabilities []string, privileged bool) ([]string, error) {
+func TweakCapabilities(basics, adds, drops []string, privileged bool) ([]string, error) {
 	switch {
 	case privileged:
 		// Privileged containers get all capabilities
 		return GetAllCapabilities(), nil
-	case capabilities != nil:
-		// Use custom set of capabilities
-		if err := ValidateCapabilities(capabilities); err != nil {
-			return nil, err
-		}
-		return capabilities, nil
 	case len(adds) == 0 && len(drops) == 0:
 		// Nothing to tweak; we're done
 		return basics, nil