K8s deployment - #94
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build-images.yml:
- Around line 17-19: Restrict workflow permissions by removing packages: write
from the root permissions and adding it only under jobs.build; leave contents:
read at the workflow level. In .github/workflows/build-images.yml lines 47-51
and 96-97, update both actions/checkout@v4 steps to set persist-credentials:
false.
- Around line 86-94: Move concurrency configuration from the deploy job to the
workflow level so it is acquired before build starts, using a branch/ref-based
group and cancellation of in-progress runs. Remove the deploy-level concurrency
block and configure cancellation to prevent obsolete builds from reaching the
deploy job with a stale SHA; preserve the existing deploy needs and runner
settings.
In `@docker-compose.yml`:
- Line 45: Require a non-empty MOTHRA_SECRET in both docker-compose.yml backend
line 45 and worker line 68 by using Compose’s mandatory, non-empty variable
syntax; apply the same guard at both environment entries so startup fails when
the host variable is missing or empty.
In `@k8s/configmap.yaml`:
- Line 9: Remove CELERY_BROKER_URL from k8s/configmap.yaml and provide it
through the mothra-secrets configuration consumed by backend and worker
components. In k8s/redis.yaml, require Redis password or ACL authentication and
add a NetworkPolicy allowing ingress only from backend and worker Pods; apply
the requested changes at k8s/configmap.yaml lines 9-9 and k8s/redis.yaml lines
19-40.
In `@k8s/README.md`:
- Line 16: Declare languages for all four fenced blocks in the README: use text
for the directory tree and bash for each command block, including the additional
fences at the referenced locations, while preserving their contents.
In `@k8s/redis.yaml`:
- Around line 17-40: Harden the pod/container security contexts in
k8s/redis.yaml (17-40), k8s/backend.yaml (17-56), k8s/worker.yaml (17-60),
k8s/ic.yaml (17-48), and k8s/text-service.yaml (17-45) by enforcing non-root
execution, disabling privilege escalation, dropping capabilities, and using
seccomp RuntimeDefault. Enable read-only root filesystems where supported; give
Redis an explicit writable /data volume, and ensure the shared model-storage
volumes in backend and worker are writable by their selected UIDs.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 543f1b55-a23c-429b-adda-02979b41abf9
📒 Files selected for processing (14)
.github/workflows/build-images.yml.gitignoredocker-compose.ymlk8s/README.mdk8s/backend.yamlk8s/configmap.yamlk8s/ic.yamlk8s/ingress.yamlk8s/redis.yamlk8s/secret.yaml.examplek8s/stored-models-pv.yamlk8s/stored-models-pvc.yamlk8s/text-service.yamlk8s/worker.yaml
| permissions: | ||
| contents: read | ||
| packages: write |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files .github/workflows/build-images.yml
wc -l .github/workflows/build-images.yml
cat -n .github/workflows/build-images.yml | sed -n '1,220p'Repository: DDMAL/mothra
Length of output: 6018
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "persist-credentials|git (push|fetch|submodule|commit|tag)|actions/checkout@v4" .github/workflows/build-images.ymlRepository: DDMAL/mothra
Length of output: 224
Limit GitHub token scope in this workflow
- Move
packages: writefrom the workflow root tojobs.build; the deploy job doesn’t need it. - Add
persist-credentials: falseto bothactions/checkout@v4steps so the token isn’t left in the repo config after checkout.
🧰 Tools
🪛 zizmor (1.26.1)
[error] 19-19: overly broad permissions (excessive-permissions): packages: write is overly broad at the workflow level
(excessive-permissions)
📍 Affects 1 file
.github/workflows/build-images.yml#L17-L19(this comment).github/workflows/build-images.yml#L47-L51.github/workflows/build-images.yml#L96-L97
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/build-images.yml around lines 17 - 19, Restrict workflow
permissions by removing packages: write from the root permissions and adding it
only under jobs.build; leave contents: read at the workflow level. In
.github/workflows/build-images.yml lines 47-51 and 96-97, update both
actions/checkout@v4 steps to set persist-credentials: false.
Source: Linters/SAST tools
| deploy: | ||
| # Runs only after all three images are built & pushed, so the sha-tagged | ||
| # images are guaranteed to exist before we roll the deployments. | ||
| needs: build | ||
| runs-on: ubuntu-latest | ||
| # Serialize deploys per branch; don't cancel an in-flight rollout. | ||
| concurrency: | ||
| group: deploy-${{ github.ref }} | ||
| cancel-in-progress: false |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Prevent an older build from rolling back a newer deployment.
Concurrency is acquired only after build. A slower earlier push can finish after a newer push has deployed, then acquire this lock and apply its stale SHA. Use workflow-level concurrency with cancellation so obsolete builds never reach deploy.
Proposed fix
+concurrency:
+ group: ci-cd-${{ github.ref }}
+ cancel-in-progress: true
+
jobs:
build:
...
deploy:
- concurrency:
- group: deploy-${{ github.ref }}
- cancel-in-progress: false📝 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.
| deploy: | |
| # Runs only after all three images are built & pushed, so the sha-tagged | |
| # images are guaranteed to exist before we roll the deployments. | |
| needs: build | |
| runs-on: ubuntu-latest | |
| # Serialize deploys per branch; don't cancel an in-flight rollout. | |
| concurrency: | |
| group: deploy-${{ github.ref }} | |
| cancel-in-progress: false | |
| concurrency: | |
| group: ci-cd-${{ github.ref }} | |
| cancel-in-progress: true | |
| jobs: | |
| build: | |
| ... | |
| deploy: | |
| # Runs only after all three images are built & pushed, so the sha-tagged | |
| # images are guaranteed to exist before we roll the deployments. | |
| needs: build | |
| runs-on: ubuntu-latest | |
| # Serialize deploys per branch; don't cancel an in-flight rollout. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/build-images.yml around lines 86 - 94, Move concurrency
configuration from the deploy job to the workflow level so it is acquired before
build starts, using a branch/ref-based group and cancellation of in-progress
runs. Remove the deploy-level concurrency block and configure cancellation to
prevent obsolete builds from reaching the deploy job with a stale SHA; preserve
the existing deploy needs and runner settings.
| DATABASE_URL: ${DATABASE_URL} | ||
| # Defaults to the bundled postgres service; set DATABASE_URL in .env to use an external/managed DB. | ||
| DATABASE_URL: ${DATABASE_URL:-postgresql://mothra:mothra@postgres:5432/mothra} | ||
| MOTHRA_SECRET: ${MOTHRA_SECRET} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== docker-compose.yml ==\n'
cat -n docker-compose.yml | sed -n '1,140p'
printf '\n== auth_api.py ==\n'
fd -a 'auth_api.py' .Repository: DDMAL/mothra
Length of output: 3094
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== landing-page/scripts/auth_api.py ==\n'
cat -n landing-page/scripts/auth_api.py | sed -n '1,220p'
printf '\n== MOTHRA_SECRET references ==\n'
rg -n "MOTHRA_SECRET|jwt|HS256|SECRET" landing-page -g '!**/__pycache__/**'Repository: DDMAL/mothra
Length of output: 10252
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== worker/celery entrypoints ==\n'
rg -n "auth_api|MOTHRA_SECRET|SECRET_KEY|jwt|jose" landing-page/scripts landing-page -g '!**/__pycache__/**' -g '!**/node_modules/**'
printf '\n== celery app file(s) ==\n'
fd -a 'celery_app.py' landing-page
fd -a 'celery.py' landing-pageRepository: DDMAL/mothra
Length of output: 5880
Fail fast when MOTHRA_SECRET is empty.
docker-compose.yml turns a missing host variable into an empty value here, and auth_api.py treats that as a real secret instead of using its random fallback. That leaves JWTs signed with a predictable empty key.
docker-compose.yml#L45-L45: require a non-emptyMOTHRA_SECRETfor the backend.docker-compose.yml#L68-L68: apply the same guard for the worker.
Proposed fix
- MOTHRA_SECRET: ${MOTHRA_SECRET}
+ MOTHRA_SECRET: ${MOTHRA_SECRET:?MOTHRA_SECRET must be set}📝 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.
| MOTHRA_SECRET: ${MOTHRA_SECRET} | |
| MOTHRA_SECRET: ${MOTHRA_SECRET:?MOTHRA_SECRET must be set} |
🧰 Tools
🪛 Checkov (3.3.8)
[medium] 44-45: Basic Auth Credentials
(CKV_SECRET_4)
📍 Affects 1 file
docker-compose.yml#L45-L45(this comment)docker-compose.yml#L68-L68
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker-compose.yml` at line 45, Require a non-empty MOTHRA_SECRET in both
docker-compose.yml backend line 45 and worker line 68 by using Compose’s
mandatory, non-empty variable syntax; apply the same guard at both environment
entries so startup fails when the host variable is missing or empty.
| name: mothra-config | ||
| namespace: mothra | ||
| data: | ||
| CELERY_BROKER_URL: "redis://redis:6379/0" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## files\n'
git ls-files k8s
printf '\n## relevant search\n'
rg -n --hidden --no-ignore-vcs -S 'CELERY_BROKER_URL|redis|NetworkPolicy|mothra-secrets|password|requirepass|acl|broker' k8s || true
printf '\n## outline configmap\n'
wc -l k8s/configmap.yaml 2>/dev/null || true
cat -n k8s/configmap.yaml
printf '\n## outline redis\n'
wc -l k8s/redis.yaml 2>/dev/null || true
cat -n k8s/redis.yaml
printf '\n## find networkpolicy manifests\n'
fd -a -t f 'networkpolicy|network-policy|policy' k8s . || trueRepository: DDMAL/mothra
Length of output: 4302
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## backend.yaml\n'
wc -l k8s/backend.yaml 2>/dev/null || true
cat -n k8s/backend.yaml
printf '\n## worker.yaml\n'
wc -l k8s/worker.yaml 2>/dev/null || true
cat -n k8s/worker.yaml
printf '\n## secret.yaml.example\n'
wc -l k8s/secret.yaml.example 2>/dev/null || true
cat -n k8s/secret.yaml.example
printf '\n## README auth mentions\n'
rg -n -S 'redis|broker|MOTHRA_SECRET|DATABASE_URL|secret|password|ACL|requirepass|NetworkPolicy' k8s/README.md k8s/*.yaml || trueRepository: DDMAL/mothra
Length of output: 7230
Lock down the Redis broker
redis://redis:6379/0 is unauthenticated, and there’s no NetworkPolicy, so any Pod in the cluster can connect, publish tasks, or drain queues. Move the broker URL into mothra-secrets, enable Redis password/ACL, and restrict access to backend/worker only.
k8s/configmap.yaml: remove the broker URL from the ConfigMap.k8s/redis.yaml: require Redis auth and add a NetworkPolicy.
📍 Affects 2 files
k8s/configmap.yaml#L9-L9(this comment)k8s/redis.yaml#L19-L40
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@k8s/configmap.yaml` at line 9, Remove CELERY_BROKER_URL from
k8s/configmap.yaml and provide it through the mothra-secrets configuration
consumed by backend and worker components. In k8s/redis.yaml, require Redis
password or ACL authentication and add a NetworkPolicy allowing ingress only
from backend and worker Pods; apply the requested changes at k8s/configmap.yaml
lines 9-9 and k8s/redis.yaml lines 19-40.
| - **Images:** `ghcr.io/ddmal/mothra-{backend,ic,text-service}`, CPU-only, pulled with | ||
| the `ghcr-pull-secret` imagePullSecret. `worker` reuses the `mothra-backend` image. | ||
|
|
||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Declare fence languages.
These four fences trigger the configured MD040 warnings. Use text for the directory tree and bash for commands.
Proposed fix
-```
+```text
k8s/
...
-```
+```bash
cp k8s/secret.yaml.example k8s/secret.yaml
...
-```
+```bash
kubectl apply -f k8s/secret.yaml -f k8s/configmap.yaml
...
-```
+```bash
kubectl -n mothra get pods,pvc,svc,ingressAlso applies to: 33-33, 48-48, 57-57
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 16-16: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@k8s/README.md` at line 16, Declare languages for all four fenced blocks in
the README: use text for the directory tree and bash for each command block,
including the additional fences at the referenced locations, while preserving
their contents.
Source: Linters/SAST tools
| spec: | ||
| containers: | ||
| - name: redis | ||
| image: redis:7-alpine | ||
| ports: | ||
| - name: redis | ||
| containerPort: 6379 | ||
| readinessProbe: | ||
| exec: | ||
| command: ["redis-cli", "ping"] | ||
| initialDelaySeconds: 5 | ||
| periodSeconds: 10 | ||
| livenessProbe: | ||
| tcpSocket: | ||
| port: 6379 | ||
| initialDelaySeconds: 15 | ||
| periodSeconds: 15 | ||
| resources: | ||
| requests: | ||
| cpu: 50m | ||
| memory: 64Mi | ||
| limits: | ||
| cpu: 250m | ||
| memory: 256Mi |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
for f in k8s/redis.yaml k8s/backend.yaml k8s/worker.yaml k8s/ic.yaml k8s/text-service.yaml; do
echo "===== $f ====="
wc -l "$f"
sed -n '1,220p' "$f" | cat -n
echo
doneRepository: DDMAL/mothra
Length of output: 9452
Harden these workloads’ pod/container security contexts.
All five manifests still rely on default privileges. Add runAsNonRoot, allowPrivilegeEscalation: false, dropped capabilities, and seccompProfile: RuntimeDefault; use readOnlyRootFilesystem: true where the container can support it.
k8s/redis.yaml: keep/datawritable with an explicit volume.k8s/backend.yamlandk8s/worker.yaml: ensure the shared model-storage volume is writable by the selected UID.k8s/ic.yamlandk8s/text-service.yaml: apply the same hardened context.
🧰 Tools
🪛 Trivy (0.72.0)
[error] 19-40: Root file system is not read-only
Container 'redis' of Deployment 'redis' should set 'securityContext.readOnlyRootFilesystem' to true
Rule: KSV-0014
(IaC/Kubernetes)
[error] 19-40: Default security context configured
container redis in mothra namespace is using the default security context
Rule: KSV-0118
(IaC/Kubernetes)
[error] 17-40: Default security context configured
deployment redis in mothra namespace is using the default security context, which allows root privileges
Rule: KSV-0118
(IaC/Kubernetes)
📍 Affects 5 files
k8s/redis.yaml#L17-L40(this comment)k8s/backend.yaml#L17-L56k8s/worker.yaml#L17-L60k8s/ic.yaml#L17-L48k8s/text-service.yaml#L17-L45
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@k8s/redis.yaml` around lines 17 - 40, Harden the pod/container security
contexts in k8s/redis.yaml (17-40), k8s/backend.yaml (17-56), k8s/worker.yaml
(17-60), k8s/ic.yaml (17-48), and k8s/text-service.yaml (17-45) by enforcing
non-root execution, disabling privilege escalation, dropping capabilities, and
using seccomp RuntimeDefault. Enable read-only root filesystems where supported;
give Redis an explicit writable /data volume, and ensure the shared
model-storage volumes in backend and worker are writable by their selected UIDs.
Source: Linters/SAST tools
1 participant
Summary by CodeRabbit
New Features
Documentation
Chores