-
Notifications
You must be signed in to change notification settings - Fork 198
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: support ssl on local with self signed url #2903
base: dev
Are you sure you want to change the base?
Conversation
|
WalkthroughThe pull request introduces a new Caddy server configuration for local development environments. It includes a Caddyfile defining multiple local domains (myapp.local, backoffice.local, workflow.local, kyb.local, and kyc.local) with reverse proxy settings that route traffic to specific internal services on different ports of the Docker host. Additionally, a Docker Compose configuration is added to set up the Caddy service, including port exposures, volume mounts, and network settings. Changes to the Vite configuration and cookie session middleware in the main application enhance CORS handling and cookie security. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Caddy
participant Service
Client->>Caddy: Request to myapp.local
Caddy->>Service: Reverse proxy to host.docker.internal:3000
Service-->>Caddy: Response
Caddy-->>Client: Forward response
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (4)
services/workflows-service/Caddyfile (2)
13-14
: Remove unnecessary empty lines.The
workflow.local
block contains extra empty lines that should be removed for consistency.
1-27
: Add health checks and timeouts to reverse proxy configurations.Consider adding health checks and timeouts to make the local development environment more resilient.
Example configuration:
myapp.local { reverse_proxy http://host.docker.internal:3000 { + health_uri /health + health_interval 30s + health_timeout 10s + health_status 200 + timeout 60s } tls internal }services/workflows-service/docker-compose.yml (2)
9-12
: Define volume permissions explicitly.The volume mounts should have explicit permissions defined for better security.
volumes: - - ./Caddyfile:/etc/caddy/Caddyfile:ro - - caddy_data:/data - - caddy_config:/config + - type: bind + source: ./Caddyfile + target: /etc/caddy/Caddyfile + read_only: true + - type: volume + source: caddy_data + target: /data + volume: + nocopy: true + - type: volume + source: caddy_config + target: /config + volume: + nocopy: true
86-86
: Add newline at end of file.Add a newline character at the end of the file to comply with POSIX standards.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 86-86: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
services/workflows-service/Caddyfile
(1 hunks)services/workflows-service/docker-compose.yml
(2 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
services/workflows-service/docker-compose.yml
[error] 86-86: no new line character at the end of file
(new-line-at-end-of-file)
myapp.local { | ||
|
||
reverse_proxy http://host.docker.internal:3000 | ||
tls internal | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add security headers and CORS configuration.
While the basic reverse proxy setup works, consider enhancing security by adding standard security headers and CORS configurations for local development. This will better mirror production security requirements.
Add these configurations to each domain block:
myapp.local {
reverse_proxy http://host.docker.internal:3000
tls internal
+ header {
+ # Security headers
+ Strict-Transport-Security "max-age=31536000; includeSubDomains"
+ X-Content-Type-Options "nosniff"
+ X-Frame-Options "DENY"
+ Referrer-Policy "strict-origin-when-cross-origin"
+ # CORS headers for local development
+ Access-Control-Allow-Origin "*"
+ Access-Control-Allow-Methods "GET, POST, PUT, DELETE, OPTIONS"
+ Access-Control-Allow-Headers "*"
+ }
}
Also applies to: 7-10, 12-17, 19-22, 24-27
@@ -1,5 +1,20 @@ | |||
version: '3' | |||
services: | |||
caddy: | |||
image: caddy:latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid using 'latest' tag for Caddy image.
Using the latest
tag can lead to unexpected behavior when the image is updated. Specify a fixed version for better reproducibility and stability.
- image: caddy:latest
+ image: caddy:2.7.6
📝 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.
image: caddy:latest | |
image: caddy:2.7.6 |
caddy: | ||
image: caddy:latest | ||
container_name: caddy_server | ||
ports: | ||
- "443:443" # Expose HTTPS | ||
- "80:80" # Expose HTTP (optional, redirects to HTTPS) | ||
volumes: | ||
- ./Caddyfile:/etc/caddy/Caddyfile:ro # Mount the Caddyfile | ||
- caddy_data:/data # Persist Caddy data | ||
- caddy_config:/config | ||
extra_hosts: | ||
- myapp.local:host-gateway | ||
- backoffice.local:host-gateway | ||
- host.docker.internal:host-gateway |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add restart policy and health check for Caddy service.
The Caddy service should be configured to restart automatically and include health checks for better reliability.
caddy:
image: caddy:latest
container_name: caddy_server
+ restart: unless-stopped
+ healthcheck:
+ test: ["CMD", "caddy", "version"]
+ interval: 30s
+ timeout: 10s
+ retries: 3
ports:
- "443:443"
- "80:80"
📝 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.
caddy: | |
image: caddy:latest | |
container_name: caddy_server | |
ports: | |
- "443:443" # Expose HTTPS | |
- "80:80" # Expose HTTP (optional, redirects to HTTPS) | |
volumes: | |
- ./Caddyfile:/etc/caddy/Caddyfile:ro # Mount the Caddyfile | |
- caddy_data:/data # Persist Caddy data | |
- caddy_config:/config | |
extra_hosts: | |
- myapp.local:host-gateway | |
- backoffice.local:host-gateway | |
- host.docker.internal:host-gateway | |
caddy: | |
image: caddy:latest | |
container_name: caddy_server | |
restart: unless-stopped | |
healthcheck: | |
test: ["CMD", "caddy", "version"] | |
interval: 30s | |
timeout: 10s | |
retries: 3 | |
ports: | |
- "443:443" # Expose HTTPS | |
- "80:80" # Expose HTTP (optional, redirects to HTTPS) | |
volumes: | |
- ./Caddyfile:/etc/caddy/Caddyfile:ro # Mount the Caddyfile | |
- caddy_data:/data # Persist Caddy data | |
- caddy_config:/config | |
extra_hosts: | |
- myapp.local:host-gateway | |
- backoffice.local:host-gateway | |
- host.docker.internal:host-gateway |
networks: | ||
mynetwork: | ||
external: true | ||
name: host |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Network configuration needs to be removed
The mynetwork
configuration in services/workflows-service/docker-compose.yml
is unnecessary and potentially problematic:
- None of the services in the file explicitly use this network
- Other docker-compose files in the project don't define or use this network
- Using the
host
network name is not recommended as it could conflict with Docker's built-in host network
- networks:
- mynetwork:
- external: true
- name: host
🔗 Analysis chain
Clarify network configuration.
The network configuration uses an external network named 'host' which might be confusing. Consider renaming it to better reflect its purpose or add comments explaining the setup.
Let's check if this network is used by other services:
networks:
mynetwork:
external: true
- name: host
+ name: local-dev-network # More descriptive name
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for network configuration references
rg -l 'networks:.*host' --type yaml
Length of output: 37
Script:
#!/bin/bash
# Let's try a broader search for network configurations in docker-compose files
fd docker-compose.yml --exec rg -l "networks:"
# Also search for any references to 'mynetwork'
rg -l "mynetwork"
# And search for any network configurations with 'host'
rg "network.*host" --type yaml
Length of output: 191
Script:
#!/bin/bash
# Let's examine the full docker-compose.yml to understand the network usage context
cat services/workflows-service/docker-compose.yml
# Also check if there are any other docker-compose files in the repository
fd docker-compose
Length of output: 2542
Script:
#!/bin/bash
# Let's check the other docker-compose files for network configurations
cat deploy/docker-compose-build-https.yml deploy/docker-compose-build.yml deploy/docker-compose-dev.yml services/workflows-service/docker-compose.db.yml
Length of output: 9550
🧰 Tools
🪛 yamllint (1.35.1)
[error] 86-86: no new line character at the end of file
(new-line-at-end-of-file)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/backoffice-v2/vite.config.ts (1)
15-15
: Enable or configure CORS based on environment as needed.
Currently,cors: true
enables CORS unconditionally. For security best practices, consider using environment-based conditions or a whitelist for allowed origins, especially for production deployments.services/workflows-service/src/main.ts (1)
101-103
: Simplify boolean assignment.
Instead of using a ternary operator to setsecure
, you can directly useenv.ENVIRONMENT_NAME !== 'local'
. This matches the static analysis recommendation and makes the code more concise.- secure: env.ENVIRONMENT_NAME !== 'local' ? true : false, + secure: env.ENVIRONMENT_NAME !== 'local',🧰 Tools
🪛 Biome (1.9.4)
[error] 102-102: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/backoffice-v2/vite.config.ts
(1 hunks)services/workflows-service/Caddyfile
(1 hunks)services/workflows-service/docker-compose.yml
(2 hunks)services/workflows-service/src/main.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- services/workflows-service/Caddyfile
🧰 Additional context used
🪛 Biome (1.9.4)
services/workflows-service/src/main.ts
[error] 102-102: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
🔇 Additional comments (2)
services/workflows-service/docker-compose.yml (2)
4-4
: Avoid using the 'latest' tag for the Caddy image.
Using caddy:latest
can cause unexpected behavior when a new image version is released. Pin to a specific version for better reproducibility.
84-86
: Reevaluate external network usage.
As previously noted, using an external network named host
can be confusing and may conflict with Docker’s built-in host
network. Consider removing or renaming this external network to avoid issues.
…-test-ssl-locally
…-test-ssl-locally
Summary by CodeRabbit
New Features
Chores