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

Allow set S3ForcePathStyle #906

Merged
merged 4 commits into from
Oct 27, 2023

Conversation

warjiang
Copy link
Contributor

Chartmuseum is a awssome tool to help us maintain and manage internal charts 👍 .

At the same time, chartmuseum provides support for cloud vendors' s3 storage. The keypoint is the chartmuseum/storage library.

Amazon S3 storage is the de facto object storage standard, and many cloud vendors provide S3-compatible object storage interfaces. As the storage interface of chartmuseum, the chartmuseum/storage library is compatible with s3 storage. When initializing the s3 client internally, if a non-empty endpoint is passed in, S3ForcePathStyle will be set to true.

service := s3.New(session.New(), &aws.Config{
    HTTPClient:       client,
    Region:           aws.String(region),
    Endpoint:         aws.String(endpoint),
    DisableSSL:       aws.Bool(strings.HasPrefix(endpoint, "http://")),
    S3ForcePathStyle: aws.Bool(endpoint != ""),
})

We are using S3-compatible storage provided by a startup cloud vendor. This cloud vendor has disabled the path style access method and forces us to use the virtual hosted access method. In this case, we need to set theendpoint and set the S3ForcePathStyle field to false

@warjiang
Copy link
Contributor Author

if this pr passed, we can init s3 storage like following:

storage.NewAmazonS3BackendWithOptions(
    conf.GetString("storage.amazon.bucket"),
    conf.GetString("storage.amazon.prefix"),
    conf.GetString("storage.amazon.region"),
    conf.GetString("storage.amazon.endpoint"),
    conf.GetString("storage.amazon.sse"),
    &storage.AmazonS3Options{
        S3ForcePathStyle: &forcePathStyle,
    },
)

This design mainly takes into account that if there are other s3 parameters that need to be compatible in the future, it can be easily expanded using options.

@scbizu
Copy link
Collaborator

scbizu commented Oct 26, 2023

LGTM , and what do you think ? @cbuto

@warjiang
Copy link
Contributor Author

Any volunteers to help me merge this PR? If there is any need to help, I willing to participate in the maintenance of the project. 🤣

@scbizu scbizu merged commit 4a95d4e into chartmuseum:main Oct 27, 2023
1 check passed
@scbizu
Copy link
Collaborator

scbizu commented Oct 27, 2023

Will provide a tag for module later

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.

3 participants