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

Support PostgreSQL Databases #437

Merged
merged 32 commits into from
Sep 30, 2024
Merged

Support PostgreSQL Databases #437

merged 32 commits into from
Sep 30, 2024

Conversation

carsonab
Copy link
Contributor

@carsonab carsonab commented Sep 28, 2024

Adds support for PostgreSQL:

  • PostgreSQL database connections with TLS
    • Local testing can be done against a alloydbomni container
    • Added gencerts.sh to create certs used in local server config and client tests to enable TLS tests
  • AlloyDB connections using alloybd-go-connector
  • Uses single database driver pgx

Local testing can be done against a alloydbomni container, which uses the same database engine as hosted AlloyDB, using a normal postgres connection. Connections with a alloydb-go-connector need a GCP AlloyDB instance to connect to. However both connection methods use the same database driver. This means our local testing of business logic and its sql queries use the same database driver and engine as higher environments, and only the connection method will differ between environments.

Tested with alloybd-go-connector successfully using hosted AlloyDB instance for both IAM and native db login.

TODOs:

  • handle closing dialer from alloydbconn.NewDialer
  • fix CI runners (these tests pass locally on macOS with docker desktop)

NOTE:

  • Tested client certs by manually editing the server's pg_hba.conf to require client certs with hostssl all all all scram-sha-256 clientcert=verify-full but not automated to do that

docker-compose.yml Outdated Show resolved Hide resolved
dsn = fmt.Sprintf(
// sslmode is disabled because the alloy db connection dialer will handle it
// no password is used with IAM
"user=%s dbname=%s sslmode=disable",
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason we format a DSN here but for vanilla postgres use their URI syntax?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Either should work. I just preferred the key=value syntax when not giving a server (since thats handled by the alloydbconn library when connecting to a hosted AlloyDB instance)

InfernoJJ
InfernoJJ previously approved these changes Sep 30, 2024
@adamdecaf adamdecaf marked this pull request as ready for review September 30, 2024 17:31
@adamdecaf adamdecaf merged commit 12d4ae9 into master Sep 30, 2024
15 checks passed
@carsonab carsonab changed the title WIP: Support PostgreSQL Databases Support PostgreSQL Databases Sep 30, 2024
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