Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement new attachment handlers #3

Draft
wants to merge 13 commits into
base: develop
Choose a base branch
from

Conversation

khakers
Copy link
Owner

@khakers khakers commented Nov 11, 2023

Adds an attachment handler interface and 2 new clients which implement it.

MongoAttachmentClient: stores attachments as binary data in a mongoDB document in the attachments collection
S3AttachmentClient: Stores attachments as S3 objects in an S3 service of the users choice.

…ment_data field"

This change ends up being *mostly* backwards compatible since we still store the url, though this will be basically useless in the future.

Attachment size is currently hardcoded to 8MB, in the future this should be able to be set by users.
@khakers khakers force-pushed the feature/in-database-image-store branch from af0f9ce to da3b814 Compare January 4, 2024 07:29
Moves the attachment handling code to its own class and creates a new abstract class to allow different handlers in the future
Uses the MinIO S3 library
@khakers khakers changed the title Refactor mongodb client append_log store images internally in "attachment_data field" Implement new attachment handlers Jan 6, 2024
@khakers
Copy link
Owner Author

khakers commented Jan 6, 2024

download_attachment & delete_attachment are currently stubs in bot implementations, and I'm not sure they're necessary for the API at the moment

@khakers

This comment was marked as outdated.

A list of dictionaries containing information about the uploaded attachments.
"""
attachments = []
tags = Tags.new_object_tags()
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should include tag for bot ID to help people that are sharing a bucket between multiple bots that they host

core/attachments/MongoAttachmentClient.py Outdated Show resolved Hide resolved
@khakers
Copy link
Owner Author

khakers commented Jan 14, 2024

As a note for use, Cloudflare and Backblaze both offer 10GB free on their S3 cloud storage systems (R2 & B2, respectively). Cloudflare has no egress fees and Backblaze has some only if you don't route through a CDN and download more than you store.

@khakers khakers marked this pull request as draft February 10, 2024 04:28
…ia config

The max size isn't automatically updated when the config is changed. The size is should always be retrieved from the attachment handler and not the config, as the attachment handler may have additional constraints
@khakers khakers added the enhancement New feature or request label Feb 10, 2024
@khakers khakers self-assigned this Feb 10, 2024
@khakers
Copy link
Owner Author

khakers commented Feb 10, 2024

Should be pretty close to final at this point.
I've implemented size checks so messages will fail to send if they have attachments that are too large.
The maximum attachment size doesn't get updated when you change the value via modmail without restarting.
The bot doesn't currently have a way to delete attachments, but that can be handled by a policy in your s3 server or manually for someone using mongodb storage.

Still iffy on the current s3 mongodb data format and if I should rename the mongodb datastore config value to "mongodb" from "internal" to reflect what it actually is.

@khakers
Copy link
Owner Author

khakers commented Feb 10, 2024

This seems to be a decent bucket access policy
(allows only getting objects, no directory listing)

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Principal": {
                "AWS": [
                    "*"
                ]
            },
            "Action": [
                "s3:GetObject"
            ],
            "Resource": [
                "arn:aws:s3:::modmail-attachments/*"
            ]
        }
    ]
}

@khakers
Copy link
Owner Author

khakers commented Mar 25, 2024

Add noop attachment handler so the option to not save attachments exists.

@khakers
Copy link
Owner Author

khakers commented Mar 25, 2024

Ability to set retention period for mongodb storage may also be desired

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant