Conversation
WalkthroughModifies the GitHub Actions build workflow to change the authentication service build matrix (adds Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Matrix as Matrix Runner
participant Builder as Build/Push Step
participant Registry as Container Registry
Note over GH,Matrix: Workflow triggered (push/PR)
GH->>Matrix: expand matrix (services + targets)
Matrix->>Builder: run job with matrix.service, matrix.target
alt target present
Builder->>Builder: set TARGET = matrix.target
Builder->>Registry: build image and tag as service-{{target}}:sha / service-{{target}}:latest
else no target
Builder->>Registry: build image and tag as service:sha / service:latest
end
Registry-->>Builder: push confirmation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (2)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/build_images.yml (1)
90-91: Image tag generation logic is correct, but formatting approach is inconsistent.The conditional target suffix logic is correct in both lines—the target suffix is appended only when defined, and the latest tag is only applied on main branch. However, line 90 uses direct string concatenation whilst line 91 uses the
format()function for the same purpose, which reduces readability and maintainability.For consistency and clarity, consider standardising to one approach. Here is a refactored version using
format()throughout:tags: | ${{ format('{0}-{1}{2}:{3}', env.IMAGE_PREFIX, matrix.service, matrix.target && format('-{0}', matrix.target) || '', github.ref_name) }} ${{ github.ref_name == 'main' && format('{0}-{1}{2}:latest', env.IMAGE_PREFIX, matrix.service, matrix.target && format('-{0}', matrix.target) || '') || '' }}Alternatively, if direct concatenation is preferred, simplify line 91 to match line 90's style.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build_images.yml(2 hunks)
🔇 Additional comments (1)
.github/workflows/build_images.yml (1)
88-88: Verify that undefined matrix variables are handled correctly with empty string target.When services without a target matrix entry (e.g., flights, aircraft, search, gateway) reach the build step,
matrix.targetis undefined. The expression${{ matrix.target || '' }}should provide an empty string fallback, but this depends on how GitHub Actions handles undefined matrix variables versus falsy values.The target parameter is supported by docker/build-push-action@v6, but it is unclear whether passing an empty string for target will skip target selection or cause unexpected behaviour. Before merging, verify this works correctly by:
- Running a test workflow that builds a service without a target defined in the matrix (e.g., services/flights)
- Confirming the resulting image is pushed successfully without target-specific suffixes
Alternatively, consider explicitly excluding or conditionally applying the target only when defined:
target: ${{ matrix.target || null }}However, this may fail if docker/build-push-action v6 does not accept null values for the target parameter.
Also applies to: 90-90, 91-91
Summary by CodeRabbit