diff mbox series

fs/oci: entrypoint and command are space-separated lists

Message ID 11033_1652186063_627A5BCF_11033_340_1_f3b8c61ce2f116c06c455a1f0163db9ca541896a.1652186060.git.yann.morin@orange.com
State Superseded
Headers show
Series fs/oci: entrypoint and command are space-separated lists | expand

Commit Message

Yann E. MORIN May 10, 2022, 12:34 p.m. UTC 
From: "Yann E. MORIN" <yann.morin@orange.com>

The prompt and variable name for the OCI "entrypoint arguments" are
somewhat incorrect. Indeed, they are in fact use to set the image
"command". Yet, using "command" would be confusing too, because the
interplay between entrypoint and command is tricky [0].

TL-DR; when both entrrypoint and command are set, command acts as
arguments passed to the entrypoint.

Additionally, we currently can only pass a single item as either
entrypoint or command. This precludes passing actual arguments to the
entrypoint, or passing multiple arguments as command.

For example:
    BR2_TARGET_ROOTFS_OCI_ENTRYPOINT="/bin/tini -g -p SIGTERM --"
    BR2_TARGET_ROOTFS_OCI_ENTRYPOINT_ARGS="/usr/bin/env sh"

generates an images with (only relevant fileds are included below):

    {
        "config": {
            "Entrypoint": [ "/bin/tini -g -p SIGTERM --" ],
            "Cmd": [ "/usr/bin/env sh" ]
        }
    }

This is obviously incorrect, and not what one would expect:

    {
        "config": {
            "Entrypoint": [ "/bin/tini", "-g", "-p", "SIGTERM --" ],
            "Cmd": [ "/usr/bin/env", "sh" ]
        }
    }

Fix that by passing as-many --entrypoint or --cmd as there are
space-separated items in the corresponding lists.

[0] https://docs.docker.com/engine/reference/builder/#understand-how-cmd-and-entrypoint-interact

Signed-off-by: Yann E. MORIN <yann.morin@orange.com>
Cc: Sergio Prado <sergio.prado@e-labworks.com>
Cc: Matthew Weber <matthew.weber@collins.com>
---
 fs/oci/oci.mk | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Peter Korsgaard May 12, 2022, 3:34 p.m. UTC | #1 
>>>>>   <yann.morin@orange.com> writes:

 > From: "Yann E. MORIN" <yann.morin@orange.com>
 > The prompt and variable name for the OCI "entrypoint arguments" are
 > somewhat incorrect. Indeed, they are in fact use to set the image
 > "command". Yet, using "command" would be confusing too, because the
 > interplay between entrypoint and command is tricky [0].

 > TL-DR; when both entrrypoint and command are set, command acts as

s/entrrypoint/entrypoint/

> arguments passed to the entrypoint.

 > Additionally, we currently can only pass a single item as either
 > entrypoint or command. This precludes passing actual arguments to the
 > entrypoint, or passing multiple arguments as command.

 > For example:
 >     BR2_TARGET_ROOTFS_OCI_ENTRYPOINT="/bin/tini -g -p SIGTERM --"
 >     BR2_TARGET_ROOTFS_OCI_ENTRYPOINT_ARGS="/usr/bin/env sh"

 > generates an images with (only relevant fileds are included below):

