Skip to content

fix bug#67

Merged
mashta-lilia merged 2 commits into
mainfrom
infra/resolveproblems
May 18, 2026
Merged

fix bug#67
mashta-lilia merged 2 commits into
mainfrom
infra/resolveproblems

Conversation

@h00yaday
Copy link
Copy Markdown
Collaborator

@h00yaday h00yaday commented May 11, 2026

Summary by CodeRabbit

  • Security

    • Enabled HTTP/2 and added stricter HTTPS security headers for stronger protection.
  • Chores

    • CI now uses Python 3.12.
    • Services run as non‑root users and deployments use atomic env updates.
    • Improved startup reliability with service health checks, deployment health verification, and automated certificate renewal.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 11, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 344f8767-be68-455b-9c1e-51e3e9972808

📥 Commits

Reviewing files that changed from the base of the PR and between 68a46db and 9b52c06.

📒 Files selected for processing (3)
  • .github/workflows/deploy.yml
  • backend/Dockerfile
  • docker-compose.yml
🚧 Files skipped from review as they are similar to previous changes (3)
  • backend/Dockerfile
  • docker-compose.yml
  • .github/workflows/deploy.yml

📝 Walkthrough

Walkthrough

Infrastructure configuration is modernized for production readiness: consolidated deploy to GHCR with VPS atomic env handling and health verification, Docker Compose healthchecks and conditional starts, backend/ml_core run as non-root users, Nginx enables HTTP/2 and adds security headers, and CI Python runtime bumped to 3.12.

Changes

Production Infrastructure Modernization

Layer / File(s) Summary
Deployment Automation & VPS Integration
.github/workflows/deploy.yml
Consolidated deploy job: builds/pushes backend, frontend, ml_core images to GHCR (SHA + latest), writes .env.tmp with chmod 600 then mv to .env, updates repo via git fetch --all + git reset --hard origin/main, runs docker compose pull, performs Alembic migrations, starts services with docker compose up -d, waits, and verifies /api/health printing backend logs on failure.
Service Health & Orchestration
docker-compose.yml
Adds healthchecks for backend_api, redis, minio; nginx and ml_worker now depend on service health/start states; certbot runs a renewal loop; minio-init waits for minio:healthy and removes anonymous public ACL steps.
Container Security: Non-Root Users
backend/Dockerfile, ml_core/Dockerfile
Backend and ML Core images create an app user/group, chown /app, and run as USER app. Backend CMD changed to production uvicorn with --workers 4, --proxy-headers, and --forwarded-allow-ips "*" (no --reload).
Network Security: HTTP/2 & Headers
infra/nginx/default.conf
Enables http2 on the TLS listener and adds security headers (HSTS, X-Content-Type-Options, X-Frame-Options, Referrer-Policy) with always.
CI Configuration
.github/workflows/ci.yml
Python setup in backend-checks updated from 3.11 to 3.12.

Sequence Diagram(s)

sequenceDiagram
  participant GitHub
  participant GHCR
  participant VPS
  participant Compose
  participant BackendAPI

  GitHub->>GHCR: build & push images
  GitHub->>VPS: run SSH deploy script
  VPS->>Compose: docker compose pull & up
  Compose->>BackendAPI: run migrations / start
  VPS->>BackendAPI: curl /api/health
  BackendAPI-->>VPS: 200 OK / non-200
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 I nibble lines and spin the flow,

Env swapped safe where green lights glow,
Health checks hum as workers rise,
No root to tread where code now flies,
HTTP/2 sings — deploys go!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The PR title 'fix bug' is too vague and generic. It does not convey any meaningful information about what was actually changed or which bug was fixed. Replace the title with a specific description of the changes, such as 'Upgrade Python to 3.12, harden container security, and improve deployment pipeline' or similar.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch infra/resolveproblems

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Collaborator Author

@h00yaday h00yaday left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вроде все работает

