Add --read-only for read only container rootfs #10093

Merged
merged 1 commit into from Jan 14, 2015

Projects

None yet

10 participants

@crosbymichael
Member

Add a --read-only flag to allow the container's root filesystem to be
mounted as read only. This can be used in combination with volumes to
force a container's process to only write to locations that will be
persisted. This is useful in many cases where the admin controls where
they would like developers to write files and error on any other
locations.

Closes #7923
Closes #8752

Signed-off-by: Michael Crosby crosbymichael@gmail.com

@LK4D4
Contributor
LK4D4 commented Jan 14, 2015

@crosbymichael Tests not compiling :)

@crosbymichael
Member

@LK4D4 i don't know what you are talking about....

@tianon
Member
tianon commented Jan 14, 2015

+1 for this even without --tmpfs 👍

:hurtrealbad:

@mrunalp mrunalp commented on an outdated diff Jan 14, 2015
docs/man/docker-run.1.md
@@ -253,6 +254,13 @@ to all devices on the host as well as set some configuration in AppArmor to
allow the container nearly all the same access to the host as processes running
outside of a container on the host.
+**--readonly**=*true*|*false*
+ Mount the container's root filesystem as readonly.
+
+ By default a container will have it's root filesystem writable allowing processes
@mrunalp
mrunalp Jan 14, 2015 Contributor

typo: it's --> its

@mrunalp mrunalp commented on an outdated diff Jan 14, 2015
docs/man/docker-run.1.md
@@ -253,6 +254,13 @@ to all devices on the host as well as set some configuration in AppArmor to
allow the container nearly all the same access to the host as processes running
outside of a container on the host.
+**--readonly**=*true*|*false*
+ Mount the container's root filesystem as readonly.
+
+ By default a container will have it's root filesystem writable allowing processes
+to write files anywhere. By specifying the `--readonly` flag the container will have
+it's root filesystem mounted as readonly prohibiting any writes.
@mrunalp
mrunalp Jan 14, 2015 Contributor

Same here.

@mrunalp
Contributor
mrunalp commented Jan 14, 2015

👍

@jessfraz
Contributor

cooooollll the diff is so smalll! LGTM

@tiborvass tiborvass and 1 other commented on an outdated diff Jan 14, 2015
docs/sources/reference/commandline/cli.md
@@ -1681,6 +1683,13 @@ will automatically create this directory on the host for you. In the
example above, Docker will create the `/doesnt/exist`
folder before starting your container.
+ $ sudo docker run -v /icanwrite --readonly busybox touch /icanwrite here
@tiborvass
tiborvass Jan 14, 2015 Contributor

Minor nit, but docker run --readonly -v /icanwrite ... could be better, as "icanwrite readonly" sounds weird.

@SvenDowideit
SvenDowideit Jan 14, 2015 Collaborator

i agree - putting the flag you're talking about first also gives it emphasis.

@tiborvass
Contributor

Should we add a small warning somewhere explaining that tmpfs mountpoints are still writeable?

@crosbymichael
Member

Any mounts are still writable, if they are mounted as rw

@tiborvass
Contributor

Oh right, it's only the rootfs that's readonly, my bad. Code LGTM

@LK4D4 LK4D4 commented on an outdated diff Jan 14, 2015
docs/sources/reference/api/docker_remote_api_v1.17.md
@@ -323,6 +326,7 @@ Return low-level information on the container `id`
"NetworkMode": "bridge",
"PortBindings": {},
"Privileged": false,
+ "ReadonlyRootfs": false,
@LK4D4
LK4D4 Jan 14, 2015 Contributor

weird formatting

@LK4D4
Contributor
LK4D4 commented Jan 14, 2015

I have no idea where, but should be in "what's new"

@SvenDowideit SvenDowideit and 2 others commented on an outdated diff Jan 14, 2015
docs/sources/reference/commandline/cli.md
@@ -1681,6 +1683,13 @@ will automatically create this directory on the host for you. In the
example above, Docker will create the `/doesnt/exist`
folder before starting your container.
+ $ sudo docker run -v /icanwrite --readonly busybox touch /icanwrite here
+
+Volumes can be used in combination with `--readonly` to control where
+a container writes files. The `--readonly` flag mounts the container's root
+filesystem as read only prohibiting writes to locations other than the
+specified volumes for the container.
+
@SvenDowideit
SvenDowideit Jan 14, 2015 Collaborator

