[PULP-659] Remove legacy django storages

.ci/ansible/settings.py.j2

@@ -26,10 +26,22 @@ API_ROOT = {{ api_root | repr }}
 {% endfor %}
 {% endif %}
 
-{% if s3_test | default(false) %}
-MEDIA_ROOT: ""
-S3_USE_SIGV4 = True
-{% if test_storages_compat_layer is defined and test_storages_compat_layer %}
+{# =======================================
+Macros for legacy and new storage settings
+========================================== -#}
+
+{%- macro s3_settings(legacy) -%}
+    {%- if legacy %}
+DEFAULT_FILE_STORAGE = "storages.backends.s3boto3.S3Boto3Storage"
+AWS_ACCESS_KEY_ID = "{{ minio_access_key }}"
+AWS_SECRET_ACCESS_KEY = "{{ minio_secret_key }}"
+AWS_S3_REGION_NAME = "eu-central-1"
+AWS_S3_ADDRESSING_STYLE = "path"
+AWS_S3_SIGNATURE_VERSION = "s3v4"
+AWS_STORAGE_BUCKET_NAME = "pulp3"
+AWS_S3_ENDPOINT_URL = "http://minio:9000"
+AWS_DEFAULT_ACL = "@none None"
+    {%- else %}
 STORAGES = {
     "default": {
         "BACKEND": "storages.backends.s3boto3.S3Boto3Storage",
@@ -48,34 +60,93 @@ STORAGES = {
       "BACKEND": "django.contrib.staticfiles.storage.StaticFilesStorage",
     },
 }
-{% else %}
-DEFAULT_FILE_STORAGE = "storages.backends.s3boto3.S3Boto3Storage"
-AWS_ACCESS_KEY_ID = "{{ minio_access_key }}"
-AWS_SECRET_ACCESS_KEY = "{{ minio_secret_key }}"
-AWS_S3_REGION_NAME = "eu-central-1"
-AWS_S3_ADDRESSING_STYLE = "path"
-AWS_S3_SIGNATURE_VERSION = "s3v4"
-AWS_STORAGE_BUCKET_NAME = "pulp3"
-AWS_S3_ENDPOINT_URL = "http://minio:9000"
-AWS_DEFAULT_ACL = "@none None"
-{% endif %}
-{% endif %}
+    {%- endif %}
+{%- endmacro -%}
 
-{% if azure_test | default(false) %}
+{%- macro azure_settings(legacy) -%}
+    {%- if legacy %}
 DEFAULT_FILE_STORAGE = "storages.backends.azure_storage.AzureStorage"
-MEDIA_ROOT = ""
 AZURE_ACCOUNT_KEY = "Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw=="
 AZURE_ACCOUNT_NAME = "devstoreaccount1"
 AZURE_CONTAINER =  "pulp-test"
 AZURE_LOCATION = "pulp3"
 AZURE_OVERWRITE_FILES = True
 AZURE_URL_EXPIRATION_SECS = 120
 AZURE_CONNECTION_STRING = 'DefaultEndpointsProtocol=http;AccountName=devstoreaccount1;AccountKey=Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==;BlobEndpoint=http://ci-azurite:10000/devstoreaccount1;'
-{% endif %}
+    {%- else %}
+STORAGES = {
+    "default": {
+        "BACKEND": "storages.backends.azure_storage.AzureStorage",
+        "OPTIONS": {
+            "account_key": "Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==",
+            "account_name": "devstoreaccount1",
+            "location": "pulp3",
+            "azure_container": "pulp-test",
+            "overwrite_files": True,
+            "expiration_secs": 120,
+            "connection_string": "DefaultEndpointsProtocol=http;AccountName=devstoreaccount1;AccountKey=Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==;BlobEndpoint=http://ci-azurite:10000/devstoreaccount1;",
+        },
+    },
+    "staticfiles": {
+      "BACKEND": "django.contrib.staticfiles.storage.StaticFilesStorage",
+    },
+}
+    {%- endif %}
+{%- endmacro -%}
 
-{% if gcp_test | default(false) %}
+{%- macro gcp_settings(legacy) -%}
+    {%- if legacy %}
 DEFAULT_FILE_STORAGE = "storages.backends.gcloud.GoogleCloudStorage"
-MEDIA_ROOT = ""
 GS_BUCKET_NAME = "gcppulp"
 GS_CUSTOM_ENDPOINT = "http://ci-gcp:4443"
+    {%- else %}
+STORAGES = {
+    "default": {
+        "BACKEND": "storages.backends.gcloud.GoogleCloudStorage",
+        "OPTIONS": {
+            "bucket_name": "gcppulp",
+            "custom_endpoint": "http://ci-gcp:4443",
+        },
+    },
+    "staticfiles": {
+      "BACKEND": "django.contrib.staticfiles.storage.StaticFilesStorage",
+    },
+}
+    {%- endif %}
+{%- endmacro -%}
+
+{#- ==========================================
+Render according to test_storages_compat_layer
+==============================================
+
+Case 1) test_storages_compat_layer is unset
+- All storages render the legacy setting
+- Branches using pulpcore <3.70 must leave this key unset (use legacy)
+
+Case 2) test_storages_compat_layer is True
+- To tests both work, only one setting uses the new storage setting
+- Branches using pulpcore >=3.70,<3.85 must set this key to True (test both)
+
+Case 3) test_storages_compat_layer is False
+- All storages render the new setting
+- Branches using pulpcore >=3.85 must set this key to False (use new)
+-#}
+
+{% if s3_test | default(false) or azure_test | default(false) or gcp_test | default(false)%}
+MEDIA_ROOT=""
 {% endif %}
