Use Runtime target

The Swarmkit api specifies a target for configs called called "Runtime"
which indicates that the config is not mounted into the container but
has some other use. This commit updates the Docker api to reflect this.

Signed-off-by: Drew Erny <drew.erny@docker.com>

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)

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
+		}
+	}
+}

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")
+	}
+
+}

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.
 
                       <p><br /></p>
 
@@ -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.
+
+                    <p><br /><p>
+
+                    > **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
+
+                    <p><br /><p>
+
+                    > **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"

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
 }

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{

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)
+		})
+	}
+}

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
 		}
 

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
 		}
 

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

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