diff --git a/api/server/router/swarm/cluster_routes.go b/api/server/router/swarm/cluster_routes.go index 0966d3a5a4..ef4157bd8a 100644 --- a/api/server/router/swarm/cluster_routes.go +++ b/api/server/router/swarm/cluster_routes.go @@ -213,24 +213,7 @@ func (sr *swarmRouter) createService(ctx context.Context, w http.ResponseWriter, if versions.LessThan(cliVersion, "1.30") { queryRegistry = true } - if versions.LessThan(cliVersion, "1.40") { - if service.TaskTemplate.ContainerSpec != nil { - // Sysctls for docker swarm services weren't supported before - // API version 1.40 - service.TaskTemplate.ContainerSpec.Sysctls = nil - - if service.TaskTemplate.ContainerSpec.Privileges != nil && service.TaskTemplate.ContainerSpec.Privileges.CredentialSpec != nil { - // Support for setting credential-spec through configs was added in API 1.40 - service.TaskTemplate.ContainerSpec.Privileges.CredentialSpec.Config = "" - } - } - - if service.TaskTemplate.Placement != nil { - // MaxReplicas for docker swarm services weren't supported before - // API version 1.40 - service.TaskTemplate.Placement.MaxReplicas = 0 - } - } + adjustForAPIVersion(cliVersion, &service) } resp, err := sr.backend.CreateService(service, encodedAuth, queryRegistry) @@ -270,24 +253,7 @@ func (sr *swarmRouter) updateService(ctx context.Context, w http.ResponseWriter, if versions.LessThan(cliVersion, "1.30") { queryRegistry = true } - if versions.LessThan(cliVersion, "1.40") { - if service.TaskTemplate.ContainerSpec != nil { - // Sysctls for docker swarm services weren't supported before - // API version 1.40 - service.TaskTemplate.ContainerSpec.Sysctls = nil - - if service.TaskTemplate.ContainerSpec.Privileges != nil && service.TaskTemplate.ContainerSpec.Privileges.CredentialSpec != nil { - // Support for setting credential-spec through configs was added in API 1.40 - service.TaskTemplate.ContainerSpec.Privileges.CredentialSpec.Config = "" - } - } - - if service.TaskTemplate.Placement != nil { - // MaxReplicas for docker swarm services weren't supported before - // API version 1.40 - service.TaskTemplate.Placement.MaxReplicas = 0 - } - } + adjustForAPIVersion(cliVersion, &service) } resp, err := sr.backend.UpdateService(vars["id"], version, service, flags, queryRegistry) diff --git a/api/server/router/swarm/helpers.go b/api/server/router/swarm/helpers.go index 1f57074f92..3120e8f8f2 100644 --- a/api/server/router/swarm/helpers.go +++ b/api/server/router/swarm/helpers.go @@ -9,6 +9,8 @@ import ( "github.com/docker/docker/api/server/httputils" basictypes "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/backend" + "github.com/docker/docker/api/types/swarm" + "github.com/docker/docker/api/types/versions" ) // swarmLogs takes an http response, request, and selector, and writes the logs @@ -64,3 +66,33 @@ func (sr *swarmRouter) swarmLogs(ctx context.Context, w io.Writer, r *http.Reque httputils.WriteLogStream(ctx, w, msgs, logsConfig, !tty) return nil } + +// adjustForAPIVersion takes a version and service spec and removes fields to +// make the spec compatible with the specified version. +func adjustForAPIVersion(cliVersion string, service *swarm.ServiceSpec) { + if cliVersion == "" { + return + } + if versions.LessThan(cliVersion, "1.40") { + if service.TaskTemplate.ContainerSpec != nil { + // Sysctls for docker swarm services weren't supported before + // API version 1.40 + service.TaskTemplate.ContainerSpec.Sysctls = nil + + if service.TaskTemplate.ContainerSpec.Privileges != nil && service.TaskTemplate.ContainerSpec.Privileges.CredentialSpec != nil { + // Support for setting credential-spec through configs was added in API 1.40 + service.TaskTemplate.ContainerSpec.Privileges.CredentialSpec.Config = "" + } + for _, config := range service.TaskTemplate.ContainerSpec.Configs { + // support for the Runtime target was added in API 1.40 + config.Runtime = nil + } + } + + if service.TaskTemplate.Placement != nil { + // MaxReplicas for docker swarm services weren't supported before + // API version 1.40 + service.TaskTemplate.Placement.MaxReplicas = 0 + } + } +} diff --git a/api/server/router/swarm/helpers_test.go b/api/server/router/swarm/helpers_test.go new file mode 100644 index 0000000000..08764e5a17 --- /dev/null +++ b/api/server/router/swarm/helpers_test.go @@ -0,0 +1,87 @@ +package swarm // import "github.com/docker/docker/api/server/router/swarm" + +import ( + "reflect" + "testing" + + "github.com/docker/docker/api/types/swarm" +) + +func TestAdjustForAPIVersion(t *testing.T) { + var ( + expectedSysctls = map[string]string{"foo": "bar"} + ) + // testing the negative -- does this leave everything else alone? -- is + // prohibitively time-consuming to write, because it would need an object + // with literally every field filled in. + spec := &swarm.ServiceSpec{ + TaskTemplate: swarm.TaskSpec{ + ContainerSpec: &swarm.ContainerSpec{ + Sysctls: expectedSysctls, + Privileges: &swarm.Privileges{ + CredentialSpec: &swarm.CredentialSpec{ + Config: "someconfig", + }, + }, + Configs: []*swarm.ConfigReference{ + { + File: &swarm.ConfigReferenceFileTarget{ + Name: "foo", + UID: "bar", + GID: "baz", + }, + ConfigID: "configFile", + ConfigName: "configFile", + }, + { + Runtime: &swarm.ConfigReferenceRuntimeTarget{}, + ConfigID: "configRuntime", + ConfigName: "configRuntime", + }, + }, + }, + Placement: &swarm.Placement{ + MaxReplicas: 222, + }, + }, + } + + // first, does calling this with a later version correctly NOT strip + // fields? do the later version first, so we can reuse this spec in the + // next test. + adjustForAPIVersion("1.40", spec) + if !reflect.DeepEqual(spec.TaskTemplate.ContainerSpec.Sysctls, expectedSysctls) { + t.Error("Sysctls was stripped from spec") + } + + if spec.TaskTemplate.ContainerSpec.Privileges.CredentialSpec.Config != "someconfig" { + t.Error("CredentialSpec.Config field was stripped from spec") + } + + if spec.TaskTemplate.ContainerSpec.Configs[1].Runtime == nil { + t.Error("ConfigReferenceRuntimeTarget was stripped from spec") + } + + if spec.TaskTemplate.Placement.MaxReplicas != 222 { + t.Error("MaxReplicas was stripped from spec") + } + + // next, does calling this with an earlier version correctly strip fields? + adjustForAPIVersion("1.29", spec) + if spec.TaskTemplate.ContainerSpec.Sysctls != nil { + t.Error("Sysctls was not stripped from spec") + } + + if spec.TaskTemplate.ContainerSpec.Privileges.CredentialSpec.Config != "" { + t.Error("CredentialSpec.Config field was not stripped from spec") + } + + if spec.TaskTemplate.ContainerSpec.Configs[1].Runtime != nil { + t.Error("ConfigReferenceRuntimeTarget was not stripped from spec") + } + + if spec.TaskTemplate.Placement.MaxReplicas != 0 { + t.Error("MaxReplicas was not stripped from spec") + } + +} diff --git a/api/swagger.yaml b/api/swagger.yaml index 124494b364..514077ff7f 100644 --- a/api/swagger.yaml +++ b/api/swagger.yaml @@ -2628,6 +2628,7 @@ definitions: example: "0bt9dmxjvjiqermk6xrop3ekq" description: | Load credential spec from a Swarm Config with the given ID. + The specified config must also be present in the Configs field with the Runtime property set.


@@ -2768,7 +2769,12 @@ definitions: type: "object" properties: File: - description: "File represents a specific target that is backed by a file." + description: | + File represents a specific target that is backed by a file. + +


+ + > **Note**: `Configs.File` and `Configs.Runtime` are mutually exclusive type: "object" properties: Name: @@ -2784,6 +2790,14 @@ definitions: description: "Mode represents the FileMode of the file." type: "integer" format: "uint32" + Runtime: + description: | + Runtime represents a target that is not mounted into the container but is used by the task + +


+ + > **Note**: `Configs.File` and `Configs.Runtime` are mutually exclusive + type: "object" ConfigID: description: "ConfigID represents the ID of the specific config that we're referencing." type: "string" diff --git a/api/types/swarm/config.go b/api/types/swarm/config.go index a1555cf43e..16202ccce6 100644 --- a/api/types/swarm/config.go +++ b/api/types/swarm/config.go @@ -27,9 +27,14 @@ type ConfigReferenceFileTarget struct { Mode os.FileMode } +// ConfigReferenceRuntimeTarget is a target for a config specifying that it +// isn't mounted into the container but instead has some other purpose. +type ConfigReferenceRuntimeTarget struct{} + // ConfigReference is a reference to a config in swarm type ConfigReference struct { - File *ConfigReferenceFileTarget + File *ConfigReferenceFileTarget `json:",omitempty"` + Runtime *ConfigReferenceRuntimeTarget `json:",omitempty"` ConfigID string ConfigName string } diff --git a/daemon/cluster/convert/container.go b/daemon/cluster/convert/container.go index 52e2d0ec61..c3eb28df54 100644 --- a/daemon/cluster/convert/container.go +++ b/daemon/cluster/convert/container.go @@ -178,14 +178,26 @@ func secretReferencesFromGRPC(sr []*swarmapi.SecretReference) []*types.SecretRef return refs } -func configReferencesToGRPC(sr []*types.ConfigReference) []*swarmapi.ConfigReference { +func configReferencesToGRPC(sr []*types.ConfigReference) ([]*swarmapi.ConfigReference, error) { refs := make([]*swarmapi.ConfigReference, 0, len(sr)) for _, s := range sr { ref := &swarmapi.ConfigReference{ ConfigID: s.ConfigID, ConfigName: s.ConfigName, } - if s.File != nil { + switch { + case s.Runtime == nil && s.File == nil: + return nil, errors.New("either File or Runtime should be set") + case s.Runtime != nil && s.File != nil: + return nil, errors.New("cannot specify both File and Runtime") + case s.Runtime != nil: + // Runtime target was added in API v1.40 and takes precedence over + // File target. However, File and Runtime targets are mutually exclusive, + // so we should never have both. + ref.Target = &swarmapi.ConfigReference_Runtime{ + Runtime: &swarmapi.RuntimeTarget{}, + } + case s.File != nil: ref.Target = &swarmapi.ConfigReference_File{ File: &swarmapi.FileTarget{ Name: s.File.Name, @@ -199,28 +211,32 @@ func configReferencesToGRPC(sr []*types.ConfigReference) []*swarmapi.ConfigRefer refs = append(refs, ref) } - return refs + return refs, nil } func configReferencesFromGRPC(sr []*swarmapi.ConfigReference) []*types.ConfigReference { refs := make([]*types.ConfigReference, 0, len(sr)) for _, s := range sr { - target := s.GetFile() - if target == nil { - // not a file target - logrus.Warnf("config target not a file: config=%s", s.ConfigID) - continue + + r := &types.ConfigReference{ + ConfigID: s.ConfigID, + ConfigName: s.ConfigName, } - refs = append(refs, &types.ConfigReference{ - File: &types.ConfigReferenceFileTarget{ + if target := s.GetRuntime(); target != nil { + r.Runtime = &types.ConfigReferenceRuntimeTarget{} + } else if target := s.GetFile(); target != nil { + r.File = &types.ConfigReferenceFileTarget{ Name: target.Name, UID: target.UID, GID: target.GID, Mode: target.Mode, - }, - ConfigID: s.ConfigID, - ConfigName: s.ConfigName, - }) + } + } else { + // not a file target + logrus.Warnf("config target not known: config=%s", s.ConfigID) + continue + } + refs = append(refs, r) } return refs @@ -243,7 +259,6 @@ func containerToGRPC(c *types.ContainerSpec) (*swarmapi.ContainerSpec, error) { ReadOnly: c.ReadOnly, Hosts: c.Hosts, Secrets: secretReferencesToGRPC(c.Secrets), - Configs: configReferencesToGRPC(c.Configs), Isolation: isolationToGRPC(c.Isolation), Init: initToGRPC(c.Init), Sysctls: c.Sysctls, @@ -284,6 +299,14 @@ func containerToGRPC(c *types.ContainerSpec) (*swarmapi.ContainerSpec, error) { } } + if c.Configs != nil { + configs, err := configReferencesToGRPC(c.Configs) + if err != nil { + return nil, errors.Wrap(err, "invalid Config") + } + containerSpec.Configs = configs + } + // Mounts for _, m := range c.Mounts { mount := swarmapi.Mount{ diff --git a/daemon/cluster/convert/service_test.go b/daemon/cluster/convert/service_test.go index a989f391b9..9c4284bd71 100644 --- a/daemon/cluster/convert/service_test.go +++ b/daemon/cluster/convert/service_test.go @@ -467,3 +467,147 @@ func TestTaskConvertFromGRPCNetworkAttachment(t *testing.T) { t.Fatalf("expected Runtime to be %v", swarmtypes.RuntimeNetworkAttachment) } } + +// TestServiceConvertFromGRPCConfigs tests that converting config references +// from GRPC is correct +func TestServiceConvertFromGRPCConfigs(t *testing.T) { + cases := []struct { + name string + from *swarmapi.ConfigReference + to *swarmtypes.ConfigReference + }{ + { + name: "file", + from: &swarmapi.ConfigReference{ + ConfigID: "configFile", + ConfigName: "configFile", + Target: &swarmapi.ConfigReference_File{ + // skip mode, if everything else here works mode will too. otherwise we'd need to import os. + File: &swarmapi.FileTarget{Name: "foo", UID: "bar", GID: "baz"}, + }, + }, + to: &swarmtypes.ConfigReference{ + ConfigID: "configFile", + ConfigName: "configFile", + File: &swarmtypes.ConfigReferenceFileTarget{Name: "foo", UID: "bar", GID: "baz"}, + }, + }, + { + name: "runtime", + from: &swarmapi.ConfigReference{ + ConfigID: "configRuntime", + ConfigName: "configRuntime", + Target: &swarmapi.ConfigReference_Runtime{Runtime: &swarmapi.RuntimeTarget{}}, + }, + to: &swarmtypes.ConfigReference{ + ConfigID: "configRuntime", + ConfigName: "configRuntime", + Runtime: &swarmtypes.ConfigReferenceRuntimeTarget{}, + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + grpcService := swarmapi.Service{ + Spec: swarmapi.ServiceSpec{ + Task: swarmapi.TaskSpec{ + Runtime: &swarmapi.TaskSpec_Container{ + Container: &swarmapi.ContainerSpec{ + Configs: []*swarmapi.ConfigReference{tc.from}, + }, + }, + }, + }, + } + + engineService, err := ServiceFromGRPC(grpcService) + assert.NilError(t, err) + assert.DeepEqual(t, + engineService.Spec.TaskTemplate.ContainerSpec.Configs[0], + tc.to, + ) + }) + } +} + +// TestServiceConvertToGRPCConfigs tests that converting config references to +// GRPC is correct +func TestServiceConvertToGRPCConfigs(t *testing.T) { + cases := []struct { + name string + from *swarmtypes.ConfigReference + to *swarmapi.ConfigReference + expectedErr string + }{ + { + name: "file", + from: &swarmtypes.ConfigReference{ + ConfigID: "configFile", + ConfigName: "configFile", + File: &swarmtypes.ConfigReferenceFileTarget{Name: "foo", UID: "bar", GID: "baz"}, + }, + to: &swarmapi.ConfigReference{ + ConfigID: "configFile", + ConfigName: "configFile", + Target: &swarmapi.ConfigReference_File{ + // skip mode, if everything else here works mode will too. otherwise we'd need to import os. + File: &swarmapi.FileTarget{Name: "foo", UID: "bar", GID: "baz"}, + }, + }, + }, + { + name: "runtime", + from: &swarmtypes.ConfigReference{ + ConfigID: "configRuntime", + ConfigName: "configRuntime", + Runtime: &swarmtypes.ConfigReferenceRuntimeTarget{}, + }, + to: &swarmapi.ConfigReference{ + ConfigID: "configRuntime", + ConfigName: "configRuntime", + Target: &swarmapi.ConfigReference_Runtime{Runtime: &swarmapi.RuntimeTarget{}}, + }, + }, + { + name: "file and runtime", + from: &swarmtypes.ConfigReference{ + ConfigID: "fileAndRuntime", + ConfigName: "fileAndRuntime", + File: &swarmtypes.ConfigReferenceFileTarget{}, + Runtime: &swarmtypes.ConfigReferenceRuntimeTarget{}, + }, + expectedErr: "invalid Config: cannot specify both File and Runtime", + }, + { + name: "none", + from: &swarmtypes.ConfigReference{ + ConfigID: "none", + ConfigName: "none", + }, + expectedErr: "invalid Config: either File or Runtime should be set", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + engineServiceSpec := swarmtypes.ServiceSpec{ + TaskTemplate: swarmtypes.TaskSpec{ + ContainerSpec: &swarmtypes.ContainerSpec{ + Configs: []*swarmtypes.ConfigReference{tc.from}, + }, + }, + } + + grpcServiceSpec, err := ServiceSpecToGRPC(engineServiceSpec) + if tc.expectedErr != "" { + assert.Error(t, err, tc.expectedErr) + return + } + + assert.NilError(t, err) + taskRuntime := grpcServiceSpec.Task.Runtime.(*swarmapi.TaskSpec_Container) + assert.DeepEqual(t, taskRuntime.Container.Configs[0], tc.to) + }) + } +} diff --git a/daemon/container_operations_unix.go b/daemon/container_operations_unix.go index 5552d09df3..c7984f7a07 100644 --- a/daemon/container_operations_unix.go +++ b/daemon/container_operations_unix.go @@ -230,7 +230,14 @@ func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) { for _, ref := range c.ConfigReferences { // TODO (ehazlett): use type switch when more are supported if ref.File == nil { - logrus.Error("config target type is not a file target") + // Runtime configs are not mounted into the container, but they're + // a valid type of config so we should not error when we encounter + // one. + if ref.Runtime == nil { + logrus.Error("config target type is not a file or runtime target") + } + // However, in any case, this isn't a file config, so we have no + // further work to do continue } diff --git a/daemon/container_operations_windows.go b/daemon/container_operations_windows.go index 10bfd53d6e..c2381e6431 100644 --- a/daemon/container_operations_windows.go +++ b/daemon/container_operations_windows.go @@ -44,7 +44,14 @@ func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) { for _, configRef := range c.ConfigReferences { // TODO (ehazlett): use type switch when more are supported if configRef.File == nil { - logrus.Error("config target type is not a file target") + // Runtime configs are not mounted into the container, but they're + // a valid type of config so we should not error when we encounter + // one. + if configRef.Runtime == nil { + logrus.Error("config target type is not a file or runtime target") + } + // However, in any case, this isn't a file config, so we have no + // further work to do continue } diff --git a/daemon/oci_windows.go b/daemon/oci_windows.go index 11868ba3a0..215d5b6d3a 100644 --- a/daemon/oci_windows.go +++ b/daemon/oci_windows.go @@ -289,18 +289,23 @@ func (daemon *Daemon) createSpecWindowsFields(c *container.Container, s *specs.S return err } } else if match, csValue = getCredentialSpec("config://", splitsOpt[1]); match { + // if the container does not have a DependencyStore, then it + // isn't swarmkit managed. In order to avoid creating any + // impression that `config://` is a valid API, return the same + // error as if you'd passed any other random word. + if c.DependencyStore == nil { + return fmt.Errorf("invalid credential spec security option - value must be prefixed file:// or registry:// followed by a value") + } + + // after this point, we can return regular swarmkit-relevant + // errors, because we'll know this container is managed. if csValue == "" { return fmt.Errorf("no value supplied for config:// credential spec security option") } - // if the container does not have a DependencyStore, then we - // return an error - if c.DependencyStore == nil { - return fmt.Errorf("cannot use config:// credential spec security option if not swarmkit managed") - } csConfig, err := c.DependencyStore.Configs().Get(csValue) if err != nil { - return fmt.Errorf("error getting value from config store: %v", err) + return errors.Wrap(err, "error getting value from config store") } // stuff the resulting secret data into a string to use as the // CredentialSpec diff --git a/docs/api/version-history.md b/docs/api/version-history.md index b7bb466f12..409b94c8c4 100644 --- a/docs/api/version-history.md +++ b/docs/api/version-history.md @@ -31,6 +31,8 @@ keywords: "API, Docker, rcli, REST, documentation" * `POST /services/{id}/update` now accepts `Sysctls` as part of the `ContainerSpec`. * `POST /services/create` now accepts `Config` as part of `ContainerSpec.Privileges.CredentialSpec`. * `POST /services/{id}/update` now accepts `Config` as part of `ContainerSpec.Privileges.CredentialSpec`. +* `POST /services/create` now includes `Runtime` as an option in `ContainerSpec.Configs` +* `POST /services/{id}/update` now includes `Runtime` as an option in `ContainerSpec.Configs` * `GET /tasks` now returns `Sysctls` as part of the `ContainerSpec`. * `GET /tasks/{id}` now returns `Sysctls` as part of the `ContainerSpec`. * `GET /nodes` now supports a filter type `node.label` filter to filter nodes based