From f6544ac237eb97b0c63f970cc570f45afafbda83 Mon Sep 17 00:00:00 2001 From: Shiva Kumar Mukkapati Date: Tue, 9 Jan 2024 00:23:25 -0500 Subject: [PATCH 1/3] cross-region-support-cache level changes --- .../S3AccessGrantsAccountIdResolver.java | 4 +- .../S3AccessGrantsBucketRegionResolver.java | 33 +++ .../cache/S3AccessGrantsCache.java | 36 ++-- ...S3AccessGrantsCachedAccountIdResolver.java | 33 +-- ...ccessGrantsCachedBucketRegionResolver.java | 199 ++++++++++++++++++ ...AccessGrantsCachedCredentialsProvider.java | 4 +- ...ssGrantsCachedCredentialsProviderImpl.java | 27 +-- .../cache/S3AccessGrantsConstants.java | 10 +- ...rantsBucketRegionResolverCreationTest.java | 117 ++++++++++ ...3AccessGrantsBucketRegionResolverTest.java | 126 +++++++++++ .../cache/S3AccessGrantsCacheTest.java | 55 +++-- ...tsCachedAccountIdResolverCreationTest.java | 25 +-- ...cessGrantsCachedAccountIdResolverTest.java | 15 +- ...antsCachedCredentialsProviderImplTest.java | 27 +-- 14 files changed, 574 insertions(+), 137 deletions(-) create mode 100644 src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsBucketRegionResolver.java create mode 100644 src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedBucketRegionResolver.java create mode 100644 src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsBucketRegionResolverCreationTest.java create mode 100644 src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsBucketRegionResolverTest.java diff --git a/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsAccountIdResolver.java b/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsAccountIdResolver.java index 5a925d9..8ab79d9 100644 --- a/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsAccountIdResolver.java +++ b/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsAccountIdResolver.java @@ -15,6 +15,7 @@ package software.amazon.awssdk.s3accessgrants.cache; +import software.amazon.awssdk.services.s3control.S3ControlAsyncClient; import software.amazon.awssdk.services.s3control.model.S3ControlException; public interface S3AccessGrantsAccountIdResolver { @@ -22,8 +23,9 @@ public interface S3AccessGrantsAccountIdResolver { * * @param accountId AWS AccountId from the request context parameter * @param s3Prefix e.g., s3://bucket-name/path/to/helloworld.txt + * @param s3ControlAsyncClient S3ControlAsynClient that will be used for making the requests * @return AWS AccountId of the S3 Access Grants Instance that owns the location scope of the s3Prefix * @throws S3ControlException propagate S3ControlException from service call */ - String resolve(String accountId, String s3Prefix) throws S3ControlException; + String resolve(String accountId, String s3Prefix, S3ControlAsyncClient s3ControlAsyncClient) throws S3ControlException; } diff --git a/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsBucketRegionResolver.java b/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsBucketRegionResolver.java new file mode 100644 index 0000000..b15ec5e --- /dev/null +++ b/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsBucketRegionResolver.java @@ -0,0 +1,33 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ +package software.amazon.awssdk.s3accessgrants.cache; + +import software.amazon.awssdk.regions.Region; +import software.amazon.awssdk.services.s3.S3Client; +import software.amazon.awssdk.services.s3.model.S3Exception; +import software.amazon.awssdk.services.s3control.S3ControlAsyncClient; +import software.amazon.awssdk.services.s3control.model.S3ControlException; + +public interface S3AccessGrantsBucketRegionResolver { + + /** + * + * @param bucket name of the bucket being accessed + * @throws S3Exception propagate S3Exception from service call + */ + + Region resolve(String bucket) throws S3Exception; + +} diff --git a/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCache.java b/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCache.java index 3e475e8..fe04261 100644 --- a/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCache.java +++ b/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCache.java @@ -43,18 +43,13 @@ public class S3AccessGrantsCache { private AsyncCache cache; - private final S3ControlAsyncClient s3ControlAsyncClient; private int maxCacheSize; private final S3AccessGrantsCachedAccountIdResolver s3AccessGrantsCachedAccountIdResolver; private final int cacheExpirationTimePercentage; private static final Logger logger = Logger.loggerFor(S3AccessGrantsCache.class); - private S3AccessGrantsCache (@NotNull S3ControlAsyncClient s3ControlAsyncClient, - S3AccessGrantsCachedAccountIdResolver resolver, int maxCacheSize, int cacheExpirationTimePercentage) { - if (s3ControlAsyncClient == null) { - throw new IllegalArgumentException("S3ControlAsyncClient is required"); - } - this.s3ControlAsyncClient = s3ControlAsyncClient; + private S3AccessGrantsCache (@NotNull S3AccessGrantsCachedAccountIdResolver resolver, int maxCacheSize, int cacheExpirationTimePercentage) { + this.s3AccessGrantsCachedAccountIdResolver = resolver; this.cacheExpirationTimePercentage = cacheExpirationTimePercentage; this.maxCacheSize = maxCacheSize; @@ -75,14 +70,12 @@ protected static S3AccessGrantsCache.Builder builder() { public interface Builder { S3AccessGrantsCache build(); S3AccessGrantsCache buildWithAccountIdResolver(); - S3AccessGrantsCache.Builder s3ControlAsyncClient(S3ControlAsyncClient s3ControlAsyncClient); S3AccessGrantsCache.Builder maxCacheSize(int maxCacheSize); S3AccessGrantsCache.Builder cacheExpirationTimePercentage(int cacheExpirationTimePercentage); S3AccessGrantsCache.Builder s3AccessGrantsCachedAccountIdResolver(S3AccessGrantsCachedAccountIdResolver s3AccessGrantsCachedAccountIdResolver); } static final class BuilderImpl implements S3AccessGrantsCache.Builder { - private S3ControlAsyncClient s3ControlAsyncClient; private int maxCacheSize = DEFAULT_ACCESS_GRANTS_MAX_CACHE_SIZE; private S3AccessGrantsCachedAccountIdResolver s3AccessGrantsCachedAccountIdResolver; private int cacheExpirationTimePercentage; @@ -93,22 +86,16 @@ private BuilderImpl() { @Override public S3AccessGrantsCache build() { S3AccessGrantsCachedAccountIdResolver s3AccessGrantsCachedAccountIdResolver = - S3AccessGrantsCachedAccountIdResolver.builder().S3ControlAsyncClient(s3ControlAsyncClient).build(); - return new S3AccessGrantsCache(s3ControlAsyncClient, s3AccessGrantsCachedAccountIdResolver, maxCacheSize, cacheExpirationTimePercentage); + S3AccessGrantsCachedAccountIdResolver.builder().build(); + return new S3AccessGrantsCache(s3AccessGrantsCachedAccountIdResolver, maxCacheSize, cacheExpirationTimePercentage); } @Override public S3AccessGrantsCache buildWithAccountIdResolver() { - return new S3AccessGrantsCache(s3ControlAsyncClient, s3AccessGrantsCachedAccountIdResolver, maxCacheSize, + return new S3AccessGrantsCache(s3AccessGrantsCachedAccountIdResolver, maxCacheSize, cacheExpirationTimePercentage); } - @Override - public Builder s3ControlAsyncClient(S3ControlAsyncClient s3ControlAsyncClient) { - this.s3ControlAsyncClient = s3ControlAsyncClient; - return this; - } - @Override public Builder maxCacheSize(int maxCacheSize) { this.maxCacheSize = maxCacheSize; @@ -134,10 +121,11 @@ public Builder s3AccessGrantsCachedAccountIdResolver(S3AccessGrantsCachedAccount * @param cacheKey CacheKey consists of AwsCredentialsIdentity, Permission, and S3Prefix. * @param accountId Account Id of the requester * @param s3AccessGrantsAccessDeniedCache instance of S3AccessGrantsAccessDeniedCache + * @param s3ControlAsyncClient S3ControlAsyncClient that will be used for making the requests * @return cached Access Grants credentials. */ protected CompletableFuture getCredentials (CacheKey cacheKey, String accountId, - S3AccessGrantsAccessDeniedCache s3AccessGrantsAccessDeniedCache) throws S3ControlException { + S3AccessGrantsAccessDeniedCache s3AccessGrantsAccessDeniedCache, S3ControlAsyncClient s3ControlAsyncClient) throws S3ControlException { logger.debug(()->"Fetching credentials from Access Grants for s3Prefix: " + cacheKey.s3Prefix); CompletableFuture credentials = searchKeyInCacheAtPrefixLevel(cacheKey); @@ -157,7 +145,10 @@ protected CompletableFuture getCredentials (CacheKey cac if (credentials == null) { try { logger.debug(()->"Credentials not available in the cache. Fetching credentials from Access Grants service."); - credentials = getCredentialsFromService(cacheKey,accountId).thenApply(getDataAccessResponse -> { + if (s3ControlAsyncClient == null) { + throw new IllegalArgumentException("S3ControlAsyncClient is required"); + } + credentials = getCredentialsFromService(cacheKey,accountId, s3ControlAsyncClient).thenApply(getDataAccessResponse -> { Credentials accessGrantsCredentials = getDataAccessResponse.credentials(); long duration = getTTL(accessGrantsCredentials.expiration()); AwsSessionCredentials sessionCredentials = AwsSessionCredentials.builder().accessKeyId(accessGrantsCredentials.accessKeyId()) @@ -198,11 +189,12 @@ long getTTL(Instant expirationTime) { * This method calls Access Grants service to get the credentials. * @param cacheKey CacheKey consists of AwsCredentialsIdentity, Permission, and S3Prefix. * @param accountId Account Id of the requester. + * @param s3ControlAsyncClient regional S3 Control client used for forwarding requests. * @return Access Grants Credentials. * @throws S3ControlException throws Exception received from service. */ - private CompletableFuture getCredentialsFromService(CacheKey cacheKey, String accountId) throws S3ControlException{ - String resolvedAccountId = s3AccessGrantsCachedAccountIdResolver.resolve(accountId, cacheKey.s3Prefix); + private CompletableFuture getCredentialsFromService(CacheKey cacheKey, String accountId, S3ControlAsyncClient s3ControlAsyncClient) throws S3ControlException{ + String resolvedAccountId = s3AccessGrantsCachedAccountIdResolver.resolve(accountId, cacheKey.s3Prefix, s3ControlAsyncClient); logger.debug(()->"Fetching credentials from Access Grants for accountId: " + resolvedAccountId + ", s3Prefix: " + cacheKey.s3Prefix + ", permission: " + cacheKey.permission + ", privilege: " + Privilege.DEFAULT); GetDataAccessRequest dataAccessRequest = GetDataAccessRequest.builder() diff --git a/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedAccountIdResolver.java b/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedAccountIdResolver.java index b0122bd..cf4ef39 100644 --- a/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedAccountIdResolver.java +++ b/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedAccountIdResolver.java @@ -41,17 +41,12 @@ */ public class S3AccessGrantsCachedAccountIdResolver implements S3AccessGrantsAccountIdResolver { - private final S3ControlAsyncClient S3ControlAsyncClient; private int maxCacheSize; private int expireCacheAfterWriteSeconds; private static final Logger logger = Logger.loggerFor(S3AccessGrantsCachedAccountIdResolver.class); private Cache cache; - public S3ControlAsyncClient S3ControlAsyncClient() { - return S3ControlAsyncClient; - } - public int maxCacheSize() { return maxCacheSize; } @@ -63,11 +58,7 @@ public int expireCacheAfterWriteSeconds() { protected CacheStats getCacheStats() { return cache.stats(); } @VisibleForTesting - S3AccessGrantsCachedAccountIdResolver(@NotNull S3ControlAsyncClient S3ControlAsyncClient) { - if (S3ControlAsyncClient == null) { - throw new IllegalArgumentException("S3ControlAsyncClient is required"); - } - this.S3ControlAsyncClient = S3ControlAsyncClient; + S3AccessGrantsCachedAccountIdResolver() { this.maxCacheSize = DEFAULT_ACCOUNT_ID_MAX_CACHE_SIZE; this.expireCacheAfterWriteSeconds = DEFAULT_ACCOUNT_ID_EXPIRE_CACHE_AFTER_WRITE_SECONDS; } @@ -82,12 +73,15 @@ public static Builder builder() { } @Override - public String resolve(String accountId, String s3Prefix) { + public String resolve(String accountId, String s3Prefix, S3ControlAsyncClient s3ControlAsyncClient) { String bucketName = getBucketName(s3Prefix); String s3PrefixAccountId = cache.getIfPresent(bucketName); if (s3PrefixAccountId == null) { logger.debug(()->"Account Id not available in the cache. Fetching account from server."); - s3PrefixAccountId = resolveFromService(accountId, s3Prefix); + if (s3ControlAsyncClient == null) { + throw new IllegalArgumentException("S3ControlAsyncClient is required for the access grants instance account resolver!"); + } + s3PrefixAccountId = resolveFromService(accountId, s3Prefix, s3ControlAsyncClient); cache.put(bucketName, s3PrefixAccountId); } return s3PrefixAccountId; @@ -98,9 +92,9 @@ public String resolve(String accountId, String s3Prefix) { * @param s3Prefix e.g., s3://bucket-name/path/to/helloworld.txt * @return accountId from the service response */ - private String resolveFromService(String accountId, String s3Prefix) { + private String resolveFromService(String accountId, String s3Prefix, S3ControlAsyncClient s3ControlAsyncClient) { CompletableFuture accessGrantsInstanceForPrefix = - S3ControlAsyncClient.getAccessGrantsInstanceForPrefix(GetAccessGrantsInstanceForPrefixRequest + s3ControlAsyncClient.getAccessGrantsInstanceForPrefix(GetAccessGrantsInstanceForPrefixRequest .builder() .accountId(accountId) .s3Prefix(s3Prefix) @@ -117,8 +111,6 @@ private String resolveFromService(String accountId, String s3Prefix) { public interface Builder { S3AccessGrantsCachedAccountIdResolver build(); - Builder S3ControlAsyncClient(S3ControlAsyncClient S3ControlAsyncClient); - Builder maxCacheSize(int maxCacheSize); Builder expireCacheAfterWriteSeconds(int expireCacheAfterWriteSeconds); @@ -133,17 +125,10 @@ private BuilderImpl() { } public BuilderImpl(S3AccessGrantsCachedAccountIdResolver s3AccessGrantsCachedAccountIdResolver) { - S3ControlAsyncClient(s3AccessGrantsCachedAccountIdResolver.S3ControlAsyncClient); maxCacheSize(s3AccessGrantsCachedAccountIdResolver.maxCacheSize); expireCacheAfterWriteSeconds(s3AccessGrantsCachedAccountIdResolver.expireCacheAfterWriteSeconds); } - @Override - public Builder S3ControlAsyncClient(S3ControlAsyncClient S3ControlAsyncClient) { - this.S3ControlAsyncClient = S3ControlAsyncClient; - return this; - } - public int maxCacheSize() { return maxCacheSize; } @@ -174,7 +159,7 @@ public Builder expireCacheAfterWriteSeconds(int expireCacheAfterWriteSeconds) { @Override public S3AccessGrantsCachedAccountIdResolver build() { - S3AccessGrantsCachedAccountIdResolver resolver = new S3AccessGrantsCachedAccountIdResolver(S3ControlAsyncClient); + S3AccessGrantsCachedAccountIdResolver resolver = new S3AccessGrantsCachedAccountIdResolver(); resolver.maxCacheSize = maxCacheSize(); resolver.expireCacheAfterWriteSeconds = expireCAcheAfterWriteSeconds(); resolver.cache = Caffeine.newBuilder() diff --git a/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedBucketRegionResolver.java b/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedBucketRegionResolver.java new file mode 100644 index 0000000..0677d1d --- /dev/null +++ b/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedBucketRegionResolver.java @@ -0,0 +1,199 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ +package software.amazon.awssdk.s3accessgrants.cache; + +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; +import com.github.benmanes.caffeine.cache.stats.CacheStats; +import software.amazon.awssdk.core.exception.SdkServiceException; +import software.amazon.awssdk.regions.Region; +import software.amazon.awssdk.services.s3.S3Client; +import software.amazon.awssdk.services.s3.model.HeadBucketRequest; +import software.amazon.awssdk.services.s3.model.HeadBucketResponse; +import software.amazon.awssdk.services.s3.model.S3Exception; +import software.amazon.awssdk.utils.Logger; + +import java.time.Duration; + +import static software.amazon.awssdk.s3accessgrants.cache.S3AccessGrantsConstants.BUCKET_REGION_CACHE_SIZE; +import static software.amazon.awssdk.s3accessgrants.cache.S3AccessGrantsConstants.BUCKET_REGION_EXPIRE_CACHE_AFTER_WRITE_SECONDS; +import static software.amazon.awssdk.s3accessgrants.cache.S3AccessGrantsConstants.MAX_BUCKET_REGION_EXPIRE_CACHE_AFTER_WRITE_SECONDS; +import static software.amazon.awssdk.s3accessgrants.cache.S3AccessGrantsConstants.MAX_BUCKET_REGION_CACHE_SIZE; + +public class S3AccessGrantsCachedBucketRegionResolver implements S3AccessGrantsBucketRegionResolver { + + + private int maxCacheSize; + + private int expireCacheAfterWriteSeconds; + + private Cache cache; + + private S3Client s3Client; + + private static final Logger logger = Logger.loggerFor(S3AccessGrantsCachedBucketRegionResolver.class); + + public int getMaxCacheSize() { + return maxCacheSize; + } + + + public int getExpireCacheAfterWriteSeconds() { + return expireCacheAfterWriteSeconds; + } + + public int expireCacheAfterWriteSeconds() { + return expireCacheAfterWriteSeconds; + } + + public int maxCacheSize() { + return maxCacheSize; + } + + protected CacheStats getCacheStats() { return cache.stats(); } + + public S3AccessGrantsCachedBucketRegionResolver.Builder toBuilder() { + return new S3AccessGrantsCachedBucketRegionResolver.BuilderImpl(this); + } + + public static S3AccessGrantsCachedBucketRegionResolver.Builder builder() { + return new S3AccessGrantsCachedBucketRegionResolver.BuilderImpl(); + } + + private S3AccessGrantsCachedBucketRegionResolver() { + this.maxCacheSize = BUCKET_REGION_CACHE_SIZE; + this.expireCacheAfterWriteSeconds = BUCKET_REGION_EXPIRE_CACHE_AFTER_WRITE_SECONDS; + } + + @Override + public Region resolve(String bucket) throws S3Exception { + Region bucketRegion = cache.getIfPresent(bucket); + if(bucketRegion == null) { + logger.debug(() -> "bucket region not available in cache, fetching the region from the service!"); + if (s3Client == null) { + throw new IllegalArgumentException("S3Client is required for the bucket region resolver!"); + } + bucketRegion = resolveFromService(bucket); + if(bucketRegion != null) { + cache.put(bucket, bucketRegion); + } + } else { + logger.debug(() -> "bucket region available in cache!"); + } + return bucketRegion; + + } + + private Region resolveFromService(String bucket) { + String resolvedRegion = null; + try { + logger.info(() -> "making a call to S3 for determining the bucket region!"); + HeadBucketRequest bucketLocationRequest = HeadBucketRequest.builder().bucket(bucket).build(); + HeadBucketResponse headBucketResponse = s3Client.headBucket(bucketLocationRequest); + resolvedRegion = headBucketResponse.bucketRegion(); + } catch (S3Exception e) { + if (e.statusCode() == 301) { + // A fallback in case S3 Clients are not able to re-direct the head bucket requests to the correct region. + String bucketRegion = e.awsErrorDetails().sdkHttpResponse().headers().get("x-amz-bucket-region").get(0); + resolvedRegion = bucketRegion; + } else { + throw e; + } + } + if(resolvedRegion == null) throw SdkServiceException.builder().message("S3 error! region cannot be determined for the specified bucket!").build(); + return Region.of(resolvedRegion); + } + + + public interface Builder { + S3AccessGrantsCachedBucketRegionResolver build(); + + S3AccessGrantsCachedBucketRegionResolver.Builder maxCacheSize(int maxCacheSize); + + S3AccessGrantsCachedBucketRegionResolver.Builder s3Client(S3Client s3Client); + + S3AccessGrantsCachedBucketRegionResolver.Builder expireCacheAfterWriteSeconds(int expireCacheAfterWriteSeconds); + } + + static final class BuilderImpl implements S3AccessGrantsCachedBucketRegionResolver.Builder { + private int maxCacheSize = BUCKET_REGION_CACHE_SIZE; + private int expireCacheAfterWriteSeconds = BUCKET_REGION_EXPIRE_CACHE_AFTER_WRITE_SECONDS; + + private S3Client s3Client; + + private BuilderImpl() { + } + + public BuilderImpl(S3AccessGrantsCachedBucketRegionResolver s3AccessGrantsCachedBucketRegionResolver) { + maxCacheSize(s3AccessGrantsCachedBucketRegionResolver.maxCacheSize); + expireCacheAfterWriteSeconds(s3AccessGrantsCachedBucketRegionResolver.expireCacheAfterWriteSeconds); + s3Client(s3AccessGrantsCachedBucketRegionResolver.s3Client); + } + + public int maxCacheSize() { + return maxCacheSize; + } + + public S3Client s3Client() { + if(s3Client == null) throw new IllegalArgumentException("S3 Client is required while configuring the S3 Bucket Region resolver!"); + return s3Client; + } + + @Override + public S3AccessGrantsCachedBucketRegionResolver.Builder maxCacheSize(int maxCacheSize) { + if (maxCacheSize <= 0 || maxCacheSize > MAX_BUCKET_REGION_CACHE_SIZE) { + throw new IllegalArgumentException(String.format("maxCacheSize needs to be in range (0, %d]", + MAX_BUCKET_REGION_CACHE_SIZE)); + } + this.maxCacheSize = maxCacheSize; + return this; + } + + @Override + public Builder s3Client(S3Client s3Client) { + if(s3Client == null) throw new IllegalArgumentException("S3 Client is required while configuring the S3 Bucket Region resolver!"); + this.s3Client = s3Client; + return this; + } + + public int expireCacheAfterWriteSeconds() { + return expireCacheAfterWriteSeconds; + } + + @Override + public S3AccessGrantsCachedBucketRegionResolver.Builder expireCacheAfterWriteSeconds(int expireCacheAfterWriteSeconds) { + if (expireCacheAfterWriteSeconds <= 0 || expireCacheAfterWriteSeconds > MAX_BUCKET_REGION_EXPIRE_CACHE_AFTER_WRITE_SECONDS) { + throw new IllegalArgumentException(String.format("expireCacheAfterWriteSeconds needs to be in range (0, %d]", + MAX_BUCKET_REGION_EXPIRE_CACHE_AFTER_WRITE_SECONDS)); + } + this.expireCacheAfterWriteSeconds = expireCacheAfterWriteSeconds; + return this; + } + + @Override + public S3AccessGrantsCachedBucketRegionResolver build() { + S3AccessGrantsCachedBucketRegionResolver resolver = new S3AccessGrantsCachedBucketRegionResolver(); + resolver.maxCacheSize = maxCacheSize(); + resolver.expireCacheAfterWriteSeconds = expireCacheAfterWriteSeconds(); + resolver.s3Client = s3Client(); + resolver.cache = Caffeine.newBuilder() + .maximumSize(maxCacheSize) + .expireAfterWrite(Duration.ofSeconds(expireCacheAfterWriteSeconds)) + .build(); + return resolver; + } + } + +} diff --git a/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedCredentialsProvider.java b/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedCredentialsProvider.java index 341cbbe..e589243 100644 --- a/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedCredentialsProvider.java +++ b/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedCredentialsProvider.java @@ -18,6 +18,7 @@ import java.util.concurrent.CompletableFuture; import software.amazon.awssdk.identity.spi.AwsCredentialsIdentity; import software.amazon.awssdk.metrics.MetricCollector; +import software.amazon.awssdk.services.s3control.S3ControlAsyncClient; import software.amazon.awssdk.services.s3control.model.Permission; import software.amazon.awssdk.services.s3control.model.S3ControlException; @@ -27,11 +28,12 @@ public interface S3AccessGrantsCachedCredentialsProvider { * @param credentials Credentials used for calling Access Grants. * @param permission Permission requested by the user. Can be Read, Write, or ReadWrite. * @param s3Prefix S3Prefix requested by the user. e.g., s3://bucket-name/path/to/helloworld.txt + * @param s3ControlAsyncClient S3ControlAsynClient that will be used for making the requests * @return Credentials from Access Grants. * @throws S3ControlException in-case exception is cached. */ CompletableFuture getDataAccess (AwsCredentialsIdentity credentials, Permission permission, String s3Prefix, - String accountId) throws Exception; + String accountId, S3ControlAsyncClient s3ControlAsyncClient) throws Exception; /** * @return metrics collected by access grants cache jar diff --git a/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedCredentialsProviderImpl.java b/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedCredentialsProviderImpl.java index 4558649..0f6555d 100644 --- a/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedCredentialsProviderImpl.java +++ b/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedCredentialsProviderImpl.java @@ -33,17 +33,16 @@ import static software.amazon.awssdk.s3accessgrants.cache.S3AccessGrantsConstants.DEFAULT_ACCESS_GRANTS_MAX_CACHE_SIZE; import static software.amazon.awssdk.s3accessgrants.cache.S3AccessGrantsConstants.MAX_LIMIT_ACCESS_GRANTS_MAX_CACHE_SIZE; -public class S3AccessGrantsCachedCredentialsProviderImpl implements S3AccessGrantsCachedCredentialsProvider{ +public class S3AccessGrantsCachedCredentialsProviderImpl implements S3AccessGrantsCachedCredentialsProvider { private final S3AccessGrantsCache accessGrantsCache; private final S3AccessGrantsAccessDeniedCache s3AccessGrantsAccessDeniedCache; DefaultMetricCollector collector = new DefaultMetricCollector("AccessGrantsMetricsCollector"); public static final Logger logger = Logger.loggerFor(S3AccessGrantsCachedCredentialsProviderImpl.class); - private S3AccessGrantsCachedCredentialsProviderImpl(S3ControlAsyncClient S3ControlAsyncClient, int maxCacheSize, int cacheExpirationTimePercentage) { + private S3AccessGrantsCachedCredentialsProviderImpl(int maxCacheSize, int cacheExpirationTimePercentage) { accessGrantsCache = S3AccessGrantsCache.builder() - .s3ControlAsyncClient(S3ControlAsyncClient) .maxCacheSize(maxCacheSize) .cacheExpirationTimePercentage(cacheExpirationTimePercentage).build(); @@ -52,11 +51,10 @@ private S3AccessGrantsCachedCredentialsProviderImpl(S3ControlAsyncClient S3Contr } @VisibleForTesting - S3AccessGrantsCachedCredentialsProviderImpl(S3ControlAsyncClient s3ControlAsyncClient, - S3AccessGrantsCachedAccountIdResolver resolver,int maxCacheSize, int cacheExpirationTimePercentage) { + S3AccessGrantsCachedCredentialsProviderImpl(S3AccessGrantsCachedAccountIdResolver resolver, + int maxCacheSize, int cacheExpirationTimePercentage) { accessGrantsCache = S3AccessGrantsCache.builder() - .s3ControlAsyncClient(s3ControlAsyncClient) .maxCacheSize(maxCacheSize) .cacheExpirationTimePercentage(cacheExpirationTimePercentage) .s3AccessGrantsCachedAccountIdResolver(resolver) @@ -72,7 +70,6 @@ public static S3AccessGrantsCachedCredentialsProviderImpl.Builder builder() { public interface Builder { S3AccessGrantsCachedCredentialsProviderImpl build(); S3AccessGrantsCachedCredentialsProviderImpl buildWithAccountIdResolver(); - S3AccessGrantsCachedCredentialsProviderImpl.Builder S3ControlAsyncClient(S3ControlAsyncClient S3ControlAsyncClient); S3AccessGrantsCachedCredentialsProviderImpl.Builder s3AccessGrantsCachedAccountIdResolver(S3AccessGrantsCachedAccountIdResolver s3AccessGrantsCachedAccountIdResolver); S3AccessGrantsCachedCredentialsProviderImpl.Builder maxCacheSize(int maxCacheSize); S3AccessGrantsCachedCredentialsProviderImpl.Builder cacheExpirationTimePercentage(int cacheExpirationTimePercentage); @@ -89,18 +86,12 @@ private BuilderImpl() { @Override public S3AccessGrantsCachedCredentialsProviderImpl build() { - return new S3AccessGrantsCachedCredentialsProviderImpl(S3ControlAsyncClient, maxCacheSize, cacheExpirationTimePercentage); + return new S3AccessGrantsCachedCredentialsProviderImpl(maxCacheSize, cacheExpirationTimePercentage); } @Override public S3AccessGrantsCachedCredentialsProviderImpl buildWithAccountIdResolver() { - return new S3AccessGrantsCachedCredentialsProviderImpl(S3ControlAsyncClient, s3AccessGrantsCachedAccountIdResolver, maxCacheSize, cacheExpirationTimePercentage); - } - - @Override - public S3AccessGrantsCachedCredentialsProviderImpl.Builder S3ControlAsyncClient(S3ControlAsyncClient S3ControlAsyncClient) { - this.S3ControlAsyncClient = S3ControlAsyncClient; - return this; + return new S3AccessGrantsCachedCredentialsProviderImpl(s3AccessGrantsCachedAccountIdResolver, maxCacheSize, cacheExpirationTimePercentage); } @Override @@ -131,8 +122,8 @@ public Builder cacheExpirationTimePercentage(int cacheExpirationTimePercentage) } @Override - public CompletableFuture getDataAccess (AwsCredentialsIdentity credentials, Permission permission, - String s3Prefix, @NotNull String accountId) throws S3ControlException { + public CompletableFuture getDataAccess(AwsCredentialsIdentity credentials, Permission permission, + String s3Prefix, @NotNull String accountId, S3ControlAsyncClient s3ControlAsyncClient) throws S3ControlException { Instant start = Instant.now(); CacheKey cacheKey = CacheKey.builder() @@ -149,7 +140,7 @@ public CompletableFuture getDataAccess (AwsCredentialsId CompletableFuture accessGrantsCredentials; try { - accessGrantsCredentials = accessGrantsCache.getCredentials(cacheKey, accountId, s3AccessGrantsAccessDeniedCache); + accessGrantsCredentials = accessGrantsCache.getCredentials(cacheKey, accountId, s3AccessGrantsAccessDeniedCache, s3ControlAsyncClient); }catch (S3ControlException e) { collector.reportMetric(MetricsCollector.ERROR_COUNT,1); throw e; diff --git a/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsConstants.java b/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsConstants.java index 8d1b257..aa17d4f 100644 --- a/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsConstants.java +++ b/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsConstants.java @@ -18,7 +18,7 @@ public class S3AccessGrantsConstants { public static final int DEFAULT_ACCOUNT_ID_MAX_CACHE_SIZE = 1_000; public static final int MAX_LIMIT_ACCOUNT_ID_MAX_CACHE_SIZE = 1_000_000; - public static final int DEFAULT_ACCOUNT_ID_EXPIRE_CACHE_AFTER_WRITE_SECONDS = 86_400; // one day + public static final int DEFAULT_ACCOUNT_ID_EXPIRE_CACHE_AFTER_WRITE_SECONDS = 6_00; // 10 mins public static final int MAX_LIMIT_ACCOUNT_ID_EXPIRE_CACHE_AFTER_WRITE_SECONDS = 2_592_000; // 30 days public static final int DEFAULT_ACCESS_GRANTS_MAX_CACHE_SIZE = 30_000; @@ -27,4 +27,12 @@ public class S3AccessGrantsConstants { public static final int ACCESS_DENIED_CACHE_SIZE = 3_000; + public static final int BUCKET_REGION_CACHE_SIZE = 1_000; + + public static final int MAX_BUCKET_REGION_CACHE_SIZE = 1_000_000; + + public static final int MAX_BUCKET_REGION_EXPIRE_CACHE_AFTER_WRITE_SECONDS = 6_00; // 10 mins + + public static final int BUCKET_REGION_EXPIRE_CACHE_AFTER_WRITE_SECONDS= 3_00; // 5 mins + } diff --git a/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsBucketRegionResolverCreationTest.java b/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsBucketRegionResolverCreationTest.java new file mode 100644 index 0000000..4b76eb5 --- /dev/null +++ b/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsBucketRegionResolverCreationTest.java @@ -0,0 +1,117 @@ +package software.amazon.awssdk.s3accessgrants.cache; + +import org.assertj.core.api.Assertions; +import org.junit.Assert; +import org.junit.BeforeClass; +import org.junit.Test; +import software.amazon.awssdk.services.s3.S3Client; + +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; +import static org.mockito.Mockito.mock; +import static software.amazon.awssdk.s3accessgrants.cache.S3AccessGrantsConstants.*; + +public class S3AccessGrantsBucketRegionResolverCreationTest { + + private static int TEST_BUCKET_REGION_CACHE_SIZE = 5_000; + private static int TEST_CACHE_EXPIRATION_DURATION = 6_0; + + private static S3Client s3Client; + + @BeforeClass + public static void setUp() { + s3Client = mock(S3Client.class); + } + + @Test + public void create_bucket_region_cache_with_default_settings() { + S3AccessGrantsCachedBucketRegionResolver cachedBucketRegionResolver = S3AccessGrantsCachedBucketRegionResolver.builder().s3Client(s3Client).build(); + Assert.assertEquals(BUCKET_REGION_CACHE_SIZE, cachedBucketRegionResolver.getMaxCacheSize()); + Assert.assertEquals(BUCKET_REGION_EXPIRE_CACHE_AFTER_WRITE_SECONDS, cachedBucketRegionResolver.getExpireCacheAfterWriteSeconds()); + } + + @Test + public void create_bucket_region_cache_with_custom_settings() { + S3AccessGrantsCachedBucketRegionResolver cachedBucketRegionResolver = S3AccessGrantsCachedBucketRegionResolver + .builder() + .maxCacheSize(TEST_BUCKET_REGION_CACHE_SIZE) + .expireCacheAfterWriteSeconds(TEST_CACHE_EXPIRATION_DURATION) + .s3Client(s3Client) + .build(); + Assert.assertEquals(TEST_BUCKET_REGION_CACHE_SIZE, cachedBucketRegionResolver.getMaxCacheSize()); + Assert.assertEquals(TEST_CACHE_EXPIRATION_DURATION, cachedBucketRegionResolver.getExpireCacheAfterWriteSeconds()); + } + + @Test + public void create_bucket_region_cache_with_builder_default_settings() { + S3AccessGrantsCachedBucketRegionResolver cachedBucketRegionResolver = S3AccessGrantsCachedBucketRegionResolver.builder().s3Client(s3Client).build(); + Assert.assertEquals(BUCKET_REGION_CACHE_SIZE, cachedBucketRegionResolver.getMaxCacheSize()); + Assert.assertEquals(BUCKET_REGION_EXPIRE_CACHE_AFTER_WRITE_SECONDS, cachedBucketRegionResolver.getExpireCacheAfterWriteSeconds()); + } + + @Test + public void create_bucket_region_cache_with_invalid_max_cache_size() { + + Assertions.assertThatThrownBy(() -> S3AccessGrantsCachedBucketRegionResolver + .builder() + .maxCacheSize(MAX_BUCKET_REGION_CACHE_SIZE+1) + .expireCacheAfterWriteSeconds(BUCKET_REGION_EXPIRE_CACHE_AFTER_WRITE_SECONDS) + .s3Client(s3Client) + .build()).isInstanceOf(IllegalArgumentException.class); + + Assertions.assertThatThrownBy(() -> S3AccessGrantsCachedBucketRegionResolver + .builder() + .maxCacheSize(0) + .expireCacheAfterWriteSeconds(BUCKET_REGION_EXPIRE_CACHE_AFTER_WRITE_SECONDS) + .s3Client(s3Client) + .build()).isInstanceOf(IllegalArgumentException.class); + + } + + @Test + public void create_bucket_region_cache_with_invalid_expiration_duration() { + + Assertions.assertThatThrownBy(() -> S3AccessGrantsCachedBucketRegionResolver + .builder() + .maxCacheSize(BUCKET_REGION_CACHE_SIZE) + .expireCacheAfterWriteSeconds(MAX_BUCKET_REGION_EXPIRE_CACHE_AFTER_WRITE_SECONDS+10) + .s3Client(s3Client) + .build()).isInstanceOf(IllegalArgumentException.class); + + Assertions.assertThatThrownBy(() -> S3AccessGrantsCachedBucketRegionResolver + .builder() + .maxCacheSize(BUCKET_REGION_CACHE_SIZE) + .expireCacheAfterWriteSeconds(0) + .s3Client(s3Client) + .build()).isInstanceOf(IllegalArgumentException.class); + + } + + @Test + public void copy_Resolver() { + + S3AccessGrantsCachedBucketRegionResolver cachedBucketRegionResolver = S3AccessGrantsCachedBucketRegionResolver + .builder() + .maxCacheSize(TEST_BUCKET_REGION_CACHE_SIZE) + .expireCacheAfterWriteSeconds(TEST_CACHE_EXPIRATION_DURATION) + .s3Client(s3Client) + .build(); + S3AccessGrantsCachedBucketRegionResolver copy = cachedBucketRegionResolver.toBuilder().build(); + + Assert.assertEquals(TEST_BUCKET_REGION_CACHE_SIZE, copy.maxCacheSize()); + Assert.assertEquals(TEST_CACHE_EXPIRATION_DURATION, copy.expireCacheAfterWriteSeconds()); + + } + + @Test + public void test_invalidS3Client() { + Assertions.assertThatThrownBy(() -> S3AccessGrantsCachedBucketRegionResolver + .builder() + .maxCacheSize(TEST_BUCKET_REGION_CACHE_SIZE) + .s3Client(null) + .expireCacheAfterWriteSeconds(TEST_CACHE_EXPIRATION_DURATION) + .build()).isInstanceOf(IllegalArgumentException.class); + } + +} + + diff --git a/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsBucketRegionResolverTest.java b/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsBucketRegionResolverTest.java new file mode 100644 index 0000000..09b92af --- /dev/null +++ b/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsBucketRegionResolverTest.java @@ -0,0 +1,126 @@ +package software.amazon.awssdk.s3accessgrants.cache; + +import org.assertj.core.api.Assertions; +import org.junit.Assert; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; +import software.amazon.awssdk.awscore.exception.AwsErrorDetails; +import software.amazon.awssdk.awscore.exception.AwsServiceException; +import software.amazon.awssdk.core.exception.SdkServiceException; +import software.amazon.awssdk.http.SdkHttpResponse; +import software.amazon.awssdk.regions.Region; +import software.amazon.awssdk.services.s3.S3Client; +import software.amazon.awssdk.services.s3.model.HeadBucketRequest; +import software.amazon.awssdk.services.s3.model.HeadBucketResponse; +import software.amazon.awssdk.services.s3.model.S3Exception; + +import java.util.ArrayList; +import java.util.List; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.*; + +public class S3AccessGrantsBucketRegionResolverTest { + + private S3Client s3Client; + private S3AccessGrantsCachedBucketRegionResolver s3AccessGrantsCachedBucketRegionResolver; + private String TEST_BUCKET_NAME; + + @Before + public void setUp() { + s3Client = mock(S3Client.class); + TEST_BUCKET_NAME = "test-bucket"; + s3AccessGrantsCachedBucketRegionResolver = S3AccessGrantsCachedBucketRegionResolver.builder().s3Client(s3Client).build(); + HeadBucketResponse headBucketResponse = HeadBucketResponse.builder().bucketRegion(Region.US_EAST_1.toString()).build(); + when(s3Client.headBucket(any(HeadBucketRequest.class))).thenReturn(headBucketResponse); + } + + @Test + public void call_resolve_should_cache_the_bucket_region() { + Assert.assertEquals(s3AccessGrantsCachedBucketRegionResolver.resolve(TEST_BUCKET_NAME), Region.US_EAST_1); + // initial request should be made to the service + verify(s3Client, times(1)).headBucket(any(HeadBucketRequest.class)); + Assert.assertEquals(s3AccessGrantsCachedBucketRegionResolver.resolve(TEST_BUCKET_NAME), Region.US_EAST_1); + // No call should be made to the service as the region is already cached + verify(s3Client, times(1)).headBucket(any(HeadBucketRequest.class)); + } + + @Test + public void call_resolve_should_not_cache_the_bucket_region() { + S3Client localS3Client = mock(S3Client.class); + S3AccessGrantsCachedBucketRegionResolver localS3AccessGrantsCachedBucketRegionResolver = S3AccessGrantsCachedBucketRegionResolver.builder().s3Client(localS3Client).build(); + HeadBucketResponse headBucketResponse = HeadBucketResponse.builder().bucketRegion(null).build(); + when(localS3Client.headBucket(any(HeadBucketRequest.class))).thenReturn(headBucketResponse); + Assertions.assertThatThrownBy(() -> localS3AccessGrantsCachedBucketRegionResolver.resolve(TEST_BUCKET_NAME)).isInstanceOf(SdkServiceException.class); + verify(localS3Client, times(1)).headBucket(any(HeadBucketRequest.class)); + Assertions.assertThatThrownBy(() -> localS3AccessGrantsCachedBucketRegionResolver.resolve(TEST_BUCKET_NAME)).isInstanceOf(SdkServiceException.class); + verify(localS3Client, times(2)).headBucket(any(HeadBucketRequest.class)); + } + + @Test + public void verify_bucket_region_cache_expiration() throws InterruptedException { + + S3AccessGrantsCachedBucketRegionResolver localCachedBucketRegionResolver = S3AccessGrantsCachedBucketRegionResolver + .builder() + .s3Client(s3Client) + .expireCacheAfterWriteSeconds(1) + .build(); + + Assert.assertEquals(Region.US_EAST_1, localCachedBucketRegionResolver.resolve(TEST_BUCKET_NAME)); + verify(s3Client, times(1)).headBucket(any(HeadBucketRequest.class)); + Thread.sleep(2000); + // should evict the entry and the subsequent request should call the service + Assert.assertEquals(Region.US_EAST_1, localCachedBucketRegionResolver.resolve(TEST_BUCKET_NAME)); + verify(s3Client, times(2)).headBucket(any(HeadBucketRequest.class)); + + } + + @Test + public void call_bucket_region_cache_with_non_existent_bucket() throws InterruptedException { + + S3Client localS3Client = mock(S3Client.class); + S3AccessGrantsCachedBucketRegionResolver localS3AccessGrantsCachedBucketRegionResolver = S3AccessGrantsCachedBucketRegionResolver.builder().s3Client(localS3Client).build(); + when(localS3Client.headBucket(any(HeadBucketRequest.class))).thenThrow(S3Exception.builder().message("Bucket does not exist").statusCode(404).build()); + Assertions.assertThatThrownBy(() -> localS3AccessGrantsCachedBucketRegionResolver.resolve(TEST_BUCKET_NAME)).isInstanceOf(S3Exception.class); + verify(localS3Client, times(1)).headBucket(any(HeadBucketRequest.class)); + + } + + @Test + public void call_bucket_region_cache_resolve_returns_redirect() throws InterruptedException { + + S3Client localS3Client = mock(S3Client.class); + AwsServiceException s3Exception = mock(S3Exception.class); + S3AccessGrantsCachedBucketRegionResolver localS3AccessGrantsCachedBucketRegionResolver = S3AccessGrantsCachedBucketRegionResolver.builder().s3Client(localS3Client).build(); + List regionList = new ArrayList<>(); + regionList.add(Region.US_EAST_1.toString()); + SdkHttpResponse sdkHttpResponse = SdkHttpResponse.builder().putHeader("x-amz-bucket-region", regionList).build(); + AwsErrorDetails awsErrorDetails = AwsErrorDetails.builder().sdkHttpResponse(sdkHttpResponse).build(); + when(s3Exception.statusCode()).thenReturn(301); + when(s3Exception.awsErrorDetails()).thenReturn(awsErrorDetails); + when(localS3Client.headBucket(any(HeadBucketRequest.class))).thenThrow(s3Exception); + Assert.assertEquals(Region.US_EAST_1, localS3AccessGrantsCachedBucketRegionResolver.resolve(TEST_BUCKET_NAME)); + verify(localS3Client, times(1)).headBucket(any(HeadBucketRequest.class)); + + } + + @Test + public void call_bucket_region_cache_resolve_returns_redirect_with_null_region() throws InterruptedException { + + S3Client localS3Client = mock(S3Client.class); + AwsServiceException s3Exception = mock(S3Exception.class); + S3AccessGrantsCachedBucketRegionResolver localS3AccessGrantsCachedBucketRegionResolver = S3AccessGrantsCachedBucketRegionResolver.builder().s3Client(localS3Client).build(); + List regionList = new ArrayList<>(); + regionList.add(null); + SdkHttpResponse sdkHttpResponse = SdkHttpResponse.builder().putHeader("x-amz-bucket-region", regionList).build(); + AwsErrorDetails awsErrorDetails = AwsErrorDetails.builder().sdkHttpResponse(sdkHttpResponse).build(); + when(s3Exception.statusCode()).thenReturn(301); + when(s3Exception.awsErrorDetails()).thenReturn(awsErrorDetails); + when(localS3Client.headBucket(any(HeadBucketRequest.class))).thenThrow(s3Exception); + Assertions.assertThatThrownBy(() -> localS3AccessGrantsCachedBucketRegionResolver.resolve(TEST_BUCKET_NAME)) + .isInstanceOf(SdkServiceException.class); + + } + +} diff --git a/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCacheTest.java b/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCacheTest.java index 744bd0d..8b42a60 100644 --- a/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCacheTest.java +++ b/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCacheTest.java @@ -15,7 +15,7 @@ package software.amazon.awssdk.s3accessgrants.cache; -import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.assertj.core.api.Assertions.assertThatNoException; import static org.assertj.core.api.AssertionsForClassTypes.assertThat; import java.time.Duration; @@ -66,11 +66,9 @@ public void setup(){ mockResolver = Mockito.mock(S3AccessGrantsCachedAccountIdResolver.class); s3ControlAsyncClient = Mockito.mock(S3ControlAsyncClient.class); cache = S3AccessGrantsCache.builder() - .s3ControlAsyncClient(s3ControlAsyncClient) .cacheExpirationTimePercentage(60) .maxCacheSize(DEFAULT_ACCESS_GRANTS_MAX_CACHE_SIZE).build(); cacheWithMockedAccountIdResolver = S3AccessGrantsCache.builder() - .s3ControlAsyncClient(s3ControlAsyncClient) .cacheExpirationTimePercentage(60) .s3AccessGrantsCachedAccountIdResolver(mockResolver) .maxCacheSize(DEFAULT_ACCESS_GRANTS_MAX_CACHE_SIZE).buildWithAccountIdResolver(); @@ -115,8 +113,8 @@ public void accessGrantsCache_accessGrantsCacheHit() { .permission(Permission.READ) .s3Prefix("s3://bucket2/foo/bar").build(); // When - AwsCredentialsIdentity cacheValue1 = cache.getCredentials(key1, TEST_S3_ACCESSGRANTS_ACCOUNT, accessDeniedCache).join(); - AwsCredentialsIdentity cacheValue2 = cache.getCredentials(key2, TEST_S3_ACCESSGRANTS_ACCOUNT, accessDeniedCache).join(); + AwsCredentialsIdentity cacheValue1 = cache.getCredentials(key1, TEST_S3_ACCESSGRANTS_ACCOUNT, accessDeniedCache, s3ControlAsyncClient).join(); + AwsCredentialsIdentity cacheValue2 = cache.getCredentials(key2, TEST_S3_ACCESSGRANTS_ACCOUNT, accessDeniedCache, s3ControlAsyncClient).join(); // Then assertThat(cacheValue2).isEqualTo(cacheValue1); } @@ -139,8 +137,8 @@ public void accessGrantsCache_grantPresentForHigherLevelPrefix() { .permission(Permission.READ) .s3Prefix("s3://bucket2/foo/bar/logs").build(); // When - AwsCredentialsIdentity cacheValue1 = cache.getCredentials(key1, TEST_S3_ACCESSGRANTS_ACCOUNT, accessDeniedCache).join(); - AwsCredentialsIdentity cacheValue2 = cache.getCredentials(key2, TEST_S3_ACCESSGRANTS_ACCOUNT, accessDeniedCache).join(); + AwsCredentialsIdentity cacheValue1 = cache.getCredentials(key1, TEST_S3_ACCESSGRANTS_ACCOUNT, accessDeniedCache, s3ControlAsyncClient).join(); + AwsCredentialsIdentity cacheValue2 = cache.getCredentials(key2, TEST_S3_ACCESSGRANTS_ACCOUNT, accessDeniedCache, s3ControlAsyncClient).join(); // Then assertThat(cacheValue2).isEqualTo(cacheValue1); } @@ -160,8 +158,8 @@ public void accessGrantsCache_readRequestShouldCheckForExistingReadWriteGrant() .permission(Permission.READ) .s3Prefix("s3://bucket2/foo/bar/logs").build(); // When - AwsCredentialsIdentity cacheValue1 = cache.getCredentials(key1, TEST_S3_ACCESSGRANTS_ACCOUNT, accessDeniedCache).join(); - AwsCredentialsIdentity cacheValue2 = cache.getCredentials(key2, TEST_S3_ACCESSGRANTS_ACCOUNT, accessDeniedCache).join(); + AwsCredentialsIdentity cacheValue1 = cache.getCredentials(key1, TEST_S3_ACCESSGRANTS_ACCOUNT, accessDeniedCache, s3ControlAsyncClient).join(); + AwsCredentialsIdentity cacheValue2 = cache.getCredentials(key2, TEST_S3_ACCESSGRANTS_ACCOUNT, accessDeniedCache, s3ControlAsyncClient).join(); // Then assertThat(cacheValue2).isEqualTo(cacheValue1); } @@ -185,12 +183,12 @@ public void accessGrantsCache_testCacheExpiry() throws Exception { .s3Prefix("s3://bucket2/foo/bar").build(); assertThat(S3_ACCESS_GRANTS_CREDENTIALS).isEqualTo(cache.getCredentials(key2, TEST_S3_ACCESSGRANTS_ACCOUNT, - accessDeniedCache).join()); + accessDeniedCache, s3ControlAsyncClient).join()); // When Thread.sleep(3000); - when(mockResolver.resolve(any(String.class), any(String.class))).thenReturn(TEST_S3_ACCESSGRANTS_ACCOUNT); + when(mockResolver.resolve(any(String.class), any(String.class), any(S3ControlAsyncClient.class))).thenReturn(TEST_S3_ACCESSGRANTS_ACCOUNT); when(s3ControlAsyncClient.getDataAccess(any(GetDataAccessRequest.class))).thenReturn(getDataAccessResponseSetUp("s3://bucket2/foo/bar")); - cacheWithMockedAccountIdResolver.getCredentials(key1, TEST_S3_ACCESSGRANTS_ACCOUNT, accessDeniedCache).join(); + cacheWithMockedAccountIdResolver.getCredentials(key1, TEST_S3_ACCESSGRANTS_ACCOUNT, accessDeniedCache, s3ControlAsyncClient).join(); // Then verify(s3ControlAsyncClient, atLeastOnce()).getDataAccess(any(GetDataAccessRequest.class)); } @@ -205,10 +203,10 @@ public void accessGrantsCache_accessGrantsCacheMiss() { .s3Prefix("s3://bucket/foo/bar").build(); CompletableFuture getDataAccessResponse = getDataAccessResponseSetUp("s3://bucket2/foo/bar"); - when(mockResolver.resolve(any(String.class), any(String.class))).thenReturn(TEST_S3_ACCESSGRANTS_ACCOUNT); + when(mockResolver.resolve(any(String.class), any(String.class), any(S3ControlAsyncClient.class))).thenReturn(TEST_S3_ACCESSGRANTS_ACCOUNT); when(s3ControlAsyncClient.getDataAccess(any(GetDataAccessRequest.class))).thenReturn(getDataAccessResponse); // When - cacheWithMockedAccountIdResolver.getCredentials(key, TEST_S3_ACCESSGRANTS_ACCOUNT, accessDeniedCache).join(); + cacheWithMockedAccountIdResolver.getCredentials(key, TEST_S3_ACCESSGRANTS_ACCOUNT, accessDeniedCache, s3ControlAsyncClient).join(); // Then verify(s3ControlAsyncClient, atLeastOnce()).getDataAccess(any(GetDataAccessRequest.class)); } @@ -229,10 +227,10 @@ public void accessGrantsCache_accessGrantsCacheMissForDifferentPermissions() { CompletableFuture getDataAccessResponse = getDataAccessResponseSetUp("s3://bucket2/foo/bar"); - when(mockResolver.resolve(any(String.class), any(String.class))).thenReturn(TEST_S3_ACCESSGRANTS_ACCOUNT); + when(mockResolver.resolve(any(String.class), any(String.class), any(S3ControlAsyncClient.class))).thenReturn(TEST_S3_ACCESSGRANTS_ACCOUNT); when(s3ControlAsyncClient.getDataAccess(any(GetDataAccessRequest.class))).thenReturn(getDataAccessResponse); // When - cacheWithMockedAccountIdResolver.getCredentials(key2, TEST_S3_ACCESSGRANTS_ACCOUNT, accessDeniedCache).join(); + cacheWithMockedAccountIdResolver.getCredentials(key2, TEST_S3_ACCESSGRANTS_ACCOUNT, accessDeniedCache, s3ControlAsyncClient).join(); // Then verify(s3ControlAsyncClient, atLeastOnce()).getDataAccess(any(GetDataAccessRequest.class)); @@ -240,10 +238,8 @@ public void accessGrantsCache_accessGrantsCacheMissForDifferentPermissions() { @Test public void accessGrantsCache_testNullS3ControlAsyncClientException() { - assertThatThrownBy(() -> S3AccessGrantsCache.builder() - .s3ControlAsyncClient(null) - .maxCacheSize(DEFAULT_ACCESS_GRANTS_MAX_CACHE_SIZE).build()) - .isInstanceOf(IllegalArgumentException.class); + assertThatNoException().isThrownBy(() -> S3AccessGrantsCache.builder() + .maxCacheSize(DEFAULT_ACCESS_GRANTS_MAX_CACHE_SIZE).build()); } @@ -257,12 +253,12 @@ public void accessGrantsCache_throwsS3ControlException() { S3ControlException s3ControlException = Mockito.mock(S3ControlException.class); - when(mockResolver.resolve(any(String.class), any(String.class))).thenReturn(TEST_S3_ACCESSGRANTS_ACCOUNT); + when(mockResolver.resolve(any(String.class), any(String.class), any(S3ControlAsyncClient.class))).thenReturn(TEST_S3_ACCESSGRANTS_ACCOUNT); when(s3ControlAsyncClient.getDataAccess(any(GetDataAccessRequest.class))).thenThrow(s3ControlException); when(s3ControlException.statusCode()).thenReturn(403); // When try { - cacheWithMockedAccountIdResolver.getCredentials(key1, TEST_S3_ACCESSGRANTS_ACCOUNT, accessDeniedCache).join(); + cacheWithMockedAccountIdResolver.getCredentials(key1, TEST_S3_ACCESSGRANTS_ACCOUNT, accessDeniedCache, s3ControlAsyncClient).join(); }catch (S3ControlException e){} // Then assertThat(accessDeniedCache.getValueFromCache(key1)).isInstanceOf(S3ControlException.class); @@ -297,18 +293,19 @@ public void accessGrantsCache_testGrantPresentForLocation() { .permission(Permission.READ) .s3Prefix("s3://bucket/foo/bar/text.txt").build(); - when(mockResolver.resolve(any(String.class), any(String.class))).thenReturn(TEST_S3_ACCESSGRANTS_ACCOUNT); + when(mockResolver.resolve(any(String.class), any(String.class), any(S3ControlAsyncClient.class))).thenReturn(TEST_S3_ACCESSGRANTS_ACCOUNT); when(s3ControlAsyncClient.getDataAccess(any(GetDataAccessRequest.class))).thenReturn(getDataAccessResponse); - cacheWithMockedAccountIdResolver.getCredentials(key1, TEST_S3_ACCESSGRANTS_ACCOUNT,accessDeniedCache).join(); + cacheWithMockedAccountIdResolver.getCredentials(key1, TEST_S3_ACCESSGRANTS_ACCOUNT,accessDeniedCache, s3ControlAsyncClient).join(); // When CacheKey key2 = CacheKey.builder() .credentials(AWS_BASIC_CREDENTIALS) .permission(Permission.READ) .s3Prefix("s3://bucket/foo/log/text.txt").build(); - cacheWithMockedAccountIdResolver.getCredentials(key2, TEST_S3_ACCESSGRANTS_ACCOUNT,accessDeniedCache).join(); + cacheWithMockedAccountIdResolver.getCredentials(key2, TEST_S3_ACCESSGRANTS_ACCOUNT,accessDeniedCache, s3ControlAsyncClient).join(); // Then verify(s3ControlAsyncClient, times(1)).getDataAccess(any(GetDataAccessRequest.class)); } + @Test public void accessGrantsCache_testGrantWithPrefix() { // Given @@ -322,7 +319,7 @@ public void accessGrantsCache_testGrantWithPrefix() { .s3Prefix("s3://bucket2/foo/text.txt").build(); cache.putValueInCache(key1, CompletableFuture.supplyAsync(() -> S3_ACCESS_GRANTS_CREDENTIALS), 2); // When - AwsCredentialsIdentity cacheValue1 = cache.getCredentials(key2, TEST_S3_ACCESSGRANTS_ACCOUNT, accessDeniedCache).join(); + AwsCredentialsIdentity cacheValue1 = cache.getCredentials(key2, TEST_S3_ACCESSGRANTS_ACCOUNT, accessDeniedCache, s3ControlAsyncClient).join(); // Then assertThat(cacheValue1).isEqualTo(S3_ACCESS_GRANTS_CREDENTIALS); verify(s3ControlAsyncClient, times(0)).getDataAccess(any(GetDataAccessRequest.class)); @@ -346,10 +343,10 @@ public void accessGrantsCache_testPutValueInCacheForObjectLevelGrant() { .permission(Permission.READ) .s3Prefix("s3://bucket/foo/bar/text.txt").build(); // When - when(mockResolver.resolve(any(String.class), any(String.class))).thenReturn(TEST_S3_ACCESSGRANTS_ACCOUNT); + when(mockResolver.resolve(any(String.class), any(String.class), any(S3ControlAsyncClient.class))).thenReturn(TEST_S3_ACCESSGRANTS_ACCOUNT); when(s3ControlAsyncClient.getDataAccess(any(GetDataAccessRequest.class))).thenReturn(getDataAccessResponse); - cacheWithMockedAccountIdResolver.getCredentials(key, TEST_S3_ACCESSGRANTS_ACCOUNT,accessDeniedCache).join(); - cacheWithMockedAccountIdResolver.getCredentials(key, TEST_S3_ACCESSGRANTS_ACCOUNT,accessDeniedCache).join(); + cacheWithMockedAccountIdResolver.getCredentials(key, TEST_S3_ACCESSGRANTS_ACCOUNT,accessDeniedCache, s3ControlAsyncClient).join(); + cacheWithMockedAccountIdResolver.getCredentials(key, TEST_S3_ACCESSGRANTS_ACCOUNT,accessDeniedCache, s3ControlAsyncClient).join(); // Then verify(s3ControlAsyncClient, times(2)).getDataAccess(any(GetDataAccessRequest.class)); diff --git a/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedAccountIdResolverCreationTest.java b/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedAccountIdResolverCreationTest.java index 0cf477b..75b3612 100644 --- a/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedAccountIdResolverCreationTest.java +++ b/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedAccountIdResolverCreationTest.java @@ -15,6 +15,7 @@ package software.amazon.awssdk.s3accessgrants.cache; +import static org.assertj.core.api.Assertions.assertThatNoException; import static org.assertj.core.api.AssertionsForClassTypes.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static software.amazon.awssdk.s3accessgrants.cache.S3AccessGrantsConstants.DEFAULT_ACCOUNT_ID_EXPIRE_CACHE_AFTER_WRITE_SECONDS; @@ -33,28 +34,19 @@ public void setup() { S3ControlAsyncClient = Mockito.mock(S3ControlAsyncClient.class); } - @Test - public void create_DefaultResolver_without_S3ControlAsyncClient_via_Constructor() { - // Given - S3ControlAsyncClient = null; - // Then - assertThatIllegalArgumentException().isThrownBy(() -> new S3AccessGrantsCachedAccountIdResolver(S3ControlAsyncClient)); - } - @Test public void create_DefaultResolver_via_Constructor() { // When - S3AccessGrantsCachedAccountIdResolver resolver = new S3AccessGrantsCachedAccountIdResolver(S3ControlAsyncClient); + S3AccessGrantsCachedAccountIdResolver resolver = new S3AccessGrantsCachedAccountIdResolver(); // Then assertThat(resolver).isNotNull(); - assertThat(resolver.S3ControlAsyncClient()).isEqualTo(S3ControlAsyncClient); assertThat(resolver.maxCacheSize()).isEqualTo(DEFAULT_ACCOUNT_ID_MAX_CACHE_SIZE); assertThat(resolver.expireCacheAfterWriteSeconds()).isEqualTo(DEFAULT_ACCOUNT_ID_EXPIRE_CACHE_AFTER_WRITE_SECONDS); } @Test public void create_Resolver_via_Builder() { - assertThatIllegalArgumentException().isThrownBy(() -> S3AccessGrantsCachedAccountIdResolver + assertThatNoException().isThrownBy(() -> S3AccessGrantsCachedAccountIdResolver .builder() .build()); } @@ -64,9 +56,8 @@ public void create_Resolver_without_S3ControlAsyncClient_via_Builder() { // Given S3ControlAsyncClient = null; //Then - assertThatIllegalArgumentException().isThrownBy(() -> S3AccessGrantsCachedAccountIdResolver + assertThatNoException().isThrownBy(() -> S3AccessGrantsCachedAccountIdResolver .builder() - .S3ControlAsyncClient(S3ControlAsyncClient) .build()); } @@ -75,11 +66,9 @@ public void create_DefaultResolver_via_Builder() { // When S3AccessGrantsCachedAccountIdResolver resolver = S3AccessGrantsCachedAccountIdResolver .builder() - .S3ControlAsyncClient(S3ControlAsyncClient) .build(); // Then assertThat(resolver).isNotNull(); - assertThat(resolver.S3ControlAsyncClient()).isEqualTo(S3ControlAsyncClient); assertThat(resolver.maxCacheSize()).isEqualTo(DEFAULT_ACCOUNT_ID_MAX_CACHE_SIZE); assertThat(resolver.expireCacheAfterWriteSeconds()).isEqualTo(DEFAULT_ACCOUNT_ID_EXPIRE_CACHE_AFTER_WRITE_SECONDS); } @@ -92,13 +81,11 @@ public void create_FullCustomizedResolver() { // When S3AccessGrantsCachedAccountIdResolver resolver = S3AccessGrantsCachedAccountIdResolver .builder() - .S3ControlAsyncClient(S3ControlAsyncClient) .maxCacheSize(customMaxCacheSize) .expireCacheAfterWriteSeconds(customExpireCacheAfterWriteSeconds) .build(); // Then assertThat(resolver).isNotNull(); - assertThat(resolver.S3ControlAsyncClient()).isEqualTo(S3ControlAsyncClient); assertThat(resolver.maxCacheSize()).isEqualTo(customMaxCacheSize); assertThat(resolver.expireCacheAfterWriteSeconds()).isEqualTo(customExpireCacheAfterWriteSeconds); } @@ -110,7 +97,6 @@ public void create_CustomizedResolver_exceeds_MaxCacheSize() { // Then assertThatIllegalArgumentException().isThrownBy(() -> S3AccessGrantsCachedAccountIdResolver .builder() - .S3ControlAsyncClient(S3ControlAsyncClient) .maxCacheSize(customMaxCacheSize) .build()); } @@ -122,7 +108,6 @@ public void create_CustomizedResolver_exceeds_ExpireCacheAfterWriteSeconds() { // Then assertThatIllegalArgumentException().isThrownBy(() -> S3AccessGrantsCachedAccountIdResolver .builder() - .S3ControlAsyncClient(S3ControlAsyncClient) .expireCacheAfterWriteSeconds(customExpireCacheAfterWriteSeconds) .build()); } @@ -135,14 +120,12 @@ public void copy_Resolver() { // When S3AccessGrantsCachedAccountIdResolver resolver = S3AccessGrantsCachedAccountIdResolver .builder() - .S3ControlAsyncClient(S3ControlAsyncClient) .maxCacheSize(customMaxCacheSize) .expireCacheAfterWriteSeconds(customExpireCacheAfterWriteSeconds) .build(); S3AccessGrantsCachedAccountIdResolver copy = resolver.toBuilder().build(); // Then assertThat(copy).isNotNull(); - assertThat(copy.S3ControlAsyncClient()).isEqualTo(S3ControlAsyncClient); assertThat(copy.maxCacheSize()).isEqualTo(customMaxCacheSize); assertThat(copy.expireCacheAfterWriteSeconds()).isEqualTo(customExpireCacheAfterWriteSeconds); } diff --git a/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedAccountIdResolverTest.java b/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedAccountIdResolverTest.java index 42d7493..1789f5d 100644 --- a/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedAccountIdResolverTest.java +++ b/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedAccountIdResolverTest.java @@ -47,7 +47,6 @@ public void setup() { S3ControlAsyncClient = Mockito.mock(S3ControlAsyncClient.class); resolver = S3AccessGrantsCachedAccountIdResolver .builder() - .S3ControlAsyncClient(S3ControlAsyncClient) .build(); } @@ -64,7 +63,7 @@ public void resolver_Returns_ExpectedAccountId() throws S3ControlException { when(S3ControlAsyncClient.getAccessGrantsInstanceForPrefix(any(GetAccessGrantsInstanceForPrefixRequest.class))) .thenReturn(response); // When - String accountId = resolver.resolve(TEST_S3_ACCESSGRANTS_ACCOUNT, TEST_S3_PREFIX); + String accountId = resolver.resolve(TEST_S3_ACCESSGRANTS_ACCOUNT, TEST_S3_PREFIX, S3ControlAsyncClient); // Then assertThat(accountId).isEqualTo(TEST_S3_ACCESSGRANTS_ACCOUNT); verify(S3ControlAsyncClient, times(1)).getAccessGrantsInstanceForPrefix(requestArgumentCaptor.capture()); @@ -84,8 +83,8 @@ public void resolver_Returns_CachedAccountId() throws S3ControlException { .accessGrantsInstanceArn(TEST_S3_ACCESSGRANTS_INSTANCE_ARN).build()); when(S3ControlAsyncClient.getAccessGrantsInstanceForPrefix(any(GetAccessGrantsInstanceForPrefixRequest.class))).thenReturn(response); // When attempting to resolve same prefix back to back - String accountId1 = resolver.resolve(TEST_S3_ACCESSGRANTS_ACCOUNT, TEST_S3_PREFIX); - String accountId2 = resolver.resolve(TEST_S3_ACCESSGRANTS_ACCOUNT, TEST_S3_PREFIX); + String accountId1 = resolver.resolve(TEST_S3_ACCESSGRANTS_ACCOUNT, TEST_S3_PREFIX, S3ControlAsyncClient); + String accountId2 = resolver.resolve(TEST_S3_ACCESSGRANTS_ACCOUNT, TEST_S3_PREFIX, S3ControlAsyncClient); // Then assertThat(accountId1).isEqualTo(TEST_S3_ACCESSGRANTS_ACCOUNT); assertThat(accountId2).isEqualTo(TEST_S3_ACCESSGRANTS_ACCOUNT); @@ -105,8 +104,8 @@ public void resolver_Returns_CachedAccountId_of_Same_Bucket() throws S3ControlEx .accessGrantsInstanceArn(TEST_S3_ACCESSGRANTS_INSTANCE_ARN).build()); when(S3ControlAsyncClient.getAccessGrantsInstanceForPrefix(any(GetAccessGrantsInstanceForPrefixRequest.class))).thenReturn(response); // When attempting to resolve same prefix back to back - String accountId1 = resolver.resolve(TEST_S3_ACCESSGRANTS_ACCOUNT, TEST_S3_PREFIX); - String accountId2 = resolver.resolve(TEST_S3_ACCESSGRANTS_ACCOUNT, TEST_S3_PREFIX_2); + String accountId1 = resolver.resolve(TEST_S3_ACCESSGRANTS_ACCOUNT, TEST_S3_PREFIX, S3ControlAsyncClient); + String accountId2 = resolver.resolve(TEST_S3_ACCESSGRANTS_ACCOUNT, TEST_S3_PREFIX_2, S3ControlAsyncClient); // Then assertThat(accountId1).isEqualTo(TEST_S3_ACCESSGRANTS_ACCOUNT); assertThat(accountId2).isEqualTo(TEST_S3_ACCESSGRANTS_ACCOUNT); @@ -120,7 +119,7 @@ public void resolver_Rethrow_S3ControlException_On_ServiceError() { when(S3ControlAsyncClient.getAccessGrantsInstanceForPrefix(any(GetAccessGrantsInstanceForPrefixRequest.class))) .thenThrow(S3ControlException.builder().build()); // Then - assertThatThrownBy(() -> resolver.resolve(TEST_S3_ACCESSGRANTS_ACCOUNT, TEST_S3_PREFIX)).isInstanceOf(S3ControlException.class); + assertThatThrownBy(() -> resolver.resolve(TEST_S3_ACCESSGRANTS_ACCOUNT, TEST_S3_PREFIX, S3ControlAsyncClient)).isInstanceOf(S3ControlException.class); } @@ -133,7 +132,7 @@ public void resolver_Throw_S3ControlException_On_Empty_ResponseArn() { .accessGrantsInstanceArn("").build()); when(S3ControlAsyncClient.getAccessGrantsInstanceForPrefix(any(GetAccessGrantsInstanceForPrefixRequest.class))).thenReturn(response); // Then - assertThatThrownBy(() -> resolver.resolve(TEST_S3_ACCESSGRANTS_ACCOUNT, TEST_S3_PREFIX)).isInstanceOf(S3ControlException.class); + assertThatThrownBy(() -> resolver.resolve(TEST_S3_ACCESSGRANTS_ACCOUNT, TEST_S3_PREFIX, S3ControlAsyncClient)).isInstanceOf(S3ControlException.class); } } diff --git a/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedCredentialsProviderImplTest.java b/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedCredentialsProviderImplTest.java index 299371d..846c0d1 100644 --- a/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedCredentialsProviderImplTest.java +++ b/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedCredentialsProviderImplTest.java @@ -18,6 +18,7 @@ import static org.assertj.core.api.AssertionsForClassTypes.assertThat; import static org.mockito.Mockito.any; import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static software.amazon.awssdk.s3accessgrants.cache.S3AccessGrantsTestConstants.ACCESS_KEY_ID; @@ -36,6 +37,7 @@ import software.amazon.awssdk.auth.credentials.AwsSessionCredentials; import software.amazon.awssdk.identity.spi.AwsCredentialsIdentity; import software.amazon.awssdk.services.s3control.S3ControlAsyncClient; +import software.amazon.awssdk.services.s3control.S3ControlAsyncClientBuilder; import software.amazon.awssdk.services.s3control.model.Credentials; import software.amazon.awssdk.services.s3control.model.GetDataAccessRequest; import software.amazon.awssdk.services.s3control.model.GetDataAccessResponse; @@ -44,16 +46,14 @@ public class S3AccessGrantsCachedCredentialsProviderImplTest { S3AccessGrantsCachedCredentialsProviderImpl cache; S3AccessGrantsCachedCredentialsProviderImpl cacheWithMockedAccountIdResolver; - static S3ControlAsyncClient S3ControlAsyncClient = Mockito.mock(S3ControlAsyncClient.class); + static S3ControlAsyncClient S3ControlAsyncClient; static S3AccessGrantsCachedAccountIdResolver mockResolver = Mockito.mock(S3AccessGrantsCachedAccountIdResolver.class); static Credentials credentials; @Before public void setup() { - cache = S3AccessGrantsCachedCredentialsProviderImpl.builder() - .S3ControlAsyncClient(S3ControlAsyncClient).build(); + cache = S3AccessGrantsCachedCredentialsProviderImpl.builder().build(); cacheWithMockedAccountIdResolver = S3AccessGrantsCachedCredentialsProviderImpl.builder() - .S3ControlAsyncClient(S3ControlAsyncClient) .s3AccessGrantsCachedAccountIdResolver(mockResolver) .buildWithAccountIdResolver(); } @@ -79,10 +79,11 @@ public CompletableFuture getDataAccessResponseSetUp(Strin @Test public void cacheImpl_cacheHit() { // Given - CompletableFuture getDataAccessResponse = getDataAccessResponseSetUp("s3://bucket2/foo/bar"); - when(mockResolver.resolve(any(String.class), any(String.class))).thenReturn(TEST_S3_ACCESSGRANTS_ACCOUNT); + S3ControlAsyncClient = Mockito.mock(S3ControlAsyncClient.class); + CompletableFuture getDataAccessResponse = getDataAccessResponseSetUp("s3://bucket2/foo/*"); + when(mockResolver.resolve(any(String.class), any(String.class), any(S3ControlAsyncClient.class))).thenReturn(TEST_S3_ACCESSGRANTS_ACCOUNT); when(S3ControlAsyncClient.getDataAccess(any(GetDataAccessRequest.class))).thenReturn(getDataAccessResponse); - cacheWithMockedAccountIdResolver.getDataAccess(AWS_SESSION_CREDENTIALS, Permission.READ, "s3://bucket2/foo/bar", TEST_S3_ACCESSGRANTS_ACCOUNT); + cacheWithMockedAccountIdResolver.getDataAccess(AWS_SESSION_CREDENTIALS, Permission.READ, "s3://bucket2/foo/bar", TEST_S3_ACCESSGRANTS_ACCOUNT, S3ControlAsyncClient); AwsSessionCredentials sessionCredentials = AwsSessionCredentials.builder().accessKeyId(credentials.accessKeyId()) .secretAccessKey(credentials.secretAccessKey()) .sessionToken(credentials.sessionToken()).build(); @@ -90,23 +91,25 @@ public void cacheImpl_cacheHit() { AwsCredentialsIdentity credentialsIdentity = cacheWithMockedAccountIdResolver.getDataAccess(AWS_SESSION_CREDENTIALS, Permission.READ, "s3://bucket2/foo/bar", - TEST_S3_ACCESSGRANTS_ACCOUNT).join(); + TEST_S3_ACCESSGRANTS_ACCOUNT, S3ControlAsyncClient).join(); // Then assertThat(credentialsIdentity).isEqualTo(sessionCredentials); + verify(S3ControlAsyncClient, times(1)).getDataAccess(any(GetDataAccessRequest.class)); } @Test public void cacheImpl_cacheMiss() { // Given + S3ControlAsyncClient = Mockito.mock(S3ControlAsyncClient.class); CompletableFuture getDataAccessResponse = getDataAccessResponseSetUp("s3://bucket2/foo/bar"); - when(mockResolver.resolve(any(String.class), any(String.class))).thenReturn(TEST_S3_ACCESSGRANTS_ACCOUNT); + when(mockResolver.resolve(any(String.class), any(String.class), any(S3ControlAsyncClient.class))).thenReturn(TEST_S3_ACCESSGRANTS_ACCOUNT); when(S3ControlAsyncClient.getDataAccess(any(GetDataAccessRequest.class))).thenReturn(getDataAccessResponse); // When - cacheWithMockedAccountIdResolver.getDataAccess(AWS_SESSION_CREDENTIALS, Permission.READ, "s3://bucket2/foo/bar", TEST_S3_ACCESSGRANTS_ACCOUNT); + cacheWithMockedAccountIdResolver.getDataAccess(AWS_SESSION_CREDENTIALS, Permission.READ, "s3://bucket2/foo/bar", TEST_S3_ACCESSGRANTS_ACCOUNT, S3ControlAsyncClient); + cacheWithMockedAccountIdResolver.getDataAccess(AWS_SESSION_CREDENTIALS, Permission.READ, "s3://bucket2/foo/bar", TEST_S3_ACCESSGRANTS_ACCOUNT, S3ControlAsyncClient); // Then - verify(S3ControlAsyncClient, atLeastOnce()).getDataAccess(any(GetDataAccessRequest.class)); + verify(S3ControlAsyncClient, times(2)).getDataAccess(any(GetDataAccessRequest.class)); } - } From 8d9a90c998efc0fe0482c8cbe315e77b40cd4fd0 Mon Sep 17 00:00:00 2001 From: Shiva Kumar Mukkapati Date: Tue, 9 Jan 2024 11:24:32 -0500 Subject: [PATCH 2/3] optimizing imports, and refactoring code --- ...AccessGrantsCachedBucketRegionResolver.java | 18 +++++++++++------- ...GrantsBucketRegionResolverCreationTest.java | 10 ++++++---- ...S3AccessGrantsBucketRegionResolverTest.java | 6 ++++-- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedBucketRegionResolver.java b/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedBucketRegionResolver.java index 0677d1d..db545f9 100644 --- a/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedBucketRegionResolver.java +++ b/src/main/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsCachedBucketRegionResolver.java @@ -32,6 +32,10 @@ import static software.amazon.awssdk.s3accessgrants.cache.S3AccessGrantsConstants.MAX_BUCKET_REGION_EXPIRE_CACHE_AFTER_WRITE_SECONDS; import static software.amazon.awssdk.s3accessgrants.cache.S3AccessGrantsConstants.MAX_BUCKET_REGION_CACHE_SIZE; +/* +* A layer for caching bucket regions. +* This cache is specifically to avoid head bucket throttling. +* */ public class S3AccessGrantsCachedBucketRegionResolver implements S3AccessGrantsBucketRegionResolver { @@ -151,6 +155,10 @@ public S3Client s3Client() { return s3Client; } + public int expireCacheAfterWriteSeconds() { + return expireCacheAfterWriteSeconds; + } + @Override public S3AccessGrantsCachedBucketRegionResolver.Builder maxCacheSize(int maxCacheSize) { if (maxCacheSize <= 0 || maxCacheSize > MAX_BUCKET_REGION_CACHE_SIZE) { @@ -162,16 +170,12 @@ public S3AccessGrantsCachedBucketRegionResolver.Builder maxCacheSize(int maxCach } @Override - public Builder s3Client(S3Client s3Client) { - if(s3Client == null) throw new IllegalArgumentException("S3 Client is required while configuring the S3 Bucket Region resolver!"); + public S3AccessGrantsCachedBucketRegionResolver.Builder s3Client(S3Client s3Client) { + if (s3Client == null) + throw new IllegalArgumentException("S3 Client is required while configuring the S3 Bucket Region resolver!"); this.s3Client = s3Client; return this; } - - public int expireCacheAfterWriteSeconds() { - return expireCacheAfterWriteSeconds; - } - @Override public S3AccessGrantsCachedBucketRegionResolver.Builder expireCacheAfterWriteSeconds(int expireCacheAfterWriteSeconds) { if (expireCacheAfterWriteSeconds <= 0 || expireCacheAfterWriteSeconds > MAX_BUCKET_REGION_EXPIRE_CACHE_AFTER_WRITE_SECONDS) { diff --git a/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsBucketRegionResolverCreationTest.java b/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsBucketRegionResolverCreationTest.java index 4b76eb5..d9922a5 100644 --- a/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsBucketRegionResolverCreationTest.java +++ b/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsBucketRegionResolverCreationTest.java @@ -6,14 +6,16 @@ import org.junit.Test; import software.amazon.awssdk.services.s3.S3Client; -import static org.assertj.core.api.AssertionsForClassTypes.assertThat; import static org.mockito.Mockito.mock; -import static software.amazon.awssdk.s3accessgrants.cache.S3AccessGrantsConstants.*; +import static software.amazon.awssdk.s3accessgrants.cache.S3AccessGrantsConstants.BUCKET_REGION_CACHE_SIZE; +import static software.amazon.awssdk.s3accessgrants.cache.S3AccessGrantsConstants.BUCKET_REGION_EXPIRE_CACHE_AFTER_WRITE_SECONDS; +import static software.amazon.awssdk.s3accessgrants.cache.S3AccessGrantsConstants.MAX_BUCKET_REGION_CACHE_SIZE; +import static software.amazon.awssdk.s3accessgrants.cache.S3AccessGrantsConstants.MAX_BUCKET_REGION_EXPIRE_CACHE_AFTER_WRITE_SECONDS; public class S3AccessGrantsBucketRegionResolverCreationTest { - private static int TEST_BUCKET_REGION_CACHE_SIZE = 5_000; - private static int TEST_CACHE_EXPIRATION_DURATION = 6_0; + private static final int TEST_BUCKET_REGION_CACHE_SIZE = 5_000; + private static final int TEST_CACHE_EXPIRATION_DURATION = 6_0; private static S3Client s3Client; diff --git a/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsBucketRegionResolverTest.java b/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsBucketRegionResolverTest.java index 09b92af..064d6c0 100644 --- a/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsBucketRegionResolverTest.java +++ b/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsBucketRegionResolverTest.java @@ -3,7 +3,6 @@ import org.assertj.core.api.Assertions; import org.junit.Assert; import org.junit.Before; -import org.junit.BeforeClass; import org.junit.Test; import software.amazon.awssdk.awscore.exception.AwsErrorDetails; import software.amazon.awssdk.awscore.exception.AwsServiceException; @@ -19,7 +18,10 @@ import java.util.List; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.*; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.mockito.Mockito.times; public class S3AccessGrantsBucketRegionResolverTest { From 29e8184ac15302c6b431573d220ddade37584347 Mon Sep 17 00:00:00 2001 From: Shiva Kumar Mukkapati Date: Tue, 9 Jan 2024 11:32:27 -0500 Subject: [PATCH 3/3] adding comments to tests --- .../cache/S3AccessGrantsBucketRegionResolverTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsBucketRegionResolverTest.java b/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsBucketRegionResolverTest.java index 064d6c0..867473b 100644 --- a/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsBucketRegionResolverTest.java +++ b/src/test/java/software/amazon/awssdk/s3accessgrants/cache/S3AccessGrantsBucketRegionResolverTest.java @@ -56,6 +56,7 @@ public void call_resolve_should_not_cache_the_bucket_region() { when(localS3Client.headBucket(any(HeadBucketRequest.class))).thenReturn(headBucketResponse); Assertions.assertThatThrownBy(() -> localS3AccessGrantsCachedBucketRegionResolver.resolve(TEST_BUCKET_NAME)).isInstanceOf(SdkServiceException.class); verify(localS3Client, times(1)).headBucket(any(HeadBucketRequest.class)); + // since bucket region is null, cache will not store the entry Assertions.assertThatThrownBy(() -> localS3AccessGrantsCachedBucketRegionResolver.resolve(TEST_BUCKET_NAME)).isInstanceOf(SdkServiceException.class); verify(localS3Client, times(2)).headBucket(any(HeadBucketRequest.class)); } @@ -120,6 +121,7 @@ public void call_bucket_region_cache_resolve_returns_redirect_with_null_region() when(s3Exception.statusCode()).thenReturn(301); when(s3Exception.awsErrorDetails()).thenReturn(awsErrorDetails); when(localS3Client.headBucket(any(HeadBucketRequest.class))).thenThrow(s3Exception); + // resolving that exceptions are thrown when bucket region cannot be determined. Assertions.assertThatThrownBy(() -> localS3AccessGrantsCachedBucketRegionResolver.resolve(TEST_BUCKET_NAME)) .isInstanceOf(SdkServiceException.class);