ci: deploying to argocd #18111
ci: deploying to argocd #18111
Conversation
* 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]>
WalkthroughThis pull request introduces a new GitHub Action for updating helm chart files by copying them to a helm-values repository. It enhances workflow files with new input parameters and outputs for merge queue management, and adds a new input for handling pull requests. Additionally, several CI Docker scripts and configuration files are introduced to support Docker tag generation, unique ID creation, data processing, and artifact uploads. Support libraries for GitHub Actions are also added as dependencies. 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
|
lodmfjord
changed the title
|
View your CI Pipeline Execution ↗ for commit 7951c65.
☁️ 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: 6
🧹 Nitpick comments (17)
scripts/ci/docker/write-data.mjs (3)
14-26: Optimize the object creation in the reduce function.Using spread syntax inside a reducer accumulator results in O(n²) time complexity as it creates a new object on each iteration.
-const result = keys.map((key) => { - const values = Object.keys(data).reduce((a, b) => { - return { - ...a, - [b]: data[b][key], - } - }, {}) - - return { - id: key, - ...values, - } -}) +const result = keys.map((key) => { + const values = Object.keys(data).reduce((acc, dataKey) => { + acc[dataKey] = data[dataKey][key]; + return acc; + }, {}); + + return { + id: key, + ...values, + } +})🧰 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)
28-29: Improve error handling and consider OS portability for file paths.The script uses a hardcoded
/tmpdirectory which may not exist on all operating systems (particularly Windows) and lacks error handling for file operations.-const tmpFilePath = path.join('/tmp', 'data.json') -fs.writeFileSync(tmpFilePath, JSON.stringify(result, null, 2)) +// Use OS temp directory for better cross-platform support +const tmpFilePath = path.join(require('os').tmpdir(), 'data.json') +try { + fs.writeFileSync(tmpFilePath, JSON.stringify(result, null, 2)) + console.log(`Data successfully written to ${tmpFilePath}`) +} catch (error) { + console.error(`Failed to write data to ${tmpFilePath}:`, error) + process.exit(1) +}
1-11: Add descriptive comments and strengthen validation.Adding descriptive comments helps maintain the code, and providing more detailed validation improves error handling.
// @ts-check +/** + * Script to process JSON data from environment variable and write to a file. + * Expects JSON_DATA environment variable containing an object with a 'value' property. + */ 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') +if (typeof data !== 'object' || data === null) { + throw new Error('Invalid data type: JSON_DATA must be a valid JSON object') +} + +if (!data.value || typeof data.value !== 'object') { + throw new Error('Invalid data structure: Missing or invalid "value" property') }scripts/ci/docker/write-build-data.mjs (1)
33-34: Fix error message inconsistencyThe error message refers to "TAG" while checking the "IMAGE_TAG" environment variable, which could be confusing.
- console.error('Error: TAG is undefined') + console.error('Error: IMAGE_TAG is undefined').github/actions/update-helm-values/action.yml (3)
2-2: Fix typo in descriptionThere's a typo in the action description.
-description: 'Copys the changeset in charts folder to helm-values repository' +description: 'Copies the changeset in charts folder to helm-values repository'
30-30: Remove debugging echo statementThe debug echo statement should be removed in production code.
- echo "$files\n$paths" + # Removed debug echo
41-48: Add error handling for git operationsThe current implementation doesn't handle potential git errors or conflicts that might occur during commit and push operations.
cd helm-values git config --global user.email "[email protected]" git config --global user.name "CI Bot" git add . - git commit -m "Updated helm charts" + if git diff --staged --quiet; then + echo "No changes to commit" + else + git commit -m "Updated helm charts" + echo "Showing changeset\n $(git show)" + git push || { echo "Failed to push changes"; exit 1; } + fi - echo "Showing changeset\n $(git show)" - git push.github/workflows/deploy-argocd.yml (1)
25-30: Consider adding error handling for manifest updateThe workflow could benefit from additional error handling for the update-helm-values action to capture and report any failures.
- name: Update helm-values repository uses: ./.github/actions/update-helm-values if: ${{ steps.manifest.outputs.MQ_HAS_OUTPUT == 'true' }} with: files: ${{ steps.manifest.outputs.MQ_CHANGED_FILES }} gh_token_pat: ${{ secrets.GH_HELM_VALUES_PAT }} + id: update_helm + - name: Check update results + if: ${{ steps.manifest.outputs.MQ_HAS_OUTPUT == 'true' && failure() }} + run: | + echo "Failed to update helm-values repository" + exit 1scripts/ci/docker/get-data.mjs (2)
28-59: Ensure robust checks for YAML image fields.
Currently, the script assumes each YAML has a validimageblock with arepository. You could enhance reliability by gracefully skipping or warning if these fields are missing, preventing unexpected exceptions when parsing.
61-123: Add defensive fetch error checks.
Thedownload()function assumes successful responses from GitHub. Consider adding try/catch blocks or status code checks before parsing JSON and before writing to disk. This would provide clearer error reporting for network or permission issues..github/workflows/install.yml (2)
104-110: Quote variables to avoid shell expansion issues.
ShellCheck SC2086 suggests double-quoting variables like$RAW_REFto prevent unintended word splitting or glob expansion. Example fix:- echo "MAIN_BRANCH=${RAW_REF#refs/heads/}" >> $GITHUB_OUTPUT + echo "MAIN_BRANCH=${RAW_REF#refs/heads/}" >> "$GITHUB_OUTPUT"🧰 Tools
🪛 actionlint (1.7.4)
107-107: shellcheck reported issue in this script: SC2086:info:1:46: Double quote to prevent globbing and word splitting
(shellcheck)
211-212: Be mindful of string quoting in shell expansions.
The code is functional, but you could quote$CHUNKSto avoid accidental shell interpretation. Currently, it’s not a critical issue; just a general precaution.scripts/ci/docker/generate-tag.mjs (5)
5-13: Initialize random tag only when needed.The random tag is created immediately at the script's start, but it's only used if certain conditions are met in
getTagname(). Consider moving the random tag generation inside the relevant functions to improve efficiency and clarity.Also, the target branch, deployment type, artifact name, and tag name are all determined without error handling around the functions that could throw exceptions. This could lead to unclear error messages.
-const randomTag = createRandomString(16) const context = github.context const eventName = context.eventName const sha = context.payload.pull_request?.head.sha || context.sha -const targetBranch = getTargetBranch() -const typeOfDeployment = getTypeOfDeployment() -const artifactName = getArtifactname() -const tagName = getTagname() +try { + const targetBranch = getTargetBranch() + const typeOfDeployment = getTypeOfDeployment() + const artifactName = getArtifactname() + const tagName = getTagname()
15-22: Add try-catch around core.setOutput calls.The output setting lacks error handling, which could cause the script to fail without clear error messages. Consider wrapping these calls in a try-catch block.
+try { core.setOutput('ARTIFACT_NAME', artifactName) core.setOutput('DOCKER_TAG', tagName) core.setOutput('GIT_BRANCH', targetBranch) core.setOutput('GIT_SHA', sha) console.info(`Artifact name: ${artifactName}`) console.info(`Docker tag: ${tagName}`) console.info(`Git branch: ${targetBranch}`) console.info(`Git SHA: ${sha}`) +} catch (error) { + core.setFailed(`Failed to set outputs: ${error.message}`) +}
64-80: Remove commented code and add more descriptive error handling.The function contains a commented line that should be removed. Additionally, consider adding more descriptive error messages to help with troubleshooting.
function getTypeOfDeployment() { if (targetBranch === '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}`) + throw new Error(`Unsupported branch: ${targetBranch}. Only 'main' and branches starting with 'release' are supported.`) }
82-93: Improve consistency in error messages and event handling.The error message should follow a consistent structure with the other functions. Consider standardizing the error message format across all functions.
function getTargetBranch() { if (eventName === 'pull_request' && context.payload?.pull_request?.base.ref) { return context.payload.pull_request.base.ref } if (eventName === 'merge_group') { return context.payload.merge_group.base_ref.replace('refs/heads/', '') } throw new Error( - `Unable to determine target branch for event type: ${eventName}`, + `Unsupported event type: ${eventName}. Cannot determine target branch.`, ) }
95-102: Use more secure random generation.For generating random strings that might be used in security-sensitive contexts, consider using a more cryptographically secure method like
crypto.randomBytes()instead of Math.random().+import crypto from 'crypto' + function createRandomString(length) { + // Cryptographically secure random string generation + return crypto + .randomBytes(Math.ceil(length / 2)) + .toString('hex') + .slice(0, length) +} -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 -}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (12)
.github/actions/update-helm-values/action.yml(1 hunks).github/workflows/deploy-argocd.yml(1 hunks).github/workflows/install.yml(5 hunks).github/workflows/merge-queue.yml(2 hunks).github/workflows/pullrequest.yml(1 hunks)package.json(1 hunks)scripts/ci/_docker.sh(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)
🧰 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/write-data.mjsscripts/ci/docker/write-build-data.mjsscripts/ci/docker/create-id.mjsscripts/ci/_docker.shscripts/ci/docker/get-data.mjsscripts/ci/docker/generate-tag.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/pullrequest.yml.github/actions/update-helm-values/action.yml.github/workflows/merge-queue.yml.github/workflows/deploy-argocd.yml.github/workflows/install.yml
🪛 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)
🪛 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)
81-81: shellcheck reported issue in this script: SC2129:style:6:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
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)
.github/workflows/deploy-argocd.yml
14-14: 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
107-107: shellcheck reported issue in this script: SC2086:info:1:46: Double quote to prevent globbing and word splitting
(shellcheck)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: prepare / install
🔇 Additional comments (16)
.github/workflows/pullrequest.yml (1)
50-51: LGTM! Good enhancement to pass the base branch reference.This change appropriately passes the base branch of the pull request to the install workflow, which can be used for accurate dependency calculations or affected file detection.
package.json (1)
318-320: LGTM! Appropriate GitHub Actions SDK dependencies added.These dependencies are necessary to support the CI/CD enhancements mentioned in the PR objectives. The @actions/artifact, @actions/core, and @actions/github packages provide essential functionality for building and extending GitHub Actions workflows.
scripts/ci/_docker.sh (2)
19-19: LGTM! Good use of environment variable with sensible default.Setting UPLOAD_ARTIFACT_DOCKER to false by default is a safe choice, allowing explicit opt-in to the artifact upload functionality.
85-85: LGTM! Appropriate placement of the upload function call.Calling _upload_artifact at the end of the script ensures it runs after the main container build process is complete.
scripts/ci/docker/create-id.mjs (1)
1-20: Clean and efficient implementationThe script correctly generates a unique ID for matrix jobs and exports the necessary variables. The error handling for the required environment variable is implemented well.
.github/workflows/deploy-argocd.yml (1)
14-14: Valid use of self-hosted runnerThe use of 'arc-runners' is correct according to the project's coding guidelines for self-hosted ARC runners.
🧰 Tools
🪛 actionlint (1.7.4)
14-14: 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 (4)
47-49: Verify usage ofmerge_group.base_ref.
Ifmerge_groupis unavailable in certain events, the workflow may fail to resolve the base branch correctly. Consider adding a fallback for scenarios wheremerge_groupmight not be set.
52-181: Consider strengthening error handling in thedocker-buildjob.
There are multiple places wherecontinue-on-error: trueis specified (e.g., line 91, 148), which can mask legitimate failures. If the build steps are critical to pipeline success, you might remove or narrow the scope ofcontinue-on-errorto ensure you don’t miss real errors. Also verify that the environment variablevars.AWS_ECR_REPO_BASEis properly defined in your repository or repository settings.🧰 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)
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)
81-81: shellcheck reported issue in this script: SC2129:style:6:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
182-214: Artifact preparation step looks solid.
The step for artifact reading and upload seems well orchestrated. Just a minor note: ensure any unusual artifacts are handled gracefully if you expand usage later. Overall, this flow is clear.🧰 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-264: Success job constraint is appropriate.
Neatly verifying thepost-docker-buildjob did not fail prevents partial success from proceeding. The logic is correct and straightforward.scripts/ci/docker/get-data.mjs (2)
1-26: Check for fallbacks when reading environment variables.
The script depends onGITHUB_TOKENand other environment variables. If they are missing or insufficiently scoped,fetch(url)calls will fail. It might be helpful to handle those scenarios gracefully or provide fallback logic for local testing.
125-163: Handle unsupported events more flexibly.
getBranch()throws an error ifeventNameisn’tpush, andgetArtifactname()handles onlydev/prodbranches. If you anticipate more event or branch types, consider future-proofing these functions with additional fallback logic..github/workflows/install.yml (4)
4-13: Validate themain_branchinput.
When referencing PR base branches other thanmain, confirm that the default and user-provided values align with real branch names.
14-21: Check output usage for merge queue.
Ifrun_merge_queueis false, ensure the steps that produceMQ_SHA,MQ_DOCKER_TAG, etc., still resolve to valid or empty strings. Otherwise, downstream jobs reading these outputs might fail unexpectedly.
73-76: Validate “MQ” and “DOCKER_CHUNKS” outputs.
If thedocker_projectsstep does not detect any Docker chunks, these outputs will be empty. Ensure this use case is acceptable or conditionally handled in downstream steps.Also applies to: 81-81
133-137: Optional queue preparation is a nice approach.
Conditionally running the merge queue step is flexible. Ensure that the required scripts and environment references exist even when the step is skipped.
fiX: ok
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
.github/workflows/deploy-argocd.yml (1)
1-27: 🛠️ Refactor suggestionAdd concurrency control to prevent race conditions
The workflow lacks concurrency settings, which is important for CI/CD pipelines that modify shared resources. Without concurrency control, simultaneous pushes to main could lead to race conditions when updating the ArgoCD repository.
name: Upload manifest to argocd repo on: push: branches: - main +concurrency: + group: argocd-${{ github.ref }} + cancel-in-progress: false jobs: upload: runs-on: arc-runners + timeout-minutes: 10🧰 Tools
🪛 actionlint (1.7.4)
10-10: 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)
🧹 Nitpick comments (4)
.github/workflows/deploy-argocd.yml (1)
22-26: Consider adding error handling for helm-values repository updateThe workflow doesn't account for potential failures in the helm-values update step. Consider adding error handling or notification mechanisms for when the update fails.
scripts/ci/docker/get-data.mjs (3)
29-33: Use environment variables for path configurationThe manifest paths are hardcoded, which reduces flexibility. Consider using environment variables or configuration files to make these paths configurable.
// Read all manifest files -const _MANIFEST_PATHS = [ - 'charts/islandis-services', - 'charts/judicial-system-services', - 'charts/identity-server-services', -] +const _MANIFEST_PATHS = (process.env.MANIFEST_PATHS || 'charts/islandis-services,charts/judicial-system-services,charts/identity-server-services').split(',')
19-26: Simplify STAGE_NAME assignment logicThe current ternary chain for STAGE_NAME is difficult to read. Consider using a more straightforward approach for readability and maintainability.
-const STAGE_NAME = typeOfDeployment.dev - ? 'dev' - : typeOfDeployment.staging - ? 'staging' - : typeOfDeployment.prod - ? 'prod' - : 'dev' +const STAGE_NAME = (() => { + if (typeOfDeployment.dev) return 'dev' + if (typeOfDeployment.staging) return 'staging' + if (typeOfDeployment.prod) return 'prod' + return 'dev' // default +})()
1-8: Add documentation for script purpose and usageThe script lacks documentation explaining its purpose, inputs, and outputs. Consider adding JSDoc comments to improve maintainability.
// @ts-check +/** + * This script updates ArgoCD manifest files with image tags from build artifacts. + * It reads YAML manifests, finds image repositories, and updates them with tags + * from the data.json file created in the build process. + * + * Environment variables: + * - GITHUB_TOKEN: GitHub token for API access + * + * Outputs: + * - MQ_HAS_OUTPUT: Whether any files were changed + * - MQ_CHANGED_FILES: Comma-separated list of changed files + */ import fs, { readFileSync } from 'node:fs' import jsyaml from 'js-yaml'
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/deploy-argocd.yml(1 hunks)scripts/ci/docker/generate-tag.mjs(1 hunks)scripts/ci/docker/get-data.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/ci/docker/generate-tag.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/get-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/deploy-argocd.yml
🪛 actionlint (1.7.4)
.github/workflows/deploy-argocd.yml
10-10: 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 (10)
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
🔇 Additional comments (1)
.github/workflows/deploy-argocd.yml (1)
10-10: Runner configuration is validThe use of
arc-runnersis valid according to the coding guidelines for self-hosted ARC runners, despite the standard linting warning.🧰 Tools
🪛 actionlint (1.7.4)
10-10: 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)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
.github/workflows/merge-queue.yml (4)
47-50: New Input Parameters in the Prepare Job
The prepare job now passes additional inputs (run_merge_queue: trueandmain_branch) to the reusable workflow at./.github/workflows/install.yml. Ensure that these parameters are documented and that the downstream workflow properly handles these inputs since they affect behavior in the merge queue.
108-137: Docker Build Arguments and Secret Handling
The “Prepare Docker build arguments” step constructs a list of build arguments and writes them into the GitHub environment file. While the approach is functional, consider consolidating the multiple appends to${GITHUB_ENV}into a single update for clarity and to reduce potential quoting issues. Also, verify that writing the NX_CLOUD_ACCESS_TOKEN to a temporary file (and referencing it later as a secret) does not inadvertently expose the value in logs.
160-169: Building Docker Images Retry Step
The retry step for building Docker images mirrors the primary build step, ensuring that transient failures are retried. Confirm that the conditions under which the retry is triggered have been thoroughly tested. The duplication in steps is acceptable here for robustness, but adding a comment to clarify the rationale might improve maintainability.
218-218: Remove Trailing Spaces
Trailing spaces were detected on line 218. For better readability and to avoid linting issues, please remove any trailing whitespace.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 218-218: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/merge-queue.yml(3 hunks)scripts/ci/docker/generate-tag.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/ci/docker/generate-tag.mjs
🧰 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)
🪛 YAMLlint (1.35.1)
.github/workflows/merge-queue.yml
[error] 218-218: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
🔇 Additional comments (6)
.github/workflows/merge-queue.yml (6)
78-91: Usage ofcontinue-on-errorin the Gather Apps Step
The “Gather apps” step usescontinue-on-error: true, which means failures in gathering application data will not halt the job. Confirm that this behavior is intentional. If an error in this step could cause downstream jobs to act on incorrect or missing data, consider handling the error more explicitly.
138-146: Ensuring Definition of AWS_ECR_REPO_BASE
The step "Check if cached buildx image exists" uses the variable${{vars.AWS_ECR_REPO_BASE}}for Docker commands. Please confirm thatAWS_ECR_REPO_BASEis defined in the appropriate context so that the pull, tag, and push commands function correctly. Undeclared or incorrect variable values may lead to a failure in pulling the image from the custom ECR endpoint.
147-159: Building Docker Images with Retry Mechanism
The primary Docker image build step (and its retry step) use the script./scripts/ci/run-in-parallel.shwith appropriate environment setup. The logging statements (echoing the Node image tag and Docker build arguments) are useful for debugging. Verify that these logs do not expose sensitive data and that the retry conditions sufficiently cover transient build failures.
181-227: Post-Docker-Build Job and Artifact Handling
The newly addedpost-docker-buildjob correctly depends on bothdocker-buildandprepare. It retrieves build outputs, prepares artifacts, and even updates the helm-values repository as needed. This job is well-structured; however, ensure that the artifact paths (such as/tmp/data.json) are correct in your environment.🧰 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)
🪛 YAMLlint (1.35.1)
[error] 218-218: trailing spaces
(trailing-spaces)
56-56: Custom Runner Label Verification in Docker-Build Job
The docker-build job specifiesruns-on: arc-runnerson line 56. If “arc-runners” is a custom self-hosted runner label, ensure that it is appropriately configured in youractionlint.yaml(or similar configuration) so that CI integrations and static analysis tools recognize it.🧰 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)
188-188: Custom Runner Label Verification in Post-Docker-Build Job
Similarly, on line 188 the post-docker-build job usesruns-on: arc-runners. Confirm that this custom label is defined for your self-hosted runners in the configuration to avoid warnings from static analysis tools.🧰 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)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
.github/workflows/merge-queue.yml (1)
61-63:⚠️ Potential issueFix undefined step reference in outputs
The outputs block references
steps.matrix-output.outputs.json, but no step with IDmatrix-outputis defined in the job. This will cause runtime errors when trying to retrieve this output.Fix this by referencing the correct step ID or adding the missing step:
outputs: - json: ${{ steps.matrix-output.outputs.json }} + json: ${{ steps.dockerbuild.outputs.json }}Alternatively, add a step with ID
matrix-outputthat produces the required JSON output.🧰 Tools
🪛 actionlint (1.7.4)
62-62: 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)
scripts/ci/docker/get-data.mjs (2)
93-98: 🛠️ Refactor suggestionFix getBranch function to handle push events
The
getBranchfunction only handles 'merge_group' events and throws an error for all other events. However, this function might be called during push events as well, which would cause failures.function getBranch() { if (context.eventName === 'merge_group') { return context.payload.merge_group.base_ref.replace('refs/heads/', '') + } else if (context.eventName === 'push') { + return context.ref.replace('refs/heads/', '') } throw new Error(`Unsupported event: ${context.eventName}`) }
122-131: 🛠️ Refactor suggestionHandle all deployment types in getArtifactname
The
getArtifactnamefunction doesn't handle the staging case even though thegetTypeOfDeploymentfunction defines it. This could lead to unexpected errors for staging deployments.function getArtifactname() { if (typeOfDeployment.dev) { return `main-${sha}` } if (typeOfDeployment.prod) { return `release-${sha}` } + if (typeOfDeployment.staging) { + return `staging-${sha}` + } - throw new Error(`Unsupported`) + throw new Error(`Unsupported deployment type: ${JSON.stringify(typeOfDeployment)}`) }
🧹 Nitpick comments (5)
.github/workflows/merge-queue.yml (1)
182-232: Good implementation of post-docker-build jobThe post-docker-build job properly collects build outputs, checks test success, and manages artifacts. The integration with helm-values repository updates is a good approach for GitOps deployments.
Remove the trailing space on line 223:
- +🧰 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)
🪛 YAMLlint (1.35.1)
[error] 223-223: trailing spaces
(trailing-spaces)
scripts/ci/docker/generate-tag.mjs (4)
9-9: Ensure proper handling of SHA from different eventsThe code relies on optional chaining for
pull_request?.head.shabut doesn't handle all possible event structures that might use this script.-const sha = context.payload.pull_request?.head.sha || context.sha +const sha = context.payload.pull_request?.head.sha || + (context.eventName === 'merge_group' ? context.payload.merge_group.head_sha : context.sha)
55-72: Clean up commented code in getArtifactnameThe function contains commented-out code that should be removed for clarity.
function getArtifactname() { if (eventName === 'pull_request' && context.payload.pull_request?.number) { throw new Error(`Unsupported event: ${eventName}`) - // return `pr-${context.payload.pull_request.number}`; } if (eventName === 'merge_group') { // Rest of the function...
88-90: Remove commented code in getTypeOfDeploymentRemove the commented-out console.error line for cleaner code.
// UNKNOWN BRANCH - // console.error(`Unknown branch: ${targetBranch} - not sure how to tag this deployment`); throw new Error(`Unsupported branch: ${targetBranch}`)
1-114: Add comprehensive module documentationThe script lacks overall documentation explaining its purpose, usage, and expected inputs/outputs.
Consider adding JSDoc comments at the top of the file:
// @ts-check +/** + * @fileoverview Generates Docker tags and artifact names based on GitHub Actions context. + * This script is intended to be used in GitHub Actions workflows for Docker deployments. + * + * @example + * // Usage in GitHub Actions workflow: + * // - name: Generate Docker tag + * // id: generate-tag + * // run: node scripts/ci/docker/generate-tag.mjs + * // + * // Outputs: + * // - ARTIFACT_NAME: The name of the artifact to be uploaded + * // - DOCKER_TAG: The tag to be used for Docker images + * // - GIT_BRANCH: The Git branch being built + * // - GIT_SHA: The Git SHA being built + * // - SHOULD_RUN_BUILD: Whether the build should run based on the event type + */ import github from '@actions/github'
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/install.yml(5 hunks).github/workflows/merge-queue.yml(3 hunks)scripts/ci/docker/const.mjs(1 hunks)scripts/ci/docker/generate-tag.mjs(1 hunks)scripts/ci/docker/get-data.mjs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- scripts/ci/docker/const.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/get-data.mjsscripts/ci/docker/generate-tag.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
🪛 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)
62-62: 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)
🪛 YAMLlint (1.35.1)
.github/workflows/merge-queue.yml
[error] 223-223: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: prepare / install
🔇 Additional comments (11)
.github/workflows/merge-queue.yml (3)
47-49: Good addition of merge queue configurationThe parameters added to the install workflow provide proper configuration for the merge queue process, using the base reference from the merge group event to determine the main branch.
52-181: Well-structured Docker build job implementationThe new docker-build job correctly implements a matrix strategy for building Docker images with appropriate caching, login, and build argument handling. It includes proper error handling with retry logic for failed builds.
🧰 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)
62-62: 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)
271-282: Updated success check is appropriateThe success job correctly checks the status of the new post-docker-build job, ensuring the workflow validates the results of all required steps before reporting success.
scripts/ci/docker/get-data.mjs (2)
64-68: Improve error handling for missing data fileThe script silently exits when the data file is missing. Consider adding appropriate logging and setting GitHub outputs to indicate the script ran but found no data to process.
async function parseData() { const fileName = `/tmp/data.json` if (!fs.existsSync(fileName)) { + console.log(`Data file not found at ${fileName}, no changes needed`) + core.setOutput(_KEY_HAS_OUTPUT, 'false') + core.setOutput(_KEY_CHANGED_FILES, '') process.exit(0) }
1-61: Good implementation of manifest parsing and image extractionThe script effectively reads YAML manifest files, extracts image repository information, and organizes it for later processing. The constants and environment variables are well-structured.
.github/workflows/install.yml (5)
4-23: Good implementation of merge queue configurationThe new input parameters and outputs enhance workflow reusability by making it configurable for different execution contexts. The merge queue outputs are properly structured to provide necessary information for downstream jobs.
75-84: Well-defined job outputs for merge queue integrationThe job outputs are properly updated to include merge queue-specific values from the new
prepare-merge-queuestep, and theDOCKER_CHUNKSsource is correctly updated to use the more specificBUILD_CHUNKSoutput.
107-113: Efficient branch name extractionThe
get-branchstep properly extracts the branch name from the input parameter, removing anyrefs/heads/prefix. This ensures consistent branch naming across the workflow.
136-141: Good conditional execution of merge queue preparationThe
prepare-merge-queuestep is properly conditioned on therun_merge_queueinput, ensuring it only runs when needed. This helps optimize workflow execution.
214-215: Improved Docker chunks output formattingThe update to use
jqfor formatting theBUILD_CHUNKSoutput ensures that the chunks are properly formatted for consumption by downstream jobs.scripts/ci/docker/generate-tag.mjs (1)
35-52: Clean up commented code and fix error message in getTagnameThe function contains commented-out code and uses an error message that refers to "artifact name" instead of "tag name".
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, '') 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(`Unable to determine tag name for merge_group event`) } throw new Error( - `Unable to determine artifact name for event type: ${eventName}`, + `Unable to determine tag name for event type: ${eventName}`, ) }
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.github/workflows/merge-queue.yml (2)
69-70: [Verify] NODE_IMAGE_VERSION Source Clarification
There is a# Fix thiscomment above theNODE_IMAGE_VERSIONassignment (line 70). Please verify that using${{ needs.pre-checks.outputs.NODE_IMAGE_VERSION }}is the intended source. The pre‑checks workflow appears to define this variable in multiple ways, so double-check that this reference is unambiguous and correct.
225-227: [Nitpick] Remove Trailing Whitespace
YAMLlint reported trailing spaces around line 227. Please remove any extraneous trailing whitespace to comply with YAML best practices and improve readability.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 227-227: trailing spaces
(trailing-spaces)
.github/workflows/install.yml (1)
107-113: [Offer Assistance] Clarify "get-branch" Step Behavior
Theget-branchstep strips a potentialrefs/heads/prefix from themain_branchinput. If the expected input is sometimes a full ref and sometimes a simple branch name, this logic is appropriate. However, if the branch input is always provided as just the branch name, the string manipulation may be redundant. Please confirm the expected format of themain_branchinput.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
scripts/ci/docker/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (8)
.github/workflows/install.yml(5 hunks).github/workflows/merge-queue.yml(3 hunks).gitignore(1 hunks)scripts/ci/docker/.editorconfig(1 hunks)scripts/ci/docker/.gitattributes(1 hunks)scripts/ci/docker/.gitignore(1 hunks)scripts/ci/docker/README.md(1 hunks)scripts/ci/docker/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- scripts/ci/docker/README.md
- scripts/ci/docker/.editorconfig
- scripts/ci/docker/.gitignore
- scripts/ci/docker/.gitattributes
- scripts/ci/docker/package.json
🧰 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.github/workflows/install.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)
62-62: 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)
192-192: 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)
🪛 YAMLlint (1.35.1)
.github/workflows/merge-queue.yml
[error] 227-227: trailing spaces
(trailing-spaces)
🔇 Additional comments (6)
.gitignore (1)
17-17: New Ignore Entry for Docker Node ModulesThe addition of
scripts/ci/docker/node_modules/correctly ensures that the node modules within the CI Docker directory are not tracked, which helps keep the repository clean from dependency files. Please verify that this directory is solely used for dependency installation and doesn't contain any source files that need version control..github/workflows/merge-queue.yml (1)
56-56: [Verification Note] Custom Runner Label "arc-runners"
Multiple jobs in this workflow useruns-on: arc-runners. Although static analysis tools flag this as an unknown label, it is valid for our self-hosted ARC runner configuration. Please ensure your actionlint configuration (e.g. inactionlint.yaml) includesarc-runnersin the allowed label list to avoid false positives.Also applies to: 192-192
🧰 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/workflows/install.yml (4)
4-12: [Approved] New Input Parameters Added
The addition of themain_branchandrun_merge_queueinputs is implemented correctly. The default value of'main'formain_branchand the defaultfalseforrun_merge_queueare reasonable.
14-35: [Approved] Outputs Updated for Merge Queue Integration
The outputs have been extended to include merge queue–related variables (e.g.MQ_SHA,MQ_DOCKER_TAG,MQ_ARTIFACT_NAME,MQ_GIT_BRANCH, andMQ_SHOULD_RUN_BUILD) as well as the updated handling for Docker chunks. This mapping appears consistent with the intended integration with subsequent workflows.
136-142: [Approved] Conditional Merge Queue Step
ThePrepare merge queuestep is guarded by the conditionif: ${{ inputs.run_merge_queue == true }}and executes a Node.js script to generate merge queue artifacts. This is implemented as expected.
208-218: [Verify] Docker Build Targets Output Formatting
In thePrepare docker build targetsstep, the script echoes aBUILD_CHUNKSoutput (and also echoes the rawCHUNKS). Please verify that the JSON produced by thejqcommand aligns with downstream expectations in the merge-queue workflow. Also, ensure that the distinction betweenBUILD_CHUNKS(from the build projects step) andDOCKER_CHUNKS(from the docker projects step) is maintained consistently across workflows.
* fix: ok * fix: ignore * fix: ok * fix: get sha * fix: ok * fix: ok * fix: ok * fix: error * fix: skip run * fix: ok * fix: merge
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
.github/workflows/merge-queue.yml (1)
61-62:⚠️ Potential issueFix undefined step output reference
The outputs block references
steps.matrix-output.outputs.json, but no step with the IDmatrix-outputis defined in this job.Apply this fix to reference the correct step that generates JSON output:
outputs: - json: ${{ steps.matrix-output.outputs.json }} + json: ${{ steps.gather.outputs.matrix_json }}Alternatively, add a step with ID
matrix-outputthat produces the required JSON output.🧰 Tools
🪛 actionlint (1.7.4)
62-62: 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)
scripts/ci/docker/get-data.mjs (1)
105-110:⚠️ Potential issueFix getBranch function to handle push events
The
getBranchfunction only handles 'merge_group' events and throws an error for all other events. However, the workflow might also be triggered on push events, which will cause this function to throw an error.function getBranch() { if (context.eventName === 'merge_group') { return context.payload.merge_group.base_ref.replace('refs/heads/', '') + } else if (context.eventName === 'push') { + return context.ref.replace('refs/heads/', '') } throw new Error(`Unsupported event: ${context.eventName}`) }scripts/ci/docker/generate-tag.mjs (2)
43-61: 🛠️ Refactor suggestionRemove commented code and correct error message.
The function contains commented code and uses an error message that mentions "artifact name" instead of "tag name".
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, '') 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(`Unable to determine tag name for merge_group event`) } throw new Error( - `Unable to determine artifact name for event type: ${eventName}`, + `Unable to determine tag name for event type: ${eventName}`, ) }
63-81: 🛠️ Refactor suggestionRemove commented code in getArtifactname function.
The function contains commented code that should be removed for clarity.
function getArtifactname() { if (eventName === 'pull_request' && context.payload.pull_request?.number) { throw new Error(`Unsupported event: ${eventName}`) - // return `pr-${context.payload.pull_request.number}`; } if (eventName === 'merge_group') { if (typeOfDeployment.dev) { return `main-${context.payload.merge_group.head_sha}` } if (typeOfDeployment.prod) { return `release-${context.payload.merge_group.head_sha}` } throw new Error(`Unable to determine artifact name for merge_group event`) } throw new Error( `Unable to determine artifact name for event type: ${eventName}`, ) }
🧹 Nitpick comments (5)
.github/workflows/merge-queue.yml (2)
209-213: Preparation of artifact using JSON dataThis step correctly prepares the artifact by processing the JSON data from the previous step. Consider adding error handling to manage cases where the script might fail.
env: JSON_DATA: ${{ steps.read.outputs.result }} run: | + set -e node scripts/ci/docker/write-data.mjs + if [ $? -ne 0 ]; then + echo "Failed to prepare artifact data" + exit 1 + fi
219-225: Clean integration with ArgoCD through helm-values repositoryThe implementation follows CI/CD best practices by conditionally updating helm values only when changes exist. The trailing space on line 225 should be removed.
files: ${{ steps.manifest.outputs.MQ_CHANGED_FILES }} gh_token_pat: ${{ secrets.GH_HELM_VALUES_PAT }} - +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 225-225: trailing spaces
(trailing-spaces)
.github/workflows/install.yml (2)
103-103: Remove trailing whitespace.There's a trailing space on this line that should be removed for consistency.
- fetch-depth: 0 + fetch-depth: 0🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 103-103: trailing spaces
(trailing-spaces)
125-141: Properly implemented SHA handling for merge queue scenarios.The logic for setting base and head SHAs in merge queue contexts is well documented with comments explaining the purpose and limitations of the current implementation.
However, there's a trailing space on line 126 that should be removed:
- if [[ "${{ inputs.run_merge_queue }}" == "true" ]]; then - # In merge que we always want to use the last + if [[ "${{ inputs.run_merge_queue }}" == "true" ]]; then + # In merge que we always want to use the last🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 126-126: trailing spaces
(trailing-spaces)
scripts/ci/docker/generate-tag.mjs (1)
83-99: Remove commented debugging code.Remove the commented debugging code to improve code clarity.
function getTypeOfDeployment() { if (MAIN_BRANCHES.includes(targetBranch)) { return { dev: true, prod: false, } } if (RELEASE_BRANCHES.includes(targetBranch)) { 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}`) }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (7)
.github/workflows/install.yml(6 hunks).github/workflows/merge-queue.yml(3 hunks)package.json(1 hunks)scripts/ci/docker/.gitignore(1 hunks)scripts/ci/docker/const.mjs(1 hunks)scripts/ci/docker/generate-tag.mjs(1 hunks)scripts/ci/docker/get-data.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/ci/docker/.gitignore
🧰 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/const.mjsscripts/ci/docker/get-data.mjsscripts/ci/docker/generate-tag.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
🪛 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)
62-62: 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/install.yml
89-89: 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)
🪛 YAMLlint (1.35.1)
.github/workflows/merge-queue.yml
[error] 225-225: trailing spaces
(trailing-spaces)
.github/workflows/install.yml
[error] 103-103: trailing spaces
(trailing-spaces)
[error] 126-126: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
🔇 Additional comments (25)
scripts/ci/docker/const.mjs (1)
1-2: New constants for branch categorizationThe constants are well-defined and follow a clear naming convention. This provides a centralized way to manage branch types for deployment targeting.
package.json (1)
69-70: Added GitHub Actions dependencies for CI scriptsThe added dependencies
@actions/coreand@actions/githubare appropriate for interacting with GitHub Actions workflows and will be used by the newly added CI scripts..github/workflows/merge-queue.yml (5)
47-49: Well-structured workflow parameters for merge queue integrationThe workflow parameters are correctly configured for merge queue integration, using the appropriate event context variables to determine the base branch.
67-70: Review NODE_IMAGE_VERSION sourceThe comment "# Fix this" indicates uncertainty about the source of NODE_IMAGE_VERSION.
Confirm whether
needs.pre-checks.outputs.NODE_IMAGE_VERSIONis the correct source for this variable or if it should come from another job. The comment suggests this implementation needs verification.
139-147: Efficient Docker buildx cache strategyThe implementation efficiently checks for a cached buildx image and pulls it from a local registry first before falling back to Docker Hub. This approach reduces external bandwidth usage and improves build time consistency.
172-182: Well-structured matrix output handlingUsing the
cloudposse/github-action-matrix-outputs-writeaction provides a clean way to store and organize matrix job outputs for later processing.
282-284: Updated success check conditionThe success job now correctly checks the post-docker-build job instead of the previously commented-out build job, ensuring proper workflow completion validation.
scripts/ci/docker/get-data.mjs (4)
1-18: Good module organization and constant definitionsThe script imports necessary modules and defines clear constants for GitHub context, branch determination, and output keys. The use of TypeScript checking with
// @ts-checkis a good practice for improved code quality.
19-40: Efficient conditional manifest preparationThe script correctly prepares manifests for different deployment environments based on the deployment type. The handling of output variables for GitHub Actions follows best practices.
41-77: Well-structured manifest parsing implementationThe
prepareManifestsfunction efficiently processes YAML files using glob patterns and extracts relevant image information. The validation of YAML structure with property presence checks is thorough.
80-83: Improve error handling for missing data fileThe script silently exits when the data file is missing. Consider adding appropriate logging and setting GitHub outputs to indicate the script ran but found no data to process.
async function parseData(IMAGE_OBJECT) { const fileName = `/tmp/data.json` if (!fs.existsSync(fileName)) { + console.log(`Data file not found at ${fileName}, no changes needed`) + core.setOutput(_KEY_HAS_OUTPUT, 'false') + core.setOutput(_KEY_CHANGED_FILES, '') process.exit(0) }.github/workflows/install.yml (8)
4-12: Added input parameters enhance workflow reusability.The new input parameters
main_branchandrun_merge_queuemake the workflow more configurable and reusable across different deployment scenarios.
14-24: Well-structured outputs for merge queue integration.These new outputs provide clear communication between this workflow and dependent workflows, allowing for seamless integration with ArgoCD deployment processes.
75-79: Job outputs properly configured for merge queue integration.The outputs correctly reference the step outputs from the prepare-merge-queue step, maintaining consistency with the workflow-level outputs.
84-84: Output source changed to match actual step output.The DOCKER_CHUNKS output now correctly references BUILD_CHUNKS from the docker_projects step, ensuring proper data flow between jobs.
90-92: Explicit permissions follow security best practices.Adding specific permissions ('read' for contents and actions) follows the principle of least privilege, enhancing the security posture of the workflow.
109-114: Branch name extraction logic is clear and efficient.The script correctly extracts the branch name from the raw reference by removing the 'refs/heads/' prefix if present.
156-160: Merge queue preparation step properly conditional.The merge queue preparation step is correctly set to run only when the run_merge_queue input is true, and it uses the generate-tag.mjs script to create the necessary Docker tag.
234-235: Improved output formatting for Docker chunks.The BUILD_CHUNKS output is now processed through jq for proper JSON formatting, making it more reliable for consumption by dependent workflows.
scripts/ci/docker/generate-tag.mjs (6)
1-5: TypeScript checking setup needs improvement.The file uses
@ts-checkdirective but is saved with the.mjsextension which may cause issues with TypeScript tooling. Consider either properly configuring TypeScript for JavaScript files or converting to a TypeScript file.
9-9: Enhance null checking for more robust operation.The direct access to
context.payload.pull_request?.head.shahas only basic null checking. Consider implementing more comprehensive checks or providing fallback mechanisms.
11-28: Well-structured output setting and logging.The script correctly sets GitHub Actions outputs and provides console logging for visibility during workflow execution. This makes troubleshooting easier and enhances workflow transparency.
31-41: Efficiently implemented run condition checking.The
shouldRun()function clearly defines when builds should proceed based on event type and target branch, preventing unnecessary workflow executions.
1-122: Add comprehensive module documentation.The script lacks overall documentation explaining its purpose, usage, and expected inputs/outputs. Add JSDoc comments to improve maintainability.
// @ts-check +/** + * @fileoverview Generates Docker tags and artifact names based on GitHub Actions context. + * This script is intended to be used in GitHub Actions workflows for Docker deployments. + * + * Input: GitHub Actions context (eventName, SHA, etc.) + * Outputs: + * - ARTIFACT_NAME: The name of the artifact to be uploaded + * - DOCKER_TAG: The tag to be used for Docker images + * - GIT_BRANCH: The Git branch being built + * - GIT_SHA: The Git SHA being built + * - SHOULD_RUN_BUILD: Whether the build should run based on event type and branch + */ import github from '@actions/github' import core from '@actions/core' import { MAIN_BRANCHES, RELEASE_BRANCHES } from './const.mjs';
114-121: Well-implemented random string generation utility.The
createRandomString()function is efficiently implemented and provides good randomization for creating unique tags.
* 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]>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/merge-queue.yml (1)
61-62:⚠️ Potential issueFix undefined step reference in outputs.
The
steps.matrix-output.outputs.jsonreference doesn't match any step ID in this job, which will cause the workflow to fail.This is attempting to reference an output from a step that doesn't exist. Replace it with the correct step ID or add a missing step.
outputs: - json: ${{ steps.matrix-output.outputs.json }} + json: ${{ steps.docker-build.outputs.matrix_json }}Note: Replace with the actual step ID and output variable that should provide the matrix JSON data.
🧰 Tools
🪛 actionlint (1.7.4)
62-62: 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)
🧹 Nitpick comments (13)
.github/actions/update-helm-values/action.yml (4)
1-2: Correct the typo in the description.The description contains a typo: "Copys" should be "Copies".
-description: 'Copys the changeset in charts folder to helm-values repository' +description: 'Copies the changeset in charts folder to helm-values repository'
22-25: Remove trailing whitespace.There's trailing whitespace on line 22 that should be removed.
- env: + env: APP_ID: ${{ inputs.app-id }} PRIVATE_KEY: ${{ inputs.ssh-key }}🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 22-22: trailing spaces
(trailing-spaces)
38-49: Consider adding error handling for file operations.The file copying process lacks error handling for potential issues like missing source files or permission problems.
Add checks to verify file existence before copying:
for path in ${paths[@]}; do export DEST="helm-values/charts/${path#charts/}" export DEST_PATH=$(dirname $DEST) mkdir -p $DEST_PATH echo "Copying filepath: ${path} to $DEST" + if [ ! -f "$path" ]; then + echo "Error: Source file '$path' does not exist" + continue + fi cp "$path" "$DEST" done
58-61: Make commit message more descriptive.The current commit message is generic. Consider including information about which charts were updated.
- git commit -m "Updated helm charts" + git commit -m "Updated helm charts: $(echo $files | tr ',' ' ')" echo "Showing changeset\n $(git show)" git pushscripts/ci/docker/get-github-token.mjs (1)
18-26: Consider logging the GitHub installation ID for debugging.When using multiple GitHub App installations, it would be helpful to know which installation is being used.
const response = await fetch('https://api.github.com/app/installations', { method: 'GET', headers: { Accept: 'application/vnd.github.v3+json', Authorization: `Bearer ${authData.token}` } }); const installations = await response.json(); +console.log(`Found ${installations.length} installation(s), using ID: ${installations[0].id}`); const INSTALL_ID = installations[0].id;.github/workflows/merge-queue.yml (3)
171-182: Combine Docker build output steps for better readability.The Docker build output step could be more descriptive about what it's doing.
- name: Docker build output + id: matrix-output uses: cloudposse/github-action-matrix-outputs-write@v1 with: matrix-step-name: ${{ github.job }} matrix-key: ${{ env.MATRIX_ID }} outputs: |- value: ${{ env.JSON_value }} project: ${{ env.JSON_project }} target: ${{ env.JSON_target }} imageName: ${{ env.JSON_imageName }} imageTag: ${{ env.JSON_imageTag }}This adds the missing
matrix-outputID referenced in the job outputs.
157-158: Add echo output to show Docker build args.The echo statement in line 157 is not correctly displaying the Docker build args.
- echo "Docker build args are: 'EXTRA_DOCKER_BUILD_ARGS'" + echo "Docker build args are: '${EXTRA_DOCKER_BUILD_ARGS}'"
226-226: Remove trailing whitespace.There's trailing whitespace on line 226.
- +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 226-226: trailing spaces
(trailing-spaces)
.github/workflows/install.yml (5)
9-10: Fix typo in the description.There's a typo in the description for the
run_merge_queueinput parameter.- description: 'Should the merge que step run' + description: 'Should the merge queue step run'
103-103: Remove trailing whitespace.There's trailing whitespace on line 103.
- fetch-depth: 0 + fetch-depth: 0🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 103-103: trailing spaces
(trailing-spaces)
125-125: Remove trailing whitespace.There's trailing whitespace on line 125.
- # In merge que we always want to use the last + # In merge que we always want to use the last🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 125-125: trailing spaces
(trailing-spaces)
125-139: Fix typo in comments.There are typos in the comments ('que' should be 'queue').
- # In merge que we always want to use the last + # In merge queue we always want to use the last # commit as head - last commit sha is alway succesful # since else it won't get merged! # checkout this PR: https://github.com/nrwl/nx-set-shas/pull/145/files # when this gets merged we can remove this hack! # # If we do not do this we will get the wrong base sha # :(🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 125-125: trailing spaces
(trailing-spaces)
232-234: Improve jq processing for Docker chunk output.The jq command for BUILD_CHUNKS output might cause issues with complex JSON structures. Consider simplifying it.
if [[ "$CHUNKS" != "[]" ]]; then - echo BUILD_CHUNKS=$(echo "$CHUNKS" | jq -cM '. | map("\(.|tostring)")') >> "$GITHUB_OUTPUT" + echo "BUILD_CHUNKS=$CHUNKS" >> "$GITHUB_OUTPUT" echo "CHUNKS='$CHUNKS'" >> "$GITHUB_OUTPUT" fi
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
.github/actions/update-helm-values/action.yml(1 hunks).github/workflows/install.yml(6 hunks).github/workflows/merge-queue.yml(3 hunks).github/workflows/pullrequest.yml(1 hunks)package.json(2 hunks)scripts/ci/docker/get-github-token.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- .github/workflows/pullrequest.yml
🧰 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/actions/update-helm-values/action.yml.github/workflows/merge-queue.yml.github/workflows/install.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/get-github-token.mjs
🪛 YAMLlint (1.35.1)
.github/actions/update-helm-values/action.yml
[error] 22-22: trailing spaces
(trailing-spaces)
.github/workflows/merge-queue.yml
[error] 226-226: trailing spaces
(trailing-spaces)
.github/workflows/install.yml
[error] 103-103: trailing spaces
(trailing-spaces)
[error] 125-125: trailing spaces
(trailing-spaces)
🪛 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)
62-62: 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/install.yml
89-89: 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 (10)
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
- GitHub Check: prepare / install
🔇 Additional comments (1)
.github/workflows/merge-queue.yml (1)
68-70: Address "Fix this" comment for NODE_IMAGE_VERSION.The code contains a TODO comment indicating that the source of NODE_IMAGE_VERSION needs attention.
The comment "# Fix this" suggests uncertainty about the NODE_IMAGE_VERSION source. Let me check whether
needs.pre-checks.outputs.NODE_IMAGE_VERSIONis the correct source:#!/bin/bash # Look for NODE_IMAGE_VERSION in pre-checks workflow rg "outputs:" .github/workflows/pre-checks.yml -A 10 | grep -A 3 "outputs:" | grep "NODE_IMAGE_VERSION"
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
.github/actions/update-helm-values/action.yml (6)
1-2: Typographical Correction in Action Description
The action description contains a typo ("Copys" should be "Copies"). Correcting this will improve clarity and professionalism.
5-9: Typographical Correction in Input Description
The description for thefilesinput uses "Comma seperated" which should be "Comma-separated" to correct the spelling.
22-22: Remove Trailing Whitespace
YAMLlint has flagged trailing whitespace on this line. Please remove any extra spaces to meet YAML formatting standards.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 22-22: trailing spaces
(trailing-spaces)
19-27: Verify Token Retrieval Script Path
Ensure that the scriptscripts/ci/docker/get-github-token.mjsexists at the expected location, so the token retrieval step functions correctly.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 22-22: trailing spaces
(trailing-spaces)
39-39: Clarify IFS Assignment
The lineIFS=$IFS,is unconventional. If the intent is to split the input strictly by commas, consider usingIFS=,instead.
54-61: Enhance Git Commit Robustness
Before committing changes, consider checking if there are any staged changes (e.g., usinggit diff --cached --exit-code) to avoid a commit failure when there is nothing to commit.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/actions/update-helm-values/action.yml(1 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/actions/update-helm-values/action.yml
🪛 YAMLlint (1.35.1)
.github/actions/update-helm-values/action.yml
[error] 22-22: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: prepare / install
🔇 Additional comments (2)
.github/actions/update-helm-values/action.yml (2)
27-34: Confirm Default Branch Checkout
The checkout step forcefully checks out themainbranch of the helm-values repository. Verify that this is the intended branch for updates.
44-44: Confirm Directory Structure
The destination path is set to"helm-values/helm-values/${path#charts/}". Please verify that the double occurrence ofhelm-valuesis intentional based on the repository structure.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
scripts/ci/docker/const.mjs (2)
1-2: Fix the inconsistent use of semicolons.There's an inconsistency in the use of semicolons - the first line doesn't have a semicolon while the second line does. For code consistency, either add a semicolon to line 1 or remove it from line 2.
-export const MAIN_BRANCHES = ['main'] -export const RELEASE_BRANCHES = ['release', 'mq-docker']; +export const MAIN_BRANCHES = ['main']; +export const RELEASE_BRANCHES = ['release', 'mq-docker'];
1-2: Consider using environment variables for branch configuration.The branch names are currently hardcoded. For greater flexibility across different environments, consider loading these values from environment variables or a configuration file. This would allow branch configurations to be modified without code changes.
-export const MAIN_BRANCHES = ['main'] -export const RELEASE_BRANCHES = ['release', 'mq-docker']; +export const MAIN_BRANCHES = process.env.MAIN_BRANCHES ? process.env.MAIN_BRANCHES.split(',') : ['main']; +export const RELEASE_BRANCHES = process.env.RELEASE_BRANCHES ? process.env.RELEASE_BRANCHES.split(',') : ['release', 'mq-docker'];
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/ci/docker/const.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/const.mjs
🔇 Additional comments (1)
scripts/ci/docker/const.mjs (1)
1-2: Clearly named constants provide good documentation.The constant names
MAIN_BRANCHESandRELEASE_BRANCHESare descriptive and make the code's intent clear when used elsewhere in the codebase. This aligns well with the guideline for script readability and maintainability.
* fix: mq-docker-main * fix: ugly hack remove this before going to main * fix: mq docker pre main
* fix: format * fix: ok
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/merge-queue.yml (2)
109-138: Docker Build Arguments Setup with Secrets Handling
This step constructs the Docker build arguments array and appends a secret (NX_CLOUD_ACCESS_TOKEN) by writing it to a temporary file. While this approach is functional, please ensure that the secret is handled securely and that nothing inadvertently logs the secret value.
226-226: Remove Trailing Whitespace
There appears to be trailing whitespace at line 226. Cleaning up this extra whitespace will help maintain clean YAML formatting.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 226-226: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/merge-queue.yml(3 hunks)package.json(2 hunks)scripts/ci/docker/const.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- scripts/ci/docker/const.mjs
- package.json
🧰 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)
62-62: 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)
🪛 YAMLlint (1.35.1)
.github/workflows/merge-queue.yml
[error] 226-226: trailing spaces
(trailing-spaces)
🔇 Additional comments (7)
.github/workflows/merge-queue.yml (7)
47-49: New Input Parameters for the Prepare Job
Thepreparejob now includes new inputs (run_merge_queue: trueandmain_branch: ${{ github.event.merge_group.base_ref }}), which should help align the workflow with the merge queue and, ultimately, with the ArgoCD deployment process. Please verify that these inputs meet your intended deployment logic.
80-92: Gather Apps Step – Handling Errors
TheGather appsstep extracts values from the matrix chunk and sets corresponding environment variables. The use ofcontinue-on-error: trueallows the workflow to proceed even if this step fails. Please verify that any downstream steps can handle the situation where these environment variables might be missing or incomplete.
148-170: Building Docker Images with Retry Logic
The primary Docker image building step, along with its retry mechanism, appears robust with the fallback strategy in place. The use ofcontinue-on-error: trueallows a retry if the initial build fails. Please confirm that this error-handling approach meets your overall CI reliability requirements.
171-178: Capturing Docker Build Output
The step usingcloudposse/github-action-matrix-outputs-write@v1to capture Docker build outputs is appropriate, assuming all the necessary environment variables are set beforehand. This helps in passing information to subsequent jobs reliably.
182-191: Post-Docker-Build Job Configuration
The newpost-docker-buildjob is configured to run conditionally—only when a build is required and docker chunks exist—and it depends on thedocker-build,prepare, andtestsjobs. Additionally, it usesruns-on: arc-runners; please confirm that this custom runner label is correctly supported in your configuration.🧰 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)
245-245: NX_HEAD Environment Variable in Tests Job
Thetestsjob now includes the environment variableNX_HEADfrom${{ needs.prepare.outputs.NX_HEAD }}. Please verify that this output is reliably provided by thepreparejob and is correctly propagated.
274-275: Success Job Dependencies Update
Thesuccessjob now depends on bothdocker-buildandpost-docker-build(in addition toprepareandtests), ensuring all Docker-related steps are completed before the final check. Confirm that this dependency update aligns with your overall CI process and that no unintended dependencies have been introduced.
Deploying to argocd
Summary by CodeRabbit