review updates

- use Filters instead of Filter for secret list
- UID, GID -> string
- getSecrets -> getSecretsByName
- updated test case for secrets with better source
- use golang.org/x/context instead of context
- for grpc conversion allocate with make
- check for nil with task.Spec.GetContainer()

Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>

api/server/router/swarm/cluster_routes.go

@@ -267,14 +267,13 @@ func (sr *swarmRouter) getSecrets(ctx context.Context, w http.ResponseWriter, r
 	if err := httputils.ParseForm(r); err != nil {
 		return err
 	}
-	filter, err := filters.FromParam(r.Form.Get("filters"))
+	filters, err := filters.FromParam(r.Form.Get("filters"))
 	if err != nil {
 		return err
 	}
 
-	secrets, err := sr.backend.GetSecrets(basictypes.SecretListOptions{Filter: filter})
+	secrets, err := sr.backend.GetSecrets(basictypes.SecretListOptions{Filters: filters})
 	if err != nil {
-		logrus.Errorf("Error getting secrets: %v", err)
 		return err
 	}
 
@@ -289,7 +288,6 @@ func (sr *swarmRouter) createSecret(ctx context.Context, w http.ResponseWriter,
 
 	id, err := sr.backend.CreateSecret(secret)
 	if err != nil {
-		logrus.Errorf("Error creating secret %s: %v", id, err)
 		return err
 	}
 
@@ -300,7 +298,6 @@ func (sr *swarmRouter) createSecret(ctx context.Context, w http.ResponseWriter,
 
 func (sr *swarmRouter) removeSecret(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
 	if err := sr.backend.RemoveSecret(vars["id"]); err != nil {
-		logrus.Errorf("Error removing secret %s: %v", vars["id"], err)
 		return err
 	}
 
@@ -310,7 +307,6 @@ func (sr *swarmRouter) removeSecret(ctx context.Context, w http.ResponseWriter,
 func (sr *swarmRouter) getSecret(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
 	secret, err := sr.backend.GetSecret(vars["id"])
 	if err != nil {
-		logrus.Errorf("Error getting secret %s: %v", vars["id"], err)
 		return err
 	}
 

api/types/container/secret.go

@@ -6,7 +6,7 @@ type ContainerSecret struct {
 	Name   string
 	Target string
 	Data   []byte
-	UID    int
-	GID    int
+	UID    string
+	GID    string
 	Mode   os.FileMode
 }

api/types/types.go

@@ -520,5 +520,5 @@ type SecretCreateResponse struct {
 
 // SecretListOptions holds parameters to list secrets
 type SecretListOptions struct {
-	Filter filters.Args
+	Filters filters.Args
 }

cli/command/secret/inspect.go

@@ -35,7 +35,7 @@ func runSecretInspect(dockerCli *command.DockerCli, opts inspectOptions) error {
 	ctx := context.Background()
 
 	// attempt to lookup secret by name
-	secrets, err := getSecrets(client, ctx, []string{opts.name})
+	secrets, err := getSecretsByName(client, ctx, []string{opts.name})
 	if err != nil {
 		return err
 	}

cli/command/secret/remove.go

@@ -32,7 +32,7 @@ func runSecretRemove(dockerCli *command.DockerCli, opts removeOptions) error {
 	ctx := context.Background()
 
 	// attempt to lookup secret by name
-	secrets, err := getSecrets(client, ctx, opts.ids)
+	secrets, err := getSecretsByName(client, ctx, opts.ids)
 	if err != nil {
 		return err
 	}

cli/command/secret/utils.go

@@ -9,13 +9,13 @@ import (
 	"github.com/docker/docker/client"
 )
 
-func getSecrets(client client.APIClient, ctx context.Context, names []string) ([]swarm.Secret, error) {
+func getSecretsByName(client client.APIClient, ctx context.Context, names []string) ([]swarm.Secret, error) {
 	args := filters.NewArgs()
 	for _, n := range names {
 		args.Add("names", n)
 	}
 
 	return client.SecretList(ctx, types.SecretListOptions{
-		Filter: args,
+		Filters: args,
 	})
 }

cli/command/service/opts_test.go

@@ -108,45 +108,45 @@ func TestHealthCheckOptionsToHealthConfigConflict(t *testing.T) {
 }
 
 func TestSecretOptionsSimple(t *testing.T) {
-	var opt SecretOpt
+	var opt opts.SecretOpt
 
-	testCase := "source=/foo,target=testing"
+	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")
+	assert.Equal(t, req.Source, "foo")
+	assert.Equal(t, req.Target, "testing")
 }
 
 func TestSecretOptionsCustomUidGid(t *testing.T) {
-	var opt SecretOpt
+	var opt opts.SecretOpt
 
-	testCase := "source=/foo,target=testing,uid=1000,gid=1001"
+	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")
+	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
+	var opt opts.SecretOpt
 
-	testCase := "source=/foo,target=testing,uid=1000,gid=1001,mode=0444"
+	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))
+	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))
 }

cli/command/service/parse.go

@@ -1,13 +1,13 @@
 package service
 
 import (
-	"context"
 	"fmt"
 
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/filters"
 	swarmtypes "github.com/docker/docker/api/types/swarm"
 	"github.com/docker/docker/client"
+	"golang.org/x/net/context"
 )
 
 // parseSecrets retrieves the secrets from the requested names and converts
@@ -39,7 +39,7 @@ func parseSecrets(client client.APIClient, requestedSecrets []*types.SecretReque
 	}
 
 	secrets, err := client.SecretList(ctx, types.SecretListOptions{
-		Filter: args,
+		Filters: args,
 	})
 	if err != nil {
 		return nil, err

client/secret_list.go

@@ -14,8 +14,8 @@ import (
 func (cli *Client) SecretList(ctx context.Context, options types.SecretListOptions) ([]swarm.Secret, error) {
 	query := url.Values{}
 
-	if options.Filter.Len() > 0 {
-		filterJSON, err := filters.ToParam(options.Filter)
+	if options.Filters.Len() > 0 {
+		filterJSON, err := filters.ToParam(options.Filters)
 		if err != nil {
 			return nil, err
 		}

client/secret_list_test.go

@@ -45,7 +45,7 @@ func TestSecretList(t *testing.T) {
 		},
 		{
 			options: types.SecretListOptions{
-				Filter: filters,
+				Filters: filters,
 			},
 			expectedQueryParams: map[string]string{
 				"filters": `{"label":{"label1":true,"label2":true}}`,

daemon/cluster/convert/container.go

@@ -78,7 +78,7 @@ func containerSpecFromGRPC(c *swarmapi.ContainerSpec) types.ContainerSpec {
 }
 
 func secretReferencesToGRPC(sr []*types.SecretReference) []*swarmapi.SecretReference {
-	refs := []*swarmapi.SecretReference{}
+	refs := make([]*swarmapi.SecretReference, 0, len(sr))
 	for _, s := range sr {
 		refs = append(refs, &swarmapi.SecretReference{
 			SecretID:   s.SecretID,
@@ -97,7 +97,7 @@ func secretReferencesToGRPC(sr []*types.SecretReference) []*swarmapi.SecretRefer
 	return refs
 }
 func secretReferencesFromGRPC(sr []*swarmapi.SecretReference) []*types.SecretReference {
-	refs := []*types.SecretReference{}
+	refs := make([]*types.SecretReference, 0, len(sr))
 	for _, s := range sr {
 		target := s.GetFile()
 		if target == nil {

daemon/cluster/executor/container/adapter.go

@@ -5,7 +5,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
-	"strconv"
 	"strings"
 	"syscall"
 	"time"
@@ -219,7 +218,11 @@ func (c *containerAdapter) create(ctx context.Context) error {
 		}
 	}
 
-	secrets := []*containertypes.ContainerSecret{}
+	container := c.container.task.Spec.GetContainer()
+	if container == nil {
+		return fmt.Errorf("unable to get container from task spec")
+	}
+	secrets := make([]*containertypes.ContainerSecret, 0, len(container.Secrets))
 	for _, s := range c.container.task.Spec.GetContainer().Secrets {
 		sec := c.secrets.Get(s.SecretID)
 		if sec == nil {
@@ -233,23 +236,13 @@ func (c *containerAdapter) create(ctx context.Context) error {
 			logrus.Warnf("secret target was not a file: secret=%s", s.SecretID)
 			continue
 		}
-		// convert uid / gid string to int
-		uid, err := strconv.Atoi(target.UID)
-		if err != nil {
-			return err
-		}
-
-		gid, err := strconv.Atoi(target.GID)
-		if err != nil {
-			return err
-		}
 
 		secrets = append(secrets, &containertypes.ContainerSecret{
 			Name:   name,
 			Target: target.Name,
 			Data:   sec.Spec.Data,
-			UID:    uid,
-			GID:    gid,
+			UID:    target.UID,
+			GID:    target.GID,
 			Mode:   target.Mode,
 		})
 	}

daemon/cluster/secrets.go

@@ -29,7 +29,7 @@ func (c *Cluster) GetSecrets(options apitypes.SecretListOptions) ([]types.Secret
 		return nil, c.errNoManager()
 	}
 
-	filters, err := newListSecretsFilters(options.Filter)
+	filters, err := newListSecretsFilters(options.Filters)
 	if err != nil {
 		return nil, err
 	}
@@ -97,6 +97,7 @@ func (c *Cluster) RemoveSecret(id string) error {
 }
 
 // UpdateSecret updates a secret in a managed swarm cluster.
+// Note: this is not exposed to the CLI but is available from the API only
 func (c *Cluster) UpdateSecret(id string, version uint64, spec types.SecretSpec) error {
 	c.RLock()
 	defer c.RUnlock()

daemon/container_operations_unix.go

@@ -191,7 +191,16 @@ func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) {
 			return errors.Wrap(err, "error injecting secret")
 		}
 
-		if err := os.Chown(fPath, s.UID, s.GID); err != nil {
+		uid, err := strconv.Atoi(s.UID)
+		if err != nil {
+			return err
+		}
+		gid, err := strconv.Atoi(s.GID)
+		if err != nil {
+			return err
+		}
+
+		if err := os.Chown(fPath, uid, gid); err != nil {
 			return errors.Wrap(err, "error setting ownership for secret")
 		}
 	}