From 9c2b1fd34ad180e02a17552b33f2b546aef74651 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 Upstream-commit: 2adbdcdf5a83b45b4e191413d3aa14158535085b Component: engine --- .../engine/cli/command/service/create.go | 3 +- components/engine/cli/command/service/opts.go | 97 ++++++++++++++++++- .../engine/cli/command/service/opts_test.go | 45 +++++++++ .../engine/cli/command/service/parse.go | 54 ++--------- .../engine/cli/command/service/update.go | 7 +- .../reference/commandline/service_create.md | 2 +- .../reference/commandline/service_update.md | 2 +- .../docker_cli_service_create_test.go | 2 +- .../docker_cli_service_update_test.go | 2 +- 9 files changed, 158 insertions(+), 56 deletions(-) diff --git a/components/engine/cli/command/service/create.go b/components/engine/cli/command/service/create.go index 8fb9070e67..e5b728d3e8 100644 --- a/components/engine/cli/command/service/create.go +++ b/components/engine/cli/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/components/engine/cli/command/service/opts.go b/components/engine/cli/command/service/opts.go index 37da5d1145..00cdecb67d 100644 --- a/components/engine/cli/command/service/opts.go +++ b/components/engine/cli/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/components/engine/cli/command/service/opts_test.go b/components/engine/cli/command/service/opts_test.go index aa2d999dcf..551dfc239c 100644 --- a/components/engine/cli/command/service/opts_test.go +++ b/components/engine/cli/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/components/engine/cli/command/service/parse.go b/components/engine/cli/command/service/parse.go index 5a22ed352c..73fa8a0cb9 100644 --- a/components/engine/cli/command/service/parse.go +++ b/components/engine/cli/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/components/engine/cli/command/service/update.go b/components/engine/cli/command/service/update.go index a9f5ac9be6..37f709e230 100644 --- a/components/engine/cli/command/service/update.go +++ b/components/engine/cli/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 { diff --git a/components/engine/docs/reference/commandline/service_create.md b/components/engine/docs/reference/commandline/service_create.md index 8e12a88d85..e98c2cc8ae 100644 --- a/components/engine/docs/reference/commandline/service_create.md +++ b/components/engine/docs/reference/commandline/service_create.md @@ -126,7 +126,7 @@ Use the `--secret` flag to give a container access to a with two secrets named `ssh-key` and `app-key`: ```bash -$ docker service create --name redis --secret ssh-key:ssh --secret app-key:app redis:3.0.6 +$ docker service create --name redis --secret source=ssh-key,target=ssh --secret source=app-key,target=app,uid=1000,gid=1001,mode=0400 redis:3.0.6 4cdgfyky7ozwh3htjfw0d12qv ``` diff --git a/components/engine/docs/reference/commandline/service_update.md b/components/engine/docs/reference/commandline/service_update.md index 2cc67996e6..261f1c3518 100644 --- a/components/engine/docs/reference/commandline/service_update.md +++ b/components/engine/docs/reference/commandline/service_update.md @@ -157,7 +157,7 @@ The following example adds a secret named `ssh-2` and removes `ssh-1`: ```bash $ docker service update \ - --secret-add ssh-2 \ + --secret-add source=ssh-2,target=ssh-2 \ --secret-rm ssh-1 \ myservice ``` diff --git a/components/engine/integration-cli/docker_cli_service_create_test.go b/components/engine/integration-cli/docker_cli_service_create_test.go index 85d07489a5..bac23b0eb1 100644 --- a/components/engine/integration-cli/docker_cli_service_create_test.go +++ b/components/engine/integration-cli/docker_cli_service_create_test.go @@ -59,7 +59,7 @@ func (s *DockerSwarmSuite) TestServiceCreateWithSecret(c *check.C) { c.Assert(id, checker.Not(checker.Equals), "", check.Commentf("secrets: %s", id)) testTarget := "testing" - out, err := d.Cmd("service", "create", "--name", serviceName, "--secret", fmt.Sprintf("%s:%s", testName, testTarget), "busybox", "top") + out, err := d.Cmd("service", "create", "--name", serviceName, "--secret", fmt.Sprintf("source=%s,target=%s", testName, testTarget), "busybox", "top") c.Assert(err, checker.IsNil, check.Commentf(out)) out, err = d.Cmd("service", "inspect", "--format", "{{ json .Spec.TaskTemplate.ContainerSpec.Secrets }}", serviceName) diff --git a/components/engine/integration-cli/docker_cli_service_update_test.go b/components/engine/integration-cli/docker_cli_service_update_test.go index 7bad23901a..548f8e64c2 100644 --- a/components/engine/integration-cli/docker_cli_service_update_test.go +++ b/components/engine/integration-cli/docker_cli_service_update_test.go @@ -103,7 +103,7 @@ func (s *DockerSwarmSuite) TestServiceUpdateSecrets(c *check.C) { c.Assert(err, checker.IsNil, check.Commentf(out)) // add secret - out, err = d.Cmd("service", "update", "test", "--secret-add", fmt.Sprintf("%s:%s", testName, testTarget)) + out, err = d.Cmd("service", "update", "test", "--secret-add", fmt.Sprintf("source=%s,target=%s", testName, testTarget)) c.Assert(err, checker.IsNil, check.Commentf(out)) out, err = d.Cmd("service", "inspect", "--format", "{{ json .Spec.TaskTemplate.ContainerSpec.Secrets }}", serviceName)