Timestamp image tagging#96
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThe CI workflow now generates an image tag via a new step, exposes it as Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Tagger as Generate image tag
participant Builder as Docker build (DEV/PROD)
participant ECR as Amazon ECR
GH->>Tagger: run tag generator (outputs.tag)
Tagger-->>GH: outputs.tag (image_tag)
GH->>Builder: build images with :latest and :${{steps.image-tag.outputs.tag}}
Builder-->>GH: built images
GH->>ECR: push images with :latest, :${{steps.image-tag.outputs.tag}}, :${{github.sha}}
ECR-->>GH: stored image tags
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/build-dpid-resolver.yaml (1)
43-45: Quote the SHA variable for shell safety.While
github.shais GitHub-controlled and safe, quoting variables is a good practice to prevent issues with unexpected characters in other contexts.💡 Suggested improvement
- name: Generate image tag id: image-tag - run: echo "tag=$(echo ${{ github.sha }} | cut -c1-7)-$(date +%s)" >> $GITHUB_OUTPUT + run: echo "tag=$(echo '${{ github.sha }}' | cut -c1-7)-$(date +%s)" >> $GITHUB_OUTPUT🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-dpid-resolver.yaml around lines 43 - 45, In the "Generate image tag" step (id: image-tag) the unquoted github.sha expansion is used; update the run line to quote the SHA when piping to cut and when embedding into the tag (i.e., use a quoted "${{ github.sha }}" in the inner echo and ensure the final tag assignment is quoted) so the value is shell-safe and robust against any unexpected characters.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-dpid-resolver.yaml:
- Line 79: The workflow currently builds images tagged with ${{
steps.image-tag.outputs.tag }} (via CONTAINER_IMAGE-dev:${{
steps.image-tag.outputs.tag }} and CONTAINER_IMAGE-prod:${{
steps.image-tag.outputs.tag }}) but the ECR push steps only push the
${GITHUB_SHA} and latest tags; update both the DEV and PROD push steps to also
push the generated timestamp tag from steps.image-tag.outputs.tag. Specifically,
in the push commands for the dev image (referencing CONTAINER_IMAGE-dev) and the
prod image (CONTAINER_IMAGE-prod), add the additional tag push using the same
${{ steps.image-tag.outputs.tag }} value so ECR receives the timestamp tag for
FluxCD to consume.
---
Nitpick comments:
In @.github/workflows/build-dpid-resolver.yaml:
- Around line 43-45: In the "Generate image tag" step (id: image-tag) the
unquoted github.sha expansion is used; update the run line to quote the SHA when
piping to cut and when embedding into the tag (i.e., use a quoted "${{
github.sha }}" in the inner echo and ensure the final tag assignment is quoted)
so the value is shell-safe and robust against any unexpected characters.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ac0d69df-c89d-481f-ae0d-2747e11d1950
📒 Files selected for processing (1)
.github/workflows/build-dpid-resolver.yaml
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.env.example (1)
35-35:⚠️ Potential issue | 🟡 MinorUpdate commented staging and production FlightSQL endpoints for consistency.
The active dev configuration was updated to use HTTPS and the new hostname (
https://ceramic-one-flight-dev.desci.com), but the commented staging (line 35) and production (line 42) sections still reference the old HTTP endpoint format (http://ceramic-one-prod.desci.com:5102).Update these commented sections to maintain consistency and prevent confusion if developers uncomment them.
📝 Proposed fix for consistency
# DPID_ENV=staging # OPTIMISM_RPC_URL=https://reverse-proxy-staging.desci.com/rpc_opt_sepolia # CERAMIC_URL=https://ceramic-prod.desci.com # IPFS_GATEWAY=https://ipfs.desci.com/ipfs -# CERAMIC_FLIGHT_URL=http://ceramic-one-prod.desci.com:5102 +# CERAMIC_FLIGHT_URL=https://ceramic-one-flight-prod.desci.com # Prod # DPID_ENV=production # OPTIMISM_RPC_URL=https://reverse-proxy-prod.desci.com/rpc_opt_sepolia # CERAMIC_URL=https://ceramic-prod.desci.com # IPFS_GATEWAY=https://ipfs.desci.com/ipfs -# CERAMIC_FLIGHT_URL=http://ceramic-one-prod.desci.com:5102 +# CERAMIC_FLIGHT_URL=https://ceramic-one-flight-prod.desci.comAlso applies to: 42-42
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.env.example at line 35, Update the commented CERAMIC_FLIGHT_URL entries for staging and production to match the dev pattern (use HTTPS and the new "ceramic-one-flight-..." hostnames); locate the commented CERAMIC_FLIGHT_URL lines for staging and production and replace the old "http://ceramic-one-prod.desci.com:5102" style values with the HTTPS variants using "ceramic-one-flight-staging.desci.com" and "ceramic-one-flight-prod.desci.com" respectively so the commented examples are consistent with the active dev setting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.env.example:
- Line 35: Update the commented CERAMIC_FLIGHT_URL entries for staging and
production to match the dev pattern (use HTTPS and the new
"ceramic-one-flight-..." hostnames); locate the commented CERAMIC_FLIGHT_URL
lines for staging and production and replace the old
"http://ceramic-one-prod.desci.com:5102" style values with the HTTPS variants
using "ceramic-one-flight-staging.desci.com" and
"ceramic-one-flight-prod.desci.com" respectively so the commented examples are
consistent with the active dev setting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c344971f-6249-404a-a3a7-b6139b90b8ef
📒 Files selected for processing (3)
.env.example.env.test.github/workflows/build-dpid-resolver.yaml
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.env.test (1)
9-9: Reorder env keys to satisfy dotenv-linterAt Line 9,
CERAMIC_FLIGHT_URLshould be placed beforeCERAMIC_URLto clear the reportedUnorderedKeywarning and avoid lint noise/failures.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.env.test at line 9, Reorder the environment variable keys so dotenv-linter's alphabetical order is satisfied: move CERAMIC_FLIGHT_URL to appear before CERAMIC_URL in the .env.test file; ensure the two keys (CERAMIC_FLIGHT_URL and CERAMIC_URL) are in correct ascending order relative to surrounding entries to clear the UnorderedKey warning..env.example (1)
24-24: Keep key ordering consistent with dotenv-linterAt Line 24, place
CERAMIC_FLIGHT_URLaboveCERAMIC_URLin this section to resolve theUnorderedKeywarning.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.env.example at line 24, Reorder the keys in .env.example to satisfy dotenv-linter by moving CERAMIC_FLIGHT_URL so it appears above CERAMIC_URL in that section; locate the lines containing CERAMIC_FLIGHT_URL and CERAMIC_URL and swap their positions so the file key ordering matches the linter's expected alphabetical/section order.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.env.example:
- Line 24: Reorder the keys in .env.example to satisfy dotenv-linter by moving
CERAMIC_FLIGHT_URL so it appears above CERAMIC_URL in that section; locate the
lines containing CERAMIC_FLIGHT_URL and CERAMIC_URL and swap their positions so
the file key ordering matches the linter's expected alphabetical/section order.
In @.env.test:
- Line 9: Reorder the environment variable keys so dotenv-linter's alphabetical
order is satisfied: move CERAMIC_FLIGHT_URL to appear before CERAMIC_URL in the
.env.test file; ensure the two keys (CERAMIC_FLIGHT_URL and CERAMIC_URL) are in
correct ascending order relative to surrounding entries to clear the
UnorderedKey warning.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0b2c32b6-ea2d-410c-a122-294c078eedbf
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
.env.example.env.testpackage.json
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
1 participant
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.
for fluxcd
Summary by CodeRabbit