From c90aea86121e4c029f0bf40c1aab049ed3e7ec59 Mon Sep 17 00:00:00 2001 From: Dmitry Severyanin Date: Mon, 12 Dec 2022 13:20:36 +0300 Subject: [PATCH 1/3] Minio 7+ adaptation implementation --- minio_storage/errors.py | 20 ++---- minio_storage/management/commands/minio.py | 26 +++---- minio_storage/storage.py | 72 ++++++++++--------- setup.py | 2 +- tests/test_app/tests/bucket_tests.py | 8 +-- .../test_app/tests/managementcommand_tests.py | 2 +- tests/test_app/tests/retrieve_tests.py | 8 +-- tests/test_app/tests/upload_tests.py | 17 +++-- tests/test_app/tests/utils.py | 2 - tox.ini | 2 +- 10 files changed, 76 insertions(+), 83 deletions(-) diff --git a/minio_storage/errors.py b/minio_storage/errors.py index 474deb2..9d4281b 100644 --- a/minio_storage/errors.py +++ b/minio_storage/errors.py @@ -9,22 +9,10 @@ def __init__(self, msg, cause): reraise = {} for v in ( - merr.APINotImplemented, - merr.AccessDenied, - merr.AccountProblem, - merr.CredentialNotSupported, - merr.CrossLocationLoggingProhibited, - merr.ExpiredToken, - merr.InvalidAccessKeyId, - merr.InvalidAddressingHeader, - merr.InvalidBucketError, - merr.InvalidBucketName, - merr.InvalidDigest, - merr.InvalidEncryptionAlgorithmError, - merr.InvalidEndpointError, - merr.InvalidSecurity, - merr.InvalidToken, - merr.NoSuchBucket, + merr.MinioException, + merr.InvalidResponseError, + merr.ServerError, + merr.S3Error, ): reraise[v] = {"err": v} diff --git a/minio_storage/management/commands/minio.py b/minio_storage/management/commands/minio.py index ba1c648..f8717f8 100644 --- a/minio_storage/management/commands/minio.py +++ b/minio_storage/management/commands/minio.py @@ -167,7 +167,7 @@ def bucket_list( summary: bool = True, ): try: - objs = storage.client.list_objects_v2( + objs = storage.client.list_objects( bucket_name, prefix=prefix, recursive=recursive ) @@ -200,24 +200,25 @@ def fmt(o): if summary: print(f"{n_files} files and {n_dirs} directories", file=sys.stderr) - except minio.error.NoSuchBucket: + except minio.error.S3Error: raise CommandError(f"bucket {bucket_name} does not exist") def bucket_create(self, storage, bucket_name): try: storage.client.make_bucket(bucket_name) print(f"created bucket: {bucket_name}", file=sys.stderr) - except minio.error.BucketAlreadyOwnedByYou: + except minio.error.S3Error: raise CommandError(f"you have already created {bucket_name}") return def bucket_delete(self, storage, bucket_name): try: storage.client.remove_bucket(bucket_name) - except minio.error.NoSuchBucket: - raise CommandError(f"bucket {bucket_name} does not exist") - except minio.error.BucketNotEmpty: - raise CommandError(f"bucket {bucket_name} is not empty") + except minio.error.S3Error as err: + if err.code == 'BucketNotEmpty': + raise CommandError(f"bucket {bucket_name} is not empty") + elif err.code == 'NoSuchBucket': + raise CommandError(f"bucket {bucket_name} does not exist") def policy_get(self, storage, bucket_name): try: @@ -225,14 +226,15 @@ def policy_get(self, storage, bucket_name): policy = json.loads(policy) policy = json.dumps(policy, ensure_ascii=False, indent=2) return policy - except minio.error.NoSuchBucket: - raise CommandError(f"bucket {bucket_name} does not exist") - except minio.error.NoSuchBucketPolicy: - raise CommandError(f"bucket {bucket_name} has no policy") + except minio.error.S3Error as err: + if err.code == 'NoSuchBucket': + raise CommandError(f"bucket {bucket_name} does not exist") + elif err.code == 'NoSuchBucketPolicy': + raise CommandError(f"bucket {bucket_name} has no policy") def policy_set(self, storage, bucket_name, policy: Policy): try: policy = Policy(policy) storage.client.set_bucket_policy(bucket_name, policy.bucket(bucket_name)) - except minio.error.NoSuchBucket as e: + except minio.error.S3Error as e: raise CommandError(e.message) diff --git a/minio_storage/storage.py b/minio_storage/storage.py index f3ddf5b..24376c9 100644 --- a/minio_storage/storage.py +++ b/minio_storage/storage.py @@ -4,7 +4,6 @@ import typing as T import urllib from logging import getLogger -from time import mktime from urllib.parse import urlsplit, urlunsplit import minio @@ -14,7 +13,6 @@ from django.core.files.storage import Storage from django.utils import timezone from django.utils.deconstruct import deconstructible -from minio.helpers import get_target_url from minio_storage.errors import minio_error from minio_storage.files import ReadOnlySpooledTemporaryFile @@ -109,15 +107,16 @@ def _create_base_url_client(client: minio.Minio, bucket_name: str, base_url: str base_url_parts = urlsplit(base_url) # Clone from the normal client, but with base_url as the endpoint + credentials = client._provider.retrieve() base_url_client = minio.Minio( base_url_parts.netloc, - access_key=client._access_key, - secret_key=client._secret_key, - session_token=client._session_token, + access_key=credentials.access_key, + secret_key=credentials.secret_key, + session_token=credentials.session_token, secure=base_url_parts.scheme == "https", # The bucket region may be auto-detected by client (via an HTTP # request), so don't just use client._region - region=client._get_bucket_region(bucket_name), + region=client._get_region(bucket_name, None), http_client=client._http, ) if hasattr(client, "_credentials"): @@ -151,7 +150,7 @@ def _examine_file(self, name, content): def _open(self, name, mode="rb"): try: f = self.file_class(self._sanitize_path(name), mode, self) - except merr.MinioError as e: + except merr.MinioException as e: raise minio_error("File {} could not be saved: {}".format(name, str(e)), e) return f @@ -169,14 +168,14 @@ def _save(self, name: str, content: bytes) -> str: metadata=self.object_metadata, ) return sane_name - except merr.ResponseError as error: + except merr.InvalidResponseError as error: raise minio_error(f"File {name} could not be saved", error) def delete(self, name: str) -> None: if self.backup_format and self.backup_bucket: try: obj = self.client.get_object(self.bucket_name, name) - except merr.ResponseError as error: + except merr.InvalidResponseError as error: raise minio_error( "Could not obtain file {} " "to make a copy of it".format(name), error, @@ -195,7 +194,7 @@ def delete(self, name: str) -> None: self.client.put_object( self.backup_bucket, target_name, obj, content_length ) - except merr.ResponseError as error: + except merr.InvalidResponseError as error: raise minio_error( "Could not make a copy of file " "{} before removing it".format(name), @@ -204,22 +203,22 @@ def delete(self, name: str) -> None: try: self.client.remove_object(self.bucket_name, name) - except merr.ResponseError as error: + except merr.InvalidResponseError as error: raise minio_error(f"Could not remove file {name}", error) def exists(self, name: str) -> bool: try: self.client.stat_object(self.bucket_name, self._sanitize_path(name)) return True - except merr.ResponseError as error: + except merr.InvalidResponseError as error: # TODO - deprecate if error.code == "NoSuchKey": return False else: raise minio_error(f"Could not stat file {name}", error) - except merr.NoSuchKey: + except merr.S3Error: return False - except merr.NoSuchBucket: + except merr.S3Error: raise except Exception as error: logger.error(error) @@ -242,7 +241,7 @@ def listdir(self, path: str) -> T.Tuple[T.List, T.List]: dirs: T.List[str] = [] files: T.List[str] = [] try: - objects = self.client.list_objects_v2(self.bucket_name, prefix=path) + objects = self.client.list_objects(self.bucket_name, prefix=path) for o in objects: p = posixpath.relpath(o.object_name, path) if o.is_dir: @@ -250,16 +249,16 @@ def listdir(self, path: str) -> T.Tuple[T.List, T.List]: else: files.append(p) return dirs, files - except merr.NoSuchBucket: + except merr.S3Error: raise - except merr.ResponseError as error: + except merr.InvalidResponseError as error: raise minio_error(f"Could not list directory {path}", error) def size(self, name: str) -> int: try: info = self.client.stat_object(self.bucket_name, name) return info.size - except merr.ResponseError as error: + except merr.InvalidResponseError as error: raise minio_error(f"Could not access file size for {name}", error) def _presigned_url( @@ -279,7 +278,7 @@ def _presigned_url( # It's assumed that self.base_url will contain bucket information, # which could be different, so remove the bucket_name component (with 1 # extra character for the leading "/") from the generated URL - url_key_path = url_parts.path[len(self.bucket_name) + 1 :] + url_key_path = url_parts.path[len(self.bucket_name) + 1:] # Prefix the URL with any path content from base_url new_url_path = base_url_parts.path + url_key_path @@ -302,30 +301,33 @@ def url( if self.presign_urls: url = self._presigned_url(name, max_age=max_age) else: - if self.base_url is not None: + def strip_beg(path): + while path.startswith("/"): + path = path[1:] + return path - def strip_beg(path): - while path.startswith("/"): - path = path[1:] - return path + def strip_end(path): + while path.endswith("/"): + path = path[:-1] + return path - def strip_end(path): - while path.endswith("/"): - path = path[:-1] - return path + if self.base_url is not None: url = "{}/{}".format( strip_end(self.base_url), urllib.parse.quote(strip_beg(name)) ) else: - url = get_target_url( - self.client._endpoint_url, - bucket_name=self.bucket_name, - object_name=name, - # bucket_region=region, + url = "{}/{}/{}".format( + strip_end(self.endpoint_url), + self.bucket_name, + urllib.parse.quote(strip_beg(name)), ) return url + @property + def endpoint_url(self): + return self.client._base_url._url.geturl() + def accessed_time(self, name: str) -> datetime.datetime: """ Not available via the S3 API @@ -341,8 +343,8 @@ def created_time(self, name: str) -> datetime.datetime: def modified_time(self, name: str) -> datetime.datetime: try: info = self.client.stat_object(self.bucket_name, name) - return datetime.datetime.fromtimestamp(mktime(info.last_modified)) - except merr.ResponseError as error: + return info.last_modified + except merr.InvalidResponseError as error: raise minio_error( f"Could not access modification time for file {name}", error ) diff --git a/setup.py b/setup.py index 24b9797..1e1b9c7 100644 --- a/setup.py +++ b/setup.py @@ -20,7 +20,7 @@ "minio_storage/management/commands/", ], setup_requires=["setuptools_scm"], - install_requires=["django>=1.11", "minio>=4.0.21,<7"], + install_requires=["django>=1.11", "minio>=7.1.12"], extras_require={"test": ["coverage", "requests"]}, classifiers=[ "Development Status :: 4 - Beta", diff --git a/tests/test_app/tests/bucket_tests.py b/tests/test_app/tests/bucket_tests.py index 9cca13e..b24dd28 100644 --- a/tests/test_app/tests/bucket_tests.py +++ b/tests/test_app/tests/bucket_tests.py @@ -77,7 +77,7 @@ def pretty(v): ) def test_auto_create_no_policy(self): ms = MinioMediaStorage() - with self.assertRaises(minio.error.NoSuchBucketPolicy): + with self.assertRaises(minio.error.S3Error): ms.client.get_bucket_policy(ms.bucket_name) @override_settings( @@ -96,7 +96,7 @@ def test_media_policy_auto_true(self): url = ms.url(fn) self.assertEqual(requests.get(url).status_code, 200) self.assertEqual( - requests.get(f"{ms.client._endpoint_url}/{ms.bucket_name}").status_code, 403 + requests.get(f"{ms.endpoint_url}/{ms.bucket_name}").status_code, 403 ) @override_settings( @@ -114,7 +114,7 @@ def test_media_policy_get(self): self.assertEqual(ms.open(fn).read(), b"test") self.assertEqual(requests.get(ms.url(fn)).status_code, 200) self.assertEqual( - requests.get(f"{ms.client._endpoint_url}/{ms.bucket_name}").status_code, 403 + requests.get(f"{ms.endpoint_url}/{ms.bucket_name}").status_code, 403 ) @override_settings( @@ -132,7 +132,7 @@ def test_media_policy_write(self): self.assertEqual(ms.open(fn).read(), b"test") self.assertEqual(requests.get(ms.url(fn)).status_code, 403) self.assertEqual( - requests.get(f"{ms.client._endpoint_url}/{ms.bucket_name}").status_code, 403 + requests.get(f"{ms.endpoint_url}/{ms.bucket_name}").status_code, 403 ) @override_settings( diff --git a/tests/test_app/tests/managementcommand_tests.py b/tests/test_app/tests/managementcommand_tests.py index 94df7d4..8599f6d 100644 --- a/tests/test_app/tests/managementcommand_tests.py +++ b/tests/test_app/tests/managementcommand_tests.py @@ -31,7 +31,7 @@ def test_management_command(self): try: self.obliterate_bucket(self.media_storage.bucket_name) - except minio.error.NoSuchBucket: + except minio.error.S3Error: pass with self.assertRaisesRegex(CommandError, f"bucket {bucket} does not exist"): diff --git a/tests/test_app/tests/retrieve_tests.py b/tests/test_app/tests/retrieve_tests.py index 8a1b631..4e4c11c 100644 --- a/tests/test_app/tests/retrieve_tests.py +++ b/tests/test_app/tests/retrieve_tests.py @@ -7,7 +7,7 @@ from django.core.files.base import ContentFile from django.test import TestCase, override_settings from freezegun import freeze_time -from minio.error import NoSuchKey +from minio.error import S3Error from minio_storage.errors import MinIOError from minio_storage.storage import MinioMediaStorage @@ -44,7 +44,7 @@ def test_file_size(self): def test_size_of_non_existent_throws(self): test_file = self.media_storage.save("sizetest.txt", ContentFile(b"1234")) self.media_storage.delete(test_file) - with self.assertRaises(NoSuchKey): + with self.assertRaises(S3Error): self.media_storage.size(test_file) def test_modified_time(self): @@ -63,7 +63,7 @@ def test_created_time(self): ) def test_modified_time_of_non_existent_throws(self): - with self.assertRaises(NoSuchKey): + with self.assertRaises(S3Error): self.media_storage.modified_time("nonexistent.jpg") def _listdir_root(self, root): @@ -128,7 +128,7 @@ def test_opening_non_existing_file_raises_minioerror(self): try: self.media_storage.open("this does not exist") except MinIOError as e: - assert e.cause.__class__ == NoSuchKey + assert e.cause.__class__ == S3Error def test_file_names_are_properly_sanitized(self): self.media_storage.save("./meh22222.txt", io.BytesIO(b"stuff")) diff --git a/tests/test_app/tests/upload_tests.py b/tests/test_app/tests/upload_tests.py index d97040f..8d951a9 100644 --- a/tests/test_app/tests/upload_tests.py +++ b/tests/test_app/tests/upload_tests.py @@ -4,7 +4,7 @@ from django.conf import settings from django.core.files.base import ContentFile, File from django.test import TestCase, override_settings -from minio.error import InvalidAccessKeyId +from minio.error import S3Error from minio_storage.storage import MinioMediaStorage @@ -22,7 +22,7 @@ def test_file_upload_success(self): MINIO_STORAGE_ACCESS_KEY="wrong_key", MINIO_STORAGE_SECRET_KEY="wrong_secret" ) def test_file_upload_fail_incorrect_keys(self): - with self.assertRaises(InvalidAccessKeyId): + with self.assertRaises(S3Error): MinioMediaStorage() def test_two_files_with_the_same_name_can_be_uploaded(self): @@ -88,7 +88,12 @@ def test_metadata(self): res = self.media_storage.client.stat_object( self.media_storage.bucket_name, ivan ) - self.assertEqual(res.metadata, {"Content-Type": "text/plain"}) + metadata_attrs = { + "Accept-Ranges", "Content-Length", "Content-Security-Policy", "Content-Type", + "ETag", "Last-Modified", "Server", "Strict-Transport-Security", "Vary", + "X-Amz-Request-Id", "X-Content-Type-Options", "X-Xss-Protection", "Date" + } + self.assertEqual(res.metadata.keys(), metadata_attrs) @override_settings( @@ -100,7 +105,5 @@ def test_default_metadata(self): res = self.media_storage.client.stat_object( self.media_storage.bucket_name, ivan ) - self.assertEqual( - res.metadata, - {"Cache-Control": "max-age=1000", "Content-Type": "text/plain"}, - ) + + self.assertEqual(res.metadata["Cache-Control"], "max-age=1000") diff --git a/tests/test_app/tests/utils.py b/tests/test_app/tests/utils.py index 9f4dd18..539efab 100644 --- a/tests/test_app/tests/utils.py +++ b/tests/test_app/tests/utils.py @@ -53,6 +53,4 @@ def obliterate_bucket(self, name, client=None): for obj in client.list_objects(name, "", True): client.remove_object(name, obj.object_name) - for obj in client.list_incomplete_uploads(name, ""): # pragma: no cover # noqa - client.remove_incomplete_upload(name, obj.objectname) client.remove_bucket(name) diff --git a/tox.ini b/tox.ini index 1237e09..065b348 100644 --- a/tox.ini +++ b/tox.ini @@ -25,7 +25,7 @@ deps = django32: Django==3.2.* django42: Django==4.2.* minio: minio - minioknown: minio==6.0.2 + minioknown: minio==7.1.12 -rdev-requirements.txt [testenv:lint] From a904809ece9919ff449d67c753f1a9f6531d3133 Mon Sep 17 00:00:00 2001 From: Dmitry Severyanin Date: Wed, 21 Dec 2022 17:16:00 +0300 Subject: [PATCH 2/3] Minio 7+: Fix renamed ResponseError --- minio_storage/files.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/minio_storage/files.py b/minio_storage/files.py index 0f29d23..7bde5eb 100644 --- a/minio_storage/files.py +++ b/minio_storage/files.py @@ -74,7 +74,7 @@ def _get_file(self): ) self._file = obj return self._file - except merr.ResponseError as error: + except merr.InvalidResponseError as error: logger.warn(error) raise OSError(f"File {self.name} does not exist") finally: @@ -131,7 +131,7 @@ def _get_file(self): self._file.write(d) self._file.seek(0) return self._file - except merr.ResponseError as error: + except merr.InvalidResponseError as error: raise minio_error(f"File {self.name} does not exist", error) finally: try: From af7184370ac770fc1659c824e812896efa057e87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Fr=C3=B6ssman?= Date: Sat, 22 Apr 2023 11:47:37 +0200 Subject: [PATCH 3/3] fix test to allow to run with older minio versions --- tests/test_app/tests/upload_tests.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tests/test_app/tests/upload_tests.py b/tests/test_app/tests/upload_tests.py index 8d951a9..311bb2c 100644 --- a/tests/test_app/tests/upload_tests.py +++ b/tests/test_app/tests/upload_tests.py @@ -89,11 +89,19 @@ def test_metadata(self): self.media_storage.bucket_name, ivan ) metadata_attrs = { - "Accept-Ranges", "Content-Length", "Content-Security-Policy", "Content-Type", - "ETag", "Last-Modified", "Server", "Strict-Transport-Security", "Vary", - "X-Amz-Request-Id", "X-Content-Type-Options", "X-Xss-Protection", "Date" + "Accept-Ranges", + "Content-Length", + "Content-Security-Policy", + "Content-Type", + "ETag", + "Last-Modified", + "Server", + "Vary", + "X-Amz-Request-Id", + "X-Xss-Protection", + "Date", } - self.assertEqual(res.metadata.keys(), metadata_attrs) + self.assertTrue(metadata_attrs.issubset(res.metadata.keys())) @override_settings(