Skip to content

Conversation

@waseemnaseeven
Copy link
Collaborator

@waseemnaseeven waseemnaseeven commented Oct 17, 2025

Summary by CodeRabbit

  • New Features

    • Added Keycloak authentication system with OAuth2 proxy integration for secure access.
    • Integrated Traefik reverse proxy with TLS/HTTPS support.
    • Expanded multi-service deployment architecture with frontend, backend, and authentication services.
  • Chores

    • Updated deployment and infrastructure configuration.
    • Added certificate generation and environment setup automation.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 17, 2025

Walkthrough

Adds infrastructure configuration for a multi-service Snak stack with Keycloak authentication, Traefik reverse proxy, and containerized components. Introduces a Makefile for orchestration, expanded docker-compose configuration with new services, Keycloak realm template, certificate generation script, and Traefik routing rules. Updates .gitignore to exclude configuration and data artifacts.

Changes

Cohort / File(s) Change Summary
Build & Orchestration
Makefile, .gitignore
New Makefile with targets for certificate generation, Keycloak realm configuration, Docker Compose orchestration, and cleanup. Updated .gitignore with patterns for Keycloak, PostgreSQL, Redis, Docker, and certificate files.
Docker Compose & Services
docker-compose.yml, packages/server/Dockerfile
Expanded docker-compose with new snaknet bridge network and volumes (redis_data, kc_postgres_data). Added services: traefik, kc_postgres, keycloak, oauth2-proxy, snak-frontend, snak-bff. Renamed server to snak-api. Updated postgres, redis, and snak-api services to use new network. Removed comment line from server Dockerfile.
Keycloak & Authentication
keycloak/realm-template.json, keycloak/traefik/certs/certs.sh, keycloak/traefik/dynamic.yml
Added Keycloak realm template with roles, clients (including snak-api), identity providers (Google, GitHub), authentication flows, and protocol mappers. Added certificate generation script for self-signed TLS (mysnakagent.com, auth.mysnakagent.com). Added Traefik dynamic configuration with HTTP routers for keycloak-public, oauth2, backend, bff, and frontend with TLS support and oauth2-proxy middleware.

Sequence Diagram(s)

sequenceDiagram
    participant Client as User Browser
    participant Traefik
    participant OAuth2 as OAuth2-Proxy
    participant Keycloak
    participant Backend as snak-api

    Client->>Traefik: Request protected resource
    Traefik->>OAuth2: Forward for auth check
    alt User authenticated
        OAuth2->>Traefik: ✓ Auth headers
        Traefik->>Backend: Route request
        Backend-->>Client: Response
    else User not authenticated
        OAuth2->>Keycloak: Redirect to login
        Keycloak-->>Client: Login form
        Client->>Keycloak: Credentials
        Keycloak-->>Client: Session + redirect
        Client->>Traefik: Retry with session
        Traefik->>OAuth2: Verify session
        OAuth2->>Traefik: ✓ Auth headers
        Traefik->>Backend: Route request
        Backend-->>Client: Response
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • docker-compose.yml: Substantial structural changes with multiple new services, network configuration, volume management, and service dependencies require careful validation of networking setup and service orchestration logic.
  • keycloak/realm-template.json: Large, complex JSON configuration with multiple nested structures (roles, clients, flows, mappings) that needs verification for correctness and security posture.
  • keycloak/traefik/dynamic.yml: Traefik routing rules, middleware chains, and service endpoints require careful review of path-based routing logic and OAuth2-proxy integration.
  • Makefile: Automation logic with environment variable substitution and host file modifications should be verified for correctness and portability.
  • keycloak/traefik/certs/certs.sh: Certificate generation parameters (validity period, key size, SANs) should be validated for production readiness.

Possibly related PRs

  • Feat/dockercompose #168: Modifies Docker Compose setup and server Dockerfile, with direct overlap in container orchestration and build configuration.

Suggested reviewers

  • antiyro

Poem

