From 660a35e0e122d1e9e2ce8c8c7e6a4a7b7854684a Mon Sep 17 00:00:00 2001 From: Bruno Rocha Date: Wed, 9 Aug 2023 20:57:37 +0100 Subject: [PATCH 1/8] Add Support for Dynamic Settings Requires Dynaconf 3.3 Issue: AAH-2009 --- .compose.env.example | 3 + CHANGES/2009.feature | 1 + dev/docker-compose.yml | 2 + docs/config/options.md | 1 + galaxy_ng/app/dynaconf_hooks.py | 85 ++++++++++ galaxy_ng/app/dynamic_settings.py | 60 +++++++ galaxy_ng/app/migrations/0042_setting.py | 59 +++++++ galaxy_ng/app/models/__init__.py | 2 + galaxy_ng/app/models/config.py | 193 +++++++++++++++++++++++ galaxy_ng/app/settings.py | 4 + galaxy_ng/app/tasks/settings_cache.py | 138 ++++++++++++++++ galaxy_ng/tests/unit/test_models.py | 78 ++++++++- 12 files changed, 625 insertions(+), 1 deletion(-) create mode 100644 CHANGES/2009.feature create mode 100644 galaxy_ng/app/dynamic_settings.py create mode 100644 galaxy_ng/app/migrations/0042_setting.py create mode 100644 galaxy_ng/app/models/config.py create mode 100644 galaxy_ng/app/tasks/settings_cache.py diff --git a/.compose.env.example b/.compose.env.example index 8c290a5dc1..a42a5628a6 100644 --- a/.compose.env.example +++ b/.compose.env.example @@ -85,3 +85,6 @@ ENABLE_SIGNING=1 ## Override the github urls for social auth # SOCIAL_AUTH_GITHUB_BASE_URL= # SOCIAL_AUTH_GITHUB_API_URL= + +# Enable Dynamic Settings +# PULP_GALAXY_DYNAMIC_SETTINGS=true diff --git a/CHANGES/2009.feature b/CHANGES/2009.feature new file mode 100644 index 0000000000..60b7cdecfb --- /dev/null +++ b/CHANGES/2009.feature @@ -0,0 +1 @@ +Add support for dynamic settings diff --git a/dev/docker-compose.yml b/dev/docker-compose.yml index 23989ac005..63ca0b00de 100644 --- a/dev/docker-compose.yml +++ b/dev/docker-compose.yml @@ -122,6 +122,8 @@ services: redis: image: "redis:5" + ports: + - "6379:6379" volumes: - "redis_data:/data" diff --git a/docs/config/options.md b/docs/config/options.md index 24d8a04f4a..70156d6c78 100644 --- a/docs/config/options.md +++ b/docs/config/options.md @@ -172,6 +172,7 @@ Here is a [diagram explaining](https://www.xmind.net/m/VPSF59/#) the loading ord | `GALAXY_SIGNATURE_UPLOAD_ENABLED` | Used by UI to hide/show the upload buttons for signature, Default: `False` | | `GALAXY_REQUIRE_SIGNATURE_FOR_APPROVAL` | Approval dashboard and move endpoint must require signature?, Default: `False` | | `GALAXY_MINIMUM_PASSWORD_LENGTH` | Minimum password lenght for validation, Default: 9 | +| `GALAXY_DYNAMIC_SETTINGS` | Enables dynamic settings feature, Default `False` | For SSO Keycloak configuration see [keycloak](../dev/docker_environment.md#keycloak) diff --git a/galaxy_ng/app/dynaconf_hooks.py b/galaxy_ng/app/dynaconf_hooks.py index abf92c87db..89754cc496 100755 --- a/galaxy_ng/app/dynaconf_hooks.py +++ b/galaxy_ng/app/dynaconf_hooks.py @@ -1,4 +1,5 @@ import json +import logging import ldap import pkg_resources import os @@ -6,6 +7,11 @@ from typing import Any, Dict, List from django_auth_ldap.config import LDAPSearch from dynaconf import Dynaconf, Validator +from galaxy_ng.app.dynamic_settings import DYNAMIC_SETTINGS_SCHEMA +from django.apps import apps + + +logger = logging.getLogger(__name__) def post(settings: Dynaconf) -> Dict[str, Any]: @@ -42,6 +48,9 @@ def post(settings: Dynaconf) -> Dict[str, Any]: # rest framework auth classes too. data.update(configure_authentication_classes(settings, data)) + # This must go last, so that all the default settings are loaded before dynamic and validation + data.update(configure_dynamic_settings(settings)) + validate(settings) return data @@ -582,3 +591,79 @@ def validate(settings: Dynaconf) -> None: ) settings.validators.validate() + + +def configure_dynamic_settings(settings: Dynaconf) -> Dict[str, Any]: + """Dynaconf 3.3 allows registration of hooks on methods `get` and `as_dict` + + For galaxy this enables the Dynamic Settings feature, which triggers a + specified function after every key is accessed. + + So after the normal get process, the registered hook will be able to + change the value before it is returned allowing reading overrides from + database and cache. + """ + if settings.get("GALAXY_DYNAMIC_SETTINGS") is not True: + return {} + + # Perform lazy imports here to avoid breaking when system runs with older + # dynaconf versions + try: + from dynaconf import DynaconfFormatError, DynaconfParseError + from dynaconf.loaders.base import SourceMetadata + from dynaconf.hooking import Hook, Action, HookValue + from dynaconf.base import RESERVED_ATTRS, UPPER_DEFAULT_SETTINGS, Settings + except ImportError as exc: + # Graceful degradation for dynaconf < 3.3 where method hooking is not available + logger.error( + "Dynaconf method hooking requires Dynaconf >=3.3: %s", + str(exc) + ) + return {} + + logger.info("Enabling Dynamic Settings Feature") + def read_settings_from_cache_or_db(settings_dict:Dict, value: HookValue, key: str, *args, **kwargs) -> Any: + """A function to be attached on Dynaconf Afterget hook. + Load everything from settings cache or db, process parsing and mergings, returns the desired key value + """ + if not apps.ready or key.upper() not in DYNAMIC_SETTINGS_SCHEMA: + # If app is starting up or key is not on allowed list bypass and just return the unwrapped value + return value.value + + # It is required to build a new Settings object to trigger parsing and mergings + temp_settings = Settings() + metadata = SourceMetadata(loader="hooking", identifier="settings_dict", env="main", merged=True) + reserved_keys = set(RESERVED_ATTRS + UPPER_DEFAULT_SETTINGS) + allowed_keys = settings_dict.keys() - reserved_keys + temp_data = {k: v for k, v in settings_dict.items() if k in allowed_keys} + temp_settings.update(temp_data, tomlfy=True, loader_identifier=metadata) + + # lazy import because it can't happen before apps are ready + from galaxy_ng.app.tasks.settings_cache import get_settings_from_cache, get_settings_from_db + if (data := get_settings_from_cache()): + metadata = SourceMetadata(loader="hooking", identifier="cache", env="main", merged=True) + else: + data = get_settings_from_db() + if data: + metadata = SourceMetadata(loader="hooking", identifier="db", env="main", merged=True) + + try: + if data: + temp_settings.update(data, loader_identifier=metadata, tomlfy=True) + except (DynaconfFormatError, DynaconfParseError) as exc: + logger.error("Error loading dynamic settings: %s", str(exc)) + + if not data: + logger.debug("Dynamic settings are empty, reading key %s from default sources", key) + elif key in [_k.split("__")[0] for _k in data]: + logger.debug("Dynamic setting for key: %s loaded from %s", key, metadata.identifier) + else: + logger.debug("Key %s not on db/cache, %s other keys loaded from %s", key, len(data), metadata.identifier) + + return temp_settings.get(key, value.value) + + return { + "_registered_hooks": { + Action.AFTER_GET: [Hook(read_settings_from_cache_or_db)] + } + } diff --git a/galaxy_ng/app/dynamic_settings.py b/galaxy_ng/app/dynamic_settings.py new file mode 100644 index 0000000000..4a8454de0c --- /dev/null +++ b/galaxy_ng/app/dynamic_settings.py @@ -0,0 +1,60 @@ +""" +type: +default: +description: +choices: +key_hints: +value_hints: +""" +DYNAMIC_SETTINGS_SCHEMA = { + "FOO": { # for testing + "type": str, + "default": "bar", + "description": "Foo bar", + "value_hints": ["bar", "baz"], + }, + "GALAXY_REQUIRE_CONTENT_APPROVAL": { + "type": bool, + "default": False, + "description": "Require content approval before it can be published", + "choices": [True, False], + }, + "GALAXY_REQUIRE_SIGNATURE_FOR_APPROVAL": { + "type": bool, + "default": False, + "description": "Require signature for content approval", + "choices": [True, False], + }, + "GALAXY_SIGNATURE_UPLOAD_ENABLED": { + "type": bool, + "default": False, + "description": "Enable signature upload", + "choices": [True, False], + }, + "GALAXY_AUTO_SIGN_COLLECTIONS": { + "type": bool, + "default": False, + "description": "Automatically sign collections during approval/upload", + "choices": [True, False], + }, + "GALAXY_FEATURE_FLAGS": { + "type": dict, + "default": {}, + "description": "Feature flags for galaxy_ng", + "key_hints": { + "execution_environments": { + "type": bool, + "default": False, + "description": "Enable execution environments", + "choices": [True, False], + }, + } + }, + # For 1816 PR + "INSIGHTS_TRACKING_STATE": {}, + "AUTOMATION_ANALYTICS_URL": {}, + "REDHAT_USERNAME": {}, + "REDHAT_PASSWORD": {}, + "AUTOMATION_ANALYTICS_LAST_GATHERED": {}, + "AUTOMATION_ANALYTICS_LAST_ENTRIES": {}, +} diff --git a/galaxy_ng/app/migrations/0042_setting.py b/galaxy_ng/app/migrations/0042_setting.py new file mode 100644 index 0000000000..acc79859c5 --- /dev/null +++ b/galaxy_ng/app/migrations/0042_setting.py @@ -0,0 +1,59 @@ +# Generated by Django 4.2.3 on 2023-07-21 15:45 + +from django.conf import settings +import django.core.validators +from django.db import migrations, models +import django.db.models.deletion +import django_lifecycle.mixins + + +class Migration(migrations.Migration): + dependencies = [ + ("galaxy", "0041_alter_containerregistryremote_remote_ptr"), + ] + + operations = [ + migrations.CreateModel( + name="Setting", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, primary_key=True, serialize=False, verbose_name="ID" + ), + ), + ( + "key", + models.CharField( + max_length=255, + validators=[ + django.core.validators.RegexValidator( + "^(?!\\d)[a-zA-Z0-9_]+$", + "alphanumeric, no spaces, no hyphen, only underscore cant start with a number.", + ) + ], + ), + ), + ("value", models.TextField()), + ("version", models.IntegerField(default=1)), + ("is_secret", models.BooleanField(default=False)), + ("created_at", models.DateTimeField(auto_now_add=True)), + ( + "user", + models.ForeignKey( + default=None, + editable=False, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="settings", + to=settings.AUTH_USER_MODEL, + ), + ), + ], + options={ + "permissions": (("edit_setting", "Can edit setting"),), + "unique_together": {("key", "version")}, + }, + bases=(django_lifecycle.mixins.LifecycleModelMixin, models.Model), + ), + ] diff --git a/galaxy_ng/app/models/__init__.py b/galaxy_ng/app/models/__init__.py index fdc0e5b9ac..af71ee1fe3 100644 --- a/galaxy_ng/app/models/__init__.py +++ b/galaxy_ng/app/models/__init__.py @@ -5,6 +5,7 @@ from .collectionimport import ( CollectionImport, ) +from .config import Setting from .namespace import ( Namespace, NamespaceLink, @@ -31,6 +32,7 @@ 'CollectionImport', 'Namespace', 'NamespaceLink', + 'Setting', 'SyncList', 'ContainerDistribution', 'ContainerDistroReadme', diff --git a/galaxy_ng/app/models/config.py b/galaxy_ng/app/models/config.py new file mode 100644 index 0000000000..2cb29b955d --- /dev/null +++ b/galaxy_ng/app/models/config.py @@ -0,0 +1,193 @@ +import logging +import time + +from django.conf import settings +from django.core.validators import RegexValidator +from django.db import models, transaction +from django.db.models.functions import Lower +from django_lifecycle import ( + AFTER_CREATE, + AFTER_DELETE, + BEFORE_CREATE, + LifecycleModelMixin, + hook +) + +from galaxy_ng.app.dynamic_settings import DYNAMIC_SETTINGS_SCHEMA + +logger = logging.getLogger(__name__) + +setting_key_validator = RegexValidator( + r"^(?!\d)[a-zA-Z0-9_]+$", + "alphanumeric, no spaces, no hyphen, only underscore cant start with a number.", +) + +__all__ = ["Setting"] + + +class SettingsManager(models.Manager): + def create(self, key, value, *args, **kwargs): + """Creates a new Setting entry, keeping the latest 10 versions in DB + uses a lock to ensure no version bump collisions. + + If Redis is not available, skip locking and just create the new entry. + """ + from galaxy_ng.app.tasks.settings_cache import acquire_lock, release_lock # noqa + + for __ in range(10): # try 10 times + lock = acquire_lock(key) + if lock is not None: + break + time.sleep(.5) # blocking + + if lock is None: + raise Exception("Could not acquire lock for key") + + existing = self.get_queryset().filter(key=key) + if existing: + kwargs["version"] = existing.latest("version").version + 1 + + new = super().create(key=key, value=value, *args, **kwargs) + + release_lock(key, lock) + + # Delete old versions + self.get_queryset().filter(key=key, version__lt=new.version - 10).delete() + + return new + + def filter(self, *args, **kwargs): + """case insensitive filter""" + if "key" in kwargs: + kwargs["key__iexact"] = kwargs.pop("key") + return super().filter(*args, **kwargs) + + +class Setting(LifecycleModelMixin, models.Model): + """Setting model stores key:value for Dynaconf + The recommended usage is via custom methods. + + Setting.set_value_in_db('FOO__BAR', 'baz') + Setting.get_value_from_db('FOO__BAR') -> 'baz' + Setting.as_dict() -> {'FOO__BAR': 'baz'} + Setting.get_all() -> [ + Setting(key='FOO__BAR', value='baz', user=None, version=1, is_secret=False, created_at=...) + ] + """ + objects = SettingsManager() + + key = models.CharField( + max_length=255, + null=False, + validators=[setting_key_validator], + ) + value = models.TextField(blank=False, null=False) + user = models.ForeignKey( + settings.AUTH_USER_MODEL, + null=True, + on_delete=models.SET_NULL, + editable=False, + default=None, + related_name="settings", + ) + version = models.IntegerField(default=1, null=False) + is_secret = models.BooleanField(default=False, null=False) + created_at = models.DateTimeField(auto_now_add=True) + + @classmethod + def update_cache(cls): + from galaxy_ng.app.tasks.settings_cache import update_setting_cache # noqa + + update_setting_cache(cls.as_dict()) + + @hook(AFTER_CREATE, on_commit=True) + def _hook_update_create(self): + """After create update the whole cache""" + self.update_cache() + logger.debug( + "Settings cache updated - create - %s[%s]:%s", self.key, self.version, self.value + ) + + @hook(AFTER_DELETE, on_commit=True) + def _hook_delete_cache(self): + self.update_cache() + logger.debug( + "Settings cache updated delete - %s[%s]:%s", self.key, self.version, self.value + ) + + @hook(BEFORE_CREATE) + def _hook_validate(self): + """Validate and transform before creation. + + 1. Fix the True/False problem: + Dynaconf only parses as boolean if values are lowercase true/false + or explicitly marked with @bool + So if value is True or False, we transform to lowercase. + OR stringify value + 2. Validate against Dynacic Settings Schema + 3. Validate against Dynaconf Validators + """ + self.value = ( + str(self.value).lower() if str(self.value) in ["True", "False"] else str(self.value) + ) + + if self.base_key not in DYNAMIC_SETTINGS_SCHEMA: + raise Exception(f"Setting {self.key} not allowed by schema") + + logger.debug("validate %s via settings validators", self.key) + # TODO: Create a settings copy, build a Validation from schema, validate it. + + @property + def base_key(self): + """Return the base key 'FOO__BAR' -> 'FOO'""" + return self.key.split("__")[0].upper() + + @property + def display_value(self): + return f"{self.value[:3]}***" if self.is_secret else self.value + + @classmethod + def get_value_from_db(cls, key): + return cls.get_setting_from_db(key).value + + @classmethod + def get_setting_from_db(cls, key): + return cls.objects.filter(key=key).latest("version") + + @classmethod + def get_all(cls): + return ( + cls.objects.annotate(key_lower=Lower("key")) + .order_by("key_lower", "-version") + .distinct("key_lower") + ) + + @classmethod + def as_dict(cls): + return {s.key: s.value for s in cls.get_all()} + + @classmethod + @transaction.atomic + def set_value_in_db(cls, key, value, user=None, is_secret=False): + return cls.objects.create(key=key, value=value, user=user, is_secret=is_secret) + + @classmethod + def set_secret_in_db(cls, key, value, user=None): + return cls.set_value_in_db(key, value, user, is_secret=True) + + @classmethod + @transaction.atomic + def delete_latest_version(cls, key): + return cls.objects.filter(key=key).latest("version").delete() + + @classmethod + @transaction.atomic + def delete_all_versions(cls, key): + return cls.objects.filter(key=key).delete() + + def __str__(self): + return f"{self.key}={self.display_value!r} [v-{self.version}]" + + class Meta: + permissions = (("edit_setting", "Can edit setting"),) + unique_together = ("key", "version") diff --git a/galaxy_ng/app/settings.py b/galaxy_ng/app/settings.py index 61f8985f41..323233428c 100644 --- a/galaxy_ng/app/settings.py +++ b/galaxy_ng/app/settings.py @@ -290,3 +290,7 @@ GALAXY_METRICS_COLLECTION_REDHAT_PASSWORD = None # RH account's org id (required for x-rh-identity auth type) GALAXY_METRICS_COLLECTION_ORG_ID = None + +# When set to True will enable the DYNAMIC settungs feature +# Individual allowed dynamic keys are set on ./dynamic_settings.py +GALAXY_DYNAMIC_SETTINGS = False diff --git a/galaxy_ng/app/tasks/settings_cache.py b/galaxy_ng/app/tasks/settings_cache.py new file mode 100644 index 0000000000..40867354e4 --- /dev/null +++ b/galaxy_ng/app/tasks/settings_cache.py @@ -0,0 +1,138 @@ +""" +Tasks related to the settings cache management. +""" +import logging +from functools import wraps +from typing import Any, Callable, Dict, Optional +from uuid import uuid4 + +from django.conf import settings +from redis import ConnectionError, Redis + +from galaxy_ng.app.models.config import Setting +from django.db.utils import OperationalError + +_conn = None + + +def get_redis_connection(): + global _conn + if _conn is None: + if redis_url := settings.get("REDIS_URL"): + _conn = Redis.from_url(redis_url, decode_responses=True) + else: + _conn = Redis( + host=settings.get("REDIS_HOST", "localhost"), + port=settings.get("REDIS_PORT", 6379), + db=settings.get("REDIS_DB", 0), + password=settings.get("REDIS_PASSWORD"), + ssl=settings.get("REDIS_SSL", False), + ssl_ca_certs=settings.get("REDIS_SSL_CA_CERTS"), + decode_responses=True, + ) + return _conn + + +conn: Optional[Redis] = get_redis_connection() +logger = logging.getLogger(__name__) +logger.setLevel(logging.DEBUG) +CACHE_KEY = "GALAXY_SETTINGS_DATA" + + +def connection_error_wrapper( + func: Optional[Callable] = None, default: Callable = lambda: 0 +) -> Callable: + """A decorator that enables sync functions which use Redis to swallow connection errors.""" + + def dispatch(func, *args, **kwargs): + """Handle connection errors, specific to the sync context, raised by the Redis client.""" + if conn is None: + logger.debug(f"{func.__name__} skipped, no Redis connection available") + return default() + try: + return func(*args, **kwargs) + except (ConnectionError, TypeError) as e: + # TypeError is raised when an invalid port number for the Redis connection is configured + logging.error(f"Redis connection error: {e}") + return default() + + if func: + + @wraps(func) + def simple_wrapper(*args, **kwargs): + """This is for decorator used without parenthesis""" + return dispatch(func, *args, **kwargs) + + return simple_wrapper + + def wrapper(func): + """This is for decorator used with parenthesis""" + + @wraps(func) + def wrapped(*args, **kwargs): + return dispatch(func, *args, **kwargs) + + return wrapped + + return wrapper + + +@connection_error_wrapper +def acquire_lock(lock_name: str, lock_timeout: int = 20): + """Acquire a lock using Redis connection""" + LOCK_KEY = f"GALAXY_SETTINGS_LOCK_{lock_name}" + if conn is None: + logger.debug("Lock acquisition skipped, no Redis connection available") + return "no-lock" # no Redis connection, assume lock is acquired + + token = str(uuid4()) + lock = conn.set(LOCK_KEY, token, nx=True, ex=lock_timeout) + return token if lock else False + + +@connection_error_wrapper +def release_lock(lock_name: str, token: str): + """Release a lock using Redis connection""" + LOCK_KEY = f"GALAXY_SETTINGS_LOCK_{lock_name}" + if conn is None: + logger.debug("Lock release skipped, no Redis connection available") + return + lock = conn.get(LOCK_KEY) + if lock == token: + conn.delete(LOCK_KEY) + + +@connection_error_wrapper +def update_setting_cache(data: Dict[str, Any]) -> int: + """Takes a python dictionary and write to Redis + as a hashmap using Redis connection""" + if conn is None: + logger.debug("Settings cache update skipped, no Redis connection available") + return 0 + + conn.delete(CACHE_KEY) + updated = 0 + if data: + updated = conn.hset(CACHE_KEY, mapping=data) + conn.expire(CACHE_KEY, settings.get("GALAXY_SETTINGS_EXPIRE", 60 * 60 * 24)) + return updated + + +@connection_error_wrapper(default=dict) +def get_settings_from_cache() -> Dict[str, Any]: + """Reads settings from Redis cache and returns a python dictionary""" + if conn is None: + logger.debug("Settings cache read skipped, no Redis connection available") + return {} + + return conn.hgetall(CACHE_KEY) + + +def get_settings_from_db(): + """Returns the data in the Setting table.""" + try: + data = Setting.as_dict() + return data + except OperationalError as exc: + logger.error("Could not read settings from database: %s", str(exc)) + return {} diff --git a/galaxy_ng/tests/unit/test_models.py b/galaxy_ng/tests/unit/test_models.py index 8306b13577..f064b9533f 100644 --- a/galaxy_ng/tests/unit/test_models.py +++ b/galaxy_ng/tests/unit/test_models.py @@ -1,7 +1,11 @@ from django.test import TestCase from pulp_ansible.app.models import AnsibleRepository, Collection -from galaxy_ng.app.models import Namespace +from galaxy_ng.app.models import Namespace, Setting +from galaxy_ng.app.dynamic_settings import DYNAMIC_SETTINGS_SCHEMA + +DYNAMIC_SETTINGS_SCHEMA["TEST"] = {} +DYNAMIC_SETTINGS_SCHEMA["FOO"] = {} class TestSignalCreateRepository(TestCase): @@ -57,3 +61,75 @@ def test_existing_namespace_not_changed(self): ) namespace = Namespace.objects.get(name=self.namespace_name) self.assertEquals(namespace.description, description) + + +class TestSetting(TestCase): + def test_create_setting_directly(self): + Setting.objects.delete() + Setting.objects.create(key='test', value='value') + + # Lowercase read + setting = Setting.get_setting_from_db('test') + self.assertEqual(setting.key, 'test') + self.assertEqual(setting.value, 'value') + + # Uppercase read + setting = Setting.get_setting_from_db('TEST') + self.assertEqual(setting.key, 'TEST') + self.assertEqual(setting.value, 'value') + + # Bump the version + first_version = setting.version + Setting.objects.create(key='test', value='value2') + setting = Setting.objects.get_setting_from_db('test') + self.assertEqual(setting.key, 'test') + self.assertEqual(setting.value, 'value2') + assert setting.version > first_version + + def test_only_latest_10_old_versions_are_kept(self): + Setting.objects.delete() + for i in range(20): + Setting.objects.create(key='test', value=f'value{i}') + + assert Setting.objects.filter(key='test').count() == 11 + + def test_get_settings_as_dict(self): + Setting.objects.delete() + Setting.set_value_in_db("FOO", "BAR") + Setting.set_value_in_db("TEST", 1) + assert Setting.as_dict() == {"FOO": "BAR", "TEST": 1} + + def test_get_settings_all(self): + Setting.objects.delete() + Setting.set_value_in_db("FOO", "BAR") + Setting.set_value_in_db("FOO", "BAR2") + Setting.set_value_in_db("TEST", 1) + assert len(Setting.get_all()) == 3 + + def test_get_setting_icase(self): + Setting.objects.delete() + Setting.set_value_in_db("FOO", "BAR") + assert Setting.get_value_from_db("foo") == "BAR" + assert Setting.get_value_from_db("FOO") == "BAR" + + def test_setting_bool_casing_fix(self): + Setting.objects.delete() + Setting.set_value_in_db("FOO", "True") + assert Setting.get_value_from_db("foo") == "true" + Setting.set_value_in_db("FOO", "False") + assert Setting.get_value_from_db("FOO") == "false" + + def test_display_secret(self): + Setting.objects.delete() + Setting.set_secret_in_db("FOO", "SECRETDATA123") + assert Setting.get_value_from_db("FOO") == "SECRETDATA123" + assert Setting.get_setting_from_db("FOO").display_value == "SEC***" + + def test_delete_all_setting_versions(self): + Setting.objects.delete() + Setting.set_value_in_db("FOO", "BAR") + Setting.set_value_in_db("FOO", "BAR2") + Setting.delete_latest_version("FOO") + assert Setting.get_value_from_db("FOO") == "BAR" + Setting.delete_all_versions("FOO") + assert Setting.objects.filter(key="FOO").count() == 0 From d0e6d7bf3ed583db1ae4f32156d18ac08d93dcf6 Mon Sep 17 00:00:00 2001 From: Bruno Rocha Date: Thu, 10 Aug 2023 22:03:40 +0100 Subject: [PATCH 2/8] Fix settings object inside the hook function Issue: AAH-2009 --- galaxy_ng/app/dynaconf_hooks.py | 37 ++++++++++++++++------------- galaxy_ng/tests/unit/test_models.py | 16 ++++++------- 2 files changed, 29 insertions(+), 24 deletions(-) diff --git a/galaxy_ng/app/dynaconf_hooks.py b/galaxy_ng/app/dynaconf_hooks.py index 89754cc496..cb12e2dd31 100755 --- a/galaxy_ng/app/dynaconf_hooks.py +++ b/galaxy_ng/app/dynaconf_hooks.py @@ -612,7 +612,7 @@ def configure_dynamic_settings(settings: Dynaconf) -> Dict[str, Any]: from dynaconf import DynaconfFormatError, DynaconfParseError from dynaconf.loaders.base import SourceMetadata from dynaconf.hooking import Hook, Action, HookValue - from dynaconf.base import RESERVED_ATTRS, UPPER_DEFAULT_SETTINGS, Settings + from dynaconf.base import Settings except ImportError as exc: # Graceful degradation for dynaconf < 3.3 where method hooking is not available logger.error( @@ -622,31 +622,33 @@ def configure_dynamic_settings(settings: Dynaconf) -> Dict[str, Any]: return {} logger.info("Enabling Dynamic Settings Feature") - def read_settings_from_cache_or_db(settings_dict:Dict, value: HookValue, key: str, *args, **kwargs) -> Any: + + def read_settings_from_cache_or_db( + temp_settings: Settings, + value: HookValue, + key: str, + *args, + **kwargs + ) -> Any: """A function to be attached on Dynaconf Afterget hook. - Load everything from settings cache or db, process parsing and mergings, returns the desired key value + Load everything from settings cache or db, process parsing and mergings, + returns the desired key value """ if not apps.ready or key.upper() not in DYNAMIC_SETTINGS_SCHEMA: - # If app is starting up or key is not on allowed list bypass and just return the unwrapped value + # If app is starting up or key is not on allowed list bypass and just return the value return value.value - # It is required to build a new Settings object to trigger parsing and mergings - temp_settings = Settings() - metadata = SourceMetadata(loader="hooking", identifier="settings_dict", env="main", merged=True) - reserved_keys = set(RESERVED_ATTRS + UPPER_DEFAULT_SETTINGS) - allowed_keys = settings_dict.keys() - reserved_keys - temp_data = {k: v for k, v in settings_dict.items() if k in allowed_keys} - temp_settings.update(temp_data, tomlfy=True, loader_identifier=metadata) - # lazy import because it can't happen before apps are ready from galaxy_ng.app.tasks.settings_cache import get_settings_from_cache, get_settings_from_db - if (data := get_settings_from_cache()): - metadata = SourceMetadata(loader="hooking", identifier="cache", env="main", merged=True) + if data := get_settings_from_cache(): + metadata = SourceMetadata(loader="hooking", identifier="cache") else: data = get_settings_from_db() if data: - metadata = SourceMetadata(loader="hooking", identifier="db", env="main", merged=True) + metadata = SourceMetadata(loader="hooking", identifier="db") + # This is the main part, it will update temp_settigns with data coming from settings db + # and by calling update it will process dynaconf parsing and merging. try: if data: temp_settings.update(data, loader_identifier=metadata, tomlfy=True) @@ -658,7 +660,10 @@ def read_settings_from_cache_or_db(settings_dict:Dict, value: HookValue, key: st elif key in [_k.split("__")[0] for _k in data]: logger.debug("Dynamic setting for key: %s loaded from %s", key, metadata.identifier) else: - logger.debug("Key %s not on db/cache, %s other keys loaded from %s", key, len(data), metadata.identifier) + logger.debug( + "Key %s not on db/cache, %s other keys loaded from %s", + key, len(data), metadata.identifier + ) return temp_settings.get(key, value.value) diff --git a/galaxy_ng/tests/unit/test_models.py b/galaxy_ng/tests/unit/test_models.py index f064b9533f..021a1573f3 100644 --- a/galaxy_ng/tests/unit/test_models.py +++ b/galaxy_ng/tests/unit/test_models.py @@ -65,7 +65,7 @@ def test_existing_namespace_not_changed(self): class TestSetting(TestCase): def test_create_setting_directly(self): - Setting.objects.delete() + Setting.objects.all().delete() Setting.objects.create(key='test', value='value') # Lowercase read @@ -87,46 +87,46 @@ def test_create_setting_directly(self): assert setting.version > first_version def test_only_latest_10_old_versions_are_kept(self): - Setting.objects.delete() + Setting.objects.all().delete() for i in range(20): Setting.objects.create(key='test', value=f'value{i}') assert Setting.objects.filter(key='test').count() == 11 def test_get_settings_as_dict(self): - Setting.objects.delete() + Setting.objects.all().delete() Setting.set_value_in_db("FOO", "BAR") Setting.set_value_in_db("TEST", 1) assert Setting.as_dict() == {"FOO": "BAR", "TEST": 1} def test_get_settings_all(self): - Setting.objects.delete() + Setting.objects.all().delete() Setting.set_value_in_db("FOO", "BAR") Setting.set_value_in_db("FOO", "BAR2") Setting.set_value_in_db("TEST", 1) assert len(Setting.get_all()) == 3 def test_get_setting_icase(self): - Setting.objects.delete() + Setting.objects.all().delete() Setting.set_value_in_db("FOO", "BAR") assert Setting.get_value_from_db("foo") == "BAR" assert Setting.get_value_from_db("FOO") == "BAR" def test_setting_bool_casing_fix(self): - Setting.objects.delete() + Setting.objects.all().delete() Setting.set_value_in_db("FOO", "True") assert Setting.get_value_from_db("foo") == "true" Setting.set_value_in_db("FOO", "False") assert Setting.get_value_from_db("FOO") == "false" def test_display_secret(self): - Setting.objects.delete() + Setting.objects.all().delete() Setting.set_secret_in_db("FOO", "SECRETDATA123") assert Setting.get_value_from_db("FOO") == "SECRETDATA123" assert Setting.get_setting_from_db("FOO").display_value == "SEC***" def test_delete_all_setting_versions(self): - Setting.objects.delete() + Setting.objects.all().delete() Setting.set_value_in_db("FOO", "BAR") Setting.set_value_in_db("FOO", "BAR2") Setting.delete_latest_version("FOO") From 2b34ec25e7af7e14ff79989e94ec9f09693f1dc5 Mon Sep 17 00:00:00 2001 From: Bruno Rocha Date: Thu, 17 Aug 2023 20:39:16 +0100 Subject: [PATCH 3/8] Validate settings before save No-Issue --- galaxy_ng/app/dynamic_settings.py | 84 ++++++++++++++++--------------- galaxy_ng/app/models/config.py | 10 +++- 2 files changed, 53 insertions(+), 41 deletions(-) diff --git a/galaxy_ng/app/dynamic_settings.py b/galaxy_ng/app/dynamic_settings.py index 4a8454de0c..7e75befdaf 100644 --- a/galaxy_ng/app/dynamic_settings.py +++ b/galaxy_ng/app/dynamic_settings.py @@ -1,54 +1,57 @@ -""" -type: -default: -description: -choices: -key_hints: -value_hints: -""" +from dynaconf import Validator + DYNAMIC_SETTINGS_SCHEMA = { - "FOO": { # for testing - "type": str, - "default": "bar", - "description": "Foo bar", - "value_hints": ["bar", "baz"], - }, "GALAXY_REQUIRE_CONTENT_APPROVAL": { - "type": bool, - "default": False, - "description": "Require content approval before it can be published", - "choices": [True, False], + "validator": Validator(is_type_of=bool), + "schema": { + "type": "boolean", + "enum": ["true", "false"], + "default": False, + "description": "Require content approval before it can be published", + } }, "GALAXY_REQUIRE_SIGNATURE_FOR_APPROVAL": { - "type": bool, - "default": False, - "description": "Require signature for content approval", - "choices": [True, False], + "validator": Validator(is_type_of=bool), + "schema": { + "type": "boolean", + "enum": ["true", "false"], + "default": "false", + "description": "Require signature for content approval", + } }, "GALAXY_SIGNATURE_UPLOAD_ENABLED": { - "type": bool, - "default": False, - "description": "Enable signature upload", - "choices": [True, False], + "validator": Validator(is_type_of=bool), + "schema": { + "type": "boolean", + "enum": ["true", "false"], + "default": "false", + "description": "Enable signature upload", + } }, "GALAXY_AUTO_SIGN_COLLECTIONS": { - "type": bool, - "default": False, - "description": "Automatically sign collections during approval/upload", - "choices": [True, False], + "validator": Validator(is_type_of=bool), + "schema": { + "type": "boolean", + "enum": ["true", "false"], + "default": "false", + "description": "Automatically sign collections during approval/upload", + } }, "GALAXY_FEATURE_FLAGS": { - "type": dict, - "default": {}, - "description": "Feature flags for galaxy_ng", - "key_hints": { - "execution_environments": { - "type": bool, - "default": False, - "description": "Enable execution environments", - "choices": [True, False], + "validator": Validator(is_type_of=dict), + "schema": { + "type": "object", + "properties": { + "execution_environments": { + "type": "boolean", + "enum": ["true", "false"], + "default": "false", + "description": "Enable execution environments", + }, }, - } + "default": {}, + "description": "Feature flags for galaxy_ng", + }, }, # For 1816 PR "INSIGHTS_TRACKING_STATE": {}, @@ -57,4 +60,5 @@ "REDHAT_PASSWORD": {}, "AUTOMATION_ANALYTICS_LAST_GATHERED": {}, "AUTOMATION_ANALYTICS_LAST_ENTRIES": {}, + "FOO": {} } diff --git a/galaxy_ng/app/models/config.py b/galaxy_ng/app/models/config.py index 2cb29b955d..f567a2086f 100644 --- a/galaxy_ng/app/models/config.py +++ b/galaxy_ng/app/models/config.py @@ -135,7 +135,15 @@ def _hook_validate(self): raise Exception(f"Setting {self.key} not allowed by schema") logger.debug("validate %s via settings validators", self.key) - # TODO: Create a settings copy, build a Validation from schema, validate it. + validator = DYNAMIC_SETTINGS_SCHEMA[self.base_key].get("validator") + if validator: + validator.names = [self.base_key] + temp_settings = settings.dynaconf_clone() + temp_settings.validators.register(validator) + temp_settings.update(self.as_dict(), tomlfy=True) + temp_settings.set(self.base_key, self.value, tomlfy=True) + temp_settings.validators.validate() + @property def base_key(self): From fe8c4814b0135dc36ec0d7fd3607c61803d07910 Mon Sep 17 00:00:00 2001 From: Bruno Rocha Date: Fri, 18 Aug 2023 17:05:37 +0100 Subject: [PATCH 4/8] Add more tests Issue: AAH-2009 --- galaxy_ng/app/dynamic_settings.py | 3 +- galaxy_ng/app/models/config.py | 102 ++++++++++++++++++++++++---- galaxy_ng/tests/unit/test_models.py | 41 +++++++---- 3 files changed, 119 insertions(+), 27 deletions(-) diff --git a/galaxy_ng/app/dynamic_settings.py b/galaxy_ng/app/dynamic_settings.py index 7e75befdaf..5c63520de9 100644 --- a/galaxy_ng/app/dynamic_settings.py +++ b/galaxy_ng/app/dynamic_settings.py @@ -60,5 +60,6 @@ "REDHAT_PASSWORD": {}, "AUTOMATION_ANALYTICS_LAST_GATHERED": {}, "AUTOMATION_ANALYTICS_LAST_ENTRIES": {}, - "FOO": {} + "FOO": {}, + "BAR": {}, } diff --git a/galaxy_ng/app/models/config.py b/galaxy_ng/app/models/config.py index f567a2086f..04d8be7aa4 100644 --- a/galaxy_ng/app/models/config.py +++ b/galaxy_ng/app/models/config.py @@ -1,6 +1,58 @@ +"""Django Models for Dynamic Settings Managed by Dynaconf. + +Functionality: + - Create a new Setting entry, keeping the latest 10 versions in DB + - A Setting model stores key:value for Dynaconf + - Validators are set on dynamic_settings.py:DYNAMIC_SETTINGS_SCHEMA + - Only keys defined in DYNAMIC_SETTINGS_SCHEMA can be set in DB + - Before saving there is a validation that checks if key:value is valid + - Values are strings parsed by Dynaconf (all dynaconf merging/markers are supported) + - The value must be accessed through Dynaconf or Setting.get method + - Creating or deleting will update the Redis Cache when redis is available + +Setting a new value in the database: + # Only keys defined in DYNAMIC_SETTINGS_SCHEMA can be set in DB + + Setting.set_value_in_db("FOO", "bar") # accepts optional `user` + Setting.set_value_in_db("MY_PATH", "@format {this.BASE_PATH}/foo/bar") + Setting.set_value_in_db("MY_LIST", "@merge [1, 2, 3]") + Setting.set_value_in_db("DATA__key__nested", "thing") + Setting.set_secret_in_db("TOKEN", "123456") + # Setting a secret only mark it as a secret, making it read only on the API. + +Reading a RAW value directly from database: (not recommended) + # This will not be parsed by Dynaconf + + Setting.get_setting_from_db("FOO") -> + Setting.get_value_from_db("MY_PATH") -> "@format {this.BASE_PATH}/foo/bar" + Setting.get_all() -> [, ] + Setting.as_dict() -> {"FOO": "bar", "MY_PATH": "@format {this.BASE_PATH}/foo/bar"} + +Reading a value parsed by Dynaconf: + + Setting.get("PATH") -> "/base_path/to/foo/bar") + + # The above is the equivalent of reading the value directly from dynaconf + # via dango.conf.settings, however this works only when the system has + # GALAXY_DYNAMIC_SETTINGS enabled. + from django.conf import settings + settings.PATH -> "/base_path/to/foo/bar" # this triggers the lookup on database/cache + +Updating Cache: + + # When Redis connection is available the cache will be created/updated + # on every call to methods that writes to database. + # However if you want to force a cache update you can call: + + Setting.update_cache() + +""" + import logging import time +from dynaconf import Validator +from dynaconf.utils import upperfy from django.conf import settings from django.core.validators import RegexValidator from django.db import models, transaction @@ -22,16 +74,20 @@ "alphanumeric, no spaces, no hyphen, only underscore cant start with a number.", ) +MAX_VERSIONS_TO_KEEP = 10 +empty = object() __all__ = ["Setting"] class SettingsManager(models.Manager): def create(self, key, value, *args, **kwargs): - """Creates a new Setting entry, keeping the latest 10 versions in DB + """Creates a new Setting entry, keeping the latest versions in DB uses a lock to ensure no version bump collisions. If Redis is not available, skip locking and just create the new entry. """ + key = upperfy(key) # Key first level must be uppercase + from galaxy_ng.app.tasks.settings_cache import acquire_lock, release_lock # noqa for __ in range(10): # try 10 times @@ -52,7 +108,10 @@ def create(self, key, value, *args, **kwargs): release_lock(key, lock) # Delete old versions - self.get_queryset().filter(key=key, version__lt=new.version - 10).delete() + self.get_queryset().filter( + key=key, + version__lt=new.version - MAX_VERSIONS_TO_KEEP + ).delete() return new @@ -64,7 +123,8 @@ def filter(self, *args, **kwargs): class Setting(LifecycleModelMixin, models.Model): - """Setting model stores key:value for Dynaconf + """Setting model stores key:value for Dynaconf. + The recommended usage is via custom methods. Setting.set_value_in_db('FOO__BAR', 'baz') @@ -135,15 +195,27 @@ def _hook_validate(self): raise Exception(f"Setting {self.key} not allowed by schema") logger.debug("validate %s via settings validators", self.key) - validator = DYNAMIC_SETTINGS_SCHEMA[self.base_key].get("validator") - if validator: - validator.names = [self.base_key] - temp_settings = settings.dynaconf_clone() - temp_settings.validators.register(validator) - temp_settings.update(self.as_dict(), tomlfy=True) - temp_settings.set(self.base_key, self.value, tomlfy=True) - temp_settings.validators.validate() + current_db_data = self.as_dict() + validator = DYNAMIC_SETTINGS_SCHEMA[self.base_key].get("validator", Validator()) + validator.names = [self.base_key] + temp_settings = settings.dynaconf_clone() + temp_settings.validators.register(validator) + temp_settings.update(current_db_data, tomlfy=True) + temp_settings.set(self.base_key, self.value, tomlfy=True) + temp_settings.validators.validate() + @classmethod + def get(cls, key, default=empty): + """Get a setting value directly from database. + but parsing though Dynaconf before returning. + """ + current_db_data = cls.as_dict() + temp_settings = settings.dynaconf_clone() + temp_settings.update(current_db_data, tomlfy=True) + value = temp_settings.get(key, default) + if value is empty: + raise KeyError(f"Setting {key} not found in database") + return value @property def base_key(self): @@ -186,12 +258,16 @@ def set_secret_in_db(cls, key, value, user=None): @classmethod @transaction.atomic def delete_latest_version(cls, key): - return cls.objects.filter(key=key).latest("version").delete() + result = cls.objects.filter(key=key).latest("version").delete() + cls.update_cache() + return result @classmethod @transaction.atomic def delete_all_versions(cls, key): - return cls.objects.filter(key=key).delete() + result = cls.objects.filter(key=key).delete() + cls.update_cache() + return result def __str__(self): return f"{self.key}={self.display_value!r} [v-{self.version}]" diff --git a/galaxy_ng/tests/unit/test_models.py b/galaxy_ng/tests/unit/test_models.py index 021a1573f3..3a9f618304 100644 --- a/galaxy_ng/tests/unit/test_models.py +++ b/galaxy_ng/tests/unit/test_models.py @@ -3,6 +3,7 @@ from galaxy_ng.app.models import Namespace, Setting from galaxy_ng.app.dynamic_settings import DYNAMIC_SETTINGS_SCHEMA +from galaxy_ng.app.models.config import MAX_VERSIONS_TO_KEEP DYNAMIC_SETTINGS_SCHEMA["TEST"] = {} DYNAMIC_SETTINGS_SCHEMA["FOO"] = {} @@ -64,13 +65,16 @@ def test_existing_namespace_not_changed(self): class TestSetting(TestCase): - def test_create_setting_directly(self): + def setUp(self): + """Ensure Table is clean before each case""" Setting.objects.all().delete() + + def test_create_setting_directly(self): Setting.objects.create(key='test', value='value') # Lowercase read setting = Setting.get_setting_from_db('test') - self.assertEqual(setting.key, 'test') + self.assertEqual(setting.key, 'TEST') self.assertEqual(setting.value, 'value') # Uppercase read @@ -82,54 +86,65 @@ def test_create_setting_directly(self): first_version = setting.version Setting.objects.create(key='test', value='value2') setting = Setting.objects.get_setting_from_db('test') - self.assertEqual(setting.key, 'test') + self.assertEqual(setting.key, 'TEST') self.assertEqual(setting.value, 'value2') assert setting.version > first_version - def test_only_latest_10_old_versions_are_kept(self): - Setting.objects.all().delete() - for i in range(20): + def test_only_latest_x_old_versions_are_kept(self): + for i in range(MAX_VERSIONS_TO_KEEP * 2): Setting.objects.create(key='test', value=f'value{i}') - assert Setting.objects.filter(key='test').count() == 11 + assert Setting.objects.filter(key='test').count() == MAX_VERSIONS_TO_KEEP + 1 def test_get_settings_as_dict(self): - Setting.objects.all().delete() Setting.set_value_in_db("FOO", "BAR") Setting.set_value_in_db("TEST", 1) assert Setting.as_dict() == {"FOO": "BAR", "TEST": 1} def test_get_settings_all(self): - Setting.objects.all().delete() Setting.set_value_in_db("FOO", "BAR") Setting.set_value_in_db("FOO", "BAR2") Setting.set_value_in_db("TEST", 1) assert len(Setting.get_all()) == 3 def test_get_setting_icase(self): - Setting.objects.all().delete() Setting.set_value_in_db("FOO", "BAR") assert Setting.get_value_from_db("foo") == "BAR" assert Setting.get_value_from_db("FOO") == "BAR" def test_setting_bool_casing_fix(self): - Setting.objects.all().delete() Setting.set_value_in_db("FOO", "True") assert Setting.get_value_from_db("foo") == "true" Setting.set_value_in_db("FOO", "False") assert Setting.get_value_from_db("FOO") == "false" def test_display_secret(self): - Setting.objects.all().delete() Setting.set_secret_in_db("FOO", "SECRETDATA123") assert Setting.get_value_from_db("FOO") == "SECRETDATA123" assert Setting.get_setting_from_db("FOO").display_value == "SEC***" def test_delete_all_setting_versions(self): - Setting.objects.all().delete() Setting.set_value_in_db("FOO", "BAR") Setting.set_value_in_db("FOO", "BAR2") Setting.delete_latest_version("FOO") assert Setting.get_value_from_db("FOO") == "BAR" Setting.delete_all_versions("FOO") assert Setting.objects.filter(key="FOO").count() == 0 + + def test_dynaconf_parsing(self): + Setting.set_value_in_db("FOO", "BAR") + Setting.set_value_in_db("TEST", "@format {this.FOO}/TEST") + assert Setting.get("TEST") == "BAR/TEST" + + Setting.set_value_in_db("FOO", "@bool 0") + Setting.set_value_in_db("TEST", "@int 42") + assert Setting.get("TEST") == 42 + assert Setting.get("FOO") is False + + Setting.set_value_in_db("FOO__key1", "BAR") + Setting.set_value_in_db("FOO__key2", "BAR2") + assert Setting.get("FOO") == {"key1": "BAR", "key2": "BAR2"} + + Setting.set_value_in_db("FOO", '@json {"colors": ["red"]}') + Setting.set_value_in_db("FOO__colors", "@merge green,blue") + assert Setting.get("FOO") == {"colors": ["red", "green", "blue"]} From efda0bc1406d42bc7dc86f423d16ba4823e22867 Mon Sep 17 00:00:00 2001 From: Bruno Rocha Date: Fri, 18 Aug 2023 18:24:44 +0100 Subject: [PATCH 5/8] Add management commands and fix settings Issue: AAH-2009 --- .../management/commands/galaxy-settings.py | 134 ++++++++++++++++++ galaxy_ng/tests/unit/test_models.py | 9 +- 2 files changed, 140 insertions(+), 3 deletions(-) create mode 100644 galaxy_ng/app/management/commands/galaxy-settings.py diff --git a/galaxy_ng/app/management/commands/galaxy-settings.py b/galaxy_ng/app/management/commands/galaxy-settings.py new file mode 100644 index 0000000000..c4b8b9ff2e --- /dev/null +++ b/galaxy_ng/app/management/commands/galaxy-settings.py @@ -0,0 +1,134 @@ +from contextlib import suppress +from django.core.management.base import BaseCommand +from galaxy_ng.app.models.config import Setting +from django.conf import settings +from dynaconf.utils import upperfy +from galaxy_ng.app.dynamic_settings import DYNAMIC_SETTINGS_SCHEMA + + +class Command(BaseCommand): + """This command sets, read, delete Galaxy Setting. + + Examples: + django-admin galaxy-settings set --key=foo --value=bar + django-admin galaxy-settings set --key=foo --value=bar --is-secret + + django-admin galaxy-settings get --key=foo + django-admin galaxy-settings get --key=foo --parsed + + django-admin galaxy-settings delete --key=foo --all-versions + django-admin galaxy-settings delete --all + + django-admin galaxy-settings list + django-admin galaxy-settings list --parsed + + django-admin galaxy-settings inspect --key=foo + + django-admin galaxy-settings update_cache + django-admin galaxy-settings clean_cache + + django-admin galaxy-settings allowed_keys + """ + + def add_arguments(self, parser): + subparsers = parser.add_subparsers(dest='subcommand', required=True) + + # Subcommand: set + set_parser = subparsers.add_parser('set', help='Set a Galaxy setting') + set_parser.add_argument('--key', required=True, help='Setting key') + set_parser.add_argument('--value', required=True, help='Setting value') + set_parser.add_argument('--is-secret', action='store_true', help='Mark as secret') + + # Subcommand: get + get_parser = subparsers.add_parser('get', help='Get a Galaxy setting') + get_parser.add_argument('--key', required=True, help='Setting key') + get_parser.add_argument('--parsed', action='store_true', help='Parse value using Dynaconf') + get_parser.add_argument('--default', help='Default value') + + # Subcommand: delete + delete_parser = subparsers.add_parser('delete', help='Delete a Galaxy setting') + delete_parser.add_argument('--key', help='Setting key') + delete_parser.add_argument( + '--all-versions', action='store_true', help='Delete all versions' + ) + delete_parser.add_argument('--all', action='store_true', help='Delete all settings') + + # Subcommand: list + list_parser = subparsers.add_parser('list', help='List Galaxy settings') + list_parser.add_argument('--parsed', action='store_true', help='Parse values with Dynaconf') + + # Subcommand: inspect + inspect_parser = subparsers.add_parser('inspect', help='Inspect a Galaxy setting') + inspect_parser.add_argument('--key', required=True, help='Setting key') + + # Subcommand: update_cache + subparsers.add_parser('update_cache', help='Update settings cache') + + # Subcommand: clean_cache + subparsers.add_parser('clean_cache', help='Clean settings cache') + + # Subcommand: allowed_keys + subparsers.add_parser('allowed_keys', help='List allowed settings keys') + + def echo(self, message): + self.stdout.write(self.style.SUCCESS(message)) + + def handle(self, *args, **options): + subcommand = options['subcommand'] + result = getattr(self, f'handle_{subcommand}')(*args, **options) + self.echo(result) + + def handle_set(self, *args, **options): + key = options['key'] + value = options['value'] + is_secret = options['is_secret'] + return Setting.set_value_in_db(key, value, is_secret=is_secret) + + def handle_get(self, *args, **options): + key = options['key'] + parsed = options['parsed'] + default = options['default'] + if parsed: + return Setting.get(upperfy(key), default=default) + try: + return Setting.get_value_from_db(upperfy(key)) + except Setting.DoesNotExist: + return default + + def handle_delete(self, *args, **options): + key = options['key'] + all_versions = options['all_versions'] + all_settings = options['all'] + if all_settings: + result = Setting.objects.all().delete() + Setting.update_cache() + return result + + with suppress(Setting.DoesNotExist): + if key and all_versions: + return Setting.delete_all_versions(upperfy(key)) + if key: + return Setting.delete_latest_version(upperfy(key)) + + return "Nothing to delete" + + def handle_list(self, *args, **options): + parsed = options['parsed'] + data = Setting.as_dict() + if parsed: + return {k: Setting.get(k) for k in data} + return data + + def handle_inspect(self, *args, **options): + key = options['key'] + from dynaconf.utils.inspect import get_history + return get_history(settings, key) + + def handle_update_cache(self, *args, **options): + return Setting.update_cache() + + def handle_clean_cache(self, *args, **options): + return Setting.clean_cache() + + def handle_allowed_keys(self, *args, **options): + return DYNAMIC_SETTINGS_SCHEMA.keys() diff --git a/galaxy_ng/tests/unit/test_models.py b/galaxy_ng/tests/unit/test_models.py index 3a9f618304..7e61937f0b 100644 --- a/galaxy_ng/tests/unit/test_models.py +++ b/galaxy_ng/tests/unit/test_models.py @@ -1,6 +1,7 @@ from django.test import TestCase from pulp_ansible.app.models import AnsibleRepository, Collection +from django.conf import settings from galaxy_ng.app.models import Namespace, Setting from galaxy_ng.app.dynamic_settings import DYNAMIC_SETTINGS_SCHEMA from galaxy_ng.app.models.config import MAX_VERSIONS_TO_KEEP @@ -85,7 +86,7 @@ def test_create_setting_directly(self): # Bump the version first_version = setting.version Setting.objects.create(key='test', value='value2') - setting = Setting.objects.get_setting_from_db('test') + setting = Setting.get_setting_from_db('test') self.assertEqual(setting.key, 'TEST') self.assertEqual(setting.value, 'value2') assert setting.version > first_version @@ -99,13 +100,14 @@ def test_only_latest_x_old_versions_are_kept(self): def test_get_settings_as_dict(self): Setting.set_value_in_db("FOO", "BAR") Setting.set_value_in_db("TEST", 1) - assert Setting.as_dict() == {"FOO": "BAR", "TEST": 1} + assert Setting.as_dict() == {"FOO": "BAR", "TEST": "1"} def test_get_settings_all(self): Setting.set_value_in_db("FOO", "BAR") Setting.set_value_in_db("FOO", "BAR2") Setting.set_value_in_db("TEST", 1) - assert len(Setting.get_all()) == 3 + assert len(Setting.get_all()) == 2 + assert Setting.objects.all().count() == 3 def test_get_setting_icase(self): Setting.set_value_in_db("FOO", "BAR") @@ -133,6 +135,7 @@ def test_delete_all_setting_versions(self): def test_dynaconf_parsing(self): Setting.set_value_in_db("FOO", "BAR") + settings.set("FOO", "BAR") Setting.set_value_in_db("TEST", "@format {this.FOO}/TEST") assert Setting.get("TEST") == "BAR/TEST" From 2caa2f53e658cbe577af48703a39c322327f0e47 Mon Sep 17 00:00:00 2001 From: Bruno Rocha Date: Fri, 18 Aug 2023 19:07:26 +0100 Subject: [PATCH 6/8] Fix test Issue: AAH-2009 --- galaxy_ng/app/dynaconf_hooks.py | 6 +++--- galaxy_ng/tests/unit/test_models.py | 6 +++++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/galaxy_ng/app/dynaconf_hooks.py b/galaxy_ng/app/dynaconf_hooks.py index cb12e2dd31..a7b981ee2a 100755 --- a/galaxy_ng/app/dynaconf_hooks.py +++ b/galaxy_ng/app/dynaconf_hooks.py @@ -594,7 +594,7 @@ def validate(settings: Dynaconf) -> None: def configure_dynamic_settings(settings: Dynaconf) -> Dict[str, Any]: - """Dynaconf 3.3 allows registration of hooks on methods `get` and `as_dict` + """Dynaconf 3.2.2 allows registration of hooks on methods `get` and `as_dict` For galaxy this enables the Dynamic Settings feature, which triggers a specified function after every key is accessed. @@ -614,9 +614,9 @@ def configure_dynamic_settings(settings: Dynaconf) -> Dict[str, Any]: from dynaconf.hooking import Hook, Action, HookValue from dynaconf.base import Settings except ImportError as exc: - # Graceful degradation for dynaconf < 3.3 where method hooking is not available + # Graceful degradation for dynaconf < 3.2.2 where method hooking is not available logger.error( - "Dynaconf method hooking requires Dynaconf >=3.3: %s", + "Dynaconf method hooking requires Dynaconf >=3.2.2: %s", str(exc) ) return {} diff --git a/galaxy_ng/tests/unit/test_models.py b/galaxy_ng/tests/unit/test_models.py index 7e61937f0b..6fa3d7d7f4 100644 --- a/galaxy_ng/tests/unit/test_models.py +++ b/galaxy_ng/tests/unit/test_models.py @@ -150,4 +150,8 @@ def test_dynaconf_parsing(self): Setting.set_value_in_db("FOO", '@json {"colors": ["red"]}') Setting.set_value_in_db("FOO__colors", "@merge green,blue") - assert Setting.get("FOO") == {"colors": ["red", "green", "blue"]} + assert Setting.get("FOO") == { + "colors": ["red", "green", "blue"], + "key1": "BAR", + "key2": "BAR2", + } From bfc6d05de1f9bcb0bfb874868817c6ee11ad6409 Mon Sep 17 00:00:00 2001 From: Bruno Rocha Date: Wed, 20 Sep 2023 15:56:28 +0100 Subject: [PATCH 7/8] Validator must get by key not base_key No-Issue --- galaxy_ng/app/dynaconf_hooks.py | 9 +++--- .../management/commands/galaxy-settings.py | 30 +++++++++---------- galaxy_ng/app/models/config.py | 2 +- galaxy_ng/app/tasks/settings_cache.py | 20 +++++++------ 4 files changed, 32 insertions(+), 29 deletions(-) diff --git a/galaxy_ng/app/dynaconf_hooks.py b/galaxy_ng/app/dynaconf_hooks.py index a7b981ee2a..5e4ee561b9 100755 --- a/galaxy_ng/app/dynaconf_hooks.py +++ b/galaxy_ng/app/dynaconf_hooks.py @@ -609,14 +609,15 @@ def configure_dynamic_settings(settings: Dynaconf) -> Dict[str, Any]: # Perform lazy imports here to avoid breaking when system runs with older # dynaconf versions try: + from dynaconf.hooking import Hook, Action, HookValue from dynaconf import DynaconfFormatError, DynaconfParseError from dynaconf.loaders.base import SourceMetadata - from dynaconf.hooking import Hook, Action, HookValue from dynaconf.base import Settings except ImportError as exc: - # Graceful degradation for dynaconf < 3.2.2 where method hooking is not available + # Graceful degradation for dynaconf < 3.2.3 where method hooking is not available logger.error( - "Dynaconf method hooking requires Dynaconf >=3.2.2: %s", + "Galaxy Dynamic Settings requires Dynaconf >=3.2.3, " + "system will work normally but dynamic settings from database will be ignored: %s", str(exc) ) return {} @@ -647,7 +648,7 @@ def read_settings_from_cache_or_db( if data: metadata = SourceMetadata(loader="hooking", identifier="db") - # This is the main part, it will update temp_settigns with data coming from settings db + # This is the main part, it will update temp_settings with data coming from settings db # and by calling update it will process dynaconf parsing and merging. try: if data: diff --git a/galaxy_ng/app/management/commands/galaxy-settings.py b/galaxy_ng/app/management/commands/galaxy-settings.py index c4b8b9ff2e..2485823723 100644 --- a/galaxy_ng/app/management/commands/galaxy-settings.py +++ b/galaxy_ng/app/management/commands/galaxy-settings.py @@ -14,13 +14,13 @@ class Command(BaseCommand): django-admin galaxy-settings set --key=foo --value=bar --is-secret django-admin galaxy-settings get --key=foo - django-admin galaxy-settings get --key=foo --parsed + django-admin galaxy-settings get --key=foo --raw django-admin galaxy-settings delete --key=foo --all-versions django-admin galaxy-settings delete --all django-admin galaxy-settings list - django-admin galaxy-settings list --parsed + django-admin galaxy-settings list --raw django-admin galaxy-settings inspect --key=foo @@ -42,7 +42,7 @@ def add_arguments(self, parser): # Subcommand: get get_parser = subparsers.add_parser('get', help='Get a Galaxy setting') get_parser.add_argument('--key', required=True, help='Setting key') - get_parser.add_argument('--parsed', action='store_true', help='Parse value using Dynaconf') + get_parser.add_argument('--raw', action='store_true', help='Raw value from DB') get_parser.add_argument('--default', help='Default value') # Subcommand: delete @@ -55,7 +55,7 @@ def add_arguments(self, parser): # Subcommand: list list_parser = subparsers.add_parser('list', help='List Galaxy settings') - list_parser.add_argument('--parsed', action='store_true', help='Parse values with Dynaconf') + list_parser.add_argument('--raw', action='store_true', help='Raw value from DB') # Subcommand: inspect inspect_parser = subparsers.add_parser('inspect', help='Inspect a Galaxy setting') @@ -86,14 +86,14 @@ def handle_set(self, *args, **options): def handle_get(self, *args, **options): key = options['key'] - parsed = options['parsed'] + raw = options['raw'] default = options['default'] - if parsed: - return Setting.get(upperfy(key), default=default) - try: - return Setting.get_value_from_db(upperfy(key)) - except Setting.DoesNotExist: - return default + if raw: + try: + return Setting.get_value_from_db(upperfy(key)) + except Setting.DoesNotExist: + return default + return Setting.get(upperfy(key), default=default) def handle_delete(self, *args, **options): key = options['key'] @@ -113,11 +113,11 @@ def handle_delete(self, *args, **options): return "Nothing to delete" def handle_list(self, *args, **options): - parsed = options['parsed'] + raw = options['raw'] data = Setting.as_dict() - if parsed: - return {k: Setting.get(k) for k in data} - return data + if raw: + return data + return {k: Setting.get(k) for k in data} def handle_inspect(self, *args, **options): key = options['key'] diff --git a/galaxy_ng/app/models/config.py b/galaxy_ng/app/models/config.py index 04d8be7aa4..9f7eb422ab 100644 --- a/galaxy_ng/app/models/config.py +++ b/galaxy_ng/app/models/config.py @@ -201,7 +201,7 @@ def _hook_validate(self): temp_settings = settings.dynaconf_clone() temp_settings.validators.register(validator) temp_settings.update(current_db_data, tomlfy=True) - temp_settings.set(self.base_key, self.value, tomlfy=True) + temp_settings.set(self.key, self.value, tomlfy=True) temp_settings.validators.validate() @classmethod diff --git a/galaxy_ng/app/tasks/settings_cache.py b/galaxy_ng/app/tasks/settings_cache.py index 40867354e4..16571edfda 100644 --- a/galaxy_ng/app/tasks/settings_cache.py +++ b/galaxy_ng/app/tasks/settings_cache.py @@ -17,13 +17,15 @@ def get_redis_connection(): global _conn + redis_host = settings.get("REDIS_HOST") + redis_url = settings.get("REDIS_URL") if _conn is None: - if redis_url := settings.get("REDIS_URL"): + if redis_url is not None: _conn = Redis.from_url(redis_url, decode_responses=True) - else: + elif redis_host is not None: _conn = Redis( - host=settings.get("REDIS_HOST", "localhost"), - port=settings.get("REDIS_PORT", 6379), + host=redis_host, + port=settings.get("REDIS_PORT") or 6379, db=settings.get("REDIS_DB", 0), password=settings.get("REDIS_PASSWORD"), ssl=settings.get("REDIS_SSL", False), @@ -47,7 +49,7 @@ def connection_error_wrapper( def dispatch(func, *args, **kwargs): """Handle connection errors, specific to the sync context, raised by the Redis client.""" if conn is None: - logger.debug(f"{func.__name__} skipped, no Redis connection available") + logger.warning(f"{func.__name__} skipped, no Redis connection available") return default() try: return func(*args, **kwargs) @@ -82,7 +84,7 @@ def acquire_lock(lock_name: str, lock_timeout: int = 20): """Acquire a lock using Redis connection""" LOCK_KEY = f"GALAXY_SETTINGS_LOCK_{lock_name}" if conn is None: - logger.debug("Lock acquisition skipped, no Redis connection available") + logger.warning("Lock acquisition skipped, no Redis connection available") return "no-lock" # no Redis connection, assume lock is acquired token = str(uuid4()) @@ -95,7 +97,7 @@ def release_lock(lock_name: str, token: str): """Release a lock using Redis connection""" LOCK_KEY = f"GALAXY_SETTINGS_LOCK_{lock_name}" if conn is None: - logger.debug("Lock release skipped, no Redis connection available") + logger.warning("Lock release skipped, no Redis connection available") return lock = conn.get(LOCK_KEY) if lock == token: @@ -107,7 +109,7 @@ def update_setting_cache(data: Dict[str, Any]) -> int: """Takes a python dictionary and write to Redis as a hashmap using Redis connection""" if conn is None: - logger.debug("Settings cache update skipped, no Redis connection available") + logger.warning("Settings cache update skipped, no Redis connection available") return 0 conn.delete(CACHE_KEY) @@ -122,7 +124,7 @@ def update_setting_cache(data: Dict[str, Any]) -> int: def get_settings_from_cache() -> Dict[str, Any]: """Reads settings from Redis cache and returns a python dictionary""" if conn is None: - logger.debug("Settings cache read skipped, no Redis connection available") + logger.warning("Settings cache read skipped, no Redis connection available") return {} return conn.hgetall(CACHE_KEY) From 5bad3e317c16651bad256ad57d2da808810ced96 Mon Sep 17 00:00:00 2001 From: Bruno Rocha Date: Thu, 21 Sep 2023 16:35:12 +0100 Subject: [PATCH 8/8] Log will emit only once when Redis is not available No-Issue --- .../app/management/commands/galaxy-settings.py | 2 +- .../{0042_setting.py => 0045_setting.py} | 2 +- galaxy_ng/app/tasks/settings_cache.py | 16 +++++++--------- 3 files changed, 9 insertions(+), 11 deletions(-) rename galaxy_ng/app/migrations/{0042_setting.py => 0045_setting.py} (96%) diff --git a/galaxy_ng/app/management/commands/galaxy-settings.py b/galaxy_ng/app/management/commands/galaxy-settings.py index 2485823723..c6a6384d40 100644 --- a/galaxy_ng/app/management/commands/galaxy-settings.py +++ b/galaxy_ng/app/management/commands/galaxy-settings.py @@ -71,7 +71,7 @@ def add_arguments(self, parser): subparsers.add_parser('allowed_keys', help='List allowed settings keys') def echo(self, message): - self.stdout.write(self.style.SUCCESS(message)) + self.stdout.write(self.style.SUCCESS(str(message))) def handle(self, *args, **options): subcommand = options['subcommand'] diff --git a/galaxy_ng/app/migrations/0042_setting.py b/galaxy_ng/app/migrations/0045_setting.py similarity index 96% rename from galaxy_ng/app/migrations/0042_setting.py rename to galaxy_ng/app/migrations/0045_setting.py index acc79859c5..1eb1b4ba4b 100644 --- a/galaxy_ng/app/migrations/0042_setting.py +++ b/galaxy_ng/app/migrations/0045_setting.py @@ -9,7 +9,7 @@ class Migration(migrations.Migration): dependencies = [ - ("galaxy", "0041_alter_containerregistryremote_remote_ptr"), + ("galaxy", "0044_legacyroleimport"), ] operations = [ diff --git a/galaxy_ng/app/tasks/settings_cache.py b/galaxy_ng/app/tasks/settings_cache.py index 16571edfda..9796f1bbab 100644 --- a/galaxy_ng/app/tasks/settings_cache.py +++ b/galaxy_ng/app/tasks/settings_cache.py @@ -12,7 +12,9 @@ from galaxy_ng.app.models.config import Setting from django.db.utils import OperationalError +logger = logging.getLogger(__name__) _conn = None +CACHE_KEY = "GALAXY_SETTINGS_DATA" def get_redis_connection(): @@ -32,13 +34,14 @@ def get_redis_connection(): ssl_ca_certs=settings.get("REDIS_SSL_CA_CERTS"), decode_responses=True, ) + else: + logger.warning( + "REDIS connection undefined, not caching dynamic settings" + ) return _conn conn: Optional[Redis] = get_redis_connection() -logger = logging.getLogger(__name__) -logger.setLevel(logging.DEBUG) -CACHE_KEY = "GALAXY_SETTINGS_DATA" def connection_error_wrapper( @@ -48,8 +51,7 @@ def connection_error_wrapper( def dispatch(func, *args, **kwargs): """Handle connection errors, specific to the sync context, raised by the Redis client.""" - if conn is None: - logger.warning(f"{func.__name__} skipped, no Redis connection available") + if conn is None: # No redis connection defined return default() try: return func(*args, **kwargs) @@ -84,7 +86,6 @@ def acquire_lock(lock_name: str, lock_timeout: int = 20): """Acquire a lock using Redis connection""" LOCK_KEY = f"GALAXY_SETTINGS_LOCK_{lock_name}" if conn is None: - logger.warning("Lock acquisition skipped, no Redis connection available") return "no-lock" # no Redis connection, assume lock is acquired token = str(uuid4()) @@ -97,7 +98,6 @@ def release_lock(lock_name: str, token: str): """Release a lock using Redis connection""" LOCK_KEY = f"GALAXY_SETTINGS_LOCK_{lock_name}" if conn is None: - logger.warning("Lock release skipped, no Redis connection available") return lock = conn.get(LOCK_KEY) if lock == token: @@ -109,7 +109,6 @@ def update_setting_cache(data: Dict[str, Any]) -> int: """Takes a python dictionary and write to Redis as a hashmap using Redis connection""" if conn is None: - logger.warning("Settings cache update skipped, no Redis connection available") return 0 conn.delete(CACHE_KEY) @@ -124,7 +123,6 @@ def update_setting_cache(data: Dict[str, Any]) -> int: def get_settings_from_cache() -> Dict[str, Any]: """Reads settings from Redis cache and returns a python dictionary""" if conn is None: - logger.warning("Settings cache read skipped, no Redis connection available") return {} return conn.hgetall(CACHE_KEY)