Merge pull request #28556 from ehazlett/container-secret-store

Fix target reference secrets and container secret store

api/types/container/secret.go

@@ -1,14 +0,0 @@
-package container
-
-import "os"
-
-// ContainerSecret represents a secret in a container.  This gets realized
-// in the container tmpfs
-type ContainerSecret struct {
-	Name   string
-	Target string
-	Data   []byte
-	UID    string
-	GID    string
-	Mode   os.FileMode
-}

api/types/swarm/secret.go

@@ -27,7 +27,7 @@ type SecretReferenceFileTarget struct {
 
 // SecretReference is a reference to a secret in swarm
 type SecretReference struct {
+	File       *SecretReferenceFileTarget
 	SecretID   string
 	SecretName string
-	Target     *SecretReferenceFileTarget
 }

cli/command/service/parse.go

@@ -17,19 +17,19 @@ func parseSecrets(client client.APIClient, requestedSecrets []*types.SecretReque
 	ctx := context.Background()
 
 	for _, secret := range requestedSecrets {
+		if _, exists := secretRefs[secret.Target]; exists {
+			return nil, fmt.Errorf("duplicate secret target for %s not allowed", secret.Source)
+		}
 		secretRef := &swarmtypes.SecretReference{
-			SecretName: secret.Source,
-			Target: &swarmtypes.SecretReferenceFileTarget{
+			File: &swarmtypes.SecretReferenceFileTarget{
 				Name: secret.Target,
 				UID:  secret.UID,
 				GID:  secret.GID,
 				Mode: secret.Mode,
 			},
+			SecretName: secret.Source,
 		}
 
-		if _, exists := secretRefs[secret.Target]; exists {
-			return nil, fmt.Errorf("duplicate secret target for %s not allowed", secret.Source)
-		}
 		secretRefs[secret.Target] = secretRef
 	}
 

container/container.go

@@ -19,6 +19,7 @@ import (
 	containertypes "github.com/docker/docker/api/types/container"
 	mounttypes "github.com/docker/docker/api/types/mount"
 	networktypes "github.com/docker/docker/api/types/network"
+	swarmtypes "github.com/docker/docker/api/types/swarm"
 	"github.com/docker/docker/container/stream"
 	"github.com/docker/docker/daemon/exec"
 	"github.com/docker/docker/daemon/logger"
@@ -41,6 +42,7 @@ import (
 	"github.com/docker/libnetwork/netlabel"
 	"github.com/docker/libnetwork/options"
 	"github.com/docker/libnetwork/types"
+	agentexec "github.com/docker/swarmkit/agent/exec"
 	"github.com/opencontainers/runc/libcontainer/label"
 )
 
@@ -90,9 +92,10 @@ type CommonContainer struct {
 	HasBeenStartedBefore   bool
 	HasBeenManuallyStopped bool // used for unless-stopped restart policy
 	MountPoints            map[string]*volume.MountPoint
-	HostConfig             *containertypes.HostConfig        `json:"-"` // do not serialize the host config in the json, otherwise we'll make the container unportable
-	ExecCommands           *exec.Store                       `json:"-"`
-	Secrets                []*containertypes.ContainerSecret `json:"-"` // do not serialize
+	HostConfig             *containertypes.HostConfig `json:"-"` // do not serialize the host config in the json, otherwise we'll make the container unportable
+	ExecCommands           *exec.Store                `json:"-"`
+	SecretStore            agentexec.SecretGetter     `json:"-"`
+	SecretReferences       []*swarmtypes.SecretReference
 	// logDriver for closing
 	LogDriver      logger.Logger  `json:"-"`
 	LogCopier      *logger.Copier `json:"-"`

container/container_unix.go

@@ -258,7 +258,7 @@ func (container *Container) IpcMounts() []Mount {
 
 // SecretMount returns the mount for the secret path
 func (container *Container) SecretMount() *Mount {
-	if len(container.Secrets) > 0 {
+	if len(container.SecretReferences) > 0 {
 		return &Mount{
 			Source:      container.SecretMountPath(),
 			Destination: containerSecretMountPath,

daemon/cluster/convert/container.go

@@ -82,18 +82,22 @@ func containerSpecFromGRPC(c *swarmapi.ContainerSpec) types.ContainerSpec {
 func secretReferencesToGRPC(sr []*types.SecretReference) []*swarmapi.SecretReference {
 	refs := make([]*swarmapi.SecretReference, 0, len(sr))
 	for _, s := range sr {
-		refs = append(refs, &swarmapi.SecretReference{
+		ref := &swarmapi.SecretReference{
 			SecretID:   s.SecretID,
 			SecretName: s.SecretName,
-			Target: &swarmapi.SecretReference_File{
+		}
+		if s.File != nil {
+			ref.Target = &swarmapi.SecretReference_File{
 				File: &swarmapi.SecretReference_FileTarget{
-					Name: s.Target.Name,
-					UID:  s.Target.UID,
-					GID:  s.Target.GID,
-					Mode: s.Target.Mode,
+					Name: s.File.Name,
+					UID:  s.File.UID,
+					GID:  s.File.GID,
+					Mode: s.File.Mode,
 				},
-			},
-		})
+			}
+		}
+
+		refs = append(refs, ref)
 	}
 
 	return refs
@@ -108,14 +112,14 @@ func secretReferencesFromGRPC(sr []*swarmapi.SecretReference) []*types.SecretRef
 			continue
 		}
 		refs = append(refs, &types.SecretReference{
-			SecretID:   s.SecretID,
-			SecretName: s.SecretName,
-			Target: &types.SecretReferenceFileTarget{
+			File: &types.SecretReferenceFileTarget{
 				Name: target.Name,
 				UID:  target.UID,
 				GID:  target.GID,
 				Mode: target.Mode,
 			},
+			SecretID:   s.SecretID,
+			SecretName: s.SecretName,
 		})
 	}
 

daemon/cluster/convert/secret.go

@@ -39,3 +39,28 @@ func SecretSpecToGRPC(s swarmtypes.SecretSpec) swarmapi.SecretSpec {
 		Data: s.Data,
 	}
 }
+
+// SecretReferencesFromGRPC converts a slice of grpc SecretReference to SecretReference
+func SecretReferencesFromGRPC(s []*swarmapi.SecretReference) []*swarmtypes.SecretReference {
+	refs := []*swarmtypes.SecretReference{}
+
+	for _, r := range s {
+		ref := &swarmtypes.SecretReference{
+			SecretID:   r.SecretID,
+			SecretName: r.SecretName,
+		}
+
+		if t, ok := r.Target.(*swarmapi.SecretReference_File); ok {
+			ref.File = &swarmtypes.SecretReferenceFileTarget{
+				Name: t.File.Name,
+				UID:  t.File.UID,
+				GID:  t.File.GID,
+				Mode: t.File.Mode,
+			}
+		}
+
+		refs = append(refs, ref)
+	}
+
+	return refs
+}

daemon/cluster/executor/backend.go

@@ -11,11 +11,13 @@ import (
 	"github.com/docker/docker/api/types/events"
 	"github.com/docker/docker/api/types/filters"
 	"github.com/docker/docker/api/types/network"
+	swarmtypes "github.com/docker/docker/api/types/swarm"
 	clustertypes "github.com/docker/docker/daemon/cluster/provider"
 	"github.com/docker/docker/reference"
 	"github.com/docker/libnetwork"
 	"github.com/docker/libnetwork/cluster"
 	networktypes "github.com/docker/libnetwork/types"
+	"github.com/docker/swarmkit/agent/exec"
 	"golang.org/x/net/context"
 )
 
@@ -38,7 +40,8 @@ type Backend interface {
 	ContainerWaitWithContext(ctx context.Context, name string) error
 	ContainerRm(name string, config *types.ContainerRmConfig) error
 	ContainerKill(name string, sig uint64) error
-	SetContainerSecrets(name string, secrets []*container.ContainerSecret) error
+	SetContainerSecretStore(name string, store exec.SecretGetter) error
+	SetContainerSecretReferences(name string, refs []*swarmtypes.SecretReference) error
 	SystemInfo() (*types.Info, error)
 	VolumeCreate(name, driverName string, opts, labels map[string]string) (*types.Volume, error)
 	Containers(config *types.ContainerListOptions) ([]*types.Container, error)

daemon/cluster/executor/container/adapter.go

@@ -16,6 +16,7 @@ import (
 	containertypes "github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/events"
 	"github.com/docker/docker/api/types/versions"
+	"github.com/docker/docker/daemon/cluster/convert"
 	executorpkg "github.com/docker/docker/daemon/cluster/executor"
 	"github.com/docker/docker/reference"
 	"github.com/docker/libnetwork"
@@ -237,33 +238,14 @@ func (c *containerAdapter) create(ctx context.Context) error {
 	if container == nil {
 		return fmt.Errorf("unable to get container from task spec")
 	}
-	secrets := make([]*containertypes.ContainerSecret, 0, len(container.Secrets))
-	for _, s := range container.Secrets {
-		sec := c.secrets.Get(s.SecretID)
-		if sec == nil {
-			logrus.Warnf("unable to get secret %s from provider", s.SecretID)
-			continue
-		}
-
-		name := sec.Spec.Annotations.Name
-		target := s.GetFile()
-		if target == nil {
-			logrus.Warnf("secret target was not a file: secret=%s", s.SecretID)
-			continue
-		}
 
-		secrets = append(secrets, &containertypes.ContainerSecret{
-			Name:   name,
-			Target: target.Name,
-			Data:   sec.Spec.Data,
-			UID:    target.UID,
-			GID:    target.GID,
-			Mode:   target.Mode,
-		})
+	// configure secrets
+	if err := c.backend.SetContainerSecretStore(cr.ID, c.secrets); err != nil {
+		return err
 	}
 
-	// configure secrets
-	if err := c.backend.SetContainerSecrets(cr.ID, secrets); err != nil {
+	refs := convert.SecretReferencesFromGRPC(container.Secrets)
+	if err := c.backend.SetContainerSecretReferences(cr.ID, refs); err != nil {
 		return err
 	}
 

daemon/container_operations_unix.go

@@ -145,7 +145,7 @@ func (daemon *Daemon) setupIpcDirs(c *container.Container) error {
 }
 
 func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) {
-	if len(c.Secrets) == 0 {
+	if len(c.SecretReferences) == 0 {
 		return nil
 	}
 
@@ -174,8 +174,17 @@ func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) {
 		return errors.Wrap(err, "unable to setup secret mount")
 	}
 
-	for _, s := range c.Secrets {
-		targetPath := filepath.Clean(s.Target)
+	for _, s := range c.SecretReferences {
+		if c.SecretStore == nil {
+			return fmt.Errorf("secret store is not initialized")
+		}
+
+		// TODO (ehazlett): use type switch when more are supported
+		if s.File == nil {
+			return fmt.Errorf("secret target type is not a file target")
+		}
+
+		targetPath := filepath.Clean(s.File.Name)
 		// ensure that the target is a filename only; no paths allowed
 		if targetPath != filepath.Base(targetPath) {
 			return fmt.Errorf("error creating secret: secret must not be a path")
@@ -187,18 +196,22 @@ func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) {
 		}
 
 		logrus.WithFields(logrus.Fields{
-			"name": s.Name,
+			"name": s.File.Name,
 			"path": fPath,
 		}).Debug("injecting secret")
-		if err := ioutil.WriteFile(fPath, s.Data, s.Mode); err != nil {
+		secret := c.SecretStore.Get(s.SecretID)
+		if secret == nil {
+			return fmt.Errorf("unable to get secret from secret store")
+		}
+		if err := ioutil.WriteFile(fPath, secret.Spec.Data, s.File.Mode); err != nil {
 			return errors.Wrap(err, "error injecting secret")
 		}
 
-		uid, err := strconv.Atoi(s.UID)
+		uid, err := strconv.Atoi(s.File.UID)
 		if err != nil {
 			return err
 		}
-		gid, err := strconv.Atoi(s.GID)
+		gid, err := strconv.Atoi(s.File.GID)
 		if err != nil {
 			return err
 		}

daemon/secrets.go

@@ -2,12 +2,25 @@ package daemon
 
 import (
 	"github.com/Sirupsen/logrus"
-	containertypes "github.com/docker/docker/api/types/container"
+	swarmtypes "github.com/docker/docker/api/types/swarm"
+	"github.com/docker/swarmkit/agent/exec"
 )
 
-// SetContainerSecrets sets the container secrets needed
-func (daemon *Daemon) SetContainerSecrets(name string, secrets []*containertypes.ContainerSecret) error {
-	if !secretsSupported() && len(secrets) > 0 {
+// SetContainerSecretStore sets the secret store backend for the container
+func (daemon *Daemon) SetContainerSecretStore(name string, store exec.SecretGetter) error {
+	c, err := daemon.GetContainer(name)
+	if err != nil {
+		return err
+	}
+
+	c.SecretStore = store
+
+	return nil
+}
+
+// SetContainerSecretReferences sets the container secret references needed
+func (daemon *Daemon) SetContainerSecretReferences(name string, refs []*swarmtypes.SecretReference) error {
+	if !secretsSupported() && len(refs) > 0 {
 		logrus.Warn("secrets are not supported on this platform")
 		return nil
 	}
@@ -17,7 +30,7 @@ func (daemon *Daemon) SetContainerSecrets(name string, secrets []*containertypes
 		return err
 	}
 
-	c.Secrets = secrets
+	c.SecretReferences = refs
 
 	return nil
 }

integration-cli/docker_cli_service_create_test.go

@@ -69,10 +69,10 @@ func (s *DockerSwarmSuite) TestServiceCreateWithSecretSimple(c *check.C) {
 	c.Assert(refs, checker.HasLen, 1)
 
 	c.Assert(refs[0].SecretName, checker.Equals, testName)
-	c.Assert(refs[0].Target, checker.Not(checker.IsNil))
-	c.Assert(refs[0].Target.Name, checker.Equals, testName)
-	c.Assert(refs[0].Target.UID, checker.Equals, "0")
-	c.Assert(refs[0].Target.GID, checker.Equals, "0")
+	c.Assert(refs[0].File, checker.Not(checker.IsNil))
+	c.Assert(refs[0].File.Name, checker.Equals, testName)
+	c.Assert(refs[0].File.UID, checker.Equals, "0")
+	c.Assert(refs[0].File.GID, checker.Equals, "0")
 }
 
 func (s *DockerSwarmSuite) TestServiceCreateWithSecretSourceTarget(c *check.C) {
@@ -100,6 +100,6 @@ func (s *DockerSwarmSuite) TestServiceCreateWithSecretSourceTarget(c *check.C) {
 	c.Assert(refs, checker.HasLen, 1)
 
 	c.Assert(refs[0].SecretName, checker.Equals, testName)
-	c.Assert(refs[0].Target, checker.Not(checker.IsNil))
-	c.Assert(refs[0].Target.Name, checker.Equals, testTarget)
+	c.Assert(refs[0].File, checker.Not(checker.IsNil))
+	c.Assert(refs[0].File.Name, checker.Equals, testTarget)
 }

integration-cli/docker_cli_service_update_test.go

@@ -115,8 +115,8 @@ func (s *DockerSwarmSuite) TestServiceUpdateSecrets(c *check.C) {
 	c.Assert(refs, checker.HasLen, 1)
 
 	c.Assert(refs[0].SecretName, checker.Equals, testName)
-	c.Assert(refs[0].Target, checker.Not(checker.IsNil))
-	c.Assert(refs[0].Target.Name, checker.Equals, testTarget)
+	c.Assert(refs[0].File, checker.Not(checker.IsNil))
+	c.Assert(refs[0].File.Name, checker.Equals, testTarget)
 
 	// remove
 	out, err = d.cmdRetryOutOfSequence("service", "update", "test", "--secret-rm", testName)