Add TLS/mTLS settings for postgreSQL and Redis #47
Conversation
There was a problem hiding this comment.
Please rename tls_settings.py to settings_helpers.py. Later on, we can move some other code to it.
Please format your code with Ruff. Only format your new file. Don't touch other files (this must be done in a dedicated PR somedays in the future).
@hongquan I have renamed the file as suggested.
and formatted the new file using ruff. |
src/pretix/settings_helpers.py
Outdated
| } | ||
| # add postgresql mTLS options | ||
| if config.has_option("database", "sslcert"): | ||
| db_tls_config.update( |
There was a problem hiding this comment.
I thinks in these lines, writing like this is clearer:
db_tls_config['sslcert'] = config.get('database', 'sslcert')There was a problem hiding this comment.
I have made the changes.
src/pretix/settings.py
Outdated
| @@ -3,7 +3,7 @@ | |||
| import os | |||
| import sys | |||
| from urllib.parse import urlparse | |||
|
|
|||
| from .settings_helpers import * | |||
There was a problem hiding this comment.
Please not use import *. Be explicit about what you want to import.
PEP8 coding style
There was a problem hiding this comment.
I have made these changes.
src/pretix/settings.py
Outdated
| "CLIENT_CLASS": "django_redis.client.DefaultClient", | ||
| "REDIS_CLIENT_KWARGS": {"health_check_interval": 30} | ||
| } | ||
| redis_tls_config = build_redis_tls_config(config) | ||
| if(redis_tls_config is not None): |
There was a problem hiding this comment.
You didn't format your code with Ruff?
There was a problem hiding this comment.
Ah, this file is an existing file, we don't run Ruff on it. Then you have to format your code manually, based on PEP8 coding style.
There was a problem hiding this comment.
formatted the code.
src/pretix/settings.py
Outdated
| @@ -209,22 +214,28 @@ | |||
|
|
|||
| HAS_REDIS = config.has_option('redis', 'location') | |||
| if HAS_REDIS: | |||
| OPTIONS = { | |||
There was a problem hiding this comment.
Please rename it to redis_options. We don't expose it as application settings, no reason to make it all-uppercase.
There was a problem hiding this comment.
Renamed to redis_options.
|
Due to challenges with the previous branch merges, it would be easiest if you could make this PR again against the current development branch. Appreciate your effort! |
This PR fixes: #28
Current behavior:-
With the current settings we can use PostgreSQL as database and Redis as cache, we only have to specify it in the configurations.
However, the current
settings.pydoes not specify any TLS/mTLS options for DB and cache.New Behavior:-
So to support TLS connections with the database and cache I have added new options only for postgresql database and redis caches.
I have added new keys such as sslmode, sslrootcert etc.
These values should be set in the configurations before running the server if we need to run in TLS/mTLS mode.