🐰 Hopping through containers with a floppy ear bounce,
Keycloak gates guarded, OAuth2 routes pounce,
Traefik tunnels forward, certificates in place,
Docker networks dancing—authentication's the ace!
🔐✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check ❓ Inconclusive The PR title "fix/keycloak_v2" is related to the changeset as it references Keycloak, which is a central component of the changes. However, the title is vague and lacks specificity about the nature and scope of the changes. The "fix/" prefix suggests bug fixes, but the changeset primarily consists of new infrastructure additions including Docker Compose orchestration, Traefik reverse proxy configuration, Keycloak realm templates, certificate generation, and multiple new service integrations. The term "keycloak_v2" is ambiguous—it doesn't clarify whether this is upgrading Keycloak, setting up a new integration, or fixing an existing implementation. A teammate scanning the commit history would not clearly understand that this is a comprehensive infrastructure setup rather than a targeted bug fix. Consider revising the PR title to be more descriptive and specific about the primary intent of the changes. A clearer title might be something like "Add Keycloak authentication infrastructure with Docker Compose and Traefik" or "Implement Keycloak integration with multi-service orchestration" to help teammates quickly understand that this changeset introduces complete authentication and infrastructure setup rather than fixing a specific issue.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/keycloak_v2

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.

❤️ Share

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

@waseemnaseeven waseemnaseeven changed the title fixing authentication + proxies with api, bff and frontend fix/keycloak_v2 Oct 17, 2025
@waseemnaseeven waseemnaseeven marked this pull request as ready for review October 22, 2025 16:43
Copy link
Contributor

@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: 5

🧹 Nitpick comments (7)
keycloak/realm-template.json (3)

1647-1655: Consider parameterizing the GitHub OAuth client ID.

The GitHub client ID Ov23livmjnssUmdh43bS is hardcoded. While client IDs are public values, parameterizing them (similar to the client secret) would improve flexibility for different environments and make the template more reusable.

Apply this pattern:

-        "clientId": "Ov23livmjnssUmdh43bS",
+        "clientId": "${GITHUB_CLIENT_ID}",

1671-1680: Consider parameterizing the Google OAuth client ID.

The Google client ID is hardcoded. For consistency with secrets handling and to support multiple environments, consider using a placeholder like ${GOOGLE_CLIENT_ID}.


1607-1620: Consider parameterizing SMTP email addresses.

The email address [email protected] is hardcoded in both the from and user fields. If this template is intended for use across different environments or organizations, these values should be parameterized.

keycloak/dummy-frontend/Dockerfile (1)

1-6: Consider adding a non-root user.

The container runs as root, which is not recommended even for test services. Adding a non-root user improves security posture.

Apply this diff:

 FROM node:20-alpine
 WORKDIR /app
 COPY index.html /app/index.html
 COPY server.js /app/server.js
+RUN addgroup -S appgroup && adduser -S appuser -G appgroup && \
+    chown -R appuser:appgroup /app
+USER appuser
 EXPOSE 8080
 CMD ["node", "server.js"]
keycloak/traefik/dynamic.yml (1)

34-38: Remove or clarify the outdated comment.

The comment states "change to snak-frontend for real app" but the service is already configured as snak-frontend. This comment appears outdated and could cause confusion.

Either remove the comment or clarify if there's a dummy-frontend/snak-frontend switching mechanism:

     frontend:
       rule: "Host(`mysnakagent.com`)"
       entryPoints: [websecure]
-      service: snak-frontend # change to snak-frontend for real app
+      service: snak-frontend
       tls: {}
Makefile (1)

22-26: Add validation for ENV_FILE and required variables.

The realms target assumes the ENV_FILE exists and contains all required variables. Missing variables will result in empty placeholders in the generated realm.json, which could cause runtime failures.

Add validation:

 realms: 
+	@test -f $(ENV_FILE) || (echo "❌ $(ENV_FILE) not found" && exit 1)
 	@printf "%b\n" "$(_STARLIGHT_BLUE)→ Generating realm.json from template$(_NO)"
 	@export $$(grep -v '^#' $(ENV_FILE) | xargs) && \
 	envsubst < $(REALM_TEMPLATE) > $(REALM_OUTPUT)
+	@grep '\$$' $(REALM_OUTPUT) && echo "⚠ Warning: Unresolved variables found in $(REALM_OUTPUT)" || true
 	@printf "%b\n" "$(_GREEN)✔ Realm file generated: $(REALM_OUTPUT)$(_NO)"
docker-compose.yml (1)

23-23: Consider reverting the healthcheck to include the database parameter for consistency and stricter validation.

