Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Summary by CodeRabbit
WalkthroughThe changes update the transport build workflow and Dockerfile to improve version handling. The workflow now extracts and validates the version from tags more precisely, outputs it for later steps, and passes it as a build argument to Docker. The Dockerfile uses this argument to fetch the correct module version during the build. Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub Actions
participant Docker Build
participant Dockerfile
participant Go Module Proxy
GitHub Actions->>GitHub Actions: Extract and validate version from tag
GitHub Actions->>Docker Build: Pass TAG_VERSION as build argument
Docker Build->>Dockerfile: Build with ARG TAG_VERSION
Dockerfile->>Go Module Proxy: go get bifrost module @TAG_VERSION
Go Module Proxy-->>Dockerfile: Return specified module version
Dockerfile-->>Docker Build: Complete build with correct module
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate Unit Tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| ARG TAG_VERSION=latest | ||
|
|
||
| # Initialize go module and get bifrost-http | ||
| # Initialize go module and get bifrost with specified tag | ||
| RUN go mod init bifrost-build && \ | ||
| go get github.com/maximhq/bifrost/transports/bifrost-${TRANSPORT_TYPE}@latest | ||
| go get github.com/maximhq/bifrost/transports/bifrost-${TRANSPORT_TYPE}@${TAG_VERSION} |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider switching from go get to go install (or go mod download) for reproducible, forward-compatible builds
go get mutates go.mod/go.sum and has been discouraged for fetching binaries since Go 1.20.
Using go install (for direct binary installation) or a plain go mod download && go build keeps the module graph immutable and avoids the deprecation warning that will soon turn into an error.
-ARG TAG_VERSION=latest
-# Initialize go module and get bifrost with specified tag
-RUN go mod init bifrost-build && \
- go get github.com/maximhq/bifrost/transports/bifrost-${TRANSPORT_TYPE}@${TAG_VERSION}
+# Explicit module initialisation
+ARG TAG_VERSION=latest
+RUN go mod init bifrost-build && \
+ go mod tidy && \
+ go install github.com/maximhq/bifrost/transports/bifrost-${TRANSPORT_TYPE}@${TAG_VERSION} && \
+ go mod downloadThis keeps the Docker layer deterministic, avoids side-effects in go.mod, and aligns with the Go tooling roadmap.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ARG TAG_VERSION=latest | |
| # Initialize go module and get bifrost-http | |
| # Initialize go module and get bifrost with specified tag | |
| RUN go mod init bifrost-build && \ | |
| go get github.com/maximhq/bifrost/transports/bifrost-${TRANSPORT_TYPE}@latest | |
| go get github.com/maximhq/bifrost/transports/bifrost-${TRANSPORT_TYPE}@${TAG_VERSION} | |
| ARG TAG_VERSION=latest | |
| # Explicit module initialization | |
| RUN go mod init bifrost-build && \ | |
| go mod tidy && \ | |
| go install github.com/maximhq/bifrost/transports/bifrost-${TRANSPORT_TYPE}@${TAG_VERSION} && \ | |
| go mod download |
🤖 Prompt for AI Agents
In transports/Dockerfile around lines 13 to 17, replace the use of 'go get' with
'go install' or 'go mod download' followed by 'go build' to fetch the bifrost
module. This avoids mutating go.mod and go.sum files during the Docker build,
ensuring reproducible and forward-compatible builds. Adjust the commands to
install or build the binary without changing the module graph, maintaining
deterministic Docker layers and preventing future deprecation errors.
| echo "tags<<EOF" >> $GITHUB_OUTPUT | ||
| echo "${{ env.REGISTRY }}/${{ env.ACCOUNT }}/${{ env.IMAGE_NAME }}:v${VERSION}" >> $GITHUB_OUTPUT | ||
| echo "${{ env.REGISTRY }}/${{ env.ACCOUNT }}/${{ env.IMAGE_NAME }}:${VERSION}" >> $GITHUB_OUTPUT | ||
| echo "${{ env.REGISTRY }}/${{ env.ACCOUNT }}/${{ env.IMAGE_NAME }}:latest" >> $GITHUB_OUTPUT | ||
| echo "EOF" >> $GITHUB_OUTPUT |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use the official multi-line $GITHUB_OUTPUT syntax to avoid missing delimiter errors
Manually echoing the tags<<EOF … EOF sequence is error-prone.
The recommended pattern is:
{
echo 'tags<<EOF'
echo "${{ env.REGISTRY }}/${{ env.ACCOUNT }}/${{ env.IMAGE_NAME }}:${VERSION}"
echo "${{ env.REGISTRY }}/${{ env.ACCOUNT }}/${{ env.IMAGE_NAME }}:latest"
echo 'EOF'
} >>"$GITHUB_OUTPUT"Wrapping the block in braces guarantees the heredoc is kept together and reduces chances of accidental redirection mistakes during future edits.
🤖 Prompt for AI Agents
In .github/workflows/transport-ci.yml around lines 162 to 165, replace the
multiple echo commands appending to $GITHUB_OUTPUT individually with a single
block wrapped in braces that echoes the heredoc lines together. This ensures the
multi-line output is written atomically and prevents missing delimiter errors by
grouping the echo statements inside braces and redirecting their combined output
once to $GITHUB_OUTPUT.
| TRANSPORT_TYPE=http | ||
| TAG_VERSION=${{ steps.meta.outputs.version }} |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Minor: keep build-arg list transport-agnostic
TRANSPORT_TYPE=http is hard-coded here while the rest of the pipeline is tag-driven. When a non-HTTP transport is tagged this step will still label the image “HTTP”, risking confusion.
Consider:
build-args: |
TRANSPORT_TYPE=${{ env.TRANSPORT_TYPE }}
TAG_VERSION=${{ steps.meta.outputs.version }}and source TRANSPORT_TYPE from the tag or job matrix to stay future-proof.
🤖 Prompt for AI Agents
In .github/workflows/transport-ci.yml at lines 198-199, the TRANSPORT_TYPE build
argument is hard-coded to "http", which conflicts with the tag-driven approach
and can cause confusion for non-HTTP transports. Update the build-args to use
TRANSPORT_TYPE from the environment variable instead, like TRANSPORT_TYPE=${{
env.TRANSPORT_TYPE }}, and ensure TRANSPORT_TYPE is set appropriately from the
tag or job matrix to keep the pipeline transport-agnostic and future-proof.
| # Extract version from tag (remove transports/ prefix) | ||
| VERSION=${TAG#transports/} | ||
| echo "version=${VERSION}" >> $GITHUB_OUTPUT | ||
|
|
||
| # Extract numeric version for validation | ||
| NUMERIC_VERSION=${VERSION#v} | ||
|
|
||
| # Validate version format | ||
| if ! echo "$VERSION" | grep -qE '^[0-9]+\.[0-9]+\.[0-9]+$'; then | ||
| if ! echo "$NUMERIC_VERSION" | grep -qE '^[0-9]+\.[0-9]+\.[0-9]+$'; then |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Path-traversal & whitespace hardening for extracted version
VERSION=${TAG#transports/} and NUMERIC_VERSION=${VERSION#v} rely on the incoming tag being perfectly formed.
If a malicious (or simply mistyped) tag like transports/v1.2.3$(printf '\nX') were pushed, the newline would bleed into subsequent echo >> $GITHUB_OUTPUT lines and break the workflow / poison the build args.
Add simple sanitation:
VERSION=${TAG#transports/}
VERSION=${VERSION//$'\n'/} # strip newlines
VERSION=${VERSION//$'\r'/} # strip carriage returnsor use printf '%s\n' instead of bare echo when writing to $GITHUB_OUTPUT.
Tiny change – big safety net.
🤖 Prompt for AI Agents
In .github/workflows/transport-ci.yml around lines 148 to 156, the extraction of
VERSION and NUMERIC_VERSION from TAG does not sanitize input, allowing newlines
or carriage returns to corrupt the output and break the workflow. To fix this,
after extracting VERSION, strip any newline and carriage return characters from
it using parameter expansion to remove $'\n' and $'\r'. Alternatively, replace
the bare echo commands writing to $GITHUB_OUTPUT with printf '%s\n' to safely
handle special characters. This sanitization prevents path traversal and
whitespace injection issues.
#132) # Fix Docker image versioning and add tag-based dependency resolution This PR improves the Docker image build process for transports in two key ways: 1. Fixes the Docker image tag format by: - Preserving the 'v' prefix in version numbers - Properly extracting the version from tags (removing only the 'transports/' prefix) - Adding a separate step to extract the numeric version for validation 2. Adds tag-based dependency resolution: - Introduces a new `TAG_VERSION` build argument to the Dockerfile - Passes the extracted version from the GitHub workflow to the Dockerfile - Updates the Go dependency resolution to use the specific tagged version instead of always using 'latest' These changes ensure that when building a specific transport version, the Docker image will use the matching tagged version of the bifrost dependency rather than always pulling the latest version.
2 participants
Fix Docker image versioning and add tag-based dependency resolution
This PR improves the Docker image build process for transports in two key ways:
Fixes the Docker image tag format by:
Adds tag-based dependency resolution:
TAG_VERSIONbuild argument to the DockerfileThese changes ensure that when building a specific transport version, the Docker image will use the matching tagged version of the bifrost dependency rather than always pulling the latest version.