-
Notifications
You must be signed in to change notification settings - Fork 204
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dev 338/integrated cd #3020
base: dev
Are you sure you want to change the base?
Dev 338/integrated cd #3020
Conversation
…k frontend changes then deploy
|
WalkthroughThis pull request enhances the GitHub Actions deployment workflow by introducing several new jobs in the deployment file. The changes add multiple checks for Prisma migration, workflows service deployment, database migration, and synchronization via ArgoCD API queries. Additionally, jobs have been added to detect updates in specific application directories and to trigger corresponding deployments (Backoffice, KYB, Dashboard) conditionally based on the checks. Overall, the modifications ensure a sequential and conditional process for deploying updated services safely. Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (2)
.github/workflows/deploy-wf-service.yml (2)
529-584
: Review: Clarify Application Changes Check Step NamesThe
check-apps-changes
job is responsible for detecting changes in application directories:
- The steps are generically named "Check for Changes in apps/v1". Consider renaming them to more descriptive titles such as "Check Backoffice Updates" (for files under
apps/backoffice-v2/
), "Check KYB Updates" (forapps/kyb-app/
), and "Check Dashboard Updates" (forapps/workflows-dashboard/
).- Ensure consistency in setting both environment variables and GitHub Action outputs.
🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 531-531: too few spaces after comma
(commas)
[error] 560-560: trailing spaces
(trailing-spaces)
[error] 569-569: trailing spaces
(trailing-spaces)
278-278
: Review: YAML Formatting ImprovementsSeveral YAML formatting issues were detected:
- Spacing: There are warnings about too few spaces after commas in list definitions (e.g., in lines 278, 344, 408, 470, 531, 587, 595, and 603). For better readability, consider formatting them as, for example,
[update-helm-chart, build-and-push-ee-image]
(with a space after the comma).- Trailing Spaces: Trailing spaces were reported on lines 340, 528, 560, and 569. Please remove these to comply with YAML standards.
- Final Newline: The file is missing a newline at the end (line 607). Ensure that a newline is added as the final character in the file.
Also applies to: 344-344, 408-408, 470-470, 531-531, 340-340, 528-528, 560-560, 569-569, 587-587, 595-595, 603-603
🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 278-278: too few spaces after comma
(commas)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/deploy-wf-service.yml
(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/deploy-wf-service.yml
589-589: could not read reusable workflow file for "./.github/workflows/deploy-backoffice.yml": open /home/jailuser/git/.github/workflows/deploy-backoffice.yml: no such file or directory
(workflow-call)
597-597: could not read reusable workflow file for "./.github/workflows/deploy-kyb.yml": open /home/jailuser/git/.github/workflows/deploy-kyb.yml: no such file or directory
(workflow-call)
605-605: could not read reusable workflow file for "./.github/workflows/deploy-dashboard.yml": open /home/jailuser/git/.github/workflows/deploy-dashboard.yml: no such file or directory
(workflow-call)
🪛 YAMLlint (1.35.1)
.github/workflows/deploy-wf-service.yml
[warning] 278-278: too few spaces after comma
(commas)
[error] 340-340: trailing spaces
(trailing-spaces)
[warning] 344-344: too few spaces after comma
(commas)
[warning] 408-408: too few spaces after comma
(commas)
[warning] 470-470: too few spaces after comma
(commas)
[error] 528-528: trailing spaces
(trailing-spaces)
[warning] 531-531: too few spaces after comma
(commas)
[error] 560-560: trailing spaces
(trailing-spaces)
[error] 569-569: trailing spaces
(trailing-spaces)
[warning] 587-587: too few spaces after comma
(commas)
[warning] 595-595: too few spaces after comma
(commas)
[warning] 603-603: too few spaces after comma
(commas)
[error] 607-607: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
.github/workflows/deploy-wf-service.yml (4)
275-341
: Review: Verify Prisma Migrate Job Logic and Environment Variable HandlingThe
check-prisma-migrate-status
job correctly polls the ArgoCD API for the Prisma migration job status. However:
- The script retrieves a token via a POST request and writes it to
$GITHUB_ENV
, then immediately uses$ARGOCD_TOKEN
within the same run block. Environment variables set viaecho ... >> $GITHUB_ENV
are typically available only in subsequent steps. Consider saving the token in a local shell variable (e.g., usingTOKEN
) and then using that variable throughout the script.- There is no timeout or maximum iteration count in the
while
loop. Adding a timeout would help avoid an infinite loop if the migration job stalls.- Formatting Note: Remove any trailing spaces at the end of line 340.
🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 278-278: too few spaces after comma
(commas)
[error] 340-340: trailing spaces
(trailing-spaces)
585-592
:⚠️ Potential issueReview: Verify Reusable Workflow File for Backoffice Deployment
The
deploy-backoffice
job references a reusable workflow file at./.github/workflows/deploy-backoffice.yml
. Static analysis indicates that this file could not be found. Please confirm that the file exists at the specified path or update the reference accordingly.🧰 Tools
🪛 actionlint (1.7.4)
589-589: could not read reusable workflow file for "./.github/workflows/deploy-backoffice.yml": open /home/jailuser/git/.github/workflows/deploy-backoffice.yml: no such file or directory
(workflow-call)
🪛 YAMLlint (1.35.1)
[warning] 587-587: too few spaces after comma
(commas)
593-600
:⚠️ Potential issueReview: Verify Reusable Workflow File for KYB Deployment
Similarly, the
deploy-kyb
job references a workflow file at./.github/workflows/deploy-kyb.yml
that appears to be missing. Please ensure that the referenced file exists or adjust the path as necessary.🧰 Tools
🪛 actionlint (1.7.4)
597-597: could not read reusable workflow file for "./.github/workflows/deploy-kyb.yml": open /home/jailuser/git/.github/workflows/deploy-kyb.yml: no such file or directory
(workflow-call)
🪛 YAMLlint (1.35.1)
[warning] 595-595: too few spaces after comma
(commas)
601-607
:⚠️ Potential issueReview: Verify Reusable Workflow File for Dashboard Deployment and File Ending
For the
deploy-dashboard
job:
- The referenced workflow file at
./.github/workflows/deploy-dashboard.yml
could not be located. Please verify and update the path if needed.- Additionally, note that the file does not end with a newline as required by YAMLlint. Adding a newline at the end will resolve this formatting error.
🧰 Tools
🪛 actionlint (1.7.4)
605-605: could not read reusable workflow file for "./.github/workflows/deploy-dashboard.yml": open /home/jailuser/git/.github/workflows/deploy-dashboard.yml: no such file or directory
(workflow-call)
🪛 YAMLlint (1.35.1)
[warning] 603-603: too few spaces after comma
(commas)
[error] 607-607: no new line character at the end of file
(new-line-at-end-of-file)
# TODO Check Application Deployment Status | ||
check-wf-service-status: | ||
runs-on: ubuntu-latest | ||
needs: [check-prisma-migrate-status,build-and-push-ee-image] | ||
if: ${{ needs.check-prisma-migrate-status.result == 'success' }} | ||
environment: ${{ inputs.environment }} | ||
steps: | ||
- name: Pre-Run Wait | ||
run: | | ||
sleep 15 | ||
- name: Check Prisma Migrate Status | ||
id: prisma-status | ||
run: | | ||
ARGOCD_API_URL="${{ secrets.ARGOCD_SERVER }}/api/v1/applications/wf-service/resource?name=wf-service-db-data-migrations&appNamespace=argocd&namespace=default&resourceName=wf-service&version=v1&kind=Deployment&group=apps" | ||
|
||
TARGET_IMAGE_TAG="${{ env.SHORT_HASH }}-${{ needs.build-and-push-ee-image.outputs.SUBMODULE_SHORT_HASH }}-${{ inputs.environment }}" | ||
|
||
while true; do | ||
# Fetch job status from ArgoCD API | ||
RESPONSE=$(curl -s -k -H "Authorization: Bearer ${{env.ARGOCD_TOKEN}}" "$ARGOCD_API_URL") | ||
# echo $RESPONSE | ||
|
||
# Extract and parse the manifest JSON | ||
MANIFEST=$(echo "$RESPONSE" | jq -r '.manifest' | jq '.') | ||
# echo $MANIFEST | ||
|
||
# Extract Start Time, Status, and Type | ||
# START_TIME=$(echo "$MANIFEST" | jq -r '.status.startTime') | ||
# echo $START_TIME | ||
STATUS=$(echo "$MANIFEST" | jq -r '.status.conditions[] | select(.type=="Progressing") | .status') | ||
echo $STATUS | ||
REASON=$(echo "$MANIFEST" | jq -r '.status.conditions[] | select(.type=="Progressing") | .reason') | ||
echo $REASON | ||
|
||
# Extract Image Tag | ||
IMAGE=$(echo "$MANIFEST" | jq -r '.spec.template.spec.containers[0].image') | ||
echo $IMAGE | ||
IMAGE_TAG=$(echo "$IMAGE" | cut -d ':' -f2) # Extract the tag after the colon | ||
echo $IMAGE_TAG | ||
|
||
# Print extracted information | ||
echo "Start Time: $START_TIME" | ||
echo "Status: $STATUS" | ||
echo "Type: $REASON" | ||
echo "Image: $IMAGE" | ||
echo "Image Tag: $IMAGE_TAG" | ||
|
||
# Nested condition to check Image Tag and Job Completion | ||
if [[ "$IMAGE_TAG" == "$TARGET_IMAGE_TAG" ]]; then | ||
if [[ "$REASON" == "NewReplicaSetAvailable" && "$STATUS" == "True" ]]; then | ||
echo "✅ Deployment has been successful with the correct image tag: $IMAGE_TAG" | ||
break | ||
else | ||
echo "⏳ Replicaset is still getting updated... Waiting for completion." | ||
fi | ||
else | ||
echo "🚨 Image tag mismatch! Expected: $TARGET_IMAGE_TAG, Found: $IMAGE_TAG" | ||
fi | ||
|
||
# Wait before the next check | ||
sleep 10 | ||
done | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Review: Correct Naming and Undefined Variable in WF Service Deployment Status Check
In the check-wf-service-status
job:
- The step is labelled "Check Prisma Migrate Status" despite the job being intended to verify the workflows service deployment. Renaming it (for example, to "Check WF Service Deployment Status") will improve clarity.
- The script echoes a variable named
START_TIME
(line 382) that is never defined (the related lines are commented out). Either remove this echo or define and useSTART_TIME
appropriately. - As with the previous job, consider adding a timeout to the polling loop.
🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 344-344: too few spaces after comma
(commas)
# TODO Check DB Migrate Status | ||
check-db-migrate-status: | ||
runs-on: ubuntu-latest | ||
needs: [check-wf-service-status,build-and-push-ee-image] | ||
if: ${{ needs.check-wf-service-status.result == 'success' }} | ||
environment: ${{ inputs.environment }} | ||
steps: | ||
- name: Pre-Run Wait | ||
run: | | ||
sleep 15 | ||
- name: Check Prisma Migrate Status | ||
id: prisma-status | ||
run: | | ||
ARGOCD_API_URL="${{ secrets.ARGOCD_SERVER }}/api/v1/applications/wf-service/resource?name=wf-service-db-data-migrations&appNamespace=argocd&namespace=default&resourceName=wf-service-db-data-migrations&version=v1&kind=Job&group=batch" | ||
|
||
TARGET_IMAGE_TAG="${{ env.SHORT_HASH }}-${{ needs.build-and-push-ee-image.outputs.SUBMODULE_SHORT_HASH }}-${{ inputs.environment }}" | ||
while true; do | ||
# Fetch job status from ArgoCD API | ||
RESPONSE=$(curl -s -k -H "Authorization: Bearer ${{env.ARGOCD_TOKEN}}" "$ARGOCD_API_URL") | ||
# echo $RESPONSE | ||
|
||
# Extract and parse the manifest JSON | ||
MANIFEST=$(echo "$RESPONSE" | jq -r '.manifest' | jq '.') | ||
# echo $MANIFEST | ||
|
||
# Extract Start Time, Status, and Type | ||
START_TIME=$(echo "$MANIFEST" | jq -r '.status.startTime') | ||
echo $START_TIME | ||
STATUS=$(echo "$MANIFEST" | jq -r '.status.conditions[] | select(.type=="Complete") | .status') | ||
echo $STATUS | ||
TYPE=$(echo "$MANIFEST" | jq -r '.status.conditions[] | select(.type=="Complete") | .type') | ||
echo $TYPE | ||
|
||
# Extract Image Tag | ||
IMAGE=$(echo "$MANIFEST" | jq -r '.spec.template.spec.containers[0].image') | ||
echo $IMAGE | ||
IMAGE_TAG=$(echo "$IMAGE" | cut -d ':' -f2) # Extract the tag after the colon | ||
echo $IMAGE_TAG | ||
|
||
# Print extracted information | ||
echo "Start Time: $START_TIME" | ||
echo "Status: $STATUS" | ||
echo "Type: $TYPE" | ||
echo "Image: $IMAGE" | ||
echo "Image Tag: $IMAGE_TAG" | ||
|
||
# Nested condition to check Image Tag and Job Completion | ||
if [[ "$IMAGE_TAG" == "$TARGET_IMAGE_TAG" ]]; then | ||
if [[ "$TYPE" == "Complete" && "$STATUS" == "True" ]]; then | ||
echo "✅ DB Migrate Job has completed successfully with the correct image tag: $IMAGE_TAG" | ||
break | ||
else | ||
echo "⏳ DB Migrate Job is still running... Waiting for completion." | ||
fi | ||
else | ||
echo "🚨 Image tag mismatch! Expected: $TARGET_IMAGE_TAG, Found: $IMAGE_TAG" | ||
fi | ||
|
||
# Wait before the next check | ||
sleep 10 | ||
done | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Review: Update Naming for DB Migrate Status Check
In the check-db-migrate-status
job:
- The step is once again named "Check Prisma Migrate Status" though it is checking the database migration job. Rename the step to "Check DB Migrate Status" to accurately reflect its purpose.
- A timeout for the polling loop is recommended to avoid potential infinite looping.
🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 408-408: too few spaces after comma
(commas)
# TODO Check DB Sync Status | ||
check-db-sync-status: | ||
runs-on: ubuntu-latest | ||
needs: [check-db-migrate-status,build-and-push-ee-image] | ||
if: ${{ needs.check-db-migrate-status.result == 'success' }} | ||
environment: ${{ inputs.environment }} | ||
steps: | ||
- name: Pre-Run Wait | ||
run: | | ||
sleep 15 | ||
- name: Check Prisma Migrate Status | ||
id: prisma-status | ||
run: | | ||
ARGOCD_API_URL="${{ secrets.ARGOCD_SERVER }}/api/v1/applications/wf-service/resource?name=wf-service-db-data-sync&appNamespace=argocd&namespace=default&resourceName=wf-service-db-data-sync&version=v1&kind=Job&group=batch" | ||
|
||
TARGET_IMAGE_TAG="${{ env.SHORT_HASH }}-${{ needs.build-and-push-ee-image.outputs.SUBMODULE_SHORT_HASH }}-${{ inputs.environment }}" | ||
while true; do | ||
# Fetch job status from ArgoCD API | ||
RESPONSE=$(curl -s -k -H "Authorization: Bearer ${{env.ARGOCD_TOKEN}}" "$ARGOCD_API_URL") | ||
# echo $RESPONSE | ||
|
||
# Extract and parse the manifest JSON | ||
MANIFEST=$(echo "$RESPONSE" | jq -r '.manifest' | jq '.') | ||
# echo $MANIFEST | ||
|
||
# Extract Start Time, Status, and Type | ||
START_TIME=$(echo "$MANIFEST" | jq -r '.status.startTime') | ||
echo $START_TIME | ||
STATUS=$(echo "$MANIFEST" | jq -r '.status.conditions[] | select(.type=="Complete") | .status') | ||
echo $STATUS | ||
TYPE=$(echo "$MANIFEST" | jq -r '.status.conditions[] | select(.type=="Complete") | .type') | ||
echo $TYPE | ||
|
||
# Extract Image Tag | ||
IMAGE=$(echo "$MANIFEST" | jq -r '.spec.template.spec.containers[0].image') | ||
echo $IMAGE | ||
IMAGE_TAG=$(echo "$IMAGE" | cut -d ':' -f2) # Extract the tag after the colon | ||
echo $IMAGE_TAG | ||
|
||
# Print extracted information | ||
echo "Start Time: $START_TIME" | ||
echo "Status: $STATUS" | ||
echo "Type: $TYPE" | ||
echo "Image: $IMAGE" | ||
echo "Image Tag: $IMAGE_TAG" | ||
|
||
# Nested condition to check Image Tag and Job Completion | ||
if [[ "$IMAGE_TAG" == "$TARGET_IMAGE_TAG" ]]; then | ||
if [[ "$TYPE" == "Complete" && "$STATUS" == "True" ]]; then | ||
echo "✅ DB Sync Job has completed successfully with the correct image tag: $IMAGE_TAG" | ||
break | ||
else | ||
echo "⏳ DB Sync Job is still running... Waiting for completion." | ||
fi | ||
else | ||
echo "🚨 Image tag mismatch! Expected: $TARGET_IMAGE_TAG, Found: $IMAGE_TAG" | ||
fi | ||
|
||
# Wait before the next check | ||
sleep 10 | ||
done | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Review: Update Naming for DB Sync Status Check
The check-db-sync-status
job:
- Uses the step name "Check Prisma Migrate Status" even though the intention is to verify the database synchronization job. Please rename this step to "Check DB Sync Status" for clarity.
- As with the other polling loops, implementing a timeout mechanism is advisable.
🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 470-470: too few spaces after comma
(commas)
[error] 528-528: trailing spaces
(trailing-spaces)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (11)
.github/workflows/deploy-wf-service.yml (11)
227-227
: Remove Trailing Spaces
YAMLlint has flagged trailing spaces on this line. Please remove them to comply with formatting standards.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 227-227: trailing spaces
(trailing-spaces)
275-295
: Review 'Check Prisma Migrate Status' Job
The newly addedcheck-prisma-migrate-status
job queries the ArgoCD API to monitor the Prisma migration job. One concern is that the polling loop does not implement a timeout. This could lead to an infinite loop if the job never completes. Consider adding a timeout or maximum iteration count.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 278-278: too few spaces after comma
(commas)
341-346
: Rename Job Comment for Deployment Check
The comment# TODO Check Application Deployment Status
precedes thecheck-wf-service-status
job. Since this job is meant to validate the WF service deployment, ensure that both the comment and subsequent step names accurately reflect that purpose.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 344-344: too few spaces after comma
(commas)
351-359
: Rename Step in WF Service Deployment Check
Within thecheck-wf-service-status
job, the step is currently labeled as "Check Prisma Migrate Status" even though it is querying the deployment status. Renaming this step to "Check WF Service Deployment Status" would improve clarity.
550-559
: Distinct Naming for Backoffice Changes Check
The step labeled "Check for Changes in apps/v1" for backoffice is very generic. Renaming it to "Check for Backoffice Changes" can enhance clarity.
561-568
: Distinct Naming for KYB Changes Check
Similarly, the step for checking changes in the KYB application should be renamed to "Check for KYB Changes" to clearly differentiate it from the other checks.
570-577
: Distinct Naming for Dashboard Changes Check
Consider renaming the dashboard changes check from "Check for Changes in apps/v1" to "Check for Dashboard Changes" so that the purpose is unmistakable for each application.
585-592
: Deploy Backoffice Trigger Placeholder
Thedeploy-backoffice
job is added as a TODO placeholder that triggers a separate workflow. Ensure that the referenced file (deploy-backoffice.yml
) is fully implemented and that its conditions match your deployment requirements.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 587-587: too few spaces after comma
(commas)
593-600
: Deploy KYB Trigger Placeholder
Thedeploy-kyb
job, which triggers the KYB deployment, is currently a placeholder. Please confirm that.github/workflows/deploy-kyb.yml
is properly configured and that its conditional logic meets the desired criteria.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 595-595: too few spaces after comma
(commas)
601-607
: Deploy Dashboard Trigger Placeholder & File Ending
Thedeploy-dashboard
job serves as a placeholder for triggering the Dashboard deployment. Verify that.github/workflows/deploy-dashboard.yml
is set up correctly. Additionally, YAMLlint indicates that there is no newline at the end of the file (line 607). Please add a newline to conform with best practices.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 603-603: too few spaces after comma
(commas)
[error] 607-607: no new line character at the end of file
(new-line-at-end-of-file)
227-227
: Fix Formatting Issues
Static analysis reports several formatting issues:
- Trailing Spaces: Lines 227, 340, 528, 560, and 569 contain trailing spaces.
- Spacing After Commas: Lines 278, 344, 408, 470, 531, 587, 595, and 603 show too few spaces after commas.
- Missing Newline: The file is missing a newline at the end (line 607).
Please address these issues to adhere to YAML formatting guidelines.Also applies to: 278-278, 340-340, 344-344, 408-408, 470-470, 528-528, 531-531, 560-560, 569-569, 587-587, 595-595, 603-603, 607-607
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 227-227: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/deploy-wf-service.yml
(3 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/deploy-wf-service.yml
[error] 227-227: trailing spaces
(trailing-spaces)
[warning] 278-278: too few spaces after comma
(commas)
[error] 340-340: trailing spaces
(trailing-spaces)
[warning] 344-344: too few spaces after comma
(commas)
[warning] 408-408: too few spaces after comma
(commas)
[warning] 470-470: too few spaces after comma
(commas)
[error] 528-528: trailing spaces
(trailing-spaces)
[warning] 531-531: too few spaces after comma
(commas)
[error] 560-560: trailing spaces
(trailing-spaces)
[error] 569-569: trailing spaces
(trailing-spaces)
[warning] 587-587: too few spaces after comma
(commas)
[warning] 595-595: too few spaces after comma
(commas)
[warning] 603-603: too few spaces after comma
(commas)
[error] 607-607: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build-and-push-image
- GitHub Check: Analyze (javascript)
🔇 Additional comments (5)
.github/workflows/deploy-wf-service.yml (5)
225-225
: Clarify Sparse-Checkout Path
The sparse-checkout entry now points tokubernetes/helm/wf-service-migration
, which appears to reflect the updated Helm chart structure. Please verify that this is the intended directory for the WF service migration charts.
231-236
: Verify Custom Values File Check
The file existence check forif [ -f "kubernetes/helm/wf-service-migration/${{ inputs.environment }}-custom-values.yaml" ]; thencorrectly determines whether an environment-specific custom values file exists. Verify that falling back to
dev-custom-values.yaml
is the desired behavior when the file is not found.
248-256
: Update Helm Chart Image Tags
The JSON block updating image tags now targets the new path underkubernetes/helm/wf-service-migration
. Please confirm that all keys (e.g.image.tag
,prismaMigrate.image.tag
,dbMigrate.image.tag
,dataSync.image.tag
) precisely reflect the configuration expected by the Helm chart.
415-422
: Correct Step Label in DB Migrate Job
In thecheck-db-migrate-status
job, the step is titled "Check Prisma Migrate Status," which is misleading. It should be renamed to "Check DB Migrate Status" to better reflect its function.
477-485
: Rename Step in DB Sync Job
The step in thecheck-db-sync-status
job is labeled "Check Prisma Migrate Status" even though it is tasked with checking the DB sync status. Please update the label to "Check DB Sync Status" for clarity.
while true; do | ||
# Fetch job status from ArgoCD API | ||
RESPONSE=$(curl -s -k -H "Authorization: Bearer ${{env.ARGOCD_TOKEN}}" "$ARGOCD_API_URL") | ||
# echo $RESPONSE | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Implement Timeout in Deployment Polling Loop
The polling loop in the deployment status check lacks a timeout. Consider adding a timeout or iteration limit to prevent potential infinite looping if the WF service remains in a transitional state.
TARGET_IMAGE_TAG="${{ env.SHORT_HASH }}-${{ needs.build-and-push-ee-image.outputs.SUBMODULE_SHORT_HASH }}-${{ inputs.environment }}" | ||
while true; do | ||
# Fetch job status from ArgoCD API | ||
RESPONSE=$(curl -s -k -H "Authorization: Bearer ${{env.ARGOCD_TOKEN}}" "$ARGOCD_API_URL") | ||
# echo $RESPONSE | ||
|
||
# Extract and parse the manifest JSON | ||
MANIFEST=$(echo "$RESPONSE" | jq -r '.manifest' | jq '.') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add Timeout to DB Migrate Loop
The DB migration status polling loop runs indefinitely until the desired status is detected. Consider adding a timeout or maximum number of iterations to avoid infinite looping under error conditions.
TARGET_IMAGE_TAG="${{ env.SHORT_HASH }}-${{ needs.build-and-push-ee-image.outputs.SUBMODULE_SHORT_HASH }}-${{ inputs.environment }}" | ||
while true; do | ||
# Fetch job status from ArgoCD API | ||
RESPONSE=$(curl -s -k -H "Authorization: Bearer ${{env.ARGOCD_TOKEN}}" "$ARGOCD_API_URL") | ||
# echo $RESPONSE | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Implement Timeout in DB Sync Loop
Similar to the other polling loops, the DB sync check loop lacks a timeout. Adding this mechanism would help avoid a potential infinite loop if the sync job stalls or fails.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
.github/workflows/deploy-wf-service.yml (2)
275-339
: 🛠️ Refactor suggestionAdd timeout to Prisma migration status check.
The polling loop for checking Prisma migration status lacks a timeout mechanism, which could lead to an infinite loop if the job fails or stalls.
Add a timeout mechanism:
TARGET_IMAGE_TAG="${{ env.SHORT_HASH }}-${{ needs.build-and-push-ee-image.outputs.SUBMODULE_SHORT_HASH }}-${{ inputs.environment }}" + MAX_ATTEMPTS=30 # 5 minutes with 10-second intervals + ATTEMPT=0 while true; do + if [ $ATTEMPT -ge $MAX_ATTEMPTS ]; then + echo "❌ Timeout waiting for Prisma migration to complete" + exit 1 + fi + ATTEMPT=$((ATTEMPT + 1)) # Rest of the loop code... sleep 10 done🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 278-278: too few spaces after comma
(commas)
341-411
:⚠️ Potential issueFix incomplete deployment status check implementation.
The workflows service deployment status check has commented-out code and uses an incorrect step name.
- Uncomment and fix the deployment status check loop
- Rename the step to match its purpose
- Add a timeout mechanism
- - name: Check Prisma Migrate Status + - name: Check Workflows Service Deployment Status id: prisma-status run: | ARGOCD_API_URL="${{ secrets.ARGOCD_SERVER }}/api/v1/applications/test-wf-service/resource?name=test-wf-service&appNamespace=argocd&namespace=default&resourceName=wf-service&version=v1&kind=Deployment&group=apps" TARGET_IMAGE_TAG="${{ env.SHORT_HASH }}-${{ needs.build-and-push-ee-image.outputs.SUBMODULE_SHORT_HASH }}-${{ inputs.environment }}" + MAX_ATTEMPTS=30 + ATTEMPT=0 - # while true; do + while true; do + if [ $ATTEMPT -ge $MAX_ATTEMPTS ]; then + echo "❌ Timeout waiting for deployment to complete" + exit 1 + fi + ATTEMPT=$((ATTEMPT + 1)) # Rest of the implementation... - # Wait before the next check - # sleep 10 - # done + sleep 10 + done🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 344-344: too few spaces after comma
(commas)
🧹 Nitpick comments (3)
.github/workflows/deploy-wf-service.yml (3)
225-254
: Add error handling for Helm chart updates.The YAML update action modifies multiple image tags but lacks error handling. Consider adding a verification step to ensure all tags were updated successfully.
Add a verification step after the YAML update:
- name: Verify image tag updates run: | # Extract and verify each image tag for component in image prismaMigrate dbMigrate dataSync; do tag=$(yq e ".$component.image.tag" "kubernetes/helm/wf-service-migration/${{steps.update_helm_check.outputs.file_name}}") if [[ "$tag" != "${{ env.SHORT_HASH }}-${{ needs.build-and-push-ee-image.outputs.SUBMODULE_SHORT_HASH }}-${{ inputs.environment }}" ]]; then echo "❌ Failed to update $component image tag" exit 1 fi done echo "✅ All image tags updated successfully"🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 227-227: trailing spaces
(trailing-spaces)
[error] 238-238: trailing spaces
(trailing-spaces)
413-473
: Standardize step names in status check jobs.The step names in the database migration and sync status check jobs are inconsistent with their purposes.
Update the step names:
- In
check-db-migrate-status
, rename "Check Prisma Migrate Status" to "Check DB Migration Status"- In
check-db-sync-status
, rename "Check Prisma Migrate Status" to "Check DB Sync Status"Also applies to: 475-535
🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 416-416: too few spaces after comma
(commas)
537-615
: Enhance error handling and logging in application change detection.The application change detection could benefit from improved error handling and logging.
Add error handling and improve logging:
- name: Check for Changes in apps/v1 id: check-backoffice run: | + set -e # Exit on error + echo "Checking for changes in backoffice-v2 directory..." if git diff --name-only HEAD^ HEAD | grep '^apps/backoffice-v2/' > /dev/null; then + echo "✅ Changes detected in backoffice-v2" echo "BACKOFFICE_UPDATED=true" >> "$GITHUB_ENV" echo "BACKOFFICE_UPDATED=true" >> "$GITHUB_OUTPUT" else + echo "ℹ️ No changes detected in backoffice-v2" echo "BACKOFFICE_UPDATED=false" >> "$GITHUB_ENV" echo "BACKOFFICE_UPDATED=false" >> "$GITHUB_OUTPUT" fiApply similar improvements to the KYB and Dashboard change detection steps.
🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 539-539: too few spaces after comma
(commas)
[error] 568-568: trailing spaces
(trailing-spaces)
[error] 577-577: trailing spaces
(trailing-spaces)
[warning] 595-595: too few spaces after comma
(commas)
[warning] 603-603: too few spaces after comma
(commas)
[warning] 611-611: too few spaces after comma
(commas)
[error] 615-615: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/deploy-wf-service.yml
(3 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/deploy-wf-service.yml
[error] 227-227: trailing spaces
(trailing-spaces)
[warning] 278-278: too few spaces after comma
(commas)
[error] 340-340: trailing spaces
(trailing-spaces)
[warning] 344-344: too few spaces after comma
(commas)
[warning] 416-416: too few spaces after comma
(commas)
[warning] 478-478: too few spaces after comma
(commas)
[error] 536-536: trailing spaces
(trailing-spaces)
[warning] 539-539: too few spaces after comma
(commas)
[error] 568-568: trailing spaces
(trailing-spaces)
[error] 577-577: trailing spaces
(trailing-spaces)
[warning] 595-595: too few spaces after comma
(commas)
[warning] 603-603: too few spaces after comma
(commas)
[warning] 611-611: too few spaces after comma
(commas)
[error] 615-615: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/deploy-wf-service.yml (1)
275-527
: 🛠️ Refactor suggestionImplement consistent timeouts and improve code reusability in status check jobs.
The status check jobs have several issues that need to be addressed:
- Missing timeouts in polling loops (could run indefinitely)
- Inconsistent step names
- Duplicated code across jobs
Here's a suggested implementation for a reusable timeout function:
+ # Add at the start of the workflow + env: + MAX_WAIT_TIME: 600 # 10 minutes in seconds + POLL_INTERVAL: 10 # seconds between checks + # Create a composite action in .github/actions/check-status/action.yml + name: 'Check ArgoCD Resource Status' + inputs: + resource-name: + required: true + resource-kind: + required: true + target-image-tag: + required: true + status-type: + required: true + default: 'Complete' + runs: + using: 'composite' + steps: + - name: Check Resource Status + shell: bash + run: | + start_time=$(date +%s) + while true; do + current_time=$(date +%s) + elapsed_time=$((current_time - start_time)) + + if [ $elapsed_time -gt ${{ env.MAX_WAIT_TIME }} ]; then + echo "❌ Timeout reached after ${elapsed_time} seconds" + exit 1 + fi + + # Your existing status check logic here + + sleep ${{ env.POLL_INTERVAL }} + done # Then update each job to use the composite action - name: Check Prisma Migrate Status + name: Check ${{ inputs.resource-name }} Status uses: ./.github/actions/check-status with: resource-name: test-wf-service-prisma-migrate resource-kind: Job target-image-tag: ${{ env.SHORT_HASH }}-${{ needs.build-and-push-ee-image.outputs.SUBMODULE_SHORT_HASH }}-${{ inputs.environment }} status-type: Complete🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 278-278: too few spaces after comma
(commas)
[error] 340-340: trailing spaces
(trailing-spaces)
[warning] 344-344: too few spaces after comma
(commas)
[warning] 408-408: too few spaces after comma
(commas)
[warning] 470-470: too few spaces after comma
(commas)
🧹 Nitpick comments (3)
.github/workflows/deploy-wf-service.yml (3)
227-254
: Add error handling for Helm chart updates.While the Helm chart update logic is correct, consider adding error handling for the YAML update operation to ensure the deployment can fail gracefully if the update fails.
Add error checking after the YAML update:
- name: Update workflow-service image version in the HelmChart uses: fjogeleit/yaml-update-action@main + id: yaml_update with: repository: ballerine-io/cloud-infra-config branch: main commitChange: true message: "Update ${{ inputs.environment }} wf-service image Version to ${{ env.SHORT_HASH }}-${{ needs.build-and-push-ee-image.outputs.SUBMODULE_SHORT_HASH }}-${{ inputs.environment }} - (Commit hash: ${{ github.sha }}, commit message: ${{ github.event.head_commit.message }})" token: ${{ secrets.GIT_TOKEN }} changes: | { "kubernetes/helm/wf-service-migration/${{steps.update_helm_check.outputs.file_name}}": { "image.tag": "${{ env.SHORT_HASH }}-${{ needs.build-and-push-ee-image.outputs.SUBMODULE_SHORT_HASH }}-${{ inputs.environment }}", "prismaMigrate.image.tag": "${{ env.SHORT_HASH }}-${{ needs.build-and-push-ee-image.outputs.SUBMODULE_SHORT_HASH }}-${{ inputs.environment }}", "dbMigrate.image.tag": "${{ env.SHORT_HASH }}-${{ needs.build-and-push-ee-image.outputs.SUBMODULE_SHORT_HASH }}-${{ inputs.environment }}", "dataSync.image.tag": "${{ env.SHORT_HASH }}-${{ needs.build-and-push-ee-image.outputs.SUBMODULE_SHORT_HASH }}-${{ inputs.environment }}" } } + - name: Check YAML update status + if: steps.yaml_update.outcome != 'success' + run: | + echo "Failed to update Helm chart values" + exit 1🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 227-227: trailing spaces
(trailing-spaces)
[error] 238-238: trailing spaces
(trailing-spaces)
529-607
: Improve error handling and clean up unused variables in app change detection.The app change detection logic has a few areas for improvement:
- Git diff command could fail silently
- Environment variables are set but not used (KYB_UPDATED and DASHBOARD_UPDATED)
- name: Check for Changes in apps/v1 id: check-kyb run: | + set -e # Exit on error if git diff --name-only HEAD^ HEAD | grep '^apps/kyb-app/' > /dev/null; then - echo "KYB_UPDATED=true" >> "$GITHUB_ENV" + echo "KYB_UPDATED=true" >> "$GITHUB_OUTPUT" else - echo "KYB_UPDATED=false" >> "$GITHUB_ENV" + echo "KYB_UPDATED=false" >> "$GITHUB_OUTPUT" fi - name: Check for Changes in apps/v1 id: check-dashboard run: | + set -e # Exit on error if git diff --name-only HEAD^ HEAD | grep '^apps/workflows-dashboard/' > /dev/null; then - echo "DASHBOARD_UPDATED=true" >> "$GITHUB_ENV" + echo "DASHBOARD_UPDATED=true" >> "$GITHUB_OUTPUT" else - echo "DASHBOARD_UPDATED=false" >> "$GITHUB_ENV" + echo "DASHBOARD_UPDATED=false" >> "$GITHUB_OUTPUT" fi🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 531-531: too few spaces after comma
(commas)
[error] 560-560: trailing spaces
(trailing-spaces)
[error] 569-569: trailing spaces
(trailing-spaces)
[warning] 587-587: too few spaces after comma
(commas)
[warning] 595-595: too few spaces after comma
(commas)
[warning] 603-603: too few spaces after comma
(commas)
[error] 607-607: no new line character at the end of file
(new-line-at-end-of-file)
227-227
: Fix formatting issues.The YAML file has several formatting inconsistencies:
- Trailing spaces on lines 227, 340, 528, 560, 569
- Insufficient spaces after commas on lines 278, 344, 408, 470, 531, 587, 595, 603
- Missing newline at end of file
Consider using a YAML formatter to automatically fix these issues.
Also applies to: 278-278, 340-340, 344-344, 408-408, 470-470, 528-528, 531-531, 560-560, 569-569, 587-587, 595-595, 603-603, 607-607
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 227-227: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/deploy-wf-service.yml
(3 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/deploy-wf-service.yml
[error] 227-227: trailing spaces
(trailing-spaces)
[warning] 278-278: too few spaces after comma
(commas)
[error] 340-340: trailing spaces
(trailing-spaces)
[warning] 344-344: too few spaces after comma
(commas)
[warning] 408-408: too few spaces after comma
(commas)
[warning] 470-470: too few spaces after comma
(commas)
[error] 528-528: trailing spaces
(trailing-spaces)
[warning] 531-531: too few spaces after comma
(commas)
[error] 560-560: trailing spaces
(trailing-spaces)
[error] 569-569: trailing spaces
(trailing-spaces)
[warning] 587-587: too few spaces after comma
(commas)
[warning] 595-595: too few spaces after comma
(commas)
[warning] 603-603: too few spaces after comma
(commas)
[error] 607-607: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build-and-push-image
- GitHub Check: Analyze (javascript)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (5)
.github/workflows/README.MD (5)
5-5
: Consider using a more descriptive image filename.The current image filename (
1b06ff8b-222b-4286-b31b-52b8b9612c85.png
) lacks meaning. Consider renaming it to something descriptive likeci-cd-architecture-diagram.png
for better maintainability.
9-11
: Add descriptions for each environment stage.The stages are listed without explaining their purposes. Consider adding brief descriptions to help readers understand the role of each environment:
- Dev Environment - SB Environments - Prod Environments + Dev Environment: Development and testing of new features + SB Environments: Staging/Beta environments for pre-production validation + Prod Environments: Production deployment for end users
14-24
: Add blank lines around the table.According to markdown best practices, tables should be surrounded by blank lines for better readability.
## List of Workflows + | Scenarios | Actions | |-----------|---------| | **Deploy to Dev Environment** | [Build and Deploy to Dev](https://github.com/ballerine-io/ballerine/actions/workflows/) | ... | **Deploy Dashboard** | [Dashboard action](https://github.com/ballerine-io/ballerine/blob/dev/.github/workflows/) | +🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
14-14: Tables should be surrounded by blank lines
null(MD058, blanks-around-tables)
34-34
: Standardize environment name formatting.Environment names are inconsistently formatted:
SB
(uppercase)prod
(lowercase)Consider standardizing to either all uppercase (
DEV
,SB
,PROD
) or all lowercase (dev
,sb
,prod
).Also applies to: 38-38
42-42
: Improve instruction phrasing variety.Consider rephrasing to avoid repetition:
- Pass the branch from which you want to run the migration code and the environment name on which you want to run the action. + Specify the source branch for migration code and target environment for execution. - Pass the branch and the environment on which you want to perform the hotfix and run the workflow. + Specify the source branch and target environment for the hotfix deployment.Also applies to: 45-45
🧰 Tools
🪛 LanguageTool
[style] ~42-~42: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...e and the environment name on which you want to run the action. 5. Perform Hotfix ...(REP_WANT_TO_VB)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
.github/workflows/1b06ff8b-222b-4286-b31b-52b8b9612c85.png
is excluded by!**/*.png
📒 Files selected for processing (1)
.github/workflows/README.MD
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
.github/workflows/README.MD
[style] ~42-~42: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...e and the environment name on which you want to run the action. 5. Perform Hotfix ...
(REP_WANT_TO_VB)
[style] ~45-~45: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...branch and the environment on which you want to perform the hotfix and run the workflow...
(REP_WANT_TO_VB)
🪛 markdownlint-cli2 (0.17.2)
.github/workflows/README.MD
14-14: Tables should be surrounded by blank lines
null
(MD058, blanks-around-tables)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
## List of Workflows | ||
| Scenarios | Actions | | ||
|-----------|---------| | ||
| **Deploy to Dev Environment** | [Build and Deploy to Dev](https://github.com/ballerine-io/ballerine/actions/workflows/) | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix incomplete workflow URLs.
The following workflow links are incomplete:
- Deploy to Dev: Missing workflow filename
- Dashboard action: Missing workflow filename
Please update with the correct workflow filenames.
Also applies to: 24-24
- Github Actions access | ||
- Repository permissions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Enhance prerequisites with specific requirements.
The current prerequisites are too vague. Consider adding more details:
- Github Actions access
- Repository permissions
+ GitHub Actions access:
+ - Workflow read/write permissions
+ - Environment deployment permissions
+ Repository permissions:
+ - Write access to deployment branches
+ - Access to environment secrets
+ - ArgoCD access tokens (if required)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
- Github Actions access | |
- Repository permissions | |
GitHub Actions access: | |
- Workflow read/write permissions | |
- Environment deployment permissions | |
Repository permissions: | |
- Write access to deployment branches | |
- Access to environment secrets | |
- ArgoCD access tokens (if required) |
Summary by CodeRabbit