The healthcheck change from pg_isready -U $${POSTGRES_USER} -d $${POSTGRES_DB} to pg_isready -U $${POSTGRES_USER} reduces validation strictness. The current check only verifies PostgreSQL is accepting connections, not that the specific database is accessible. While snak-api depends on this healthcheck with condition: service_healthy, it could theoretically start before the database is fully initialized.

The .env.example defines POSTGRES_DB=postgres (the default PostgreSQL system database), so reverting to include the -d parameter is both safe and recommended:

    healthcheck:
-     test: ['CMD-SHELL', 'pg_isready -U $${POSTGRES_USER}']
+     test: ['CMD-SHELL', 'pg_isready -U $${POSTGRES_USER} -d $${POSTGRES_DB}']

Note: The kc_postgres service in the same file already uses this stricter form, so aligning postgres for consistency is advisable.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 14d1ac7 and 5385e30.

📒 Files selected for processing (10)
  • .gitignore (1 hunks)
  • Makefile (1 hunks)
  • docker-compose.yml (4 hunks)
  • keycloak/dummy-frontend/Dockerfile (1 hunks)
  • keycloak/dummy-frontend/index.html (1 hunks)
  • keycloak/dummy-frontend/server.js (1 hunks)
  • keycloak/realm-template.json (1 hunks)
  • keycloak/traefik/certs/certs.sh (1 hunks)
  • keycloak/traefik/dynamic.yml (1 hunks)
  • packages/server/Dockerfile (0 hunks)
💤 Files with no reviewable changes (1)
  • packages/server/Dockerfile
🧰 Additional context used
🪛 checkmake (0.2.2)
Makefile

[warning] 7-7: Target body for "certs" exceeds allowed length of 5 (8).

(maxbodylength)


[warning] 33-33: Target body for "clean" exceeds allowed length of 5 (8).

(maxbodylength)


[warning] 43-43: Missing required phony target "test"

(minphony)

🪛 Shellcheck (0.11.0)
keycloak/traefik/certs/certs.sh

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)

🔇 Additional comments (6)
keycloak/dummy-frontend/server.js (1)

1-21: LGTM - Clean implementation.

The server implementation is straightforward and appropriate for a dummy frontend. Error handling is present, and the use of Node.js built-in modules keeps dependencies minimal.

Makefile (1)

7-15: Document the /etc/hosts modification requirement.

The certs target modifies /etc/hosts with sudo privileges. This is a significant system change that should be documented in a README or printed as a warning before execution.

Consider adding a confirmation prompt or clear documentation:

 certs:
+	@printf "%b\n" "$(_YELLOW)⚠ This target will modify /etc/hosts (requires sudo)$(_NO)"
 	@cd keycloak/traefik/certs && \
 	./certs.sh
 	@printf "%b\n" "→ Ensuring hosts entries"

Also, ensure the PR description or README documents this requirement for users running the setup for the first time.

docker-compose.yml (4)

102-117: Verify Keycloak realm import path and version compatibility.

The Keycloak service (v26.0.2) mounts a realm configuration at /opt/keycloak/data/import/realm.json. Verify that:

  1. The file ./keycloak/realm.json exists in the repository.
  2. The import path /opt/keycloak/data/import/realm.json is correct for Keycloak 26.0.2 (this path may vary between versions).
  3. The realm.json file contains valid Keycloak realm configuration with all necessary clients and settings for oauth2-proxy integration.

87-100: Verify kc_postgres environment configuration and initialization.

The Keycloak PostgreSQL service depends on environment variables from keycloak/.env.keycloak for the healthcheck (e.g., POSTGRES_USER, POSTGRES_DB). Ensure that:

  1. The file keycloak/.env.keycloak exists and defines all required variables.
  2. The PostgreSQL initialization completes before the service_healthy condition passes (verify that the retries and interval settings are appropriate).
  3. The database and user are created successfully before Keycloak attempts to connect.

1-8: Structural additions approved.

The volume and network additions (redis_data, kc_postgres_data, and snaknet bridge) are well-structured and support the new services. The explicit bridge network improves service isolation and communication.


70-85: The review comment is partially incorrect and misses the actual issue.

