From 1dd8c393ef3b295a5b7eb4b81890df59ec181433 Mon Sep 17 00:00:00 2001 From: Maksym Yankin Date: Mon, 3 Feb 2025 10:06:19 +0200 Subject: [PATCH 1/3] [MIG] session_redis: Migration to 18.0 --- session_redis/__manifest__.py | 3 +- session_redis/http.py | 75 +++++++++++++++------------ session_redis/json_encoding.py | 25 ++++----- session_redis/session.py | 94 +++++++++++++++++----------------- session_redis/strtobool.py | 15 ++++-- 5 files changed, 113 insertions(+), 99 deletions(-) diff --git a/session_redis/__manifest__.py b/session_redis/__manifest__.py index a0deae6..45d32e2 100644 --- a/session_redis/__manifest__.py +++ b/session_redis/__manifest__.py @@ -14,6 +14,5 @@ "python": ["redis"], }, "website": "https://github.com/camptocamp/odoo-cloud-platform", - "data": [], - "installable": False, + "installable": True, } diff --git a/session_redis/http.py b/session_redis/http.py index f4e86d0..219abe2 100644 --- a/session_redis/http.py +++ b/session_redis/http.py @@ -3,6 +3,8 @@ import logging import os +from pathlib import Path +from typing import Optional from odoo import http from odoo.tools import config @@ -21,29 +23,32 @@ except ImportError: _logger.debug("Cannot 'import redis'.") -def is_true(strval): - return bool(strtobool(strval or "0".lower())) +def is_true(strval: Optional[str]) -> bool: + """Convert string value to boolean.""" + return bool(strtobool((strval or "0").lower())) -sentinel_host = os.environ.get("ODOO_SESSION_REDIS_SENTINEL_HOST") -sentinel_master_name = os.environ.get("ODOO_SESSION_REDIS_SENTINEL_MASTER_NAME") +# Retrieve Redis session configurations from environment variables +sentinel_host = os.getenv("ODOO_SESSION_REDIS_SENTINEL_HOST") +sentinel_master_name = os.getenv("ODOO_SESSION_REDIS_SENTINEL_MASTER_NAME") if sentinel_host and not sentinel_master_name: raise Exception( "ODOO_SESSION_REDIS_SENTINEL_MASTER_NAME must be defined " "when using session_redis" ) -sentinel_port = int(os.environ.get("ODOO_SESSION_REDIS_SENTINEL_PORT", 26379)) -host = os.environ.get("ODOO_SESSION_REDIS_HOST", "localhost") -port = int(os.environ.get("ODOO_SESSION_REDIS_PORT", 6379)) -prefix = os.environ.get("ODOO_SESSION_REDIS_PREFIX") -url = os.environ.get("ODOO_SESSION_REDIS_URL") -password = os.environ.get("ODOO_SESSION_REDIS_PASSWORD") -expiration = os.environ.get("ODOO_SESSION_REDIS_EXPIRATION") -anon_expiration = os.environ.get("ODOO_SESSION_REDIS_EXPIRATION_ANONYMOUS") +sentinel_port = int(os.getenv("ODOO_SESSION_REDIS_SENTINEL_PORT", 26379)) +host = os.getenv("ODOO_SESSION_REDIS_HOST", "localhost") +port = int(os.getenv("ODOO_SESSION_REDIS_PORT", 6379)) +prefix = os.getenv("ODOO_SESSION_REDIS_PREFIX") +url = os.getenv("ODOO_SESSION_REDIS_URL") +password = os.getenv("ODOO_SESSION_REDIS_PASSWORD") +expiration = os.getenv("ODOO_SESSION_REDIS_EXPIRATION") +anon_expiration = os.getenv("ODOO_SESSION_REDIS_EXPIRATION_ANONYMOUS") @lazy_property -def session_store(self): +def session_store(self) -> RedisSessionStore: + """Configure Redis session storage.""" if sentinel_host: sentinel = Sentinel([(sentinel_host, sentinel_port)], password=password) redis_client = sentinel.master_for(sentinel_master_name) @@ -61,30 +66,34 @@ def session_store(self): def purge_fs_sessions(path): - for fname in os.listdir(path): - path = os.path.join(path, fname) + """Remove old file-based sessions.""" + session_path = Path(path) + if not session_path.exists(): + _logger.warning(f"Session directory '{session_path}' does not exist.") + return + + for session_file in session_path.iterdir(): try: - os.unlink(path) - except OSError: - _logger.warning("OS Error during purge of redis sessions.") + session_file.unlink() + _logger.debug(f"Deleted session file: {session_file}") + except PermissionError: + _logger.warning( + f"Permission denied while deleting session file: {session_file}" + ) + except OSError as e: + _logger.warning(f"Error deleting session file {session_file}: {str(e)}") -if is_true(os.environ.get("ODOO_SESSION_REDIS")): +if is_true(os.getenv("ODOO_SESSION_REDIS")): + storage_info = f"Redis with prefix '{prefix}' on " if sentinel_host: - _logger.debug( - "HTTP sessions stored in Redis with prefix '%s'. " - "Using Sentinel on %s:%s", - prefix or "", - sentinel_host, - sentinel_port, - ) + storage_info += f"Sentinel {sentinel_host}:{sentinel_port}" else: - _logger.debug( - "HTTP sessions stored in Redis with prefix '%s' on " "%s:%s", - prefix or "", - host, - port, - ) + storage_info += f"{host}:{port}" + + _logger.debug("HTTP sessions stored in %s.", storage_info) + http.Application.session_store = session_store - # clean the existing sessions on the file system + + # Clean existing sessions stored in the file system purge_fs_sessions(config.session_dir) diff --git a/session_redis/json_encoding.py b/session_redis/json_encoding.py index 86992b2..effd0c3 100644 --- a/session_redis/json_encoding.py +++ b/session_redis/json_encoding.py @@ -3,8 +3,9 @@ import json from datetime import date, datetime +from typing import Any -import dateutil +import dateutil.parser class SessionEncoder(json.JSONEncoder): @@ -13,30 +14,30 @@ class SessionEncoder(json.JSONEncoder): So that we can later recompose them if they were stored in the session """ - def default(self, obj): + def default(self, obj: Any) -> Any: if isinstance(obj, datetime): return {"_type": "datetime_isoformat", "value": obj.isoformat()} - elif isinstance(obj, date): + if isinstance(obj, date): return {"_type": "date_isoformat", "value": obj.isoformat()} - elif isinstance(obj, set): - return {"_type": "set", "value": tuple(obj)} - return json.JSONEncoder.default(self, obj) + if isinstance(obj, set): + return {"_type": "set", "value": tuple(sorted(obj))} + return super().default(obj) class SessionDecoder(json.JSONDecoder): """Decode json, recomposing recordsets and date/datetime""" - def __init__(self, *args, **kwargs): + def __init__(self, *args, **kwargs) -> None: super().__init__(*args, object_hook=self.object_hook, **kwargs) - def object_hook(self, obj): + def object_hook(self, obj: dict[str, Any]) -> Any: + """Convert serialized data back into its original Python type.""" if "_type" not in obj: return obj - type_ = obj["_type"] - if type_ == "datetime_isoformat": + if obj["_type"] == "datetime_isoformat": return dateutil.parser.parse(obj["value"]) - elif type_ == "date_isoformat": + if obj["_type"] == "date_isoformat": return dateutil.parser.parse(obj["value"]).date() - elif type_ == "set": + if obj["_type"] == "set": return set(obj["value"]) return obj diff --git a/session_redis/session.py b/session_redis/session.py index ec19b7c..2851829 100644 --- a/session_redis/session.py +++ b/session_redis/session.py @@ -3,6 +3,7 @@ import json import logging +from typing import Optional from odoo.service import security from odoo.tools._vendor.sessions import SessionStore @@ -24,46 +25,44 @@ class RedisSessionStore(SessionStore): self, redis, session_class=None, - prefix="", - expiration=None, - anon_expiration=None, - ): + prefix: str = "", + expiration: Optional[int] = None, + anon_expiration: Optional[int] = None, + ) -> None: super().__init__(session_class=session_class) self.redis = redis - if expiration is None: - self.expiration = DEFAULT_SESSION_TIMEOUT - else: - self.expiration = expiration - if anon_expiration is None: - self.anon_expiration = DEFAULT_SESSION_TIMEOUT_ANONYMOUS - else: - self.anon_expiration = anon_expiration - self.prefix = "session:" - if prefix: - self.prefix = f"{self.prefix}:{prefix}:" + self.expiration = ( + expiration if expiration is not None else DEFAULT_SESSION_TIMEOUT + ) + self.anon_expiration = ( + anon_expiration + if anon_expiration is not None + else DEFAULT_SESSION_TIMEOUT_ANONYMOUS + ) + self.prefix = f"session:{prefix}:" if prefix else "session:" - def build_key(self, sid): + def build_key(self, sid: str) -> str: + """Build the Redis key for a session ID.""" return f"{self.prefix}{sid}" - def save(self, session): + def save(self, session) -> Optional[bool]: + """Save session data in Redis with an expiration time.""" key = self.build_key(session.sid) # allow to set a custom expiration for a session # such as a very short one for monitoring requests - if session.uid: - expiration = session.expiration or self.expiration - else: - expiration = session.expiration or self.anon_expiration + expiration = session.expiration or ( + self.expiration if session.uid else self.anon_expiration + ) + if _logger.isEnabledFor(logging.DEBUG): - if session.uid: - user_msg = f"user '{session.login}' (id: {session.uid})" - else: - user_msg = "anonymous user" + user_msg = ( + f"user '{session.login}' (id: {session.uid})" + if session.uid + else "anonymous user" + ) _logger.debug( - "saving session with key '%s' and " "expiration of %s seconds for %s", - key, - expiration, - user_msg, + f"Saving session '{key}' with expiration {expiration} seconds for {user_msg}" ) data = json.dumps(dict(session), cls=json_encoding.SessionEncoder).encode( @@ -71,17 +70,19 @@ class RedisSessionStore(SessionStore): ) if self.redis.set(key, data): return self.redis.expire(key, expiration) + return None - def delete(self, session): + def delete(self, session) -> int: + """Delete a session from Redis.""" key = self.build_key(session.sid) - _logger.debug("deleting session with key %s", key) + _logger.debug(f"Deleting session with '{key}'") return self.redis.delete(key) - def get(self, sid): + def get(self, sid: str): + """Retrieve a session from Redis, or return a new one if not found.""" if not self.is_valid_key(sid): _logger.debug( - "session with invalid sid '%s' has been asked, " "returning a new one", - sid, + f"Invalid session ID '{sid}' requested, returning a new session." ) return self.new() @@ -89,28 +90,27 @@ class RedisSessionStore(SessionStore): saved = self.redis.get(key) if not saved: _logger.debug( - "session with non-existent key '%s' has been asked, " - "returning a new one", - key, + f"Non-existent session '{key}' requested, returning a new session." ) return self.new() + try: data = json.loads(saved.decode("utf-8"), cls=json_encoding.SessionDecoder) - except ValueError: - _logger.debug( - "session for key '%s' has been asked but its json " - "content could not be read, it has been reset", - key, - ) + except (ValueError, json.JSONDecodeError): + _logger.warning(f"Corrupt session data for key '{key}', resetting session.") data = {} + return self.session_class(data, sid, False) - def list(self): - keys = self.redis.keys("%s*" % self.prefix) + def list(self) -> list[str]: + """List all session keys in Redis.""" + # More efficient scanning + keys = [key for key in self.redis.scan_iter(f"{self.prefix}*")] _logger.debug("a listing redis keys has been called") - return [key[len(self.prefix) :] for key in keys] + return [key.decode("utf-8")[len(self.prefix) :] for key in keys] - def rotate(self, session, env): + def rotate(self, session, env) -> None: + """Rotate session ID and regenerate session token if user is logged in.""" self.delete(session) session.sid = self.generate_key() if session.uid and env: diff --git a/session_redis/strtobool.py b/session_redis/strtobool.py index 1a7ad55..e112061 100644 --- a/session_redis/strtobool.py +++ b/session_redis/strtobool.py @@ -1,3 +1,5 @@ +from typing import Union + _MAP = { "y": True, "yes": True, @@ -14,8 +16,11 @@ _MAP = { } -def strtobool(value): - try: - return _MAP[str(value).lower()] - except KeyError as error: - raise ValueError(f'"{value}" is not a valid bool value') from error +def strtobool(value: Union[str, int]) -> bool: + """Convert a string or integer representation to a boolean value.""" + result = _MAP.get(str(value).strip().lower()) + + if result is None: + raise ValueError(f"Invalid boolean value: {repr(value)}") + + return result From 2428189308cd1b94f87b406783f128615154a7df Mon Sep 17 00:00:00 2001 From: Maksym Yankin Date: Mon, 3 Feb 2025 10:12:25 +0200 Subject: [PATCH 2/3] Revert feat: remove unmaintened modules (#443) --- .pre-commit-config.yaml | 2 +- attachment_s3/README.rst | 58 ++++++ attachment_s3/__init__.py | 1 + attachment_s3/__manifest__.py | 19 ++ attachment_s3/models/__init__.py | 1 + attachment_s3/models/ir_attachment.py | 177 ++++++++++++++++++ attachment_s3/s3uri.py | 20 ++ test_base_fileurl_field/models/res_partner.py | 1 - test_base_fileurl_field/models/res_users.py | 1 - 9 files changed, 277 insertions(+), 3 deletions(-) create mode 100644 attachment_s3/README.rst create mode 100644 attachment_s3/__init__.py create mode 100644 attachment_s3/__manifest__.py create mode 100644 attachment_s3/models/__init__.py create mode 100644 attachment_s3/models/ir_attachment.py create mode 100644 attachment_s3/s3uri.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index df9807c..106246e 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,10 +1,10 @@ exclude: | (?x) # NOT INSTALLABLE ADDONS + ^attachment_s3/| ^base_fileurl_field/| ^monitoring_log_requests/| ^monitoring_statsd/| - ^test_base_fileurl_field/| # END NOT INSTALLABLE ADDONS # Files and folders generated by bots, to avoid loops ^setup/|/static/description/index\.html$| diff --git a/attachment_s3/README.rst b/attachment_s3/README.rst new file mode 100644 index 0000000..0d03699 --- /dev/null +++ b/attachment_s3/README.rst @@ -0,0 +1,58 @@ +Attachments on S3 storage +========================= + +This addon allows to store the attachments (documents and assets) on S3 or any +other S3-compatible Object Storage. + +Configuration +------------- + +Activate S3 storage: + +* Create or set the system parameter with the key ``ir_attachment.location`` + and the value in the form ``s3``. + +Configure accesses with environment variables: + +* ``AWS_HOST`` (not required if using AWS services) +* ``AWS_REGION`` (required if using AWS services) +* ``AWS_ACCESS_KEY_ID`` +* ``AWS_SECRET_ACCESS_KEY`` +* ``AWS_BUCKETNAME`` (optional {db} placeholder) + +Read-only mode: + +The bucket and the file key are stored in the attachment. So if you change the +``AWS_BUCKETNAME`` or the ``ir_attachment.location``, the existing attachments +will still be read on their former bucket. But as soon as they are written over +or new attachments are created, they will be created on the new bucket or on +the other location (db or filesystem). This is a convenient way to be able to +read the production attachments on a replication (since you have the +credentials) without any risk to alter the production data. + +This addon must be added in the server wide addons with (``--load`` option): + +``--load=web,attachment_s3`` + +The System Parameter ``ir_attachment.storage.force.database`` can be customized to +force storage of files in the database. See the documentation of the module +``base_attachment_object_storage``. + +Multi-tenancy +------------- + +Use the `{db}` placeholder to handle multi-tenancy. + +On instances that hold multiple databases, it's preferable to have one bucket per database. + +To handle this, you can insert the `{db}` placeholder in your bucket name variable ``AWS_BUCKETNAME``. +It will be replaced by the database name. +This will give you a unique bucketname per database. + + +Limitations +----------- + +* You need to call ``env['ir.attachment'].force_storage()`` after + having changed the ``ir_attachment.location`` configuration in order to + migrate the existing attachments to S3. diff --git a/attachment_s3/__init__.py b/attachment_s3/__init__.py new file mode 100644 index 0000000..0650744 --- /dev/null +++ b/attachment_s3/__init__.py @@ -0,0 +1 @@ +from . import models diff --git a/attachment_s3/__manifest__.py b/attachment_s3/__manifest__.py new file mode 100644 index 0000000..7958e62 --- /dev/null +++ b/attachment_s3/__manifest__.py @@ -0,0 +1,19 @@ +# Copyright 2016-2021 Camptocamp SA +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html) + + +{ + "name": "Attachments on S3 storage", + "summary": "Store assets and attachments on a S3 compatible object storage", + "version": "15.0.1.0.0", + "author": "Camptocamp,Odoo Community Association (OCA)", + "license": "AGPL-3", + "category": "Knowledge Management", + "depends": ["base", "base_attachment_object_storage"], + "external_dependencies": { + "python": ["boto3"], + }, + "website": "https://github.com/camptocamp/odoo-cloud-platform", + "data": [], + "installable": False, +} diff --git a/attachment_s3/models/__init__.py b/attachment_s3/models/__init__.py new file mode 100644 index 0000000..aaf38a1 --- /dev/null +++ b/attachment_s3/models/__init__.py @@ -0,0 +1 @@ +from . import ir_attachment diff --git a/attachment_s3/models/ir_attachment.py b/attachment_s3/models/ir_attachment.py new file mode 100644 index 0000000..d18c0e2 --- /dev/null +++ b/attachment_s3/models/ir_attachment.py @@ -0,0 +1,177 @@ +# Copyright 2016-2019 Camptocamp SA +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html) + + +import io +import logging +import os +from urllib.parse import urlsplit + +from odoo import _, api, exceptions, models + +from ..s3uri import S3Uri + +_logger = logging.getLogger(__name__) + +try: + import boto3 + from botocore.exceptions import ClientError, EndpointConnectionError +except ImportError: + boto3 = None # noqa + ClientError = None # noqa + EndpointConnectionError = None # noqa + _logger.debug("Cannot 'import boto3'.") + + +class IrAttachment(models.Model): + _inherit = "ir.attachment" + + def _get_stores(self): + return ["s3"] + super()._get_stores() + + @api.model + def _get_s3_bucket(self, name=None): + """Connect to S3 and return the bucket + + The following environment variables can be set: + * ``AWS_HOST`` + * ``AWS_REGION`` + * ``AWS_ACCESS_KEY_ID`` + * ``AWS_SECRET_ACCESS_KEY`` + * ``AWS_BUCKETNAME`` + + If a name is provided, we'll read this bucket, otherwise, the bucket + from the environment variable ``AWS_BUCKETNAME`` will be read. + + """ + host = os.environ.get("AWS_HOST") + + # Ensure host is prefixed with a scheme (use https as default) + if host and not urlsplit(host).scheme: + host = "https://%s" % host + + region_name = os.environ.get("AWS_REGION") + access_key = os.environ.get("AWS_ACCESS_KEY_ID") + secret_key = os.environ.get("AWS_SECRET_ACCESS_KEY") + bucket_name = name or os.environ.get("AWS_BUCKETNAME") + # replaces {db} by the database name to handle multi-tenancy + bucket_name = bucket_name.format(db=self.env.cr.dbname) + + params = { + "aws_access_key_id": access_key, + "aws_secret_access_key": secret_key, + } + if host: + params["endpoint_url"] = host + if region_name: + params["region_name"] = region_name + if not (access_key and secret_key and bucket_name): + msg = _( + "If you want to read from the %(bucket_name)s S3 bucket, the following " + "environment variables must be set:\n" + "* AWS_ACCESS_KEY_ID\n" + "* AWS_SECRET_ACCESS_KEY\n" + "If you want to write in the %(bucket_name)s S3 bucket, this variable " + "must be set as well:\n" + "* AWS_BUCKETNAME\n" + "Optionally, the S3 host can be changed with:\n" + "* AWS_HOST\n" + ).format(bucket_name=bucket_name) + + raise exceptions.UserError(msg) + # try: + s3 = boto3.resource("s3", **params) + bucket = s3.Bucket(bucket_name) + exists = True + try: + s3.meta.client.head_bucket(Bucket=bucket_name) + except ClientError as e: + # If a client error is thrown, then check that it was a 404 error. + # If it was a 404 error, then the bucket does not exist. + error_code = e.response["Error"]["Code"] + if error_code == "404": + exists = False + except EndpointConnectionError as error: + # log verbose error from s3, return short message for user + msg = _logger.exception("Error during connection on S3") + raise exceptions.UserError(str(error)) from None + + if not exists: + if not region_name: + bucket = s3.create_bucket(Bucket=bucket_name) + else: + bucket = s3.create_bucket( + Bucket=bucket_name, + CreateBucketConfiguration={"LocationConstraint": region_name}, + ) + return bucket + + @api.model + def _store_file_read(self, fname): + if fname.startswith("s3://"): + s3uri = S3Uri(fname) + try: + bucket = self._get_s3_bucket(name=s3uri.bucket()) + except exceptions.UserError: + _logger.exception( + "error reading attachment '%s' from object storage", fname + ) + return "" + try: + key = s3uri.item() + bucket.meta.client.head_object(Bucket=bucket.name, Key=key) + with io.BytesIO() as res: + bucket.download_fileobj(key, res) + res.seek(0) + read = res.read() + except ClientError: + read = "" + _logger.info("attachment '%s' missing on object storage", fname) + return read + else: + return super()._store_file_read(fname) + + @api.model + def _store_file_write(self, key, bin_data): + location = self.env.context.get("storage_location") or self._storage() + if location == "s3": + bucket = self._get_s3_bucket() + obj = bucket.Object(key=key) + with io.BytesIO() as file: + file.write(bin_data) + file.seek(0) + filename = "s3://%s/%s" % (bucket.name, key) + try: + obj.upload_fileobj(file) + except ClientError as error: + # log verbose error from s3, return short message for user + _logger.exception("Error during storage of the file %s" % filename) + raise exceptions.UserError( + _("The file could not be stored: %s") % str(error) + ) from None + else: + _super = super() + filename = _super._store_file_write(key, bin_data) + return filename + + @api.model + def _store_file_delete(self, fname): + if fname.startswith("s3://"): + s3uri = S3Uri(fname) + bucket_name = s3uri.bucket() + item_name = s3uri.item() + # delete the file only if it is on the current configured bucket + # otherwise, we might delete files used on a different environment + if bucket_name == os.environ.get("AWS_BUCKETNAME"): + bucket = self._get_s3_bucket() + obj = bucket.Object(key=item_name) + try: + bucket.meta.client.head_object(Bucket=bucket.name, Key=item_name) + obj.delete() + _logger.info("file %s deleted on the object storage" % (fname,)) + except ClientError: + # log verbose error from s3, return short message for + # user + _logger.exception("Error during deletion of the file %s" % fname) + else: + return super()._store_file_delete(fname) diff --git a/attachment_s3/s3uri.py b/attachment_s3/s3uri.py new file mode 100644 index 0000000..5bbe11a --- /dev/null +++ b/attachment_s3/s3uri.py @@ -0,0 +1,20 @@ +# Copyright 2016-2019 Camptocamp SA +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html) + +import re + + +class S3Uri: + _url_re = re.compile("^s3:///*([^/]*)/?(.*)", re.IGNORECASE | re.UNICODE) + + def __init__(self, uri): + match = self._url_re.match(uri) + if not match: + raise ValueError("%s: is not a valid S3 URI" % (uri,)) + self._bucket, self._item = match.groups() + + def bucket(self): + return self._bucket + + def item(self): + return self._item diff --git a/test_base_fileurl_field/models/res_partner.py b/test_base_fileurl_field/models/res_partner.py index 48ca0a4..3842f58 100644 --- a/test_base_fileurl_field/models/res_partner.py +++ b/test_base_fileurl_field/models/res_partner.py @@ -5,7 +5,6 @@ from odoo.exceptions import ValidationError class ResPartner(models.Model): - _inherit = "res.partner" name = fields.Char() diff --git a/test_base_fileurl_field/models/res_users.py b/test_base_fileurl_field/models/res_users.py index c8bc324..58e21c4 100644 --- a/test_base_fileurl_field/models/res_users.py +++ b/test_base_fileurl_field/models/res_users.py @@ -4,7 +4,6 @@ from odoo import fields, models class ResUsers(models.Model): - _inherit = "res.users" partner_url_file = fields.FileURL(related="partner_id.url_file") From 987c303064283c103c767d5e7d6ba0d094c8aa24 Mon Sep 17 00:00:00 2001 From: Maksym Yankin Date: Thu, 30 Jan 2025 16:43:32 +0200 Subject: [PATCH 3/3] [MIG] attachment_s3: Migration to 18.0 --- .pre-commit-config.yaml | 9 +- attachment_s3/__manifest__.py | 5 +- attachment_s3/models/ir_attachment.py | 148 +++++++++++++------------- attachment_s3/pyproject.toml | 3 + attachment_s3/s3uri.py | 18 ++-- requirements.txt | 5 +- 6 files changed, 100 insertions(+), 88 deletions(-) create mode 100644 attachment_s3/pyproject.toml diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 106246e..f9b8a12 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,10 +1,17 @@ exclude: | (?x) # NOT INSTALLABLE ADDONS - ^attachment_s3/| + ^attachment_azure/| + ^base_attachment_object_storage/| ^base_fileurl_field/| + ^cloud_platform/| + ^cloud_platform_azure/| + ^logging_json/| ^monitoring_log_requests/| + ^monitoring_prometheus/| ^monitoring_statsd/| + ^monitoring_status/| + ^test_base_fileurl_field/| # END NOT INSTALLABLE ADDONS # Files and folders generated by bots, to avoid loops ^setup/|/static/description/index\.html$| diff --git a/attachment_s3/__manifest__.py b/attachment_s3/__manifest__.py index 7958e62..614c46c 100644 --- a/attachment_s3/__manifest__.py +++ b/attachment_s3/__manifest__.py @@ -5,7 +5,7 @@ { "name": "Attachments on S3 storage", "summary": "Store assets and attachments on a S3 compatible object storage", - "version": "15.0.1.0.0", + "version": "18.0.1.0.0", "author": "Camptocamp,Odoo Community Association (OCA)", "license": "AGPL-3", "category": "Knowledge Management", @@ -14,6 +14,5 @@ "python": ["boto3"], }, "website": "https://github.com/camptocamp/odoo-cloud-platform", - "data": [], - "installable": False, + "installable": True, } diff --git a/attachment_s3/models/ir_attachment.py b/attachment_s3/models/ir_attachment.py index d18c0e2..17caeb1 100644 --- a/attachment_s3/models/ir_attachment.py +++ b/attachment_s3/models/ir_attachment.py @@ -6,8 +6,10 @@ import io import logging import os from urllib.parse import urlsplit +from typing import Optional -from odoo import _, api, exceptions, models +from odoo import api, models +from odoo.exceptions import UserError from ..s3uri import S3Uri @@ -30,7 +32,7 @@ class IrAttachment(models.Model): return ["s3"] + super()._get_stores() @api.model - def _get_s3_bucket(self, name=None): + def _get_s3_bucket(self, name: Optional[str] = None): """Connect to S3 and return the bucket The following environment variables can be set: @@ -44,29 +46,23 @@ class IrAttachment(models.Model): from the environment variable ``AWS_BUCKETNAME`` will be read. """ - host = os.environ.get("AWS_HOST") + if not boto3: + raise UserError(self.env._("boto3 library is required for S3 integration.")) - # Ensure host is prefixed with a scheme (use https as default) + host = os.getenv("AWS_HOST").strip() + + # Ensure `host`` is prefixed with a scheme (use https as default) if host and not urlsplit(host).scheme: - host = "https://%s" % host + host = f"https://{host}" - region_name = os.environ.get("AWS_REGION") - access_key = os.environ.get("AWS_ACCESS_KEY_ID") - secret_key = os.environ.get("AWS_SECRET_ACCESS_KEY") - bucket_name = name or os.environ.get("AWS_BUCKETNAME") - # replaces {db} by the database name to handle multi-tenancy - bucket_name = bucket_name.format(db=self.env.cr.dbname) + region_name = os.getenv("AWS_REGION") + access_key = os.getenv("AWS_ACCESS_KEY_ID") + secret_key = os.getenv("AWS_SECRET_ACCESS_KEY") + bucket_name = (name or os.getenv("AWS_BUCKETNAME", "")).format(db=self.env.cr.dbname) - params = { - "aws_access_key_id": access_key, - "aws_secret_access_key": secret_key, - } - if host: - params["endpoint_url"] = host - if region_name: - params["region_name"] = region_name - if not (access_key and secret_key and bucket_name): - msg = _( + if not all([access_key, secret_key, bucket_name]): + msg = self.env._( + "Missing AWS credentials." "If you want to read from the %(bucket_name)s S3 bucket, the following " "environment variables must be set:\n" "* AWS_ACCESS_KEY_ID\n" @@ -77,101 +73,107 @@ class IrAttachment(models.Model): "Optionally, the S3 host can be changed with:\n" "* AWS_HOST\n" ).format(bucket_name=bucket_name) + raise UserError(msg) - raise exceptions.UserError(msg) - # try: - s3 = boto3.resource("s3", **params) + s3_params = { + "aws_access_key_id": access_key, + "aws_secret_access_key": secret_key, + } + if host: + s3_params["endpoint_url"] = host + if region_name: + s3_params["region_name"] = region_name + + s3 = boto3.resource("s3", **s3_params) bucket = s3.Bucket(bucket_name) - exists = True + try: s3.meta.client.head_bucket(Bucket=bucket_name) except ClientError as e: # If a client error is thrown, then check that it was a 404 error. # If it was a 404 error, then the bucket does not exist. - error_code = e.response["Error"]["Code"] - if error_code == "404": - exists = False + if e.response.get("Error", {}).get("Code") == "404": + _logger.warning(f"S3 bucket '{bucket_name}' does not exist.") + return self._create_s3_bucket(s3, bucket_name, region_name) + raise UserError(f"Failed to connect to S3 bucket: {str(e)}") from None except EndpointConnectionError as error: # log verbose error from s3, return short message for user - msg = _logger.exception("Error during connection on S3") - raise exceptions.UserError(str(error)) from None + _logger.exception("Error during S3 connection.") + raise UserError(str(error)) from None - if not exists: - if not region_name: - bucket = s3.create_bucket(Bucket=bucket_name) - else: - bucket = s3.create_bucket( - Bucket=bucket_name, - CreateBucketConfiguration={"LocationConstraint": region_name}, - ) return bucket + def _create_s3_bucket(self, s3, bucket_name: str, region_name: Optional[str]): + """Create an S3 bucket if it does not exist.""" + params = {"Bucket": bucket_name} + if region_name: + params["CreateBucketConfiguration"] = {"LocationConstraint": region_name} + + try: + return s3.create_bucket(**params) + except ClientError as e: + _logger.exception(f"Failed to create S3 bucket '{bucket_name}'") + raise UserError(f"Bucket creation failed: {str(e)}") from None + @api.model - def _store_file_read(self, fname): + def _store_file_read(self, fname: str): if fname.startswith("s3://"): s3uri = S3Uri(fname) try: - bucket = self._get_s3_bucket(name=s3uri.bucket()) - except exceptions.UserError: - _logger.exception( - "error reading attachment '%s' from object storage", fname - ) + bucket = self._get_s3_bucket(name=s3uri.bucket) + except UserError: + _logger.exception(f"Error reading attachment '{fname}' from S3.") return "" + + key = s3uri.item() try: - key = s3uri.item() bucket.meta.client.head_object(Bucket=bucket.name, Key=key) with io.BytesIO() as res: bucket.download_fileobj(key, res) res.seek(0) - read = res.read() + return res.read() except ClientError: - read = "" - _logger.info("attachment '%s' missing on object storage", fname) - return read - else: - return super()._store_file_read(fname) + _logger.info(f"Attachment '{fname}' missing on S3.") + return "" + return super()._store_file_read(fname) @api.model - def _store_file_write(self, key, bin_data): + def _store_file_write(self, key: str, bin_data: bytes) -> str: location = self.env.context.get("storage_location") or self._storage() if location == "s3": bucket = self._get_s3_bucket() obj = bucket.Object(key=key) - with io.BytesIO() as file: - file.write(bin_data) - file.seek(0) - filename = "s3://%s/%s" % (bucket.name, key) - try: + filename = f"s3://{bucket.name}/{key}" + + try: + with io.BytesIO(bin_data) as file: obj.upload_fileobj(file) - except ClientError as error: - # log verbose error from s3, return short message for user - _logger.exception("Error during storage of the file %s" % filename) - raise exceptions.UserError( - _("The file could not be stored: %s") % str(error) - ) from None - else: - _super = super() - filename = _super._store_file_write(key, bin_data) - return filename + except ClientError as error: + # log verbose error from s3, return short message for user + _logger.exception(f"Error storing file {filename} on S3.") + raise UserError(self.env._("The file could not be stored: %s") % str(error)) from None + + return filename + return super()._store_file_write(key, bin_data) @api.model - def _store_file_delete(self, fname): + def _store_file_delete(self, fname: str): if fname.startswith("s3://"): s3uri = S3Uri(fname) bucket_name = s3uri.bucket() item_name = s3uri.item() # delete the file only if it is on the current configured bucket # otherwise, we might delete files used on a different environment - if bucket_name == os.environ.get("AWS_BUCKETNAME"): + if bucket_name == os.getenv("AWS_BUCKETNAME"): bucket = self._get_s3_bucket() obj = bucket.Object(key=item_name) + try: bucket.meta.client.head_object(Bucket=bucket.name, Key=item_name) obj.delete() - _logger.info("file %s deleted on the object storage" % (fname,)) + _logger.info(f"File {fname} deleted from S3.") except ClientError: - # log verbose error from s3, return short message for - # user - _logger.exception("Error during deletion of the file %s" % fname) + # log verbose error from s3, return short message for user + _logger.exception(f"Error deleting file {fname} from S3.") else: return super()._store_file_delete(fname) diff --git a/attachment_s3/pyproject.toml b/attachment_s3/pyproject.toml new file mode 100644 index 0000000..4231d0c --- /dev/null +++ b/attachment_s3/pyproject.toml @@ -0,0 +1,3 @@ +[build-system] +requires = ["whool"] +build-backend = "whool.buildapi" diff --git a/attachment_s3/s3uri.py b/attachment_s3/s3uri.py index 5bbe11a..40b3eb8 100644 --- a/attachment_s3/s3uri.py +++ b/attachment_s3/s3uri.py @@ -2,19 +2,23 @@ # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html) import re - +from typing import Optional class S3Uri: - _url_re = re.compile("^s3:///*([^/]*)/?(.*)", re.IGNORECASE | re.UNICODE) + _url_re = re.compile(r"^s3://+([^/]+)/?(.*)", re.IGNORECASE | re.UNICODE) - def __init__(self, uri): + def __init__(self, uri: str) -> None: match = self._url_re.match(uri) if not match: - raise ValueError("%s: is not a valid S3 URI" % (uri,)) - self._bucket, self._item = match.groups() + raise ValueError(f"{uri}: is not a valid S3 URI") + + self._bucket: str = match.group(1) + self._item: Optional[str] = match.group(2) if match.group(2) else None - def bucket(self): + @property + def bucket(self) -> str: return self._bucket - def item(self): + @property + def item(self) -> Optional[str]: return self._item diff --git a/requirements.txt b/requirements.txt index d99c37b..9af76ab 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,3 @@ # generated from manifests external_dependencies -azure-identity -azure-storage-blob -prometheus_client -python-json-logger +boto3 redis