From ab5d604fe386d1fd482d2eb298e38b7850fdd8f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Olivier=20L=C3=A9obal?= Date: Mon, 26 Jun 2023 12:18:20 +0200 Subject: [PATCH] fix: issues from SECRET_KEY_LOAD_AND_STORE and settings reorganization (#676) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #668 added a new `SECRET_KEY_LOAD_AND_STORE` boolean setting in the backend. If `SECRET_KEY_LOAD_AND_STORE` is false, then the `SECRET_KEY` is generated at startup and not stored. If it is true, the program tries reading it from `SECRET_KEY_PATH`, and if that fails generate a new one and write it there. The intent was to allow configuring components that use the same settings as the backend server not to write the `SECRET_KEY` on the file system, because this eases configuration in production. In #668 this was set to `False` by default. However to match the pre-668 behavior you'd need to set it to `True` by default. Possibly we could get rid of this setting instead and just write the key every time. I don't know _why_ frontend authentication on k8s breaks when this setting is set to `false`. The `SECRET_KEY` is used for signing JWTs, so presumably at some point Django is restarted and so previously generated JWTs aren't valid anymore.. But JWTs are requested (and therefore generated) right before they're needed, so that's weird. --- #668 also moved the file containing this line: ```python BASE_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) ``` without updating it, which broke paths Co-authored-by: Guilhem Barthés Signed-off-by: Olivier Léobal --- backend/backend/settings/common.py | 2 +- backend/backend/settings/deps/path.py | 11 +++++------ backend/backend/settings/deps/secret_key.py | 8 ++++---- backend/backend/settings/dev.py | 6 +++--- backend/backend/settings/prod.py | 2 +- docs/settings.md | 4 ++-- 6 files changed, 16 insertions(+), 17 deletions(-) diff --git a/backend/backend/settings/common.py b/backend/backend/settings/common.py index 1097ad53e..bc34f2a9c 100644 --- a/backend/backend/settings/common.py +++ b/backend/backend/settings/common.py @@ -181,7 +181,7 @@ # https://docs.djangoproject.com/en/2.0/howto/static-files/ STATIC_URL = f"/{SUBPATH}static/" -MEDIA_ROOT = os.path.join(PROJECT_ROOT, "medias") +MEDIA_ROOT = PROJECT_ROOT / "medias" SITE_ID = 1 TASK = { diff --git a/backend/backend/settings/deps/path.py b/backend/backend/settings/deps/path.py index a49009cfc..65b141626 100644 --- a/backend/backend/settings/deps/path.py +++ b/backend/backend/settings/deps/path.py @@ -1,9 +1,8 @@ -import os import sys +from pathlib import Path -# Build paths inside the project like this: os.path.join(BASE_DIR, ...) -BASE_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) -PROJECT_ROOT = os.path.dirname(BASE_DIR) +BASE_DIR: Path = Path(__file__).resolve().parents[2] +PROJECT_ROOT = BASE_DIR.parent -sys.path.append(PROJECT_ROOT) -sys.path.append(os.path.normpath(os.path.join(PROJECT_ROOT, "libs"))) +sys.path.append(str(PROJECT_ROOT)) +sys.path.append(str(PROJECT_ROOT / "libs")) diff --git a/backend/backend/settings/deps/secret_key.py b/backend/backend/settings/deps/secret_key.py index 3ad15fcec..11954dba7 100644 --- a/backend/backend/settings/deps/secret_key.py +++ b/backend/backend/settings/deps/secret_key.py @@ -3,13 +3,13 @@ """ import os -import pathlib import secrets +from pathlib import Path from . import path from .utils import to_bool -SECRET_KEY_PATH = os.environ.get("SECRET_KEY_PATH", os.path.normpath(os.path.join(path.PROJECT_ROOT, "SECRET"))) +SECRET_KEY_PATH = Path(os.environ.get("SECRET_KEY_PATH", path.PROJECT_ROOT / "SECRET")) def _generate_secret_key(): @@ -17,12 +17,12 @@ def _generate_secret_key(): _SECRET_KEY_LOAD_AND_STORE = to_bool( - os.environ.get("SECRET_KEY_LOAD_AND_STORE", "False") + os.environ.get("SECRET_KEY_LOAD_AND_STORE", "True") ) # Whether to load the secret key from file (and write it there if it doesn't exist) if _SECRET_KEY_LOAD_AND_STORE: try: - SECRET_KEY = pathlib.Path(SECRET_KEY_PATH).read_text().strip() + SECRET_KEY = SECRET_KEY_PATH.read_text().strip() except IOError: try: SECRET_KEY = _generate_secret_key() diff --git a/backend/backend/settings/dev.py b/backend/backend/settings/dev.py index bdbccf8d4..941414e17 100644 --- a/backend/backend/settings/dev.py +++ b/backend/backend/settings/dev.py @@ -11,7 +11,7 @@ DEBUG = True -STATIC_ROOT = os.path.join(BASE_DIR, "statics") +STATIC_ROOT = BASE_DIR / "statics" # Enable Browsable API REST_FRAMEWORK["DEFAULT_RENDERER_CLASSES"] += ("rest_framework.renderers.BrowsableAPIRenderer",) @@ -25,8 +25,8 @@ MODEL_BUCKET_NAME = "substra-model" MODEL_STORAGE = MinioStorage(MODEL_BUCKET_NAME) -MEDIA_ROOT = os.environ.get("MEDIA_ROOT", os.path.join(PROJECT_ROOT, f"medias/{ORG_NAME}")) -SERVERMEDIAS_ROOT = os.environ.get("SERVERMEDIAS_ROOT", os.path.join(PROJECT_ROOT, f"servermedias/{ORG_NAME}")) +MEDIA_ROOT = os.environ.get("MEDIA_ROOT", str(PROJECT_ROOT / "medias" / ORG_NAME)) +SERVERMEDIAS_ROOT = os.environ.get("SERVERMEDIAS_ROOT", str(PROJECT_ROOT / "servermedias" / ORG_NAME)) FUNCTION_BUCKET_NAME = "substra-function" FUNCTION_STORAGE = MinioStorage(FUNCTION_BUCKET_NAME) diff --git a/backend/backend/settings/prod.py b/backend/backend/settings/prod.py index d428249c3..bf8c4fb78 100644 --- a/backend/backend/settings/prod.py +++ b/backend/backend/settings/prod.py @@ -20,7 +20,7 @@ SESSION_COOKIE_SECURE = True CSRF_COOKIE_SECURE = True -STATIC_ROOT = os.path.join(BASE_DIR, "statics") +STATIC_ROOT = BASE_DIR / "statics" DATASAMPLE_BUCKET_NAME = "substra-datasample" DATASAMPLE_STORAGE = MinioStorage(DATASAMPLE_BUCKET_NAME) diff --git a/docs/settings.md b/docs/settings.md index 6d694e396..8926ecbb6 100644 --- a/docs/settings.md +++ b/docs/settings.md @@ -67,8 +67,8 @@ Accepted true values for `bool` are: `1`, `ON`, `On`, `on`, `T`, `t`, `TRUE`, `T | Type | Setting | Default value | Comment | |------|---------|---------------|---------| -| bool | `SECRET_KEY_LOAD_AND_STORE` | `False` | Whether to load the secret key from file (and write it there if it doesn't exist) | -| string | `SECRET_KEY_PATH` | `os.path.normpath(os.path.join(path.PROJECT_ROOT, 'SECRET'))` | | +| bool | `SECRET_KEY_LOAD_AND_STORE` | `True` | Whether to load the secret key from file (and write it there if it doesn't exist) | +| string | `SECRET_KEY_PATH` | `path.PROJECT_ROOT / 'SECRET'` | | ## JWT settings