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 0a8bd4a23fe3bcee5da7a647d9c10c06758c3d41..fb694c5fbe0072037f48a6352857ac9dff2acade 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 f45a619f6aa835ec3fd976f80084f2006acf6de7..91ca4388f06cb85de4117c87aaed69da437730ff 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 df26f13d4651a2d72c0a70fcd79f91e0242a9dd3..8cde36f729cbb2496826936a6ec540c38c606c9b 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 a67b4d3927f60d186d52637136303889ad5d565e..2694884a1c373041cc45f0d8fb6b1f5cb4ce0a3e 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)