Refactor Dockerfiles, enhance GHCR CI workflow, and update README#289
Refactor Dockerfiles, enhance GHCR CI workflow, and update README#289watal wants to merge 9 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the Docker build infrastructure to improve security and supply chain transparency. It introduces multi-stage builds for smaller production images, separates production and development images, and enhances the CI/CD pipeline with multi-platform builds, SBOM generation, provenance attestation, and Cosign image signing.
Changes:
- Refactored Dockerfiles to use multi-stage builds (production) and separate development images with distinct tagging schemes (latest vs latest-dev)
- Enhanced GitHub Actions workflow with matrix builds for prod/dev variants, multi-platform support (amd64/arm64), and security features (SBOM, provenance, Cosign signing)
- Updated examples and documentation to reflect new image tags and build instructions
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| build/package/Dockerfile | Refactored to multi-stage build with debian:bookworm-slim runtime for minimal production image |
| build/package/Dockerfile.dev | New development Dockerfile with full toolchain and debugging utilities |
| .github/workflows/ghcr.yml | Enhanced workflow with matrix builds, multi-platform support, SBOM, provenance, and Cosign signing |
| build/package/README.md | Updated documentation with separate instructions for published vs locally-built images |
| examples/tinet/sr-mpls_explicit-path_l3vpn/spec.yaml | Updated to use latest-dev tag for local development builds |
| examples/containerlab/srv6_explicit-path_l3vpn/srv6_te_l3vpn.clab.yml | Updated to use latest-dev tag from GHCR |
| examples/containerlab/sr-mpls_explicit-path/sr-mpls_pcep.clab.yml | Updated to use latest-dev tag from GHCR |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
watal
force-pushed
the
build/docker-refactor
branch
2 times, most recently
from
487be09 to
61f990b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
watal
force-pushed
the
build/docker-refactor
branch
from
bcf444b to
60d0252
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| LOGDIR="$(pwd)/logs" | ||
| mkdir -p "$LOGDIR" |
There was a problem hiding this comment.
The LOGDIR is set to an absolute path "/var/log/pola" in the original version but changed to a relative path "$(pwd)/logs" in the new version. While the new version is more portable and doesn't require root permissions, this is a significant behavioral change. Users upgrading from the old instructions will have logs in a different location. Consider documenting this change or providing a note about log directory location.
| # Start the container | ||
| docker run -d --network host \ | ||
| -v "$(pwd)/$MOUNTDIR:/$MOUNTDIR" \ | ||
| -v "$LOGDIR:$LOGDIR" \ | ||
| -w "/$MOUNTDIR" \ | ||
| ghcr.io/nttcom/pola:latest \ | ||
| polad -f polad.yaml |
There was a problem hiding this comment.
The docker run command uses '-d' (detach mode) instead of the original '-itd' (interactive, TTY, detach). This change is appropriate since polad runs as a daemon and doesn't need an interactive TTY. However, this means users cannot easily attach to the container for debugging. Consider documenting how to access the container for debugging purposes (e.g., using 'docker exec').
| pola-pce: | ||
| kind: linux | ||
| image: ghcr.io/nttcom/pola:latest | ||
| image: ghcr.io/nttcom/pola:latest-dev |
There was a problem hiding this comment.
The containerlab configuration uses the latest-dev image which has 'polad' as the ENTRYPOINT (as defined in Dockerfile.dev). However, the configuration doesn't specify a 'cmd' or override the entrypoint. The container will start with 'polad -f polad.yaml' by default, but the default working directory and the location of polad.yaml may not align. Consider adding a 'cmd' field to specify the correct path to the configuration file, or verify that the default CMD arguments work correctly with the bind mount at '/polad.yaml'.
| image: ghcr.io/nttcom/pola:latest-dev | |
| image: ghcr.io/nttcom/pola:latest-dev | |
| cmd: ["-f", "/polad.yaml"] |
| pola-pce: | ||
| kind: linux | ||
| image: ghcr.io/nttcom/pola:latest | ||
| image: ghcr.io/nttcom/pola:latest-dev |
There was a problem hiding this comment.
The containerlab configuration uses the latest-dev image which has 'polad' as the ENTRYPOINT (as defined in Dockerfile.dev). However, the configuration doesn't specify a 'cmd' or override the entrypoint. The container will start with 'polad -f polad.yaml' by default, but the default working directory and the location of polad.yaml may not align. Consider adding a 'cmd' field to specify the correct path to the configuration file, or verify that the default CMD arguments work correctly with the bind mount at '/polad.yaml'.
| image: ghcr.io/nttcom/pola:latest-dev | |
| image: ghcr.io/nttcom/pola:latest-dev | |
| cmd: ["-f", "/polad.yaml"] |
watal
force-pushed
the
build/docker-refactor
branch
3 times, most recently
from
c778a8c to
71215c5
Compare
watal
force-pushed
the
build/docker-refactor
branch
2 times, most recently
from
074703a to
18ce806
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
watal
force-pushed
the
build/docker-refactor
branch
from
18ce806 to
a8e5bcc
Compare
Description
Refactored Dockerfiles to adopt a multi-stage build and a minimal runtime image.
Enhanced the GHCR GitHub Actions workflow by adding:
Updated README and example configurations to reflect the new image usage and local build instructions.
Type of change
Motivation and Context
This change improves:
How is This Tested?
test/)examples/tinet/sr-mpls_explicit-path_l3vpnexamples/containerlab/srv6_te_l3vpnexamples/containerlab/sr-mpls_explicit-path