Deployement: CD for kaapi guardrail in EC2#95
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughA new GitHub Actions workflow for deploying the staging branch to an EC2 instance was added, and the guardrail validator normalization now removes the Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant STS as AWS STS (assume-role)
participant SSM as AWS SSM
participant EC2 as Staging EC2 instance (SSM Agent)
participant Repo as origin remote
rect rgba(0,128,0,0.5)
GH->>STS: request temporary credentials (assume `EC2_DEPLOY_ROLE_ARN`)
STS-->>GH: short-lived AWS creds
end
rect rgba(0,0,255,0.5)
GH->>SSM: send-command (AWS-RunShellScript) targeting `STAGING_EC2_INSTANCE_ID` with script (uses BUILD_DIRECTORY)
SSM-->>GH: CommandId
end
rect rgba(255,165,0,0.5)
SSM->>EC2: deliver & execute script via SSM Agent
EC2->>Repo: git fetch && git reset --hard origin/enhancement/cd_staging
EC2->>EC2: build docker compose, run alembic upgrade, start containers, prune images
EC2-->>SSM: command invocation status & output
end
rect rgba(128,0,128,0.5)
GH->>SSM: poll get-command-invocation using CommandId until Success or failure/timeouts
SSM-->>GH: final status + stdout/stderr
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 49 minutes and 28 seconds.Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/app/schemas/guardrail_config.py (1)
78-88:⚠️ Potential issue | 🟡 MinorRemove the duplicate
nameentry fromdrop_fields.Line 87 repeats
"name", which is already present on Line 80. This is behaviorally harmless but triggers Ruff B033 and adds noise.Proposed fix
drop_fields = { "id", "name", "organization_id", "project_id", "stage", "is_enabled", "created_at", "updated_at", - "name", }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/schemas/guardrail_config.py` around lines 78 - 88, Remove the duplicate "name" entry from the drop_fields set in guardrail_config.py: locate the drop_fields set definition and delete the repeated "name" element so each field appears only once (this will silence Ruff B033 and clean up the set literal).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/cd-staging.yml:
- Around line 3-5: The workflow triggers on branches: [enhancement/cd_staging]
but the deployment step pulls from origin main; update the CD logic in
.github/workflows/cd-staging.yml so the deployed ref matches the trigger—for
example, change the git pull/reset in the EC2 deployment step to use the exact
commit ref ${{ github.sha }} (or checkout with ref: ${{ github.sha }}) or else
pull/reset to the staging branch name (the same branch listed in branches:
[enhancement/cd_staging]) instead of always pulling origin main; ensure the
deployment command references github.sha or the incoming ref to deploy the same
ref that triggered the workflow.
- Around line 48-55: The script currently runs `aws ssm wait command-executed`
under bash's default `set -e`, so if the waiter fails the subsequent `aws ssm
get-command-invocation` never runs; modify the block around `aws ssm wait
command-executed` to temporarily disable errexit (e.g., `set +e`), run `aws ssm
wait command-executed --command-id "$CMD_ID" --instance-id "$INSTANCE_ID"` and
capture its exit code into a variable, then always run `aws ssm
get-command-invocation --command-id "$CMD_ID" --instance-id "$INSTANCE_ID"
--query
'{Status:Status,Stdout:StandardOutputContent,Stderr:StandardErrorContent}'
--output json` to fetch stdout/stderr, and finally restore errexit (e.g., `set
-e`) and exit or handle based on the saved exit code so failures are preserved
while invocation output is still captured.
---
Outside diff comments:
In `@backend/app/schemas/guardrail_config.py`:
- Around line 78-88: Remove the duplicate "name" entry from the drop_fields set
in guardrail_config.py: locate the drop_fields set definition and delete the
repeated "name" element so each field appears only once (this will silence Ruff
B033 and clean up the set literal).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: eecfbdc4-2233-4a65-9639-5b6e7f14ba5d
📒 Files selected for processing (3)
.github/workflows/cd-staging.yml.github/workflows/continuous-integration.ymlbackend/app/schemas/guardrail_config.py
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.github/workflows/cd-staging.yml (2)
38-38:⚠️ Potential issue | 🟠 MajorDeploy the same ref that triggered this workflow.
Line 38 still pulls
origin main, so a push toenhancement/cd_stagingcan deploy unrelated code.Suggested fix
- --parameters commands='["set -eux","sudo -iu ec2-user bash -lc \"cd ${BUILD_DIRECTORY} && git fetch --all && git pull origin main && docker compose build && docker compose run --rm backend uv run alembic upgrade head && docker compose up -d --remove-orphans && docker image prune -f\""]' \ + --parameters commands='["set -eux","sudo -iu ec2-user bash -lc \"cd ${BUILD_DIRECTORY} && git fetch origin ${{ github.ref_name }} && git reset --hard origin/${{ github.ref_name }} && docker compose build && docker compose run --rm backend uv run alembic upgrade head && docker compose up -d --remove-orphans && docker image prune -f\""]' \🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/cd-staging.yml at line 38, The deploy command currently runs "git pull origin main" which can deploy code different from the workflow-triggering ref; change the remote git steps inside the commands parameter (the long commands string that operates in BUILD_DIRECTORY) to fetch and check out the exact commit/ref that triggered the workflow instead of pulling main—e.g., fetch origin and git checkout --detach <DEPLOY_REF> or git reset --hard <DEPLOY_SHA>; pass the workflow's commit SHA or ref (GITHUB_SHA or a workflow input) into the remote command and use that value in the git checkout/reset so the EC2 deploy uses the exact same ref that started the workflow.
49-56:⚠️ Potential issue | 🟡 MinorAlways fetch SSM invocation output, even when waiter fails.
If
aws ssm wait command-executedexits non-zero, the step stops beforeget-command-invocation, so stdout/stderr are lost when debugging failures.Suggested fix
+ set +e aws ssm wait command-executed \ --command-id "$CMD_ID" \ --instance-id "$INSTANCE_ID" + WAIT_RC=$? + set -e aws ssm get-command-invocation \ --command-id "$CMD_ID" \ --instance-id "$INSTANCE_ID" \ --query '{Status:Status,Stdout:StandardOutputContent,Stderr:StandardErrorContent}' \ --output json + exit "$WAIT_RC"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/cd-staging.yml around lines 49 - 56, The pipeline currently runs "aws ssm wait command-executed" which, if it exits non-zero, prevents the subsequent "aws ssm get-command-invocation" from running and loses stdout/stderr; modify the step so that "aws ssm get-command-invocation" always runs regardless of the wait's exit (e.g., run the wait with error-tolerant behavior like "set +e" before the aws ssm wait or append "|| true" to the wait, then capture the wait exit code if needed, and always execute the "aws ssm get-command-invocation" command to fetch '{Status:Status,Stdout:StandardOutputContent,Stderr:StandardErrorContent}' --output json for debugging).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/cd-staging.yml:
- Line 38: The parameter string passed to --parameters commands currently uses
single quotes which prevents expansion of the runner environment variable
BUILD_DIRECTORY; update the invocation that includes --parameters
commands='[...]' to use double quotes so ${BUILD_DIRECTORY} is expanded on the
GitHub Actions runner (ensure you escape the inner quotes used for sudo -iu
ec2-user bash -lc "..."); specifically modify the argument containing
--parameters commands and the substring cd ${BUILD_DIRECTORY} && git fetch --all
&& git pull origin main && docker compose build ... so the runner expands
BUILD_DIRECTORY before sending to SSM/EC2.
---
Duplicate comments:
In @.github/workflows/cd-staging.yml:
- Line 38: The deploy command currently runs "git pull origin main" which can
deploy code different from the workflow-triggering ref; change the remote git
steps inside the commands parameter (the long commands string that operates in
BUILD_DIRECTORY) to fetch and check out the exact commit/ref that triggered the
workflow instead of pulling main—e.g., fetch origin and git checkout --detach
<DEPLOY_REF> or git reset --hard <DEPLOY_SHA>; pass the workflow's commit SHA or
ref (GITHUB_SHA or a workflow input) into the remote command and use that value
in the git checkout/reset so the EC2 deploy uses the exact same ref that started
the workflow.
- Around line 49-56: The pipeline currently runs "aws ssm wait command-executed"
which, if it exits non-zero, prevents the subsequent "aws ssm
get-command-invocation" from running and loses stdout/stderr; modify the step so
that "aws ssm get-command-invocation" always runs regardless of the wait's exit
(e.g., run the wait with error-tolerant behavior like "set +e" before the aws
ssm wait or append "|| true" to the wait, then capture the wait exit code if
needed, and always execute the "aws ssm get-command-invocation" command to fetch
'{Status:Status,Stdout:StandardOutputContent,Stderr:StandardErrorContent}'
--output json for debugging).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ab9c955c-df2b-46b4-9f1f-516471aff5eb
📒 Files selected for processing (1)
.github/workflows/cd-staging.yml
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/cd-staging.yml:
- Line 38: Replace the failing use of "docker compose exec backend uv run
alembic upgrade head" with a one-off container run so migrations execute against
the newly built image; specifically, change the command referenced in the
workflow parameters to use "docker compose run --rm backend uv run alembic
upgrade head" (so the migration runs in a fresh temporary container before
"docker compose up -d" and avoids requiring an already-running backend
container).
- Around line 50-55: The polling loop using for i in {1..20} that runs aws ssm
get-command-invocation (with CMD_ID and INSTANCE_ID) lacks error handling for
transient SSM visibility errors; modify the call to capture the aws CLI exit
code and stderr, detect transient errors such as InvocationDoesNotExist (or
other temporary JSON/404-like messages), and on those errors sleep and continue
retrying instead of treating it as a fatal failure—only fail the step on
non-transient errors or after exhausting retries, and ensure STATUS is only set
from a successful aws call before further processing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7de35c0e-2e8b-4ac7-96da-fe56612e0f69
📒 Files selected for processing (1)
.github/workflows/cd-staging.yml
| jobs: | ||
| deploy: | ||
| runs-on: ubuntu-latest | ||
| environment: AWS_STAGING_ENV_SECRETS |
There was a problem hiding this comment.
| environment: AWS_STAGING_ENV_SECRETS | |
| environment: AWS_ENV_SECRETS |
Target issue is #70
Summary
Summary by CodeRabbit
Chores
Bug Fixes