Browse code

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 <yong.tang.github@outlook.com>

Yong Tang authored on 2016/12/08 04:06:07
Showing 4 changed files
... ...
@@ -33,13 +33,9 @@ func runSecretInspect(dockerCli *command.DockerCli, opts inspectOptions) error {
33 33
 	client := dockerCli.Client()
34 34
 	ctx := context.Background()
35 35
 
36
-	ids, err := getCliRequestedSecretIDs(ctx, client, opts.names)
37
-	if err != nil {
38
-		return err
39
-	}
40 36
 	getRef := func(id string) (interface{}, []byte, error) {
41 37
 		return client.SecretInspectWithRaw(ctx, id)
42 38
 	}
43 39
 
44
-	return inspect.Inspect(dockerCli.Out(), ids, opts.format, getRef)
40
+	return inspect.Inspect(dockerCli.Out(), opts.names, opts.format, getRef)
45 41
 }
... ...
@@ -33,20 +33,15 @@ func runSecretRemove(dockerCli *command.DockerCli, opts removeOptions) error {
33 33
 	client := dockerCli.Client()
34 34
 	ctx := context.Background()
35 35
 
36
-	ids, err := getCliRequestedSecretIDs(ctx, client, opts.names)
37
-	if err != nil {
38
-		return err
39
-	}
40
-
41 36
 	var errs []string
42 37
 
43
-	for _, id := range ids {
44
-		if err := client.SecretRemove(ctx, id); err != nil {
38
+	for _, name := range opts.names {
39
+		if err := client.SecretRemove(ctx, name); err != nil {
45 40
 			errs = append(errs, err.Error())
46 41
 			continue
47 42
 		}
48 43
 
49
-		fmt.Fprintln(dockerCli.Out(), id)
44
+		fmt.Fprintln(dockerCli.Out(), name)
50 45
 	}
51 46
 
52 47
 	if len(errs) > 0 {
... ...
@@ -1,14 +1,63 @@
1 1
 package cluster
2 2
 
3 3
 import (
4
+	"fmt"
5
+	"strings"
6
+
4 7
 	apitypes "github.com/docker/docker/api/types"
5 8
 	types "github.com/docker/docker/api/types/swarm"
6 9
 	"github.com/docker/docker/daemon/cluster/convert"
7 10
 	swarmapi "github.com/docker/swarmkit/api"
11
+	"golang.org/x/net/context"
8 12
 )
9 13
 
14
+func getSecretByNameOrIDPrefix(ctx context.Context, state *nodeState, nameOrIDPrefix string) (*swarmapi.Secret, error) {
15
+	// attempt to lookup secret by full ID
16
+	if r, err := state.controlClient.GetSecret(ctx, &swarmapi.GetSecretRequest{
17
+		SecretID: nameOrIDPrefix,
18
+	}); err == nil {
19
+		return r.Secret, nil
20
+	}
21
+
22
+	// attempt to lookup secret by full name and partial ID
23
+	// Note here ListSecretRequest_Filters operate with `or`
24
+	r, err := state.controlClient.ListSecrets(ctx, &swarmapi.ListSecretsRequest{
25
+		Filters: &swarmapi.ListSecretsRequest_Filters{
26
+			Names:      []string{nameOrIDPrefix},
27
+			IDPrefixes: []string{nameOrIDPrefix},
28
+		},
29
+	})
30
+	if err != nil {
31
+		return nil, err
32
+	}
33
+
34
+	// attempt to lookup secret by full name
35
+	for _, s := range r.Secrets {
36
+		if s.Spec.Annotations.Name == nameOrIDPrefix {
37
+			return s, nil
38
+		}
39
+	}
40
+	// attempt to lookup secret by partial ID (prefix)
41
+	// return error if more than one matches found (ambiguous)
42
+	n := 0
43
+	var found *swarmapi.Secret
44
+	for _, s := range r.Secrets {
45
+		if strings.HasPrefix(s.ID, nameOrIDPrefix) {
46
+			found = s
47
+			n++
48
+		}
49
+	}
50
+	if n > 1 {
51
+		return nil, fmt.Errorf("secret %s is ambiguous (%d matches found)", nameOrIDPrefix, n)
52
+	}
53
+	if found == nil {
54
+		return nil, fmt.Errorf("no such secret: %s", nameOrIDPrefix)
55
+	}
56
+	return found, nil
57
+}
58
+
10 59
 // GetSecret returns a secret from a managed swarm cluster
11
-func (c *Cluster) GetSecret(id string) (types.Secret, error) {
60
+func (c *Cluster) GetSecret(nameOrIDPrefix string) (types.Secret, error) {
12 61
 	c.mu.RLock()
13 62
 	defer c.mu.RUnlock()
14 63
 
... ...
@@ -20,12 +69,11 @@ func (c *Cluster) GetSecret(id string) (types.Secret, error) {
20 20
 	ctx, cancel := c.getRequestContext()
21 21
 	defer cancel()
22 22
 
23
-	r, err := state.controlClient.GetSecret(ctx, &swarmapi.GetSecretRequest{SecretID: id})
23
+	secret, err := getSecretByNameOrIDPrefix(ctx, &state, nameOrIDPrefix)
24 24
 	if err != nil {
25 25
 		return types.Secret{}, err
26 26
 	}
27
-
28
-	return convert.SecretFromGRPC(r.Secret), nil
27
+	return convert.SecretFromGRPC(secret), nil
29 28
 }
30 29
 
31 30
 // GetSecrets returns all secrets of a managed swarm cluster.
... ...
@@ -85,7 +133,7 @@ func (c *Cluster) CreateSecret(s types.SecretSpec) (string, error) {
85 85
 }
86 86
 
87 87
 // RemoveSecret removes a secret from a managed swarm cluster.
88
-func (c *Cluster) RemoveSecret(id string) error {
88
+func (c *Cluster) RemoveSecret(nameOrIDPrefix string) error {
89 89
 	c.mu.RLock()
90 90
 	defer c.mu.RUnlock()
91 91
 
... ...
@@ -97,11 +145,16 @@ func (c *Cluster) RemoveSecret(id string) error {
97 97
 	ctx, cancel := c.getRequestContext()
98 98
 	defer cancel()
99 99
 
100
+	secret, err := getSecretByNameOrIDPrefix(ctx, &state, nameOrIDPrefix)
101
+	if err != nil {
102
+		return err
103
+	}
104
+
100 105
 	req := &swarmapi.RemoveSecretRequest{
101
-		SecretID: id,
106
+		SecretID: secret.ID,
102 107
 	}
103 108
 
104
-	_, err := state.controlClient.RemoveSecret(ctx, req)
109
+	_, err = state.controlClient.RemoveSecret(ctx, req)
105 110
 	return err
106 111
 }
107 112
 
... ...
@@ -101,7 +101,7 @@ func (s *DockerSwarmSuite) TestSecretCreateResolve(c *check.C) {
101 101
 
102 102
 	// Remove based on ID prefix of the fake one should succeed
103 103
 	out, err = d.Cmd("secret", "rm", fake[:5])
104
-	c.Assert(out, checker.Contains, fake)
104
+	c.Assert(out, checker.Contains, fake[:5])
105 105
 	out, err = d.Cmd("secret", "ls")
106 106
 	c.Assert(err, checker.IsNil)
107 107
 	c.Assert(out, checker.Not(checker.Contains), name)