validation: LinuxUIDMapping: fix tests #597

alban · 2018-03-08T18:59:26Z

Don't validate uid mappings and gid mappings separately: containers with
only user mappings or with only group mappings are not usable.

Additionally, don't rely on the runtime to create the directories to be
mounted. runc mounts them in the easy cases but it does not work with
user namespaces.

Marking as WIP/RFC because this is in discussion in opencontainers/runtime-spec#955

Signed-off-by: Alban Crequy [email protected]

wking · 2018-03-08T19:15:24Z

On Thu, Mar 08, 2018 at 10:59:26AM -0800, Alban Crequy wrote: Additionnally, …

nit: typo; should be “Additionally”. In the commit message too.

wking · 2018-03-08T19:16:32Z

validation/linux_uid_mappings.go

+	err := util.RuntimeInsideValidate(g, func(path string) error {
+		_ = mkdir(path, "proc")
+		_ = mkdir(path, "dev")
+		_ = mkdir(path, "sys")


If we need these (and it sounds like we do for now), I think they should be part of PrepareBundle. Until we have clearer spec wording around creation, I'd rather not rely on the details of runc's “sometimes we can do this” implementation.

alban · 2018-03-09T14:30:58Z

I added /proc, /dev, /sys in the list of files for the rootfs and I had to update the Gentoo scripts.

The tests about uid/gid mappings now pass with runc:

$ sudo validation/linux_uid_mappings.t
TAP version 13
not ok 1 - root filesystem
  ---
  {
    "error": "rootfs must not be readonly\nRefer to: https://github.com/opencontainers/runtime-spec/blob/v1.0.0/config.md#root"
  }
  ...
ok 2 - hostname
ok 3 - process
ok 4 - mounts
ok 5 - user
ok 6 - rlimits
ok 7 - capabilities
ok 8 - default symlinks
ok 9 - default file system
ok 10 - default devices
ok 11 - linux devices
ok 12 - linux process
ok 13 - masked paths
ok 14 - oom score adj
ok 1 # SKIP syscall action SCMP_ACT_ALLOW
ok 2 # SKIP syscall action SCMP_ACT_ALLOW
ok 3 # SKIP syscall action SCMP_ACT_ALLOW
ok 4 # SKIP syscall action SCMP_ACT_ALLOW
ok 5 # SKIP syscall action SCMP_ACT_ALLOW
ok 6 # SKIP syscall action SCMP_ACT_ALLOW
ok 15 - seccomp
ok 16 - read only paths
ok 17 - rootfs propagation
ok 18 - sysctls
ok 19 - uid mappings
ok 20 - gid mappings
1..20

wking · 2018-03-09T15:45:24Z

contrib/rootfs-builder/get-stage3.sh

-DATE=$(echo "${LATEST}" | sed -n "s|/stage3-${STAGE3_ARCH}-[0-9]*[.]tar[.]bz2.*||p")
-ARCH_URL="${ARCH_URL:-${BASE_ARCH_URL}${DATE}/}"
-STAGE3="${STAGE3:-stage3-${STAGE3_ARCH}-${DATE}.tar.bz2}"
+DATE=$(echo "${LATEST}" | sed -n "s|/.*stage3-${STAGE3_ARCH}-systemd-[0-9]*[.]tar[.]bz2.*||p" | tail -1)


Thanks for pointing out the current breakage :).

I don't think we want to use the systemd image, since we're just grabbing the BusyBox binary. It's currently quite a bit bigger:

20180308T214502Z/stage3-amd64-20180308T214502Z.tar.xz 188851072 … 20180303/systemd/stage3-amd64-systemd-20180303.tar.bz2 281873162

I'm in the process of updating the rootfs builder for the new format, but haven't gotten through the variable compression (xz, bz2, …) yet. I've filed a WIP #598 with where I am now.

Ok, I'll rebase my PR when yours gets merged.

wking · 2018-03-09T15:54:36Z

contrib/rootfs-builder/Makefile

 .PRECIOUS: rootfs/%/bin/echo
 rootfs/%/bin/echo: rootfs/%/bin/busybox
-	sudo sh -c 'COMMANDS=$$($< --list) || exit 1; for COMMAND in $${COMMANDS}; do \
+	sudo sh -c 'COMMANDS=$$($< --list | grep -v "^busybox$$") || exit 1; for COMMAND in $${COMMANDS}; do \


Why exclude it? It's a valid command in its own right. Ah, because you don't want to link it to itself, which makes sense. I wonder if this is new to recent BusyBox...

Because the Makefile will call ln -rs busybox busybox and it will fail:

sudo sh -c 'COMMANDS=$(rootfs/amd64/bin/busybox --list) || exit 1; for COMMAND in ${COMMANDS}; do \ test -L "rootfs/amd64/bin/${COMMAND}" || ln -rs rootfs/amd64/bin/busybox "rootfs/amd64/bin/${COMMAND}" || exit; \ done' ln: failed to create symbolic link 'rootfs/amd64/bin/busybox': File exists

The busybox command doesn't need a symlink.

alban · 2018-03-09T16:54:28Z

$ sudo validation/linux_uid_mappings.t
TAP version 13
not ok 1 - root filesystem
  ---
  {
    "error": "rootfs must not be readonly\nRefer to: https://github.com/opencontainers/runtime-spec/blob/v1.0.0/config.md#root"
  }

This error is addressed separately in #599

Don't validate uid mappings and gid mappings separately: containers with only user mappings or with only group mappings are not usable. The tests about uid/gid mappings now pass with runc: ``` $ sudo validation/linux_uid_mappings.t TAP version 13 ok 1 - root filesystem ok 2 - hostname ok 3 - process ok 4 - mounts ok 5 - user ok 6 - rlimits ok 7 - capabilities ok 8 - default symlinks ok 9 - default file system ok 10 - default devices ok 11 - linux devices ok 12 - linux process ok 13 - masked paths ok 14 - oom score adj ok 1 # SKIP syscall action SCMP_ACT_ALLOW ok 2 # SKIP syscall action SCMP_ACT_ALLOW ok 3 # SKIP syscall action SCMP_ACT_ALLOW ok 4 # SKIP syscall action SCMP_ACT_ALLOW ok 5 # SKIP syscall action SCMP_ACT_ALLOW ok 6 # SKIP syscall action SCMP_ACT_ALLOW ok 15 - seccomp ok 16 - read only paths ok 17 - rootfs propagation ok 18 - sysctls ok 19 - uid mappings ok 20 - gid mappings 1..20 ``` Signed-off-by: Alban Crequy <[email protected]>

alban · 2018-03-14T15:57:21Z

@wking I rebased this PR on master (with #599 and #598) and updated the commitmsg.

zhouhao3 · 2018-03-20T01:17:18Z

LGTM

alban mentioned this pull request Mar 8, 2018

Mounts: allow runtimes to create the destination opencontainers/runtime-spec#955

Closed

wking reviewed Mar 8, 2018

View reviewed changes

alban force-pushed the alban/uid-mapping-mkdir branch from 0247f07 to 7a436fd Compare March 9, 2018 14:23

alban changed the title ~~[WIP][RFC] validation: LinuxUIDMapping: fix tests~~ validation: LinuxUIDMapping: fix tests Mar 9, 2018

wking reviewed Mar 9, 2018

View reviewed changes

wking mentioned this pull request Mar 9, 2018

contrib/rootfs-builder: Support timestamps and xz compression #598

Merged

alban force-pushed the alban/uid-mapping-mkdir branch 2 times, most recently from c3978cc to dabdc17 Compare March 14, 2018 15:52

alban force-pushed the alban/uid-mapping-mkdir branch from dabdc17 to 0f3cf9d Compare March 14, 2018 15:54

wking approved these changes Mar 14, 2018

View reviewed changes

zhouhao3 merged commit ed542c1 into opencontainers:master Mar 20, 2018

dongsupark deleted the alban/uid-mapping-mkdir branch May 22, 2018 09:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

validation: LinuxUIDMapping: fix tests #597

validation: LinuxUIDMapping: fix tests #597

Uh oh!

alban commented Mar 8, 2018 •

edited

Loading

Uh oh!

wking commented Mar 8, 2018 via email

Uh oh!

wking Mar 8, 2018

Uh oh!

alban commented Mar 9, 2018

Uh oh!

wking Mar 9, 2018 •

edited

Loading

Uh oh!

alban Mar 9, 2018 •

edited

Loading

Uh oh!

wking Mar 9, 2018 •

edited

Loading

Uh oh!

alban Mar 9, 2018

Uh oh!

alban commented Mar 9, 2018

Uh oh!

alban commented Mar 14, 2018

Uh oh!

zhouhao3 commented Mar 20, 2018 •

edited by caniszczyk

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

validation: LinuxUIDMapping: fix tests #597

validation: LinuxUIDMapping: fix tests #597

Uh oh!

Conversation

alban commented Mar 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wking commented Mar 8, 2018 via email

Uh oh!

wking Mar 8, 2018

Choose a reason for hiding this comment

Uh oh!

alban commented Mar 9, 2018

Uh oh!

wking Mar 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alban Mar 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wking Mar 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alban Mar 9, 2018

Choose a reason for hiding this comment

Uh oh!

alban commented Mar 9, 2018

Uh oh!

alban commented Mar 14, 2018

Uh oh!

zhouhao3 commented Mar 20, 2018 • edited by caniszczyk Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

alban commented Mar 8, 2018 •

edited

Loading

wking Mar 9, 2018 •

edited

Loading

alban Mar 9, 2018 •

edited

Loading

wking Mar 9, 2018 •

edited

Loading

zhouhao3 commented Mar 20, 2018 •

edited by caniszczyk

Loading