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

adding support for publishing bundle to Minio Object Storage (#5395) #5757

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Dimss
Copy link

@Dimss Dimss commented Jan 3, 2025

As a part of #5395 this PR add support to Minio S3 storage.
The PR does not meant to be merge to main yet.
I do need someone look on the PR and provide initial feedback.
I wanna make sure this functionality is still needed and overall logic does make sense.
if all good, I'll continue to work on it, add tests, etc...

@sorindumitru
Copy link
Collaborator

Thanks for the contribution @Dimss! I'll try to take a look at it this week.

@sorindumitru
Copy link
Collaborator

@Dimss Minio is API compatible with AWS S3 API, so it should be possible to use the existing awss3 bundle publisher here. Have you tried using that? Are there any issues or missing functionality?

@Dimss
Copy link
Author

Dimss commented Jan 12, 2025

@sorindumitru awss3 might be working with Minio.
However, by default when user using awss3 bundle publisher he doesn't have an option to configure what's Minio calls Endpoint URL. We possibly can extend the awss3 configuration with Endpoint URL, but then perhaps it will be not awss3 but generic s3 bundle publisher.
In addition, Minio allow insecure connections to buckets (might be useful for dev/test envs)

I think the main question here, should it be a generic S3 publisher or publisher for each provider with it's backend implementation.

@kfox1111
Copy link
Contributor

Thank you so much for working on this! :)

Its using the minio client, but I think it could work against any s3 server as coded?

There are a lot of s3 compatible implementations out there (many storage vendors support it) and they all work with the aws s3 client I believe, or one of the implementations like minio's client.

Having one generic s3 driver I think would be preferable to one per storage implementation, as they all essentially work the same.

@sorindumitru
Copy link
Collaborator

Having separate implementations for different platforms may make sense eventually, but I feel that for now it only adds maintenance burden.

If someone does end up needing something from this plugin where it makes sense to have it use platform specific APIs, such as platform specific authentication methods, we should look into splitting off that part into a separate plugin. @Dimss, do you need anything from an S3 publisher that is Minio specific?

For now, it's probably easiest to just add support for specifying an optional endpoint to the awss3 plugin.

@Dimss
Copy link
Author

Dimss commented Jan 19, 2025

@sorindumitru ok by me.
So then I'll extend existing awss3 plugin with extra optional parameter endpoint and insecure.
Do you agree?

@kfox1111
Copy link
Contributor

Just endpoint for now I think? if you need to load in a custom ca for talking to minio, it can be done via the system trust bundle. If its actually insecure, not sure there is any advantage to funneling it through the s3 service rather then just using the agents insecure_bootstrap flag?

@Dimss
Copy link
Author

Dimss commented Jan 19, 2025

@kfox1111 agree. then I'll just add one optional endpoint param into existing awss3 plugin

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

Successfully merging this pull request may close these issues.

4 participants