chore: fix some small issue for deploy #161
Conversation
WalkthroughThis pull request makes several modifications across backend, Docker, and frontend components. In the backend, the authentication logic is updated so that email confirmation is conditionally enforced based on a new mail configuration flag, and new optional environment variables have been added for database configuration. The Vite configuration and package build scripts have been adjusted for improved flexibility. New Docker Compose and Traefik configurations have been introduced to enable a reverse proxy with TLS support. On the frontend, environment variables and URL construction have been enhanced to dynamically handle TLS settings. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AuthService
participant Database
User->>AuthService: Initiate login with credentials
alt Mail Enabled (isMailEnabled = true)
AuthService->>Database: Verify email confirmation
alt Email Confirmed
AuthService-->>User: Return successful login response
else Email Not Confirmed
AuthService-->>User: Return error for unconfirmed email
end
else Mail Disabled (isMailEnabled = false)
AuthService-->>User: Return successful login response (skip email check)
end
sequenceDiagram
participant Client
participant RunProjectFunction
participant Docker
Client->>RunProjectFunction: Request to run project container
alt TLS Enabled (TLS = true)
RunProjectFunction->>Docker: Execute run command with “websecure” and Traefik TLS labels
else TLS Disabled (TLS = false)
RunProjectFunction->>Docker: Execute run command with “web” entry point
end
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
frontend/src/app/api/runProject/route.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/frontend/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. frontend/src/utils/const.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/frontend/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. frontend/src/components/chat/code-engine/web-view.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/frontend/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team.
✨ Finishing Touches
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. 🪧 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 (
|
![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
🧹 Nitpick comments (10)
backend/template/react-ts/vite.config.ts (1)
33-33: Security setting change for development server accessibility.Adding
allowedHosts: trueallows the Vite development server to accept requests from any host. This is useful in containerized environments or when using reverse proxies, but has security implications.This change makes sense if you're deploying behind a reverse proxy or using Docker networking. However, be aware that this setting removes host header validation which could potentially expose your development server to broader access than intended. This change should only be used in controlled environments.
If you're working with Docker or Traefik as mentioned in the PR summary, this change is appropriate and likely necessary for proper functionality.
backend/src/auth/auth.service.ts (1)
47-49: Changed email confirmation default behavior.The default value for
MAIL_ENABLEDenvironment variable has been changed from 'true' to 'false', which means email functionality will be disabled by default unless explicitly enabled.This change shifts the default behavior to not require email verification. Make sure this aligns with your security requirements. For production environments, you might want to explicitly set
MAIL_ENABLED=trueif email verification is required for user accounts.Check if this change impacts any existing documentation or deployment scripts that might assume the previous default behavior.
backend/src/config/env.validation.ts (1)
4-23: Added optional database configuration environment variables.New optional environment variables for database configuration have been added, which provides more flexibility for configuring the database connection through environment variables.
These additions follow best practices for configuration management:
- They're properly typed and validated
- They're marked as optional, maintaining backward compatibility
- They allow for more flexible deployment options
This change enables better separation of configuration from code and supports different deployment environments more effectively. Make sure to update your documentation to reflect these new configuration options.
frontend/src/utils/const.ts (2)
6-7: Consider using stricter type checking for environment variables.The current implementation uses equality comparison (
==), which can lead to unexpected behavior with JavaScript type coercion. For environment variables, a more explicit approach would be better.- export const URL_PROTOCOL_PREFIX = - process.env.TLS == 'false' ? 'http' : 'https'; + export const URL_PROTOCOL_PREFIX = + process.env.TLS === 'false' ? 'http' : 'https';
9-12: Apply consistent type checking for environment variables.Similar to the previous constant, use strict equality (
===) for more predictable behavior when processing environment variables.- export const TLS = process.env.TLS == 'true'; + export const TLS = process.env.TLS === 'true';docker/docker-compose.pord.yml (1)
1-1: Fix typo in filename: "pord" should be "prod".The filename appears to have a typo - "pord" instead of "prod". Consider renaming the file to
docker-compose.prod.ymlfor clarity.frontend/src/app/api/runProject/route.ts (1)
233-233: Update log message to use dynamic protocol based on TLS setting.The log message uses hardcoded
http://protocol, which doesn't match the actual protocol when TLS is enabled.- `Container ${containerName} is now running at http://${domain}` + `Container ${containerName} is now running at ${TLS ? 'https' : 'http'}://${domain}`frontend/src/components/chat/code-engine/web-view.tsx (1)
56-58: Update console.log to use more descriptive logging.While the code is correct, the logging statement could be more informative, especially since we're now dynamically choosing between HTTP and HTTPS.
- const baseUrl = `${URL_PROTOCOL_PREFIX}://${domain}`; - console.log('baseUrl:', baseUrl); - setBaseUrl(baseUrl); + const baseUrl = `${URL_PROTOCOL_PREFIX}://${domain}`; + console.log(`Preview URL set to: ${baseUrl} (TLS: ${URL_PROTOCOL_PREFIX === 'https'})`); + setBaseUrl(baseUrl);docker/traefik-config/services.yml (2)
26-40: Consider a more robust service endpoint for the noop serviceThe
noopservice points tohttp://localhost:9000, which might not be accessible or might point to an unintended service. Since this is used for the redirect middleware, consider using a more explicit non-existent address or documenting what should be running at this address.noop: loadBalancer: servers: - - url: 'http://localhost:9000' + - url: 'http://127.0.0.1:9000' # Explicit loopback address
48-59: Consider more restrictive CORS configurationThe current CORS configuration allows a wide range of methods and basic headers. For production environments, it's generally recommended to restrict allowed origins and be more specific about which methods are actually needed by your API.
cors: headers: + accessControlAllowOriginList: + - "https://codefox.net" + - "http://localhost:3000" accessControlAllowMethods: - GET - POST - PUT - DELETE - - OPTIONS + # OPTIONS is automatically allowed when CORS is configured accessControlAllowHeaders: - Content-Type - Authorization + - Accept + accessControlMaxAge: 86400
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
backend/src/auth/auth.service.ts(2 hunks)backend/src/config/env.validation.ts(1 hunks)backend/template/react-ts/vite.config.ts(1 hunks)codefox-common/package.json(1 hunks)docker/docker-compose.pord.yml(1 hunks)docker/traefik-config/services.yml(1 hunks)docker/traefik-config/tls.yml(1 hunks)frontend/.env.example(1 hunks)frontend/src/app/api/runProject/route.ts(2 hunks)frontend/src/components/chat/code-engine/web-view.tsx(3 hunks)frontend/src/components/root/expand-card.tsx(3 hunks)frontend/src/utils/const.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- docker/traefik-config/tls.yml
- frontend/.env.example
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Install and Build Frontend
🔇 Additional comments (13)
codefox-common/package.json (1)
12-12: Improved build process with cleanup step.Adding
rimraf distbefore building ensures a clean build environment by removing any leftover files from previous builds, preventing potential conflicts.This is a good practice that helps prevent issues caused by stale build artifacts. The change helps ensure consistent builds across different environments.
backend/src/auth/auth.service.ts (1)
195-195: Made email confirmation conditional based on mail configuration.The email confirmation check is now only enforced when email functionality is enabled, allowing users to log in without confirming their email when the email system is disabled.
This is a logical change that aligns with the new default behavior. It provides more flexibility in development, testing, and production environments where email verification might not be needed or possible.
backend/src/config/env.validation.ts (1)
1-1: Added new import for database port validation.The
IsPortvalidator has been imported to ensure proper validation of the database port configuration.This is a good practice to ensure that the database port follows the expected format.
frontend/src/app/api/runProject/route.ts (1)
10-10: LGTM: Import of TLS constant is properly implemented.The TLS constant is correctly imported from the utils file.
frontend/src/components/chat/code-engine/web-view.tsx (4)
15-15: LGTM: Proper import of URL_PROTOCOL_PREFIX constant.The constant is correctly imported from the utils file.
45-45: LGTM: URL construction correctly uses dynamic protocol.The URL construction now correctly uses the dynamic protocol prefix instead of hardcoded 'http://'.
56-56: LGTM: Base URL construction properly uses dynamic protocol.The base URL construction correctly uses the dynamic protocol prefix.
70-70:❓ Verification inconclusive
Verify all URL constructions have been updated to use the dynamic protocol.
While some occurrences of URL construction have been updated to use
URL_PROTOCOL_PREFIX, a few places in the code still rely on thebaseUrlvariable. This is correct sincebaseUrlis already constructed with the proper protocol, but it's worth double-checking to ensure consistency.Also applies to: 83-83, 225-225
🏁 Script executed:
#!/bin/bash # Verify all URL constructions in the file use the proper protocol # This is to confirm that no hardcoded 'http://' remains in the code echo "Checking for hardcoded protocol references in web-view.tsx:" rg "http://" frontend/src/components/chat/code-engine/web-view.tsxLength of output: 195
Dynamic Protocol Usage Appears Consistent – Please Verify Constructor Details
- No hardcoded
"http://"references were found infrontend/src/components/chat/code-engine/web-view.tsx, including at lines 70, 83, and 225.- The use of
baseUrlin constructingfullUrlon line 70 is acceptable provided thatbaseUrlis correctly built using the dynamic protocol (i.e., viaURL_PROTOCOL_PREFIX).- Please ensure that the construction of
baseUrl—and its reuse at lines 83 and 225—remains consistent with the dynamic protocol update across the codebase.frontend/src/components/root/expand-card.tsx (3)
7-7: Improved URL protocol handlingGood addition of the dynamic URL protocol prefix import. This change supports the ability to handle both HTTP and HTTPS protocols based on environment configuration rather than hardcoding the protocol.
41-41: Dynamic protocol implementationThe change from hardcoded
http://to dynamic${URL_PROTOCOL_PREFIX}://allows for proper TLS support. This is a good improvement that enables the application to work correctly in both secure and non-secure environments.
124-124: Consistent dynamic protocol usageGood consistency in applying the protocol prefix change here as well. This ensures uniform behavior across all URL constructions in the component.
docker/traefik-config/services.yml (2)
1-25:❓ Verification inconclusive
Verify Docker host resolution in production
The Traefik configuration for routing looks good, but be aware that
host.docker.internal(used in the services section) is primarily supported in Docker Desktop for Mac and Windows. In Linux or production environments, this might not resolve correctly without additional configuration.
🏁 Script executed:
#!/bin/bash # Check if host.docker.internal is properly defined in the Docker environment echo "Checking if host.docker.internal is resolvable in the Docker environment..." # Look for any Docker Compose files that might contain extra_hosts configuration echo "Checking Docker Compose files for extra_hosts configuration:" fd -e yml -e yaml | xargs grep -l "extra_hosts" || echo "No extra_hosts configuration found in Docker Compose files" # Check if the Docker daemon.json has been configured with host.docker.internal echo "Checking if Docker daemon configuration includes host.docker.internal resolution:" if [ -f /etc/docker/daemon.json ]; then cat /etc/docker/daemon.json | grep -i "host.docker.internal" || echo "No host.docker.internal configuration found in daemon.json" else echo "Docker daemon.json not found" fiLength of output: 647
Action: Confirm Production Host Resolution via extra_hosts
The Traefik routing rules in
docker/traefik-config/services.ymllook correct. However, as a reminder,host.docker.internalis natively supported only in Docker Desktop on Mac and Windows. Our check indicates that your Docker Compose file (docker/docker-compose.pord.yml) includes anextra_hostsconfiguration—which should help in production—but no corresponding entry was found in/etc/docker/daemon.json. Please ensure that this extra configuration consistently enableshost.docker.internalresolution in your production (Linux) environment.
3-9:✅ Verification successful
Explicitly configure TLS certificates
The TLS configuration is using empty settings (
tls: {}), which will use default certificates. For production, you should explicitly configure certificates or use Let's Encrypt integration.
🏁 Script executed:
#!/bin/bash # Check for TLS configuration in Traefik echo "Checking for TLS configuration in Traefik..." # Look for tls.yml file or other TLS configuration echo "Looking for TLS configuration files:" fd -t f "tls" docker/traefik-config/ || echo "No specific TLS configuration file found" # Check for Let's Encrypt configuration echo "Checking for Let's Encrypt configuration:" fd -t f | xargs grep -l "acme" || echo "No Let's Encrypt configuration found" # Check for volume mounts for certificates echo "Checking for certificate volume mounts in Docker Compose files:" fd -e yml -e yaml | xargs grep -l "certificates" || echo "No certificate volume mounts found"Length of output: 677
I'll inspect the contents of the
docker/traefik-config/tls.ymlfile to see if it provides explicit TLS certificate configuration details. Let's run a script to display its contents.
🏁 Script executed:
#!/bin/bash # Display the TLS configuration file content cat docker/traefik-config/tls.ymlLength of output: 181
TLS Configuration Is Explicit and Valid
I verified that the explicit TLS certificate details are provided in the external file
docker/traefik-config/tls.yml:
- certFile:
/etc/letsencrypt/live/codefox.net/fullchain.pem- keyFile:
/etc/letsencrypt/live/codefox.net/privkey.pemAlthough the router configuration in
docker/traefik-config/services.ymlshowstls: {}, this shorthand is acceptable because it delegates the certificate configuration to the centralizedtls.yml. To improve clarity, consider adding a comment inservices.ymlnoting that TLS certificates are explicitly managed viatls.yml.
| /** | ||
| * @description: API URL | ||
| * @type {string} | ||
| * @example 'https://api.example.com' | ||
| */ | ||
| export const URL_PROTOCOL_PREFIX = | ||
| process.env.TLS == 'false' ? 'http' : 'https'; | ||
|
|
||
| /** | ||
| * Validate if the current environment is using TLS | ||
| */ | ||
| export const TLS = process.env.TLS == 'true'; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider adding a fallback for undefined environment variables.
The current implementation doesn't handle cases where the TLS environment variable is not defined. This could lead to unexpected behavior in different environments.
/**
* @description: API URL
* @type {string}
* @example 'https://api.example.com'
*/
export const URL_PROTOCOL_PREFIX =
- process.env.TLS == 'false' ? 'http' : 'https';
+ process.env.TLS === 'false' ? 'http' : 'https';
/**
* Validate if the current environment is using TLS
*/
-export const TLS = process.env.TLS == 'true';
+export const TLS = process.env.TLS === 'true';I'd also recommend ensuring these variables are properly set in your environments by verifying the configuration files:
🏁 Script executed:
#!/bin/bash
# Check if TLS is defined in environment configuration files
# Look for TLS in .env files
echo "Checking .env files for TLS configuration:"
fd -g ".env*" -x cat {} | grep -i "TLS"
# Look for TLS in next.config.js
echo "Checking next.config.js for TLS configuration:"
fd -g "next.config.js" -x cat {}Length of output: 482
Action Required: Implement Fallbacks for Undefined TLS and Update Environment File Search
Please make the following changes to ensure consistent behavior across environments:
-
Handle Undefined TLS:
Instead of directly comparingprocess.env.TLS, assign it to a variable with a default fallback. For example, if you expect a default of'false'(or adjust as needed), you might update the file as follows:/** * @description: API URL * @type {string} * @example 'https://api.example.com' */ -export const URL_PROTOCOL_PREFIX = - process.env.TLS == 'false' ? 'http' : 'https'; +const TLS_VALUE = typeof process.env.TLS !== 'undefined' ? process.env.TLS : 'false'; +export const URL_PROTOCOL_PREFIX = + TLS_VALUE === 'false' ? 'http' : 'https'; /** * Validate if the current environment is using TLS */ -export const TLS = process.env.TLS == 'true'; +export const TLS = TLS_VALUE === 'true'; -
Improve Environment Variable Verification:
The shell script output indicates that hidden files (like your.envfiles) are not being searched by default. When verifying configuration files, update the search command to include hidden files. For example:-fd -g ".env*" -x cat {} | grep -i "TLS" +fd --hidden -g ".env*" -x cat {} | grep -i "TLS"
-
Confirm Proper Configuration:
Ensure that your environment configuration files (andnext.config.jsif applicable) indeed set theTLSvariable as expected. This will help avoid unexpected behavior, especially in environments where the variable might be omitted.
Please address these points to improve robustness and consistency in TLS handling.
📝 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.
| /** | |
| * @description: API URL | |
| * @type {string} | |
| * @example 'https://api.example.com' | |
| */ | |
| export const URL_PROTOCOL_PREFIX = | |
| process.env.TLS == 'false' ? 'http' : 'https'; | |
| /** | |
| * Validate if the current environment is using TLS | |
| */ | |
| export const TLS = process.env.TLS == 'true'; | |
| /** | |
| * @description: API URL | |
| * @type {string} | |
| * @example 'https://api.example.com' | |
| */ | |
| const TLS_VALUE = typeof process.env.TLS !== 'undefined' ? process.env.TLS : 'false'; | |
| export const URL_PROTOCOL_PREFIX = | |
| TLS_VALUE === 'false' ? 'http' : 'https'; | |
| /** | |
| * Validate if the current environment is using TLS | |
| */ | |
| export const TLS = TLS_VALUE === 'true'; |
| - '443:443' | ||
| - '9001:8080' | ||
| volumes: | ||
| - /var/run/docker.sock:/var/run/docker.sock |
There was a problem hiding this comment.
Security concern: Docker socket is mounted as a volume.
Mounting the Docker socket (/var/run/docker.sock) gives Traefik full access to the Docker daemon, which is a significant security risk if Traefik is compromised.
Consider using a more secure approach:
- Use Docker socket proxy to limit access
- Apply proper permissions
- Or use alternative Docker provider methods
- - /var/run/docker.sock:/var/run/docker.sock
+ # Use a socket proxy with limited permissions instead
+ - socket-proxy:/var/run/docker.sockYou would then need to add a socket proxy service to your compose file.
📝 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.
| - /var/run/docker.sock:/var/run/docker.sock | |
| # Use a socket proxy with limited permissions instead | |
| - socket-proxy:/var/run/docker.sock |
| networks: | ||
| - traefik_network | ||
| extra_hosts: |
There was a problem hiding this comment.
Network name inconsistency with code reference.
The network defined here (traefik_network) doesn't match the network referenced in the Docker run command in route.ts (codefox_traefik_network).
Ensure network names are consistent across all files:
networks:
- traefik_network:
+ codefox_traefik_network:
driver: bridgeCommittable suggestion skipped: line range outside the PR's diff.
| reverse-proxy: | ||
| image: traefik:v3.3 | ||
| command: | ||
| - '--api.insecure=true' |
There was a problem hiding this comment.
Security concern: Traefik API is enabled in insecure mode.
Enabling Traefik's API in insecure mode (--api.insecure=true) in a production environment exposes the dashboard without authentication, which could be a security risk.
Consider either:
- Disabling the API in production
- - '--api.insecure=true'
+ # API disabled in production for security- Or securing it with authentication and HTTPS:
- - '--api.insecure=true'
+ - '--api.dashboard=true'
+ - '--api.insecure=false'
+ - '--entrypoints.traefik.address=:9001'
+ - '--entrypoints.traefik.http.tls=true'📝 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.
| - '--api.insecure=true' | |
| # API disabled in production for security |
| - '--api.insecure=true' | |
| - '--api.dashboard=true' | |
| - '--api.insecure=false' | |
| - '--entrypoints.traefik.address=:9001' | |
| - '--entrypoints.traefik.http.tls=true' |
| -l "traefik.http.routers.${subdomain}.entrypoints=websecure" \ | ||
| -l "traefik.http.routers.${subdomain}.tls=true" \ | ||
| -l "traefik.http.services.${subdomain}.loadbalancer.server.port=5173" \ | ||
| --network=codefox_traefik_network -p ${exposedPort}:5173 \ |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify network name consistency with docker-compose file.
The Docker run command references codefox_traefik_network, but the docker-compose.pord.yml file defines traefik_network. Ensure these network names are consistent.
🏁 Script executed:
#!/bin/bash
# Check for network name consistency across the codebase
echo "Network references in Docker files:"
fd -g "docker-compose*.yml" -x grep -E "network|traefik_network" {}
echo "\nNetwork references in code:"
rg -A 1 -B 1 "network=.*traefik" --glob "*.ts" --glob "*.tsx" --glob "*.js"Length of output: 1042
Align Docker network naming for consistency
The frontend/src/app/api/runProject/route.ts file uses --network=codefox_traefik_network at line 158, which doesn’t match the traefik_network defined in the docker-compose files. Please update the network name in the Docker run command (or adjust the docker-compose configuration) so that both references are consistent.
- Location:
frontend/src/app/api/runProject/route.ts(line 158) - Suggested update: Change
codefox_traefik_networktotraefik_network
| let runCommand; | ||
| if (TLS) { | ||
| runCommand = `docker run -d --name ${containerName} -l "traefik.enable=true" \ | ||
| -l "traefik.http.routers.${subdomain}.rule=Host(\\"${domain}\\")" \ | ||
| -l "traefik.http.routers.${subdomain}.entrypoints=websecure" \ | ||
| -l "traefik.http.routers.${subdomain}.tls=true" \ | ||
| -l "traefik.http.services.${subdomain}.loadbalancer.server.port=5173" \ | ||
| --network=codefox_traefik_network -p ${exposedPort}:5173 \ | ||
| -v "${directory}:/app" \ | ||
| ${imageName}`; | ||
| } else { | ||
| runCommand = `docker run -d --name ${containerName} -l "traefik.enable=true" \ | ||
| -l "traefik.http.routers.${subdomain}.rule=Host(\\"${domain}\\")" \ | ||
| -l "traefik.http.routers.${subdomain}.entrypoints=web" \ | ||
| -l "traefik.http.services.${subdomain}.loadbalancer.server.port=5173" \ | ||
| --network=codefox_traefik_network -p ${exposedPort}:5173 \ | ||
| -v "${directory}:/app" \ | ||
| ${imageName}`; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor to reduce duplication in Docker run commands.
The Docker run commands for TLS and non-TLS modes contain significant duplication. This makes maintenance difficult and increases the risk of inconsistencies.
Consider refactoring to a more maintainable approach:
- let runCommand;
- if (TLS) {
- runCommand = `docker run -d --name ${containerName} -l "traefik.enable=true" \
- -l "traefik.http.routers.${subdomain}.rule=Host(\\"${domain}\\")" \
- -l "traefik.http.routers.${subdomain}.entrypoints=websecure" \
- -l "traefik.http.routers.${subdomain}.tls=true" \
- -l "traefik.http.services.${subdomain}.loadbalancer.server.port=5173" \
- --network=codefox_traefik_network -p ${exposedPort}:5173 \
- -v "${directory}:/app" \
- ${imageName}`;
- } else {
- runCommand = `docker run -d --name ${containerName} -l "traefik.enable=true" \
- -l "traefik.http.routers.${subdomain}.rule=Host(\\"${domain}\\")" \
- -l "traefik.http.routers.${subdomain}.entrypoints=web" \
- -l "traefik.http.services.${subdomain}.loadbalancer.server.port=5173" \
- --network=codefox_traefik_network -p ${exposedPort}:5173 \
- -v "${directory}:/app" \
- ${imageName}`;
- }
+ // Set entrypoint and TLS options based on TLS setting
+ const entrypoint = TLS ? 'websecure' : 'web';
+ const tlsOption = TLS ? '-l "traefik.http.routers.${subdomain}.tls=true" ' : '';
+
+ const runCommand = `docker run -d --name ${containerName} -l "traefik.enable=true" \
+ -l "traefik.http.routers.${subdomain}.rule=Host(\\"${domain}\\")" \
+ -l "traefik.http.routers.${subdomain}.entrypoints=${entrypoint}" \
+ ${tlsOption}\
+ -l "traefik.http.services.${subdomain}.loadbalancer.server.port=5173" \
+ --network=codefox_traefik_network -p ${exposedPort}:5173 \
+ -v "${directory}:/app" \
+ ${imageName}`;📝 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.
| let runCommand; | |
| if (TLS) { | |
| runCommand = `docker run -d --name ${containerName} -l "traefik.enable=true" \ | |
| -l "traefik.http.routers.${subdomain}.rule=Host(\\"${domain}\\")" \ | |
| -l "traefik.http.routers.${subdomain}.entrypoints=websecure" \ | |
| -l "traefik.http.routers.${subdomain}.tls=true" \ | |
| -l "traefik.http.services.${subdomain}.loadbalancer.server.port=5173" \ | |
| --network=codefox_traefik_network -p ${exposedPort}:5173 \ | |
| -v "${directory}:/app" \ | |
| ${imageName}`; | |
| } else { | |
| runCommand = `docker run -d --name ${containerName} -l "traefik.enable=true" \ | |
| -l "traefik.http.routers.${subdomain}.rule=Host(\\"${domain}\\")" \ | |
| -l "traefik.http.routers.${subdomain}.entrypoints=web" \ | |
| -l "traefik.http.services.${subdomain}.loadbalancer.server.port=5173" \ | |
| --network=codefox_traefik_network -p ${exposedPort}:5173 \ | |
| -v "${directory}:/app" \ | |
| ${imageName}`; | |
| } | |
| // Set entrypoint and TLS options based on TLS setting | |
| const entrypoint = TLS ? 'websecure' : 'web'; | |
| const tlsOption = TLS ? '-l "traefik.http.routers.${subdomain}.tls=true" ' : ''; | |
| const runCommand = `docker run -d --name ${containerName} -l "traefik.enable=true" \ | |
| -l "traefik.http.routers.${subdomain}.rule=Host(\\"${domain}\\")" \ | |
| -l "traefik.http.routers.${subdomain}.entrypoints=${entrypoint}" \ | |
| ${tlsOption}\ | |
| -l "traefik.http.services.${subdomain}.loadbalancer.server.port=5173" \ | |
| --network=codefox_traefik_network -p ${exposedPort}:5173 \ | |
| -v "${directory}:/app" \ | |
| ${imageName}`; |
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> - **New Features** - Updated login flow: Email confirmation is now conditional, allowing sign-in when email functionality is disabled. - Introduced dynamic protocol handling for project previews and container deployments, enabling seamless HTTP/HTTPS transitions. - Added secure reverse proxy and TLS configurations to enhance connectivity and security. - **Chores** - Expanded configuration options for database and service settings. - Improved the build process with enhanced cleanup and modern networking adjustments. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Summary by CodeRabbit
New Features
Chores