From bebd472e40cae91b548e983872a2665a5836ba43 Mon Sep 17 00:00:00 2001 From: Evan Hazlett Date: Tue, 15 Nov 2016 10:04:36 -0500 Subject: [PATCH] do not force target type for secret references Signed-off-by: Evan Hazlett use secret store interface instead of embedded secret data into container Signed-off-by: Evan Hazlett --- api/types/container/secret.go | 14 -------- api/types/swarm/secret.go | 2 +- cli/command/service/parse.go | 10 +++--- container/container.go | 9 ++++-- container/container_unix.go | 2 +- daemon/cluster/convert/container.go | 26 ++++++++------- daemon/cluster/convert/secret.go | 24 ++++++++++++++ daemon/cluster/executor/backend.go | 5 ++- daemon/cluster/executor/container/adapter.go | 32 ++++--------------- daemon/container_operations_unix.go | 27 ++++++++++++---- daemon/secrets.go | 23 ++++++++++--- .../docker_cli_service_create_test.go | 12 +++---- .../docker_cli_service_update_test.go | 4 +-- 13 files changed, 109 insertions(+), 81 deletions(-) delete mode 100644 api/types/container/secret.go diff --git a/api/types/container/secret.go b/api/types/container/secret.go deleted file mode 100644 index a9bbd2f041..0000000000 --- a/api/types/container/secret.go +++ /dev/null @@ -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 -} diff --git a/api/types/swarm/secret.go b/api/types/swarm/secret.go index dbed63af8e..b232d6dae1 100644 --- a/api/types/swarm/secret.go +++ b/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 } diff --git a/cli/command/service/parse.go b/cli/command/service/parse.go index 368bc6d449..ff3249e581 100644 --- a/cli/command/service/parse.go +++ b/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 } diff --git a/container/container.go b/container/container.go index 77d978070b..e0a9da424d 100644 --- a/container/container.go +++ b/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:"-"` diff --git a/container/container_unix.go b/container/container_unix.go index 6e9c251400..f92d586204 100644 --- a/container/container_unix.go +++ b/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, diff --git a/daemon/cluster/convert/container.go b/daemon/cluster/convert/container.go index eb1a84c9c4..8eb56b6eb7 100644 --- a/daemon/cluster/convert/container.go +++ b/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, }) } diff --git a/daemon/cluster/convert/secret.go b/daemon/cluster/convert/secret.go index f8cb4ff80c..47b87b20d3 100644 --- a/daemon/cluster/convert/secret.go +++ b/daemon/cluster/convert/secret.go @@ -39,3 +39,27 @@ func SecretSpecToGRPC(s swarmtypes.SecretSpec) swarmapi.SecretSpec { Data: s.Data, } } + +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 +} diff --git a/daemon/cluster/executor/backend.go b/daemon/cluster/executor/backend.go index e7c058b2eb..db7991b4dd 100644 --- a/daemon/cluster/executor/backend.go +++ b/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) diff --git a/daemon/cluster/executor/container/adapter.go b/daemon/cluster/executor/container/adapter.go index a61ad35df3..2e0306c772 100644 --- a/daemon/cluster/executor/container/adapter.go +++ b/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.SetContainerSecrets(cr.ID, secrets); err != nil { + if err := c.backend.SetContainerSecretStore(cr.ID, c.secrets); err != nil { + return err + } + + refs := convert.SecretReferencesFromGRPC(container.Secrets) + if err := c.backend.SetContainerSecretReferences(cr.ID, refs); err != nil { return err } diff --git a/daemon/container_operations_unix.go b/daemon/container_operations_unix.go index 5e1da60414..a55c688334 100644 --- a/daemon/container_operations_unix.go +++ b/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 } diff --git a/daemon/secrets.go b/daemon/secrets.go index 1d13adaaa4..355cb1e139 100644 --- a/daemon/secrets.go +++ b/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 } diff --git a/integration-cli/docker_cli_service_create_test.go b/integration-cli/docker_cli_service_create_test.go index 7af6a5c6de..4056e1f0ec 100644 --- a/integration-cli/docker_cli_service_create_test.go +++ b/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) } diff --git a/integration-cli/docker_cli_service_update_test.go b/integration-cli/docker_cli_service_update_test.go index 27fa2d0dd8..837370ceeb 100644 --- a/integration-cli/docker_cli_service_update_test.go +++ b/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)