From a6e6cffaed2a5b6b96210b607f805adbee2fa68a Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Thu, 15 Jun 2017 11:21:05 -0700 Subject: [PATCH 1/8] executor: Use a TemplatedDependencyGetter to support template expansion Signed-off-by: Aaron Lehmann Upstream-commit: 56da5fd7d31c9a627fc6a3c482cb0bf0ffb2d26e Component: engine --- .../engine/daemon/cluster/executor/container/executor.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/components/engine/daemon/cluster/executor/container/executor.go b/components/engine/daemon/cluster/executor/container/executor.go index 48ec7e0d25..2c4f619cf0 100644 --- a/components/engine/daemon/cluster/executor/container/executor.go +++ b/components/engine/daemon/cluster/executor/container/executor.go @@ -19,6 +19,7 @@ import ( "github.com/docker/swarmkit/agent/exec" "github.com/docker/swarmkit/api" "github.com/docker/swarmkit/api/naming" + "github.com/docker/swarmkit/template" "github.com/sirupsen/logrus" "golang.org/x/net/context" ) @@ -191,7 +192,7 @@ func (e *executor) Configure(ctx context.Context, node *api.Node) error { // Controller returns a docker container runner. func (e *executor) Controller(t *api.Task) (exec.Controller, error) { - dependencyGetter := agent.Restrict(e.dependencies, t) + dependencyGetter := template.NewTemplatedDependencyGetter(agent.Restrict(e.dependencies, t), t, nil) // Get the node description from the executor field e.mutex.Lock() From fc6a93f926fc35f01245f9a586df57c8c238c1d7 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Thu, 15 Jun 2017 11:28:41 -0700 Subject: [PATCH 2/8] api: Add Templating parameter to SecretSpec and ConfigSpec Signed-off-by: Aaron Lehmann Upstream-commit: c5df7235f6a4811f26b37441db401f6b04858504 Component: engine --- components/engine/api/swagger.yaml | 24 +++++++++++++++++++ components/engine/api/types/swarm/config.go | 4 ++++ components/engine/api/types/swarm/secret.go | 4 ++++ .../engine/daemon/cluster/convert/config.go | 19 ++++++++++++++- .../engine/daemon/cluster/convert/secret.go | 19 ++++++++++++++- 5 files changed, 68 insertions(+), 2 deletions(-) diff --git a/components/engine/api/swagger.yaml b/components/engine/api/swagger.yaml index 8174ce387c..12fdd57537 100644 --- a/components/engine/api/swagger.yaml +++ b/components/engine/api/swagger.yaml @@ -3338,6 +3338,18 @@ definitions: Driver: description: "Name of the secrets driver used to fetch the secret's value from an external secret store" $ref: "#/definitions/Driver" + Templating: + description: "Templating driver, if applicable" + type: "object" + properties: + Name: + description: "Name of the templating driver (i.e. 'golang')" + type: "string" + Options: + description: "key/value map of driver specific options." + type: "object" + additionalProperties: + type: "string" Secret: type: "object" @@ -3374,6 +3386,18 @@ definitions: Base64-url-safe-encoded ([RFC 4648](https://tools.ietf.org/html/rfc4648#section-3.2)) config data. type: "string" + Templating: + description: "Templating driver, if applicable" + type: "object" + properties: + Name: + description: "Name of the templating driver (i.e. 'golang')" + type: "string" + Options: + description: "key/value map of driver specific options." + type: "object" + additionalProperties: + type: "string" Config: type: "object" diff --git a/components/engine/api/types/swarm/config.go b/components/engine/api/types/swarm/config.go index c1fdf3b3e4..a1555cf43e 100644 --- a/components/engine/api/types/swarm/config.go +++ b/components/engine/api/types/swarm/config.go @@ -13,6 +13,10 @@ type Config struct { type ConfigSpec struct { Annotations Data []byte `json:",omitempty"` + + // Templating controls whether and how to evaluate the config payload as + // a template. If it is not set, no templating is used. + Templating *Driver `json:",omitempty"` } // ConfigReferenceFileTarget is a file target in a config reference diff --git a/components/engine/api/types/swarm/secret.go b/components/engine/api/types/swarm/secret.go index cfba1141d8..d5213ec981 100644 --- a/components/engine/api/types/swarm/secret.go +++ b/components/engine/api/types/swarm/secret.go @@ -14,6 +14,10 @@ type SecretSpec struct { Annotations Data []byte `json:",omitempty"` Driver *Driver `json:",omitempty"` // name of the secrets driver used to fetch the secret's value from an external secret store + + // Templating controls whether and how to evaluate the secret payload as + // a template. If it is not set, no templating is used. + Templating *Driver `json:",omitempty"` } // SecretReferenceFileTarget is a file target in a secret reference diff --git a/components/engine/daemon/cluster/convert/config.go b/components/engine/daemon/cluster/convert/config.go index ba7920ec94..16b3475af8 100644 --- a/components/engine/daemon/cluster/convert/config.go +++ b/components/engine/daemon/cluster/convert/config.go @@ -2,6 +2,7 @@ package convert // import "github.com/docker/docker/daemon/cluster/convert" import ( swarmtypes "github.com/docker/docker/api/types/swarm" + types "github.com/docker/docker/api/types/swarm" swarmapi "github.com/docker/swarmkit/api" gogotypes "github.com/gogo/protobuf/types" ) @@ -21,18 +22,34 @@ func ConfigFromGRPC(s *swarmapi.Config) swarmtypes.Config { config.CreatedAt, _ = gogotypes.TimestampFromProto(s.Meta.CreatedAt) config.UpdatedAt, _ = gogotypes.TimestampFromProto(s.Meta.UpdatedAt) + if s.Spec.Templating != nil { + config.Spec.Templating = &types.Driver{ + Name: s.Spec.Templating.Name, + Options: s.Spec.Templating.Options, + } + } + return config } // ConfigSpecToGRPC converts Config to a grpc Config. func ConfigSpecToGRPC(s swarmtypes.ConfigSpec) swarmapi.ConfigSpec { - return swarmapi.ConfigSpec{ + spec := swarmapi.ConfigSpec{ Annotations: swarmapi.Annotations{ Name: s.Name, Labels: s.Labels, }, Data: s.Data, } + + if s.Templating != nil { + spec.Templating = &swarmapi.Driver{ + Name: s.Templating.Name, + Options: s.Templating.Options, + } + } + + return spec } // ConfigReferencesFromGRPC converts a slice of grpc ConfigReference to ConfigReference diff --git a/components/engine/daemon/cluster/convert/secret.go b/components/engine/daemon/cluster/convert/secret.go index 3ec2a353dd..d0e5ac45d2 100644 --- a/components/engine/daemon/cluster/convert/secret.go +++ b/components/engine/daemon/cluster/convert/secret.go @@ -2,6 +2,7 @@ package convert // import "github.com/docker/docker/daemon/cluster/convert" import ( swarmtypes "github.com/docker/docker/api/types/swarm" + types "github.com/docker/docker/api/types/swarm" swarmapi "github.com/docker/swarmkit/api" gogotypes "github.com/gogo/protobuf/types" ) @@ -22,12 +23,19 @@ func SecretFromGRPC(s *swarmapi.Secret) swarmtypes.Secret { secret.CreatedAt, _ = gogotypes.TimestampFromProto(s.Meta.CreatedAt) secret.UpdatedAt, _ = gogotypes.TimestampFromProto(s.Meta.UpdatedAt) + if s.Spec.Templating != nil { + secret.Spec.Templating = &types.Driver{ + Name: s.Spec.Templating.Name, + Options: s.Spec.Templating.Options, + } + } + return secret } // SecretSpecToGRPC converts Secret to a grpc Secret. func SecretSpecToGRPC(s swarmtypes.SecretSpec) swarmapi.SecretSpec { - return swarmapi.SecretSpec{ + spec := swarmapi.SecretSpec{ Annotations: swarmapi.Annotations{ Name: s.Name, Labels: s.Labels, @@ -35,6 +43,15 @@ func SecretSpecToGRPC(s swarmtypes.SecretSpec) swarmapi.SecretSpec { Data: s.Data, Driver: driverToGRPC(s.Driver), } + + if s.Templating != nil { + spec.Templating = &swarmapi.Driver{ + Name: s.Templating.Name, + Options: s.Templating.Options, + } + } + + return spec } // SecretReferencesFromGRPC converts a slice of grpc SecretReference to SecretReference From 82ebb2a6fde351faada2ddcbd5edc5d3f4ca2072 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Thu, 15 Jun 2017 12:06:08 -0700 Subject: [PATCH 3/8] integration-cli: Add secret/config templating tests Signed-off-by: Aaron Lehmann Upstream-commit: cdd2e6efdbf402c629844cb20955f160327917b9 Component: engine --- .../engine/integration/config/config_test.go | 130 ++++++++++++++++++ .../integration/internal/swarm/service.go | 122 ++++++++++++++++ .../engine/integration/secret/secret_test.go | 130 ++++++++++++++++++ 3 files changed, 382 insertions(+) diff --git a/components/engine/integration/config/config_test.go b/components/engine/integration/config/config_test.go index fa2a205953..6a96986e4a 100644 --- a/components/engine/integration/config/config_test.go +++ b/components/engine/integration/config/config_test.go @@ -1,8 +1,10 @@ package config import ( + "bytes" "sort" "testing" + "time" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/filters" @@ -10,6 +12,7 @@ import ( "github.com/docker/docker/client" "github.com/docker/docker/integration/internal/swarm" "github.com/docker/docker/internal/testutil" + "github.com/docker/docker/pkg/stdcopy" "github.com/gotestyourself/gotestyourself/skip" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -188,3 +191,130 @@ func TestConfigsUpdate(t *testing.T) { err = client.ConfigUpdate(ctx, configID, insp.Version, insp.Spec) testutil.ErrorContains(t, err, "only updates to Labels are allowed") } + +func TestTemplatedConfig(t *testing.T) { + d := swarm.NewSwarm(t, testEnv) + defer d.Stop(t) + + ctx := context.Background() + client := swarm.GetClient(t, d) + + referencedSecretSpec := swarmtypes.SecretSpec{ + Annotations: swarmtypes.Annotations{ + Name: "referencedsecret", + }, + Data: []byte("this is a secret"), + } + referencedSecret, err := client.SecretCreate(ctx, referencedSecretSpec) + assert.NoError(t, err) + + referencedConfigSpec := swarmtypes.ConfigSpec{ + Annotations: swarmtypes.Annotations{ + Name: "referencedconfig", + }, + Data: []byte("this is a config"), + } + referencedConfig, err := client.ConfigCreate(ctx, referencedConfigSpec) + assert.NoError(t, err) + + configSpec := swarmtypes.ConfigSpec{ + Annotations: swarmtypes.Annotations{ + Name: "templated_config", + }, + Templating: &swarmtypes.Driver{ + Name: "golang", + }, + Data: []byte("SERVICE_NAME={{.Service.Name}}\n" + + "{{secret \"referencedsecrettarget\"}}\n" + + "{{config \"referencedconfigtarget\"}}\n"), + } + + templatedConfig, err := client.ConfigCreate(ctx, configSpec) + assert.NoError(t, err) + + serviceID := swarm.CreateService(t, d, + swarm.ServiceWithConfig( + &swarmtypes.ConfigReference{ + File: &swarmtypes.ConfigReferenceFileTarget{ + Name: "/templated_config", + UID: "0", + GID: "0", + Mode: 0600, + }, + ConfigID: templatedConfig.ID, + ConfigName: "templated_config", + }, + ), + swarm.ServiceWithConfig( + &swarmtypes.ConfigReference{ + File: &swarmtypes.ConfigReferenceFileTarget{ + Name: "referencedconfigtarget", + UID: "0", + GID: "0", + Mode: 0600, + }, + ConfigID: referencedConfig.ID, + ConfigName: "referencedconfig", + }, + ), + swarm.ServiceWithSecret( + &swarmtypes.SecretReference{ + File: &swarmtypes.SecretReferenceFileTarget{ + Name: "referencedsecrettarget", + UID: "0", + GID: "0", + Mode: 0600, + }, + SecretID: referencedSecret.ID, + SecretName: "referencedsecret", + }, + ), + swarm.ServiceWithName("svc"), + ) + + var tasks []swarmtypes.Task + waitAndAssert(t, 60*time.Second, func(t *testing.T) bool { + tasks = swarm.GetRunningTasks(t, d, serviceID) + return len(tasks) > 0 + }) + + task := tasks[0] + waitAndAssert(t, 60*time.Second, func(t *testing.T) bool { + if task.NodeID == "" || (task.Status.ContainerStatus == nil || task.Status.ContainerStatus.ContainerID == "") { + task, _, _ = client.TaskInspectWithRaw(context.Background(), task.ID) + } + return task.NodeID != "" && task.Status.ContainerStatus != nil && task.Status.ContainerStatus.ContainerID != "" + }) + + attach := swarm.ExecTask(t, d, task, types.ExecConfig{ + Cmd: []string{"/bin/cat", "/templated_config"}, + AttachStdout: true, + AttachStderr: true, + }) + + buf := bytes.NewBuffer(nil) + _, err = stdcopy.StdCopy(buf, buf, attach.Reader) + require.NoError(t, err) + + expect := "SERVICE_NAME=svc\n" + + "this is a secret\n" + + "this is a config\n" + + assert.Equal(t, expect, buf.String()) +} + +func waitAndAssert(t *testing.T, timeout time.Duration, f func(*testing.T) bool) { + t.Helper() + after := time.After(timeout) + for { + select { + case <-after: + t.Fatalf("timed out waiting for condition") + default: + } + if f(t) { + return + } + time.Sleep(100 * time.Millisecond) + } +} diff --git a/components/engine/integration/internal/swarm/service.go b/components/engine/integration/internal/swarm/service.go index bd001ba0b4..a46b02e146 100644 --- a/components/engine/integration/internal/swarm/service.go +++ b/components/engine/integration/internal/swarm/service.go @@ -1,10 +1,14 @@ package swarm import ( + "context" "fmt" "testing" + "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" "github.com/docker/docker/integration-cli/daemon" "github.com/docker/docker/internal/test/environment" "github.com/stretchr/testify/require" @@ -34,3 +38,121 @@ func NewSwarm(t *testing.T, testEnv *environment.Execution) *daemon.Swarm { require.NoError(t, d.Init(swarmtypes.InitRequest{})) return d } + +// ServiceSpecOpt is used with `CreateService` to pass in service spec modifiers +type ServiceSpecOpt func(*swarmtypes.ServiceSpec) + +// CreateService creates a service on the passed in swarm daemon. +func CreateService(t *testing.T, d *daemon.Swarm, opts ...ServiceSpecOpt) string { + spec := defaultServiceSpec() + for _, o := range opts { + o(&spec) + } + + client := GetClient(t, d) + + resp, err := client.ServiceCreate(context.Background(), spec, types.ServiceCreateOptions{}) + require.NoError(t, err, "error creating service") + return resp.ID +} + +func defaultServiceSpec() swarmtypes.ServiceSpec { + var spec swarmtypes.ServiceSpec + ServiceWithImage("busybox:latest")(&spec) + ServiceWithCommand([]string{"/bin/top"})(&spec) + ServiceWithReplicas(1)(&spec) + return spec +} + +// ServiceWithImage sets the image to use for the service +func ServiceWithImage(image string) func(*swarmtypes.ServiceSpec) { + return func(spec *swarmtypes.ServiceSpec) { + ensureContainerSpec(spec) + spec.TaskTemplate.ContainerSpec.Image = image + } +} + +// ServiceWithCommand sets the command to use for the service +func ServiceWithCommand(cmd []string) ServiceSpecOpt { + return func(spec *swarmtypes.ServiceSpec) { + ensureContainerSpec(spec) + spec.TaskTemplate.ContainerSpec.Command = cmd + } +} + +// ServiceWithConfig adds the config reference to the service +func ServiceWithConfig(configRef *swarmtypes.ConfigReference) ServiceSpecOpt { + return func(spec *swarmtypes.ServiceSpec) { + ensureContainerSpec(spec) + spec.TaskTemplate.ContainerSpec.Configs = append(spec.TaskTemplate.ContainerSpec.Configs, configRef) + } +} + +// ServiceWithSecret adds the secret reference to the service +func ServiceWithSecret(secretRef *swarmtypes.SecretReference) ServiceSpecOpt { + return func(spec *swarmtypes.ServiceSpec) { + ensureContainerSpec(spec) + spec.TaskTemplate.ContainerSpec.Secrets = append(spec.TaskTemplate.ContainerSpec.Secrets, secretRef) + } +} + +// ServiceWithReplicas sets the replicas for the service +func ServiceWithReplicas(n uint64) ServiceSpecOpt { + return func(spec *swarmtypes.ServiceSpec) { + spec.Mode = swarmtypes.ServiceMode{ + Replicated: &swarmtypes.ReplicatedService{ + Replicas: &n, + }, + } + } +} + +// ServiceWithName sets the name of the service +func ServiceWithName(name string) ServiceSpecOpt { + return func(spec *swarmtypes.ServiceSpec) { + spec.Annotations.Name = name + } +} + +// GetRunningTasks gets the list of running tasks for a service +func GetRunningTasks(t *testing.T, d *daemon.Swarm, serviceID string) []swarmtypes.Task { + client := GetClient(t, d) + + filterArgs := filters.NewArgs() + filterArgs.Add("desired-state", "running") + filterArgs.Add("service", serviceID) + + options := types.TaskListOptions{ + Filters: filterArgs, + } + tasks, err := client.TaskList(context.Background(), options) + require.NoError(t, err) + return tasks +} + +// ExecTask runs the passed in exec config on the given task +func ExecTask(t *testing.T, d *daemon.Swarm, task swarmtypes.Task, config types.ExecConfig) types.HijackedResponse { + client := GetClient(t, d) + + ctx := context.Background() + resp, err := client.ContainerExecCreate(ctx, task.Status.ContainerStatus.ContainerID, config) + require.NoError(t, err, "error creating exec") + + startCheck := types.ExecStartCheck{} + attach, err := client.ContainerExecAttach(ctx, resp.ID, startCheck) + require.NoError(t, err, "error attaching to exec") + return attach +} + +func ensureContainerSpec(spec *swarmtypes.ServiceSpec) { + if spec.TaskTemplate.ContainerSpec == nil { + spec.TaskTemplate.ContainerSpec = &swarmtypes.ContainerSpec{} + } +} + +// GetClient creates a new client for the passed in swarm daemon. +func GetClient(t *testing.T, d *daemon.Swarm) client.APIClient { + client, err := client.NewClientWithOpts(client.WithHost((d.Sock()))) + require.NoError(t, err) + return client +} diff --git a/components/engine/integration/secret/secret_test.go b/components/engine/integration/secret/secret_test.go index a6e9983e23..e26d8a698d 100644 --- a/components/engine/integration/secret/secret_test.go +++ b/components/engine/integration/secret/secret_test.go @@ -1,8 +1,10 @@ package secret import ( + "bytes" "sort" "testing" + "time" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/filters" @@ -10,6 +12,7 @@ import ( "github.com/docker/docker/client" "github.com/docker/docker/integration/internal/swarm" "github.com/docker/docker/internal/testutil" + "github.com/docker/docker/pkg/stdcopy" "github.com/gotestyourself/gotestyourself/skip" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -232,3 +235,130 @@ func TestSecretsUpdate(t *testing.T) { err = client.SecretUpdate(ctx, secretID, insp.Version, insp.Spec) testutil.ErrorContains(t, err, "only updates to Labels are allowed") } + +func TestTemplatedSecret(t *testing.T) { + d := swarm.NewSwarm(t, testEnv) + defer d.Stop(t) + + ctx := context.Background() + client := swarm.GetClient(t, d) + + referencedSecretSpec := swarmtypes.SecretSpec{ + Annotations: swarmtypes.Annotations{ + Name: "referencedsecret", + }, + Data: []byte("this is a secret"), + } + referencedSecret, err := client.SecretCreate(ctx, referencedSecretSpec) + assert.NoError(t, err) + + referencedConfigSpec := swarmtypes.ConfigSpec{ + Annotations: swarmtypes.Annotations{ + Name: "referencedconfig", + }, + Data: []byte("this is a config"), + } + referencedConfig, err := client.ConfigCreate(ctx, referencedConfigSpec) + assert.NoError(t, err) + + secretSpec := swarmtypes.SecretSpec{ + Annotations: swarmtypes.Annotations{ + Name: "templated_secret", + }, + Templating: &swarmtypes.Driver{ + Name: "golang", + }, + Data: []byte("SERVICE_NAME={{.Service.Name}}\n" + + "{{secret \"referencedsecrettarget\"}}\n" + + "{{config \"referencedconfigtarget\"}}\n"), + } + + templatedSecret, err := client.SecretCreate(ctx, secretSpec) + assert.NoError(t, err) + + serviceID := swarm.CreateService(t, d, + swarm.ServiceWithSecret( + &swarmtypes.SecretReference{ + File: &swarmtypes.SecretReferenceFileTarget{ + Name: "templated_secret", + UID: "0", + GID: "0", + Mode: 0600, + }, + SecretID: templatedSecret.ID, + SecretName: "templated_secret", + }, + ), + swarm.ServiceWithConfig( + &swarmtypes.ConfigReference{ + File: &swarmtypes.ConfigReferenceFileTarget{ + Name: "referencedconfigtarget", + UID: "0", + GID: "0", + Mode: 0600, + }, + ConfigID: referencedConfig.ID, + ConfigName: "referencedconfig", + }, + ), + swarm.ServiceWithSecret( + &swarmtypes.SecretReference{ + File: &swarmtypes.SecretReferenceFileTarget{ + Name: "referencedsecrettarget", + UID: "0", + GID: "0", + Mode: 0600, + }, + SecretID: referencedSecret.ID, + SecretName: "referencedsecret", + }, + ), + swarm.ServiceWithName("svc"), + ) + + var tasks []swarmtypes.Task + waitAndAssert(t, 60*time.Second, func(t *testing.T) bool { + tasks = swarm.GetRunningTasks(t, d, serviceID) + return len(tasks) > 0 + }) + + task := tasks[0] + waitAndAssert(t, 60*time.Second, func(t *testing.T) bool { + if task.NodeID == "" || (task.Status.ContainerStatus == nil || task.Status.ContainerStatus.ContainerID == "") { + task, _, _ = client.TaskInspectWithRaw(context.Background(), task.ID) + } + return task.NodeID != "" && task.Status.ContainerStatus != nil && task.Status.ContainerStatus.ContainerID != "" + }) + + attach := swarm.ExecTask(t, d, task, types.ExecConfig{ + Cmd: []string{"/bin/cat", "/run/secrets/templated_secret"}, + AttachStdout: true, + AttachStderr: true, + }) + + buf := bytes.NewBuffer(nil) + _, err = stdcopy.StdCopy(buf, buf, attach.Reader) + require.NoError(t, err) + + expect := "SERVICE_NAME=svc\n" + + "this is a secret\n" + + "this is a config\n" + + assert.Equal(t, expect, buf.String()) +} + +func waitAndAssert(t *testing.T, timeout time.Duration, f func(*testing.T) bool) { + t.Helper() + after := time.After(timeout) + for { + select { + case <-after: + t.Fatalf("timed out waiting for condition") + default: + } + if f(t) { + return + } + time.Sleep(100 * time.Millisecond) + } +} From 599f92e497b8772f603264b94e1559d38adcba39 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Mon, 26 Jun 2017 18:46:30 -0700 Subject: [PATCH 4/8] Store configs that contain secrets on tmpfs Signed-off-by: Aaron Lehmann Upstream-commit: cd3d0486a6f62afac50f2cf74e2b9d8728848c97 Component: engine --- components/engine/container/container.go | 19 ++- components/engine/container/container_unix.go | 18 ++- components/engine/daemon/configs.go | 5 +- .../daemon/container_operations_unix.go | 112 ++++++++++++------ .../daemon/container_operations_windows.go | 2 +- components/engine/daemon/oci_linux.go | 19 ++- 6 files changed, 131 insertions(+), 44 deletions(-) diff --git a/components/engine/container/container.go b/components/engine/container/container.go index 938d66f889..32389e3804 100644 --- a/components/engine/container/container.go +++ b/components/engine/container/container.go @@ -68,6 +68,13 @@ type ExitStatus struct { ExitedAt time.Time } +// ConfigReference wraps swarmtypes.ConfigReference to add a Sensitive flag. +type ConfigReference struct { + *swarmtypes.ConfigReference + // Sensitive is set if this config should not be written to disk. + Sensitive bool +} + // Container holds the structure defining a container object. type Container struct { StreamConfig *stream.Config @@ -99,7 +106,7 @@ type Container struct { ExecCommands *exec.Store `json:"-"` DependencyStore agentexec.DependencyGetter `json:"-"` SecretReferences []*swarmtypes.SecretReference - ConfigReferences []*swarmtypes.ConfigReference + ConfigReferences []*ConfigReference // logDriver for closing LogDriver logger.Logger `json:"-"` LogCopier *logger.Copier `json:"-"` @@ -1064,6 +1071,16 @@ func (container *Container) ConfigFilePath(configRef swarmtypes.ConfigReference) return filepath.Join(configs, configRef.ConfigID), nil } +// SensitiveConfigFilePath returns the path to the location of a config mounted +// as a secret. +func (container *Container) SensitiveConfigFilePath(configRef swarmtypes.ConfigReference) (string, error) { + secretMountPath, err := container.SecretMountPath() + if err != nil { + return "", err + } + return filepath.Join(secretMountPath, configRef.ConfigID+"c"), nil +} + // CreateDaemonEnvironment creates a new environment variable slice for this container. func (container *Container) CreateDaemonEnvironment(tty bool, linkedEnv []string) []string { // Setup environment diff --git a/components/engine/container/container_unix.go b/components/engine/container/container_unix.go index 6f4d91b919..ec90921acc 100644 --- a/components/engine/container/container_unix.go +++ b/components/engine/container/container_unix.go @@ -233,6 +233,20 @@ func (container *Container) SecretMounts() ([]Mount, error) { Writable: false, }) } + for _, r := range container.ConfigReferences { + if !r.Sensitive || r.File == nil { + continue + } + fPath, err := container.SensitiveConfigFilePath(*r.ConfigReference) + if err != nil { + return nil, err + } + mounts = append(mounts, Mount{ + Source: fPath, + Destination: r.File.Name, + Writable: false, + }) + } return mounts, nil } @@ -257,10 +271,10 @@ func (container *Container) UnmountSecrets() error { func (container *Container) ConfigMounts() ([]Mount, error) { var mounts []Mount for _, configRef := range container.ConfigReferences { - if configRef.File == nil { + if configRef.Sensitive || configRef.File == nil { continue } - src, err := container.ConfigFilePath(*configRef) + src, err := container.ConfigFilePath(*configRef.ConfigReference) if err != nil { return nil, err } diff --git a/components/engine/daemon/configs.go b/components/engine/daemon/configs.go index f85c51db2e..c19b7ff3fd 100644 --- a/components/engine/daemon/configs.go +++ b/components/engine/daemon/configs.go @@ -2,6 +2,7 @@ package daemon // import "github.com/docker/docker/daemon" import ( swarmtypes "github.com/docker/docker/api/types/swarm" + "github.com/docker/docker/container" "github.com/sirupsen/logrus" ) @@ -17,7 +18,9 @@ func (daemon *Daemon) SetContainerConfigReferences(name string, refs []*swarmtyp return err } - c.ConfigReferences = refs + for _, ref := range refs { + c.ConfigReferences = append(c.ConfigReferences, &container.ConfigReference{ConfigReference: ref}) + } return nil } diff --git a/components/engine/daemon/container_operations_unix.go b/components/engine/daemon/container_operations_unix.go index abd47c807f..1523ac067c 100644 --- a/components/engine/daemon/container_operations_unix.go +++ b/components/engine/daemon/container_operations_unix.go @@ -19,6 +19,7 @@ import ( "github.com/docker/docker/pkg/stringid" "github.com/docker/docker/runconfig" "github.com/docker/libnetwork" + "github.com/docker/swarmkit/template" "github.com/opencontainers/selinux/go-selinux/label" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -160,44 +161,23 @@ func (daemon *Daemon) setupIpcDirs(c *container.Container) error { return nil } -func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) { +func (daemon *Daemon) setupSecretDir(c *container.Container, hasSecretDir *bool) (setupErr error) { if len(c.SecretReferences) == 0 { return nil } - localMountPath, err := c.SecretMountPath() - if err != nil { - return errors.Wrap(err, "error getting secrets mount dir") - } - logrus.Debugf("secrets: setting up secret dir: %s", localMountPath) - - // retrieve possible remapped range start for root UID, GID - rootIDs := daemon.idMappings.RootPair() - // create tmpfs - if err := idtools.MkdirAllAndChown(localMountPath, 0700, rootIDs); err != nil { - return errors.Wrap(err, "error creating secret local mount path") - } - - defer func() { - if setupErr != nil { - // cleanup - _ = detachMounted(localMountPath) - - if err := os.RemoveAll(localMountPath); err != nil { - logrus.Errorf("error cleaning up secret mount: %s", err) - } - } - }() - - tmpfsOwnership := fmt.Sprintf("uid=%d,gid=%d", rootIDs.UID, rootIDs.GID) - if err := mount.Mount("tmpfs", localMountPath, "tmpfs", "nodev,nosuid,noexec,"+tmpfsOwnership); err != nil { - return errors.Wrap(err, "unable to setup secret mount") + if !*hasSecretDir { + daemon.createSecretDir(c) + *hasSecretDir = true } if c.DependencyStore == nil { return fmt.Errorf("secret store is not initialized") } + // retrieve possible remapped range start for root UID, GID + rootIDs := daemon.idMappings.RootPair() + for _, s := range c.SecretReferences { // TODO (ehazlett): use type switch when more are supported if s.File == nil { @@ -244,8 +224,42 @@ func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) { } } + return nil +} + +func (daemon *Daemon) createSecretDir(c *container.Container) error { + localMountPath, err := c.SecretMountPath() + if err != nil { + return err + } + logrus.Debugf("secrets: setting up secret dir: %s", localMountPath) + + // retrieve possible remapped range start for root UID, GID + rootIDs := daemon.idMappings.RootPair() + // create tmpfs + if err := idtools.MkdirAllAndChown(localMountPath, 0700, rootIDs); err != nil { + return errors.Wrap(err, "error creating secret local mount path") + } + + tmpfsOwnership := fmt.Sprintf("uid=%d,gid=%d", rootIDs.UID, rootIDs.GID) + if err := mount.Mount("tmpfs", localMountPath, "tmpfs", "nodev,nosuid,noexec,"+tmpfsOwnership); err != nil { + return errors.Wrap(err, "unable to setup secret mount") + } + + return nil +} + +func (daemon *Daemon) remountSecretDir(c *container.Container) error { + localMountPath, err := c.SecretMountPath() + if err != nil { + return err + } + label.Relabel(localMountPath, c.MountLabel, false) + rootIDs := daemon.idMappings.RootPair() + tmpfsOwnership := fmt.Sprintf("uid=%d,gid=%d", rootIDs.UID, rootIDs.GID) + // remount secrets ro if err := mount.Mount("tmpfs", localMountPath, "tmpfs", "remount,ro,"+tmpfsOwnership); err != nil { return errors.Wrap(err, "unable to remount secret dir as readonly") @@ -254,7 +268,20 @@ func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) { return nil } -func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) { +func (daemon *Daemon) cleanupSecretDir(c *container.Container) { + localMountPath, err := c.SecretMountPath() + if err != nil { + logrus.WithError(err).WithField("container", c.ID).Errorf("error getting secrets mounth path for cleanup") + } + + detachMounted(localMountPath) + + if err := os.RemoveAll(localMountPath); err != nil { + logrus.Errorf("error cleaning up secret mount: %s", err) + } +} + +func (daemon *Daemon) setupConfigDir(c *container.Container, hasSecretDir *bool) (setupErr error) { if len(c.ConfigReferences) == 0 { return nil } @@ -291,22 +318,35 @@ func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) { continue } - fPath, err := c.ConfigFilePath(*configRef) + getter := c.DependencyStore.Configs().(template.TemplatedConfigGetter) + config, sensitive, err := getter.GetAndFlagSecretData(configRef.ConfigID) if err != nil { - return err + return errors.Wrap(err, "unable to get config from config store") + } + + var fPath string + if sensitive { + configRef.Sensitive = true + fPath, err = c.SensitiveConfigFilePath(*configRef.ConfigReference) + if !*hasSecretDir { + daemon.createSecretDir(c) + *hasSecretDir = true + } + } else { + fPath, err = c.ConfigFilePath(*configRef.ConfigReference) + } + if err != nil { + return errors.Wrap(err, "error getting config file path") } log := logrus.WithFields(logrus.Fields{"name": configRef.File.Name, "path": fPath}) + log.Debug("injecting config") + if err := idtools.MkdirAllAndChown(filepath.Dir(fPath), 0700, rootIDs); err != nil { return errors.Wrap(err, "error creating config path") } - log.Debug("injecting config") - config, err := c.DependencyStore.Configs().Get(configRef.ConfigID) - if err != nil { - return errors.Wrap(err, "unable to get config from config store") - } if err := ioutil.WriteFile(fPath, config.Spec.Data, configRef.File.Mode); err != nil { return errors.Wrap(err, "error injecting config") } diff --git a/components/engine/daemon/container_operations_windows.go b/components/engine/daemon/container_operations_windows.go index e3914f9410..fe9795b8ba 100644 --- a/components/engine/daemon/container_operations_windows.go +++ b/components/engine/daemon/container_operations_windows.go @@ -51,7 +51,7 @@ func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) { continue } - fPath, err := c.ConfigFilePath(*configRef) + fPath, err := c.ConfigFilePath(*configRef.ConfigReference) if err != nil { return err } diff --git a/components/engine/daemon/oci_linux.go b/components/engine/daemon/oci_linux.go index 2a19dd5db8..e0d93d5ca4 100644 --- a/components/engine/daemon/oci_linux.go +++ b/components/engine/daemon/oci_linux.go @@ -760,7 +760,7 @@ func (daemon *Daemon) populateCommonSpec(s *specs.Spec, c *container.Container) return nil } -func (daemon *Daemon) createSpec(c *container.Container) (*specs.Spec, error) { +func (daemon *Daemon) createSpec(c *container.Container) (retSpec *specs.Spec, err error) { s := oci.DefaultSpec() if err := daemon.populateCommonSpec(&s, c); err != nil { return nil, err @@ -842,14 +842,27 @@ func (daemon *Daemon) createSpec(c *container.Container) (*specs.Spec, error) { return nil, err } - if err := daemon.setupSecretDir(c); err != nil { + var hasSecretDir bool + defer func() { + if hasSecretDir && err != nil { + daemon.cleanupSecretDir(c) + } + }() + + if err := daemon.setupSecretDir(c, &hasSecretDir); err != nil { return nil, err } - if err := daemon.setupConfigDir(c); err != nil { + if err := daemon.setupConfigDir(c, &hasSecretDir); err != nil { return nil, err } + if hasSecretDir { + if err := daemon.remountSecretDir(c); err != nil { + return nil, err + } + } + ms, err := daemon.setupMounts(c) if err != nil { return nil, err From 40e1524cb370b92118c5721614653ffbb31af01a Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Wed, 26 Jul 2017 10:37:32 -0700 Subject: [PATCH 5/8] daemon: Check return value of createSecretDir Signed-off-by: Aaron Lehmann Upstream-commit: 426f4e48e3e53b2445835585d7957043a5fe6ab3 Component: engine --- components/engine/daemon/container_operations_unix.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/components/engine/daemon/container_operations_unix.go b/components/engine/daemon/container_operations_unix.go index 1523ac067c..d875be73b3 100644 --- a/components/engine/daemon/container_operations_unix.go +++ b/components/engine/daemon/container_operations_unix.go @@ -167,7 +167,9 @@ func (daemon *Daemon) setupSecretDir(c *container.Container, hasSecretDir *bool) } if !*hasSecretDir { - daemon.createSecretDir(c) + if err := daemon.createSecretDir(c); err != nil { + return err + } *hasSecretDir = true } @@ -329,7 +331,9 @@ func (daemon *Daemon) setupConfigDir(c *container.Container, hasSecretDir *bool) configRef.Sensitive = true fPath, err = c.SensitiveConfigFilePath(*configRef.ConfigReference) if !*hasSecretDir { - daemon.createSecretDir(c) + if err := daemon.createSecretDir(c); err != nil { + return err + } *hasSecretDir = true } } else { From 850e2bff8c7c49a0990ca3c8c540d8df81d9c426 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Thu, 11 Jan 2018 17:28:56 -0500 Subject: [PATCH 6/8] Always mount configs with tmpfs This makes configs and secrets behavior identical. Signed-off-by: Brian Goff Upstream-commit: 8e8f5f4457d8e1b02031576dbc18c903be4bcfb6 Component: engine --- components/engine/api/swagger.yaml | 34 ++-- .../daemon/container_operations_unix.go | 151 ++++++++---------- components/engine/daemon/oci_linux.go | 24 +-- 3 files changed, 88 insertions(+), 121 deletions(-) diff --git a/components/engine/api/swagger.yaml b/components/engine/api/swagger.yaml index 12fdd57537..cb4201be90 100644 --- a/components/engine/api/swagger.yaml +++ b/components/engine/api/swagger.yaml @@ -3339,17 +3339,12 @@ definitions: description: "Name of the secrets driver used to fetch the secret's value from an external secret store" $ref: "#/definitions/Driver" Templating: - description: "Templating driver, if applicable" - type: "object" - properties: - Name: - description: "Name of the templating driver (i.e. 'golang')" - type: "string" - Options: - description: "key/value map of driver specific options." - type: "object" - additionalProperties: - type: "string" + description: | + Templating driver, if applicable + + Templating controls whether and how to evaluate the config payload as + a template. If no driver is set, no templating is used. + $ref: "#/definitions/Driver" Secret: type: "object" @@ -3387,17 +3382,12 @@ definitions: config data. type: "string" Templating: - description: "Templating driver, if applicable" - type: "object" - properties: - Name: - description: "Name of the templating driver (i.e. 'golang')" - type: "string" - Options: - description: "key/value map of driver specific options." - type: "object" - additionalProperties: - type: "string" + description: | + Templating driver, if applicable + + Templating controls whether and how to evaluate the config payload as + a template. If no driver is set, no templating is used. + $ref: "#/definitions/Driver" Config: type: "object" diff --git a/components/engine/daemon/container_operations_unix.go b/components/engine/daemon/container_operations_unix.go index d875be73b3..8c23b36e3c 100644 --- a/components/engine/daemon/container_operations_unix.go +++ b/components/engine/daemon/container_operations_unix.go @@ -19,7 +19,6 @@ import ( "github.com/docker/docker/pkg/stringid" "github.com/docker/docker/runconfig" "github.com/docker/libnetwork" - "github.com/docker/swarmkit/template" "github.com/opencontainers/selinux/go-selinux/label" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -161,17 +160,23 @@ func (daemon *Daemon) setupIpcDirs(c *container.Container) error { return nil } -func (daemon *Daemon) setupSecretDir(c *container.Container, hasSecretDir *bool) (setupErr error) { +func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) { if len(c.SecretReferences) == 0 { return nil } - if !*hasSecretDir { - if err := daemon.createSecretDir(c); err != nil { - return err - } - *hasSecretDir = true + localMountPath, err := c.SecretMountPath() + if err != nil { + return errors.Wrap(err, "error getting secrets mount path for container") } + if err := daemon.createSecretsDir(localMountPath); err != nil { + return err + } + defer func() { + if setupErr != nil { + daemon.cleanupSecretDir(localMountPath) + } + }() if c.DependencyStore == nil { return fmt.Errorf("secret store is not initialized") @@ -226,64 +231,52 @@ func (daemon *Daemon) setupSecretDir(c *container.Container, hasSecretDir *bool) } } - return nil + return daemon.remountSecretDir(c.MountLabel, localMountPath) } -func (daemon *Daemon) createSecretDir(c *container.Container) error { - localMountPath, err := c.SecretMountPath() - if err != nil { - return err - } - logrus.Debugf("secrets: setting up secret dir: %s", localMountPath) - +// createSecretsDir is used to create a dir suitable for storing container secrets. +// In practice this is using a tmpfs mount and is used for both "configs" and "secrets" +func (daemon *Daemon) createSecretsDir(dir string) error { // retrieve possible remapped range start for root UID, GID rootIDs := daemon.idMappings.RootPair() // create tmpfs - if err := idtools.MkdirAllAndChown(localMountPath, 0700, rootIDs); err != nil { + if err := idtools.MkdirAllAndChown(dir, 0700, rootIDs); err != nil { return errors.Wrap(err, "error creating secret local mount path") } tmpfsOwnership := fmt.Sprintf("uid=%d,gid=%d", rootIDs.UID, rootIDs.GID) - if err := mount.Mount("tmpfs", localMountPath, "tmpfs", "nodev,nosuid,noexec,"+tmpfsOwnership); err != nil { + if err := mount.Mount("tmpfs", dir, "tmpfs", "nodev,nosuid,noexec,"+tmpfsOwnership); err != nil { return errors.Wrap(err, "unable to setup secret mount") } return nil } -func (daemon *Daemon) remountSecretDir(c *container.Container) error { - localMountPath, err := c.SecretMountPath() - if err != nil { - return err +func (daemon *Daemon) remountSecretDir(mountLabel, dir string) error { + if err := label.Relabel(dir, mountLabel, false); err != nil { + logrus.WithError(err).WithField("dir", dir).Warn("Error while attempting to set selinux label") } - - label.Relabel(localMountPath, c.MountLabel, false) - rootIDs := daemon.idMappings.RootPair() tmpfsOwnership := fmt.Sprintf("uid=%d,gid=%d", rootIDs.UID, rootIDs.GID) // remount secrets ro - if err := mount.Mount("tmpfs", localMountPath, "tmpfs", "remount,ro,"+tmpfsOwnership); err != nil { - return errors.Wrap(err, "unable to remount secret dir as readonly") + if err := mount.Mount("tmpfs", dir, "tmpfs", "remount,ro,"+tmpfsOwnership); err != nil { + return errors.Wrap(err, "unable to remount dir as readonly") } return nil } -func (daemon *Daemon) cleanupSecretDir(c *container.Container) { - localMountPath, err := c.SecretMountPath() - if err != nil { - logrus.WithError(err).WithField("container", c.ID).Errorf("error getting secrets mounth path for cleanup") +func (daemon *Daemon) cleanupSecretDir(dir string) { + if err := mount.RecursiveUnmount(dir); err != nil { + logrus.WithField("dir", dir).WithError(err).Warn("Error while attmepting to unmount dir, this may prevent removal of container.") } - - detachMounted(localMountPath) - - if err := os.RemoveAll(localMountPath); err != nil { - logrus.Errorf("error cleaning up secret mount: %s", err) + if err := os.RemoveAll(dir); err != nil && !os.IsNotExist(err) { + logrus.WithField("dir", dir).WithError(err).Error("Error removing dir.") } } -func (daemon *Daemon) setupConfigDir(c *container.Container, hasSecretDir *bool) (setupErr error) { +func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) { if len(c.ConfigReferences) == 0 { return nil } @@ -293,73 +286,55 @@ func (daemon *Daemon) setupConfigDir(c *container.Container, hasSecretDir *bool) return err } logrus.Debugf("configs: setting up config dir: %s", localPath) - - // retrieve possible remapped range start for root UID, GID - rootIDs := daemon.idMappings.RootPair() - // create tmpfs - if err := idtools.MkdirAllAndChown(localPath, 0700, rootIDs); err != nil { - return errors.Wrap(err, "error creating config dir") + if err := daemon.createSecretsDir(localPath); err != nil { + return err } - defer func() { if setupErr != nil { - if err := os.RemoveAll(localPath); err != nil { - logrus.Errorf("error cleaning up config dir: %s", err) - } + daemon.cleanupSecretDir(localPath) } }() if c.DependencyStore == nil { - return fmt.Errorf("config store is not initialized") + return errors.New("config store is not initialized") } - for _, configRef := range c.ConfigReferences { + // retrieve possible remapped range start for root UID, GID + rootIDs := daemon.idMappings.RootPair() + + for _, ref := range c.ConfigReferences { // TODO (ehazlett): use type switch when more are supported - if configRef.File == nil { + if ref.File == nil { logrus.Error("config target type is not a file target") continue } - - getter := c.DependencyStore.Configs().(template.TemplatedConfigGetter) - config, sensitive, err := getter.GetAndFlagSecretData(configRef.ConfigID) - if err != nil { - return errors.Wrap(err, "unable to get config from config store") - } - - var fPath string - if sensitive { - configRef.Sensitive = true - fPath, err = c.SensitiveConfigFilePath(*configRef.ConfigReference) - if !*hasSecretDir { - if err := daemon.createSecretDir(c); err != nil { - return err - } - *hasSecretDir = true - } - } else { - fPath, err = c.ConfigFilePath(*configRef.ConfigReference) - } - if err != nil { - return errors.Wrap(err, "error getting config file path") - } - - log := logrus.WithFields(logrus.Fields{"name": configRef.File.Name, "path": fPath}) - - log.Debug("injecting config") - - if err := idtools.MkdirAllAndChown(filepath.Dir(fPath), 0700, rootIDs); err != nil { - return errors.Wrap(err, "error creating config path") - } - - if err := ioutil.WriteFile(fPath, config.Spec.Data, configRef.File.Mode); err != nil { - return errors.Wrap(err, "error injecting config") - } - - uid, err := strconv.Atoi(configRef.File.UID) + // configs are created in the ConfigsDirPath on the host, at a + // single level + fPath, err := c.ConfigFilePath(*ref.ConfigReference) if err != nil { return err } - gid, err := strconv.Atoi(configRef.File.GID) + if err := idtools.MkdirAllAndChown(filepath.Dir(fPath), 0700, rootIDs); err != nil { + return errors.Wrap(err, "error creating config mount path") + } + + logrus.WithFields(logrus.Fields{ + "name": ref.File.Name, + "path": fPath, + }).Debug("injecting config") + config, err := c.DependencyStore.Configs().Get(ref.ConfigID) + if err != nil { + return errors.Wrap(err, "unable to get config from config store") + } + if err := ioutil.WriteFile(fPath, config.Spec.Data, ref.File.Mode); err != nil { + return errors.Wrap(err, "error injecting config") + } + + uid, err := strconv.Atoi(ref.File.UID) + if err != nil { + return err + } + gid, err := strconv.Atoi(ref.File.GID) if err != nil { return err } @@ -374,7 +349,7 @@ func (daemon *Daemon) setupConfigDir(c *container.Container, hasSecretDir *bool) label.Relabel(fPath, c.MountLabel, false) } - return nil + return daemon.remountSecretDir(c.MountLabel, localPath) } func killProcessDirectly(cntr *container.Container) error { diff --git a/components/engine/daemon/oci_linux.go b/components/engine/daemon/oci_linux.go index e0d93d5ca4..52a944f2a2 100644 --- a/components/engine/daemon/oci_linux.go +++ b/components/engine/daemon/oci_linux.go @@ -842,27 +842,29 @@ func (daemon *Daemon) createSpec(c *container.Container) (retSpec *specs.Spec, e return nil, err } - var hasSecretDir bool + secretMountPath, err := c.SecretMountPath() + if err != nil { + return nil, err + } + configsMountPath, err := c.ConfigsDirPath() + if err != nil { + return nil, err + } defer func() { - if hasSecretDir && err != nil { - daemon.cleanupSecretDir(c) + if err != nil { + daemon.cleanupSecretDir(secretMountPath) + daemon.cleanupSecretDir(configsMountPath) } }() - if err := daemon.setupSecretDir(c, &hasSecretDir); err != nil { + if err := daemon.setupSecretDir(c); err != nil { return nil, err } - if err := daemon.setupConfigDir(c, &hasSecretDir); err != nil { + if err := daemon.setupConfigDir(c); err != nil { return nil, err } - if hasSecretDir { - if err := daemon.remountSecretDir(c); err != nil { - return nil, err - } - } - ms, err := daemon.setupMounts(c) if err != nil { return nil, err From f68c84b9a0bbdc45deb0f5b51ac711d391fc9c87 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Wed, 17 Jan 2018 10:49:58 -0500 Subject: [PATCH 7/8] Merge configs/secrets in unix implementation On unix, merge secrets/configs handling. This is important because configs can contain secrets (via templating) and potentially a config could just simply have secret information "by accident" from the user. This just make sure that configs are as secure as secrets and de-dups a lot of code. Generally this makes everything simpler and configs more secure. Signed-off-by: Brian Goff Upstream-commit: c02171802b788fb2d4d48bebcee2a57c8eabeeaa Component: engine --- components/engine/container/container.go | 34 +--- components/engine/container/container_unix.go | 38 ++--- .../engine/container/container_windows.go | 25 +-- components/engine/daemon/configs.go | 7 +- .../daemon/container_operations_unix.go | 149 ++++++++---------- .../daemon/container_operations_windows.go | 11 +- components/engine/daemon/oci_linux.go | 21 +-- components/engine/daemon/oci_windows.go | 5 +- .../engine/integration/config/config_test.go | 19 ++- .../engine/integration/secret/secret_test.go | 19 ++- 10 files changed, 126 insertions(+), 202 deletions(-) diff --git a/components/engine/container/container.go b/components/engine/container/container.go index 32389e3804..46e592ab45 100644 --- a/components/engine/container/container.go +++ b/components/engine/container/container.go @@ -68,13 +68,6 @@ type ExitStatus struct { ExitedAt time.Time } -// ConfigReference wraps swarmtypes.ConfigReference to add a Sensitive flag. -type ConfigReference struct { - *swarmtypes.ConfigReference - // Sensitive is set if this config should not be written to disk. - Sensitive bool -} - // Container holds the structure defining a container object. type Container struct { StreamConfig *stream.Config @@ -106,7 +99,7 @@ type Container struct { ExecCommands *exec.Store `json:"-"` DependencyStore agentexec.DependencyGetter `json:"-"` SecretReferences []*swarmtypes.SecretReference - ConfigReferences []*ConfigReference + ConfigReferences []*swarmtypes.ConfigReference // logDriver for closing LogDriver logger.Logger `json:"-"` LogCopier *logger.Copier `json:"-"` @@ -1056,31 +1049,6 @@ func getSecretTargetPath(r *swarmtypes.SecretReference) string { return filepath.Join(containerSecretMountPath, r.File.Name) } -// ConfigsDirPath returns the path to the directory where configs are stored on -// disk. -func (container *Container) ConfigsDirPath() (string, error) { - return container.GetRootResourcePath("configs") -} - -// ConfigFilePath returns the path to the on-disk location of a config. -func (container *Container) ConfigFilePath(configRef swarmtypes.ConfigReference) (string, error) { - configs, err := container.ConfigsDirPath() - if err != nil { - return "", err - } - return filepath.Join(configs, configRef.ConfigID), nil -} - -// SensitiveConfigFilePath returns the path to the location of a config mounted -// as a secret. -func (container *Container) SensitiveConfigFilePath(configRef swarmtypes.ConfigReference) (string, error) { - secretMountPath, err := container.SecretMountPath() - if err != nil { - return "", err - } - return filepath.Join(secretMountPath, configRef.ConfigID+"c"), nil -} - // CreateDaemonEnvironment creates a new environment variable slice for this container. func (container *Container) CreateDaemonEnvironment(tty bool, linkedEnv []string) []string { // Setup environment diff --git a/components/engine/container/container_unix.go b/components/engine/container/container_unix.go index ec90921acc..e5cecf3166 100644 --- a/components/engine/container/container_unix.go +++ b/components/engine/container/container_unix.go @@ -5,11 +5,13 @@ package container // import "github.com/docker/docker/container" import ( "io/ioutil" "os" + "path/filepath" "github.com/containerd/continuity/fs" "github.com/docker/docker/api/types" containertypes "github.com/docker/docker/api/types/container" mounttypes "github.com/docker/docker/api/types/mount" + swarmtypes "github.com/docker/docker/api/types/swarm" "github.com/docker/docker/pkg/mount" "github.com/docker/docker/pkg/stringid" "github.com/docker/docker/volume" @@ -234,10 +236,7 @@ func (container *Container) SecretMounts() ([]Mount, error) { }) } for _, r := range container.ConfigReferences { - if !r.Sensitive || r.File == nil { - continue - } - fPath, err := container.SensitiveConfigFilePath(*r.ConfigReference) + fPath, err := container.ConfigFilePath(*r) if err != nil { return nil, err } @@ -267,27 +266,6 @@ func (container *Container) UnmountSecrets() error { return mount.RecursiveUnmount(p) } -// ConfigMounts returns the mounts for configs. -func (container *Container) ConfigMounts() ([]Mount, error) { - var mounts []Mount - for _, configRef := range container.ConfigReferences { - if configRef.Sensitive || configRef.File == nil { - continue - } - src, err := container.ConfigFilePath(*configRef.ConfigReference) - if err != nil { - return nil, err - } - mounts = append(mounts, Mount{ - Source: src, - Destination: configRef.File.Name, - Writable: false, - }) - } - - return mounts, nil -} - type conflictingUpdateOptions string func (e conflictingUpdateOptions) Error() string { @@ -471,3 +449,13 @@ func (container *Container) GetMountPoints() []types.MountPoint { } return mountPoints } + +// ConfigFilePath returns the path to the on-disk location of a config. +// On unix, configs are always considered secret +func (container *Container) ConfigFilePath(configRef swarmtypes.ConfigReference) (string, error) { + mounts, err := container.SecretMountPath() + if err != nil { + return "", err + } + return filepath.Join(mounts, configRef.ConfigID), nil +} diff --git a/components/engine/container/container_windows.go b/components/engine/container/container_windows.go index 44b646a1ad..b5bdb5bc34 100644 --- a/components/engine/container/container_windows.go +++ b/components/engine/container/container_windows.go @@ -7,6 +7,7 @@ import ( "github.com/docker/docker/api/types" containertypes "github.com/docker/docker/api/types/container" + swarmtypes "github.com/docker/docker/api/types/swarm" "github.com/docker/docker/pkg/system" ) @@ -102,23 +103,20 @@ func (container *Container) CreateConfigSymlinks() error { } // ConfigMounts returns the mount for configs. -// All configs are stored in a single mount on Windows. Target symlinks are -// created for each config, pointing to the files in this mount. -func (container *Container) ConfigMounts() ([]Mount, error) { +// TODO: Right now Windows doesn't really have a "secure" storage for secrets, +// however some configs may contain secrets. Once secure storage is worked out, +// configs and secret handling should be merged. +func (container *Container) ConfigMounts() []Mount { var mounts []Mount if len(container.ConfigReferences) > 0 { - src, err := container.ConfigsDirPath() - if err != nil { - return nil, err - } mounts = append(mounts, Mount{ - Source: src, + Source: container.ConfigsDirPath(), Destination: containerInternalConfigsDirPath, Writable: false, }) } - return mounts, nil + return mounts } // DetachAndUnmount unmounts all volumes. @@ -204,3 +202,12 @@ func (container *Container) GetMountPoints() []types.MountPoint { } return mountPoints } + +func (container *Container) ConfigsDirPath() string { + return filepath.Join(container.Root, "configs") +} + +// ConfigFilePath returns the path to the on-disk location of a config. +func (container *Container) ConfigFilePath(configRef swarmtypes.ConfigReference) string { + return filepath.Join(container.ConfigsDirPath(), configRef.ConfigID) +} diff --git a/components/engine/daemon/configs.go b/components/engine/daemon/configs.go index c19b7ff3fd..4fd0d2272c 100644 --- a/components/engine/daemon/configs.go +++ b/components/engine/daemon/configs.go @@ -2,7 +2,6 @@ package daemon // import "github.com/docker/docker/daemon" import ( swarmtypes "github.com/docker/docker/api/types/swarm" - "github.com/docker/docker/container" "github.com/sirupsen/logrus" ) @@ -17,10 +16,6 @@ func (daemon *Daemon) SetContainerConfigReferences(name string, refs []*swarmtyp if err != nil { return err } - - for _, ref := range refs { - c.ConfigReferences = append(c.ConfigReferences, &container.ConfigReference{ConfigReference: ref}) - } - + c.ConfigReferences = append(c.ConfigReferences, refs...) return nil } diff --git a/components/engine/daemon/container_operations_unix.go b/components/engine/daemon/container_operations_unix.go index 8c23b36e3c..4e92b6392e 100644 --- a/components/engine/daemon/container_operations_unix.go +++ b/components/engine/daemon/container_operations_unix.go @@ -161,20 +161,16 @@ func (daemon *Daemon) setupIpcDirs(c *container.Container) error { } func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) { - if len(c.SecretReferences) == 0 { + if len(c.SecretReferences) == 0 && len(c.ConfigReferences) == 0 { return nil } - localMountPath, err := c.SecretMountPath() - if err != nil { - return errors.Wrap(err, "error getting secrets mount path for container") - } - if err := daemon.createSecretsDir(localMountPath); err != nil { + if err := daemon.createSecretsDir(c); err != nil { return err } defer func() { if setupErr != nil { - daemon.cleanupSecretDir(localMountPath) + daemon.cleanupSecretDir(c) } }() @@ -231,88 +227,16 @@ func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) { } } - return daemon.remountSecretDir(c.MountLabel, localMountPath) -} - -// createSecretsDir is used to create a dir suitable for storing container secrets. -// In practice this is using a tmpfs mount and is used for both "configs" and "secrets" -func (daemon *Daemon) createSecretsDir(dir string) error { - // retrieve possible remapped range start for root UID, GID - rootIDs := daemon.idMappings.RootPair() - // create tmpfs - if err := idtools.MkdirAllAndChown(dir, 0700, rootIDs); err != nil { - return errors.Wrap(err, "error creating secret local mount path") - } - - tmpfsOwnership := fmt.Sprintf("uid=%d,gid=%d", rootIDs.UID, rootIDs.GID) - if err := mount.Mount("tmpfs", dir, "tmpfs", "nodev,nosuid,noexec,"+tmpfsOwnership); err != nil { - return errors.Wrap(err, "unable to setup secret mount") - } - - return nil -} - -func (daemon *Daemon) remountSecretDir(mountLabel, dir string) error { - if err := label.Relabel(dir, mountLabel, false); err != nil { - logrus.WithError(err).WithField("dir", dir).Warn("Error while attempting to set selinux label") - } - rootIDs := daemon.idMappings.RootPair() - tmpfsOwnership := fmt.Sprintf("uid=%d,gid=%d", rootIDs.UID, rootIDs.GID) - - // remount secrets ro - if err := mount.Mount("tmpfs", dir, "tmpfs", "remount,ro,"+tmpfsOwnership); err != nil { - return errors.Wrap(err, "unable to remount dir as readonly") - } - - return nil -} - -func (daemon *Daemon) cleanupSecretDir(dir string) { - if err := mount.RecursiveUnmount(dir); err != nil { - logrus.WithField("dir", dir).WithError(err).Warn("Error while attmepting to unmount dir, this may prevent removal of container.") - } - if err := os.RemoveAll(dir); err != nil && !os.IsNotExist(err) { - logrus.WithField("dir", dir).WithError(err).Error("Error removing dir.") - } -} - -func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) { - if len(c.ConfigReferences) == 0 { - return nil - } - - localPath, err := c.ConfigsDirPath() - if err != nil { - return err - } - logrus.Debugf("configs: setting up config dir: %s", localPath) - if err := daemon.createSecretsDir(localPath); err != nil { - return err - } - defer func() { - if setupErr != nil { - daemon.cleanupSecretDir(localPath) - } - }() - - if c.DependencyStore == nil { - return errors.New("config store is not initialized") - } - - // retrieve possible remapped range start for root UID, GID - rootIDs := daemon.idMappings.RootPair() - for _, ref := range c.ConfigReferences { // TODO (ehazlett): use type switch when more are supported if ref.File == nil { logrus.Error("config target type is not a file target") continue } - // configs are created in the ConfigsDirPath on the host, at a - // single level - fPath, err := c.ConfigFilePath(*ref.ConfigReference) + + fPath, err := c.ConfigFilePath(*ref) if err != nil { - return err + return errors.Wrap(err, "error getting config file path for container") } if err := idtools.MkdirAllAndChown(filepath.Dir(fPath), 0700, rootIDs); err != nil { return errors.Wrap(err, "error creating config mount path") @@ -342,14 +266,67 @@ func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) { if err := os.Chown(fPath, rootIDs.UID+uid, rootIDs.GID+gid); err != nil { return errors.Wrap(err, "error setting ownership for config") } - if err := os.Chmod(fPath, configRef.File.Mode); err != nil { + if err := os.Chmod(fPath, ref.File.Mode); err != nil { return errors.Wrap(err, "error setting file mode for config") } - - label.Relabel(fPath, c.MountLabel, false) } - return daemon.remountSecretDir(c.MountLabel, localPath) + return daemon.remountSecretDir(c) +} + +// createSecretsDir is used to create a dir suitable for storing container secrets. +// In practice this is using a tmpfs mount and is used for both "configs" and "secrets" +func (daemon *Daemon) createSecretsDir(c *container.Container) error { + // retrieve possible remapped range start for root UID, GID + rootIDs := daemon.idMappings.RootPair() + dir, err := c.SecretMountPath() + if err != nil { + return errors.Wrap(err, "error getting container secrets dir") + } + + // create tmpfs + if err := idtools.MkdirAllAndChown(dir, 0700, rootIDs); err != nil { + return errors.Wrap(err, "error creating secret local mount path") + } + + tmpfsOwnership := fmt.Sprintf("uid=%d,gid=%d", rootIDs.UID, rootIDs.GID) + if err := mount.Mount("tmpfs", dir, "tmpfs", "nodev,nosuid,noexec,"+tmpfsOwnership); err != nil { + return errors.Wrap(err, "unable to setup secret mount") + } + + return nil +} + +func (daemon *Daemon) remountSecretDir(c *container.Container) error { + dir, err := c.SecretMountPath() + if err != nil { + return errors.Wrap(err, "error getting container secrets path") + } + if err := label.Relabel(dir, c.MountLabel, false); err != nil { + logrus.WithError(err).WithField("dir", dir).Warn("Error while attempting to set selinux label") + } + rootIDs := daemon.idMappings.RootPair() + tmpfsOwnership := fmt.Sprintf("uid=%d,gid=%d", rootIDs.UID, rootIDs.GID) + + // remount secrets ro + if err := mount.Mount("tmpfs", dir, "tmpfs", "remount,ro,"+tmpfsOwnership); err != nil { + return errors.Wrap(err, "unable to remount dir as readonly") + } + + return nil +} + +func (daemon *Daemon) cleanupSecretDir(c *container.Container) { + dir, err := c.SecretMountPath() + if err != nil { + logrus.WithError(err).WithField("container", c.ID).Warn("error getting secrets mount path for container") + } + if err := mount.RecursiveUnmount(dir); err != nil { + logrus.WithField("dir", dir).WithError(err).Warn("Error while attmepting to unmount dir, this may prevent removal of container.") + } + if err := os.RemoveAll(dir); err != nil && !os.IsNotExist(err) { + logrus.WithField("dir", dir).WithError(err).Error("Error removing dir.") + } } func killProcessDirectly(cntr *container.Container) error { diff --git a/components/engine/daemon/container_operations_windows.go b/components/engine/daemon/container_operations_windows.go index fe9795b8ba..0559b8ac3e 100644 --- a/components/engine/daemon/container_operations_windows.go +++ b/components/engine/daemon/container_operations_windows.go @@ -21,10 +21,7 @@ func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) { return nil } - localPath, err := c.ConfigsDirPath() - if err != nil { - return err - } + localPath := c.ConfigsDirPath() logrus.Debugf("configs: setting up config dir: %s", localPath) // create local config root @@ -51,11 +48,7 @@ func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) { continue } - fPath, err := c.ConfigFilePath(*configRef.ConfigReference) - if err != nil { - return err - } - + fPath := c.ConfigFilePath(*configRef) log := logrus.WithFields(logrus.Fields{"name": configRef.File.Name, "path": fPath}) log.Debug("injecting config") diff --git a/components/engine/daemon/oci_linux.go b/components/engine/daemon/oci_linux.go index 52a944f2a2..68311bd605 100644 --- a/components/engine/daemon/oci_linux.go +++ b/components/engine/daemon/oci_linux.go @@ -842,18 +842,9 @@ func (daemon *Daemon) createSpec(c *container.Container) (retSpec *specs.Spec, e return nil, err } - secretMountPath, err := c.SecretMountPath() - if err != nil { - return nil, err - } - configsMountPath, err := c.ConfigsDirPath() - if err != nil { - return nil, err - } defer func() { if err != nil { - daemon.cleanupSecretDir(secretMountPath) - daemon.cleanupSecretDir(configsMountPath) + daemon.cleanupSecretDir(c) } }() @@ -861,10 +852,6 @@ func (daemon *Daemon) createSpec(c *container.Container) (retSpec *specs.Spec, e return nil, err } - if err := daemon.setupConfigDir(c); err != nil { - return nil, err - } - ms, err := daemon.setupMounts(c) if err != nil { return nil, err @@ -886,12 +873,6 @@ func (daemon *Daemon) createSpec(c *container.Container) (retSpec *specs.Spec, e } ms = append(ms, secretMounts...) - configMounts, err := c.ConfigMounts() - if err != nil { - return nil, err - } - ms = append(ms, configMounts...) - sort.Sort(mounts(ms)) if err := setMounts(daemon, &s, c, ms); err != nil { return nil, fmt.Errorf("linux mounts: %v", err) diff --git a/components/engine/daemon/oci_windows.go b/components/engine/daemon/oci_windows.go index 47b1301eee..64c651c4af 100644 --- a/components/engine/daemon/oci_windows.go +++ b/components/engine/daemon/oci_windows.go @@ -102,10 +102,7 @@ func (daemon *Daemon) createSpec(c *container.Container) (*specs.Spec, error) { mounts = append(mounts, secretMounts...) } - configMounts, err := c.ConfigMounts() - if err != nil { - return nil, err - } + configMounts := c.ConfigMounts() if configMounts != nil { mounts = append(mounts, configMounts...) } diff --git a/components/engine/integration/config/config_test.go b/components/engine/integration/config/config_test.go index 6a96986e4a..c152be59bf 100644 --- a/components/engine/integration/config/config_test.go +++ b/components/engine/integration/config/config_test.go @@ -292,15 +292,24 @@ func TestTemplatedConfig(t *testing.T) { AttachStderr: true, }) - buf := bytes.NewBuffer(nil) - _, err = stdcopy.StdCopy(buf, buf, attach.Reader) - require.NoError(t, err) - expect := "SERVICE_NAME=svc\n" + "this is a secret\n" + "this is a config\n" + assertAttachedStream(t, attach, expect) - assert.Equal(t, expect, buf.String()) + attach = swarm.ExecTask(t, d, task, types.ExecConfig{ + Cmd: []string{"mount"}, + AttachStdout: true, + AttachStderr: true, + }) + assertAttachedStream(t, attach, "tmpfs on /templated_config type tmpfs") +} + +func assertAttachedStream(t *testing.T, attach types.HijackedResponse, expect string) { + buf := bytes.NewBuffer(nil) + _, err := stdcopy.StdCopy(buf, buf, attach.Reader) + require.NoError(t, err) + assert.Contains(t, buf.String(), expect) } func waitAndAssert(t *testing.T, timeout time.Duration, f func(*testing.T) bool) { diff --git a/components/engine/integration/secret/secret_test.go b/components/engine/integration/secret/secret_test.go index e26d8a698d..3b5e66a5bf 100644 --- a/components/engine/integration/secret/secret_test.go +++ b/components/engine/integration/secret/secret_test.go @@ -336,15 +336,24 @@ func TestTemplatedSecret(t *testing.T) { AttachStderr: true, }) - buf := bytes.NewBuffer(nil) - _, err = stdcopy.StdCopy(buf, buf, attach.Reader) - require.NoError(t, err) - expect := "SERVICE_NAME=svc\n" + "this is a secret\n" + "this is a config\n" + assertAttachedStream(t, attach, expect) - assert.Equal(t, expect, buf.String()) + attach = swarm.ExecTask(t, d, task, types.ExecConfig{ + Cmd: []string{"mount"}, + AttachStdout: true, + AttachStderr: true, + }) + assertAttachedStream(t, attach, "tmpfs on /run/secrets/templated_secret type tmpfs") +} + +func assertAttachedStream(t *testing.T, attach types.HijackedResponse, expect string) { + buf := bytes.NewBuffer(nil) + _, err := stdcopy.StdCopy(buf, buf, attach.Reader) + require.NoError(t, err) + assert.Contains(t, buf.String(), expect) } func waitAndAssert(t *testing.T, timeout time.Duration, f func(*testing.T) bool) { From e537ce0b314ee0e590a8a09ffd87ddd301aa9a18 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Wed, 14 Feb 2018 12:45:28 -0500 Subject: [PATCH 8/8] Error out on secret/config templates for older API Makes sure if the user specifies an older API version that we don't pass through templating options for versions that templating was not supported. Signed-off-by: Brian Goff Upstream-commit: a407761e483d9c5ea425a6fd5e55fec03a90485c Component: engine --- .../engine/api/server/router/swarm/cluster_routes.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/components/engine/api/server/router/swarm/cluster_routes.go b/components/engine/api/server/router/swarm/cluster_routes.go index 33e52a3af1..865ed6add6 100644 --- a/components/engine/api/server/router/swarm/cluster_routes.go +++ b/components/engine/api/server/router/swarm/cluster_routes.go @@ -372,6 +372,10 @@ func (sr *swarmRouter) createSecret(ctx context.Context, w http.ResponseWriter, if err := json.NewDecoder(r.Body).Decode(&secret); err != nil { return err } + version := httputils.VersionFromContext(ctx) + if secret.Templating != nil && versions.LessThan(version, "1.36") { + return errdefs.InvalidParameter(errors.Errorf("secret templating is not supported on the specified API version: %s", version)) + } id, err := sr.backend.CreateSecret(secret) if err != nil { @@ -440,6 +444,11 @@ func (sr *swarmRouter) createConfig(ctx context.Context, w http.ResponseWriter, return err } + version := httputils.VersionFromContext(ctx) + if config.Templating != nil && versions.LessThan(version, "1.36") { + return errdefs.InvalidParameter(errors.Errorf("config templating is not supported on the specified API version: %s", version)) + } + id, err := sr.backend.CreateConfig(config) if err != nil { return err