review changes

- fix lint issues
- use errors pkg for wrapping errors
- cleanup on error when setting up secrets mount
- fix erroneous import
- remove unneeded switch for secret reference mode
- return single mount for secrets instead of slice

Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Upstream-commit: 857e60c2f943a09e3ec0ac0f236821b797935900
Component: engine

components/engine/daemon/cluster/convert/container.go

@@ -79,12 +79,9 @@ func containerSpecFromGRPC(c *swarmapi.ContainerSpec) types.ContainerSpec {
 func secretReferencesToGRPC(sr []*types.SecretReference) []*swarmapi.SecretReference {
 	refs := []*swarmapi.SecretReference{}
 	for _, s := range sr {
-		var mode swarmapi.SecretReference_Mode
-		switch s.Mode {
-		case types.SecretReferenceSystem:
+		mode := swarmapi.SecretReference_FILE
+		if s.Mode == types.SecretReferenceSystem {
 			mode = swarmapi.SecretReference_SYSTEM
-		default:
-			mode = swarmapi.SecretReference_FILE
 		}
 		refs = append(refs, &swarmapi.SecretReference{
 			SecretID:   s.SecretID,

components/engine/daemon/cluster/convert/secret.go

@@ -1,7 +1,6 @@
 package convert
 
 import (
-	"github.com/Sirupsen/logrus"
 	swarmtypes "github.com/docker/docker/api/types/swarm"
 	swarmapi "github.com/docker/swarmkit/api"
 	"github.com/docker/swarmkit/protobuf/ptypes"
@@ -9,7 +8,6 @@ import (
 
 // SecretFromGRPC converts a grpc Secret to a Secret.
 func SecretFromGRPC(s *swarmapi.Secret) swarmtypes.Secret {
-	logrus.Debugf("%+v", s)
 	secret := swarmtypes.Secret{
 		ID:         s.ID,
 		Digest:     s.Digest,
@@ -33,14 +31,12 @@ func SecretFromGRPC(s *swarmapi.Secret) swarmtypes.Secret {
 }
 
 // SecretSpecToGRPC converts Secret to a grpc Secret.
-func SecretSpecToGRPC(s swarmtypes.SecretSpec) (swarmapi.SecretSpec, error) {
-	spec := swarmapi.SecretSpec{
+func SecretSpecToGRPC(s swarmtypes.SecretSpec) swarmapi.SecretSpec {
+	return swarmapi.SecretSpec{
 		Annotations: swarmapi.Annotations{
 			Name:   s.Name,
 			Labels: s.Labels,
 		},
 		Data: s.Data,
 	}
-
-	return spec, nil
 }

components/engine/daemon/cluster/executor/container/attachment.go

@@ -2,9 +2,9 @@ package container
 
 import (
 	executorpkg "github.com/docker/docker/daemon/cluster/executor"
+	"github.com/docker/swarmkit/agent/exec"
 	"github.com/docker/swarmkit/api"
 	"golang.org/x/net/context"
-	"src/github.com/docker/swarmkit/agent/exec"
 )
 
 // networkAttacherController implements agent.Controller against docker's API.

components/engine/daemon/cluster/secrets.go

@@ -63,10 +63,7 @@ func (c *Cluster) CreateSecret(s types.SecretSpec) (string, error) {
 	ctx, cancel := c.getRequestContext()
 	defer cancel()
 
-	secretSpec, err := convert.SecretSpecToGRPC(s)
-	if err != nil {
-		return "", err
-	}
+	secretSpec := convert.SecretSpecToGRPC(s)
 
 	r, err := c.node.client.CreateSecret(ctx,
 		&swarmapi.CreateSecretRequest{Spec: &secretSpec})
@@ -111,10 +108,7 @@ func (c *Cluster) UpdateSecret(id string, version uint64, spec types.SecretSpec)
 	ctx, cancel := c.getRequestContext()
 	defer cancel()
 
-	secretSpec, err := convert.SecretSpecToGRPC(spec)
-	if err != nil {
-		return err
-	}
+	secretSpec := convert.SecretSpecToGRPC(spec)
 
 	if _, err := c.client.UpdateSecret(ctx,
 		&swarmapi.UpdateSecretRequest{

components/engine/daemon/container_operations_unix.go

@@ -13,6 +13,7 @@ import (
 	"time"
 
 	"github.com/Sirupsen/logrus"
+	"github.com/cloudflare/cfssl/log"
 	containertypes "github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/container"
 	"github.com/docker/docker/daemon/links"
@@ -25,6 +26,7 @@ import (
 	"github.com/opencontainers/runc/libcontainer/devices"
 	"github.com/opencontainers/runc/libcontainer/label"
 	"github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/pkg/errors"
 )
 
 func u32Ptr(i int64) *uint32     { u := uint32(i); return &u }
@@ -146,36 +148,58 @@ func (daemon *Daemon) setupSecretDir(c *container.Container) error {
 	localMountPath := c.SecretMountPath()
 	logrus.Debugf("secrets: setting up secret dir: %s", localMountPath)
 
+	var setupErr error
+
+	defer func(err error) {
+		if err != nil {
+			// cleanup
+			_ = detachMounted(localMountPath)
+
+			if err := os.RemoveAll(localMountPath); err != nil {
+				log.Errorf("error cleaning up secret mount: %s", err)
+			}
+		}
+	}(setupErr)
+
 	// create tmpfs
 	if err := os.MkdirAll(localMountPath, 0700); err != nil {
-		return fmt.Errorf("error creating secret local mount path: %s", err)
+		setupErr = errors.Wrap(err, "error creating secret local mount path")
 	}
 	if err := mount.Mount("tmpfs", localMountPath, "tmpfs", "nodev"); err != nil {
-		return fmt.Errorf("unable to setup secret mount: %s", err)
+		setupErr = errors.Wrap(err, "unable to setup secret mount")
 	}
 
 	for _, s := range c.Secrets {
-		fPath := filepath.Join(localMountPath, s.Target)
-		if err := os.MkdirAll(filepath.Dir(fPath), 0700); err != nil {
-			return fmt.Errorf("error creating secret mount path: %s", err)
+		// ensure that the target is a filename only; no paths allowed
+		tDir, tPath := filepath.Split(s.Target)
+		if tDir != "" {
+			setupErr = fmt.Errorf("error creating secret: secret must not have a path")
 		}
 
-		logrus.Debugf("injecting secret: name=%s path=%s", s.Name, fPath)
+		fPath := filepath.Join(localMountPath, tPath)
+		if err := os.MkdirAll(filepath.Dir(fPath), 0700); err != nil {
+			setupErr = errors.Wrap(err, "error creating secret mount path")
+		}
+
+		logrus.WithFields(logrus.Fields{
+			"name": s.Name,
+			"path": fPath,
+		}).Debug("injecting secret")
 		if err := ioutil.WriteFile(fPath, s.Data, s.Mode); err != nil {
-			return fmt.Errorf("error injecting secret: %s", err)
+			setupErr = errors.Wrap(err, "error injecting secret")
 		}
 
 		if err := os.Chown(fPath, s.Uid, s.Gid); err != nil {
-			return fmt.Errorf("error setting ownership for secret: %s", err)
+			setupErr = errors.Wrap(err, "error setting ownership for secret")
 		}
 	}
 
 	// remount secrets ro
 	if err := mount.Mount("tmpfs", localMountPath, "tmpfs", "remount,ro"); err != nil {
-		return fmt.Errorf("unable to remount secret dir as readonly: %s", err)
+		setupErr = errors.Wrap(err, "unable to remount secret dir as readonly")
 	}
 
-	return nil
+	return setupErr
 }
 
 func killProcessDirectly(container *container.Container) error {

components/engine/daemon/oci_linux.go

@@ -718,7 +718,8 @@ func (daemon *Daemon) createSpec(c *container.Container) (*specs.Spec, error) {
 	}
 	ms = append(ms, tmpfsMounts...)
 
-	ms = append(ms, c.SecretMounts()...)
+	ms = append(ms, c.SecretMount())
+
 	sort.Sort(mounts(ms))
 	if err := setMounts(daemon, &s, c, ms); err != nil {
 		return nil, fmt.Errorf("linux mounts: %v", err)

components/engine/daemon/secrets.go

@@ -5,8 +5,9 @@ import (
 	containertypes "github.com/docker/docker/api/types/container"
 )
 
+// SetContainerSecrets sets the container secrets needed
 func (daemon *Daemon) SetContainerSecrets(name string, secrets []*containertypes.ContainerSecret) error {
-	if !secretsSupported() {
+	if !secretsSupported() && len(secrets) > 0 {
 		logrus.Warn("secrets are not supported on this platform")
 		return nil
 	}