From 432537033e99345abda3675c2f7cc556d14a9471 Mon Sep 17 00:00:00 2001 From: Wilfried BARADAT Date: Fri, 9 Dec 2022 16:46:52 +0100 Subject: [PATCH 1/2] =?UTF-8?q?=E2=9C=A8(backends)=20add=20support=20for?= =?UTF-8?q?=20AWS=20S3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implement s3 backend using low-level interface client from the boto3 library --- .env.dist | 8 + CHANGELOG.md | 1 + docs/backends.md | 19 ++ setup.cfg | 4 + src/ralph/backends/storage/s3.py | 150 +++++++++++ src/ralph/conf.py | 13 + tests/backends/storage/test_s3.py | 398 ++++++++++++++++++++++++++++++ tests/conftest.py | 2 + tests/fixtures/backends.py | 33 +++ tests/test_cli.py | 22 +- 10 files changed, 646 insertions(+), 4 deletions(-) create mode 100644 src/ralph/backends/storage/s3.py create mode 100644 tests/backends/storage/test_s3.py diff --git a/.env.dist b/.env.dist index 2ce36590e..083f9f702 100644 --- a/.env.dist +++ b/.env.dist @@ -33,6 +33,14 @@ RALPH_APP_DIR=/app/.ralph # RALPH_BACKENDS__STORAGE__SWIFT__OS_REGION_NAME=RegionOne # RALPH_BACKENDS__STORAGE__SWIFT__OS_STORAGE_URL=http://swift:8080/v1/KEY_cd238e84310a46e58af7f1d515887d88/test_container +# S3 storage backend + +# RALPH_BACKENDS__STORAGE__S3__ACCESS_KEY_ID= +# RALPH_BACKENDS__STORAGE__S3__SECRET_ACCESS_KEY= +# RALPH_BACKENDS__STORAGE__S3__SESSION_TOKEN= +# RALPH_BACKENDS__STORAGE__S3__DEFAULT_REGION= +# RALPH_BACKENDS__STORAGE__S3__BUCKET_NAME= + # ES database backend RALPH_BACKENDS__DATABASE__ES__HOSTS=http://elasticsearch:9200 diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e1b5dede..7fd4b56c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to - Add a new `auth` subcommand to generate required credentials file for the LRS - Add an official Helm Chart (experimental) +- Implement support for AWS S3 storage backend ### Changed diff --git a/docs/backends.md b/docs/backends.md index 61fa39b45..829ddba62 100644 --- a/docs/backends.md +++ b/docs/backends.md @@ -100,6 +100,25 @@ Secondary parameters are required to work with the target container: - `os_tenant_name`: the name of the tenant of your container - `os_tenant_id`: the identifier of the tenant of your container +### Amazon S3 + +S3 is the Amazon Simple Storage Service. This storage backend is fully +supported (read and write operations) to stream and store log archives. + +#### Backend parameters + +Primarily required parameters correspond to a standard authentication with AWS CLI: + +- `access_key_id`: the access key for your AWS account +- `secret_access_key`: the secret key for your AWS account +- `session_token`: the session key for your AWS account (only needed when you are using +temporary credentials). + +Secondary parameters are required to work with the target bucket: + +- `default_region`: the region where your bucket is +- `bucket_name`: the name of your S3 bucket + ### File system The file system backend is a dummy template that can be used to develop your diff --git a/setup.cfg b/setup.cfg index a8b4d8e8b..24460101a 100644 --- a/setup.cfg +++ b/setup.cfg @@ -42,6 +42,9 @@ backend-ldp = requests>=2.0.0 backend-mongo = pymongo[srv]>=4.0.0 +backend-s3 = + boto3>=1.24.70 + botocore>=1.27.71 backend-swift = python-keystoneclient>=5.0.0 python-swiftclient>=4.0.0 @@ -68,6 +71,7 @@ dev = mkdocs-click==0.8.0 mkdocs-material==8.5.11 mkdocstrings[python-legacy]==0.19.1 + moto==4.0.3 pydocstyle==6.1.1 pyfakefs==5.0.0 pylint==2.15.9 diff --git a/src/ralph/backends/storage/s3.py b/src/ralph/backends/storage/s3.py new file mode 100644 index 000000000..4f25bf0fa --- /dev/null +++ b/src/ralph/backends/storage/s3.py @@ -0,0 +1,150 @@ +"""S3 storage backend for Ralph""" + +import logging + +import boto3 +from botocore.exceptions import ClientError, ParamValidationError + +from ralph.conf import settings +from ralph.exceptions import BackendException, BackendParameterException +from ralph.utils import now + +from ..mixins import HistoryMixin +from .base import BaseStorage + +s3_settings = settings.BACKENDS.STORAGE.S3 +logger = logging.getLogger(__name__) + + +class S3Storage( + HistoryMixin, BaseStorage +): # pylint: disable=too-many-instance-attributes + """AWS S3 storage backend.""" + + name = "s3" + + # pylint: disable=too-many-arguments + + def __init__( + self, + access_key_id: str = s3_settings.ACCESS_KEY_ID, + secret_access_key: str = s3_settings.SECRET_ACCESS_KEY, + session_token: str = s3_settings.SESSION_TOKEN, + default_region: str = s3_settings.DEFAULT_REGION, + bucket_name: str = s3_settings.BUCKET_NAME, + ): + """Instantiates the AWS S3 client.""" + + self.access_key_id = access_key_id + self.secret_access_key = secret_access_key + self.session_token = session_token + self.default_region = default_region + self.bucket_name = bucket_name + + self.client = boto3.client( + "s3", + aws_access_key_id=self.access_key_id, + aws_secret_access_key=self.secret_access_key, + aws_session_token=self.session_token, + region_name=self.default_region, + ) + + # Check whether bucket exists and is accessible + try: + self.client.head_bucket(Bucket=self.bucket_name) + except ClientError as err: + error_msg = err.response["Error"]["Message"] + msg = "Unable to connect to the requested bucket: %s" + logger.error(msg, error_msg) + raise BackendParameterException(msg % error_msg) from err + + def list(self, details=False, new=False): + """Lists archives in the storage backend.""" + + archives_to_skip = set() + if new: + archives_to_skip = set(self.get_command_history(self.name, "fetch")) + + try: + paginator = self.client.get_paginator("list_objects_v2") + page_iterator = paginator.paginate(Bucket=self.bucket_name) + for archives in page_iterator: + if "Contents" not in archives: + continue + for archive in archives["Contents"]: + if new and archive["Key"] in archives_to_skip: + continue + if details: + archive["LastModified"] = archive["LastModified"].strftime( + "%Y-%m-%d %H:%M:%S" + ) + yield archive + else: + yield archive["Key"] + except ClientError as err: + error_msg = err.response["Error"]["Message"] + msg = "Failed to list the bucket %s: %s" + logger.error(msg, self.bucket_name, error_msg) + raise BackendException(msg % (self.bucket_name, error_msg)) from err + + def url(self, name): + """Gets `name` file absolute URL.""" + + return f"{self.bucket_name}.s3.{self.default_region}.amazonaws.com/{name}" + + def read(self, name, chunk_size: int = 4096): + """Reads `name` file and yields its content by chunks of a given size.""" + + logger.debug("Getting archive: %s", name) + + try: + obj = self.client.get_object(Bucket=self.bucket_name, Key=name) + except ClientError as err: + error_msg = err.response["Error"]["Message"] + msg = "Failed to download %s: %s" + logger.error(msg, name, error_msg) + raise BackendException(msg % (name, error_msg)) from err + + size = 0 + for chunk in obj["Body"].iter_chunks(chunk_size): + logger.debug("Chunk length %s", len(chunk)) + size += len(chunk) + yield chunk + + # Archive fetched, add a new entry to the history + self.append_to_history( + { + "backend": self.name, + "command": "fetch", + "id": name, + "size": size, + "fetched_at": now(), + } + ) + + def write(self, stream, name, overwrite=False): + """Writes data from `stream` to the `name` target.""" + + if not overwrite and name in list(self.list()): + msg = "%s already exists and overwrite is not allowed" + logger.error(msg, name) + raise FileExistsError(msg % name) + + logger.debug("Creating archive: %s", name) + + try: + self.client.upload_fileobj(stream, self.bucket_name, name) + except (ClientError, ParamValidationError) as exc: + msg = "Failed to upload" + logger.error(msg) + raise BackendException(msg) from exc + + # Archive written, add a new entry to the history + self.append_to_history( + { + "backend": self.name, + "command": "push", + "id": name, + "pushed_at": now(), + } + ) diff --git a/src/ralph/conf.py b/src/ralph/conf.py index 2c9273120..1f8a683ea 100644 --- a/src/ralph/conf.py +++ b/src/ralph/conf.py @@ -145,12 +145,25 @@ class SWIFTStorageBackendSettings(InstantiableSettingsItem): OS_IDENTITY_API_VERSION: str = "3" +class S3StorageBackendSettings(InstantiableSettingsItem): + """Represents the S3 storage backend configuration settings.""" + + _class_path: str = "ralph.backends.storage.s3.S3Storage" + + ACCESS_KEY_ID: str = None + SECRET_ACCESS_KEY: str = None + SESSION_TOKEN: str = None + DEFAULT_REGION: str = None + BUCKET_NAME: str = None + + class StorageBackendSettings(BaseModel): """Pydantic model for storage backend configuration settings.""" LDP: LDPStorageBackendSettings = LDPStorageBackendSettings() FS: FSStorageBackendSettings = FSStorageBackendSettings() SWIFT: SWIFTStorageBackendSettings = SWIFTStorageBackendSettings() + S3: S3StorageBackendSettings = S3StorageBackendSettings() # Active storage backend Settings. diff --git a/tests/backends/storage/test_s3.py b/tests/backends/storage/test_s3.py new file mode 100644 index 000000000..afd75468f --- /dev/null +++ b/tests/backends/storage/test_s3.py @@ -0,0 +1,398 @@ +"""Tests for Ralph S3 storage backend.""" + +import datetime +import json +import logging +import sys +from io import BytesIO + +import boto3 +import pytest +from moto import mock_s3 + +from ralph.conf import settings +from ralph.exceptions import BackendException, BackendParameterException + + +@mock_s3 +def test_backends_storage_s3_storage_instantiation_should_raise_exception( + s3, caplog +): # pylint:disable=invalid-name + """S3 backend instantiation test. + + Checks that S3Storage raises BackendParameterException on failure. + """ + # Regions outside of us-east-1 require the appropriate LocationConstraint + s3_client = boto3.client("s3", region_name="us-east-1") + # Create an invalid bucket in Moto's 'virtual' AWS account + bucket_name = "my-test-bucket" + s3_client.create_bucket(Bucket=bucket_name) + + error = "Not Found" + caplog.set_level(logging.ERROR) + + with pytest.raises(BackendParameterException): + s3() + logger_name = "ralph.backends.storage.s3" + msg = f"Unable to connect to the requested bucket: {error}" + assert caplog.record_tuples == [(logger_name, logging.ERROR, msg)] + + +@mock_s3 +def test_backends_storage_s3_storage_instantiation_failure_should_not_raise_exception( + s3, +): # pylint:disable=invalid-name + """S3 backend instantiation test. + + Checks that S3Storage doesn't raise exceptions when the connection is + successful. + """ + # Regions outside of us-east-1 require the appropriate LocationConstraint + s3_client = boto3.client("s3", region_name="us-east-1") + # Create a valid bucket in Moto's 'virtual' AWS account + bucket_name = "bucket_name" + s3_client.create_bucket(Bucket=bucket_name) + + try: + s3() + except Exception: # pylint:disable=broad-except + pytest.fail("S3Storage should not raise exception on successful connection") + + +@mock_s3 +def test_backends_storage_s3_list_should_yield_archive_names( + moto_fs, s3, fs +): # pylint:disable=unused-argument, invalid-name + """S3 backend list test. + + Tests that given S3Service.list method successfully connects to the S3 + storage, the S3Storage list method should yield the archives. + """ + # Regions outside of us-east-1 require the appropriate LocationConstraint + s3_client = boto3.client("s3", region_name="us-east-1") + # Create a valid bucket + bucket_name = "bucket_name" + s3_client.create_bucket(Bucket=bucket_name) + + s3_client.put_object( + Bucket=bucket_name, + Key="2022-04-29.gz", + Body=json.dumps({"id": "1", "foo": "bar"}), + ) + + s3_client.put_object( + Bucket=bucket_name, + Key="2022-04-30.gz", + Body=json.dumps({"id": "2", "some": "data"}), + ) + + s3_client.put_object( + Bucket=bucket_name, + Key="2022-10-01.gz", + Body=json.dumps({"id": "3", "other": "info"}), + ) + + listing = [ + {"name": "2022-04-29.gz"}, + {"name": "2022-04-30.gz"}, + {"name": "2022-10-01.gz"}, + ] + + history = [ + {"id": "2022-04-29.gz", "backend": "s3", "command": "fetch"}, + {"id": "2022-04-30.gz", "backend": "s3", "command": "fetch"}, + ] + + s3 = s3() + try: + response_list = s3.list() + response_list_new = s3.list(new=True) + response_list_details = s3.list(details=True) + except Exception: # pylint:disable=broad-except + pytest.fail("S3Storage should not raise exception on successful list") + + fs.create_file(settings.HISTORY_FILE, contents=json.dumps(history)) + + assert list(response_list) == [x["name"] for x in listing] + assert list(response_list_new) == ["2022-10-01.gz"] + assert [x["Key"] for x in response_list_details] == [x["name"] for x in listing] + + +@mock_s3 +def test_backends_storage_s3_list_on_empty_bucket_should_do_nothing( + moto_fs, s3, fs +): # pylint:disable=unused-argument, invalid-name + """S3 backend list test. + + Tests that given S3Service.list method successfully connects to the S3 + storage, the S3Storage list method on an empty bucket should do nothing. + """ + # Regions outside of us-east-1 require the appropriate LocationConstraint + s3_client = boto3.client("s3", region_name="us-east-1") + # Create a valid bucket + bucket_name = "bucket_name" + s3_client.create_bucket(Bucket=bucket_name) + + listing = [] + + history = [] + + s3 = s3() + try: + response_list = s3.list() + except Exception: # pylint:disable=broad-except + pytest.fail("S3Storage should not raise exception on successful list") + + fs.create_file(settings.HISTORY_FILE, contents=json.dumps(history)) + + assert list(response_list) == [x["name"] for x in listing] + + +@mock_s3 +def test_backends_storage_s3_list_with_failed_connection_should_log_the_error( + moto_fs, s3, fs, caplog +): # pylint:disable=unused-argument, invalid-name + """S3 backend list test. + + Tests that given S3Service.list method fails to retrieve the list of archives, + the S3Storage list method should log the error and raise a BackendException. + """ + # Regions outside of us-east-1 require the appropriate LocationConstraint + s3_client = boto3.client("s3", region_name="us-east-1") + # Create a valid bucket in Moto's 'virtual' AWS account + bucket_name = "bucket_name" + s3_client.create_bucket(Bucket=bucket_name) + + s3_client.put_object( + Bucket=bucket_name, + Key="2022-04-29.gz", + Body=json.dumps({"id": "1", "foo": "bar"}), + ) + + s3 = s3() + s3.bucket_name = "wrong_name" + + fs.create_file(settings.HISTORY_FILE, contents=json.dumps([])) + caplog.set_level(logging.ERROR) + error = "The specified bucket does not exist" + msg = f"Failed to list the bucket wrong_name: {error}" + + with pytest.raises(BackendException, match=msg): + next(s3.list()) + with pytest.raises(BackendException, match=msg): + next(s3.list(new=True)) + with pytest.raises(BackendException, match=msg): + next(s3.list(details=True)) + logger_name = "ralph.backends.storage.s3" + assert caplog.record_tuples == [(logger_name, logging.ERROR, msg)] * 3 + + +@mock_s3 +def test_backends_storage_s3_read_with_valid_name_should_write_to_history( + moto_fs, s3, monkeypatch, fs +): # pylint:disable=unused-argument, invalid-name + """S3 backend read test. + + Tests that given S3Service.download method successfully retrieves from the + S3 storage the object with the provided name (the object exists), + the S3Storage read method should write the entry to the history. + """ + # Regions outside of us-east-1 require the appropriate LocationConstraint + s3_client = boto3.client("s3", region_name="us-east-1") + # Create a valid bucket in Moto's 'virtual' AWS account + bucket_name = "bucket_name" + s3_client.create_bucket(Bucket=bucket_name) + + body = b"some contents in the body" + + s3_client.put_object( + Bucket=bucket_name, + Key="2022-09-29.gz", + Body=body, + ) + + freezed_now = datetime.datetime.now(tz=datetime.timezone.utc).isoformat() + monkeypatch.setattr("ralph.backends.storage.s3.now", lambda: freezed_now) + fs.create_file(settings.HISTORY_FILE, contents=json.dumps([])) + + try: + s3 = s3() + list(s3.read("2022-09-29.gz")) + except Exception: # pylint:disable=broad-except + pytest.fail("S3Storage should not raise exception on successful read") + + assert s3.history == [ + { + "backend": "s3", + "command": "fetch", + "id": "2022-09-29.gz", + "size": len(body), + "fetched_at": freezed_now, + } + ] + + +@mock_s3 +def test_backends_storage_s3_read_with_invalid_name_should_log_the_error( + moto_fs, s3, fs, caplog +): # pylint:disable=unused-argument, invalid-name + """S3 backend read test. + + Tests that given S3Service.download method fails to retrieve from the S3 + storage the object with the provided name (the object does not exists on S3), + the S3Storage read method should log the error, not write to history and raise a + BackendException. + """ + # Regions outside of us-east-1 require the appropriate LocationConstraint + s3_client = boto3.client("s3", region_name="us-east-1") + # Create a valid bucket in Moto's 'virtual' AWS account + bucket_name = "bucket_name" + s3_client.create_bucket(Bucket=bucket_name) + + body = b"some contents in the body" + + s3_client.put_object( + Bucket=bucket_name, + Key="2022-09-29.gz", + Body=body, + ) + + fs.create_file(settings.HISTORY_FILE, contents=json.dumps([])) + caplog.set_level(logging.ERROR) + error = "The specified key does not exist." + + with pytest.raises(BackendException): + s3 = s3() + list(s3.read("invalid_name.gz")) + logger_name = "ralph.backends.storage.s3" + msg = f"Failed to download invalid_name.gz: {error}" + assert caplog.record_tuples == [(logger_name, logging.ERROR, msg)] + assert s3.history == [] + + +# pylint: disable=line-too-long +@pytest.mark.parametrize("overwrite", [False, True]) +@pytest.mark.parametrize("new_archive", [False, True]) +@mock_s3 +def test_backends_storage_s3_write_should_write_to_history_new_or_overwritten_archives( # noqa + moto_fs, overwrite, new_archive, s3, monkeypatch, fs, caplog +): # pylint:disable=unused-argument, invalid-name, too-many-arguments, too-many-locals + """S3 backend write test. + + Tests that given S3Service list/upload method successfully connects to the + S3 storage, the S3Storage write method should update the history file when + overwrite is True or when the name of the archive is not in the history. + In case overwrite is False and the archive is in the history, the write method + should raise a FileExistsError. + """ + # Regions outside of us-east-1 require the appropriate LocationConstraint + s3_client = boto3.client("s3", region_name="us-east-1") + # Create a valid bucket in Moto's 'virtual' AWS account + bucket_name = "bucket_name" + s3_client.create_bucket(Bucket=bucket_name) + + body = b"some contents in the body" + + s3_client.put_object( + Bucket=bucket_name, + Key="2022-09-29.gz", + Body=body, + ) + + history = [ + {"id": "2022-09-29.gz", "backend": "s3", "command": "fetch"}, + {"id": "2022-09-30.gz", "backend": "s3", "command": "fetch"}, + ] + + freezed_now = datetime.datetime.now(tz=datetime.timezone.utc).isoformat() + archive_name = "not_in_history.gz" if new_archive else "2022-09-29.gz" + new_history_entry = [ + { + "backend": "s3", + "command": "push", + "id": archive_name, + "pushed_at": freezed_now, + } + ] + + stream_content = b"some contents in the stream file to upload" + monkeypatch.setattr(sys, "stdin", BytesIO(stream_content)) + monkeypatch.setattr("ralph.backends.storage.s3.now", lambda: freezed_now) + fs.create_file(settings.HISTORY_FILE, contents=json.dumps(history)) + caplog.set_level(logging.ERROR) + + s3 = s3() + if not overwrite and not new_archive: + new_history_entry = [] + msg = f"{archive_name} already exists and overwrite is not allowed" + with pytest.raises(FileExistsError, match=msg): + s3.write(sys.stdin, archive_name, overwrite=overwrite) + logger_name = "ralph.backends.storage.s3" + assert caplog.record_tuples == [(logger_name, logging.ERROR, msg)] + else: + s3.write(sys.stdin, archive_name, overwrite=overwrite) + assert s3.history == history + new_history_entry + + +@mock_s3 +def test_backends_storage_s3_write_should_log_the_error( + moto_fs, s3, monkeypatch, fs, caplog +): # pylint:disable=unused-argument, invalid-name + """S3 backend write test. + + Tests that given S3Service.upload method fails to write the archive, + the S3Storage write method should log the error, raise a BackendException + and not write to history. + """ + # Regions outside of us-east-1 require the appropriate LocationConstraint + s3_client = boto3.client("s3", region_name="us-east-1") + # Create a valid bucket in Moto's 'virtual' AWS account + bucket_name = "bucket_name" + s3_client.create_bucket(Bucket=bucket_name) + + body = b"some contents in the body" + + s3_client.put_object( + Bucket=bucket_name, + Key="2022-09-29.gz", + Body=body, + ) + + history = [ + {"id": "2022-09-29.gz", "backend": "s3", "command": "fetch"}, + {"id": "2022-09-30.gz", "backend": "s3", "command": "fetch"}, + ] + + fs.create_file(settings.HISTORY_FILE, contents=json.dumps(history)) + caplog.set_level(logging.ERROR) + + s3 = s3() + + error = "Failed to upload" + + stream_content = b"some contents in the stream file to upload" + monkeypatch.setattr(sys, "stdin", BytesIO(stream_content)) + + with pytest.raises(BackendException): + s3.write(sys.stdin, "", overwrite=True) + logger_name = "ralph.backends.storage.s3" + assert caplog.record_tuples == [(logger_name, logging.ERROR, error)] + assert s3.history == history + + +@mock_s3 +def test_backends_storage_url_should_concatenate_the_storage_url_and_name( + s3, +): # pylint:disable=invalid-name + """S3 backend url test. + + Checks the url method returns `bucket_name.s3.default_region + .amazonaws.com/name`. + """ + # Regions outside of us-east-1 require the appropriate LocationConstraint + s3_client = boto3.client("s3", region_name="us-east-1") + # Create a valid bucket in Moto's 'virtual' AWS account + bucket_name = "bucket_name" + s3_client.create_bucket(Bucket=bucket_name) + + assert s3().url("name") == "bucket_name.s3.default-region.amazonaws.com/name" diff --git a/tests/conftest.py b/tests/conftest.py index 51804c1fe..8ecc58699 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -13,6 +13,8 @@ lrs, mongo, mongo_forwarding, + moto_fs, + s3, swift, ws, ) diff --git a/tests/fixtures/backends.py b/tests/fixtures/backends.py index b6dd733fc..112be14aa 100644 --- a/tests/fixtures/backends.py +++ b/tests/fixtures/backends.py @@ -9,7 +9,10 @@ from enum import Enum from functools import lru_cache from multiprocessing import Process +from pathlib import Path +import boto3 +import botocore import pytest import uvicorn import websockets @@ -20,6 +23,7 @@ from ralph.backends.database.es import ESDatabase from ralph.backends.database.mongo import MongoDatabase +from ralph.backends.storage.s3 import S3Storage from ralph.backends.storage.swift import SwiftStorage # Elasticsearch backend defaults @@ -232,6 +236,35 @@ def get_swift_storage(): return get_swift_storage +@pytest.fixture() +def moto_fs(fs): + """Fix the incompatibility between moto and pyfakefs""" + # pylint:disable=invalid-name + + for module in [boto3, botocore]: + module_dir = Path(module.__file__).parent + fs.add_real_directory(module_dir, lazy_read=False) + + +@pytest.fixture +def s3(): + """Returns get_s3_storage function.""" + # pylint:disable=invalid-name + + def get_s3_storage(): + """Returns an instance of S3Storage.""" + + return S3Storage( + access_key_id="access_key_id", + secret_access_key="secret_access_key", + session_token="session_token", + default_region="default-region", + bucket_name="bucket_name", + ) + + return get_s3_storage + + @pytest.fixture def events(): """Returns test events fixture.""" diff --git a/tests/test_cli.py b/tests/test_cli.py index 235fdef24..e7973b858 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -460,12 +460,19 @@ def test_cli_fetch_command_usage(): result = runner.invoke(cli, ["fetch", "--help"]) assert result.exit_code == 0 + assert ( "Options:\n" - " -b, --backend [es|mongo|ldp|fs|swift|ws]\n" + " -b, --backend [es|mongo|ldp|fs|swift|s3|ws]\n" " Backend [required]\n" " ws backend: \n" " --ws-uri TEXT\n" + " s3 backend: \n" + " --s3-bucket-name TEXT\n" + " --s3-default-region TEXT\n" + " --s3-session-token TEXT\n" + " --s3-secret-access-key TEXT\n" + " --s3-access-key-id TEXT\n" " swift backend: \n" " --swift-os-identity-api-version TEXT\n" " --swift-os-auth-url TEXT\n" @@ -506,7 +513,7 @@ def test_cli_fetch_command_usage(): assert result.exit_code > 0 assert ( "Error: Missing option '-b' / '--backend'. " - "Choose from:\n\tes,\n\tmongo,\n\tldp,\n\tfs,\n\tswift,\n\tws\n" + "Choose from:\n\tes,\n\tmongo,\n\tldp,\n\tfs,\n\tswift,\n\ts3,\n\tws\n" ) in result.output @@ -666,7 +673,14 @@ def test_cli_list_command_usage(): assert result.exit_code == 0 assert ( "Options:\n" - " -b, --backend [ldp|fs|swift] Backend [required]\n" + " -b, --backend [ldp|fs|swift|s3]\n" + " Backend [required]\n" + " s3 backend: \n" + " --s3-bucket-name TEXT\n" + " --s3-default-region TEXT\n" + " --s3-session-token TEXT\n" + " --s3-secret-access-key TEXT\n" + " --s3-access-key-id TEXT\n" " swift backend: \n" " --swift-os-identity-api-version TEXT\n" " --swift-os-auth-url TEXT\n" @@ -695,7 +709,7 @@ def test_cli_list_command_usage(): assert result.exit_code > 0 assert ( "Error: Missing option '-b' / '--backend'. Choose from:\n\tldp,\n\tfs,\n\t" - "swift\n" + "swift,\n\ts3\n" ) in result.output From 3199baafc66b906cbf6923dd5cb52e114f42e785 Mon Sep 17 00:00:00 2001 From: Wilfried BARADAT Date: Wed, 14 Dec 2022 11:59:54 +0100 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=94=A7(linter)=20fix=20pydocstyle=20l?= =?UTF-8?q?inter?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix pydocstyle linter now processing all src files and fix linter warnings. --- setup.cfg | 3 ++- src/ralph/api/auth.py | 19 +++++++++++++------ src/ralph/backends/storage/s3.py | 7 +------ 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/setup.cfg b/setup.cfg index 24460101a..49f44d5d0 100644 --- a/setup.cfg +++ b/setup.cfg @@ -119,7 +119,8 @@ exclude = [pydocstyle] convention = google -match = ^src/ralph.*\.py +match_dir = ^(?!tests).* +match = ^(?!(setup)\.(py)$).*\.(py)$ [isort] known_ralph=ralph diff --git a/src/ralph/api/auth.py b/src/ralph/api/auth.py index 0bae285f0..4ca6809b3 100644 --- a/src/ralph/api/auth.py +++ b/src/ralph/api/auth.py @@ -49,21 +49,28 @@ class UserCredentials(AuthenticatedUser): class ServerUsersCredentials(BaseModel): - """Custom root pydantic model describing expected list of all server users - credentials as stored in the credentials file.""" + """Custom root pydantic model. + + Describes expected list of all server users credentials as stored in + the credentials file. + + Attributes: + __root__ (List): Custom root consisting of the + list of all server users credentials. + """ __root__: list[UserCredentials] - def __add__(self, other): + def __add__(self, other): # noqa: D105 return ServerUsersCredentials.parse_obj(self.__root__ + other.__root__) - def __getitem__(self, item: int): + def __getitem__(self, item: int): # noqa: D105 return self.__root__[item] - def __len__(self): + def __len__(self): # noqa: D105 return len(self.__root__) - def __iter__(self): + def __iter__(self): # noqa: D105 return iter(self.__root__) @root_validator diff --git a/src/ralph/backends/storage/s3.py b/src/ralph/backends/storage/s3.py index 4f25bf0fa..78da16ef2 100644 --- a/src/ralph/backends/storage/s3.py +++ b/src/ralph/backends/storage/s3.py @@ -1,4 +1,4 @@ -"""S3 storage backend for Ralph""" +"""S3 storage backend for Ralph.""" import logging @@ -34,7 +34,6 @@ def __init__( bucket_name: str = s3_settings.BUCKET_NAME, ): """Instantiates the AWS S3 client.""" - self.access_key_id = access_key_id self.secret_access_key = secret_access_key self.session_token = session_token @@ -60,7 +59,6 @@ def __init__( def list(self, details=False, new=False): """Lists archives in the storage backend.""" - archives_to_skip = set() if new: archives_to_skip = set(self.get_command_history(self.name, "fetch")) @@ -89,12 +87,10 @@ def list(self, details=False, new=False): def url(self, name): """Gets `name` file absolute URL.""" - return f"{self.bucket_name}.s3.{self.default_region}.amazonaws.com/{name}" def read(self, name, chunk_size: int = 4096): """Reads `name` file and yields its content by chunks of a given size.""" - logger.debug("Getting archive: %s", name) try: @@ -124,7 +120,6 @@ def read(self, name, chunk_size: int = 4096): def write(self, stream, name, overwrite=False): """Writes data from `stream` to the `name` target.""" - if not overwrite and name in list(self.list()): msg = "%s already exists and overwrite is not allowed" logger.error(msg, name)