Skip to content

Commit ad72db0

Browse files
mocsharpmocsharp
authored
Bug fixes (#63)
* Log temporary credential policy for debugging * Update policy to return distinct paths * Fix unit test * Enable concurrent calls to create MinIo clients Signed-off-by: Victor Chang <[email protected]>
1 parent 5b37ce4 commit ad72db0

12 files changed

+65
-64
lines changed

‎doc/dependency_decisions.yml‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,21 @@
44
- :who: mocsharp
55
:why: Apache-2.0 (https://github.com/aws/aws-sdk-net/raw/master/License.txt)
66
:versions:
7-
- 3.7.12.26
7+
- 3.7.13.5
88
:when: 2022-08-29 18:11:12.923214877 Z
99
- - :approve
1010
- AWSSDK.S3
1111
- :who: mocsharp
1212
:why: Apache-2.0 (https://github.com/aws/aws-sdk-net/raw/master/License.txt)
1313
:versions:
14-
- 3.7.9.44
14+
- 3.7.9.54
1515
:when: 2022-08-29 18:11:13.354973002 Z
1616
- - :approve
1717
- AWSSDK.SecurityToken
1818
- :who: mocsharp
1919
:why: Apache-2.0 (https://github.com/aws/aws-sdk-net/raw/master/License.txt)
2020
:versions:
21-
- 3.7.1.190
21+
- 3.7.1.200
2222
:when: 2022-08-16 18:11:13.781079769 Z
2323
- - :approve
2424
- Ardalis.GuardClauses
@@ -144,14 +144,14 @@
144144
- :who: mocsharp
145145
:why: MIT (https://github.com/dotnet/aspnetcore/raw/main/LICENSE.txt)
146146
:versions:
147-
- 6.0.8
147+
- 6.0.9
148148
:when: 2022-08-29 18:11:22.090772006 Z
149149
- - :approve
150150
- Microsoft.Extensions.Diagnostics.HealthChecks.Abstractions
151151
- :who: mocsharp
152152
:why: MIT (https://github.com/dotnet/aspnetcore/raw/main/LICENSE.txt)
153153
:versions:
154-
- 6.0.8
154+
- 6.0.9
155155
:when: 2022-08-29 18:11:22.090772006 Z
156156
- - :approve
157157
- Microsoft.Extensions.FileProviders.Abstractions
@@ -200,7 +200,7 @@
200200
- :who: mocsharp
201201
:why: MIT (https://github.com/dotnet/runtime/raw/main/LICENSE.TXT)
202202
:versions:
203-
- 6.0.1
203+
- 6.0.2
204204
:when: 2022-08-29 18:11:25.167886026 Z
205205
- - :approve
206206
- Microsoft.Extensions.Logging.Configuration
@@ -487,14 +487,14 @@
487487
- :who: mocsharp
488488
:why: MIT ( https://github.com/TestableIO/System.IO.Abstractions/raw/main/LICENSE)
489489
:versions:
490-
- 17.1.1
490+
- 17.2.3
491491
:when: 2022-08-16 18:11:44.550586685 Z
492492
- - :approve
493493
- System.IO.Abstractions.TestingHelpers
494494
- :who: mocsharp
495495
:why: MIT ( https://github.com/TestableIO/System.IO.Abstractions/raw/main/LICENSE)
496496
:versions:
497-
- 17.1.1
497+
- 17.2.3
498498
:when: 2022-08-16 18:11:44.984891276 Z
499499
- - :approve
500500
- System.IO.Compression

‎src/Plugins/AWSS3/Monai.Deploy.Storage.AWSS3.csproj‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<!--
1+
<!--
22
~ Copyright 2022 MONAI Consortium
33
~
44
~ Licensed under the Apache License, Version 2.0 (the "License");
@@ -49,8 +49,8 @@
4949

5050
<ItemGroup>
5151
<PackageReference Include="Ardalis.GuardClauses" Version="4.0.1" />
52-
<PackageReference Include="AWSSDK.S3" Version="3.7.9.44" />
53-
<PackageReference Include="AWSSDK.SecurityToken" Version="3.7.1.190" />
52+
<PackageReference Include="AWSSDK.S3" Version="3.7.9.54" />
53+
<PackageReference Include="AWSSDK.SecurityToken" Version="3.7.1.200" />
5454
<PackageReference Include="Microsoft.Extensions.Logging" Version="6.0.0" />
5555
<PackageReference Include="Microsoft.Extensions.Options" Version="6.0.0" />
5656
<PackageReference Include="Newtonsoft.Json" Version="13.0.1" />

‎src/Plugins/MinIO/LoggerMethods.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,5 +31,8 @@ public static partial class LoggerMethods
3131

3232
[LoggerMessage(EventId = 20003, Level = LogLevel.Error, Message = "Health check failure.")]
3333
public static partial void HealthCheckError(this ILogger logger, Exception ex);
34+
35+
[LoggerMessage(EventId = 20004, Level = LogLevel.Debug, Message = "Temporary credential policy={policy}.")]
36+
public static partial void TemporaryCredentialPolicy(this ILogger logger, string policy);
3437
}
3538
}

‎src/Plugins/MinIO/MinIoClientFactory.cs‎

Lines changed: 21 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
* limitations under the License.
1515
*/
1616

17+
using System.Collections.Concurrent;
1718
using Amazon.SecurityToken.Model;
1819
using Ardalis.GuardClauses;
1920
using Microsoft.Extensions.Options;
@@ -25,7 +26,7 @@ namespace Monai.Deploy.Storage.MinIO
2526
public class MinIoClientFactory : IMinIoClientFactory
2627
{
2728
private static readonly string DefaultClient = "_DEFAULT_";
28-
private readonly Dictionary<string, MinioClient> _clients;
29+
private readonly ConcurrentDictionary<string, MinioClient> _clients;
2930

3031
private StorageServiceConfiguration Options { get; }
3132

@@ -38,22 +39,19 @@ public MinIoClientFactory(IOptions<StorageServiceConfiguration> options)
3839

3940
Options = configuration;
4041

41-
_clients = new Dictionary<string, MinioClient>();
42+
_clients = new ConcurrentDictionary<string, MinioClient>();
4243
}
4344

4445
public MinioClient GetClient()
4546
{
46-
if (_clients.ContainsKey(DefaultClient))
47-
{
48-
return _clients[DefaultClient];
49-
}
50-
51-
var accessKey = Options.Settings[ConfigurationKeys.AccessKey];
52-
var accessToken = Options.Settings[ConfigurationKeys.AccessToken];
53-
var client = CreateClient(accessKey, accessToken);
54-
55-
_clients[DefaultClient] = client.Build();
56-
return _clients[DefaultClient];
47+
return _clients.GetOrAdd(DefaultClient, _ =>
48+
{
49+
var accessKey = Options.Settings[ConfigurationKeys.AccessKey];
50+
var accessToken = Options.Settings[ConfigurationKeys.AccessToken];
51+
var client = CreateClient(accessKey, accessToken);
52+
53+
return client.Build();
54+
});
5755
}
5856

5957
public MinioClient GetClient(Credentials credentials)
@@ -68,21 +66,20 @@ public MinioClient GetClient(Credentials credentials, string region)
6866
Guard.Against.NullOrWhiteSpace(credentials.SecretAccessKey, nameof(credentials.SecretAccessKey));
6967
Guard.Against.NullOrWhiteSpace(credentials.SessionToken, nameof(credentials.SessionToken));
7068

71-
if (_clients.ContainsKey(credentials.SessionToken))
69+
return _clients.GetOrAdd(credentials.SessionToken, _ =>
7270
{
73-
return _clients[credentials.SessionToken];
74-
}
71+
var client = CreateClient(credentials.AccessKeyId, credentials.SecretAccessKey);
72+
client.WithSessionToken(credentials.SessionToken);
7573

76-
var client = CreateClient(credentials.AccessKeyId, credentials.SecretAccessKey);
77-
client.WithSessionToken(credentials.SessionToken);
74+
if (!string.IsNullOrWhiteSpace(region))
75+
{
76+
client.WithRegion(region);
77+
}
78+
79+
return client.Build();
80+
});
7881

79-
if (!string.IsNullOrWhiteSpace(region))
80-
{
81-
client.WithRegion(region);
82-
}
8382

84-
_clients[DefaultClient] = client.Build();
85-
return _clients[DefaultClient];
8683
}
8784

8885
private MinioClient CreateClient(string accessKey, string accessToken)

‎src/Plugins/MinIO/MinIoStorageService.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,7 @@ public async Task<Credentials> CreateTemporaryCredentialsAsync(string bucketName
203203

204204
var policyString = JsonConvert.SerializeObject(policy, Formatting.None, new JsonSerializerSettings { NullValueHandling = NullValueHandling.Ignore });
205205

206+
_logger.TemporaryCredentialPolicy(policyString);
206207
var assumeRoleRequest = new AssumeRoleRequest
207208
{
208209
DurationSeconds = durationSeconds,

‎src/Plugins/MinIO/Monai.Deploy.Storage.MinIO.csproj‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<!--
1+
<!--
22
~ Copyright 2022 MONAI Consortium
33
~
44
~ Licensed under the Apache License, Version 2.0 (the "License");
@@ -48,7 +48,7 @@
4848

4949
<ItemGroup>
5050
<PackageReference Include="Ardalis.GuardClauses" Version="4.0.1" />
51-
<PackageReference Include="AWSSDK.SecurityToken" Version="3.7.1.190" />
51+
<PackageReference Include="AWSSDK.SecurityToken" Version="3.7.1.200" />
5252
<PackageReference Include="Microsoft.Extensions.Logging" Version="6.0.0" />
5353
<PackageReference Include="Microsoft.Extensions.Options" Version="6.0.0" />
5454
<PackageReference Include="Minio" Version="4.0.5" />

‎src/Plugins/MinIO/Tests/Monai.Deploy.Storage.MinIO.Tests.csproj‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
<PackageReference Include="Microsoft.Extensions.Hosting" Version="6.0.1" />
2929
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.3.1" />
3030
<PackageReference Include="Moq" Version="4.18.2" />
31-
<PackageReference Include="System.IO.Abstractions.TestingHelpers" Version="17.1.1" />
31+
<PackageReference Include="System.IO.Abstractions.TestingHelpers" Version="17.2.3" />
3232
<PackageReference Include="xunit" Version="2.4.2" />
3333
<PackageReference Include="xunit.runner.visualstudio" Version="2.4.5">
3434
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>

‎src/S3Policy/PolicyExtensions.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public static Policy ToPolicy(string? bucketName, string? folderName)
5353
{
5454
StringEquals = new StringEquals
5555
{
56-
S3Prefix = pathList.ToArray(),
56+
S3Prefix = pathList.Distinct().ToArray(),
5757
S3Delimiter = new string[] { "/" }
5858
}
5959
}
@@ -68,7 +68,7 @@ public static Policy ToPolicy(string? bucketName, string? folderName)
6868
{
6969
StringEquals = new StringEquals
7070
{
71-
S3Prefix = new string[] {$"{folderName}/*" }
71+
S3Prefix = new string[] {$"{folderName}/*", folderName }
7272
}
7373
}
7474
},
@@ -128,7 +128,7 @@ public static Policy ToPolicy(PolicyRequest[] policyRequests)
128128
{
129129
S3Prefix = policyRequests
130130
.Select(pr => $"{pr.FolderName}/*")
131-
.Union( policyRequests.Select(pr => $"{pr.FolderName}"))
131+
.Union( policyRequests.Select(pr => pr.FolderName))
132132
.Distinct().ToArray()
133133
}
134134
}

‎src/S3Policy/Tests/Extensions/PolicyExtensionsTest.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ public void ToPolicy_ValidBucketAndFolder()
8686

8787
var policyString = JsonConvert.SerializeObject(policy, Formatting.None, new JsonSerializerSettings { NullValueHandling = NullValueHandling.Ignore });
8888

89-
Assert.Equal("{\"Version\":\"2012-10-17\",\"Statement\":[{\"Sid\":\"AllowUserToSeeBucketListInTheConsole\",\"Action\":[\"s3:ListAllMyBuckets\",\"s3:GetBucketLocation\"],\"Effect\":\"Allow\",\"Resource\":[\"arn:aws:s3:::*\"]},{\"Sid\":\"AllowRootAndHomeListingOfBucket\",\"Action\":[\"s3:ListBucket\"],\"Effect\":\"Allow\",\"Resource\":[\"arn:aws:s3:::test-bucket\"],\"Condition\":{\"StringEquals\":{\"s3:prefix\":[\"Jack/Is/The/Best\",\"Jack/Is/The/\",\"Jack/Is/\",\"Jack/\",\"\"],\"s3:delimiter\":[\"/\"]}}},{\"Sid\":\"AllowListingOfUserFolder\",\"Action\":[\"s3:ListBucket\"],\"Effect\":\"Allow\",\"Resource\":[\"arn:aws:s3:::test-bucket\"],\"Condition\":{\"StringEquals\":{\"s3:prefix\":[\"Jack/Is/The/Best/*\"]}}},{\"Sid\":\"AllowAllS3ActionsInUserFolder\",\"Action\":[\"s3:*\"],\"Effect\":\"Allow\",\"Resource\":[\"arn:aws:s3:::test-bucket/Jack/Is/The/Best\",\"arn:aws:s3:::test-bucket/Jack/Is/The/Best/*\"]}]}", policyString);
89+
Assert.Equal("{\"Version\":\"2012-10-17\",\"Statement\":[{\"Sid\":\"AllowUserToSeeBucketListInTheConsole\",\"Action\":[\"s3:ListAllMyBuckets\",\"s3:GetBucketLocation\"],\"Effect\":\"Allow\",\"Resource\":[\"arn:aws:s3:::*\"]},{\"Sid\":\"AllowRootAndHomeListingOfBucket\",\"Action\":[\"s3:ListBucket\"],\"Effect\":\"Allow\",\"Resource\":[\"arn:aws:s3:::test-bucket\"],\"Condition\":{\"StringEquals\":{\"s3:prefix\":[\"Jack/Is/The/Best\",\"Jack/Is/The/\",\"Jack/Is/\",\"Jack/\",\"\"],\"s3:delimiter\":[\"/\"]}}},{\"Sid\":\"AllowListingOfUserFolder\",\"Action\":[\"s3:ListBucket\"],\"Effect\":\"Allow\",\"Resource\":[\"arn:aws:s3:::test-bucket\"],\"Condition\":{\"StringEquals\":{\"s3:prefix\":[\"Jack/Is/The/Best/*\",\"Jack/Is/The/Best\"]}}},{\"Sid\":\"AllowAllS3ActionsInUserFolder\",\"Action\":[\"s3:*\"],\"Effect\":\"Allow\",\"Resource\":[\"arn:aws:s3:::test-bucket/Jack/Is/The/Best\",\"arn:aws:s3:::test-bucket/Jack/Is/The/Best/*\"]}]}", policyString);
9090
}
9191

9292
[Fact]

‎src/Storage/Monai.Deploy.Storage.csproj‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,11 @@
6262

6363
<ItemGroup>
6464
<PackageReference Include="Ardalis.GuardClauses" Version="4.0.1" />
65-
<PackageReference Include="AWSSDK.SecurityToken" Version="3.7.1.190" />
65+
<PackageReference Include="AWSSDK.SecurityToken" Version="3.7.1.200" />
6666
<PackageReference Include="Microsoft.Extensions.Configuration" Version="6.0.1" />
67-
<PackageReference Include="Microsoft.Extensions.Diagnostics.HealthChecks" Version="6.0.8" />
67+
<PackageReference Include="Microsoft.Extensions.Diagnostics.HealthChecks" Version="6.0.9" />
6868
<PackageReference Include="Microsoft.Extensions.Logging" Version="6.0.0" />
69-
<PackageReference Include="System.IO.Abstractions" Version="17.1.1" />
69+
<PackageReference Include="System.IO.Abstractions" Version="17.2.3" />
7070
</ItemGroup>
7171

7272
<ItemGroup>

0 commit comments

Comments
 (0)