secrets: use explicit format when using secrets

Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>

command/service/create.go

@@ -39,6 +39,7 @@ func newCreateCommand(dockerCli *command.DockerCli) *cobra.Command {
 	flags.Var(&opts.mounts, flagMount, "Attach a filesystem mount to the service")
 	flags.Var(&opts.constraints, flagConstraint, "Placement constraints")
 	flags.Var(&opts.networks, flagNetwork, "Network attachments")
+	flags.Var(&opts.secrets, flagSecret, "Specify secrets to expose to the service")
 	flags.VarP(&opts.endpoint.ports, flagPublish, "p", "Publish a port as a node port")
 	flags.Var(&opts.groups, flagGroup, "Set one or more supplementary user groups for the container")
 	flags.Var(&opts.dns, flagDNS, "Set custom DNS servers")
@@ -59,7 +60,7 @@ func runCreate(dockerCli *command.DockerCli, opts *serviceOptions) error {
 	}
 
 	// parse and validate secrets
-	secrets, err := parseSecrets(apiClient, opts.secrets)
+	secrets, err := parseSecrets(apiClient, opts.secrets.Value())
 	if err != nil {
 		return err
 	}

command/service/opts.go

@@ -1,7 +1,10 @@
 package service
 
 import (
+	"encoding/csv"
 	"fmt"
+	"os"
+	"path/filepath"
 	"strconv"
 	"strings"
 	"time"
@@ -139,6 +142,98 @@ func (f *floatValue) Value() float32 {
 	return float32(*f)
 }
 
+// SecretRequestSpec is a type for requesting secrets
+type SecretRequestSpec struct {
+	source string
+	target string
+	uid    string
+	gid    string
+	mode   os.FileMode
+}
+
+// SecretOpt is a Value type for parsing secrets
+type SecretOpt struct {
+	values []*SecretRequestSpec
+}
+
+// Set a new secret value
+func (o *SecretOpt) Set(value string) error {
+	csvReader := csv.NewReader(strings.NewReader(value))
+	fields, err := csvReader.Read()
+	if err != nil {
+		return err
+	}
+
+	spec := &SecretRequestSpec{
+		source: "",
+		target: "",
+		uid:    "0",
+		gid:    "0",
+		mode:   0444,
+	}
+
+	for _, field := range fields {
+		parts := strings.SplitN(field, "=", 2)
+		key := strings.ToLower(parts[0])
+
+		if len(parts) != 2 {
+			return fmt.Errorf("invalid field '%s' must be a key=value pair", field)
+		}
+
+		value := parts[1]
+		switch key {
+		case "source":
+			spec.source = value
+		case "target":
+			tDir, _ := filepath.Split(value)
+			if tDir != "" {
+				return fmt.Errorf("target must not have a path")
+			}
+			spec.target = value
+		case "uid":
+			spec.uid = value
+		case "gid":
+			spec.gid = value
+		case "mode":
+			m, err := strconv.ParseUint(value, 0, 32)
+			if err != nil {
+				return fmt.Errorf("invalid mode specified: %v", err)
+			}
+
+			spec.mode = os.FileMode(m)
+		default:
+			return fmt.Errorf("invalid field in secret request: %s", key)
+		}
+	}
+
+	if spec.source == "" {
+		return fmt.Errorf("source is required")
+	}
+
+	o.values = append(o.values, spec)
+	return nil
+}
+
+// Type returns the type of this option
+func (o *SecretOpt) Type() string {
+	return "secret"
+}
+
+// String returns a string repr of this option
+func (o *SecretOpt) String() string {
+	secrets := []string{}
+	for _, secret := range o.values {
+		repr := fmt.Sprintf("%s -> %s", secret.source, secret.target)
+		secrets = append(secrets, repr)
+	}
+	return strings.Join(secrets, ", ")
+}
+
+// Value returns the secret requests
+func (o *SecretOpt) Value() []*SecretRequestSpec {
+	return o.values
+}
+
 type updateOptions struct {
 	parallelism     uint64
 	delay           time.Duration
@@ -337,7 +432,7 @@ type serviceOptions struct {
 	logDriver logDriverOptions
 
 	healthcheck healthCheckOptions
-	secrets     []string
+	secrets     SecretOpt
 }
 
 func newServiceOptions() *serviceOptions {

command/service/opts_test.go

@@ -1,6 +1,7 @@
 package service
 
 import (
+	"os"
 	"reflect"
 	"testing"
 	"time"
@@ -105,3 +106,47 @@ func TestHealthCheckOptionsToHealthConfigConflict(t *testing.T) {
 	_, err := opt.toHealthConfig()
 	assert.Error(t, err, "--no-healthcheck conflicts with --health-* options")
 }
+
+func TestSecretOptionsSimple(t *testing.T) {
+	var opt SecretOpt
+
+	testCase := "source=/foo,target=testing"
+	assert.NilError(t, opt.Set(testCase))
+
+	reqs := opt.Value()
+	assert.Equal(t, len(reqs), 1)
+	req := reqs[0]
+	assert.Equal(t, req.source, "/foo")
+	assert.Equal(t, req.target, "testing")
+}
+
+func TestSecretOptionsCustomUidGid(t *testing.T) {
+	var opt SecretOpt
+
+	testCase := "source=/foo,target=testing,uid=1000,gid=1001"
+	assert.NilError(t, opt.Set(testCase))
+
+	reqs := opt.Value()
+	assert.Equal(t, len(reqs), 1)
+	req := reqs[0]
+	assert.Equal(t, req.source, "/foo")
+	assert.Equal(t, req.target, "testing")
+	assert.Equal(t, req.uid, "1000")
+	assert.Equal(t, req.gid, "1001")
+}
+
+func TestSecretOptionsCustomMode(t *testing.T) {
+	var opt SecretOpt
+
+	testCase := "source=/foo,target=testing,uid=1000,gid=1001,mode=0444"
+	assert.NilError(t, opt.Set(testCase))
+
+	reqs := opt.Value()
+	assert.Equal(t, len(reqs), 1)
+	req := reqs[0]
+	assert.Equal(t, req.source, "/foo")
+	assert.Equal(t, req.target, "testing")
+	assert.Equal(t, req.uid, "1000")
+	assert.Equal(t, req.gid, "1001")
+	assert.Equal(t, req.mode, os.FileMode(0444))
+}

command/service/parse.go

@@ -3,8 +3,6 @@ package service
 import (
 	"context"
 	"fmt"
-	"path/filepath"
-	"strings"
 
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/filters"
@@ -12,61 +10,27 @@ import (
 	"github.com/docker/docker/client"
 )
 
-// parseSecretString parses the requested secret and returns the secret name
-// and target.  Expects format SECRET_NAME:TARGET
-func parseSecretString(secretString string) (string, string, error) {
-	tokens := strings.Split(secretString, ":")
-
-	secretName := strings.TrimSpace(tokens[0])
-	targetName := secretName
-
-	if secretName == "" {
-		return "", "", fmt.Errorf("invalid secret name provided")
-	}
-
-	if len(tokens) > 1 {
-		targetName = strings.TrimSpace(tokens[1])
-		if targetName == "" {
-			return "", "", fmt.Errorf("invalid presentation name provided")
-		}
-	}
-
-	// ensure target is a filename only; no paths allowed
-	tDir, _ := filepath.Split(targetName)
-	if tDir != "" {
-		return "", "", fmt.Errorf("target must not have a path")
-	}
-
-	return secretName, targetName, nil
-}
-
 // parseSecrets retrieves the secrets from the requested names and converts
 // them to secret references to use with the spec
-func parseSecrets(client client.APIClient, requestedSecrets []string) ([]*swarmtypes.SecretReference, error) {
+func parseSecrets(client client.APIClient, requestedSecrets []*SecretRequestSpec) ([]*swarmtypes.SecretReference, error) {
 	secretRefs := make(map[string]*swarmtypes.SecretReference)
 	ctx := context.Background()
 
 	for _, secret := range requestedSecrets {
-		n, t, err := parseSecretString(secret)
-		if err != nil {
-			return nil, err
-		}
-
 		secretRef := &swarmtypes.SecretReference{
-			SecretName: n,
-			// TODO (ehazlett): parse these from cli request
+			SecretName: secret.source,
 			Target: swarmtypes.SecretReferenceFileTarget{
-				Name: t,
-				UID:  "0",
-				GID:  "0",
-				Mode: 0444,
+				Name: secret.target,
+				UID:  secret.uid,
+				GID:  secret.gid,
+				Mode: secret.mode,
 			},
 		}
 
-		if _, exists := secretRefs[t]; exists {
-			return nil, fmt.Errorf("duplicate secret target for %s not allowed", n)
+		if _, exists := secretRefs[secret.target]; exists {
+			return nil, fmt.Errorf("duplicate secret target for %s not allowed", secret.source)
 		}
-		secretRefs[t] = secretRef
+		secretRefs[secret.target] = secretRef
 	}
 
 	args := filters.NewArgs()

command/service/update.go

@@ -56,7 +56,7 @@ func newUpdateCommand(dockerCli *command.DockerCli) *cobra.Command {
 	flags.Var(&opts.containerLabels, flagContainerLabelAdd, "Add or update a container label")
 	flags.Var(&opts.env, flagEnvAdd, "Add or update an environment variable")
 	flags.Var(newListOptsVar(), flagSecretRemove, "Remove a secret")
-	flags.StringSliceVar(&opts.secrets, flagSecretAdd, []string{}, "Add a secret")
+	flags.Var(&opts.secrets, flagSecretAdd, "Add or update a secret on a service")
 	flags.Var(&opts.mounts, flagMountAdd, "Add or update a mount on a service")
 	flags.Var(&opts.constraints, flagConstraintAdd, "Add or update a placement constraint")
 	flags.Var(&opts.endpoint.ports, flagPublishAdd, "Add or update a published port")
@@ -413,10 +413,7 @@ func updateEnvironment(flags *pflag.FlagSet, field *[]string) {
 
 func getUpdatedSecrets(apiClient client.APIClient, flags *pflag.FlagSet, secrets []*swarm.SecretReference) ([]*swarm.SecretReference, error) {
 	if flags.Changed(flagSecretAdd) {
-		values, err := flags.GetStringSlice(flagSecretAdd)
-		if err != nil {
-			return nil, err
-		}
+		values := flags.Lookup(flagSecretAdd).Value.(*SecretOpt).Value()
 
 		addSecrets, err := parseSecrets(apiClient, values)
 		if err != nil {