I'd flip the first sentence to be more like

To control where a container can write files, you can combine the--readonlyflag with volumes of volume containers

Starting a discussion about readonly with Volumes made me wonder if it was out of place.

no matter tho, easy for us to discuss post-merge.

@crosbymichael
crosbymichael Jan 14, 2015 Member

@fredlf ! ! ! ^^^ I'm trying to apply what you told us but @SvenDowideit is saying you are wrong or I did it wrong.

@fredlf
fredlf Jan 15, 2015 Contributor

Heh, this is one of those cases where we can see the fact that writing is not code. There's no right or wrong, here, it's a question of what we most want to emphasize. @crosbymichael 's original sentence emphasizes the idea of "control over where a container writes". @SvenDowideit 's rewrite places reader emphasis on the idea of "combining the --readonly flag with volumes." Only the writer knows what he actually wanted to emphasize. But @SvenDowideit's response as a reader (another term for RET is reader-response criticism), gives an important clue: he did not have any context for mentally processing the concept of "volumes" when it was introduced. So, let's give the reader the context they need and expect: "The --readonly flag can be used in combination with volumes to control where a container writes files."

@SvenDowideit
Collaborator

This is a very cool featurette! Docs LGTM @fredlf @jamtur01

Need to create a working example showing how its useful tho

@jamtur01
Contributor

I think readonly should probably be read-only both in option and text. It's definitely read-only when used in a sentence.

@jamtur01
Contributor

Otherwise LGTM

@crosbymichael
Member

@jamtur01 you mean --read-only?

@jamtur01
Contributor

Yes - though I'm open to be told I'm wrong as an option - but definitely the docs should say "read-only" - readonly is wrong.

@crosbymichael
Member

@tianon what do you think? --readonly or --read-only for a cli flag?

@tianon
Member
tianon commented Jan 14, 2015

I think --read-only for consistency, even though I like --readonly.

from man mount:

       -r, --read-only
              Mount the filesystem read-only. A synonym is -o ro.

              Note  that,  depending  on the filesystem type, state and kernel
              behavior, the system may still write to the device. For example,
              ext3 or ext4 will replay its journal if the filesystem is dirty.
              To prevent this kind of write access, you may want to mount ext3
              or  ext4  filesystem  with  "ro,noload" mount options or set the
              block device to read-only mode, see command blockdev(8).
@crosbymichael
Member

@tianon thanks, consistency wins, i'll make the change in docs and code.

@crosbymichael
Member

@jamtur01 made the changes and changed the flag name to be consistent with other tools.

@jamtur01
Contributor

LGTM

@crosbymichael crosbymichael changed the title from Add --readonly for read only container rootfs to Add --read-only for read only container rootfs Jan 14, 2015
@LK4D4
Contributor
LK4D4 commented Jan 14, 2015

Okay, I was ignored again :(
I found What's new section for you @crosbymichael : docs/sources/reference/api/docker_remote_api.md.

@crosbymichael
Member

@LK4D4 I saw your comment, I just don't know what to say about adding a new field to an api object, it looks to be more about endpoints.

@LK4D4
Contributor
LK4D4 commented Jan 14, 2015

@crosbymichael Now it is possible to mount container rootfs as read-only!!!!!! or something like this. This is new feature in api, this should be there.
ping @SvenDowideit maybe you know how to write it better.

@crosbymichael crosbymichael Add --readonly for read only container rootfs
Add a --readonly flag to allow the container's root filesystem to be
mounted as readonly.  This can be used in combination with volumes to
force a container's process to only write to locations that will be
persisted.  This is useful in many cases where the admin controls where
they would like developers to write files and error on any other
locations.

Closes #7923
Closes #8752

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
4094070
@crosbymichael
Member

@LK4D4 added

@LK4D4
Contributor
LK4D4 commented Jan 14, 2015

LGTM

@LK4D4 LK4D4 merged commit 95c0f07 into docker:master Jan 14, 2015

1 check passed

default The build succeeded on drone.io
Details
@crosbymichael crosbymichael deleted the crosbymichael:readonly-containers branch Jan 14, 2015
@noisy
noisy commented Feb 16, 2015

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment