ci: docker deployment test #18084
ci: docker deployment test #18084
Conversation
WalkthroughThis pull request introduces a new GitHub Action for updating Helm chart files by copying specified files into a Helm values repository and committing the changes. It also adds several new and updated GitHub workflows for deployment and merge-queue processing—including Docker build and artifact upload steps. Additionally, multiple CI scripts have been added or modified to support unique ID generation, tag generation, manifest data handling, and build data output. New development dependencies for GitHub Actions are also incorporated. Changes
Sequence Diagram(s)sequenceDiagram
participant A as Helm-Values Action
participant R as Helm Values Repo
A->>R: Checkout repository (main branch)
loop For each input file
A->>A: Parse file path and determine destination
A->>R: Copy file to designated charts directory
end
A->>R: Stage, commit, and push changes
sequenceDiagram
participant C as CI Runner
participant D as Docker Build
participant P as Post-Build Job
C->>D: Execute docker-build job (checkout, yarn setup, ECR login, build image)
D-->>P: Return build outputs and artifact info
P->>C: Prepare and upload artifact, finalizing the build process
Suggested labels
Suggested reviewers
✨ Finishing Touches
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
|
|
View your CI Pipeline Execution ↗ for commit ea01182.
☁️ Nx Cloud last updated this comment at |
![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
♻️ Duplicate comments (2)
scripts/ci/docker/generate-tag.mjs (2)
24-42: 🛠️ Refactor suggestionClean up commented code and improve error handling.
The function has commented code and error messages that could be more specific.
function getTagname() { if (eventName === 'pull_request' && context.payload.pull_request?.number) { - throw new Error(`Unsupported event: ${eventName}`) - // return `pr-${context.payload.pull_request.number}-${randomTag}`; + throw new Error(`Pull request events are not supported`) } if (eventName === 'merge_group') { const dateString = new Date().toISOString().split('T')[0].replace(/-/g, '') if (typeOfDeployment.dev) { return `main-${dateString}-${randomTag}` } if (typeOfDeployment.prod) { return `release-${dateString}-${randomTag}` } - throw new Error(`Unable to determine artifact name for merge_group event`) + throw new Error(`Unsupported deployment type for merge_group event`) } throw new Error( - `Unable to determine artifact name for event type: ${eventName}`, + `Unsupported event type: ${eventName}`, ) }
95-102: 🛠️ Refactor suggestionUse crypto-secure random string generation.
Replace Math.random() with crypto-secure random generation for better security.
+import crypto from 'crypto'; + function createRandomString(length) { const chars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789' - let result = '' - for (let i = 0; i < length; i++) { - result += chars.charAt(Math.floor(Math.random() * chars.length)) - } - return result + return Array.from(crypto.getRandomValues(new Uint8Array(length))) + .map(x => chars[x % chars.length]) + .join('') }
🧹 Nitpick comments (3)
scripts/ci/docker/write-data.mjs (1)
1-30: Efficient data transformation script that could benefit from minor optimizations.The script effectively processes JSON data from environment variables and outputs a transformed structure, but there are a couple of areas for improvement:
- The performance of the reducer could be optimized
- The output path is hardcoded rather than configurable
Consider these improvements:
// @ts-check import fs from 'fs' import path from 'path' const data = process.env.JSON_DATA ? JSON.parse(process.env.JSON_DATA) : null if (typeof data !== 'object' || data == null) { throw new Error('Invalid data type') } const keys = Object.keys(data.value) const result = keys.map((key) => { const values = Object.keys(data).reduce((a, b) => { - return { - ...a, - [b]: data[b][key], - } + a[b] = data[b][key]; + return a; }, {}) return { id: key, ...values, } }) -const tmpFilePath = path.join('/tmp', 'data.json') +// Use a configurable path with a fallback +const outputDir = process.env.OUTPUT_DIR || '/tmp' +const tmpFilePath = path.join(outputDir, 'data.json') fs.writeFileSync(tmpFilePath, JSON.stringify(result, null, 2))This addresses the spread performance issue flagged by Biome and adds configurability to the output path.
🧰 Tools
🪛 Biome (1.9.4)
[error] 17-17: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
scripts/ci/docker/generate-tag.mjs (2)
64-80: Use consistent equality operators and remove commented code.For better maintainability and code consistency:
- Use strict equality operators consistently
- Remove commented debugging code
function getTypeOfDeployment() { - if (targetBranch === 'main' || targetBranch == 'mq-docker-pre-main') { + if (targetBranch === 'main' || targetBranch === 'mq-docker-pre-main') { return { dev: true, prod: false, } } if (targetBranch.startsWith('release')) { return { dev: false, prod: true, } } - // UNKNOWN BRANCH - // console.error(`Unknown branch: ${targetBranch} - not sure how to tag this deployment`); throw new Error(`Unsupported branch: ${targetBranch}`) }
30-36: Consider adding version information to Docker tags.Including a version component in the Docker tag would provide better traceability and align with Docker tagging best practices.
function getTagname() { if (eventName === 'pull_request' && context.payload.pull_request?.number) { throw new Error(`Unsupported event: ${eventName}`) // return `pr-${context.payload.pull_request.number}-${randomTag}`; } if (eventName === 'merge_group') { const dateString = new Date().toISOString().split('T')[0].replace(/-/g, '') + const version = process.env.VERSION || 'v1' if (typeOfDeployment.dev) { - return `main-${dateString}-${randomTag}` + return `main-${version}-${dateString}-${randomTag}` } if (typeOfDeployment.prod) { - return `release-${dateString}-${randomTag}` + return `release-${version}-${dateString}-${randomTag}` } throw new Error(`Unable to determine artifact name for merge_group event`) }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/workflows/deploy-argocd.yml(1 hunks).github/workflows/install.yml(5 hunks).github/workflows/merge-queue.yml(2 hunks)scripts/ci/docker/create-id.mjs(1 hunks)scripts/ci/docker/generate-tag.mjs(1 hunks)scripts/ci/docker/get-data.mjs(1 hunks)scripts/ci/docker/write-build-data.mjs(1 hunks)scripts/ci/docker/write-data.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- scripts/ci/docker/write-build-data.mjs
- scripts/ci/docker/create-id.mjs
- scripts/ci/docker/get-data.mjs
🧰 Additional context used
📓 Path-based instructions (2)
`scripts/**/*`: "Confirm that the code adheres to the follow...
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."
scripts/ci/docker/generate-tag.mjsscripts/ci/docker/write-data.mjs
`.github/**/*`: "Confirm that the code adheres to the follow...
.github/**/*: "Confirm that the code adheres to the following:
- GitHub Actions workflows use 'arc-runners' as the valid runner for self-hosted ARC runners.
- CI pipelines are efficient, using appropriate caching strategies and minimal resource consumption.
- Reusable workflows and composite actions are properly structured for reusability.
- Dependency management workflows meet security requirements.
- Note: 'runs-on: arc-runners' is valid for our self-hosted runner configuration, despite standard linting warnings."
.github/workflows/merge-queue.yml.github/workflows/install.yml.github/workflows/deploy-argocd.yml
🪛 actionlint (1.7.4)
.github/workflows/merge-queue.yml
59-59: label "arc-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
64-64: property "matrix-output" is not defined in object type {cache-check: {conclusion: string; outcome: string; outputs: {string => string}}; dockerargs: {conclusion: string; outcome: string; outputs: {string => string}}; dockerbuild: {conclusion: string; outcome: string; outputs: {string => string}}; gather: {conclusion: string; outcome: string; outputs: {string => string}}}
(expression)
191-191: label "arc-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
.github/workflows/deploy-argocd.yml
15-15: label "arc-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
🪛 Biome (1.9.4)
scripts/ci/docker/write-data.mjs
[error] 17-17: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
🔇 Additional comments (9)
.github/workflows/deploy-argocd.yml (1)
1-32: Well-structured ArgoCD deployment workflow.This workflow is well-organized with appropriate triggers, concurrency controls, and conditional execution. It effectively uses reusable actions and handles secrets properly.
Note: The static analysis warning about "arc-runners" can be safely ignored as per your coding guidelines, which specify this is a valid runner for your self-hosted ARC configuration.
🧰 Tools
🪛 actionlint (1.7.4)
15-15: label "arc-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
.github/workflows/merge-queue.yml (3)
55-183: New docker-build job looks good, but has a few issues to address.The docker-build job implementation is well-structured with proper matrix handling, but there are a couple of issues to resolve:
- There's a reference to
steps.matrix-output.outputs.jsonin the outputs section, but no step with that ID exists- The Docker build retry logic could be improved (as noted in a previous review)
Fix the outputs reference:
outputs: - json: ${{ steps.matrix-output.outputs.json }} + json: ${{ steps.dockerbuild.outputs.json }}Or add the missing step ID to the appropriate step.
🧰 Tools
🪛 actionlint (1.7.4)
59-59: label "arc-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
64-64: property "matrix-output" is not defined in object type {cache-check: {conclusion: string; outcome: string; outputs: {string => string}}; dockerargs: {conclusion: string; outcome: string; outputs: {string => string}}; dockerbuild: {conclusion: string; outcome: string; outputs: {string => string}}; gather: {conclusion: string; outcome: string; outputs: {string => string}}}
(expression)
163-172: Enhance Docker build retry logic.The retry logic could be more robust by:
- Adding a maximum retry count
- Implementing exponential backoff
- Logging failure reasons
- name: Building Docker images Retry if: steps.gather.outcome == 'success' && steps.dockerbuild.outcome == 'failure' env: DOCKER_BASE_IMAGE_REGISTRY: ${{ env.DOCKER_BASE_IMAGE_REGISTRY }} + MAX_RETRIES: 3 + RETRY_DELAY: 10 run: | set -x - echo "Node image tag is: '${NODE_IMAGE_VERSION}'" - echo "Docker build args are: 'EXTRA_DOCKER_BUILD_ARGS'" - export EXTRA_DOCKER_BUILD_ARGS - ./scripts/ci/run-in-parallel.sh "90_${DOCKER_TYPE}" + attempt=1 + while [ $attempt -le $MAX_RETRIES ]; do + echo "Retry attempt $attempt of $MAX_RETRIES" + if ./scripts/ci/run-in-parallel.sh "90_${DOCKER_TYPE}"; then + echo "Build succeeded on retry attempt $attempt" + exit 0 + fi + echo "Build failed on retry attempt $attempt" + sleep $((RETRY_DELAY * attempt)) + attempt=$((attempt + 1)) + done + echo "Build failed after $MAX_RETRIES attempts" + exit 1
184-217: Post-docker-build job is well implemented.The post-docker-build job is properly structured with appropriate permissions, dependencies, and artifact handling.
The job effectively:
- Retrieves output from the docker-build job
- Processes the data with the write-data.mjs script
- Properly uploads the artifact with appropriate retention settings
🧰 Tools
🪛 actionlint (1.7.4)
191-191: label "arc-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
.github/workflows/install.yml (4)
4-21: Good implementation of merge queue preparation inputs and outputs.The addition of inputs with default values and corresponding outputs are well-structured, making the workflow more flexible and reusable.
73-81: Properly structured output mappings for merge queue outputs.The output mappings correctly reference the step outputs from the prepare-merge-queue step.
104-138: Effective branch handling and merge queue preparation.The implementation properly:
- Extracts the branch name from the input, handling potential refs/heads/ prefix
- Uses the extracted branch name in the nx-set-shas action
- Conditionally runs the merge queue preparation step
211-212: Improved Docker chunks output format.The change to use BUILD_CHUNKS instead of CHUNKS and the improved JSON formatting makes the output more consistent and usable by downstream jobs.
scripts/ci/docker/generate-tag.mjs (1)
1-4: LGTM: Proper script setup with TypeScript checking.The script has proper TypeScript checking and imports necessary modules from the GitHub Actions SDK.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
.github/workflows/merge-queue.yml (3)
70-71: Clarify NODE_IMAGE_VERSION Assignment
There is a placeholder comment# Fix thispreceding the assignment ofNODE_IMAGE_VERSION: ${{ needs.pre-checks.outputs.NODE_IMAGE_VERSION }}. Confirm that this is the correct source for the node image version and, if so, remove the comment to avoid confusion.
157-158: Improve Logging for Docker Build Arguments
In the "Building Docker images" step, the log output prints:echo "Docker build args are: 'EXTRA_DOCKER_BUILD_ARGS'"It appears the literal string is being logged instead of the actual value of
EXTRA_DOCKER_BUILD_ARGS. Consider interpolating the variable (e.g., using${EXTRA_DOCKER_BUILD_ARGS}) to make debugging easier.
162-171: Enhance Docker Build Retry Logic
The "Building Docker images Retry" step re-executes the Docker build command if the initial attempt fails. Consider implementing a more robust retry mechanism with a maximum retry count and exponential backoff. This enhancement can provide better resilience against transient errors.scripts/ci/docker/generate-tag.mjs (2)
1-23: Consider leveraging environment variables for flexibility.The top-level script initialization looks fine, but to better adhere to the coding guidelines for
scripts/**/*, consider configuring event names or targets via environment variables or configuration files for future extensibility. This approach can make it easier to apply changes without modifying code.
64-80: Consider supporting additional branches.The function currently only supports “main”, “mq-docker-pre-main”, and “release” branches, throwing an error otherwise. If the CI/CD process expands to other branches or environments, you may want to handle them gracefully or allow overrides from configuration.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/merge-queue.yml(2 hunks)scripts/ci/docker/generate-tag.mjs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`.github/**/*`: "Confirm that the code adheres to the follow...
.github/**/*: "Confirm that the code adheres to the following:
- GitHub Actions workflows use 'arc-runners' as the valid runner for self-hosted ARC runners.
- CI pipelines are efficient, using appropriate caching strategies and minimal resource consumption.
- Reusable workflows and composite actions are properly structured for reusability.
- Dependency management workflows meet security requirements.
- Note: 'runs-on: arc-runners' is valid for our self-hosted runner configuration, despite standard linting warnings."
.github/workflows/merge-queue.yml
`scripts/**/*`: "Confirm that the code adheres to the follow...
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."
scripts/ci/docker/generate-tag.mjs
🪛 actionlint (1.7.4)
.github/workflows/merge-queue.yml
58-58: label "arc-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
63-63: property "matrix-output" is not defined in object type {cache-check: {conclusion: string; outcome: string; outputs: {string => string}}; dockerargs: {conclusion: string; outcome: string; outputs: {string => string}}; dockerbuild: {conclusion: string; outcome: string; outputs: {string => string}}; gather: {conclusion: string; outcome: string; outputs: {string => string}}}
(expression)
190-190: label "arc-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: prepare / install
🔇 Additional comments (12)
.github/workflows/merge-queue.yml (8)
49-51: Prepare Job Inputs Updated
Thepreparejob now includes inputsrun_merge_queue: trueandmain_branch: ${{ github.base_ref }}. Please verify that these inputs align with downstream expectations in dependent workflows.
58-58: Runner Label Verification for docker-build Job
Thedocker-buildjob usesruns-on: arc-runners. While static analysis tools may flag this as unknown, this is intentional for our self-hosted ARC runners configuration. Please ensure that the internal configuration inactionlint.yaml(or similar) correctly listsarc-runnersas a valid label.🧰 Tools
🪛 actionlint (1.7.4)
58-58: label "arc-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
190-190: Runner Label Verification for post-docker-build Job
Similarly, thepost-docker-buildjob is configured to run onarc-runners. Confirm that this label is correctly defined in our self-hosted runner setup so that the workflow executes as expected.🧰 Tools
🪛 actionlint (1.7.4)
190-190: label "arc-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
143-147: Verify AWS_ECR_REPO_BASE Environment Variable
In the "Check if cached buildx image exists" step, the script uses${{vars.AWS_ECR_REPO_BASE}}. Please ensure that this variable is properly defined in the environment or in a global configuration, as its absence could cause the image pull/tag/push sequence to fail.
183-217: Post-Docker-Build Job Validation
Thepost-docker-buildjob correctly retrieves the output from the Docker build, checks out the code, sets up Yarn, and handles artifact preparation and upload. Everything appears to be configured as intended.🧰 Tools
🪛 actionlint (1.7.4)
190-190: label "arc-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
218-248: Tests Job Configuration is Correct
Thetestsjob is set up to check out the repository, install dependencies, and run unit tests with the defined matrix. The use ofarc-runnersand environment variable propagation appears consistent with our internal configurations.🧰 Tools
🪛 actionlint (1.7.4)
222-222: label "arc-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
249-267: Success Job Dependency and Checks
Thesuccessjob aggregates outcomes fromprepare,tests, andpost-docker-buildand then runs checks to ensure none have failed. Verify that the conditions (e.g., checking if a result equals "failure") are robust and cover all undesired outcomes.🧰 Tools
🪛 actionlint (1.7.4)
250-250: label "arc-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
268-278: Codeowners Check Job Setup
Thecodeowners-checkjob is configured to run a validation script that currently exits with 0. Confirm that this always-passing behavior is intentional for linting CODEOWNERS files, as it might be used purely for informational or legacy reasons.🧰 Tools
🪛 actionlint (1.7.4)
271-271: label "arc-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
scripts/ci/docker/generate-tag.mjs (4)
24-28: Remove or uncomment leftover code.
This block contains commented-out logic for handling pull_request events, which was previously flagged for removal.
44-48: Remove or uncomment leftover code.
Similar to thegetTagnamefunction, this function retains commented-out logic for handling pull_request events.
82-93: Logic looks good.
The branch extraction forpull_requestandmerge_groupappears consistent and clear.
95-102: Use crypto-secure random generation.Using
Math.random()for random tag creation is not cryptographically secure. This issue was previously highlighted; consider replacing it with a crypto-based solution as suggested in earlier reviews.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
.github/workflows/merge-queue.yml (4)
78-91: Error Handling in the Gather Apps Step
The "Gather apps" step setscontinue-on-error: true, which could potentially mask errors in parsing or setting critical environment variables (such asAFFECTED_PROJECTSorDOCKER_TYPE). Consider whether failure in this step should halt the job to avoid cascading issues later in the workflow.
108-137: Secure and Clean Handling of Docker Build Arguments
The "Prepare Docker build arguments" step constructsEXTRA_DOCKER_BUILD_ARGSand writes a temporary file containing theNX_CLOUD_ACCESS_TOKEN. While this approach is functional, it might be beneficial to add cleanup logic to removenx_cloud_access_token.txtafter its use to reduce the risk of sensitive file persistence on the runner.
160-169: Retry Logic Could be Enhanced for Docker Build
The "Building Docker images Retry" step reruns the build on failure but lacks advanced retry features (e.g., maximum retry count or exponential backoff). Enhancing this step with a loop that implements these mechanisms could improve robustness. For example:-./scripts/ci/run-in-parallel.sh "90_${DOCKER_TYPE}" +attempt=1 +MAX_RETRIES=3 +RETRY_DELAY=10 +while [ $attempt -le $MAX_RETRIES ]; do + echo "Retry attempt $attempt of $MAX_RETRIES" + if ./scripts/ci/run-in-parallel.sh "90_${DOCKER_TYPE}"; then + echo "Build succeeded on retry attempt $attempt" + exit 0 + fi + echo "Build failed on retry attempt $attempt" + sleep $((RETRY_DELAY * attempt)) + attempt=$((attempt + 1)) +done +echo "Build failed after $MAX_RETRIES attempts" +exit 1This would introduce robustness to transient Docker build failures.
267-276: Reevaluate the Codeowners-Check Job
Thecodeowners-checkjob currently only runs a simple exit with 0 after a comment suggesting its removal as a required status check. If this job is no longer enforcing any standards, consider either removing it entirely or replacing it with a more meaningful validation.🧰 Tools
🪛 actionlint (1.7.4)
269-269: label "arc-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/merge-queue.yml(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`.github/**/*`: "Confirm that the code adheres to the follow...
.github/**/*: "Confirm that the code adheres to the following:
- GitHub Actions workflows use 'arc-runners' as the valid runner for self-hosted ARC runners.
- CI pipelines are efficient, using appropriate caching strategies and minimal resource consumption.
- Reusable workflows and composite actions are properly structured for reusability.
- Dependency management workflows meet security requirements.
- Note: 'runs-on: arc-runners' is valid for our self-hosted runner configuration, despite standard linting warnings."
.github/workflows/merge-queue.yml
🪛 actionlint (1.7.4)
.github/workflows/merge-queue.yml
56-56: label "arc-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
61-61: property "matrix-output" is not defined in object type {cache-check: {conclusion: string; outcome: string; outputs: {string => string}}; dockerargs: {conclusion: string; outcome: string; outputs: {string => string}}; dockerbuild: {conclusion: string; outcome: string; outputs: {string => string}}; gather: {conclusion: string; outcome: string; outputs: {string => string}}}
(expression)
188-188: label "arc-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: prepare / install
🔇 Additional comments (8)
.github/workflows/merge-queue.yml (8)
47-49: Clarify New Inputs for the Prepare Job
The added inputs (run_merge_queueandmain_branch) in thepreparejob are clear and offer explicit control for merge queue execution. Please double-check that the variablegithub.event.merge_group.base_refis the intended reference for the main branch in your workflow context.
62-69: Verification of NODE_IMAGE_VERSION Source
The environment variableNODE_IMAGE_VERSIONis assigned from${{ needs.pre-checks.outputs.NODE_IMAGE_VERSION }}(with a comment “# Fix this”). Please confirm whether this value should indeed come from thepre-checksjob or if it was intended to be retrieved from thepreparejob (or another source).
138-146: Ensure Variable Definition for Cached Buildx Image Check
In the step "Check if cached buildx image exists", the code uses${{vars.AWS_ECR_REPO_BASE}}to pull, tag, and push an image. Please verify that thevars.AWS_ECR_REPO_BASEvariable is defined in the workflow’s environment or elsewhere in the configuration to prevent runtime errors.
147-159: Standard Docker Build Step Appears Correct
The "Building Docker images" step is configured to run the Docker build via./scripts/ci/run-in-parallel.sh "90_${DOCKER_TYPE}"and usescontinue-on-error: truein preparation for a retry. The logging of the node image tag and build arguments is helpful for debugging.
170-180: Output Step Verification
The "Docker build output" step uses thecloudposse/github-action-matrix-outputs-write@v1action to store several outputs. Please ensure that the environment variables (e.g.,JSON_value,JSON_project, etc.) are properly set prior to this step. Verifying these values will help ensure successful downstream consumption.
181-214: Post-Docker-Build Job for Artifact Handling
The newly introducedpost-docker-buildjob properly depends on both thedocker-buildandpreparejobs and handles artifact preparation and upload. The permissions and steps appear well structured. Please confirm that the uploaded artifact at/tmp/data.jsonis the intended file and that the artifact name from${{ needs.prepare.outputs.MQ_ARTIFACT_NAME}}is correctly generated.🧰 Tools
🪛 actionlint (1.7.4)
188-188: label "arc-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
253-253: Success Job Dependency and Build Check Update
In thesuccessjob, the dependency onpost-docker-buildhas been added and the final "Check build success" step now verifiesneeds.post-docker-build.result. Ensure that thepost-docker-buildjob correctly returns a non-"failure" result in all scenarios. This change aligns the success criteria with the updated build process but please verify consistency.Also applies to: 263-264
56-56: Custom Runner Label:arc-runnersUsage
Multiple jobs are configured to run onarc-runners. Although this is acceptable for your self-hosted ARC runner setup, please ensure that youractionlint.yamlconfiguration includesarc-runnersin its list of valid labels to avoid false warnings from static analysis tools.Also applies to: 188-188, 269-269
🧰 Tools
🪛 actionlint (1.7.4)
56-56: label "arc-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
scripts/ci/docker/generate-tag.mjs (2)
1-4: Consider externalizing configuration and verifying NX usage.
These lines do not rely on environment variables or configuration files for flexibility, nor do they show any direct integration with NX workspace utilities or commands. If the script will be used in diverse environments or pipelines, consider externalizing these settings and verifying whether integration with NX commands is expected.
64-80: Allow flexible branch mappings.
Hardcoding special branches such asmq-docker-pre-mainand therelease/*pattern could limit maintainability. Consider reading these from configuration or environment variables so that you can adapt the script to new workflows without changing code.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/ci/docker/generate-tag.mjs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`scripts/**/*`: "Confirm that the code adheres to the follow...
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."
scripts/ci/docker/generate-tag.mjs
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: prepare / install
🔇 Additional comments (5)
scripts/ci/docker/generate-tag.mjs (5)
5-23: Initialization logic looks good.
The approach to retrieve GitHub Action context and define initial variables is straightforward and readable. No further concerns here.
24-42: Remove commented-out return statement.
The commented-out code in lines 27–28 was previously flagged in past reviews. Removing or converting it to a conditional logic block would maintain clarity and consistency.
44-62: Eliminate commented-out return statement.
Similar to thegetTagnamefunction, the commented-out code in lines 47–48 should be fully removed to maintain clarity and avoid confusion for future maintainers.
82-93: Target branch resolution logic is well-structured.
The checks for pull request and merge group contexts are clear, and the error message is sufficiently descriptive.
95-102: Adopt crypto-secure random string generation.
Relying onMath.random()for randomness is non-cryptographic and was previously flagged in older commits. Replacing it with a cryptographically secure approach (e.g., Node’scryptomodule) would enhance security.
* ci: docker deployment test (#18084) * chore: test * fix: test * fix: test * chore: test 2 * fix: better generate tag script * chore: test build * fix: test 2 * fix: add prepare * fix: oops * fix: wtf * fix: faster * fix: forgot * fix: ok * fix: ok * fix: wtf * fix: ok * fix: ok * fix: i hope * fix: debug * fix: ok * fix: debug * fix: wtf * fix: ok * fix: oops * fix: texst 2 * fix: debug * fix: oops * fix: export variable * fix: docker chunks * fix: chunks * fix: oops * fix: oh god * fix: oops * fix: add git branch * fix: add sha * feat: write build data * fix: upload artifact * fix: typo * fix: ok * fix: test * fix: ok * fix: use cloudposse for matrix output * feat: upload artifact * fix: write data * fix: permission * fix: test * fix: upload * fix: ok * fix: ok * fix: deploy * fix: oops * fix: okok * fix: ok * fix: ok * fix: ok * fix: ok * fix: ok * fix: fix * fix: 2 * fix: ok hehe * fix: ok changes * fix: ok * fix: ok * fix: test * fix: ok * fix: ok * fix: ok * fix: hoho * Feature/update helm values (#18100) * feat(ci): Commit helm charts to helm-values repository * feat(ci): Changing the steps order of the implementation * feat(ci): Implementing logic * feat(ci): Change implementation to use composite action --------- Co-authored-by: Ingvar Sigurðsson <[email protected]> * fix: fix fix * fix: ok * fix: ok * fix: sorry * fix: ok * fix: update manifest with bs * fix: changed files * fix: copy * fix: create directory if it does not exist * fix: test * fix: sha * fix: target * fix: ok * fix: ok * fix: ok * fix: ok * fix: test * fix: sha --------- Co-authored-by: Ingvar Sigurðsson <[email protected]> * fix: cleanup * fix: apply on mq (#18113) fiX: ok * feat: fix tag (#18118) fix: debug msg * fix: small fixes (#18127) * fix: remove test branch * fix: move yarn install * feat: get ready for release (#18136) * fix: ok * fix: ignore * fix: ok * fix: get sha * fix: ok * fix: ok * fix: ok * fix: error * fix: skip run * fix: ok * fix: merge * fix: nothing should happen (#18149) * chore: addding ssh key (#18145) * addding ssh key * fix: passing the key * fix: missed a spot * fix: test * fix: remove * fix: ok * fix: set secret * fix: test * fix: ok * fix: ok * fix: install 2 * fix: remove --------- Co-authored-by: lommi <[email protected]> * fix: update path using helm values * fix: mq-docker * chore: test prod and staging (#18160) * fix: mq-docker-main * fix: ugly hack remove this before going to main * fix: mq docker pre main * fix: mq workflow (#18162) * fix: format * fix: ok --------- Co-authored-by: Ingvar Sigurðsson <[email protected]> Co-authored-by: Róberta Andersen <[email protected]>
Summary by CodeRabbit
New Features
Chores