[PP-1775] Ensure ssl connections to postgres work when ssl required #286

dbernstein · 2024-10-23T18:29:38Z

Description

When we upgraded to Postgres 16 in the tpp-dev environment, VLC was failing because PG 16 requires SSL connections by default.

This update fixes the problem in the docker container. In order to verify SSL connections are happening I enabled SSL for postgres 16 in the docker compose script and for extra certainty that the client is using SSL I updated the dev settings to specify the pg ssl mode.

THIS PR should be merged after #285 is merged.

Motivation and Context

https://ebce-lyrasis.atlassian.net/browse/PP-1775

How Has This Been Tested?

Manually tested using docker compose up. Without the update to the docker image I was getting the same error we were seeing in tpp-dev. With it I, I am able to connect.

Checklist

I have updated the documentation accordingly.
All new and existing tests passed.

codecov · 2024-10-23T19:34:18Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.15%. Comparing base (1b1c78c) to head (95125f3).
Report is 5 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #286   +/-   ##
=======================================
  Coverage   88.15%   88.15%           
=======================================
  Files          37       37           
  Lines        2271     2271           
  Branches      304      304           
=======================================
  Hits         2002     2002           
  Misses        215      215           
  Partials       54       54

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

…uired. Additional updates: * upgrades image to python 3.12 * upgrades postgres to 16 (to match latest RDS deployment)

jonathangreen

Made a few comments that would be good to clean up before merging.

There is also a reference to postgres:12 in the README that should get updated to postgres:16

virtual-library-card/README.md

Lines 142 to 146 in 1b1c78c

    
           ### 2. Create PostgreSQL Container (Only for testing) 
        
           Either create a new database in the production PostgreSQL Database. Or use the docker PostgreSQL container for testing. 
        
               docker run -d --name pg --rm -e POSTGRES_USER=vlc -e POSTGRES_PASSWORD=test -e POSTGRES_DB=virtual_library_card postgres:12

jonathangreen · 2024-10-23T18:44:02Z

Dockerfile

@@ -1,4 +1,4 @@
-FROM python:3.10-slim
+FROM python:3.12-slim


This change updates the Python version in the container to Python 3.12. Probably don't want to make this change until we update CI to test against Python 3.12 as well. Maybe leave this at Python 3.10 for now, since all our other containers are using 3.10 at the moment.

jonathangreen · 2024-10-24T12:29:52Z

Dockerfile

+# required for postgres ssl: the crt file doesn't exist
+# but the path must point to a visible directory otherwise we
+# get a permissions error
+ENV PGSSLCERT /tmp/postgresql.crt


Should update this to the new env var format to fix the deprecation warning.

Suggested change

ENV PGSSLCERT /tmp/postgresql.crt

ENV PGSSLCERT=/tmp/postgresql.crt

jonathangreen · 2024-10-24T12:42:11Z

docker-compose.yml

+    command: >
+             -c ssl=on
+             -c ssl_cert_file=/etc/ssl/certs/ssl-cert-snakeoil.pem
+             -c ssl_key_file=/etc/ssl/private/ssl-cert-snakeoil.key


Might be helpful to our future selves to add a comment here about why these lines are required. I'm thinking something like:

AWS RDS requires SSL, so we enable SSL for the database with a self signed snakeoil cert, so this database mimics the production databases as closely as we can.

jonathangreen · 2024-10-24T12:50:26Z

tox.ini

 environment =
    POSTGRES_USER=vlc
    POSTGRES_PASSWORD=test
    POSTGRES_DB=test_virtual_library_card_dev
+


Minor: Unnecessary space added

jonathangreen · 2024-10-24T12:50:40Z

tox.ini

@@ -14,6 +14,8 @@ docker =
    docker: minio-vlc
 setenv =
    COVERAGE_FILE = .coverage.{envname}
+    PG_SSL_MODE = allow
+


Minor: Unnecessary space added

jonathangreen · 2024-10-24T13:02:18Z

virtual_library_card/settings/dev.py

@@ -22,6 +22,7 @@
        "PASSWORD": "test",
        "HOST": os.environ.get("VLC_DEV_DB_HOST", "pg"),
        "PORT": os.environ.get("VLC_DEV_DB_PORT", "5432"),
+        "OPTIONS": {"sslmode": os.environ.get("PG_SSL_MODE", "require")},


I think this should follow the same naming scheme we already have for these environment variables. Based on the name, I assumed it was a PG environment variable and I was looking in the PG docs to try to figure out what it did, until I found it defined here.

I'd suggest something like VLC_DEV_DB_SSL_MODE.

dbernstein · 2024-10-24T17:11:36Z

Thank you for the thorough review :)

dbernstein force-pushed the PP-1775-ensure-ssl-connections-to-postgres-work-when-ssl-required branch 2 times, most recently from d39ee8c to 0f19917 Compare October 23, 2024 18:38

dbernstein force-pushed the PP-1775-ensure-ssl-connections-to-postgres-work-when-ssl-required branch 2 times, most recently from d025abf to d69aeea Compare October 23, 2024 19:43

dbernstein requested a review from jonathangreen October 23, 2024 22:41

dbernstein force-pushed the PP-1775-ensure-ssl-connections-to-postgres-work-when-ssl-required branch from 492ff49 to de5e2f4 Compare October 23, 2024 22:45

dbernstein added 3 commits October 23, 2024 15:52

[PP-1775] Ensure that application to talk to postgres when SSL is req…

929c385

…uired. Additional updates: * upgrades image to python 3.12 * upgrades postgres to 16 (to match latest RDS deployment)

Fix tox

fc9c682

Remove obsolete version in compose file.

c19bb60

dbernstein force-pushed the PP-1775-ensure-ssl-connections-to-postgres-work-when-ssl-required branch from de5e2f4 to c19bb60 Compare October 23, 2024 22:53

jonathangreen approved these changes Oct 24, 2024

View reviewed changes

PR cleanup.

95125f3

dbernstein merged commit f5b625d into main Oct 24, 2024
8 checks passed

dbernstein deleted the PP-1775-ensure-ssl-connections-to-postgres-work-when-ssl-required branch October 24, 2024 17:37

dbernstein added the bug Something isn't working label Oct 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PP-1775] Ensure ssl connections to postgres work when ssl required #286

[PP-1775] Ensure ssl connections to postgres work when ssl required #286

dbernstein commented Oct 23, 2024 •

edited

Loading

codecov bot commented Oct 23, 2024 •

edited

Loading

jonathangreen left a comment

jonathangreen Oct 23, 2024

jonathangreen Oct 24, 2024

jonathangreen Oct 24, 2024

jonathangreen Oct 24, 2024

jonathangreen Oct 24, 2024

jonathangreen Oct 24, 2024

dbernstein commented Oct 24, 2024

	### 2. Create PostgreSQL Container (Only for testing)

	Either create a new database in the production PostgreSQL Database. Or use the docker PostgreSQL container for testing.

	docker run -d --name pg --rm -e POSTGRES_USER=vlc -e POSTGRES_PASSWORD=test -e POSTGRES_DB=virtual_library_card postgres:12

	ENV PGSSLCERT /tmp/postgresql.crt
	ENV PGSSLCERT=/tmp/postgresql.crt

[PP-1775] Ensure ssl connections to postgres work when ssl required #286

[PP-1775] Ensure ssl connections to postgres work when ssl required #286

Conversation

dbernstein commented Oct 23, 2024 • edited Loading

Description

Motivation and Context

How Has This Been Tested?

Checklist

codecov bot commented Oct 23, 2024 • edited Loading

Codecov Report

jonathangreen left a comment

Choose a reason for hiding this comment

jonathangreen Oct 23, 2024

Choose a reason for hiding this comment

jonathangreen Oct 24, 2024

Choose a reason for hiding this comment

jonathangreen Oct 24, 2024

Choose a reason for hiding this comment

jonathangreen Oct 24, 2024

Choose a reason for hiding this comment

jonathangreen Oct 24, 2024

Choose a reason for hiding this comment

jonathangreen Oct 24, 2024

Choose a reason for hiding this comment

dbernstein commented Oct 24, 2024

dbernstein commented Oct 23, 2024 •

edited

Loading

codecov bot commented Oct 23, 2024 •

edited

Loading