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.

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

HERE-DOC syntax issue when a line contains a dockerfile directive in CMD #5240

Closed
EatPrilosec opened this issue Aug 11, 2024 · 18 comments
Closed

Comments

@EatPrilosec
Copy link

EatPrilosec commented Aug 11, 2024

Description

when using HERE-DOC syntax for RUN CMD in a dockerfile, while attemting to execute the env linux command, it gets interpreted as ENV for the dockerfile, and fails to build.

Reproduce

  1. create a dockerfile with HERE-DOC syntax for one of its directives that support it CMD

  2. make one of the lines in the here-doc "env" or "run" or something that is also a dockerfile directive

  3. attempt to build

Expected behavior

docker should interpret the HERE-DOC correctly

docker version

Client: Docker Engine - Community
 Version:           25.0.4
 API version:       1.44
 Go version:        go1.21.8
 Git commit:        1a576c5
 Built:             Wed Mar  6 16:32:14 2024
 OS/Arch:           linux/amd64
 Context:           default

Server: Docker Engine - Community
 Engine:
  Version:          25.0.4
  API version:      1.44 (minimum version 1.24)
  Go version:       go1.21.8
  Git commit:       061aa95
  Built:            Wed Mar  6 16:32:14 2024
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          1.6.28
  GitCommit:        ae07eda36dd25f8a1b98dfbf587313b99c0190bb
 runc:
  Version:          1.1.12
  GitCommit:        v1.1.12-0-g51d5e94
 docker-init:
  Version:          0.19.0
  GitCommit:        de40ad0

docker info

Client: Docker Engine - Community
 Version:    25.0.4
 Context:    default
 Debug Mode: false
 Plugins:
  buildx: Docker Buildx (Docker Inc.)
    Version:  v0.13.0
    Path:     /usr/libexec/docker/cli-plugins/docker-buildx
  compose: Docker Compose (Docker Inc.)
    Version:  v2.24.7
    Path:     /usr/libexec/docker/cli-plugins/docker-compose

Server:
 Containers: 37
  Running: 37
  Paused: 0
  Stopped: 0
 Images: 36
 Server Version: 25.0.4
 Storage Driver: overlay2
  Backing Filesystem: extfs
  Supports d_type: true
  Using metacopy: false
  Native Overlay Diff: true
  userxattr: false
 Logging Driver: json-file
 Cgroup Driver: systemd
 Cgroup Version: 2
 Plugins:
  Volume: local
  Network: bridge host ipvlan macvlan null overlay
  Log: awslogs fluentd gcplogs gelf journald json-file local splunk syslog
 Swarm: inactive
 Runtimes: io.containerd.runc.v2 runc
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: ae07eda36dd25f8a1b98dfbf587313b99c0190bb
 runc version: v1.1.12-0-g51d5e94
 init version: de40ad0
 Security Options:
  apparmor
  seccomp
   Profile: builtin
  cgroupns
 Kernel Version: 6.5.13-1-pve
 Operating System: Debian GNU/Linux 12 (bookworm)
 OSType: linux
 Architecture: x86_64
 CPUs: 4
 Total Memory: 15.39GiB
 Name: pve
 ID: 12ac2ca8-b5bc-4a22-93ac-da1a1ece74eb
 Docker Root Dir: /var/lib/docker
 Debug Mode: false
 Experimental: false
 Insecure Registries:
  127.0.0.0/8
 Live Restore Enabled: false

WARNING: bridge-nf-call-iptables is disabled
WARNING: bridge-nf-call-ip6tables is disabled

Additional Info

No response

@thaJeztah
Copy link
Member

thaJeztah commented Aug 12, 2024

Thanks for reporting; looks indeed that som other parsing picks this up;

docker build --progress=plain --no-cache -<<'EOF'
# syntax=docker/dockerfile:1

FROM alpine
RUN -<<'EOT'
env
EOT
EOF


moby/moby#1 [internal] load build definition from Dockerfile
moby/moby#1 transferring dockerfile: 100B done
moby/moby#1 DONE 0.0s

moby/moby#2 resolve image config for docker-image://docker.io/docker/dockerfile:1
moby/moby#2 DONE 0.5s

moby/moby#3 docker-image://docker.io/docker/dockerfile:1@sha256:fe40cf4e92cd0c467be2cfc30657a680ae2398318afd50b0c80585784c604f28
moby/moby#3 resolve docker.io/docker/dockerfile:1@sha256:fe40cf4e92cd0c467be2cfc30657a680ae2398318afd50b0c80585784c604f28 0.0s done
moby/moby#3 CACHED

 1 warning found (use docker --debug to expand):
 - ConsistentInstructionCasing: Command 'env' should match the case of the command majority (uppercase) (line 5)