s/fileds/fields/

 >     {
 >         "config": {
 >             "Entrypoint": [ "/bin/tini -g -p SIGTERM --" ],
 >             "Cmd": [ "/usr/bin/env sh" ]
 >         }
 >     }

 > This is obviously incorrect, and not what one would expect:

 >     {
 >         "config": {
 >             "Entrypoint": [ "/bin/tini", "-g", "-p", "SIGTERM --" ],
 >             "Cmd": [ "/usr/bin/env", "sh" ]
 >         }
 >     }

 > Fix that by passing as-many --entrypoint or --cmd as there are
 > space-separated items in the corresponding lists.

 > [0] https://docs.docker.com/engine/reference/builder/#understand-how-cmd-and-entrypoint-interact

 > Signed-off-by: Yann E. MORIN <yann.morin@orange.com>
 > Cc: Sergio Prado <sergio.prado@e-labworks.com>
 > Cc: Matthew Weber <matthew.weber@collins.com>
 > ---
 >  fs/oci/oci.mk | 4 ++--
 >  1 file changed, 2 insertions(+), 2 deletions(-)

 > diff --git a/fs/oci/oci.mk b/fs/oci/oci.mk
 > index aa81920d36..f98db0a8c8 100644
 > --- a/fs/oci/oci.mk
 > +++ b/fs/oci/oci.mk
 > @@ -15,13 +15,13 @@ OCI_SLOCI_IMAGE_OPTS += $(and $(GO_GOARM),--arch-variant v$(GO_GOARM))
 >  # entrypoint
 >  OCI_ENTRYPOINT = $(call qstrip,$(BR2_TARGET_ROOTFS_OCI_ENTRYPOINT))
 >  ifneq ($(OCI_ENTRYPOINT),)
 > -OCI_SLOCI_IMAGE_OPTS += --entrypoint "$(OCI_ENTRYPOINT)"
 > +OCI_SLOCI_IMAGE_OPTS += $(patsubst %,--entrypoint %,$(OCI_ENTRYPOINT))
 >  endif
 
 >  # entrypoint arguments
 >  OCI_ENTRYPOINT_ARGS = $(call qstrip,$(BR2_TARGET_ROOTFS_OCI_ENTRYPOINT_ARGS))
 >  ifneq ($(OCI_ENTRYPOINT_ARGS),)
 > -OCI_SLOCI_IMAGE_OPTS += --cmd "$(OCI_ENTRYPOINT_ARGS)"
 > +OCI_SLOCI_IMAGE_OPTS += $(patsubst %,--cmd %,$(OCI_ENTRYPOINT_ARGS))

It indeed makes sense to split the entrypoint on white space, but for
the args (cmd) it exchanges one set of breakage for another -
E.G. sometimes people want to run sh -c "my shell logic goes here".

I'm not sure what to do about that though. As you are not quoting the
arguments you cannot even set it to stuff without white space but ith
shell characters, E.G. ENTRYPOINT="sh -c" ARGS="date;date" as the ; gets
enterpreted in the call to sloci-image.

In general I'm not sure how much value the oci support brings over just
doing:

docker import -c '<Dockerfile command>' output/images/rootfs.tar <name>:<tag>
Arnout Vandecappelle May 12, 2022, 10:31 p.m. UTC | #2 
On 12/05/2022 17:34, Peter Korsgaard wrote:
>>>>>>    <yann.morin@orange.com> writes:
> 
>   > From: "Yann E. MORIN" <yann.morin@orange.com>
>   > The prompt and variable name for the OCI "entrypoint arguments" are
>   > somewhat incorrect. Indeed, they are in fact use to set the image
>   > "command". Yet, using "command" would be confusing too, because the
>   > interplay between entrypoint and command is tricky [0].
> 
>   > TL-DR; when both entrrypoint and command are set, command acts as
> 
> s/entrrypoint/entrypoint/
> 
>> arguments passed to the entrypoint.
> 
>   > Additionally, we currently can only pass a single item as either
>   > entrypoint or command. This precludes passing actual arguments to the
>   > entrypoint, or passing multiple arguments as command.
> 
>   > For example:
>   >     BR2_TARGET_ROOTFS_OCI_ENTRYPOINT="/bin/tini -g -p SIGTERM --"
>   >     BR2_TARGET_ROOTFS_OCI_ENTRYPOINT_ARGS="/usr/bin/env sh"
> 
>   > generates an images with (only relevant fileds are included below):
> 
> s/fileds/fields/
> 
>   >     {
>   >         "config": {
>   >             "Entrypoint": [ "/bin/tini -g -p SIGTERM --" ],
>   >             "Cmd": [ "/usr/bin/env sh" ]
>   >         }
>   >     }
> 
>   > This is obviously incorrect, and not what one would expect:
> 
>   >     {
>   >         "config": {
>   >             "Entrypoint": [ "/bin/tini", "-g", "-p", "SIGTERM --" ],
>   >             "Cmd": [ "/usr/bin/env", "sh" ]
>   >         }
>   >     }
> 
>   > Fix that by passing as-many --entrypoint or --cmd as there are
>   > space-separated items in the corresponding lists.
> 
>   > [0] https://docs.docker.com/engine/reference/builder/#understand-how-cmd-and-entrypoint-interact
> 
>   > Signed-off-by: Yann E. MORIN <yann.morin@orange.com>
>   > Cc: Sergio Prado <sergio.prado@e-labworks.com>
>   > Cc: Matthew Weber <matthew.weber@collins.com>
>   > ---
>   >  fs/oci/oci.mk | 4 ++--
>   >  1 file changed, 2 insertions(+), 2 deletions(-)
> 
>   > diff --git a/fs/oci/oci.mk b/fs/oci/oci.mk
>   > index aa81920d36..f98db0a8c8 100644
>   > --- a/fs/oci/oci.mk
>   > +++ b/fs/oci/oci.mk
>   > @@ -15,13 +15,13 @@ OCI_SLOCI_IMAGE_OPTS += $(and $(GO_GOARM),--arch-variant v$(GO_GOARM))
>   >  # entrypoint
>   >  OCI_ENTRYPOINT = $(call qstrip,$(BR2_TARGET_ROOTFS_OCI_ENTRYPOINT))
>   >  ifneq ($(OCI_ENTRYPOINT),)
>   > -OCI_SLOCI_IMAGE_OPTS += --entrypoint "$(OCI_ENTRYPOINT)"
>   > +OCI_SLOCI_IMAGE_OPTS += $(patsubst %,--entrypoint %,$(OCI_ENTRYPOINT))
>   >  endif
>   
>   >  # entrypoint arguments
>   >  OCI_ENTRYPOINT_ARGS = $(call qstrip,$(BR2_TARGET_ROOTFS_OCI_ENTRYPOINT_ARGS))
>   >  ifneq ($(OCI_ENTRYPOINT_ARGS),)
>   > -OCI_SLOCI_IMAGE_OPTS += --cmd "$(OCI_ENTRYPOINT_ARGS)"
>   > +OCI_SLOCI_IMAGE_OPTS += $(patsubst %,--cmd %,$(OCI_ENTRYPOINT_ARGS))
> 
> It indeed makes sense to split the entrypoint on white space, but for
> the args (cmd) it exchanges one set of breakage for another -
> E.G. sometimes people want to run sh -c "my shell logic goes here".
> 
> I'm not sure what to do about that though. As you are not quoting the
> arguments you cannot even set it to stuff without white space but ith
> shell characters, E.G. ENTRYPOINT="sh -c" ARGS="date;date" as the ; gets
> enterpreted in the call to sloci-image.

  Do you mean

BR2_TARGET_ROOTFS_OCI_ENTRYPOINT="sh -c"
BR2_TARGET_ROOTFS_OCI_ENTRYPOINT_ARGS="'date; date'"

? Yeah, we'd need a full quoting/splitting parse to support that. In this 
particular case

BR2_TARGET_ROOTFS_OCI_ENTRYPOINT_ARGS="date\;date"

would work, but it's kludgy.

  I do think it would help to put quotes around the options:

OCI_SLOCI_IMAGE_OPTS += $(patsubst %,--cmd '%',$(OCI_ENTRYPOINT_ARGS))

  That way, the usual cases (which Yann put in the commit message) are covered, 