The dynamic.yml file is already complete and properly configured with routes, middlewares, and TLS certificate references for all services (keycloak, oauth2-proxy, snak-api, snak-bff, snak-frontend). The concern about incomplete dynamic routing is unfounded.

However, the review overlooks a critical issue: The certificate files (mysnakagent.crt and mysnakagent.key) do not actually exist in keycloak/traefik/certs/—only the generation script certs.sh is present. Traefik will fail to start if these certificates are missing.

Regarding the HTTP entry point: The HTTPS-only configuration appears intentional (a valid security practice). Adding HTTP for redirects is a design suggestion but not a configuration error, as not all deployments require HTTP.

Actions needed:

  • Run keycloak/traefik/certs/certs.sh to generate the required certificate files before starting the stack, OR document this setup step.
  • Clarify with the team whether HTTP-to-HTTPS redirects are needed; if yes, add HTTP entry point.

Likely an incorrect or invalid review comment.

Comment on lines +119 to +130
oauth2-proxy:
image: quay.io/oauth2-proxy/oauth2-proxy:v7.6.0
container_name: oauth2_proxy
restart: always
depends_on:
- keycloak
env_file:
- keycloak/.env.keycloak
networks: [snaknet]
ports:
- "4180:4180"
- "44180:44180"
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

oauth2-proxy lacks service_healthy dependency check on keycloak.

The oauth2-proxy service depends on keycloak but does not specify condition: service_healthy. This means oauth2-proxy may start before Keycloak is fully initialized, causing connection failures or initialization race conditions.

Apply this diff to add a health check condition:

  oauth2-proxy:
    image: quay.io/oauth2-proxy/oauth2-proxy:v7.6.0
    container_name: oauth2_proxy
    restart: always
    depends_on:
-     - keycloak
+     keycloak:
+       condition: service_healthy
    env_file:
      - keycloak/.env.keycloak

Note: Verify that keycloak service includes a healthcheck block to support this dependency condition. If not, add one.


🏁 Script executed:

#!/bin/bash
# Find docker-compose.yml and examine the keycloak and oauth2-proxy services
echo "=== Searching for docker-compose.yml ==="
fd -e yml -e yaml | grep -i docker-compose | head -5

echo -e "\n=== Full content of docker-compose.yml ==="
find . -name "docker-compose.yml" -o -name "docker-compose.yaml" | head -1 | xargs cat

Length of output: 3688


🏁 Script executed:

#!/bin/bash
# Alternative: search for keycloak service definition to check for healthcheck
echo "=== Searching for keycloak service definition and healthcheck ==="
grep -n -A 20 "keycloak:" docker-compose.yml | head -50

Length of output: 711


oauth2-proxy lacks service_healthy dependency check on keycloak, and keycloak is missing a healthcheck block.

The oauth2-proxy service depends on keycloak using a simple list format without a health condition. Additionally, the keycloak service lacks a healthcheck block, which is required to support condition: service_healthy. This causes oauth2-proxy to start before Keycloak is fully initialized, creating potential race conditions.

Apply this diff to add a healthcheck to keycloak and update oauth2-proxy's dependency:

  keycloak:
    image: quay.io/keycloak/keycloak:26.0.2
    container_name: keycloak
    command: ["start", "--import-realm"]
    restart: always
    depends_on:
      kc_postgres:
        condition: service_healthy
+   healthcheck:
+     test: ["CMD", "curl", "-f", "http://localhost:8080/health/ready"]
+     interval: 10s
+     timeout: 5s
+     retries: 5
    volumes:
      - ./keycloak/realm.json:/opt/keycloak/data/import/realm.json:ro
    env_file:
      - keycloak/.env.keycloak
    networks: [snaknet]
    ports: 
      - "8080:8080"
      - "9000:9000"

  oauth2-proxy:
    image: quay.io/oauth2-proxy/oauth2-proxy:v7.6.0
    container_name: oauth2_proxy
    restart: always
    depends_on:
-     - keycloak
+     keycloak:
+       condition: service_healthy
    env_file:
      - keycloak/.env.keycloak

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In docker-compose.yml around lines 119 to 130, add a proper healthcheck block to
the keycloak service and change oauth2-proxy's depends_on to wait for
service_healthy; specifically, add a healthcheck that polls Keycloak's HTTP
endpoint (e.g. a curl or wget command hitting the admin or realm endpoint) with
sensible interval, timeout and retries (for example interval 10s, timeout 5s,
retries 10), and update oauth2-proxy's depends_on entry to use the
service_healthy condition so oauth2-proxy will only start after Keycloak passes
its healthcheck.

