-
Notifications
You must be signed in to change notification settings - Fork 392
feat: allow for postgres ssl connections #607
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
Conversation
@JGaudette is attempting to deploy a commit to the goldflag's projects Team on Vercel.
A member of the Team first needs to authorize it.
WalkthroughIntroduces a POSTGRES_SSL environment variable across config, deployment, and docs. Backend Postgres client (drizzle and postgres client) now enables SSL only when POSTGRES_SSL == "true". Adds the variable to .env.example and docker-compose.cloud.yml, and documents it in self-hosting-advanced.mdx. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Admin as Operator/Admin
participant Backend as Backend (Drizzle / PG client)
participant PG as PostgreSQL
Admin->>Backend: Set env POSTGRES_SSL=("true"|"false")
Note right of Backend: process.env.POSTGRES_SSL read during startup
alt POSTGRES_SSL == "true"
Backend->>PG: Connect with SSL enabled
PG-->>Backend: Connection established (SSL)
else POSTGRES_SSL != "true"
Backend->>PG: Connect without SSL
PG-->>Backend: Connection established (non-SSL)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
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)
.env.example (1)
26-26: Consider reordering for consistency.The static analysis tool suggests placing
POSTGRES_SSLbeforePOSTGRES_USERto maintain alphabetical ordering within the PostgreSQL configuration section.Apply this diff to reorder the keys:
# PostgreSQL Database Configuration POSTGRES_DB=analytics +POSTGRES_SSL=false POSTGRES_USER=frog POSTGRES_PASSWORD=frog -POSTGRES_SSL=falseserver/drizzle.config.ts (1)
16-16: Simplify the boolean expression.The ternary operator is redundant since the equality check already returns a boolean.
Apply this diff to simplify:
- ssl: process.env.POSTGRES_SSL === "true" ? true : false, + ssl: process.env.POSTGRES_SSL === "true",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.env.example(1 hunks)docker-compose.cloud.yml(1 hunks)docs/src/content/self-hosting-advanced.mdx(1 hunks)server/drizzle.config.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
{client,server}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{client,server}/**/*.{ts,tsx}: Use TypeScript with strict typing throughout both client and server
Use try/catch blocks with specific error types for error handling
Use camelCase for variables and functions, PascalCase for components and types
Group imports by external, then internal, and sort alphabetically within groups
Files:
server/drizzle.config.ts
server/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Backend: Use Fastify, Drizzle ORM (Postgres), ClickHouse, and Zod
Files:
server/drizzle.config.ts
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 26-26: [UnorderedKey] The POSTGRES_SSL key should go before the POSTGRES_USER key
(UnorderedKey)
🔇 Additional comments (2)
docs/src/content/self-hosting-advanced.mdx (1)
70-70: LGTM!The documentation correctly reflects the new
POSTGRES_SSLenvironment variable with the appropriate default value for backwards compatibility.docker-compose.cloud.yml (1)
89-89: LGTM!The environment variable is correctly configured with a sensible default, maintaining backwards compatibility while allowing SSL to be enabled when needed.
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: 1
🧹 Nitpick comments (1)
server/src/db/postgres/postgres.ts (1)
15-15: Simplify the redundant ternary expression.The expression
process.env.POSTGRES_SSL === "true" ? true : falsecan be simplified.Apply this diff to simplify:
- ssl: process.env.POSTGRES_SSL === "true" ? true : false, + ssl: process.env.POSTGRES_SSL === "true",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
server/src/db/postgres/postgres.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
{client,server}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{client,server}/**/*.{ts,tsx}: Use TypeScript with strict typing throughout both client and server
Use try/catch blocks with specific error types for error handling
Use camelCase for variables and functions, PascalCase for components and types
Group imports by external, then internal, and sort alphabetically within groups
Files:
server/src/db/postgres/postgres.ts
server/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Backend: Use Fastify, Drizzle ORM (Postgres), ClickHouse, and Zod
Files:
server/src/db/postgres/postgres.ts
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.
Consider SSL certificate validation for production environments.
While enabling SSL with ssl: true encrypts the connection, it doesn't validate the server's certificate by default. For production environments like RDS, you should verify the server certificate to prevent man-in-the-middle attacks.
Consider using an object configuration for SSL to enable certificate validation:
- ssl: process.env.POSTGRES_SSL === "true" ? true : false, + ssl: process.env.POSTGRES_SSL === "true" ? { rejectUnauthorized: true } : false,
Note: RDS instances typically require certificate validation. If you need to provide a custom CA certificate, the postgres library also supports:
ssl: { rejectUnauthorized: true, ca: fs.readFileSync('/path/to/ca-certificate.crt').toString(), }
🤖 Prompt for AI Agents
In server/src/db/postgres/postgres.ts around line 15, the SSL option is
currently set as a boolean which enables encryption but does not enforce server
certificate validation; change the ssl option to an object that sets
rejectUnauthorized: true for production (e.g., when NODE_ENV === "production" or
POSTGRES_SSL === "true") and, if required by your RDS setup, load a CA bundle
from a configured path (read the CA file with fs.readFileSync and pass it as
ca); ensure you conditionally construct this object (or leave ssl: false for
non-SSL envs), and add the needed fs import and error handling for missing CA
file.
Uh oh!
There was an error while loading. Please reload this page.
We use RDS and require an ssl connection in our environment. This PR still defaults the ssl mode to false for backwards compatibility, but also introduced an environment variable to overwrite that and enable SSL mode on the postgres connection.
Summary by CodeRabbit