search

slimtoolkit / slim

Slim(toolkit): Don't change anything in your container image and minify it by up to 30x (and for compiled languages even more) making it secure too! (free and open source)
Apache License 2.0
18.97k stars 706 forks source link

Slim doesn't properly parse image history with HEALTHCHECK if the layer was inserted by buildah bud #509

Closed crcady closed 7 months ago

crcady commented 1 year ago

Expected Behavior

When processing an image built with buildah bud, slim should not crash.

Actual Behavior

slim panics when attempting to process a HEALTHCHECK inserted into the image history using buildah bud.

Steps to Reproduce the Problem

  1. Have slim and buildah installed

  2. Create a Dockerfile that has a Healthcheck command

    # cat Dockerfile
FROM docker.io/nginx:latest
HEALTHCHECK --interval=5m --timeout=3s \
CMD curl -f http://localhost/ || exit 1

  3. Build the image with buildah bud

    buildah bud -t nginx-healthcheck-bud .

    The output should be something like the below (I stripper the layers out for ease of reading)

    STEP 1/2: FROM docker.io/nginx:latest
STEP 2/2: HEALTHCHECK --interval=5m --timeout=3s    CMD curl -f http://localhost/ || exit 1
COMMIT nginx-healthcheck-bud
WARN[0000] Healthcheck is not supported for OCI image format and will be ignored. Must use `docker` format
Getting image source signatures
...
Writing manifest to image destination
Storing signatures
--> 1a8a069331c
Successfully tagged localhost/nginx-healthcheck-bud:latest
1a8a069331c7df36e4d71659f8bc8afe4520f23e3ef88037498f799c5483a680

  4. Push the image to somewhere that slim can access it

    buildah push localhost/nginx-healthcheck-bud docker-daemon:nginx-healthcheck-bud:latest

    You should see buildah walk through the layers and then push into the registry:

    Getting image source signatures
Copying blob ... done [snipped]
Writing manifest to image destination
Storing signatures

  5. Run a slim command that processes the image history (xray, build, etc.)

    slim xray nginx-healthcheck-bud

    results in the following output:

    
app='slim' message='Join the Gitter channel to ask questions or to share your feedback' info='https://gitter.im/docker-slim/community'
app='slim' message='Join the Discord server to ask questions or to share your feedback' info='https://discord.gg/9tDyxYS'
app='slim' message='GitHub Discussions' info='https://github.com/slimtoolkit/slim/discussions'
cmd=xray state=started
cmd=xray info=params add-image-config='false' rm-file-artifacts='false' target='nginx-healthcheck-bud' add-image-manifest='false'
cmd=xray state=image.api.inspection.start
cmd=xray info=image id='sha256:5f8467158bb77c8fa0b38998f8bdabd1f9146657327667bf071e3b143a2e93f3' size.bytes='142129665' size.human='142 MB' architecture='amd64'
app='slim' message='Join the Gitter channel to ask questions or to share your feedback' info='https://gitter.im/docker-slim/community'
app='slim' message='Join the Discord server to ask questions or to share your feedback' info='https://discord.gg/9tDyxYS'
app='slim' message='GitHub Discussions' info='https://github.com/slimtoolkit/slim/discussions'
panic: runtime error: index out of range [1] with length 1

