From fa358a87571f9212f91d8fde6696926d76ecca64 Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Wed, 7 Dec 2016 11:06:07 -0800 Subject: [PATCH] Move secret name or ID prefix resolving from client to daemon This fix is a follow up for comment: https://github.com/docker/docker/pull/28896#issuecomment-265392703 Currently secret name or ID prefix resolving is done at the client side, which means different behavior of API and CMD. This fix moves the resolving from client to daemon, with exactly the same rule: - Full ID - Full Name - Partial ID (prefix) All existing tests should pass. This fix is related to #288896, #28884 and may be related to #29125. Signed-off-by: Yong Tang --- cli/command/secret/inspect.go | 6 +- cli/command/secret/remove.go | 11 +-- daemon/cluster/secrets.go | 67 +++++++++++++++++-- .../docker_cli_secret_create_test.go | 2 +- 4 files changed, 65 insertions(+), 21 deletions(-) diff --git a/cli/command/secret/inspect.go b/cli/command/secret/inspect.go index 0a8bd4a23fe3b..fb694c5fbe007 100644 --- a/cli/command/secret/inspect.go +++ b/cli/command/secret/inspect.go @@ -33,13 +33,9 @@ func runSecretInspect(dockerCli *command.DockerCli, opts inspectOptions) error { client := dockerCli.Client() ctx := context.Background() - ids, err := getCliRequestedSecretIDs(ctx, client, opts.names) - if err != nil { - return err - } getRef := func(id string) (interface{}, []byte, error) { return client.SecretInspectWithRaw(ctx, id) } - return inspect.Inspect(dockerCli.Out(), ids, opts.format, getRef) + return inspect.Inspect(dockerCli.Out(), opts.names, opts.format, getRef) } diff --git a/cli/command/secret/remove.go b/cli/command/secret/remove.go index f45a619f6aa83..91ca4388f06cb 100644 --- a/cli/command/secret/remove.go +++ b/cli/command/secret/remove.go @@ -33,20 +33,15 @@ func runSecretRemove(dockerCli *command.DockerCli, opts removeOptions) error { client := dockerCli.Client() ctx := context.Background() - ids, err := getCliRequestedSecretIDs(ctx, client, opts.names) - if err != nil { - return err - } - var errs []string - for _, id := range ids { - if err := client.SecretRemove(ctx, id); err != nil { + for _, name := range opts.names { + if err := client.SecretRemove(ctx, name); err != nil { errs = append(errs, err.Error()) continue } - fmt.Fprintln(dockerCli.Out(), id) + fmt.Fprintln(dockerCli.Out(), name) } if len(errs) > 0 { diff --git a/daemon/cluster/secrets.go b/daemon/cluster/secrets.go index df26f13d4651a..8cde36f729cbb 100644 --- a/daemon/cluster/secrets.go +++ b/daemon/cluster/secrets.go @@ -1,14 +1,63 @@ package cluster import ( + "fmt" + "strings" + apitypes "github.com/docker/docker/api/types" types "github.com/docker/docker/api/types/swarm" "github.com/docker/docker/daemon/cluster/convert" swarmapi "github.com/docker/swarmkit/api" + "golang.org/x/net/context" ) +func getSecretByNameOrIDPrefix(ctx context.Context, state *nodeState, nameOrIDPrefix string) (*swarmapi.Secret, error) { + // attempt to lookup secret by full ID + if r, err := state.controlClient.GetSecret(ctx, &swarmapi.GetSecretRequest{ + SecretID: nameOrIDPrefix, + }); err == nil { + return r.Secret, nil + } + + // attempt to lookup secret by full name and partial ID + // Note here ListSecretRequest_Filters operate with `or` + r, err := state.controlClient.ListSecrets(ctx, &swarmapi.ListSecretsRequest{ + Filters: &swarmapi.ListSecretsRequest_Filters{ + Names: []string{nameOrIDPrefix}, + IDPrefixes: []string{nameOrIDPrefix}, + }, + }) + if err != nil { + return nil, err + } + + // attempt to lookup secret by full name + for _, s := range r.Secrets { + if s.Spec.Annotations.Name == nameOrIDPrefix { + return s, nil + } + } + // attempt to lookup secret by partial ID (prefix) + // return error if more than one matches found (ambiguous) + n := 0 + var found *swarmapi.Secret + for _, s := range r.Secrets { + if strings.HasPrefix(s.ID, nameOrIDPrefix) { + found = s + n++ + } + } + if n > 1 { + return nil, fmt.Errorf("secret %s is ambiguous (%d matches found)", nameOrIDPrefix, n) + } + if found == nil { + return nil, fmt.Errorf("no such secret: %s", nameOrIDPrefix) + } + return found, nil +} + // GetSecret returns a secret from a managed swarm cluster -func (c *Cluster) GetSecret(id string) (types.Secret, error) { +func (c *Cluster) GetSecret(nameOrIDPrefix string) (types.Secret, error) { c.mu.RLock() defer c.mu.RUnlock() @@ -20,12 +69,11 @@ func (c *Cluster) GetSecret(id string) (types.Secret, error) { ctx, cancel := c.getRequestContext() defer cancel() - r, err := state.controlClient.GetSecret(ctx, &swarmapi.GetSecretRequest{SecretID: id}) + secret, err := getSecretByNameOrIDPrefix(ctx, &state, nameOrIDPrefix) if err != nil { return types.Secret{}, err } - - return convert.SecretFromGRPC(r.Secret), nil + return convert.SecretFromGRPC(secret), nil } // GetSecrets returns all secrets of a managed swarm cluster. @@ -85,7 +133,7 @@ func (c *Cluster) CreateSecret(s types.SecretSpec) (string, error) { } // RemoveSecret removes a secret from a managed swarm cluster. -func (c *Cluster) RemoveSecret(id string) error { +func (c *Cluster) RemoveSecret(nameOrIDPrefix string) error { c.mu.RLock() defer c.mu.RUnlock() @@ -97,11 +145,16 @@ func (c *Cluster) RemoveSecret(id string) error { ctx, cancel := c.getRequestContext() defer cancel() + secret, err := getSecretByNameOrIDPrefix(ctx, &state, nameOrIDPrefix) + if err != nil { + return err + } + req := &swarmapi.RemoveSecretRequest{ - SecretID: id, + SecretID: secret.ID, } - _, err := state.controlClient.RemoveSecret(ctx, req) + _, err = state.controlClient.RemoveSecret(ctx, req) return err } diff --git a/integration-cli/docker_cli_secret_create_test.go b/integration-cli/docker_cli_secret_create_test.go index a67b4d3927f60..2694884a1c373 100644 --- a/integration-cli/docker_cli_secret_create_test.go +++ b/integration-cli/docker_cli_secret_create_test.go @@ -101,7 +101,7 @@ func (s *DockerSwarmSuite) TestSecretCreateResolve(c *check.C) { // Remove based on ID prefix of the fake one should succeed out, err = d.Cmd("secret", "rm", fake[:5]) - c.Assert(out, checker.Contains, fake) + c.Assert(out, checker.Contains, fake[:5]) out, err = d.Cmd("secret", "ls") c.Assert(err, checker.IsNil) c.Assert(out, checker.Not(checker.Contains), name)