From 189f89301e0abfee32447f2ca23dacd3a96de06d Mon Sep 17 00:00:00 2001 From: Evan Hazlett Date: Thu, 27 Oct 2016 00:41:32 -0700 Subject: [PATCH] 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 --- api/server/router/swarm/cluster.go | 2 +- api/types/swarm/secret.go | 16 ++++++++-------- cli/command/service/opts.go | 15 +-------------- cli/command/service/parse.go | 12 +++++------- client/errors.go | 2 +- client/secret_create.go | 2 +- client/secret_create_test.go | 2 +- container/container_unix.go | 8 +++----- daemon/cluster/convert/secret.go | 2 +- daemon/cluster/executor/container/executor.go | 4 ---- daemon/oci_linux.go | 5 ++++- integration-cli/daemon_swarm.go | 4 ++-- integration-cli/docker_api_swarm_test.go | 16 ---------------- 13 files changed, 28 insertions(+), 62 deletions(-) diff --git a/api/server/router/swarm/cluster.go b/api/server/router/swarm/cluster.go index ec92e7aaa0..b6222c7498 100644 --- a/api/server/router/swarm/cluster.go +++ b/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), diff --git a/api/types/swarm/secret.go b/api/types/swarm/secret.go index 0700177588..86a6beafeb 100644 --- a/api/types/swarm/secret.go +++ b/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 } diff --git a/cli/command/service/opts.go b/cli/command/service/opts.go index a4fd08881c..3ef24ee33c 100644 --- a/cli/command/service/opts.go +++ b/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(), diff --git a/cli/command/service/parse.go b/cli/command/service/parse.go index 596d8e50d8..f3061660a2 100644 --- a/cli/command/service/parse.go +++ b/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) } diff --git a/client/errors.go b/client/errors.go index db7294daa8..94c22a728a 100644 --- a/client/errors.go +++ b/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 diff --git a/client/secret_create.go b/client/secret_create.go index de8b041567..f92a3d1510 100644 --- a/client/secret_create.go +++ b/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 } diff --git a/client/secret_create_test.go b/client/secret_create_test.go index d264eb6692..b7def89d0e 100644 --- a/client/secret_create_test.go +++ b/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) { diff --git a/container/container_unix.go b/container/container_unix.go index 6fc6c53c27..3cd0d4c17a 100644 --- a/container/container_unix.go +++ b/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 diff --git a/daemon/cluster/convert/secret.go b/daemon/cluster/convert/secret.go index fc5b43594f..acfc863efa 100644 --- a/daemon/cluster/convert/secret.go +++ b/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, diff --git a/daemon/cluster/executor/container/executor.go b/daemon/cluster/executor/container/executor.go index fdd270006d..029a4b9b9c 100644 --- a/daemon/cluster/executor/container/executor.go +++ b/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{ diff --git a/daemon/oci_linux.go b/daemon/oci_linux.go index 6889a737c5..3ce21dfb93 100644 --- a/daemon/oci_linux.go +++ b/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 { diff --git a/integration-cli/daemon_swarm.go b/integration-cli/daemon_swarm.go index 222fa018e9..ac933bb26f 100644 --- a/integration-cli/daemon_swarm.go +++ b/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 { diff --git a/integration-cli/docker_api_swarm_test.go b/integration-cli/docker_api_swarm_test.go index be6efeed4c..8fc56a1271 100644 --- a/integration-cli/docker_api_swarm_test.go +++ b/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) -//}