remote controller: Fix entrypoint interaction bugs

[ktock]

Following-up: #1870

This commit tries to fix the same issue as #1870 for remote controller mode as well. For remote controller, the root cause of this issue is that buildx can't distinguish InvokeConfig.Cmd being unset(nil) and zero-length([]string{}) over protobuf.

This commit fixes this by introducing a new field InvokeConfig.NoCmd (bool) for explicitly separating the above cases.

  bool NoCmd = 11; // Do not set cmd but use the image's default

In the future, we can upgrade protoc to the recent version and use optional fields but I'm not sure we can safely do this now because proto files in buildx heavily import buildkit's assets.

Testing with Dockerifle

FROM busybox
RUN <<EOF
cat <<'EOT' > /entrypoint.sh
#/bin/sh
$@
EOT
chmod 0775 /entrypoint.sh
EOF
ENTRYPOINT ["sh", "/entrypoint.sh"]
CMD ["sh"]

On master version, --invoke entrypoint=sh and exec sh return the following because it tries to run sh sh and this PR fixes this error.

$ BUILDX_EXPERIMENTAL=1 /tmp/out/buildx build --detach=true --invoke entrypoint=sh /tmp/ctx

...

sh: can't open 'sh': No such file or directory

[jedevc]

Just wondering, would it be possible to just change this to len(cfg.Cmd) != 0 for a simpler fix?

Can a user actually specify an empty cmd? I'm not sure if this has a parallel with docker run (cc @tianon)

[tianon]

This logic was hard for me to reason about, so I'm going to give some examples of docker run behavior in the hopes that it helps make things more clear. 😅

Given an image with ENTRYPOINT set to a and CMD set to b (keeping these intentionally simple and including the obvious ones too -- let's not get into shell form vs JSON form because that gets nasty and doesn't really change this basic operation very much 😂):

• docker run img -> runs a b
• docker run img c -> runs a c
• docker run --entrypoint c img -> runs c (no a, but more importantly no b also!)
• docker run --entrypoint c img d -> c d

In other words, the behavior is similar to that of ENTRYPOINT within a Dockerfile - if it's specified, it resets CMD (so we have to detect command from the CLI separate from command from the image).

[ktock]

Thanks for the explanation! Based on the examples, len(cfg.Cmd) != 0 doesn't look like a solution here because it doesn't implement the behaviour of docker run --entrypoint c img (--invoke entrypoint=c on buildx) that specifies an empty cmd and needs to ignore the image's default.

[jedevc]

Can we put this after the Cmd field? We can keep the number the same, but would be good to group them together.

[ktock]

CI failure doesn't seem to be related to this PR ? 🤔

https://github.com/docker/buildx/actions/runs/5675011549/job/15379622203?pr=1970#step:6:60

 === FAIL: tests TestIntegration (8.19s)
time="2023-07-27T00:25:38Z" level=info msg="trying next host - response was http.StatusNotFound" host="localhost:40429"
    run.go:266: copied docker.io/amd64/busybox:latest@sha256:0d5a701f0ca53f38723108687add000e1922f812d4187dea7feaee85d2f5a6c5 to local mirror localhost:40429/library/busybox:latest
time="2023-07-27T00:25:38Z" level=info msg="trying next host - response was http.StatusNotFound" host="localhost:40429"
    run.go:266: copied docker.io/amd64/alpine:latest@sha256:c0d488a800e4127c334ad20d61d7bc21b4097540327217dfab52262adc02380c to local mirror localhost:40429/library/alpine:latest
time="2023-07-27T00:25:38Z" level=info msg="trying next host - response was http.StatusNotFound" host="localhost:40429"
    run.go:161: 
        	Error Trace:	/src/vendor/github.com/moby/buildkit/util/testutil/integration/run.go:161
        	            				/src/tests/integration_test.go:45
        	            				/src/tests/integration_test.go:27
        	Error:      	Received unexpected error:
        	            	failed to copy: stream error: stream ID 81; INTERNAL_ERROR; received from peer
        	            	github.com/moby/buildkit/util/contentutil.CopyChain
        	            		/src/vendor/github.com/moby/buildkit/util/contentutil/copy.go:87
        	            	github.com/moby/buildkit/util/testutil/integration.copyImagesLocal
        	            		/src/vendor/github.com/moby/buildkit/util/testutil/integration/run.go:263
        	            	github.com/moby/buildkit/util/testutil/integration.runMirror
        	            		/src/vendor/github.com/moby/buildkit/util/testutil/integration/run.go:354
        	            	github.com/moby/buildkit/util/testutil/integration.Run
        	            		/src/vendor/github.com/moby/buildkit/util/testutil/integration/run.go:160
        	            	github.com/docker/buildx/tests.testIntegration
        	            		/src/tests/integration_test.go:45
        	            	github.com/docker/buildx/tests.TestIntegration
        	            		/src/tests/integration_test.go:27
        	            	testing.tRunner
        	            		/usr/local/go/src/testing/testing.go:1576
        	            	runtime.goexit
        	            		/usr/local/go/src/runtime/asm_amd64.s:1598
        	Test:       	TestIntegration