Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add ArgsEscaped field to image config #829

Closed
wants to merge 1 commit into from
Closed

Add ArgsEscaped field to image config #829

wants to merge 1 commit into from

Conversation

kevpar
Copy link
Contributor

@kevpar kevpar commented Mar 24, 2021 •
edited
Loading

This PR is an attempt to fix containerd/containerd#5067, which currently prevents certain Docker-built Windows images from working in containerd. See the containerd issue for full details. Brief overview below.

Docker has been relying on the unofficial ArgsEscaped field in the image config for several years now. It is added when a Windows image is built using a "shell form" ENTRYPOINT/CMD in the Dockerfile, and indicates that the first element of the Entrypoint/Cmd field contains a full command line, rather than just a path to a binary.

This change was made in Docker to improve support for Windows containers, given differences in how Windows and Linux handle command line args and escaping. Given that there are existing images in the world that rely on this field being set, the easiest way to address the problem appears to be simply adding ArgsEscaped officially to the image spec.

For context, this is how ArgsEscaped is represented in Docker's image config type: https://github.com/moby/moby/blob/46cdcd206c56172b95ba5c77b827a722dab426c5/api/types/container/config.go#L57

@kevpar
Add ArgsEscaped field to image config
f2b6e6e 
This change officially adds ArgsEscaped to the image config. This field
has already been used by Docker for several years, so adding it here
allows images that depend on its behavior to work with other runtimes.

Signed-off-by: Kevin Parsons <[email protected]>
@kevpar
Copy link
Contributor Author

kevpar commented Mar 24, 2021

After some internal discussion, going to look into another option for fixing this. Might re-open this in the future if that doesn't pan out.

@kevpar kevpar closed this Mar 24, 2021
@siprbaum siprbaum mentioned this pull request Jun 16, 2021
Closed
@jterry75
Copy link

jterry75 commented Dec 6, 2021

@kevpar - Can we revisit this? Were there any objections from OCI to adding ArgsEscaped to the config spec here? It doesn't seem harmful to add given that Docker (/Buildkit) support ArgsEscaped and are the largest producer of images. It's easy to document that ArgsEscaped==true means that the EntryPoint is a pre-escaped single entry array and should not be additionally escaped. Additionally, we already have a provision in the Runtime spec for CommandLine rather than Args specifically to address this issue on Windows. It seems reasonable to have this IMO and then I can make the change to containerd to support it natively. Thoughts?

@jonjohnsonjr jonjohnsonjr mentioned this pull request Dec 20, 2021
Closed
@estesp
Copy link
Contributor

estesp commented Jan 27, 2022

Seems with the suggestion of using this in containerd via containerd/containerd#6479 we should re-open this for consideration?

@jterry75 jterry75 mentioned this pull request Jan 27, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CRI: Windows images created with "shell form" CMD in Dockerfile are broken
3 participants
@kevpar @jterry75 @estesp