Docs freshness updates to Docker run command docs

[ChrisChinchilla]

This ended up being a lot more changes than expected, but is something of a refresh for consistency, style, understanding, and freshness.

[thaJeztah]

Thank you for contributing! It appears your commit message is missing a DCO sign-off,
causing the DCO check to fail.

We require all commit messages to have a Signed-off-by line with your name
and e-mail (see "Sign your work"
in the CONTRIBUTING.md in this repository), which looks something like:

Signed-off-by: YourFirsName YourLastName <yourname@example.org>

There is no need to open a new pull request, but to fix this (and make CI pass),
you need to amend the commit(s) in this pull request, and "force push" the amended
commit.

Unfortunately, it's not possible to do so through GitHub's web UI, so this needs
to be done through the git commandline.

You can find some instructions in the output of the DCO check (which can be found
in the "checks" tab on this pull request), as well as in the Moby contributing guide.

Steps to do so "roughly" come down to:

1. Set your name and e-mail in git's configuration:

   git config --global user.name "YourFirstName YourLastName"
   git config --global user.email "yourname@example.org"

   (Make sure to use your real name (not your GitHub username/handle) and e-mail)

2. Clone your fork locally

3. Check out the branch associated with this pull request

4. Sign-off and amend the existing commit(s)

   git commit --amend --no-edit --signoff

   If your pull request contains multiple commits, either squash the commits (if
   needed) or sign-off each individual commit.

5. Force push your branch to GitHub (using the --force or --force-with-lease flags) to update the pull request.

Sorry for the hassle (I wish GitHub would make this a bit easier to do), and let me know if you need help or more detailed instructions!

[codecov-commenter]

Codecov Report

Merging #4144 (f753047) into master (6d58b07) will decrease coverage by 0.01%.
The diff coverage is n/a.

❗ Current head f753047 differs from pull request most recent head e693e7f. Consider uploading reports for the commit e693e7f to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4144      +/-   ##
==========================================
- Coverage   58.87%   58.86%   -0.01%     
==========================================
  Files         570      572       +2     
  Lines       49558    49572      +14     
==========================================
+ Hits        29178    29182       +4     
- Misses      18616    18624       +8     
- Partials     1764     1766       +2     

[thaJeztah]

