Add EC2 deployment configuration for Celery workers and Flower, and update Docker Compose for Flower service #388
Conversation
…pdate Docker Compose for Flower service
WalkthroughAdds Flower monitoring and EC2 deployment configuration: new environment variables in Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Git Push
participant GH as GitHub Actions
participant ECR as AWS ECR
participant ECS as AWS ECS (Backend)
participant EC2 as EC2 Host (Celery/Flower)
Dev->>GH: Push to branch (staging|production)
rect rgb(238,246,255)
note over GH: Build & Push Images
GH->>ECR: Build & Push Backend Image
GH->>ECR: Build & Push Celery Image
end
rect rgb(240,255,240)
note over GH,ECS: Backend deployment to ECS
GH->>ECS: Deploy Backend to ECS (update service image)
end
rect rgb(255,248,240)
note over GH,EC2: Celery & Flower deploy to EC2
GH->>EC2: SCP generated deploy-celery.sh & deploy-flower.sh
GH->>EC2: SSH execute deploy-celery.sh (stop, pull, run workers)
GH->>EC2: SSH execute deploy-flower.sh (stop, pull, run Flower on FLOWER_PORT)
EC2->>ECR: docker login & pull (performed by scripts)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
.github/workflows/cd-production.yml (2)
92-116: Ensure Flower pulls the new image and enforce auth flags.The Flower script doesn’t login to ECR or docker pull; docker run may use a stale local tag. Also, Flower auth/flags aren’t passed, leaving the dashboard public if port 5555 is reachable.
Apply this diff:
- name: Deploy Flower to EC2 @@ # Create flower deployment script cat > deploy-flower.sh << 'EOF' #!/bin/bash + set -Eeuo pipefail # Stop existing flower container docker stop flower-production || true docker rm flower-production || true + # Login and pull latest image + aws ecr get-login-password --region ap-south-1 | docker login --username AWS --password-stdin $1 + docker pull $1/$2:production + # Start flower using the same celery image docker run -d --name flower-production --restart unless-stopped \ --env-file /home/ubuntu/.env.production \ -p 5555:5555 \ $1/$2:production \ - uv run celery -A app.celery.celery_app flower --port=5555 + uv run celery -A app.celery.celery_app flower \ + --port=${FLOWER_PORT:-5555} \ + --basic_auth="${FLOWER_BASIC_AUTH:-}" \ + --url_prefix="${FLOWER_URL_PREFIX:-}" \ + --max_tasks=${FLOWER_MAX_TASKS:-10000} EOFAdditionally, restrict access to 5555 via the EC2 security group or reverse-proxy behind auth.
1-116: Trim trailing whitespace in .github/workflows/cd-production.yml Remove trailing spaces on lines 64, 68, 72, 78–79, 85, 100, 104 and 112 to satisfy CI..github/workflows/cd-staging.yml (2)
90-114: Ensure Flower pulls the new image and enforce auth flags.Same issues as production: no ECR login/pull; no auth flags.
Apply this diff:
- name: Deploy Flower to EC2 @@ # Create flower deployment script cat > deploy-flower.sh << 'EOF' #!/bin/bash + set -Eeuo pipefail # Stop existing flower container docker stop flower-staging || true docker rm flower-staging || true + # Login and pull latest image + aws ecr get-login-password --region ap-south-1 | docker login --username AWS --password-stdin $1 + docker pull $1/$2:staging + # Start flower using the same celery image docker run -d --name flower-staging --restart unless-stopped \ --env-file /home/ubuntu/.env.staging \ -p 5555:5555 \ $1/$2:staging \ - uv run celery -A app.celery.celery_app flower --port=5555 + uv run celery -A app.celery.celery_app flower \ + --port=${FLOWER_PORT:-5555} \ + --basic_auth="${FLOWER_BASIC_AUTH:-}" \ + --url_prefix="${FLOWER_URL_PREFIX:-}" \ + --max_tasks=${FLOWER_MAX_TASKS:-10000} EOFAlso restrict port 5555 exposure via the security group.
1-114: Remove trailing whitespace in .github/workflows/cd-staging.yml
Trailing spaces at lines 62, 66, 70, 76–77, 83, 98, 102, 110 must be trimmed to pass CI.
🧹 Nitpick comments (7)
docker-compose.dev.yml (1)
115-117: Optional: remove hard dependency on celery-worker.Flower can start without waiting for a worker; it connects to the broker/backend. Dropping this dependency can speed up local bring-up and reduce coupling.
Apply this diff if you agree:
depends_on: - - redis - - rabbitmq - - celery-worker + - redis + - rabbitmq.github/workflows/cd-production.yml (2)
32-40: Use release tag in addition to latest; current TAG env is unused.Images are only tagged/pushed as latest. Tag with the Git ref as well to enable rollbacks and provenance.
Apply this diff:
- - name: Build and Push Backend Docker Image + - name: Build and Push Backend Docker Image env: REGISTRY: ${{ steps.login-ecr.outputs.registry }} REPOSITORY: ${{ github.event.repository.name }}-repo TAG: ${{ github.ref_name }} run: | - docker build -t $REGISTRY/$REPOSITORY:latest ./backend - docker push $REGISTRY/$REPOSITORY:latest + docker build -t $REGISTRY/$REPOSITORY:latest -t $REGISTRY/$REPOSITORY:$TAG ./backend + docker push $REGISTRY/$REPOSITORY:latest + docker push $REGISTRY/$REPOSITORY:$TAG
56-91: Harden the remote deploy script.Add strict mode to fail fast and improve reliability. Consider ensuring AWS CLI exists on the host (assumes instance profile). Minor, but recommended.
Apply this diff inside the heredoc (top of deploy-celery.sh):
#!/bin/bash +set -Eeuo pipefail +# Optional: verify AWS CLI present +command -v aws >/dev/null 2>&1 || { echo "aws CLI not found on host"; exit 1; }.env.example (2)
84-86: Fix dotenv style (spaces around =).Clean up spacing to satisfy linters.
Based on static analysis hints
Apply this diff:-CALLBACK_CONNECT_TIMEOUT = 3 -CALLBACK_READ_TIMEOUT = 10 +CALLBACK_CONNECT_TIMEOUT=3 +CALLBACK_READ_TIMEOUT=10
88-101: Key ordering/blank line nits from dotenv-linter.Reorder keys as suggested and remove extra blank line to keep lint green. Low priority.
Based on static analysis hints
If you want to conform strictly, reorder:
- EC2_STAGING_KEY_NAME before EC2_STAGING_USER
- EC2_PRODUCTION_KEY_NAME before EC2_PRODUCTION_USER
And remove the extra blank line at Line 87..github/workflows/cd-staging.yml (2)
34-41: Tag backend image with a unique tag as well as latest.Mirror production: use a stable tag (e.g., commit or “staging”) for traceability.
Apply this diff:
- - name: Build and Push Backend Docker Image + - name: Build and Push Backend Docker Image env: REGISTRY: ${{ steps.login-ecr.outputs.registry }} REPOSITORY: ${{ github.event.repository.name }}-staging-repo run: | - docker build -t $REGISTRY/$REPOSITORY:latest ./backend - docker push $REGISTRY/$REPOSITORY:latest + docker build -t $REGISTRY/$REPOSITORY:latest -t $REGISTRY/$REPOSITORY:${{ github.sha }} ./backend + docker push $REGISTRY/$REPOSITORY:latest + docker push $REGISTRY/$REPOSITORY:${{ github.sha }}
54-89: Harden the remote deploy script.Add strict mode; optional check for AWS CLI presence.
Apply this diff inside deploy-celery.sh heredoc:
#!/bin/bash +set -Eeuo pipefail +command -v aws >/dev/null 2>&1 || { echo "aws CLI not found on host"; exit 1; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.env.example(1 hunks).github/workflows/cd-production.yml(3 hunks).github/workflows/cd-staging.yml(1 hunks)docker-compose.dev.yml(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: AI Platform CI
.github/workflows/cd-staging.yml
[error] 1-1: Trailing whitespace detected and removed by pre-commit hook.
.github/workflows/cd-production.yml
[error] 1-1: Trailing whitespace detected and removed by pre-commit hook.
.env.example
[error] 1-1: Trailing whitespace detected and removed by pre-commit hook.
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 79-79: [UnorderedKey] The FLOWER_BASIC_AUTH key should go before the FLOWER_PORT key
(UnorderedKey)
[warning] 81-81: [UnorderedKey] The FLOWER_MAX_TASKS key should go before the FLOWER_PORT key
(UnorderedKey)
[warning] 84-84: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 85-85: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 87-87: [ExtraBlankLine] Extra blank line detected
(ExtraBlankLine)
[warning] 92-92: [UnorderedKey] The EC2_STAGING_KEY_NAME key should go before the EC2_STAGING_USER key
(UnorderedKey)
[warning] 97-97: [UnorderedKey] The EC2_PRODUCTION_KEY_NAME key should go before the EC2_PRODUCTION_USER key
(UnorderedKey)
| # Flower Configuration (Celery Monitoring) | ||
| FLOWER_PORT=5555 | ||
| FLOWER_BASIC_AUTH=admin:changethis | ||
| FLOWER_URL_PREFIX= | ||
| FLOWER_MAX_TASKS=10000 |
There was a problem hiding this comment.
Flower defaults: warn and align with CLI usage.
Good to document, but note Flower won’t auto-read these names; they must be passed via CLI flags (addressed in workflows/compose). Also, don’t ship weak defaults.
Apply this diff to discourage weak creds:
-FLOWER_BASIC_AUTH=admin:changethis
+FLOWER_BASIC_AUTH=please-change-user:please-change-password📝 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.
| # Flower Configuration (Celery Monitoring) | |
| FLOWER_PORT=5555 | |
| FLOWER_BASIC_AUTH=admin:changethis | |
| FLOWER_URL_PREFIX= | |
| FLOWER_MAX_TASKS=10000 | |
| # Flower Configuration (Celery Monitoring) | |
| FLOWER_PORT=5555 | |
| FLOWER_BASIC_AUTH=please-change-user:please-change-password | |
| FLOWER_URL_PREFIX= | |
| FLOWER_MAX_TASKS=10000 |
🧰 Tools
🪛 dotenv-linter (3.3.0)
[warning] 79-79: [UnorderedKey] The FLOWER_BASIC_AUTH key should go before the FLOWER_PORT key
(UnorderedKey)
[warning] 81-81: [UnorderedKey] The FLOWER_MAX_TASKS key should go before the FLOWER_PORT key
(UnorderedKey)
🤖 Prompt for AI Agents
In .env.example around lines 77 to 81, replace the weak default
FLOWER_BASIC_AUTH and add a warning that these env vars are not automatically
applied by Flower and must be passed to the Flower CLI via flags (e.g.,
--basic_auth) in compose/workflow files; remove the insecure default value and
replace it with a blank or a strong placeholder like "admin:CHANGE_ME" and add a
short comment telling users to set a strong password and not to ship with
defaults, keeping FLOWER_PORT, FLOWER_URL_PREFIX and FLOWER_MAX_TASKS but
documenting their CLI-equivalent usage.
| command: ["uv", "run", "celery", "-A", "app.celery.celery_app", "flower", "--port=5555"] | ||
|
|
There was a problem hiding this comment.
Secure Flower: enforce auth and use env-driven flags (switch to shell form).
Flower is exposed on 0.0.0.0:5555 without basic auth or URL prefix; env vars in .env.example are not consumed by Flower unless passed as CLI flags. Also, array-form command won’t reliably expand env vars. Use shell-form, add auth, and honor FLOWER_*.
Apply this diff:
- command: ["uv", "run", "celery", "-A", "app.celery.celery_app", "flower", "--port=5555"]
+ command: uv run celery -A app.celery.celery_app flower \
+ --port=${FLOWER_PORT:-5555} \
+ --basic_auth="${FLOWER_BASIC_AUTH:-}" \
+ --url_prefix="${FLOWER_URL_PREFIX:-}" \
+ --max_tasks=${FLOWER_MAX_TASKS:-10000}📝 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.
| command: ["uv", "run", "celery", "-A", "app.celery.celery_app", "flower", "--port=5555"] | |
| command: uv run celery -A app.celery.celery_app flower \ | |
| --port=${FLOWER_PORT:-5555} \ | |
| --basic_auth="${FLOWER_BASIC_AUTH:-}" \ | |
| --url_prefix="${FLOWER_URL_PREFIX:-}" \ | |
| --max_tasks=${FLOWER_MAX_TASKS:-10000} |
🤖 Prompt for AI Agents
In docker-compose.dev.yml around lines 118-119, the current array-form command
launches Flower without basic auth, without honoring .env vars, and array form
prevents shell expansion; change to shell form so env vars expand and add flags
to enforce auth and optional URL prefix. Replace the array with a single shell
command that runs uv run celery -A app.celery.celery_app flower and passes flags
using environment variables (e.g. --port=${FLOWER_PORT:-5555},
--basic_auth=${FLOWER_BASIC_AUTH}, --url_prefix=${FLOWER_URL_PREFIX:-/flower})
and include a passthrough ${FLOWER_EXTRA_ARGS:-} so additional flags from .env
can be consumed; ensure FLOWER_BASIC_AUTH is set in .env (or fail-fast/validate)
so Flower is not exposed without credentials.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
.env.example (2)
77-81: Weak default password and Flower CLI flag requirement.The
FLOWER_BASIC_AUTH=admin:changethisdefault is insecure and should not be shipped as-is. Additionally, Flower does not automatically read these environment variables—they must be explicitly passed as CLI flags (e.g.,--basic_auth,--port,--url_prefix).Based on past review comments, apply this diff to use a more obvious placeholder:
-FLOWER_BASIC_AUTH=admin:changethis +FLOWER_BASIC_AUTH=please-change-user:please-change-passwordAdd a comment documenting that these must be passed as CLI flags:
# Flower Configuration (Celery Monitoring) +# Note: Flower does not auto-read these env vars. Pass them as CLI flags: +# --basic_auth=$FLOWER_BASIC_AUTH --port=$FLOWER_PORT --url_prefix=$FLOWER_URL_PREFIX --max_tasks=$FLOWER_MAX_TASKS FLOWER_PORT=5555
99-100: Remove unused DOCKER_IMAGE_CELERY variable.The
DOCKER_IMAGE_CELERY=celery-workerentry is not used in the workflows. The CD workflow (cd-staging.yml line 45) constructs the repository name directly as${github.event.repository.name}-celery-repowithout referencing this environment variable.Remove this unused variable to avoid confusion:
-# Docker Registry for Celery Images -DOCKER_IMAGE_CELERY=celery-workerBased on past review comments.
🧹 Nitpick comments (4)
.github/workflows/cd-staging.yml (2)
54-88: Add timeout and connection retry for SSH operations.SSH operations (lines 87-88) have no timeout, which can cause the workflow to hang indefinitely if the EC2 instance is unreachable or slow to respond.
Add connection timeout and retry logic:
- scp -i staging_key.pem -o StrictHostKeyChecking=no deploy-celery.sh ubuntu@${{ secrets.EC2_STAGING_HOST }}:/tmp/ - ssh -i staging_key.pem -o StrictHostKeyChecking=no ubuntu@${{ secrets.EC2_STAGING_HOST }} "chmod +x /tmp/deploy-celery.sh && /tmp/deploy-celery.sh $REGISTRY $REPOSITORY" + scp -i staging_key.pem -o StrictHostKeyChecking=no -o ConnectTimeout=10 -o ConnectionAttempts=3 deploy-celery.sh ubuntu@${{ secrets.EC2_STAGING_HOST }}:/tmp/ + ssh -i staging_key.pem -o StrictHostKeyChecking=no -o ConnectTimeout=10 -o ConnectionAttempts=3 ubuntu@${{ secrets.EC2_STAGING_HOST }} "chmod +x /tmp/deploy-celery.sh && /tmp/deploy-celery.sh $REGISTRY $REPOSITORY"
112-113: Add timeout and connection retry for Flower SSH operations.Similar to the Celery deployment, SSH operations for Flower have no timeout.
Add connection timeout and retry logic:
- scp -i staging_key.pem -o StrictHostKeyChecking=no deploy-flower.sh ubuntu@${{ secrets.EC2_STAGING_HOST }}:/tmp/ - ssh -i staging_key.pem -o StrictHostKeyChecking=no ubuntu@${{ secrets.EC2_STAGING_HOST }} "chmod +x /tmp/deploy-flower.sh && /tmp/deploy-flower.sh $REGISTRY $REPOSITORY" + scp -i staging_key.pem -o StrictHostKeyChecking=no -o ConnectTimeout=10 -o ConnectionAttempts=3 deploy-flower.sh ubuntu@${{ secrets.EC2_STAGING_HOST }}:/tmp/ + ssh -i staging_key.pem -o StrictHostKeyChecking=no -o ConnectTimeout=10 -o ConnectionAttempts=3 ubuntu@${{ secrets.EC2_STAGING_HOST }} "chmod +x /tmp/deploy-flower.sh && /tmp/deploy-flower.sh $REGISTRY $REPOSITORY".env.example (2)
84-85: Remove spaces around equals signs.Lines 84-85 have spaces around the
=operator, which is inconsistent with standard .env file conventions and flagged by the linter.Apply this diff:
-CALLBACK_CONNECT_TIMEOUT = 3 -CALLBACK_READ_TIMEOUT = 10 +CALLBACK_CONNECT_TIMEOUT=3 +CALLBACK_READ_TIMEOUT=10
87-87: Remove extra blank line.Line 87 is an extra blank line flagged by the linter.
Remove the extra blank line to match the file's formatting style.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.env.example(1 hunks).github/workflows/cd-production.yml(3 hunks).github/workflows/cd-staging.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/cd-production.yml
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 79-79: [UnorderedKey] The FLOWER_BASIC_AUTH key should go before the FLOWER_PORT key
(UnorderedKey)
[warning] 81-81: [UnorderedKey] The FLOWER_MAX_TASKS key should go before the FLOWER_PORT key
(UnorderedKey)
[warning] 84-84: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 85-85: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 87-87: [ExtraBlankLine] Extra blank line detected
(ExtraBlankLine)
[warning] 92-92: [UnorderedKey] The EC2_STAGING_KEY_NAME key should go before the EC2_STAGING_USER key
(UnorderedKey)
[warning] 97-97: [UnorderedKey] The EC2_PRODUCTION_KEY_NAME key should go before the EC2_PRODUCTION_USER key
(UnorderedKey)
🔇 Additional comments (5)
.github/workflows/cd-staging.yml (4)
34-40: LGTM: Clear naming improvement.The rename clarifies this step builds the backend image, distinguishing it from the new Celery image build step.
42-48: Verify tagging strategy consistency.The backend image is tagged with
latest(line 39), while the Celery image usesstaging(line 47). This inconsistency could lead to confusion about which images are deployed where. Ensure this difference is intentional and aligns with your deployment strategy.
50-52: LGTM: Clear naming improvement.The rename clarifies this step deploys the backend to ECS, distinguishing it from the new Celery/Flower EC2 deployments.
104-108: Document Flower security configuration.Flower is exposed on port 5555 (line 106) with basic authentication from the env file. Ensure that:
- EC2 security groups restrict access to port 5555 (e.g., only from VPN or office IPs)
- The FLOWER_BASIC_AUTH in the env file uses a strong password (not the weak default from .env.example)
- Consider using HTTPS/TLS termination if Flower is exposed beyond localhost
.env.example (1)
88-97: LGTM: EC2 configuration placeholders are appropriate.The EC2 deployment configuration uses clear placeholder values that guide users to replace them with actual values. The naming convention (EC2_{ENVIRONMENT}_{ATTRIBUTE}) is consistent and self-documenting.
…iable names and enhance error handling
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/cd-production.yml (1)
103-139: Flower on EC2: add ECR login/pull and enforce auth; avoid exposing 5555 unauthenticated.Currently runs Flower without basic auth and doesn’t login to ECR; if image isn’t present, pull will fail. Also hardcodes port 5555.
Apply:
- ECR login + pull
- Source remote env to parameterize host port
- Pass --basic_auth/--url_prefix/--max_tasks/--port to Flower
- Optionally bind to 127.0.0.1 or restrict via SG
cat > deploy-flower.sh << 'EOF' #!/bin/bash set -e + # Load environment for host-side vars (e.g., FLOWER_PORT) + set -a + [ -f /home/ubuntu/.env.production ] && . /home/ubuntu/.env.production + set +a + # Stop existing flower container docker stop flower-production 2>/dev/null || true docker rm flower-production 2>/dev/null || true - # Start flower using the same celery image + # Ensure ECR login and pull image + aws ecr get-login-password --region ap-south-1 | docker login --username AWS --password-stdin $1 + docker pull $1/$2:production + + # Start flower using the same celery image (secured) docker run -d --name flower-production --restart unless-stopped \ --env-file /home/ubuntu/.env.production \ - -p 5555:5555 \ + -p ${FLOWER_PORT:-5555}:5555 \ $1/$2:production \ - uv run celery -A app.celery.celery_app flower --port=5555 + uv run celery -A app.celery.celery_app flower \ + --port=${FLOWER_PORT:-5555} \ + ${FLOWER_BASIC_AUTH:+--basic_auth="${FLOWER_BASIC_AUTH}"} \ + ${FLOWER_URL_PREFIX:+--url_prefix="${FLOWER_URL_PREFIX}"} \ + ${FLOWER_MAX_TASKS:+--max_tasks="${FLOWER_MAX_TASKS}"} + + # If not behind a reverse proxy/VPN, consider binding to localhost: + # replace -p ${FLOWER_PORT:-5555}:5555 with -p 127.0.0.1:${FLOWER_PORT:-5555}:5555Note: if your security model exposes 5555 publicly, basic auth is mandatory and SG should restrict source IPs.
.github/workflows/cd-staging.yml (1)
101-137: Secure Flower in staging and ensure image auth/pull.Mirror the production fixes: login to ECR, docker pull, pass auth/limits/port flags, and parametrize port from remote env. Optionally bind to localhost.
cat > deploy-flower.sh << 'EOF' #!/bin/bash set -e + set -a + [ -f /home/ubuntu/.env.staging ] && . /home/ubuntu/.env.staging + set +a # Stop existing flower container docker stop flower-staging 2>/dev/null || true docker rm flower-staging 2>/dev/null || true - # Start flower using the same celery image + # Ensure ECR login and pull + aws ecr get-login-password --region ap-south-1 | docker login --username AWS --password-stdin $1 + docker pull $1/$2:staging + + # Start flower using the same celery image (secured) docker run -d --name flower-staging --restart unless-stopped \ --env-file /home/ubuntu/.env.staging \ - -p 5555:5555 \ + -p ${FLOWER_PORT:-5555}:5555 \ $1/$2:staging \ - uv run celery -A app.celery.celery_app flower --port=5555 + uv run celery -A app.celery.celery_app flower \ + --port=${FLOWER_PORT:-5555} \ + ${FLOWER_BASIC_AUTH:+--basic_auth="${FLOWER_BASIC_AUTH}"} \ + ${FLOWER_URL_PREFIX:+--url_prefix="${FLOWER_URL_PREFIX}"} \ + ${FLOWER_MAX_TASKS:+--max_tasks="${FLOWER_MAX_TASKS}"}If exposed publicly, enforce basic auth and restrict SG sources; otherwise prefer binding to 127.0.0.1 with a reverse proxy.
♻️ Duplicate comments (1)
.env.example (1)
77-81: Flower defaults: fix weak creds, document CLI flags, and order keys.Use strong placeholders, and note these vars must be passed to Flower via CLI flags. Also resolve dotenv-linter UnorderedKey warnings.
-# Flower Configuration (Celery Monitoring) -FLOWER_PORT=5555 -FLOWER_BASIC_AUTH=admin:changethis -FLOWER_URL_PREFIX= -FLOWER_MAX_TASKS=10000 +# Flower Configuration (Celery Monitoring) +# NOTE: Flower does NOT auto-read these; pass as CLI flags in compose/workflows, e.g.: +# flower --port="${FLOWER_PORT}" --basic_auth="${FLOWER_BASIC_AUTH}" \ +# --url_prefix="${FLOWER_URL_PREFIX}" --max_tasks="${FLOWER_MAX_TASKS}" +FLOWER_BASIC_AUTH=admin:CHANGE_ME_STRONG +FLOWER_MAX_TASKS=10000 +FLOWER_PORT=5555 +FLOWER_URL_PREFIX=
🧹 Nitpick comments (4)
.env.example (1)
83-86: dotenv-linter: spacing and extra blank line.Remove spaces around “=” and the extra blank line.
-CALLBACK_CONNECT_TIMEOUT = 3 -CALLBACK_READ_TIMEOUT = 10 - +CALLBACK_CONNECT_TIMEOUT=3 +CALLBACK_READ_TIMEOUT=10Also applies to: 87-88
.github/workflows/cd-production.yml (1)
32-40: Use immutable tags (and latest) when pushing backend image; TAG is currently unused.Push both the git tag and latest for traceability; you already set TAG but don’t use it.
- - name: Build and Push Backend Docker Image + - name: Build and Push Backend Docker Image env: REGISTRY: ${{ steps.login-ecr.outputs.registry }} REPOSITORY: ${{ github.event.repository.name }}-repo - TAG: ${{ github.ref_name }} + TAG: ${{ github.ref_name }} run: | - docker build -t $REGISTRY/$REPOSITORY:latest ./backend - docker push $REGISTRY/$REPOSITORY:latest + docker build -t $REGISTRY/$REPOSITORY:$TAG -t $REGISTRY/$REPOSITORY:latest ./backend + docker push $REGISTRY/$REPOSITORY:$TAG + docker push $REGISTRY/$REPOSITORY:latest.github/workflows/cd-staging.yml (2)
54-99: Celery EC2 staging deploy: improvements already applied.
- Good: set -e, region param, startup verification, secrets cleanup.
- Optional: add log rotation options or --health-cmd on workers.
130-136: StrictHostKeyChecking=no: acceptable for bootstrap; consider pinning host key.To reduce MITM risk, pre-populate known_hosts via ssh-keyscan.
- scp -i staging_key.pem -o StrictHostKeyChecking=no deploy-flower.sh ${{ secrets.EC2_USER }}@${{ secrets.EC2_HOST }}:/tmp/ - ssh -i staging_key.pem -o StrictHostKeyChecking=no ${{ secrets.EC2_USER }}@${{ secrets.EC2_HOST }} "chmod +x /tmp/deploy-flower.sh && /tmp/deploy-flower.sh $REGISTRY $REPOSITORY" + ssh-keyscan -H "${{ secrets.EC2_HOST }}" >> ~/.ssh/known_hosts + scp -i staging_key.pem deploy-flower.sh ${{ secrets.EC2_USER }}@${{ secrets.EC2_HOST }}:/tmp/ + ssh -i staging_key.pem ${{ secrets.EC2_USER }}@${{ secrets.EC2_HOST }} "chmod +x /tmp/deploy-flower.sh && /tmp/deploy-flower.sh $REGISTRY $REPOSITORY"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.env.example(1 hunks).github/workflows/cd-production.yml(3 hunks).github/workflows/cd-staging.yml(1 hunks)
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 79-79: [UnorderedKey] The FLOWER_BASIC_AUTH key should go before the FLOWER_PORT key
(UnorderedKey)
[warning] 81-81: [UnorderedKey] The FLOWER_MAX_TASKS key should go before the FLOWER_PORT key
(UnorderedKey)
[warning] 84-84: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 85-85: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 87-87: [ExtraBlankLine] Extra blank line detected
(ExtraBlankLine)
[warning] 91-91: [UnorderedKey] The EC2_KEY key should go before the EC2_USER key
(UnorderedKey)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: checks (3.11.7, 6)
🔇 Additional comments (2)
.github/workflows/cd-staging.yml (1)
42-49: Celery image build/push for staging: LGTM..github/workflows/cd-production.yml (1)
56-102: Verify ‘uv’ CLI availability in the Docker image
I didn’t see ‘uv’ being installed inbackend/Dockerfile.celery; confirm the ‘uv’ binary is present in the production image or add installation steps.
| # EC2 Deployment Configuration for Celery Workers | ||
| EC2_HOST=production-celery-host.example.com | ||
| EC2_USER=ubuntu | ||
| EC2_KEY=production-ec2-key |
There was a problem hiding this comment.
Don’t suggest storing SSH private keys in env files.
EC2_KEY in an env example nudges insecure practices. Use GitHub Actions secrets or SSM Parameter Store; keep only host/user hints in .env.example.
-# EC2 Deployment Configuration for Celery Workers
-EC2_HOST=production-celery-host.example.com
-EC2_USER=ubuntu
-EC2_KEY=production-ec2-key
+# EC2 Deployment Configuration (reference only; use GitHub Secrets/SSM in CI)
+EC2_HOST=production-celery-host.example.com
+EC2_USER=ubuntu
+# EC2_KEY is intentionally not listed; store as a secret (not in .env files).📝 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.
| # EC2 Deployment Configuration for Celery Workers | |
| EC2_HOST=production-celery-host.example.com | |
| EC2_USER=ubuntu | |
| EC2_KEY=production-ec2-key | |
| # EC2 Deployment Configuration (reference only; use GitHub Secrets/SSM in CI) | |
| EC2_HOST=production-celery-host.example.com | |
| EC2_USER=ubuntu | |
| # EC2_KEY is intentionally not listed; store as a secret (not in .env files). |
🧰 Tools
🪛 dotenv-linter (3.3.0)
[warning] 91-91: [UnorderedKey] The EC2_KEY key should go before the EC2_USER key
(UnorderedKey)
🤖 Prompt for AI Agents
In .env.example around lines 88 to 91, remove the EC2_KEY entry (do not suggest
storing SSH private keys in env files) and replace it with a short hint or
placeholder indicating that SSH keys should be managed via a secrets store
(e.g., GitHub Actions Secrets, AWS SSM/Secrets Manager) or referenced by a path
to a key stored outside source control; keep EC2_HOST and EC2_USER as examples
only, and update the project README or a deployment docs section to explicitly
instruct contributors to use secrets management for private keys and not to
commit keys to the repository.
1 participant
Summary
Target issue is #PLEASE_TYPE_ISSUE_NUMBER
Explain the motivation for making this change. What existing problem does the pull request solve?
Checklist
Before submitting a pull request, please ensure that you mark these task.
fastapi run --reload app/main.pyordocker compose upin the repository root and test.Notes
Please add here if any other information is required for the reviewer.
Summary by CodeRabbit