Reviewed in CodeRabbit Review Stack

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🤖 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/deploy.yml:
- Around line 30-38: The workflow currently only builds and pushes the backend
image, so add equivalent build and push steps for the frontend and ml_core
images using the same tagging scheme (use env.IMAGE_PREFIX with :${{ github.sha
}} and :latest) so the remote docker compose pull && up -d will fetch the
updated images; specifically duplicate the docker build and docker push commands
for ./frontend and ./ml_core (same tags as backend) so IMAGE_PREFIX, frontend,
ml_core and the :${{ github.sha }}/:latest tags are used consistently.
- Around line 49-57: The .env.tmp keys written by the workflow don’t match what
docker-compose expects; update the echo lines so the environment variable names
match Compose (e.g., write POSTGRES_USER and POSTGRES_PASSWORD instead of
PROD_DB_USER/PROD_DB_PASSWORD, and add POSTGRES_DB using the appropriate secret
like secrets.PROD_DB_NAME), and either remove or map unused keys (DATABASE_URL,
PROD_REDIS_URL) to the environment names docker-compose actually reads;
specifically change the echo lines that write PROD_DB_USER/PROD_DB_PASSWORD to
echo "POSTGRES_USER=${{ secrets.PROD_DB_USER }}" and echo "POSTGRES_PASSWORD=${{
secrets.PROD_DB_PASSWORD }}", add echo "POSTGRES_DB=${{ secrets.PROD_DB_NAME }}"
(or the correct DB-name secret), and adjust or drop DATABASE_URL and
PROD_REDIS_URL so names align with docker-compose.yml.
- Around line 21-24: Update the GitHub Actions step references to supported,
pinned versions: replace actions/checkout@v3 with actions/checkout@v6,
docker/login-action@v2 with docker/login-action@v4, and change
appleboy/ssh-action@master to a specific stable tag (e.g.,
appleboy/ssh-action@v1 or appleboy/ssh-action@v1.2.5) so the workflow uses
supported runtimes and avoids mutable branch references; locate the three uses
of actions by their identifiers (actions/checkout, docker/login-action,
appleboy/ssh-action) and update the version tags accordingly in the deploy
workflow.

In `@backend/Dockerfile`:
- Line 28: The CMD invoking uvicorn currently uses --proxy-headers but omits
--forwarded-allow-ips, so Uvicorn will ignore forwarded headers from Docker
Compose networking; update the uvicorn startup command (the CMD array that
starts uvicorn for main:app) to include --forwarded-allow-ips "*" (or the
specific Docker network CIDR) alongside --proxy-headers so X-Forwarded-Proto and
X-Forwarded-For are respected.

In `@docker-compose.yml`:
- Around line 79-84: The healthcheck for the backend_api service uses curl which
isn't installed in the backend image; change the healthcheck test to use the
Python binary already in the image (instead of "curl") to perform an HTTP GET
against /api/health and exit non-zero on failure so Docker reports healthy
correctly; update the healthcheck block (the healthcheck: test: entry for
backend_api) to run a python one-liner that opens a connection to
localhost:8000, requests /api/health, checks for a 200 response, and exits with
code 0 on success or non-zero on any error or non-200 status.
- Around line 193-198: The minio-init service never receives MINIO_ROOT_PASSWORD
so the entrypoint line running `mc alias set myminio ... admin
$${MINIO_ROOT_PASSWORD}` authenticates with an empty password; fix by adding an
environment mapping for MINIO_ROOT_PASSWORD to the minio-init service (e.g.
environment: - MINIO_ROOT_PASSWORD: ${MINIO_ROOT_PASSWORD}) so the container has
the value, and adjust the entrypoint to use the in-container shell variable
(single $) in the `mc alias set` call (refer to the minio-init service, its
entrypoint, the `mc alias set` invocation, and the MINIO_ROOT_PASSWORD
variable).

In `@frontend/Dockerfile`:
- Around line 21-25: The Dockerfile currently creates appgroup/appuser (RUN
addgroup -S appgroup && adduser -S appuser -G appgroup) then runs chown -R
appuser:appgroup /app and sets USER appuser, but /app does not exist and running
Nginx as a non-root user on port 80 will fail; fix by changing the chown target
to the actual asset path (chown -R appuser:appgroup /usr/share/nginx/html) or
remove the chown entirely, and then choose one of two approaches for Nginx:
either run as root by removing the USER appuser line so Nginx can bind to port
80, or reconfigure Nginx to listen on a >=1024 port and update EXPOSE
accordingly; apply these changes around the RUN chown and USER appuser
statements and any EXPOSE/NGINX config references.
🪄 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 Plus

Run ID: 7fd9d79e-ded2-4b7f-9270-f6cb1bd80bf2

📥 Commits

Reviewing files that changed from the base of the PR and between f624858 and c15368e.

📒 Files selected for processing (7)
  • .github/workflows/ci.yml
  • .github/workflows/deploy.yml
  • backend/Dockerfile
  • docker-compose.yml
  • frontend/Dockerfile
  • infra/nginx/default.conf
  • ml_core/Dockerfile

Comment thread .github/workflows/deploy.yml Outdated
Comment thread .github/workflows/deploy.yml
Comment thread .github/workflows/deploy.yml Outdated
Comment thread backend/Dockerfile Outdated
Comment thread docker-compose.yml
Comment thread docker-compose.yml Outdated
Comment thread frontend/Dockerfile Outdated
@h00yaday h00yaday force-pushed the infra/resolveproblems branch from 68a46db to 9b52c06 Compare May 12, 2026 09:08
@mashta-lilia mashta-lilia merged commit d17689f into main May 18, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants