feat: add version tags to Docker images #3060
Conversation
Signed-off-by: SoulSniper1212 <[email protected]>
SoulSniper1212
requested review from
Br0wnHammer,
Owaiseimdad,
ajhollid and
karenvicent
as code owners
|
Note
|
| Cohort / File(s) | Change Summary |
|---|---|
CI/CD workflow .github/workflows/deploy-images.yml |
Added a "Get version" step (compute VERSION from git describe), set fetch-depth: 0 on server checkout, export VERSION to env, pass version into builds, and push both :latest and :<VERSION> tags for client, server, mongo, and mono images. |
Dockerfiles — ARM server docker/dist-arm/server.Dockerfile |
Added ARG VITE_APP_VERSION to the frontend-build stage to expose version during the frontend build. |
Dockerfiles — Mono server docker/dist-mono/server.Dockerfile |
Added ARG VITE_APP_VERSION declarations to the frontend-build stage and the app stage so VITE_APP_VERSION is available across build stages. |
Sequence Diagram
sequenceDiagram
participant GH as GitHub Actions
participant Git as Git Repo
participant Docker as Docker Build
participant Reg as Container Registry
GH->>Git: checkout (fetch-depth: 0)
GH->>Git: run `git describe` -> VERSION
Note right of GH: export VERSION to job env
GH->>Docker: build client (build-arg VITE_APP_VERSION=VERSION)
GH->>Docker: build server/mongo/mono (pass VERSION where applicable)
Docker->>Reg: push image:latest
Docker->>Reg: push image:VERSION
Note over Reg: Both tags available for deployment
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
- Areas to verify:
git describeproduces the intended format in all branches/tags used by CI.fetch-depth: 0is set wherever VERSION is computed.- All image build steps receive VITE_APP_VERSION where needed and Dockerfiles reference it correctly.
- Tagging/pushing logic includes both architectures and matches registry tag expectations.
- Ensure removal of server VITE_APP_VERSION build-arg (if deliberate) does not break server image contents.
Poem
🐰 I hopped through commits and found a name,
Tags now wear numbers and not just "latest" fame.
Fromgit describeI pull the thread,
Two tags now sit atop each sled.
Hop, build, push — a tidy game.
Pre-merge checks and finishing touches
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Description check | The PR description addresses the issue number and outlines the key changes, but does not follow the repository's required template structure and is missing several required checklist items. | Use the repository's required PR description template, complete all checklist items (especially deployment testing and self-review), and ensure all required sections are properly filled out. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title 'feat: add version tags to Docker images' accurately and specifically describes the main change in the PR: adding versioned tags to Docker images. |
| Linked Issues check | ✅ Passed | The PR successfully implements the core requirement from #3051: adding version tags to Docker images. Changes include workflow updates to tag with version numbers and Dockerfile modifications to accept version arguments. |
| Out of Scope Changes check | ✅ Passed | All changes are directly related to implementing version tagging for Docker images as specified in #3051. Modifications to workflows and Dockerfiles align with the stated objectives. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✨ Finishing touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/deploy-images.yml (1)
23-25: Add error handling togit describecommand—repository has no tags.The
git describe --tags --abbrev=0command will fail immediately since the repository currently has no tags, halting the workflow before any Docker images are built. Add error handling with a fallback version:- name: Get version id: vars - run: echo "VERSION=$(git describe --tags --abbrev=0)" >> $GITHUB_ENV + run: | + VERSION=$(git describe --tags --abbrev=0 2>/dev/null || echo "0.0.0") + echo "VERSION=$VERSION" >> $GITHUB_ENVThis applies to all four instances of this step in the file (lines 25, 59, 111, 145).
🧹 Nitpick comments (2)
docker/dist-mono/server.Dockerfile (1)
15-15: Remove unnecessary ARG from app stage.The
appstage performs a backend build and doesn't needVITE_APP_VERSION. Frontend assets are copied from thefrontend-buildstage (line 31), so versioning is only relevant to the frontend-build stage.FROM node:20-slim AS app -ARG VITE_APP_VERSION - # Install ping.github/workflows/deploy-images.yml (1)
57-59: Repeated version extraction needs error handling in all jobs.The
Get versionstep is repeated across three jobs (lines 57-59, 109-111, 143-145) with the same git describe command and the same failure mode. Apply the error handling fix to all instances:- name: Get version id: vars - run: echo "VERSION=$(git describe --tags --abbrev=0)" >> $GITHUB_ENV + run: | + VERSION=$(git describe --tags --abbrev=0 2>/dev/null || echo "0.0.0") + echo "VERSION=$VERSION" >> $GITHUB_ENVConsider extracting this to a reusable action or workflow to avoid repetition.
Also applies to: 109-111, 143-145
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
docker/dist/server.Dockerfileis excluded by!**/dist/**
📒 Files selected for processing (3)
.github/workflows/deploy-images.yml(5 hunks)docker/dist-arm/server.Dockerfile(1 hunks)docker/dist-mono/server.Dockerfile(2 hunks)
🔇 Additional comments (2)
.github/workflows/deploy-images.yml (2)
26-39: Versioned image tagging is consistent and well-structured.The client image build/push (and similar patterns across other images) correctly:
- Passes version as build-arg (line 33)
- Tags both
:latestand:${{ env.VERSION }}(lines 29-30, 38-39)- Fetches full git history (line 14)
Once the Dockerfiles are fixed to use
ENV VITE_APP_VERSION=$VITE_APP_VERSION, the versioning will propagate correctly.
128-160: docker-build-and-push-server-mono job is well-structured.The mono build/push follows the same versioning pattern as other images and is correctly implemented. Versioning will work once the Dockerfile is fixed to propagate VITE_APP_VERSION to the frontend build.
| # --------------------- | ||
| FROM node:24-slim AS frontend-build | ||
|
|
||
| ARG VITE_APP_VERSION |
There was a problem hiding this comment.
🧩 Analysis chain
ARG VITE_APP_VERSION is declared but never used.
The build argument is not passed to the frontend build process. To propagate the version to the npm build, convert the ARG to an environment variable before the npm run build command:
ARG VITE_APP_VERSION
WORKDIR /app/client
COPY client/package.json ./
RUN npm install
RUN npm install [email protected] --build-from-source
COPY client ./
+ENV VITE_APP_VERSION=$VITE_APP_VERSION
RUN npm run buildI'll verify the review comment by examining the Dockerfile and checking how VITE_APP_VERSION is actually used.
---
🏁 Script executed:
```shell
#!/bin/bash
# Read the entire Dockerfile to understand the build context
cat docker/dist-arm/server.Dockerfile
Length of output: 769
🏁 Script executed:
#!/bin/bash
# Search for all references to VITE_APP_VERSION in the Dockerfile
rg -n "VITE_APP_VERSION" docker/dist-arm/server.DockerfileLength of output: 22
🏁 Script executed:
#!/bin/bash
# Check if there's a vite config or build script that references this variable
fd -e "vite.config" -e "vite.config.js" -e "vite.config.ts" -e ".env" -e "package.json" client/ 2>/dev/null | head -20Length of output: 0
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for vite config files more broadly
find client -name "*vite*" -type f 2>/dev/null
Length of output: 56
🏁 Script executed:
#!/bin/bash
# Find and read client package.json to see build script
find client -name "package.json" -type f -exec cat {} \;Length of output: 1720
🏁 Script executed:
#!/bin/bash
# Search for any references to VITE_APP_VERSION throughout the client directory
rg -r "VITE_APP_VERSION" client/Length of output: 60430
🏁 Script executed:
#!/bin/bash
# Check for environment variable patterns in the build or client setup
rg -n "process.env" client/ --type=ts --type=js | head -20Length of output: 0
---
🏁 Script executed:
```shell
#!/bin/bash
# Read the Vite config to see how environment variables are used
cat client/vite.config.ts
Length of output: 617
🏁 Script executed:
#!/bin/bash
# Search for VITE_ prefix usage in client source code to understand Vite env pattern
rg "import\.meta\.env" client/src --type=ts --type=js -C 2Length of output: 1637
ARG VITE_APP_VERSION is declared but never used.
The build argument is not passed to the frontend build process. Docker's ARG is only available during build time; it must be converted to an environment variable to be accessible to npm. To propagate the version to the npm build, add an ENV directive before npm run build:
ARG VITE_APP_VERSION
WORKDIR /app/client
COPY client/package.json ./
RUN npm install
RUN npm install [email protected] --build-from-source
COPY client ./
+ENV VITE_APP_VERSION=$VITE_APP_VERSION
RUN npm run buildCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In docker/dist-arm/server.Dockerfile around line 6, ARG VITE_APP_VERSION is
declared but never passed into the frontend build; convert the build ARG into an
environment variable before invoking npm run build so the frontend can read
VITE_APP_VERSION (e.g., add an ENV VITE_APP_VERSION=${VITE_APP_VERSION} prior to
the build step or set it in the same RUN that executes npm run build), ensuring
the variable is available at build time for Vite/npm.
| @@ -1,5 +1,7 @@ | |||
| FROM node:20-slim AS frontend-build | |||
|
|
|||
| ARG VITE_APP_VERSION | |||
There was a problem hiding this comment.
ARG in frontend-build stage not propagated to npm build.
The version argument must be converted to an environment variable before the npm build runs:
ARG VITE_APP_VERSION
WORKDIR /app/client
COPY client/package*.json ./
RUN npm install
COPY client ./
+ENV VITE_APP_VERSION=$VITE_APP_VERSION
RUN npm run buildCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In docker/dist-mono/server.Dockerfile around line 3, the ARG VITE_APP_VERSION
declared in the frontend-build stage is not exported to the environment so npm
build cannot access it; change the Dockerfile to convert the ARG into an ENV
(e.g., add a line after the ARG like ENV VITE_APP_VERSION=${VITE_APP_VERSION})
before running npm install/npm run build so the build tooling can read
process.env.VITE_APP_VERSION (or VITE_APP_VERSION) during the frontend build.
![llamapreview[bot]](https://avatars.githubusercontent.com/in/1023541?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI Code Review by LlamaPReview
🎯 TL;DR & Recommendation
Recommendation: Request Changes
This PR adds version tagging to Docker images but introduces an architectural inconsistency with unused build arguments in server images, which could mislead future maintainers.
🌟 Strengths
- Implements version tagging for Docker images, enhancing traceability and deployment management as intended.
| Priority | File | Category | Impact Summary | Anchors |
|---|---|---|---|---|
| P1 | .github/workflows/deploy-images.yml |
Architecture | Unused build arg misleads maintainers | path:docker/dist/server.Dockerfile |
| P2 | .github/workflows/deploy-images.yml |
Testing | Version extraction may fail in clones | |
| P2 | .github/workflows/deploy-images.yml |
Architecture | Inconsistent naming causes confusion | path:docker/dist-arm/docker-compose.yaml |
| P2 | docker/dist/server.Dockerfile |
Maintainability | Unused ARG adds noise | |
| P2 | .github/workflows/deploy-images.yml |
Testing | Lacks validation for image equivalence | path:docker/prod/docker-compose.yaml |
🔍 Notable Themes
- Unused Build Arguments: Multiple findings highlight unnecessary
VITE_APP_VERSIONarguments in server builds, suggesting a need to align build arguments with actual usage. - Version Handling Consistency: Issues with version extraction and validation indicate opportunities to standardize and verify version-related processes.
💡 Have feedback? We'd love to hear it in our GitHub Discussions.
✨ This review was generated by LlamaPReview Advanced, which is free for all open-source projects. Learn more.
| @@ -1,5 +1,7 @@ | |||
| FROM node:20-slim | |||
There was a problem hiding this comment.
P2 | Confidence: High
The Dockerfile declares ARG VITE_APP_VERSION but never uses it in any RUN, CMD, or ENV instructions. This unused variable adds noise without providing functional value. Since server code doesn't use Vite (a frontend build tool), this argument serves no purpose in this context.
| FROM node:20-slim | |
| Remove the unused `ARG` declaration unless there are plans to incorporate version metadata into server runtime. |
| - name: Get version | ||
| id: vars | ||
| run: echo "VERSION=$(git describe --tags --abbrev=0)" >> $GITHUB_ENV |
There was a problem hiding this comment.
P2 | Confidence: Medium
The version extraction uses git describe --tags --abbrev=0 which fetches the latest tag. This approach may not work correctly in shallow clones or when building from non-tagged commits. The related_context shows other workflows use similar version extraction, suggesting this pattern is established but potentially fragile across different CI scenarios.
Code Suggestion:
- name: Get version
id: vars
run: |
VERSION=$(git describe --tags --abbrev=0 2>/dev/null || git rev-parse --short HEAD)
echo "VERSION=$VERSION" >> $GITHUB_ENV| - name: Push Server Docker image | ||
| run: | | ||
| docker push ghcr.io/bluewave-labs/checkmate-backend:latest | ||
| docker push ghcr.io/bluewave-labs/checkmate-backend:${{ env.VERSION }} |
There was a problem hiding this comment.
P2 | Confidence: Medium
Speculative: The version tagging implementation lacks validation that versioned images are functionally equivalent to latest. The related_context shows compose files use specific image tags (frontend-demo, frontend-staging), suggesting the project has multiple deployment environments. Without automated verification, versioned images might contain undetected regressions.
Code Suggestion:
Add a smoke test step that deploys and validates the versioned image in an isolated environment before promoting it.Evidence: path:docker/prod/docker-compose.yaml, path:docker/staging/docker-compose.yaml
| with: | ||
| context: . | ||
| file: ./docker/dist-arm/server.Dockerfile | ||
| push: true |
There was a problem hiding this comment.
[Contextual Comment]
This comment refers to code near real line 113. Anchored to nearest_changed(118) line 118.
P2 | Confidence: High
The multi-arch image naming (checkmate-backend-mono-multiarch) differs from other server images (checkmate-backend, checkmate-backend-mono). The related_context shows docker/dist-arm/docker-compose.yaml uses checkmate-backend-mono-multiarch:latest, confirming this is a production dependency. This naming inconsistency could cause confusion in deployment tooling and documentation.
Co-authored-by: llamapreview[bot] <184758061+llamapreview[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/deploy-images.yml (1)
41-66: Build step for server image is indeed missing in docker-build-and-push-server job.The review comment is correct. The job attempts to push
checkmate-backend:latestandcheckmate-backend:${{ env.VERSION }}without building the image first. The "Push Server Docker image" step comes immediately after "Get version" (line 60–63), with no build step preceding it. This will fail at runtime because the image doesn't exist in the local Docker daemon.Compare to the client job (lines 26–34), which correctly builds the image before pushing. The
docker-build-and-push-server-monoanddocker-build-and-push-server-mono-multiarchjobs also follow the correct build-then-push pattern.Add a "Build Server Docker image" step between "Get version" and "Push Server Docker image". Verify whether
--build-arg VITE_APP_VERSIONshould be included (the embedded comment suggests reviewing this).
♻️ Duplicate comments (2)
.github/workflows/deploy-images.yml (2)
75-75: Verify that VITE_APP_VERSION is actually used in MongoDB and server Dockerfiles.
VITE_APP_VERSIONis passed as a build argument to MongoDB (line 75), multi-arch server (line 118), and mono-server (line 146) builds. However,VITE_APP_VERSIONis a Vite-specific environment variable typically used only in frontend/client builds. Confirm that these Dockerfiles actually consume this variable; if not, remove these unused build arguments to avoid confusion and reduce build overhead.Please verify by checking:
- Does
docker/dist/mongoDB.DockerfileuseVITE_APP_VERSION?- Does
docker/dist-arm/server.DockerfileuseVITE_APP_VERSION?- Does
docker/dist-mono/server.DockerfileuseVITE_APP_VERSION?If not used, apply this pattern to remove the unused argument:
- --build-arg VITE_APP_VERSION=${{ env.VERSION }} \Also applies to: 118-118, 146-146
61-61: Invalid YAML syntax—remove non-YAML content from workflow file.Line 61 contains a plain-text comment that breaks YAML parsing. This is causing the static analysis errors on lines 61 and 63. Remove this line entirely as it is not valid workflow syntax.
-Remove `--build-arg VITE_APP_VERSION` from server image builds unless server code actually consumes this variable. -
🧹 Nitpick comments (1)
.github/workflows/deploy-images.yml (1)
23-25: Consolidate duplicated "Get version" steps.The version extraction logic (
git describe --tags --abbrev=0) is duplicated identically across four jobs. Consider extracting this as a reusable workflow or composite action to reduce duplication and ensure consistency across all version-extraction invocations.Example: Create a composite action
actions/get-version.yml:name: Get Version description: Extract version from git tags outputs: version: description: Extracted version value: ${{ steps.get.outputs.version }} steps: - id: get run: echo "version=$(git describe --tags --abbrev=0 2>/dev/null || echo 'unknown')" >> $GITHUB_OUTPUTThen replace each duplication with:
- uses: ./.github/actions/get-version id: version - name: Use version run: echo "VERSION=${{ steps.version.outputs.version }}" >> $GITHUB_ENVBased on learnings from past reviews, this also addresses the fragility concern: add fallback logic (e.g.,
2>/dev/null || echo 'unknown') to handle edge cases gracefully.Also applies to: 57-59, 101-103, 135-137
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/deploy-images.yml(5 hunks)
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/deploy-images.yml
61-61: could not parse as YAML: could not find expected ':'
(syntax-check)
🪛 YAMLlint (1.37.1)
.github/workflows/deploy-images.yml
[error] 63-63: syntax error: could not find expected ':'
(syntax)
1 participant
Overview
This pull request addresses issue #3051 by adding version tags to the Docker images during the build and push process. This will allow for better image management and rollback capabilities.
Checklist
Proof
The changes modify the GitHub Actions workflow to tag Docker images with both
latestand the current version. The Dockerfiles are updated to accept theVITE_APP_VERSIONargument. This ensures that each image build includes a version tag, improving traceability and deployment management.Closes #3051
Summary by CodeRabbit