runc@master failing with Moby CI: TestTemplatedConfig: Options:[rbind ro]}: mount destination templated_config not absolute: unknown

[AkihiroSuda]

--- FAIL: TestTemplatedConfig (61.70s)

[2021-05-01T17:09:25.901Z]     config_test.go:307: timeout hit after 1m0s: waiting for tasks to enter run state. task failed with error: starting container failed: failed to create shim: OCI runtime create failed: invalid mount {Destination:templated_config Type:bind Source:/go/src/github.com/docker/docker/bundles/test-integration/TestTemplatedConfig/d1fa52d447d48/root/containers/301097d0bae895342d137fd9217923474fddc25e6f06af4e3c3bd5a17cb5283c/mounts/secrets/l447q2mi60ayqntjg5bajt1hd Options:[rbind ro]}: mount destination templated_config not absolute: unknown

moby/moby#42308
(moby/moby@59751bb , runc @ d279ebd)

Regression in 2192670 ( #2917 )

libct/configs/validate: validate mounts
    
Add a check that mount destination is absolute (as per OCI spec).

OCI spec: https://github.com/opencontainers/runtime-spec/blob/master/config.md#mounts

[AkihiroSuda]

• Option 1: revert Validate mounts #2917, and violate OCI spec
• Option 2: revert Validate mounts #2917, and modify OCI spec to allow slash-less relative path
• Option 3: modify Moby to avoid using relative paths

Option 3 seems the best solution (Being tested in Moby CI: moby/moby#42308)

cc @thaJeztah @tiborvass

[cyphar]

Yeah I agree option 3 is the best solution.

[AkihiroSuda]

Let's close this and proceed the release process.

[thaJeztah]

Generally, option 3 sounds good to me (actually surprised that this worked).

If we fix this in Moby/Docker, the only concern would be for existing versions of docker that would upgrade to a newer version of RunC.

Having a quick look, and it looks like regular bind-mounts already produce an error (on docker 20.10, runc rc93);

$ docker run -it --rm -w /app -v $(pwd):somewhere alpine sh
docker: Error response from daemon: invalid volume specification: '/host_mnt/Users/sebastiaan/go/src/github.com/docker/docker.github.io:somewhere': invalid mount config for type "bind": invalid mount path: 'somewhere' mount path must be absolute.
See 'docker run --help'.

$ docker run -it --rm -w /app --mount type=bind,src=$(pwd),target=somewhere alpine sh
docker: Error response from daemon: invalid mount config for type "bind": invalid mount path: 'somewhere' mount path must be absolute.
See 'docker run --help'.

I think this would only affect swarm configs; for swarm secrets, the /run/secrets/ path is prepended, but for configs, there is no default path (assumes /);

$ echo 'bla' | docker config create bla -
$ echo 'bla' | docker secret create bla -
$ docker service create \
   --name myservice \
   --config src=bla,target=configblatarget \
   --secret src=bla,target=secretblatarget \
   nginx:alpine

Above doesn't produce an error, and the config is mounted at /configblatarget, and the secret at /run/secret/secretblatarget

I think it's a bug, and it should either produce an error, or convert the path to be absolute (relative to / for configs)

[cpuguy83]

Reminder that moby is not the only project that uses runc.

[cpuguy83]

Basically, a configuration that worked before no longer works.
We should also test cri-containerd and cri-o.

[AkihiroSuda]

Cri-containerd is passing the tests with runc master containerd/containerd#5442

[cpuguy83]

Right but are the tests testing this case?

[cpuguy83]

Applying through the normal flow seems to be ok in cri-containerd at least (it prepends a /).

[kolyshkin]

AFACS fixing in it higher-level software is straightforward (make the paths absolute).

At least some of cri-o tests are run against HEAD runc and we did not see failures (surely there might be no such test case)