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

Improve usage of the Docker image #42

Closed

Conversation

marcofranssen
Copy link

@marcofranssen marcofranssen commented Jan 2, 2023

This PR improves the usage of the Docker image by using a WORKDIR (You can't mount a volume at /).

Also it improves the Dockerfile itself by using chainguards static image which ships automatically with

  • ca-certs
  • tzdata

FROM scratch
COPY --from=builder /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/

FROM cgr.dev/chainguard/static:latest-20230102
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not interested in using anything besides SCRATCH

Copy link
Author

@marcofranssen marcofranssen Jan 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Essentially this is the scratch image that already has the ssl certs.

chainguard is updating these images continuously to have zero vulnerabilities.

https://github.com/chainguard-images/images

These images are based on wolfi-os which is basically a Linux (un)distro.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I known (and trust) the Chainguard, but this provides no added value over the scratch image. The container is intentionally as lightweight as possible.

@@ -53,7 +53,7 @@ image: 'ubuntu@sha256:47f14534bda344d9fe6ffd6effb95eefe579f4be0d508b7445cf77f61a
**⚠️ Warning!** The README corresponds to the `main` branch of ratchet's
development, and it may contain unreleased features.

**Pin to specific versions:**
### Pin to specific versions
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes seem unnecessary

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It allows me to link to the readme using anchors. Used same style for the new doc section added.

# Normally we would set this to run as "nobody", but to play nicely with GitHub
# Actions, it must run as the default user:
#
# https://docs.github.com/en/actions/creating-actions/dockerfile-support-for-github-actions#user
#
# USER nobody
WORKDIR /ws
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see the value in hard coding this. Users can boot the image with "-w" to set a working directory if needed.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It gives a much better out of the box experience. Of course users can customize anything they desire, but this gives at least out of the box an easy experience without customizations.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't agree. This relies on the user being willing to recognize and remember a random directory name (/ws) versus being explicit. I'm fine updating the README with an explicit example for running the container as a binary, but the rest of these changes seem unnecessary:

docker run -it --rm -v $PWD:$PWD -w $PWD ghcr.io/sethvargo/ratchet:0.4.0 "$@"

Or create yourself an alias in your `~/.bashrc` or `~/.zshrc`

```shell
function ratchet {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be docker run -it --rm --volume $PWD:$PWD -w $PWD ghcr.io/sethvargo/ratchet:0.4.0 "$@" which would require no changes to the Dockerfile.

@sethvargo sethvargo closed this in 4c346f7 Jan 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants