Skip to content
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

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions services/workflows-service/Caddyfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
myapp.local {

reverse_proxy http://host.docker.internal:3000
tls internal
}
Comment on lines +1 to +5
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

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


backoffice.local {
reverse_proxy http://host.docker.internal:5137
tls internal
}

workflow.local {


reverse_proxy http://host.docker.internal:5200
tls internal
}

kyb.local {
reverse_proxy http://host.docker.internal:5201
tls internal
}

kyc.local {
reverse_proxy http://host.docker.internal:5202
tls internal
}
21 changes: 21 additions & 0 deletions services/workflows-service/docker-compose.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,20 @@
version: '3'
services:
caddy:
image: caddy:latest
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

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.

Suggested change
image: caddy:latest
image: caddy:2.7.6

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

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.

Suggested change
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


server:
build:
context: .
Expand Down Expand Up @@ -63,3 +78,9 @@ services:
retries: 10
volumes:
postgres15: ~
caddy_config:
caddy_data:
networks:
mynetwork:
external: true
name: host
Copy link
Contributor

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)

Loading