shim: create workdir if it does not exist

[AkihiroSuda]

This PR let shim to create the workdir when it does not exist, as in docker run --workdir and Dockerfile WORKDIR.

BuildKit (moby/buildkit#121) requires this.

How to test:

• ctr run -t --rm --workdir /foobar docker.io/library/alpine:latest foo pwd
• ctr run -t --rm --workdir /foobar --readonly docker.io/library/alpine:latest foo pwd
• ctr run -t --rm --workdir /../../../../../../../../../../../../../HACKED docker.io/library/alpine:latest foo ls -ld /HACKED (Make sure it doesn't create /HACKED on the host)

cc @tonistiigi @crosbymichael @dmcgowan

[codecov-io]

Codecov Report

Merging #1585 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1585   +/-   ##
=======================================
  Coverage   42.44%   42.44%           
=======================================
  Files          24       24           
  Lines        3362     3362           
=======================================
  Hits         1427     1427           
  Misses       1608     1608           
  Partials      327      327

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8ded4fe...cca5bbb. Read the comment docs.

[estesp]

One comment about dir creation being user namespace aware; also, you should you probably add a test for the "breakout" example in your PR first comment

[estesp]

I think you will also need to look at the spec to see if user namespaces are configured for this container and Mkdir with proper ownership in mind or else the Cwd will be inaccessible to the process started.

[crosbymichael]

if anything, runc should create this because its already in a userns, pivot_root'ed and can be a simple MkdirAll(cwd, 0755) and no other code would need to be added.

However, I'm not sure its runc job to create these dirs, but, we do that for mount targets so maybe it would get accepted

[crosbymichael]

I'm not sure about this change. Did you notice that NOTHING reads the spec in containerd? There is a reason for that.

I read your issue and you say it works with the runc worker but not containerd. How is that possible? runc does not create the cwd as well.

Why can you not create the cwd in build kit as well? I don't think its our responsibility to create your directories nor is it runc's responsibility.

[AkihiroSuda]

Yes, BuildKit could create cwd.

However, for non-buildkit usecases such as ctr run with non-existing Cwd (image config or --workdir flag), I think containerd should be responsible for creating workdir.

[crosbymichael]

I don't think it should especially when nothing else in the entire code base reads the spec and has to jump though hoops to do dir creation. Especially not for ctr.

[AkihiroSuda]

May I open PR to runc (and runtime spec) then?

#1585 (comment)

[crosbymichael]

You can give it a shot and see what the other maintainers think. Make sure you state that it would be helpful for userns and that runc already does it for mounts

[AkihiroSuda]

Then I'll remove mkdir from this PR but leave WithProcessCwd spec opt

[crosbymichael]

@AkihiroSuda SGTM, the option is good

[crosbymichael]

I'd also rename the flag to --cwd instead of workdir

[AkihiroSuda]

OK.

For archival purpose I'm closing this PR, and open a new PR later for --cwd

[AkihiroSuda]

opened opencontainers/runc#1604