goroutine 1 [running]: github.com/docker-slim/docker-slim/pkg/docker/dockerfile/reverse.deserialiseHealtheckInstruction({0xc0003b93c2?, 0x4f?}) github.com/docker-slim/docker-slim/pkg/docker/dockerfile/reverse/reverse.go:732 +0x1226 github.com/docker-slim/docker-slim/pkg/docker/dockerfile/reverse.DockerfileFromHistoryStruct({0xc0006a7500, 0x10, 0x13}) github.com/docker-slim/docker-slim/pkg/docker/dockerfile/reverse/reverse.go:469 +0x2b69 github.com/docker-slim/docker-slim/pkg/docker/dockerfile/reverse.DockerfileFromHistory(0xc00075de40?, {0x7fff0bf64729?, 0xc00075e688?}) github.com/docker-slim/docker-slim/pkg/docker/dockerfile/reverse/reverse.go:210 +0x3c github.com/docker-slim/docker-slim/pkg/app/master/inspectors/image.(Inspector).ProcessCollectedData(0xc00075de40) github.com/docker-slim/docker-slim/pkg/app/master/inspectors/image/imageinspector.go:306 +0x45 github.com/docker-slim/docker-slim/pkg/app/master/commands/xray.OnCommand(0xc00075f1b0, 0xc0005887e0, {0x7fff0bf64729, }, , {, }, {, _}, {0x0, ...}, ...) github.com/docker-slim/docker-slim/pkg/app/master/commands/xray/handler.go:192 +0x1625 github.com/docker-slim/docker-slim/pkg/app/master/commands/xray.glob..func1(0xc00004f080) github.com/docker-slim/docker-slim/pkg/app/master/commands/xray/cli.go:229 +0x14ca github.com/urfave/cli/v2.(Command).Run(0x2d4a7a0, 0xc00061e540) github.com/urfave/cli/v2@v2.3.0/command.go:163 +0x5dc github.com/urfave/cli/v2.(App).RunContext(0xc0005a2680, {0x1f46a50?, 0xc000040068}, {0xc00003e090, 0x3, 0x3}) github.com/urfave/cli/v2@v2.3.0/app.go:313 +0xb7d github.com/urfave/cli/v2.(App).Run(...) github.com/urfave/cli/v2@v2.3.0/app.go:224 github.com/docker-slim/docker-slim/pkg/app/master.Run() github.com/docker-slim/docker-slim/pkg/app/master/app.go:15 +0x46 main.main() github.com/docker-slim/docker-slim/cmd/slim/main.go:15 +0x18

---
Specifications
=================
  - Version: 1.40.1
```shell
slim --version
slim version linux|Transformer|1.40.1|9c5e69ab1fd4564b0a5426d47be038155e63e4c1|2023-04-05_11:22:53PM
crcady commented 1 year ago

It looks like the reason this happens is that buildah bud doesn't serialize the HEALTHCHECK instruction in this history:

docker history nginx-healthcheck-bud
IMAGE          CREATED          CREATED BY                                      SIZE      COMMENT
1a8a069331c7   11 minutes ago   /bin/sh -c #(nop) HEALTHCHECK --interval=5m …   0B        FROM docker.io/library/nginx:latest
# rest of the image history stripped for clarity

If you build the same image with docker build, you get a serialized HEALTHCHECK:

docker history nginx-healthcheck-docker
IMAGE          CREATED       CREATED BY                                      SIZE      COMMENT
8752c36a0f7a   11 days ago   HEALTHCHECK &{["CMD-SHELL" "curl -f http://l…   0B        buildkit.dockerfile.v0
# rest of the image history stripped for clarity

I think the simplest way to avoid this is to check for serialization in pkg/docker/dockerfile/reverse.go (probably here), and if it doesn't look like a serialized command just emit the instruction wholesale. If that makes sense, I'll gin up a PR and submit it.

kcq commented 1 year ago

@crcady thanks for sharing the bug! can you also add the non-truncated history for the HEALTHCHECK instruction from the buildah bud generated image?

crcady commented 1 year ago

@kcq Here's the output from a super-minimal example (this one is from scratch because the NGINX no-truncate history was murdering my terminal) - it's exactly the same example as above, except the source image in the FROM line is scratch instead of docker.io/nginx:latest. It looks like buildah bud puts the entirety of the HEALTHCHECK in there after stripping out line continuations.

docker image history --no-trunc scratch-healthcheck:latest
IMAGE                                                                     CREATED              CREATED BY                                                                                          SIZE      COMMENT
sha256:39e7ba3a2d3586e5b21a5685c364a479ad93e847ed5bffd9a8e945a3875e8ee2   About a minute ago   /bin/sh -c #(nop) HEALTHCHECK --interval=5m --timeout=3s  CMD curl -f http://localhost/ || exit 1   0B
kcq commented 1 year ago

@crcady thanks! will have a new release in a couple of days. it'll be good to test again

kcq commented 1 year ago

@crcady the problem should have been addressed in the 1.40.2 release. Now there's also 1.40.3 where it has more useful enhancements including an experimental 'merge' command. Check it out. Let me know if you have any other buildah related problems and if the new enhancements look interesting to you.