but weird stuff is still possible as long as you pay attention to spaces.

  If you really need spaces, you can still use $(space). So, after adding single 
quotes, the following should work:

BR2_TARGET_ROOTFS_OCI_ENTRYPOINT_ARGS="date;$(space)date"


> In general I'm not sure how much value the oci support brings over just
> doing:
> 
> docker import -c '<Dockerfile command>' output/images/rootfs.tar <name>:<tag>

  That requires a docker daemon installed on the host, while sloci-image can be 
run as normal user without any host support required.

  Regards,
  Arnout
Peter Korsgaard May 13, 2022, 6:06 a.m. UTC | #3 
>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:

Hi,

 >> It indeed makes sense to split the entrypoint on white space, but
 >> for
 >> the args (cmd) it exchanges one set of breakage for another -
 >> E.G. sometimes people want to run sh -c "my shell logic goes here".
 >> I'm not sure what to do about that though. As you are not quoting
 >> the
 >> arguments you cannot even set it to stuff without white space but ith
 >> shell characters, E.G. ENTRYPOINT="sh -c" ARGS="date;date" as the ; gets
 >> enterpreted in the call to sloci-image.

 >  Do you mean

 > BR2_TARGET_ROOTFS_OCI_ENTRYPOINT="sh -c"
 > BR2_TARGET_ROOTFS_OCI_ENTRYPOINT_ARGS="'date; date'"

Yes, exactly.


 > ? Yeah, we'd need a full quoting/splitting parse to support that. In
 > this particular case

 > BR2_TARGET_ROOTFS_OCI_ENTRYPOINT_ARGS="date\;date"

 > would work, but it's kludgy.

 >  I do think it would help to put quotes around the options:

 > OCI_SLOCI_IMAGE_OPTS += $(patsubst %,--cmd '%',$(OCI_ENTRYPOINT_ARGS))

Yes, agreed. That atleast fixes the special shell characters (except for
' itself).


 >  That way, the usual cases (which Yann put in the commit message) are
 >  covered, but weird stuff is still possible as long as you pay
 > attention to spaces.

 >  If you really need spaces, you can still use $(space). So, after
 >  adding single quotes, the following should work:

 > BR2_TARGET_ROOTFS_OCI_ENTRYPOINT_ARGS="date;$(space)date"

Nice and easy for newcomers :P


 >> In general I'm not sure how much value the oci support brings over just
 >> doing:
 >> docker import -c '<Dockerfile command>' output/images/rootfs.tar
 >> <name>:<tag>

 >  That requires a docker daemon installed on the host, while
 >  sloci-image can be run as normal user without any host support
 > required.

s/docker import/podman import/. With that said, wanting to build an OCI
image and not having access docker/podman is quite unlikely I think.
Arnout Vandecappelle May 13, 2022, 7:42 p.m. UTC | #4 
On 13/05/2022 08:06, Peter Korsgaard wrote:
>>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:
[snip]
>   >  That way, the usual cases (which Yann put in the commit message) are
>   >  covered, but weird stuff is still possible as long as you pay
>   > attention to spaces.
> 
>   >  If you really need spaces, you can still use $(space). So, after
>   >  adding single quotes, the following should work:
> 
>   > BR2_TARGET_ROOTFS_OCI_ENTRYPOINT_ARGS="date;$(space)date"
> 
> Nice and easy for newcomers :P

  Sure, but do you have an actual use case where you want spaces in the CMD or 
ENTRYPOINT? Note that even in the Dockerfile itself you need to switch to JSON 
syntax to support that case. I think it's exceedingly rare.

  If you really have such complicated cases, you're probably better off doing it 
with a post-image script that imports the tarball after all. But the simple case 
that covers 95% of the use cases can be covered by fs/oci.

>   >> In general I'm not sure how much value the oci support brings over just
>   >> doing:
>   >> docker import -c '<Dockerfile command>' output/images/rootfs.tar
>   >> <name>:<tag>
> 
>   >  That requires a docker daemon installed on the host, while
>   >  sloci-image can be run as normal user without any host support
>   > required.
> 
> s/docker import/podman import/.

  So it requires a podman daemon installed on the host... podman can do rootless 
containers, but it's still mediated by the daemon.

> With that said, wanting to build an OCI
> image and not having access docker/podman is quite unlikely I think.

  As explained on IRC: it is very likely that your CI does builds in a docker 
container. You normally *don't* do it in a DinD or socket-mounted container. So 
CI normally doesn't have access to docker.

  Regards,
  Arnout
Peter Korsgaard May 13, 2022, 8:54 p.m. UTC | #5 
>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:

Hi,

 >> >  If you really need spaces, you can still use $(space). So,
 >> after
 >> >  adding single quotes, the following should work:
 >> > BR2_TARGET_ROOTFS_OCI_ENTRYPOINT_ARGS="date;$(space)date"
 >> Nice and easy for newcomers :P

 >  Sure, but do you have an actual use case where you want spaces in the
 >  CMD or ENTRYPOINT? Note that even in the Dockerfile itself you need
 > to switch to JSON syntax to support that case. I think it's
 > exceedingly rare.

Sorry, what do you mean? If I have a silly container like:

FROM busybox

CMD for i in 1 2 3; do date; done

Then that works fine (as CMD expands to sh -c "<args>" if you don't use
the array format).

docker inspect <container> | jq '.[] | .Config.Cmd'
[
  "/bin/sh",
  "-c",
  "for i in 1 2 3; do date; done"
]


I don't think I have ever seen embedded spaces in ENTRYPOINT (but I have
seen multiple arguments), but in CMD I have.


 >  If you really have such complicated cases, you're probably better off
 >  doing it with a post-image script that imports the tarball after
 > all. But the simple case that covers 95% of the use cases can be
 > covered by fs/oci.

I would argue that just doing docker (or podman) import in the
post-image script is easier than fs/oci as people doing container stuff
are likely to already know docker and Dockerfiles, but oh well.


 >> >  That requires a docker daemon installed on the host, while
 >> >  sloci-image can be run as normal user without any host support
 >> > required.
 >> s/docker import/podman import/.

 >  So it requires a podman daemon installed on the host... podman can do
 >  rootless containers, but it's still mediated by the daemon.

No, podman is daemon-less.
diff mbox series

Patch

diff --git a/fs/oci/oci.mk b/fs/oci/oci.mk
index aa81920d36..f98db0a8c8 100644
--- a/fs/oci/oci.mk
+++ b/fs/oci/oci.mk
@@ -15,13 +15,13 @@  OCI_SLOCI_IMAGE_OPTS += $(and $(GO_GOARM),--arch-variant v$(GO_GOARM))
 # entrypoint
 OCI_ENTRYPOINT = $(call qstrip,$(BR2_TARGET_ROOTFS_OCI_ENTRYPOINT))
 ifneq ($(OCI_ENTRYPOINT),)
-OCI_SLOCI_IMAGE_OPTS += --entrypoint "$(OCI_ENTRYPOINT)"
+OCI_SLOCI_IMAGE_OPTS += $(patsubst %,--entrypoint %,$(OCI_ENTRYPOINT))
 endif
 
 # entrypoint arguments
 OCI_ENTRYPOINT_ARGS = $(call qstrip,$(BR2_TARGET_ROOTFS_OCI_ENTRYPOINT_ARGS))
 ifneq ($(OCI_ENTRYPOINT_ARGS),)
-OCI_SLOCI_IMAGE_OPTS += --cmd "$(OCI_ENTRYPOINT_ARGS)"
+OCI_SLOCI_IMAGE_OPTS += $(patsubst %,--cmd %,$(OCI_ENTRYPOINT_ARGS))
 endif
 
 # author