+{%- if test_storages_compat_layer is not defined -%}
+    {%- if s3_test | default(false) -%}{{ s3_settings(legacy=True) }}{%- endif -%}
+    {%- if azure_test | default(false) -%}{{ azure_settings(legacy=True) }}{%- endif -%}
+    {%- if gcp_test | default(false) -%}{{ gcp_settings(legacy=True) }}{%- endif -%}
+{%- else -%}
+    {%- if test_storages_compat_layer is true -%}
+        {%- if s3_test | default(false) -%}{{ s3_settings(legacy=False) }}{%- endif -%}
+        {%- if azure_test | default(false) -%}{{ azure_settings(legacy=True) }}{%- endif -%}
+        {%- if gcp_test | default(false) -%}{{ gcp_settings(legacy=True) }}{%- endif -%}
+    {%- elif test_storages_compat_layer is false -%}
+        {%- if s3_test | default(false) -%}{{ s3_settings(legacy=False) }}{%- endif -%}
+        {%- if azure_test | default(false) -%}{{ azure_settings(legacy=False) }}{%- endif -%}
+        {%- if gcp_test | default(false) -%}{{ gcp_settings(legacy=False) }}{%- endif -%}
+    {%- endif -%}
+{%- endif -%}

.github/workflows/scripts/install.sh

@@ -110,7 +110,7 @@ minio_access_key: "'$MINIO_ACCESS_KEY'"\
 minio_secret_key: "'$MINIO_SECRET_KEY'"\
 pulp_scenario_settings: {"DISABLED_authentication_backends": "@merge django.contrib.auth.backends.RemoteUserBackend", "DISABLED_authentication_json_header": "HTTP_X_RH_IDENTITY", "DISABLED_authentication_json_header_jq_filter": ".identity.user.username", "DISABLED_authentication_json_header_openapi_security_scheme": {"description": "External OAuth integration", "flows": {"clientCredentials": {"scopes": {"api.console": "grant_access_to_pulp"}, "tokenUrl": "https://your-identity-provider/token/issuer"}}, "type": "oauth2"}, "DISABLED_rest_framework__default_authentication_classes": "@merge pulpcore.app.authentication.JSONHeaderRemoteAuthentication", "domain_enabled": true, "hide_guarded_distributions": true, "rest_framework__default_permission_classes": ["pulpcore.plugin.access_policy.AccessPolicyFromSettings"], "spectacular_settings__oas_version": "3.0.3"}\
 pulp_scenario_env: {}\
-test_storages_compat_layer: true\
+test_storages_compat_layer: false\
 ' vars/main.yaml
   export PULP_API_ROOT="/rerouted/djnd/"
 fi
@@ -125,6 +125,7 @@ if [ "$TEST" = "azure" ]; then
   sed -i -e '$a azure_test: true\
 pulp_scenario_settings: {"api_root_rewrite_header": "X-API-Root", "content_origin": null, "domain_enabled": true, "rest_framework__default_authentication_classes": "@merge pulpcore.app.authentication.PulpRemoteUserAuthentication", "rest_framework__default_permission_classes": ["pulpcore.plugin.access_policy.DefaultAccessPolicy"], "task_diagnostics": ["memory"]}\
 pulp_scenario_env: {}\
+test_storages_compat_layer: false\
 ' vars/main.yaml
 fi
 

CHANGES/6807.removal

@@ -0,0 +1,3 @@
+Removed support for DEFAULT_FILE_STORAGE and it's corresponding storage options.
+Use STORAGES instead:
+<https://pulpproject.org/pulpcore/docs/admin/reference/settings/#storages>

docs/admin/reference/settings.md