Comment on lines +1 to +5
openssl req -x509 -nodes -newkey rsa:2048 -days 365 \
-keyout mysnakagent.key \
-out mysnakagent.crt \
-subj "/CN=mysnakagent.com" \
-addext "subjectAltName=DNS:mysnakagent.com,DNS:auth.mysnakagent.com,IP:127.0.0.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Add shebang and error handling.

The script is missing a shebang line, which can cause portability issues and unexpected behavior depending on how it's executed.

Apply this diff:

+#!/bin/bash
+set -euo pipefail
+
 openssl req -x509 -nodes -newkey rsa:2048 -days 365 \
   -keyout mysnakagent.key \
   -out mysnakagent.crt \
   -subj "/CN=mysnakagent.com" \
   -addext "subjectAltName=DNS:mysnakagent.com,DNS:auth.mysnakagent.com,IP:127.0.0.1"
+
+echo "✅ Certificates generated successfully"

The set -euo pipefail ensures the script exits on errors, undefined variables, or pipe failures.

📝 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.

Suggested change
openssl req -x509 -nodes -newkey rsa:2048 -days 365 \
-keyout mysnakagent.key \
-out mysnakagent.crt \
-subj "/CN=mysnakagent.com" \
-addext "subjectAltName=DNS:mysnakagent.com,DNS:auth.mysnakagent.com,IP:127.0.0.1"
#!/bin/bash
set -euo pipefail
openssl req -x509 -nodes -newkey rsa:2048 -days 365 \
-keyout mysnakagent.key \
-out mysnakagent.crt \
-subj "/CN=mysnakagent.com" \
-addext "subjectAltName=DNS:mysnakagent.com,DNS:auth.mysnakagent.com,IP:127.0.0.1"
echo "✅ Certificates generated successfully"
🧰 Tools
🪛 Shellcheck (0.11.0)

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)

🤖 Prompt for AI Agents
In keycloak/traefik/certs/certs.sh around lines 1 to 5, the script lacks a
shebang and robust error handling; add a POSIX-compatible shebang (e.g.,
#!/usr/bin/env bash) as the first line and immediately after add set -euo
pipefail to make the script exit on errors, treat unset variables as errors, and
fail pipelines on the first failing command; ensure these lines are placed
before any commands so openssl runs under the intended shell with strict error
handling.

Comment on lines +33 to +41
clean:
@printf "%b\n" "$(_STARLIGHT_BLUE)→ Cleaning all (stacks + realm.json)$(_NO)"
@docker compose down -v
@docker volume prune -af
@docker system prune -af
rm -f $(REALM_OUTPUT)
rm -f keycloak/traefik/certs/mysnakagent.crt
rm -f keycloak/traefik/certs/mysnakagent.key
@printf "%b\n" "$(_GREEN)✔ Clean done$(_NO)"
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Warning: The clean target is very aggressive.

Line 37 uses docker system prune -af, which removes all unused Docker resources system-wide, not just those related to this project. This could unexpectedly remove containers, images, and volumes from other projects.

Consider making it less aggressive or adding a clear warning:

 clean:
 	@printf "%b\n" "$(_STARLIGHT_BLUE)→ Cleaning all (stacks + realm.json)$(_NO)"
+	@printf "%b\n" "$(_YELLOW)⚠ WARNING: This will prune ALL unused Docker resources system-wide!$(_NO)"
 	@docker compose down -v
-	@docker volume prune -af
-	@docker system prune -af
+	@docker volume prune -f --filter "label=com.docker.compose.project=snak"
+	@docker image prune -f --filter "label=com.docker.compose.project=snak"
 	rm -f $(REALM_OUTPUT)
 	rm -f keycloak/traefik/certs/mysnakagent.crt
 	rm -f keycloak/traefik/certs/mysnakagent.key

Or create a separate clean-all target for the aggressive cleanup and keep clean project-scoped.

🧰 Tools
🪛 checkmake (0.2.2)

[warning] 33-33: Target body for "clean" exceeds allowed length of 5 (8).

(maxbodylength)

🤖 Prompt for AI Agents
Makefile lines 33-41: the clean target calls `docker system prune -af`, which is
too aggressive and may remove other projects' resources; remove that line from
the clean target and either (A) add a new `clean-all` target that performs the
aggressive `docker system prune -af` (documented and optional), or (B) replace
it with project-scoped cleanup commands (keep `docker compose down -v` and
`docker volume prune -f` or use prune commands filtered by a project label or
specific volumes/images) or (C) prompt for confirmation before running the
global prune; implement one of these options and update the Makefile comments so
`clean` is safe for local project use.

@antiyro antiyro requested a review from 0xhijo October 24, 2025 12:23
Copy link
Contributor

@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: 1

♻️ Duplicate comments (1)
docker-compose.yml (1)

119-130: oauth2-proxy still lacks service_healthy dependency condition on keycloak, and keycloak has no healthcheck.

This is a duplicate of the previously flagged issue. The oauth2-proxy service depends on keycloak using a simple list format (line 124) without specifying condition: service_healthy. Additionally, the keycloak service (lines 102–117) still lacks a healthcheck block, which is required to support the health condition. This creates a race condition where oauth2-proxy may start before Keycloak is fully initialized.

Apply this diff to add a healthcheck to keycloak and update oauth2-proxy's dependency format:

  keycloak:
    image: quay.io/keycloak/keycloak:26.0.2
    container_name: keycloak
    command: ["start", "--import-realm"]
    restart: always
    depends_on:
      kc_postgres:
        condition: service_healthy
+   healthcheck:
+     test: ["CMD", "curl", "-f", "http://localhost:8080/health/ready"]
+     interval: 10s
+     timeout: 5s
+     retries: 5
    volumes:
      - ./keycloak/realm.json:/opt/keycloak/data/import/realm.json:ro
    env_file:
      - keycloak/.env.keycloak
    networks: [snaknet]
    ports: 
      - "8080:8080"
      - "9000:9000"

  oauth2-proxy:
    image: quay.io/oauth2-proxy/oauth2-proxy:v7.6.0
    container_name: oauth2_proxy
    restart: always
    depends_on:
-     - keycloak
+     keycloak:
+       condition: service_healthy
    env_file:
      - keycloak/.env.keycloak
    networks: [snaknet]
    ports: 
      - "4180:4180"
      - "44180:44180"
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5385e30 and 4b16480.

📒 Files selected for processing (2)
  • .gitignore (1 hunks)
  • docker-compose.yml (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • .gitignore
⏰ 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). (2)
  • GitHub Check: Build Check
  • GitHub Check: validate-build
🔇 Additional comments (2)
docker-compose.yml (2)

132-170: Resolved: Build context issue for snak-frontend and snak-bff.

The previous review flagged non-existent external build contexts (../snak-app/frontend, ../snak-app/backend). This PR now uses pre-built images from ghcr.io (ghcr.io/kasarlabs/snak-app/snak-frontend:latest and ghcr.io/kasarlabs/snak-app/snak-bff:latest) instead of local build contexts, eliminating the dependency on external directories. The commented-out old build configuration (lines 132–152) provides good documentation of the prior approach.


70-85: Original review comment is incorrect—all volume mount paths exist.

The verification confirms both required host paths are present in the repository:

  • ./keycloak/traefik/dynamic.yml exists (99 lines)
  • ./keycloak/traefik/certs/ directory exists with contents

The docker-compose configuration will not fail at runtime due to missing mounted paths. No action needed.

Likely an incorrect or invalid review comment.

Comment on lines +87 to +100
kc_postgres:
image: postgres:15
container_name: kc_postgres
restart: always
env_file:
- keycloak/.env.keycloak
healthcheck:
test: ["CMD-SHELL", "pg_isready -U $${POSTGRES_USER} -d $${POSTGRES_DB}"]
interval: 5s
timeout: 3s
retries: 30
volumes:
- kc_postgres_data:/var/lib/postgresql/data
networks: [snaknet]
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify required environment files exist before deployment.

Multiple services reference environment files that must be pre-configured: keycloak/.env.keycloak, .env.snak-frontend, and .env.snak-bff. If these files are missing, docker-compose will fail or services will start with incomplete configuration.

Also applies to: 102-117, 119-130, 154-170


🏁 Script executed:

#!/bin/bash
# Verify required environment files exist
echo "=== Checking environment files ==="

for env_file in "keycloak/.env.keycloak" ".env.snak-frontend" ".env.snak-bff"; do
  if [ -f "$env_file" ]; then
    echo "$env_file exists"
    echo "  Variables defined: $(grep -c '^[^#]' "$env_file" 2>/dev/null || echo 0)"
  else
    echo "$env_file NOT FOUND"
  fi
done

echo ""
echo "=== All .env* files in repository root ==="
ls -la .env* 2>/dev/null || echo "No .env files found in root"

echo ""
echo "=== All .env* files under keycloak/ ==="
find ./keycloak -name ".env*" -o -name "*.env" 2>/dev/null || echo "No .env files found under keycloak/"

Length of output: 273



---

🏁 Script executed:

```shell
#!/bin/bash
# Verify env_file references in docker-compose.yml
echo "=== Checking env_file references in docker-compose.yml ==="
grep -n "env_file:" docker-compose.yml || echo "No env_file entries found"

echo ""
echo "=== Content of .env.example ==="
head -30 .env.example

echo ""
echo "=== Looking for setup documentation ==="
for doc in "README.md" "CONTRIBUTING.md" "docs/setup.md" ".github/workflows"*; do
  if [ -f "$doc" ] || [ -d "$doc" ]; then
    echo "Found: $doc"
  fi
done

Length of output: 1404



---

🏁 Script executed:

```shell
#!/bin/bash
# Get exact env_file references at the specified line ranges
echo "=== Lines 87-100 (kc_postgres service) ==="
sed -n '87,100p' docker-compose.yml

echo ""
echo "=== Lines 102-117 (snak-frontend service) ==="
sed -n '102,117p' docker-compose.yml

echo ""
echo "=== Lines 119-130 (snak-bff service) ==="
sed -n '119,130p' docker-compose.yml

echo ""
echo "=== Lines 154-170 (services with env_file) ==="
sed -n '154,170p' docker-compose.yml

echo ""
echo "=== README.md setup/env configuration section ==="
grep -A 20 -i "environment\|env_file\|\.env\|setup" README.md | head -50

Length of output: 3593



---

🏁 Script executed:

```shell
#!/bin/bash
# Check for any example/template files for service configs
echo "=== Searching for example/template env files ==="
find . -type f \( -name "*.example" -o -name "*example*" -o -name "*template*" \) 2>/dev/null | grep -E "(env|keycloak|snak)" | head -20

echo ""
echo "=== Contents of keycloak/ directory ==="
ls -la keycloak/ 2>/dev/null || echo "keycloak/ directory check failed"

echo ""
echo "=== Check if .dockerignore or .gitignore ignores .env files ==="
echo "--- .gitignore relevant entries ---"
grep "\.env" .gitignore 2>/dev/null || echo "No .env entries in .gitignore"

Length of output: 543


Add setup documentation for service-specific environment files.

The required environment files (keycloak/.env.keycloak, .env.snak-frontend, .env.snak-bff) are intentionally git-ignored but missing. While .env.example covers root configuration, the README does not document how to create the service-specific env files, which will cause docker-compose to fail at deployment. Add setup instructions in README.md explaining which environment variables each service requires, or create example templates (e.g., keycloak/.env.keycloak.example, .env.snak-frontend.example, .env.snak-bff.example) that developers can copy and customize.

🤖 Prompt for AI Agents
In docker-compose.yml around lines 87 to 100 the compose references git-ignored
service-specific env files (keycloak/.env.keycloak, .env.snak-frontend,
.env.snak-bff) but the README lacks setup instructions and there are no example
templates for developers to copy; add documentation and/or example template
files: update README.md with a "Service-specific environment files" section
listing required env filenames and the required variables per service and their
purpose, and add example template files (keycloak/.env.keycloak.example,
.env.snak-frontend.example, .env.snak-bff.example) containing placeholder values
and comments so developers can copy to the real filenames and customize before
running docker-compose.

@antiyro antiyro merged commit df23036 into main Oct 30, 2025
4 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.

3 participants