more review updates

- use /secrets for swarm secret create route
- do not specify omitempty for secret and secret reference
- simplify lookup for secret ids
- do not use pointer for secret grpc conversion

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

api/server/router/swarm/cluster.go

@@ -41,7 +41,7 @@ func (sr *swarmRouter) initRoutes() {
 		router.NewGetRoute("/tasks", sr.getTasks),
 		router.NewGetRoute("/tasks/{id:.*}", sr.getTask),
 		router.NewGetRoute("/secrets", sr.getSecrets),
-		router.NewPostRoute("/secrets/create", sr.createSecret),
+		router.NewPostRoute("/secrets", sr.createSecret),
 		router.NewDeleteRoute("/secrets/{id:.*}", sr.removeSecret),
 		router.NewGetRoute("/secrets/{id:.*}", sr.getSecret),
 		router.NewPostRoute("/secrets/{id:.*}/update", sr.updateSecret),

api/types/swarm/secret.go

@@ -4,14 +4,14 @@ package swarm
 type Secret struct {
 	ID string
 	Meta
-	Spec       *SecretSpec `json:",omitempty"`
-	Digest     string      `json:",omitempty"`
-	SecretSize int64       `json:",omitempty"`
+	Spec       SecretSpec
+	Digest     string
+	SecretSize int64
 }
 
 type SecretSpec struct {
 	Annotations
-	Data []byte `json:",omitempty"`
+	Data []byte
 }
 
 type SecretReferenceMode int
@@ -23,8 +23,8 @@ const (
 )
 
 type SecretReference struct {
-	SecretID   string              `json:",omitempty"`
-	Mode       SecretReferenceMode `json:",omitempty"`
-	Target     string              `json:",omitempty"`
-	SecretName string              `json:",omitempty"`
+	SecretID   string
+	Mode       SecretReferenceMode
+	Target     string
+	SecretName string
 }

cli/command/service/opts.go

@@ -191,19 +191,6 @@ func convertNetworks(networks []string) []swarm.NetworkAttachmentConfig {
 	return nets
 }
 
-func convertSecrets(secrets []string) []*swarm.SecretReference {
-	sec := []*swarm.SecretReference{}
-	for _, s := range secrets {
-		sec = append(sec, &swarm.SecretReference{
-			SecretID: s,
-			Mode:     swarm.SecretReferenceFile,
-			Target:   "",
-		})
-	}
-
-	return sec
-}
-
 type endpointOptions struct {
 	mode  string
 	ports opts.ListOpts
@@ -417,7 +404,7 @@ func (opts *serviceOptions) ToService() (swarm.ServiceSpec, error) {
 					Options:     opts.dnsOptions.GetAll(),
 				},
 				StopGracePeriod: opts.stopGrace.Value(),
-				Secrets:         convertSecrets(opts.secrets),
+				Secrets:         nil,
 			},
 			Networks:      convertNetworks(opts.networks.GetAll()),
 			Resources:     opts.resources.ToResourceRequirements(),

cli/command/service/parse.go

@@ -18,7 +18,7 @@ func parseSecretString(secretString string) (string, string, error) {
 	tokens := strings.Split(secretString, ":")
 
 	secretName := strings.TrimSpace(tokens[0])
-	targetName := ""
+	targetName := secretName
 
 	if secretName == "" {
 		return "", "", fmt.Errorf("invalid secret name provided")
@@ -29,8 +29,6 @@ func parseSecretString(secretString string) (string, string, error) {
 		if targetName == "" {
 			return "", "", fmt.Errorf("invalid presentation name provided")
 		}
-	} else {
-		targetName = secretName
 	}
 
 	// ensure target is a filename only; no paths allowed
@@ -77,22 +75,22 @@ func parseSecrets(client client.APIClient, requestedSecrets []string) ([]*swarmt
 		return nil, err
 	}
 
-	foundSecrets := make(map[string]*swarmtypes.Secret)
+	foundSecrets := make(map[string]string)
 	for _, secret := range secrets {
-		foundSecrets[secret.Spec.Annotations.Name] = &secret
+		foundSecrets[secret.Spec.Annotations.Name] = secret.ID
 	}
 
 	addedSecrets := []*swarmtypes.SecretReference{}
 
 	for secretName, secretRef := range needSecrets {
-		s, ok := foundSecrets[secretName]
+		id, ok := foundSecrets[secretName]
 		if !ok {
 			return nil, fmt.Errorf("secret not found: %s", secretName)
 		}
 
 		// set the id for the ref to properly assign in swarm
 		// since swarm needs the ID instead of the name
-		secretRef.SecretID = s.ID
+		secretRef.SecretID = id
 		addedSecrets = append(addedSecrets, secretRef)
 	}
 

client/errors.go

@@ -225,7 +225,7 @@ type secretNotFoundError struct {
 
 // Error returns a string representation of a secretNotFoundError
 func (e secretNotFoundError) Error() string {
-	return fmt.Sprintf("Error: No such secret: %s", e.name)
+	return fmt.Sprintf("Error: no such secret: %s", e.name)
 }
 
 // NoFound indicates that this error type is of NotFound

client/secret_create.go

@@ -13,7 +13,7 @@ func (cli *Client) SecretCreate(ctx context.Context, secret swarm.SecretSpec) (t
 	var headers map[string][]string
 
 	var response types.SecretCreateResponse
-	resp, err := cli.post(ctx, "/secrets/create", nil, secret, headers)
+	resp, err := cli.post(ctx, "/secrets", nil, secret, headers)
 	if err != nil {
 		return response, err
 	}

client/secret_create_test.go

@@ -25,7 +25,7 @@ func TestSecretCreateError(t *testing.T) {
 }
 
 func TestSecretCreate(t *testing.T) {
-	expectedURL := "/secrets/create"
+	expectedURL := "/secrets"
 	client := &Client{
 		client: newMockClient(func(req *http.Request) (*http.Response, error) {
 			if !strings.HasPrefix(req.URL.Path, expectedURL) {

container/container_unix.go

@@ -269,18 +269,16 @@ func (container *Container) IpcMounts() []Mount {
 }
 
 // SecretMount returns the list of Secret mounts
-func (container *Container) SecretMount() Mount {
-	var mount Mount
-
+func (container *Container) SecretMount() *Mount {
 	if len(container.Secrets) > 0 {
-		mount = Mount{
+		return &Mount{
 			Source:      container.SecretMountPath(),
 			Destination: containerSecretMountPath,
 			Writable:    false,
 		}
 	}
 
-	return mount
+	return nil
 }
 
 // UnmountSecrets unmounts the local tmpfs for secrets

daemon/cluster/convert/secret.go

@@ -19,7 +19,7 @@ func SecretFromGRPC(s *swarmapi.Secret) swarmtypes.Secret {
 	secret.CreatedAt, _ = ptypes.Timestamp(s.Meta.CreatedAt)
 	secret.UpdatedAt, _ = ptypes.Timestamp(s.Meta.UpdatedAt)
 
-	secret.Spec = &swarmtypes.SecretSpec{
+	secret.Spec = swarmtypes.SecretSpec{
 		Annotations: swarmtypes.Annotations{
 			Name:   s.Spec.Annotations.Name,
 			Labels: s.Spec.Annotations.Labels,

daemon/cluster/executor/container/executor.go

@@ -18,10 +18,6 @@ type executor struct {
 	backend executorpkg.Backend
 }
 
-type secretProvider interface {
-	Get(secretID string) *api.Secret
-}
-
 // NewExecutor returns an executor from the docker client.
 func NewExecutor(b executorpkg.Backend) exec.Executor {
 	return &executor{

daemon/oci_linux.go

@@ -710,6 +710,7 @@ func (daemon *Daemon) createSpec(c *container.Container) (*specs.Spec, error) {
 	if err != nil {
 		return nil, err
 	}
+
 	ms = append(ms, c.IpcMounts()...)
 
 	tmpfsMounts, err := c.TmpfsMounts()
@@ -718,7 +719,9 @@ func (daemon *Daemon) createSpec(c *container.Container) (*specs.Spec, error) {
 	}
 	ms = append(ms, tmpfsMounts...)
 
-	ms = append(ms, c.SecretMount())
+	if m := c.SecretMount(); m != nil {
+		ms = append(ms, *m)
+	}
 
 	sort.Sort(mounts(ms))
 	if err := setMounts(daemon, &s, c, ms); err != nil {

integration-cli/daemon_swarm.go

@@ -284,14 +284,14 @@ func (d *SwarmDaemon) listServices(c *check.C) []swarm.Service {
 	return services
 }
 
-func (d *SwarmDaemon) listSecrets(c *check.C) []swarm.Service {
+func (d *SwarmDaemon) listSecrets(c *check.C) []swarm.Secret {
 	status, out, err := d.SockRequest("GET", "/secrets", nil)
 	c.Assert(err, checker.IsNil, check.Commentf(string(out)))
 	c.Assert(status, checker.Equals, http.StatusOK, check.Commentf("output: %q", string(out)))
 
 	secrets := []swarm.Secret{}
 	c.Assert(json.Unmarshal(out, &secrets), checker.IsNil)
-	return services
+	return secrets
 }
 
 func (d *SwarmDaemon) getSwarm(c *check.C) swarm.Swarm {

integration-cli/docker_api_swarm_test.go

@@ -1271,19 +1271,3 @@ func (s *DockerSwarmSuite) TestAPISwarmSecretsEmptyList(c *check.C) {
 	c.Assert(secrets, checker.NotNil)
 	c.Assert(len(secrets), checker.Equals, 0, check.Commentf("secrets: %#v", secrets))
 }
-
-//func (s *DockerSwarmSuite) TestAPISwarmServicesCreate(c *check.C) {
-//	d := s.AddDaemon(c, true, true)
-//
-//	instances := 2
-//	id := d.createService(c, simpleTestService, setInstances(instances))
-//	waitAndAssert(c, defaultReconciliationTimeout, d.checkActiveContainerCount, checker.Equals, instances)
-//
-//	service := d.getService(c, id)
-//	instances = 5
-//	d.updateService(c, service, setInstances(instances))
-//	waitAndAssert(c, defaultReconciliationTimeout, d.checkActiveContainerCount, checker.Equals, instances)
-//
-//	d.removeService(c, service.ID)
-//	waitAndAssert(c, defaultReconciliationTimeout, d.checkActiveContainerCount, checker.Equals, 0)
-//}