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

feat: add zizmor Docker image #319

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

trallard
Copy link

Closes #135

I am opening a WIP PR since I have some outstanding questions.

Notes 📝

  • I had originally suggested basing this on the uv Docker image. However, I think that might be overkill for a "vanilla" image for Zizmor, so I used the Rust image instead (bookworm-slim, which also helps keep the image quite small). That way, we provide the minimal required amount and leave it to users to build off as they need.

Questions/needs input 💬

  • I set the entry point as ENTRYPOINT ["/bin/bash"], or would it be best to point it to Zizmor directly?
  • For building and pushing the images:
    • I generally favour pushing images as part of my release processes, so I plan to add a reusable workflow that can be called from the release.yml workflow.
    • I think it would be suitable to build both linux/amd64 and linux/arm64 images.
  • Also, it seems suitable to add a Docker entry to the zizmor docs (similar to https://docs.astral.sh/uv/guides/integration/docker/)

@trallard trallard changed the title feat: add Dockerfile for zizmor feat: add zizmor Docker image Dec 17, 2024
@woodruffw
Copy link
Owner

Thanks @trallard, I appreciate you opening this!

  • However, I think that might be overkill for a "vanilla" image for Zizmor, so I used the Rust image instead (bookworm-slim, which also helps keep the image quite small). That way, we provide the minimal required amount and leave it to users to build off as they need.

Good call IMO 🙂 -- however, I wonder if we could keep the PyPI installation (i.e. via uv tool install or pip install rather than rebuilding with cargo install. I think this would keep our CI fast and simplify the arm64/amd64 image builds (since we already have native builds for both). WDYT?

  • I set the entry point as ENTRYPOINT ["/bin/bash"], or would it be best to point it to Zizmor directly?

Pointing to zizmor directly makes sense to me!

  • I generally favour pushing images as part of my release processes, so I plan to add a reusable workflow that can be called from the release.yml workflow.
  • I think it would be suitable to build both linux/amd64 and linux/arm64 images.

SGTM -- either a reusable workflow or a new docker job within the existing workflow would be fine by me. Dealer's choice 🙂

Yep! Ideally this PR would include those doc changes, although they could also be a follow-up.

@trallard
Copy link
Author

Yay! Thanks for the quick follow up.

These all seem good so I will work the rest of the things and can add some docs in this PR, then I can iterate if needed.

Good call IMO 🙂 -- however, I wonder if we could keep the PyPI installation (i.e. via uv tool install or pip install rather than rebuilding with cargo install. I think this would keep our CI fast and simplify the arm64/amd64 image builds (since we already have native builds for both). WDYT?

I can certainly do that. Should be easy enough to swap.

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.

Feature: Add a docker image
2 participants