diff --git a/CHANGES/7228.bugfix b/CHANGES/7228.bugfix new file mode 100644 index 00000000000..56454c2b5fc --- /dev/null +++ b/CHANGES/7228.bugfix @@ -0,0 +1 @@ +Fixed not supporting the new rename of the S3 storage backend. diff --git a/pulpcore/app/serializers/domain.py b/pulpcore/app/serializers/domain.py index 478e976bdc6..1b17ae4ca75 100644 --- a/pulpcore/app/serializers/domain.py +++ b/pulpcore/app/serializers/domain.py @@ -16,7 +16,8 @@ BACKEND_CHOICES = ( ("pulpcore.app.models.storage.FileSystem", "Use local filesystem as storage"), # ("pulpcore.app.models.storage.PulpSFTPStorage", "Use SFTP server as storage"), - ("storages.backends.s3boto3.S3Boto3Storage", "Use Amazon S3 as storage"), + ("storages.backends.s3boto3.S3Boto3Storage", "Use Amazon S3 as storage [deprecated]"), + ("storages.backends.s3.S3Storage", "Use Amazon S3 as storage"), ("storages.backends.azure_storage.AzureStorage", "Use Azure Blob as storage"), # ("storages.backends.gcloud.GoogleCloudStorage", "Use Google Cloud as storage"), ) @@ -115,6 +116,7 @@ class SFTPSettingsSerializer(BaseSettingsClass): class AmazonS3SettingsSerializer(BaseSettingsClass): """A Serializer for Amazon S3 storage settings.""" + STORAGE_CLASS = "storages.backends.s3.S3Storage" SETTING_MAPPING = { "aws_s3_access_key_id": "access_key", "aws_access_key_id": "access_key", @@ -261,6 +263,7 @@ class StorageSettingsSerializer(serializers.Serializer): STORAGE_MAPPING = { "pulpcore.app.models.storage.FileSystem": FileSystemSettingsSerializer, "pulpcore.app.models.storage.PulpSFTPStorage": SFTPSettingsSerializer, + "storages.backends.s3.S3Storage": AmazonS3SettingsSerializer, "storages.backends.s3boto3.S3Boto3Storage": AmazonS3SettingsSerializer, "storages.backends.azure_storage.AzureStorage": AzureSettingsSerializer, "storages.backends.gcloud.GoogleCloudStorage": GoogleSettingsSerializer, diff --git a/pulpcore/app/util.py b/pulpcore/app/util.py index 9ff52fe2594..42d343974e0 100644 --- a/pulpcore/app/util.py +++ b/pulpcore/app/util.py @@ -366,7 +366,10 @@ def get_artifact_url(artifact, headers=None, http_method=None): or not artifact_domain.redirect_to_object_storage ): return _artifact_serving_distribution().artifact_url(artifact) - elif artifact_domain.storage_class == "storages.backends.s3boto3.S3Boto3Storage": + elif artifact_domain.storage_class in ( + "storages.backends.s3boto3.S3Boto3Storage", + "storages.backends.s3.S3Storage", + ): parameters = {"ResponseContentDisposition": content_disposition} if headers and headers.get("Content-Type"): parameters["ResponseContentType"] = headers.get("Content-Type") diff --git a/pulpcore/constants.py b/pulpcore/constants.py index 80b7e46b6b4..da0c380ae21 100644 --- a/pulpcore/constants.py +++ b/pulpcore/constants.py @@ -101,6 +101,7 @@ # Storage-type mapped to storage-response-map STORAGE_RESPONSE_MAP = { "storages.backends.s3boto3.S3Boto3Storage": S3_RESPONSE_HEADER_MAP, + "storages.backends.s3.S3Storage": S3_RESPONSE_HEADER_MAP, "storages.backends.azure_storage.AzureStorage": AZURE_RESPONSE_HEADER_MAP, "storages.backends.gcloud.GoogleCloudStorage": GCS_RESPONSE_HEADER_MAP, } diff --git a/pulpcore/content/handler.py b/pulpcore/content/handler.py index 62b6ef3271e..0c24f420025 100644 --- a/pulpcore/content/handler.py +++ b/pulpcore/content/handler.py @@ -919,7 +919,10 @@ def _set_params_from_headers(hdrs, storage_domain): return FileResponse(path, headers=headers) elif not domain.redirect_to_object_storage: return ArtifactResponse(content_artifact.artifact, headers=headers) - elif domain.storage_class == "storages.backends.s3boto3.S3Boto3Storage": + elif domain.storage_class in ( + "storages.backends.s3boto3.S3Boto3Storage", + "storages.backends.s3.S3Storage", + ): headers["Content-Disposition"] = content_disposition parameters = _set_params_from_headers(headers, domain.storage_class) url = URL( diff --git a/pulpcore/tests/functional/api/test_artifact_distribution.py b/pulpcore/tests/functional/api/test_artifact_distribution.py index d6735347c22..c743c8a7188 100644 --- a/pulpcore/tests/functional/api/test_artifact_distribution.py +++ b/pulpcore/tests/functional/api/test_artifact_distribution.py @@ -7,6 +7,7 @@ OBJECT_STORAGES = ( "storages.backends.s3boto3.S3Boto3Storage", + "storages.backends.s3.S3Storage", "storages.backends.azure_storage.AzureStorage", "storages.backends.gcloud.GoogleCloudStorage", ) diff --git a/pulpcore/tests/functional/api/test_crud_domains.py b/pulpcore/tests/functional/api/test_crud_domains.py index 781fea2d90c..43718c77787 100644 --- a/pulpcore/tests/functional/api/test_crud_domains.py +++ b/pulpcore/tests/functional/api/test_crud_domains.py @@ -186,6 +186,7 @@ def test_special_domain_creation(domains_api_client, gen_object_with_cleanup): "pulpcore.app.models.storage.FileSystem", # "pulpcore.app.models.storage.PulpSFTPStorage", "storages.backends.s3boto3.S3Boto3Storage", + "storages.backends.s3.S3Storage", "storages.backends.azure_storage.AzureStorage", # "storages.backends.gcloud.GoogleCloudStorage", } @@ -200,7 +201,7 @@ def test_special_domain_creation(domains_api_client, gen_object_with_cleanup): "key_filename": "/etc/pulp/certs/storage_id_ed25519", }, }, - "storages.backends.s3boto3.S3Boto3Storage": { + "storages.backends.s3.S3Storage": { "AWS_ACCESS_KEY_ID": "random", "AWS_SECRET_ACCESS_KEY": "random", "AWS_STORAGE_BUCKET_NAME": "pulp3", @@ -223,6 +224,9 @@ def test_special_domain_creation(domains_api_client, gen_object_with_cleanup): "GS_CUSTOM_ENDPOINT": "http://custom-endpoint", }, } + storage_settings["storages.backends.s3boto3.S3Boto3Storage"] = storage_settings[ + "storages.backends.s3.S3Storage" + ] installed_backends = [] domain_names = set() @@ -240,7 +244,8 @@ def test_special_domain_creation(domains_api_client, gen_object_with_cleanup): assert e.status == 400 assert "Backend is not installed on Pulp." in e.body else: - installed_backends.append(backend) + if backend != "storages.backends.s3boto3.S3Boto3Storage": + installed_backends.append(backend) domain_names.add(domain.name) # Try creating domains with correct settings for backend in installed_backends: @@ -253,6 +258,7 @@ def test_special_domain_creation(domains_api_client, gen_object_with_cleanup): domain_names.add(domain.name) # Try creating domains with incorrect settings + storage_types.remove("storages.backends.s3boto3.S3Boto3Storage") for backend in installed_backends: random_backend = random.choice(tuple(storage_types - {backend})) body = { diff --git a/pulpcore/tests/unit/serializers/test_domain.py b/pulpcore/tests/unit/serializers/test_domain.py index 68c852db5f5..d72290f5ed6 100644 --- a/pulpcore/tests/unit/serializers/test_domain.py +++ b/pulpcore/tests/unit/serializers/test_domain.py @@ -30,6 +30,7 @@ def _no_validate_storage_backend(monkeypatch): params=[ "pulpcore.app.models.storage.FileSystem", "storages.backends.s3boto3.S3Boto3Storage", + "storages.backends.s3.S3Storage", "storages.backends.azure_storage.AzureStorage", ] ) @@ -41,7 +42,10 @@ def storage_class(request): def serializer_class(storage_class): if storage_class == "pulpcore.app.models.storage.FileSystem": return FileSystemSettingsSerializer - elif storage_class == "storages.backends.s3boto3.S3Boto3Storage": + elif storage_class in ( + "storages.backends.s3boto3.S3Boto3Storage", + "storages.backends.s3.S3Storage", + ): return AmazonS3SettingsSerializer elif storage_class == "storages.backends.azure_storage.AzureStorage": return AzureSettingsSerializer @@ -51,14 +55,28 @@ def serializer_class(storage_class): def required_settings(storage_class): if storage_class == "pulpcore.app.models.storage.FileSystem": return {"location": "/var/lib/pulp/media/"} - elif storage_class == "storages.backends.s3boto3.S3Boto3Storage": + elif storage_class in ( + "storages.backends.s3boto3.S3Boto3Storage", + "storages.backends.s3.S3Storage", + ): return {"access_key": "testing", "secret_key": "secret", "bucket_name": "test"} elif storage_class == "storages.backends.azure_storage.AzureStorage": return {"account_name": "test", "account_key": "secret", "azure_container": "test"} @pytest.fixture -def all_settings(serializer_class, required_settings): +def extra_required_settings(storage_class): + """For fields required in the serializer's validate, but not on the field itself.""" + if storage_class in ( + "storages.backends.s3boto3.S3Boto3Storage", + "storages.backends.s3.S3Storage", + ): + return {"secret_key": "secret"} + return {} + + +@pytest.fixture +def all_settings(serializer_class, required_settings, extra_required_settings): serializer = serializer_class() fields = serializer.get_fields() default_settings = { @@ -113,6 +131,29 @@ def test_using_setting_names(storage_class, serializer_class, all_settings): assert storage_settings == all_settings +@pytest.mark.django_db +def test_cloudfront_s3_storage_settings(storage_class, required_settings): + if storage_class not in ( + "storages.backends.s3boto3.S3Boto3Storage", + "storages.backends.s3.S3Storage", + ): + pytest.skip("This test only make sense when using S3 as storage backend.") + + domain = SimpleNamespace(storage_class=storage_class, **MIN_DOMAIN_SETTINGS) + data = { + "storage_settings": { + "secret_key": "secret_key", + "custom_domain": "custom_domain.cloudfront.net", + "cloudfront_key_id": "key_id", + "cloudfront_key": "cloudfront_key", + **required_settings, + } + } + serializer = DomainSerializer(domain, data=data, partial=True) + + assert serializer.is_valid(raise_exception=True) + + class DomainSettingsBaseMixin: storage_class = None serializer_class = None