Handle Entrypoint and Cmd correctly #20
Conversation
iaguis
force-pushed
the
iaguis/entrypoint-cmd
branch
from
36e7480
to
7b38306
Compare
| return nil | ||
| } | ||
| command = append(entrypoint, cmd...) | ||
| // non-absolute paths are not allowed, fallback to "/bin/sh -c command" |
There was a problem hiding this comment.
Starting to feel like we should have a means to provide warnings during the conversion process when things like this happen, thoughts? (In the real world this would probably very rarely cause problems since having /bin/sh in Docker images is a really common pattern, but would be good practise to give people some hint that we are doing this kind of translation and the produced image is potentially non-functional)
There was a problem hiding this comment.
We could warn with something like Exec is not an absolute path, using "/bin/sh -c EXEC". The resulting ACI could be non-functional.
This will be printed for each layer that we convert so it might be too verbose, especially if we're squashing. We could accumulate the warnings and print only one of each at the end. Something like:
Warnings found when converting the image. The resulting ACI could be non-fuctional:
Exec is not an absolute path, using "/bin/sh -c EXEC"
Docker User is empty, assuming root
There was a problem hiding this comment.
Yeah, printing is less than ideal since this is a library. Maybe we could allow callers to pass in something to record warnings. Not sure.
There was a problem hiding this comment.
Let's follow up with that later, filed #24
iaguis
force-pushed
the
iaguis/entrypoint-cmd
branch
from
7b38306
to
1c8d969
Compare
| // this can't fail because the old name is legal | ||
| nameWithoutLayerID, _ := types.NewACName(strings.Split(manifest.Name.String(), "-")[0]) |
There was a problem hiding this comment.
huh, why was this necessary?
There was a problem hiding this comment.
Every layer's name has the docker layerID at the end, this strips it from the squashed image name. It was wrong because the image name can have dashes.
There was a problem hiding this comment.
I'll put it in another PR
Use Entrypoint + Cmd when both are present or just Cmd when Entrypoint is not present. Since non-absolute paths are not allowed by the spec, if the resulting command is not absolute we fall back to running it with "/bin/sh -c".
iaguis
force-pushed
the
iaguis/entrypoint-cmd
branch
from
1c8d969
to
030537c
Compare
|
Since we have #24 now, I'm merging this. |
Handle Entrypoint and Cmd correctly
Use Entrypoint + Cmd when both are present or just Cmd when Entrypoint
is not present.
Since non-absolute paths are not allowed by the spec, if the resulting
command is not absolute we fall back to running it with "/bin/sh -c".
Fixes #14