From 15b97a39d7668569a3bb5c018f593ad1bcf42b77 Mon Sep 17 00:00:00 2001 From: Evan Hazlett Date: Tue, 1 Nov 2016 22:28:32 -0400 Subject: [PATCH] secrets: use explicit format when using secrets Signed-off-by: Evan Hazlett --- command/service/create.go | 3 +- command/service/opts.go | 97 +++++++++++++++++++++++++++++++++++- command/service/opts_test.go | 45 +++++++++++++++++ command/service/parse.go | 54 ++++---------------- command/service/update.go | 7 +-- 5 files changed, 154 insertions(+), 52 deletions(-) diff --git a/command/service/create.go b/command/service/create.go index 8fb9070e67..e5b728d3e8 100644 --- a/command/service/create.go +++ b/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 } diff --git a/command/service/opts.go b/command/service/opts.go index 37da5d1145..00cdecb67d 100644 --- a/command/service/opts.go +++ b/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 { diff --git a/command/service/opts_test.go b/command/service/opts_test.go index aa2d999dcf..551dfc239c 100644 --- a/command/service/opts_test.go +++ b/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)) +} diff --git a/command/service/parse.go b/command/service/parse.go index 5a22ed352c..73fa8a0cb9 100644 --- a/command/service/parse.go +++ b/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() diff --git a/command/service/update.go b/command/service/update.go index a9f5ac9be6..37f709e230 100644 --- a/command/service/update.go +++ b/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 {