Conversation
alanshaw
left a comment
alanshaw
left a comment
There was a problem hiding this comment.
LGTM - left some suggestions.
| ## Status | ||
|
|
||
| - **Status**: Review | ||
| - **Applies to**: piri, piri-signing-service, delegator, indexing-service, and more |
There was a problem hiding this comment.
I would use them as examples, and generalise to "Storacha services with a public API" or similar.
|
|
||
| ### Release (tag v*) | ||
|
|
||
| A release tag should produce **prod** images only. Nobody ships a debugger to production on purpose. |
There was a problem hiding this comment.
I'd maybe include that container tag should match the git tag and obviously build from the same commit.
|
|
||
| ### Pull Requests | ||
|
|
||
| PR builds should validate that the Dockerfile compiles but need not push images anywhere. Building on a single platform (amd64) is sufficient here — the goal is fast feedback, not architectural completeness. |
There was a problem hiding this comment.
Is there any chance that building for a different arch could fail? My guess is no, or at least very unlikely, but if we're concerned then we should probably build on both.
|
|
||
| ## CI/CD Considerations | ||
|
|
||
| The workflow should live at `.github/workflows/publish-ghcr.yml` and be named `Container`. |
volmedo
left a comment
volmedo
left a comment
There was a problem hiding this comment.
Looks good overall. I left some suggestions.
| The production target should be small, fast, and boring: | ||
|
|
||
| - Stripped binary (`-ldflags="-s -w"`) | ||
| - Minimal base image (`debian:bookworm-slim`) |
There was a problem hiding this comment.
For Go I'd go with scratch for a really minimal image with the smallest attack surface possible. The caveat is that scratch has nothing in it, so it requires copying CA certificates from the builder image (we currently do this in our services deployed via storoku), and setting a non-root image up is a bit more involved.
Alternatively, gcr.io/distroless/static could be a good middle ground, as it is small but comes with CA certificates and a non-root user pre-baked in the image.
Of course, both alternatives work only for statically-linked binaries. But that is what we are doing currently.
|
|
||
| - Stripped binary (`-ldflags="-s -w"`) | ||
| - Minimal base image (`debian:bookworm-slim`) | ||
| - Only what's needed to run and health-check: `ca-certificates`, `curl` |
There was a problem hiding this comment.
To run a healthcheck on a different service? Having curl installed in a prod image it's an uncommon requirement IMO.
|
|
||
| The precise selection of debugging tools is a matter of taste. The principle is not: include everything that may be required. | ||
|
|
||
| ### Non-root user |
|
|
||
| The key insight: `$BUILDPLATFORM` is the CI runner's architecture (x86_64), and `$TARGETPLATFORM` is the final image's architecture. Go handles cross-compilation natively, so the compiler runs fast while producing binaries for whatever architecture you need. | ||
|
|
||
| QEMU remains necessary for the runtime stages, where `apt-get` and friends must execute on the target architecture. These operations are lightweight compared to compilation, so the tradeoff is favorable. |
There was a problem hiding this comment.
Avoiding the RUN command would also avoid the need of emulation completely. CA certificates can be installed in the builder and copied over to the runtime image (or use an image that comes with them pre-installed, such as distroless/static). Getting curl would be trickier though, but I'm not sure that should be a requirement.
| | `<sha>` | prod | No | Specific commit (production build) | | ||
| | `<sha>-dev` | dev | No | Specific commit (dev build) | |
There was a problem hiding this comment.
These are short SHAs, correct? Might be worth making it explicit.
3 participants
Preview