@@ -39,9 +39,14 @@ For instructions on how to configure the database, refer to [Django database set
 
 ### DEFAULT\_FILE\_STORAGE
 
-!!! warning "Deprecated in `3.70`"
-    The `DEFAULT_FILE_STORAGE` setting was deprecated in [django 4.2] and will be removed from pulpcore on `3.85`.
-    Between `3.70` and `3.85`, replace it with [`STORAGES`](#storages).
+!!! warning "Deprecated in `pulpcore 3.70`"
+
+    Learn how to update it to [`STORAGES`](#storages) in the [Storages Upgrade Guide].
+    Note this must be done between `3.70` and `3.85`.
+
+!!! warning "Removed in `pulpcore 3.85`"
+
+[storages upgrade guide]: https://discourse.pulpproject.org/t/action-required-upgrade-your-storage-settings-before-pulpcore-3-85/2072>
 
 ### LOGGING
 

pulpcore/app/apps.py

@@ -5,6 +5,7 @@
 from importlib import import_module
 
 from django import apps
+from django.conf import settings
 from django.core.exceptions import ImproperlyConfigured
 from django.db import connection, transaction
 from django.db.models.signals import post_migrate
@@ -67,15 +68,6 @@ class PulpPluginAppConfig(apps.AppConfig):
 
     def __init__(self, app_name, app_module):
         super().__init__(app_name, app_module)
-        # begin Compatilibity layer for DEFAULT_FILE_STORAGE deprecation
-        # Remove on pulpcore=3.85 or pulpcore=4.0
-        # * Workaround for getting the up-to-date settings instance, otherwise is doesnt
-        #   get the patch in settings.py
-        # * Update code in signal handlers to use module-level imports again
-        from django.conf import settings
-
-        self.settings = settings
-        # end
 
         try:
             self.version
@@ -322,7 +314,6 @@ def _populate_system_id(sender, apps, verbosity, **kwargs):
 
 
 def _ensure_default_domain(sender, **kwargs):
-    settings = sender.settings
     table_names = connection.introspection.table_names()
     if "core_domain" in table_names:
         from pulpcore.app.util import get_default_domain
@@ -402,7 +393,6 @@ def _get_permission(perm):
 
 
 def _populate_artifact_serving_distribution(sender, apps, verbosity, **kwargs):
-    settings = sender.settings
     if (
         settings.STORAGES["default"]["BACKEND"] == "pulpcore.app.models.storage.FileSystem"
         or not settings.REDIRECT_TO_OBJECT_STORAGE

pulpcore/app/settings.py

@@ -17,12 +17,9 @@
 from pathlib import Path
 
 from cryptography.fernet import Fernet
-from django.core.files.storage import storages
-from django.conf import global_settings
 from django.core.exceptions import ImproperlyConfigured
 from django.db import connection
-from pulpcore.app.loggers import deprecation_logger
-from dynaconf import DjangoDynaconf, Dynaconf, Validator, get_history
+from dynaconf import DjangoDynaconf, Dynaconf, Validator
 
 from pulpcore import constants
 
@@ -73,16 +70,7 @@
 STATIC_URL = "/assets/"
 STATIC_ROOT = DEPLOY_ROOT / STATIC_URL.strip("/")
 
-# begin compatibility layer for DEFAULT_FILE_STORAGE
-# Remove on pulpcore=3.85 or pulpcore=4.0
-
-# - What is this?
-# We shouldn't use STORAGES or DEFAULT_FILE_STORAGE directly because those are
-# mutually exclusive by django, which constraints users to use whatever we use.
-# This is a hack/workaround to set Pulp's default while still enabling users to choose
-# the legacy or the new storage setting.
-_DEFAULT_FILE_STORAGE = "pulpcore.app.models.storage.FileSystem"
-_STORAGES = {
+STORAGES = {
     "default": {
         "BACKEND": "pulpcore.app.models.storage.FileSystem",
     },
@@ -91,10 +79,6 @@
     },
 }
 
-setattr(global_settings, "DEFAULT_FILE_STORAGE", _DEFAULT_FILE_STORAGE)
-setattr(global_settings, "STORAGES", _STORAGES)
-# end DEFAULT_FILE_STORAGE deprecation layer
-
 REDIRECT_TO_OBJECT_STORAGE = True
 
 WORKING_DIRECTORY = DEPLOY_ROOT / "tmp"
@@ -430,19 +414,17 @@
 
 # Validators
 
-storage_keys = ("STORAGES.default.BACKEND", "DEFAULT_FILE_STORAGE")
 storage_validator = (
     Validator("REDIRECT_TO_OBJECT_STORAGE", eq=False)
-    | Validator(*storage_keys, eq="pulpcore.app.models.storage.FileSystem")
-    | Validator(*storage_keys, eq="storages.backends.azure_storage.AzureStorage")
-    | Validator(*storage_keys, eq="storages.backends.s3.S3Storage")
-    | Validator(*storage_keys, eq="storages.backends.s3boto3.S3Boto3Storage")
-    | Validator(*storage_keys, eq="storages.backends.gcloud.GoogleCloudStorage")
+    | Validator("STORAGES.default.BACKEND", eq="pulpcore.app.models.storage.FileSystem")
+    | Validator("STORAGES.default.BACKEND", eq="storages.backends.azure_storage.AzureStorage")
+    | Validator("STORAGES.default.BACKEND", eq="storages.backends.s3.S3Storage")
+    | Validator("STORAGES.default.BACKEND", eq="storages.backends.s3boto3.S3Boto3Storage")
+    | Validator("STORAGES.default.BACKEND", eq="storages.backends.gcloud.GoogleCloudStorage")
 )
 storage_validator.messages["combined"] = (
     "'REDIRECT_TO_OBJECT_STORAGE=True' is only supported with the local file, S3, GCP or Azure "
-    "storage backend configured in STORAGES['default']['BACKEND'] "
-    "(deprecated DEFAULT_FILE_STORAGE)."
+    "storage backend configured in STORAGES['default']['BACKEND']."
 )
 
 cache_enabled_validator = Validator("CACHE_ENABLED", eq=True)
@@ -549,21 +531,6 @@ def otel_middleware_hook(settings):
     post_hooks=(otel_middleware_hook,),
 )
 
-# begin compatibility layer for DEFAULT_FILE_STORAGE
-# Remove on pulpcore=3.85 or pulpcore=4.0
-using_deprecated_storage_settings = len(get_history(settings, key="DEFAULT_FILE_STORAGE")) > 1
-if using_deprecated_storage_settings:
-    deprecation_logger.warning(
-        "[deprecation] DEFAULT_FILE_STORAGE will be removed in pulpcore 3.85. "
-        "Learn how to upgrade to STORAGES:\n"
-        "https://discourse.pulpproject.org/t/"
-        "action-required-upgrade-your-storage-settings-before-pulpcore-3-85/2072/2"
-    )
-# Ensures the cached property storage.backends uses the the right value
-storages._backends = settings.STORAGES.copy()
-storages.backends
-# end compatibility layer
-
 _logger = getLogger(__name__)
 
 

pulpcore/pytest_plugin.py

@@ -608,28 +608,28 @@ def _settings_factory(storage_class=None, storage_settings=None):
             "bucket_name",
         ]
         keys["storages.backends.azure_storage.AzureStorage"] = [
-            "AZURE_ACCOUNT_NAME",
-            "AZURE_CONTAINER",
-            "AZURE_ACCOUNT_KEY",
-            "AZURE_URL_EXPIRATION_SECS",
-            "AZURE_OVERWRITE_FILES",
-            "AZURE_LOCATION",
-            "AZURE_CONNECTION_STRING",
+            "account_name",
+            "azure_container",
+            "account_key",
+            "expiration_secs",
+            "overwrite_files",
+            "location",
+            "connection_string",
         ]
-        settings = storage_settings or dict()
-        backend = storage_class or pulp_settings.STORAGES["default"]["BACKEND"]
-        not_defined_settings = (k for k in keys[backend] if k not in settings)
-        # The CI configures s3 with STORAGES and Azure with legacy
-        # Move all to STORAGES structure on DEFAULT_FILE_STORAGE removal
-        if backend == "storages.backends.s3boto3.S3Boto3Storage":
-            storages_dict = getattr(pulp_settings, "STORAGES", {})
-            storage_options = storages_dict.get("default", {}).get("OPTIONS", {})
-            for key in not_defined_settings:
-                settings[key] = storage_options.get(key)
-        else:
-            for key in not_defined_settings:
-                settings[key] = getattr(pulp_settings, key, None)
-        return backend, settings
+
+        def get_installation_storage_option(key, backend):
+            value = pulp_settings.STORAGES["default"]["OPTIONS"].get(key)
+            # Some FileSystem backend options may be defined in the top settings module
+            if backend == "pulpcore.app.models.storage.FileSystem" and not value:
+                value = getattr(pulp_settings, key, None)
+            return value
+
+        storage_settings = storage_settings or dict()
+        storage_backend = storage_class or pulp_settings.STORAGES["default"]["BACKEND"]
+        unset_storage_settings = (k for k in keys[storage_backend] if k not in storage_settings)
+        for key in unset_storage_settings:
+            storage_settings[key] = get_installation_storage_option(key, storage_backend)
+        return storage_backend, storage_settings
 
     return _settings_factory
 

template_config.yml

@@ -112,5 +112,5 @@ test_lowerbounds: true
 test_performance: false
 test_reroute: true
 test_s3: true
-test_storages_compat_layer: true
+test_storages_compat_layer: false
 use_issue_template: true