From 499a0dd43e50c6f253f8890f5c54ae99675b1e7e Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Sat, 14 Jan 2017 00:12:19 -0800 Subject: [PATCH] Add `--read-only` for `service create` and `service update` This fix tries to address the issue raised in 29972 where it was not possible to specify `--read-only` for `docker service create` and `docker service update`, in order to have the container's root file system to be read only. This fix adds `--read-only` and update the `ReadonlyRootfs` in `HostConfig` through `service create` and `service update`. Related docs has been updated. Integration test has been added. This fix fixes 29972. Signed-off-by: Yong Tang --- api/swagger.yaml | 3 +++ api/types/swarm/container.go | 1 + cli/command/service/opts.go | 6 +++++ cli/command/service/update.go | 8 +++++++ cli/command/service/update_test.go | 22 +++++++++++++++++++ daemon/cluster/convert/container.go | 2 ++ .../cluster/executor/container/container.go | 9 ++++---- docs/reference/commandline/service_create.md | 1 + docs/reference/commandline/service_update.md | 1 + integration-cli/docker_cli_swarm_test.go | 21 ++++++++++++++++++ 10 files changed, 70 insertions(+), 4 deletions(-) diff --git a/api/swagger.yaml b/api/swagger.yaml index e753bb5f5a..e93a4a9aa0 100644 --- a/api/swagger.yaml +++ b/api/swagger.yaml @@ -1884,6 +1884,9 @@ definitions: TTY: description: "Whether a pseudo-TTY should be allocated." type: "boolean" + ReadOnly: + description: "Mount the container's root filesystem as read only." + type: "boolean" Mounts: description: "Specification for mounts to be added to containers created as part of the service." type: "array" diff --git a/api/types/swarm/container.go b/api/types/swarm/container.go index 4ab476ccc3..f385fa0402 100644 --- a/api/types/swarm/container.go +++ b/api/types/swarm/container.go @@ -34,6 +34,7 @@ type ContainerSpec struct { Groups []string `json:",omitempty"` TTY bool `json:",omitempty"` OpenStdin bool `json:",omitempty"` + ReadOnly bool `json:",omitempty"` Mounts []mount.Mount `json:",omitempty"` StopGracePeriod *time.Duration `json:",omitempty"` Healthcheck *container.HealthConfig `json:",omitempty"` diff --git a/cli/command/service/opts.go b/cli/command/service/opts.go index 2218890aa3..dcd52ac7a3 100644 --- a/cli/command/service/opts.go +++ b/cli/command/service/opts.go @@ -303,6 +303,7 @@ type serviceOptions struct { user string groups opts.ListOpts tty bool + readOnly bool mounts opts.MountOpt dns opts.ListOpts dnsSearch opts.ListOpts @@ -384,6 +385,7 @@ func (opts *serviceOptions) ToService() (swarm.ServiceSpec, error) { User: opts.user, Groups: opts.groups.GetAll(), TTY: opts.tty, + ReadOnly: opts.readOnly, Mounts: opts.mounts.Value(), DNSConfig: &swarm.DNSConfig{ Nameservers: opts.dns.GetAll(), @@ -488,6 +490,9 @@ func addServiceFlags(cmd *cobra.Command, opts *serviceOptions) { flags.BoolVarP(&opts.tty, flagTTY, "t", false, "Allocate a pseudo-TTY") flags.SetAnnotation(flagTTY, "version", []string{"1.25"}) + + flags.BoolVar(&opts.readOnly, flagReadOnly, false, "Mount the container's root filesystem as read only") + flags.SetAnnotation(flagReadOnly, "version", []string{"1.26"}) } const ( @@ -532,6 +537,7 @@ const ( flagPublish = "publish" flagPublishRemove = "publish-rm" flagPublishAdd = "publish-add" + flagReadOnly = "read-only" flagReplicas = "replicas" flagReserveCPU = "reserve-cpu" flagReserveMemory = "reserve-memory" diff --git a/cli/command/service/update.go b/cli/command/service/update.go index 66002b69e0..57a4577f88 100644 --- a/cli/command/service/update.go +++ b/cli/command/service/update.go @@ -341,6 +341,14 @@ func updateService(flags *pflag.FlagSet, spec *swarm.ServiceSpec) error { cspec.TTY = tty } + if flags.Changed(flagReadOnly) { + readOnly, err := flags.GetBool(flagReadOnly) + if err != nil { + return err + } + cspec.ReadOnly = readOnly + } + return nil } diff --git a/cli/command/service/update_test.go b/cli/command/service/update_test.go index bb931929c0..992ae9ef3b 100644 --- a/cli/command/service/update_test.go +++ b/cli/command/service/update_test.go @@ -442,3 +442,25 @@ func TestUpdateSecretUpdateInPlace(t *testing.T) { assert.Equal(t, updatedSecrets[0].SecretName, "foo") assert.Equal(t, updatedSecrets[0].File.Name, "foo2") } + +func TestUpdateReadOnly(t *testing.T) { + spec := &swarm.ServiceSpec{} + cspec := &spec.TaskTemplate.ContainerSpec + + // Update with --read-only=true, changed to true + flags := newUpdateCommand(nil).Flags() + flags.Set("read-only", "true") + updateService(flags, spec) + assert.Equal(t, cspec.ReadOnly, true) + + // Update without --read-only, no change + flags = newUpdateCommand(nil).Flags() + updateService(flags, spec) + assert.Equal(t, cspec.ReadOnly, true) + + // Update with --read-only=false, changed to false + flags = newUpdateCommand(nil).Flags() + flags.Set("read-only", "false") + updateService(flags, spec) + assert.Equal(t, cspec.ReadOnly, false) +} diff --git a/daemon/cluster/convert/container.go b/daemon/cluster/convert/container.go index b160c6a482..b4c1e0cee6 100644 --- a/daemon/cluster/convert/container.go +++ b/daemon/cluster/convert/container.go @@ -25,6 +25,7 @@ func containerSpecFromGRPC(c *swarmapi.ContainerSpec) types.ContainerSpec { Groups: c.Groups, TTY: c.TTY, OpenStdin: c.OpenStdin, + ReadOnly: c.ReadOnly, Hosts: c.Hosts, Secrets: secretReferencesFromGRPC(c.Secrets), } @@ -146,6 +147,7 @@ func containerToGRPC(c types.ContainerSpec) (*swarmapi.ContainerSpec, error) { Groups: c.Groups, TTY: c.TTY, OpenStdin: c.OpenStdin, + ReadOnly: c.ReadOnly, Hosts: c.Hosts, Secrets: secretReferencesToGRPC(c.Secrets), } diff --git a/daemon/cluster/executor/container/container.go b/daemon/cluster/executor/container/container.go index f0a10697f4..fdb06c25e1 100644 --- a/daemon/cluster/executor/container/container.go +++ b/daemon/cluster/executor/container/container.go @@ -335,10 +335,11 @@ func (c *containerConfig) healthcheck() *enginecontainer.HealthConfig { func (c *containerConfig) hostConfig() *enginecontainer.HostConfig { hc := &enginecontainer.HostConfig{ - Resources: c.resources(), - GroupAdd: c.spec().Groups, - PortBindings: c.portBindings(), - Mounts: c.mounts(), + Resources: c.resources(), + GroupAdd: c.spec().Groups, + PortBindings: c.portBindings(), + Mounts: c.mounts(), + ReadonlyRootfs: c.spec().ReadOnly, } if c.spec().DNSConfig != nil { diff --git a/docs/reference/commandline/service_create.md b/docs/reference/commandline/service_create.md index c9e298096b..08771272a7 100644 --- a/docs/reference/commandline/service_create.md +++ b/docs/reference/commandline/service_create.md @@ -48,6 +48,7 @@ Options: --network list Network attachments (default []) --no-healthcheck Disable any container-specified HEALTHCHECK -p, --publish port Publish a port as a node port + --read-only Mount the container's root filesystem as read only --replicas uint Number of tasks --reserve-cpu decimal Reserve CPUs (default 0.000) --reserve-memory bytes Reserve Memory (default 0 B) diff --git a/docs/reference/commandline/service_update.md b/docs/reference/commandline/service_update.md index 301a0eabe8..654c59b875 100644 --- a/docs/reference/commandline/service_update.md +++ b/docs/reference/commandline/service_update.md @@ -58,6 +58,7 @@ Options: --no-healthcheck Disable any container-specified HEALTHCHECK --publish-add port Add or update a published port --publish-rm port Remove a published port by its target port + --read-only Mount the container's root filesystem as read only --replicas uint Number of tasks --reserve-cpu decimal Reserve CPUs (default 0.000) --reserve-memory bytes Reserve Memory (default 0 B) diff --git a/integration-cli/docker_cli_swarm_test.go b/integration-cli/docker_cli_swarm_test.go index 426346952a..3a88a00394 100644 --- a/integration-cli/docker_cli_swarm_test.go +++ b/integration-cli/docker_cli_swarm_test.go @@ -1644,3 +1644,24 @@ func (s *DockerSwarmSuite) TestSwarmInitWithDrain(c *check.C) { c.Assert(err, checker.IsNil) c.Assert(out, checker.Contains, "Drain") } + +func (s *DockerSwarmSuite) TestSwarmReadonlyRootfs(c *check.C) { + testRequires(c, DaemonIsLinux, UserNamespaceROMount) + + d := s.AddDaemon(c, true, true) + + out, err := d.Cmd("service", "create", "--name", "top", "--read-only", "busybox", "top") + c.Assert(err, checker.IsNil, check.Commentf(out)) + + // make sure task has been deployed. + waitAndAssert(c, defaultReconciliationTimeout, d.CheckActiveContainerCount, checker.Equals, 1) + + out, err = d.Cmd("service", "inspect", "--format", "{{ .Spec.TaskTemplate.ContainerSpec.ReadOnly }}", "top") + c.Assert(err, checker.IsNil, check.Commentf(out)) + c.Assert(strings.TrimSpace(out), checker.Equals, "true") + + containers := d.ActiveContainers() + out, err = d.Cmd("inspect", "--type", "container", "--format", "{{.HostConfig.ReadonlyRootfs}}", containers[0]) + c.Assert(err, checker.IsNil, check.Commentf(out)) + c.Assert(strings.TrimSpace(out), checker.Equals, "true") +}