Skip to content

refactor(db): parse DSN when defaulting postgres sslmode - #1199

Open
madhavilosetty-intel wants to merge 2 commits into
mainfrom
fix/postgres-migrate-sslmode
Open

refactor(db): parse DSN when defaulting postgres sslmode#1199
madhavilosetty-intel wants to merge 2 commits into
mainfrom
fix/postgres-migrate-sslmode

Conversation

@madhavilosetty-intel

@madhavilosetty-intel madhavilosetty-intel commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

setupHostedDB concatenated "?sslmode=disable" onto DB_URL, which forced plaintext for schema migrations and produced a malformed DSN when the URL already carried a query string.

Parse the DSN(Data Source Name) and only default sslmode when the operator hasn't set one, so verify-full and sslrootcert pass through untouched while the plaintext dev/compose Postgres keeps working.

@madhavilosetty-intel
madhavilosetty-intel requested a review from a team as a code owner August 12, 2026 23:46
@codecov

codecov Bot commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 70.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.14%. Comparing base (2296f65) to head (5490454).

Files with missing lines Patch % Lines
internal/app/migrate.go 0.00% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1199      +/-   ##
==========================================
+ Coverage   50.09%   50.14%   +0.04%     
==========================================
  Files         146      147       +1     
  Lines       13552    13564      +12     
==========================================
+ Hits         6789     6801      +12     
  Misses       6171     6171              
  Partials      592      592              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors Postgres migration DSN handling to avoid forcing sslmode=disable (and producing malformed URLs) by parsing the DSN and only defaulting sslmode when it’s not explicitly provided.

Changes:

  • Added applySSLMode to parse a Postgres URL and default sslmode=disable only when absent.
  • Updated hosted DB migration setup to use applySSLMode instead of string concatenation.
  • Added unit tests for applySSLMode and documented DSN TLS examples in .env.example.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
internal/app/migrate.go Introduces applySSLMode and applies it before running Postgres migrations to avoid malformed DSNs and preserve explicit TLS settings.
internal/app/migrate_test.go Adds table-driven tests covering sslmode defaulting/passthrough and malformed URL behavior.
.env.example Adds documentation guidance for Postgres TLS configuration via DB_URL.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .env.example Outdated
Comment thread pkg/db/dsn_test.go
@madhavilosetty-intel
madhavilosetty-intel force-pushed the fix/postgres-migrate-sslmode branch from 707af22 to 905aead Compare August 13, 2026 02:52
rsdmike
rsdmike previously approved these changes Aug 13, 2026
setupHostedDB concatenated "?sslmode=disable" onto DB_URL, which
forced plaintext for schema migrations and produced a malformed DSN
when the URL already carried a query string.

Parse the DSN and only default sslmode when the operator hasn't set
one, so verify-full and sslrootcert pass through untouched while the
plaintext dev/compose Postgres keeps working.
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