build: switch to Go-based multi-stage build and improve#146
Open
c41ms0n wants to merge 8 commits intoshenxn:masterfrom
Open
build: switch to Go-based multi-stage build and improve#146c41ms0n wants to merge 8 commits intoshenxn:masterfrom
c41ms0n wants to merge 8 commits intoshenxn:masterfrom
Conversation
d00860a to
a3c684f
Compare
|
This seems really great. Have you considered merging this @simonfelding? |
- Use golang:1.26-trixie builder instead of debian:sid - Build proton-bridge from source via version argument/envelopment - Add support for PTY tools (dtach, abduco, reptyr) for interactive sessions - Introduce manage and attach commands for bridge CLI sessions - Improve daemon startup with port readiness checks - Add HEALTHCHECK and configurable CMD/ENTRYPOINT - Harden entrypoint with strict bash flags and better error handling - Install additional runtime deps (libfido2, procps) and optional PTY tools
Collaborator
There was a problem hiding this comment.
Pull request overview
This PR modernizes the build image by switching to a Go-based multi-stage build, adding optional PTY tooling to support interactive CLI sessions, and improving runtime robustness via readiness checks and a healthcheck.
Changes:
- Switch
build/Dockerfileto agolang:*builder stage and add runtime deps + optional PTY tools (dtach/abduco/reptyr), plusHEALTHCHECKand defaultCMD. - Rewrite
build/entrypoint.shto supportinit,manage,attach, andrunmodes, with stricter bash flags and bridge port readiness checks. - Add logic for interactive session management via PTY tools.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| build/entrypoint.sh | Replaces the previous minimal init/run logic with strict-mode bash, new manage/attach commands, PTY helpers, and port readiness gating before starting socat. |
| build/Dockerfile | Moves to a Go builder stage, installs additional deps and optional PTY tools, and adds HEALTHCHECK + ENTRYPOINT/CMD defaults. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4 tasks
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
… handling - Validate PTY_TOOL at startup and verify the selected binary exists - Add abduco_session_alive() using `abduco -l` instead of checking BRIDGE_SOCK (abduco does not create a socket file, so the old check always failed) - Fix wait_for_session, manage blocking loop, and attach to use tool-specific liveness checks rather than grouping dtach and abduco together - Add default (*) error branches to all PTY_TOOL case statements - Check kill -0 BRIDGE_PID inside the port readiness loop to fail fast if the bridge exits before its ports are ready - Add SIGTERM/SIGINT trap with a cleanup() function in run mode so Docker stop properly reaps bridge and socat children instead of waiting for the kill timeout - Replace \$(hostname) with <container> placeholder in user-facing error messages
- Replace legacy entrypoint with the current build/ version (PTY_TOOL support, init/manage/attach/run commands, SIGTERM trap, port-readiness liveness check, abduco session detection, <container> placeholder in error messages); only difference is binary name protonmail-bridge instead of /protonmail/proton-bridge - Add ARG/ENV PTY_TOOL and conditional apt install of dtach/abduco/reptyr - Add HEALTHCHECK on 127.0.0.1 for all four ports (25, 143, 1025, 1143) - Switch from CMD bash ... to ENTRYPOINT + CMD ["run"]
install.sh strips the official deb Depends line, removing libfido2-1. The bridge binary still links against it, so it must be installed explicitly.
Author
|
@simonfelding Deploying to production, but before Copilot review of the changes. abduco/reptyr not tested |
Add persistent cache mounts for the Go module cache, Go build cache, and apt so only changed packages are re-downloaded or recompiled on subsequent builds. CGO LTO is injected via `make LIBFIDO2_LDFLAGS=...` rather than ENV because the Makefile sets CGO_LDFLAGS inline in go-build-finalize, clobbering any inherited environment variable. Binary stripping is done with strip --strip-all post-build since the Makefile owns the -ldflags chain and cannot be extended without losing the -X version constants.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
golang:1.26-trixiebuilder instead ofdebian:sid(386, amd64, arm/v7, arm64/v8, ppc64le, riscv64, s390x)versionargument/envelopmentdtach,abduco,reptyr) for interactive sessions( Run the bridge inside tmux to enable easy user interaction #138)HEALTHCHECKand configurableCMD/ENTRYPOINT(feat: add healthcheck #128)libfido2,procps) and optional PTY tools (Add libfido2 dependency for Bridge v3.22.0 #139, Fix #135: Add libfido2 and libcbor to fix build of 3.22 #136)