Browse Source

Address Copilot review on AWS S3-compatible provider

- Skip container wiring in test module when AWS credentials are absent
- Dispose AmazonS3Client in test cleanup
- Validate Region or ServiceURL early in DefaultAmazonS3ClientFactory
- Normalize trailing slash in ServiceURL test assertions
- Clarify Region and ServiceURL coupling in XML docs and aws.md
pull/22962/head
maliming 4 months ago
parent
commit
e8bcb11f32
No known key found for this signature in database GPG Key ID: A646B9CB645ECEA4
  1. 2
      docs/en/framework/infrastructure/blob-storing/aws.md
  2. 14
      framework/src/Volo.Abp.BlobStoring.Aws/Volo/Abp/BlobStoring/Aws/AwsBlobProviderConfiguration.cs
  3. 6
      framework/src/Volo.Abp.BlobStoring.Aws/Volo/Abp/BlobStoring/Aws/DefaultAmazonS3ClientFactory.cs
  4. 11
      framework/test/Volo.Abp.BlobStoring.Aws.Tests/Volo/Abp/BlobStoring/Aws/AbpBlobStoringAwsTestModule.cs
  5. 20
      framework/test/Volo.Abp.BlobStoring.Aws.Tests/Volo/Abp/BlobStoring/Aws/DefaultAmazonS3ClientFactory_Tests.cs

2
docs/en/framework/infrastructure/blob-storing/aws.md

@ -65,7 +65,7 @@ Configure<AbpBlobStoringOptions>(options =>
* **UseTemporaryFederatedCredentials** (bool): Use [federated user temporary credentials](https://docs.aws.amazon.com/AmazonS3/latest/dev/AuthUsingTempFederationToken.html) to access AWS services, default : `false`.
* **ProfileName** (string): The [name of the profile](https://docs.aws.amazon.com/sdk-for-net/v3/developer-guide/net-dg-config-creds.html) to get credentials from.
* **ProfilesLocation** (string): The path to the aws credentials file to look at.
* **Region** (string): The system name of the service.
* **Region** (string): The system name of the AWS region (e.g., `us-east-1`). **Required** for real AWS S3. Optional when `ServiceURL` is configured for an S3-compatible service; some services accept any value (or `auto` for Cloudflare R2).
* **ServiceURL** (string): Custom service URL for S3-compatible APIs (e.g., MinIO, DigitalOcean Spaces, Cloudflare R2). If not specified, the default AWS S3 service URL will be used based on the region. When using S3-compatible services, this should point to your service endpoint (e.g., `https://minio.example.com:9000`). The AWS SDK automatically appends a trailing slash to the configured value.
* **DisablePayloadSigning** (bool): Default `false`. When set to `true`, the provider sends `x-amz-content-sha256: UNSIGNED-PAYLOAD` on `PutObject` requests instead of the streaming chunked signature (`STREAMING-AWS4-HMAC-SHA256-PAYLOAD`) that the AWS SDK v4 uses by default. Required for Cloudflare R2 and other S3-compatible services that do not implement streaming signing. The endpoint must be HTTPS when this option is enabled. Leave as `false` for real AWS S3.
* **Policy** (string): An IAM policy in JSON format that you want to use as an inline session policy.

14
framework/src/Volo.Abp.BlobStoring.Aws/Volo/Abp/BlobStoring/Aws/AwsBlobProviderConfiguration.cs

@ -57,6 +57,11 @@ public class AwsBlobProviderConfiguration
set => _containerConfiguration.SetConfiguration(AwsBlobProviderConfigurationNames.Policy, value);
}
/// <summary>
/// The system name of the AWS region (e.g., <c>us-east-1</c>). Required for real AWS S3.
/// Optional when <see cref="ServiceURL"/> is configured for an S3-compatible service;
/// some services accept any value (or <c>auto</c> for Cloudflare R2).
/// </summary>
public string? Region {
get => _containerConfiguration.GetConfigurationOrDefault<string>(AwsBlobProviderConfigurationNames.Region);
set => _containerConfiguration.SetConfiguration(AwsBlobProviderConfigurationNames.Region, value);
@ -64,8 +69,8 @@ public class AwsBlobProviderConfiguration
/// <summary>
/// Custom service URL for S3-compatible APIs (e.g., MinIO, DigitalOcean Spaces, Cloudflare R2).
/// If not specified, the default AWS S3 service URL will be used based on the region.
/// Note: The AWS SDK automatically appends a trailing slash to the configured value.
/// When omitted, <see cref="Region"/> becomes required and the default AWS S3 endpoint for that
/// region is used. The AWS SDK automatically appends a trailing slash to the configured value.
/// </summary>
public string? ServiceURL {
get => _containerConfiguration.GetConfigurationOrDefault<string>(AwsBlobProviderConfigurationNames.ServiceURL);
@ -73,11 +78,12 @@ public class AwsBlobProviderConfiguration
}
/// <summary>
/// When true, payload signing is disabled on PutObject (and similar) requests so the SDK sends
/// When true, payload signing is disabled on PutObject upload requests so the SDK sends
/// <c>x-amz-content-sha256: UNSIGNED-PAYLOAD</c> instead of the streaming chunked signature
/// (<c>STREAMING-AWS4-HMAC-SHA256-PAYLOAD</c>) that AWS SDK v4 uses by default. Required for
/// Cloudflare R2 and other S3-compatible services that do not implement streaming signing.
/// Default: false (keep AWS SDK defaults for real AWS S3).
/// The endpoint must be HTTPS when this option is enabled (AWS SDK rejects unsigned payloads
/// over HTTP). Default: false (keep AWS SDK defaults for real AWS S3 and MinIO).
/// </summary>
public bool DisablePayloadSigning {
get => _containerConfiguration.GetConfigurationOrDefault(AwsBlobProviderConfigurationNames.DisablePayloadSigning, false);

6
framework/src/Volo.Abp.BlobStoring.Aws/Volo/Abp/BlobStoring/Aws/DefaultAmazonS3ClientFactory.cs

@ -30,6 +30,12 @@ public class DefaultAmazonS3ClientFactory : IAmazonS3ClientFactory, ITransientDe
public virtual async Task<AmazonS3Client> GetAmazonS3Client(
AwsBlobProviderConfiguration configuration)
{
if (configuration.Region.IsNullOrWhiteSpace() && configuration.ServiceURL.IsNullOrWhiteSpace())
{
throw new AbpException(
$"Either {nameof(AwsBlobProviderConfiguration.Region)} or {nameof(AwsBlobProviderConfiguration.ServiceURL)} must be configured on {nameof(AwsBlobProviderConfiguration)}.");
}
var region = !configuration.Region.IsNullOrWhiteSpace()
? RegionEndpoint.GetBySystemName(configuration.Region)
: null;

11
framework/test/Volo.Abp.BlobStoring.Aws.Tests/Volo/Abp/BlobStoring/Aws/AbpBlobStoringAwsTestModule.cs

@ -56,6 +56,15 @@ public class AbpBlobStoringAwsTestModule : AbpModule
var configuration = context.Services.GetConfiguration();
var accessKeyId = configuration["Aws:AccessKeyId"];
var secretAccessKey = configuration["Aws:SecretAccessKey"];
// No credentials configured (e.g., CI without user secrets) → skip container wiring.
// `BlobContainerConfiguration.SetConfiguration` rejects nulls, so any attempt to resolve
// a container with null values from configuration would throw at runtime.
if (string.IsNullOrEmpty(accessKeyId) || string.IsNullOrEmpty(secretAccessKey))
{
return;
}
var region = configuration["Aws:Region"];
var serviceUrl = configuration["Aws:ServiceURL"];
var disablePayloadSigning = bool.TryParse(configuration["Aws:DisablePayloadSigning"], out var dps) && dps;
@ -109,7 +118,7 @@ public class AbpBlobStoringAwsTestModule : AbpModule
try
{
var amazonS3Client = await context.ServiceProvider.GetRequiredService<IAmazonS3ClientFactory>()
using var amazonS3Client = await context.ServiceProvider.GetRequiredService<IAmazonS3ClientFactory>()
.GetAmazonS3Client(_configuration);
if (!await AmazonS3Util.DoesS3BucketExistV2Async(amazonS3Client, _actualContainerName))

20
framework/test/Volo.Abp.BlobStoring.Aws.Tests/Volo/Abp/BlobStoring/Aws/DefaultAmazonS3ClientFactory_Tests.cs

@ -1,4 +1,5 @@
using System.Threading.Tasks;
using Amazon.Runtime;
using Amazon.S3;
using Microsoft.Extensions.DependencyInjection;
using Shouldly;
@ -35,8 +36,8 @@ public class DefaultAmazonS3ClientFactory_Tests : AbpBlobStoringAwsTestBase
// Assert
s3Client.ShouldNotBeNull();
s3Client.Config.ServiceURL.ShouldBe(serviceUrl + "/"); // AWS SDK automatically appends trailing slash
((AmazonS3Config)s3Client.Config).ForcePathStyle.ShouldBeTrue(); // Should be enabled for S3-compatible services
s3Client.Config.ServiceURL?.TrimEnd('/').ShouldBe(serviceUrl);
((AmazonS3Config)s3Client.Config).ForcePathStyle.ShouldBeTrue();
}
[Fact]
@ -81,8 +82,8 @@ public class DefaultAmazonS3ClientFactory_Tests : AbpBlobStoringAwsTestBase
// Assert
s3Client.ShouldNotBeNull();
s3Client.Config.ServiceURL.ShouldBe("https://minio.example.com:9000/"); // AWS SDK automatically appends trailing slash
((AmazonS3Config)s3Client.Config).ForcePathStyle.ShouldBeTrue(); // Should be enabled for S3-compatible services
s3Client.Config.ServiceURL?.TrimEnd('/').ShouldBe("https://minio.example.com:9000");
((AmazonS3Config)s3Client.Config).ForcePathStyle.ShouldBeTrue();
}
[Fact]
@ -105,12 +106,9 @@ public class DefaultAmazonS3ClientFactory_Tests : AbpBlobStoringAwsTestBase
// Assert
s3Client.ShouldNotBeNull();
var config = (AmazonS3Config)s3Client.Config;
config.ServiceURL.ShouldBe("https://r2.cloudflarestorage.com/");
config.ServiceURL?.TrimEnd('/').ShouldBe("https://r2.cloudflarestorage.com");
config.ForcePathStyle.ShouldBeTrue();
// Verify checksum properties are set for S3-compatible services (required for Cloudflare R2)
// We just verify they are not null/default, indicating they have been set
config.RequestChecksumCalculation.ShouldNotBe(default);
config.ResponseChecksumValidation.ShouldNotBe(default);
config.RequestChecksumCalculation.ShouldBe(RequestChecksumCalculation.WHEN_REQUIRED);
config.ResponseChecksumValidation.ShouldBe(ResponseChecksumValidation.WHEN_REQUIRED);
}
}
}

Loading…
Cancel
Save