Thanks! Quite some improvements in here. While reading, I found some issues, and left some comments (some aren't new, but might be something we need to look into, which can be in a follow up)

Let me know if you want to go through some of them!

[thaJeztah]

For a follow-up, we should make a pass at searching for --privileged examples. Using --privileged is a BIG hammer, and effectively disables all security that containers provide (making a privileged container effectively "not a container", or a container that "doesn't contain). For cases where we can we should document the minimum amount of (additional) privileges needed. (And have a canonical section on "privileged" with tons of ⚠️ warnings (don't use this unless you really, really need it, and know what you're doing ⚠️))

For this specific example, it looks like we need CAP_SYS_ADMIN, so the example can look like;

$ docker run -it --cap-add CAP_SYS_ADMIN ubuntu bash
root@50e3f57e16e6:/# mount -t tmpfs none /mnt
root@50e3f57e16e6:/# df -h
Filesystem      Size  Used Avail Use% Mounted on
none            1.9G     0  1.9G   0% /mnt

Or even (using alpine);

$ docker run -it --cap-add CAP_SYS_ADMIN alpine
/ # mount -t tmpfs none /mnt
/ # df -h
Filesystem                Size      Used Available Use% Mounted on
overlay                  58.4G     27.8G     27.6G  50% /
tmpfs                    64.0M         0     64.0M   0% /dev
shm                      64.0M         0     64.0M   0% /dev/shm
/dev/vda1                58.4G     27.8G     27.6G  50% /etc/resolv.conf
/dev/vda1                58.4G     27.8G     27.6G  50% /etc/hostname
/dev/vda1                58.4G     27.8G     27.6G  50% /etc/hosts
tmpfs                    64.0M         0     64.0M   0% /proc/kcore
tmpfs                    64.0M         0     64.0M   0% /proc/keys
tmpfs                    64.0M         0     64.0M   0% /proc/timer_list
tmpfs                     3.8G         0      3.8G   0% /sys/firmware
none                      3.8G         0      3.8G   0% /mnt

I should note that the existing example only shows the "essential" lines from the output (the mnt mount) for illustrational purposes.

• I guess we could make it df -h | grep mnt (so that the output matches actual output) but we'd loose the headers :thinking_face:
• Should we use a mount destination that more clearly stands out as "this is an example"? (mkdir /example-mount && mount -t tmpfs none /example-mount)? Note that the existing example uses /mnt (which already exists), but other locations won't (so users would run into an error).

Some other bits (for follow-ups);

• Review the "Runtime privilege and Linux capabilities" to check if the list of capabilities is complete (recent kernel versions added some new capabilities).
• While we accept both SYS_ADMIN and CAP_SYS_ADMIN (as well as lowercase variants), I know that internally (at least daemon-side) we normalize to CAP_SOME_CAPABILITY (see https://github.com/moby/moby/blob/58c027ac8ba19a3fa339c65274ea6704ccbec977/oci/caps/utils.go#L56-L79), so I wonder if we should update examples (and the table) to use the same for consistency
• Some examples use ubuntu (ubuntu:latest); there's two issues there (possibly worth considering);
  • depending on the example, ubuntu may not always be needed, and we could make the examples more lightweight by using alpine (alpine's apk add --no-cache install is super fast as well, which makes it great to provide quick examples that need additional packages installed), and we should probably prefer alpine over busybox (where we use it).
  • this might be a bit of a stretch, but :latest (while it reduces maintenance on our side) rarely is "best practice". Now, this is a bit of a grey area; for quick "one-off" things (such as docker run alpine echo hello), or other things one may do, I guess "it's fine" (don't want to be typing too much). On the other hand, building an image using ubuntu:latest (e.g.) is definitely not the way to go (latest may be a new version of Ubuntu "tomorrow", and break your image!) so we need to find the right balance between that (show best practices where needed to make users familiar with them).

[ChrisChinchilla]

@thaJeztah The existing text around this example says…

This doesn't work, because by default, Docker drops most potentially dangerous kernel
capabilities, including CAP_SYS_ADMIN (which is required to mount
filesystems).

So is that also wrong? As the proposed example uses something the text explicitly says won't work.

And noted for all the follow up.

[thaJeztah]

@ChrisChinchilla the example is correct that it doesn't work. The issue here is a bit that we use --privileged to work around that, whereas (for this example) just adding the CAP_SYS_ADMIN capability would've worked.

--privileged is a bit of a big hammer (and should be avoided in most cases, as it removes all security that a container provides); granting CAP_SYS_ADMIN is just one thing it does (it grants "all capabilities", but also disables seccomp, selinux, apparmor, and grants access to some paths that are otherwise protected.

TL;DR it works, but should not be the recommended approach for this specific case

[thaJeztah]

Ugh! Looks like GitHub is a bit over-enthusiastic again, and hid some of my comments, so you may have to expand those ⚠️

[thaJeztah]

@ChrisChinchilla the example is correct that it doesn't work. The issue here is a bit that we use --privileged to work around that, whereas (for this example) just adding the CAP_SYS_ADMIN capability would've worked.

--privileged is a bit of a big hammer (and should be avoided in most cases, as it removes all security that a container provides); granting CAP_SYS_ADMIN is just one thing it does (it grants "all capabilities", but also disables seccomp, selinux, apparmor, and grants access to some paths that are otherwise protected.

TL;DR it works, but should not be the recommended approach for this specific case

[thaJeztah]

changes LGTM; could you squash the commits?

[ChrisChinchilla]

@thaJeztah Done!

[thaJeztah]

LGTM, thanks!

[thaJeztah]

oh! looks like the markdown needs to be regenerated; let me do so quickly

[thaJeztah]

All green now 👍