Skip to content

Add support for MiniO storage backend #667

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.

Sign up for GitHub

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

Open
alfeg wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add support for MiniO storage backend #667

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f6ba2fc
Add support for MiniO storage backend
alfeg Jul 14, 2021
ec40066
Update src/BaGet.Aws/S3StorageOptions.cs
loic-sharma Sep 25, 2021
ef4aefc
Update docs/installation/aws.md
loic-sharma Sep 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion docs/installation/aws.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,36 @@ Update the `appsettings.json` file:
"Region": "us-west-1",
"Bucket": "foo",
"AccessKey": "",
"SecretKey": ""
"SecretKey": "",
Comment thread
loic-sharma marked this conversation as resolved.
"ServiceUrl": "",
"UseHttp": false,
"ForcePathStyle": false
Comment on lines +26 to +28
Copy link
Copy Markdown
Owner

@loic-sharma loic-sharma Sep 25, 2021

Choose a reason for hiding this comment

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

It'd be less confusing to folks if we separate the AWS S3 and MinIO samples:

Suggested change
"ServiceUrl": "",
"UseHttp": false,
"ForcePathStyle": false

Copy link
Copy Markdown
Contributor

@viceice viceice Sep 25, 2021

Choose a reason for hiding this comment

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

Sound good

},

...
}
```

To use MinIO as an S3 compatible storage backend, provide the MinIO installation URL in `ServiceUrl` and set `ForcePathStyle` to `true`:

```json
{
...

"Storage": {
"Type": "AwsS3",
"Region": "us-west-1",
"Bucket": "foo",
"AccessKey": "",
"SecretKey": "",
"ServiceUrl": "",
"ForcePathStyle": false,
"UseHttp": false
},

...
}

### Amazon RDS

To use PostgreSQL, update the `appsettings.json` file:
Expand Down
15 changes: 14 additions & 1 deletion src/BaGet.Aws/AwsApplicationExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,24 @@ public static BaGetApplication AddAwsS3Storage(this BaGetApplication app)
{
var options = provider.GetRequiredService<IOptions<S3StorageOptions>>().Value;

if (options.ForcePathStyle)
{
// Required to support presigned urls generation for MiniO configured to us-east-1 region
AWSConfigsS3.UseSignatureVersion4 = true;
}

var config = new AmazonS3Config
{
RegionEndpoint = RegionEndpoint.GetBySystemName(options.Region)
RegionEndpoint = RegionEndpoint.GetBySystemName(options.Region),
Copy link
Copy Markdown
Owner

@loic-sharma loic-sharma Sep 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

From my understanding the region can be set using the service URL. If so should we follow @DTeuchert's pattern in #272:

Suggested change
RegionEndpoint = RegionEndpoint.GetBySystemName(options.Region),
RegionEndpoint = (options.Region != null)
? RegionEndpoint.GetBySystemName(options.Region)
: null,
ServiceURL = options.ServiceUrl

This will let us delete lines 47-50 below:

-               if (!string.IsNullOrWhiteSpace(options.ServiceUrl))
-               {
-                   config.ServiceURL = options.ServiceUrl;
-               }

Copy link
Copy Markdown
Contributor

@viceice viceice Sep 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I think the AWS SDK requires a region independent from service url, but im not fully sure.

ForcePathStyle = options.ForcePathStyle,
UseHttp = options.UseHttp
};

if (!string.IsNullOrWhiteSpace(options.ServiceUrl))
{
config.ServiceURL = options.ServiceUrl;
}

if (options.UseInstanceProfile)
{
var credentials = FallbackCredentialsFactory.GetCredentials();
Expand Down
6 changes: 6 additions & 0 deletions src/BaGet.Aws/S3StorageOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,11 @@ public class S3StorageOptions
public bool UseInstanceProfile { get; set; }

public string AssumeRoleArn { get; set; }

public bool ForcePathStyle { get; set; }

public string ServiceUrl { get; set; }
Copy link
Copy Markdown
Owner

@loic-sharma loic-sharma Sep 25, 2021

Choose a reason for hiding this comment

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

Could we add the data annotation attributes that @DTeuchert had used in #272?

-       [Required]
+       [RequiredIf(nameof(ServiceUrl), null)]
        public string Region { get; set; }

+       [RequiredIf(nameof(Region), null)]
        public string ServiceUrl { get; set; }

Copy link
Copy Markdown

@DTeuchert DTeuchert Sep 25, 2021

Choose a reason for hiding this comment

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

Yes in my understanding we need a service url for a private / on premise solutions or a region.


public bool UseHttp { get; set; }
}
}