Dockerfile:5
--------------------
   3 |     FROM alpine
   4 |     RUN -<<'EOT'
   5 | >>> env
   6 |     EOT
   7 |
--------------------
ERROR: failed to solve: dockerfile parse error on line 5: ENV requires at least one argument

Let me more this ticket to the BuildKit repository, which is where the code is maintained for parsing the Dockerfile.

@thaJeztah thaJeztah transferred this issue from moby/moby Aug 12, 2024
@tonistiigi
Copy link
Member

@daghack Looks like consistent-casing check just walks all top-level AST https://github.com/moby/buildkit/blob/v0.15.1/frontend/dockerfile/dockerfile2llb/convert.go#L2214-L2228 . It needs to be smarter to detect the nodes that were part of the heredoc of the previous command. Probably this check should happen after instructions.Parse where all the actual commands have been parsed out.

@daghack
Copy link
Collaborator

daghack commented Aug 12, 2024

@tonistiigi PR PR that address your comment. To clarify, it does not fix this issue, which has to do with the actual parsing, rather than the checks around the parsing, but does address the issue of the check operating at the AST level, which is less than ideal.

@tonistiigi
Copy link
Member

tonistiigi commented Aug 19, 2024

I'm confused now. What is -<<EOT supposed to mean? The syntax for skipping tabs is <<-EOT. If I try that, then all seems to work fine for me, and there is no example case provided from the issue author.

@thaJeztah
Copy link
Member

I'm confused now. What is -<<EOT supposed to mean?

Ugh. That's my mistake 🙈 I must've added the - out of habit from using heredocs to pass a Dockerfile to stdin as I did for passing it in the example 🙈

@daghack
Copy link
Collaborator

daghack commented Aug 19, 2024

@EatPrilosec Are you able to give us an example Dockerfile, whether your original case or a simple reproduction, for this issue?

@EatPrilosec
Copy link
Author

EatPrilosec commented Aug 19, 2024

@EatPrilosec Are you able to give us an example Dockerfile, whether your original case or a simple reproduction, for this issue?

https://github.com/EatPrilosec/epg-cron-docker/blob/1d1d6ad2f19308a8ae5fc1157e06078784cc7141/Dockerfile#L88

here is an old commit where i was first experiencing the issue.

if you fork, there should be a github workflow action to build the docker, just mentioning for convenience

edit: in fact, heres the workflow run for that commit :)

https://github.com/EatPrilosec/epg-cron-docker/actions/runs/10342066164/job/28624575134

(PS, the docker version and environment that i provided was for my pc, not githubs runner)

@thaJeztah
Copy link
Member

I notice the HereDoc end marker is not at the start of the line (has leading white space); wondering if that's related here; https://github.com/EatPrilosec/epg-cron-docker/blob/1d1d6ad2f19308a8ae5fc1157e06078784cc7141/Dockerfile#L105

@EatPrilosec
Copy link
Author

I notice the HereDoc end marker is not at the start of the line (has leading white space); wondering if that's related here; https://github.com/EatPrilosec/epg-cron-docker/blob/1d1d6ad2f19308a8ae5fc1157e06078784cc7141/Dockerfile#L105

i believe i checked this, let me look through the commits

@EatPrilosec
Copy link
Author

EatPrilosec commented Aug 19, 2024

yea, here i used EOF for troubleshooting
EatPrilosec/epg-cron-docker@c29d999
https://github.com/EatPrilosec/epg-cron-docker/actions/runs/10342078882

and here was before i even added indents -- i thought the LACK of indents was the issue at the time
https://github.com/EatPrilosec/epg-cron-docker/blob/a8df2e4345553f5313f8968f998b727c0d8c7317/Dockerfile#L105
https://github.com/EatPrilosec/epg-cron-docker/actions/runs/10342038853

PS sorry for not providing these earlier, slipped my mind at the time. and thanks for taking a look!

@EatPrilosec
Copy link
Author

Hmmm. These examples are definitely building correctly for me locally. @tonistiigi Any thoughts?

Wait, really? Now that suprising because it failed for the GitHub runner AND locally (on my PC) for me.

you're telling me that the docker file(s) I provided, build? And just to ask the dumb question, you cloned one of the commits I provided and not the current right? Current doesn't have the heredoc.

Assuming you didn't clone head, my gut says line endings, just to throw my two cents in there. But they should all have been LF not CRLF

@thaJeztah
Copy link
Member

I don't think it's the syntax / formatting there; I noticed the Dockerfile example uses heredoc for CMD, not RUN, and I don't think that supported;

