-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use the released tag version to identify newly-created docker images #592
Conversation
WalkthroughThe changes in this pull request involve updates to the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- .github/workflows/build.yml (2 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/build.yml
10-10: context "env" is not allowed here. available contexts are "github", "inputs", "secrets", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details
(expression)
🔇 Additional comments (1)
.github/workflows/build.yml (1)
Line range hint
1-40
: Overall assessment: Changes align with PR objectivesThe modifications to this workflow file effectively implement a versioning system for Docker images using Git tags, which aligns well with the PR objectives and addresses the issue raised in #489. Once the suggested corrections are applied, this solution will:
- Use the latest Git tag as the version for the Docker image.
- Ensure that the Docker image is built and tagged with this version.
- Push the versioned image to the repository.
This approach should resolve the issue of users being unable to pull the
onflow/flow-evm-gateway
image from Docker Hub by providing clearly versioned images.To further improve the workflow and ensure it meets all requirements:
- Consider adding a step to validate that a tag exists before proceeding with the build.
- Add error handling for cases where no tags are found.
- Include a step to output the built image details (repository and tag) for easy reference in the workflow summary.
These additional steps will enhance the robustness and usability of the workflow.
🧰 Tools
🪛 actionlint
10-10: context "env" is not allowed here. available contexts are "github", "inputs", "secrets", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details
(expression)
145578f
to
ad3d5af
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- .github/workflows/build.yml (3 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/build.yml
25-25: shellcheck reported issue in this script: SC2086:info:1:73: Double quote to prevent globbing and word splitting
(shellcheck)
🔇 Additional comments (3)
.github/workflows/build.yml (3)
9-9
: LGTM: Environment variable simplificationThe simplification of the
DOCKER_IMAGE_URL
environment variable is a good change. It now correctly uses thevars
context to set the Docker image URL, which aligns with the PR objective of using released tag versions for Docker images. This change also resolves the previous issue of using theenv
context in the globalenv
section.
Line range hint
1-44
: Summary: PR objectives achieved with minor improvements neededThis PR successfully addresses the main objective of using released tag versions to identify newly-created Docker images. The changes in the workflow file improve the versioning process and simplify the Docker image URL, which should resolve the issues mentioned in the linked GitHub issue #489.
Key improvements:
- Simplified
DOCKER_IMAGE_URL
definition.- Added a step to set
GATEWAY_VERSION
based on Git tags.- Updated Docker build and push commands to use the
GATEWAY_VERSION
.These changes should allow users to pull the
onflow/flow-evm-gateway
image from Docker Hub without encountering access problems, as the images will now be properly versioned and pushed to the repository.To fully complete the PR objectives:
- Apply the suggested quoting fix in the "Set Gateway Version" step.
- Run the verification script to ensure consistent usage of
GATEWAY_VERSION
.- Update the Dockerfile (if not already done) to use the
GATEWAY_VERSION
build argument.Once these minor improvements are implemented, this PR will be ready for merge, effectively addressing the versioning issue for Docker images.
🧰 Tools
🪛 actionlint
25-25: shellcheck reported issue in this script: SC2086:info:1:73: Double quote to prevent globbing and word splitting
(shellcheck)
43-44
: Approve: Docker build and push commands updated correctlyThe changes to the Docker build and push commands are well-implemented. They correctly use the
GATEWAY_VERSION
from the step output for both tagging the image and as a build argument. This approach ensures consistency in versioning throughout the build process and aligns with the PR objective of using released tag versions for Docker images.To ensure these changes have the intended effect across the workflow, let's verify the usage of
GATEWAY_VERSION
:This script will help confirm that the
GATEWAY_VERSION
is used consistently in the workflow and that the Dockerfile is prepared to receive and use this build argument.
7701506
to
b679e52
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
.github/workflows/build.yml (1)
23-25
: Approve with minor suggestion: Gateway Version setting stepGreat addition of the "Set Gateway Version" step. This correctly addresses the issue raised in past comments by setting the version in a step rather than in the global env. The use of
git describe
aligns well with the PR objective of using released tag versions for Docker images.To address the static analysis hint and further improve robustness, consider updating the command with proper quoting:
run: echo "GATEWAY_VERSION=$(git describe --tags --abbrev=0 2>/dev/null || echo 'unknown')" >> "$GITHUB_OUTPUT"This change adds quotes around the
$GITHUB_OUTPUT
variable, preventing potential issues with word splitting or globbing.🧰 Tools
🪛 actionlint
25-25: shellcheck reported issue in this script: SC2086:info:1:91: Double quote to prevent globbing and word splitting
(shellcheck)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- .github/workflows/build.yml (3 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/build.yml
25-25: shellcheck reported issue in this script: SC2086:info:1:91: Double quote to prevent globbing and word splitting
(shellcheck)
🔇 Additional comments (3)
.github/workflows/build.yml (3)
9-9
: LGTM: Simplified Docker image URLThe change to
DOCKER_IMAGE_URL
aligns well with the PR objective of using released tag versions for Docker images. By removing the Git SHA and using only theREPO_DOCKER_IMAGE_URL
variable, we've simplified the image URL definition, which should help address the issues mentioned in #489.
Line range hint
1-44
: Summary: Excellent progress on implementing released tag versions for Docker imagesThe changes made in this file significantly contribute to resolving the issues mentioned in #489 and align well with the PR objectives. Here's a summary of the improvements:
- Simplified
DOCKER_IMAGE_URL
definition- Added a robust step to set the
GATEWAY_VERSION
based on Git tags- Updated Docker build and push commands to use the
GATEWAY_VERSION
consistentlyThese changes should address the inability to pull the
onflow/flow-evm-gateway
image from Docker Hub by implementing a clear versioning system based on Git tags. The new approach should make it easier for users to access and manage the correct image versions.Great work on implementing these changes! Once the minor suggestion for quoting in the "Set Gateway Version" step is addressed, this PR will be in excellent shape to merge.
🧰 Tools
🪛 actionlint
25-25: shellcheck reported issue in this script: SC2086:info:1:91: Double quote to prevent globbing and word splitting
(shellcheck)
43-44
: LGTM: Correct usage of GATEWAY_VERSION in Docker commandsThe changes to the Docker build and push commands look great. They correctly use the
GATEWAY_VERSION
from theset_version
step, both for tagging the image and as a build argument. This aligns perfectly with the PR objective of using released tag versions for Docker images and addresses the recommendations from past comments.These modifications should help resolve the issue of pulling the correct image version mentioned in #489, as the image tag now consistently uses the Git tag version.
To ensure the changes are working as expected, you can run the following verification script:
✅ Verification successful
Verification Complete: GATEWAY_VERSION usage in Docker commands confirmed
All Docker build and push commands correctly utilize the
GATEWAY_VERSION
from theset_version
step. This aligns with the PR objective of using released tag versions for Docker images and addresses issue #489.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the Docker image URL and tag format # Test: Check if the Docker image URL and tag format match the expected pattern rg --type yaml 'docker build -t \$\{\{ env\.DOCKER_IMAGE_URL \}\}:\$\{\{ steps\.set_version\.outputs\.GATEWAY_VERSION \}\}' # Test: Ensure the GATEWAY_VERSION is used as a build argument rg --type yaml '--build-arg GATEWAY_VERSION=\$\{\{ steps\.set_version\.outputs\.GATEWAY_VERSION \}\}' # Test: Verify the Docker push command uses the correct tag rg --type yaml 'docker push \$\{\{ env\.DOCKER_IMAGE_URL \}\}:\$\{\{ steps\.set_version\.outputs\.GATEWAY_VERSION \}\}'Length of output: 1079
Script:
#!/bin/bash # Description: Search for Docker build and push commands in build.yml # Search for docker build command with GATEWAY_VERSION rg --fixed-strings 'docker build -t ${{ env.DOCKER_IMAGE_URL }}:${{ steps.set_version.outputs.GATEWAY_VERSION }}' .github/workflows/build.yml # Search for docker push command with GATEWAY_VERSION rg --fixed-strings 'docker push ${{ env.DOCKER_IMAGE_URL }}:${{ steps.set_version.outputs.GATEWAY_VERSION }}' .github/workflows/build.ymlLength of output: 582
802eb0a
to
69a0ced
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
.github/workflows/build.yml (1)
23-25
: LGTM: Gateway Version setting step with minor improvement neededThe addition of the "Set Gateway Version" step is a good improvement. It correctly sets the version based on the latest Git tag and uses GITHUB_OUTPUT for sharing the value between steps.
To address the static analysis hint and prevent potential issues with word splitting or globbing, please update the command with proper quoting:
run: echo "GATEWAY_VERSION=$(git describe --tags --abbrev=0 2>/dev/null || echo 'unknown')" >> $GITHUB_OUTPUTThis change adds quotes around the command substitution, improving the robustness of the step.
🧰 Tools
🪛 actionlint
25-25: shellcheck reported issue in this script: SC2086:info:1:91: Double quote to prevent globbing and word splitting
(shellcheck)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- .github/workflows/build.yml (3 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/build.yml
25-25: shellcheck reported issue in this script: SC2086:info:1:91: Double quote to prevent globbing and word splitting
(shellcheck)
🔇 Additional comments (3)
.github/workflows/build.yml (3)
9-9
: LGTM: Simplified Docker image URLThe change to
DOCKER_IMAGE_URL
aligns well with the PR objective. Using${{ vars.REPO_DOCKER_IMAGE_URL }}
simplifies the configuration and allows for easier management of the Docker image URL across different environments or projects.
43-44
: LGTM: Correct implementation of Gateway Version in Docker commandsThe changes to the Docker build and push commands correctly implement the use of the GATEWAY_VERSION:
- The Docker image is now built with the tag
${{ steps.set_version.outputs.GATEWAY_VERSION }}
.- The GATEWAY_VERSION is passed as a build argument, allowing it to be used within the Dockerfile.
- The Docker push command uses the same version tag.
These changes successfully address the PR objective and the linked issue (#489) by implementing a versioning system for Docker images based on Git tags. This will make it easier for users to pull specific versions of the image and manage dependencies.
Line range hint
9-44
: Summary: Successful implementation of Docker image versioningThe changes in this PR successfully implement the use of released tag versions for Docker images, addressing the objectives outlined in the PR description and the linked issue (#489). Key improvements include:
- Simplified
DOCKER_IMAGE_URL
definition.- Introduction of a "Set Gateway Version" step to capture the latest Git tag.
- Consistent use of the GATEWAY_VERSION in Docker build and push commands.
These changes will resolve the issue of users being unable to pull the
onflow/flow-evm-gateway
image from Docker Hub by providing a clear versioning system based on Git tags.There's a minor suggestion to improve the quoting in the "Set Gateway Version" step, but overall, the implementation is solid and achieves the desired outcome.
Great work on addressing this important issue!
🧰 Tools
🪛 actionlint
25-25: shellcheck reported issue in this script: SC2086:info:1:91: Double quote to prevent globbing and word splitting
(shellcheck)
228a397
to
57f159a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
.github/workflows/build.yml (1)
23-26
: Approve with minor improvement: Gateway Version setting stepThe addition of the "Set Gateway Version" step is an excellent improvement. It correctly addresses the issue raised in past comments by setting the version in a step rather than in the global env. The use of
git describe
to get the latest tag and the fallback to 'unknown' are good practices.However, there's a minor improvement needed to address the static analysis hint:
Please update the command with proper quoting to prevent potential issues with word splitting or globbing:
run: echo "GATEWAY_VERSION=$(git describe --tags --abbrev=0 2>/dev/null || echo 'unknown')" >> "$GITHUB_OUTPUT"This change adds quotes around the
$GITHUB_OUTPUT
variable, improving the robustness of the step.🧰 Tools
🪛 actionlint
25-25: shellcheck reported issue in this script: SC2086:info:1:91: Double quote to prevent globbing and word splitting
(shellcheck)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- .github/workflows/build.yml (3 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/build.yml
25-25: shellcheck reported issue in this script: SC2086:info:1:91: Double quote to prevent globbing and word splitting
(shellcheck)
🔇 Additional comments (3)
.github/workflows/build.yml (3)
9-9
: LGTM: Environment variable updateThe simplification of the
DOCKER_IMAGE_URL
environment variable is a good change. It aligns with the PR objective of using released tag versions instead of commit hashes for Docker images. This change provides more flexibility in tagging the images later in the workflow.
43-44
: LGTM: Docker build and push commands updatedThe changes to the Docker build and push commands are excellent. They correctly implement the use of the
GATEWAY_VERSION
from the step output, both for tagging the image and as a build argument. This ensures consistency between the version used in the build process and the tag applied to the image.These updates fully address the PR objective of using released tag versions for Docker images and resolve the issues mentioned in the past review comments.
Line range hint
1-44
: Summary: Successful implementation of released tag versioning for Docker imagesThe changes in this workflow file successfully implement the use of released tag versions for Docker images, addressing the main objective of the PR. The key improvements include:
- Simplification of the
DOCKER_IMAGE_URL
environment variable.- Addition of a step to set the
GATEWAY_VERSION
based on the latest Git tag.- Consistent use of the
GATEWAY_VERSION
in both Docker build and push commands.These changes should resolve the issue of users being unable to pull the
onflow/flow-evm-gateway
image from Docker Hub, as mentioned in the linked issue #489.To ensure the changes work as expected, let's verify the implementation:
This script will help verify that all the necessary changes are in place and correctly implemented.
✅ Verification successful
✅ Verification Successful: Workflow changes correctly implement Docker image versioning.
The verification script confirms that:
- The workflow is triggered on tag pushes.
- The
DOCKER_IMAGE_URL
environment variable is properly set.- The "Set Gateway Version" step is correctly implemented.
- Docker build and push commands utilize the
GATEWAY_VERSION
as intended.These findings validate that the workflow changes align with the PR objectives and effectively address the issue of Docker image versioning.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the workflow changes for Docker image versioning # Test 1: Check if the workflow is triggered on tag push echo "Test 1: Verifying workflow trigger" grep -n 'on:' .github/workflows/build.yml -A 5 # Test 2: Verify the DOCKER_IMAGE_URL environment variable echo "Test 2: Checking DOCKER_IMAGE_URL" grep -n 'DOCKER_IMAGE_URL:' .github/workflows/build.yml # Test 3: Check the "Set Gateway Version" step echo "Test 3: Verifying Set Gateway Version step" grep -n 'Set Gateway Version' .github/workflows/build.yml -A 3 # Test 4: Verify Docker build and push commands echo "Test 4: Checking Docker build and push commands" grep -n 'docker build' .github/workflows/build.yml -A 2Length of output: 1338
🧰 Tools
🪛 actionlint
25-25: shellcheck reported issue in this script: SC2086:info:1:91: Double quote to prevent globbing and word splitting
(shellcheck)
Closes: #489
Description
The new docker image URL will include the released tag version, instead of a git commit hash, e.g.:
For contributor use:
master
branchFiles changed
in the Github PR explorerSummary by CodeRabbit
Summary by CodeRabbit