chore(ci): Set correct GIT_* on all images
#16705
Conversation
WalkthroughThe pull request introduces updates to the GitHub Actions workflow and the Dockerfile within the monorepo pipeline. Key changes include upgrading action versions for Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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
Documentation and Community
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16705 +/- ##
=======================================
Coverage 36.54% 36.54%
=======================================
Files 6890 6890
Lines 143639 143639
Branches 40925 40925
=======================================
+ Hits 52486 52488 +2
+ Misses 91153 91151 -2
Flags with carried forward coverage won't be shown. Click here to find out more. see 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 18 Total Test Services: 0 Failed, 17 Passed Test ServicesThis report shows up to 10 services
|
|
Affected services are: Deployed services: . |
AndesKrrrrrrrrrrr
added
test everything
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (3)
scripts/ci/Dockerfile (2)
5-7: LGTM! Consider adding documentation for registry variables.The centralization of image registry references improves maintainability and follows the DRY principle.
Consider adding a comment block explaining the purpose and expected format of these registry variables.
+# Registry configuration for base images +# Format: <registry_url>/[<org>/]<image>:<tag> ARG NODE_IMAGE_REGISTRY=${DOCKER_IMAGE_REGISTRY}/docker/library/node:${NODE_IMAGE_TAG} ARG ALPINE_IMAGE_REGISTRY=${DOCKER_IMAGE_REGISTRY}/nginx/nginx:1.21-alpine ARG PLAYWRIGHT_IMAGE_REGISTRY=mcr.microsoft.com/playwright:v${PLAYWRIGHT_VERSION}-focal
43-48: Consider adding validation for Git environment variables.While the centralization of Git metadata is good, these critical values should be validated to prevent empty or invalid values from propagating through the build stages.
Consider adding validation:
FROM ${NODE_IMAGE_REGISTRY} AS output-base ARG GIT_BRANCH GIT_SHA GIT_REPOSITORY_URL +RUN [ -n "$GIT_BRANCH" ] && [ -n "$GIT_SHA" ] && [ -n "$GIT_REPOSITORY_URL" ] || \ + (echo "Error: GIT_BRANCH, GIT_SHA, and GIT_REPOSITORY_URL must be set" && exit 1) ENV GIT_BRANCH=${GIT_BRANCH} GIT_SHA=${GIT_SHA} GIT_REPOSITORY_URL=${GIT_REPOSITORY_URL} ENV DD_GIT_BRANCH=${GIT_BRANCH} DD_GIT_SHA=${GIT_SHA} DD_GIT_REPOSITORY_URL=${GIT_REPOSITORY_URL}.github/workflows/push.yml (1)
573-582: Consider using multi-line YAML syntax for better readability.While the current array-based approach works, consider using YAML's multi-line syntax for better maintainability. Also, the same code block is duplicated in the retry step.
Consider refactoring both blocks to use YAML's multi-line syntax and DRY principle:
env: EXTRA_DOCKER_BUILD_ARGS: >- --build-arg DOCKER_IMAGE_REGISTRY=${DOCKER_BASE_IMAGE_REGISTRY} --build-arg NODE_IMAGE_TAG=${NODE_IMAGE_TAG} --build-arg GIT_BRANCH=${GIT_BRANCH} --build-arg GIT_SHA=${SHA} --build-arg GIT_REPOSITORY_URL=${{ github.repositoryUrl }}Then in both steps just use:
run: | echo "Node image tag is: '${NODE_IMAGE_TAG}'" ./scripts/ci/run-in-parallel.sh "90_${DOCKER_TYPE}"Also applies to: 591-600
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
.github/workflows/push.yml(4 hunks)scripts/ci/Dockerfile(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
scripts/ci/Dockerfile (1)
Pattern scripts/**/*: "Confirm that the code adheres to the following:
- Script efficiency, readability, and maintainability.
- Usage of environmental variables and configuration files for flexibility.
- Integration with NX workspace utilities and commands."
🔇 Additional comments (1)
.github/workflows/push.yml (1)
346-346: LGTM: Action version upgrades look good.
The upgrade of docker/setup-qemu-action and docker/setup-buildx-action from v1 to v3 is a good practice to ensure you're using the latest stable versions with security fixes and improvements.
Also applies to: 555-555
This reverts commit 645e21d.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
scripts/ci/Dockerfile(7 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
scripts/ci/Dockerfile (1)
Pattern scripts/**/*: "Confirm that the code adheres to the following:
- Script efficiency, readability, and maintainability.
- Usage of environmental variables and configuration files for flexibility.
- Integration with NX workspace utilities and commands."
* Sameness for all DD_* and GIT_* ARG and ENV * Reorder ARG and ENVS * Ignore explicit versioning warning * more build args * Update buildx action * Debug script * Move docker args to own prepare step * Add more ARG and ENV to output-* layers * Whitespaces * Revert Dockerfile to main * Revert "Revert Dockerfile to main" This reverts commit 645e21d. * Revert image reference hacks to main * Move ARG and ENV to end of output image * Corrected echoing * Better repository url --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Summary by CodeRabbit
New Features
Bug Fixes
Chores