So in that case it will likely see the env line as an ENV command, and possibly trigger the linter warning before that (at least before #5243).

Not sure why it errors on the env, and doesn't error on any of the other lines before that though (i.e., why doesn't it fail on echo or usermod being unknown Dockerfile commands?)

@daghack
Copy link
Collaborator

daghack commented Aug 20, 2024

Wait, really? Now that suprising because it failed for the GitHub runner AND locally (on my PC) for me.

Reviewed what I was testing, and I was absolutely incorrect there, good callout! At some point in the testing, I had replaced the CMD with an RUN. So while it built, it was not the same example. Whoops! 😅 That being said, it does confirm what @thaJeztah is saying. The easy answer is "heredoc's with CMD are not supported", but there's definitely something incongruous with how the behavior is presenting that should be fixed.

Digging into this to find out what's happening and how we can make it better/easier to understand what's failing in the future!

@EatPrilosec
Copy link
Author

EatPrilosec commented Aug 20, 2024

now that makes more sense.

and now that the distinction has been pointed out, i think what happened is, while trying to format my complex CMD line correctly, i went to the docs
https://docs.docker.com/reference/dockerfile/#cmd

the second sentence: " You can specify CMD instructions using shell or exec forms"

which then led me to heredocs in shell form documentation. https://docs.docker.com/reference/dockerfile/#shell-form

this led me to misinterpret that RUN and CMD were both heredoc compatible already.

however i'd vote for here-doc support in CMD, its useful.

PS: sorry for the confusion, ive updated the github issue to reflect the true problem

@EatPrilosec EatPrilosec changed the title HERE-DOC syntax issue when a line contains a dockerfile directive HERE-DOC syntax issue when a line contains a dockerfile directive in CMD Aug 20, 2024
@daghack
Copy link
Collaborator

daghack commented Aug 20, 2024

Not sure why it errors on the env, and doesn't error on any of the other lines before that though (i.e., why doesn't it fail on echo or usermod being unknown Dockerfile commands?)

It looks like what's happening is that because CMD is not a heredoc-supporting command, the would-be heredoc contents get treated as regular lines when we are assembling the AST. At this stage, the behavior for unknown commands is to emit empty nodes. So when it sees echo or usermod, it knows to just ignore it. env on the other hand IS a dockerfile command, so it is getting more scrutiny at this stage.

Ultimately, it seems like the behavior we might want is that if a user uses heredoc syntax with an unsupported command, we produce an appropriate error.

and now that the distinction has been pointed out, i think what happened is, while trying to format my complex CMD line correctly, i went to the docs
https://docs.docker.com/reference/dockerfile/#cmd

Ah! Well, that's definitely an issue!

@EatPrilosec
Copy link
Author

EatPrilosec commented Aug 20, 2024

At this stage, the behavior for unknown commands is to emit empty nodes. So when it sees echo or usermod, it knows to just ignore it.

this might also explain while double checking just now, on my local linux server which appears to have an older docker engine, it errors out on echo, but in the github runner, it errors out on env

heredoc syntax with an unsupported command, we produce an appropriate error.

id also like to, as a user, take this opportunity to stress my vote for supporting heredocs in CMD at some point. my current solution for a complex CMD line-- other than a start script which adds the necessity for extra files and a COPY directive-- is an extremely messy shell form line with lots of escaping. the heredoc would be more elegant and allow for things like PUID/PGID without a start script.

@daghack
Copy link
Collaborator

daghack commented Aug 29, 2024

heredoc syntax with an unsupported command, we produce an appropriate error.

id also like to, as a user, take this opportunity to stress my vote for supporting heredocs in CMD at some point. my current solution for a complex CMD line-- other than a start script which adds the necessity for extra files and a COPY directive-- is an extremely messy shell form line with lots of escaping. the heredoc would be more elegant and allow for things like PUID/PGID without a start script.

Seems like a reasonable position — it does seem pretty sensible to have this syntax on CMD, and it looks like there's some discussion that happened around that feature here.

That being said, I'm going to close this particular issue, since we've identified the issue and the answer is "Not supported", but I encourage you to open an issue relating more specifically to heredocs not being supported with the CMD command if you're interested in opening discussion around that. :)

@daghack daghack closed this as completed Aug 29, 2024
@jedevc
Copy link
Member

jedevc commented Aug 29, 2024

but I encourage you to open an issue relating more specifically to heredocs not being supported with the CMD command if you're interested in opening discussion around that. :)

#2170 🎉 never got round to doing it, mostly due to lack of demand 😢

@tonistiigi tonistiigi removed this from the v0.17.0 milestone Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants