search

containers / common

Location for shared common files in github.com/containers repos.
Apache License 2.0
190 stars 196 forks source link

Image-import change handles space's badly #1289

Open cevich opened 1 year ago

cevich commented 1 year ago

I'm told the libimage.ImageConfigFromChanges() function is broken. Reproducer:

(The exact image used doesn't matter)

$ podman pull registry.fedoraproject.org/fedora:latest
$ podman save registry.fedoraproject.org/fedora:latest > asdf.tar
$ podman import -c 'ENV=PS1=$(whoami) $(hostname) $ ' asdf.tar

Output:

Error: invalid change "ENV=PS1=$(whoami) $(hostname) $ " - invalid instruction ENV=PS1=$(WHOAMI)

What does work is:

$ podman import -c 'ENV "PS1=$(whoami) $(hostname) $ "' asdf.tar

Note, without the double-quotes it imports the image, but fails to preserve the trailing space:

$ podman import -c 'ENV PS1=$(whoami) $(hostname) $ ' asdf.tar
$ podman image inspect --format '{{.Config.Env}}' 1bde4dd5da92
PS1=$(whoami) $(hostname) $]

(Space at the end is lost)

Luap99 commented 1 year ago

Looking at the code this seems expected. The code assume a Dockerfile like syntax. INSTRUCTION followed by a spcae then the arguments. It only falls back to the equal sign when it did not find any space. https://github.com/containers/common/blob/9f28ae53c858dd54ab96585666090e1d6210dbb7/libimage/image_config.go#L39-L51

The code also always trims space so that part is expected as well. The podman import and podman commit man page do not really explain the format. They only show some examples so I understand the confusion.

In any case I do not see a good way to change the code. I would say this should just be documented better.

cevich commented 1 year ago

I would say this should just be documented better.

I would be okay with that. The confusing parsing caught at least one user off-guard - meaning, there are probably 5-more users that didn't report the problem.

The first place Nalin and I turned to look (the man page) was (as you've seen) not very helpful. So a docs-update may very well be good'nuff of a fix.

I only hesitate shortly, before to suggesting the docs should focus on using a space-separator and never = - for consistency.