From 85fa102a1ba5e4d130176ba29a5c70731a35b049 Mon Sep 17 00:00:00 2001 From: Jerome Van Der Linden Date: Wed, 12 Jul 2023 15:20:22 +0200 Subject: [PATCH 1/2] fix issue 1280 --- powertools-parameters/pom.xml | 14 ++++++ .../parameters/DynamoDbProvider.java | 43 +++++++++++------- .../powertools/parameters/ParamManager.java | 9 +++- .../powertools/parameters/SSMProvider.java | 44 ++++++++++++------- .../parameters/SecretsProvider.java | 42 +++++++++++------- .../parameters/ParamManagerTest.java | 20 ++++++++- 6 files changed, 120 insertions(+), 52 deletions(-) diff --git a/powertools-parameters/pom.xml b/powertools-parameters/pom.xml index af980a4c3..b50844d99 100644 --- a/powertools-parameters/pom.xml +++ b/powertools-parameters/pom.xml @@ -133,4 +133,18 @@ + + + + maven-surefire-plugin + 3.1.2 + + + eu-central-1 + + + + + + diff --git a/powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/DynamoDbProvider.java b/powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/DynamoDbProvider.java index 5144af0c2..1b77aed88 100644 --- a/powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/DynamoDbProvider.java +++ b/powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/DynamoDbProvider.java @@ -5,11 +5,13 @@ import software.amazon.awssdk.http.urlconnection.UrlConnectionHttpClient; import software.amazon.awssdk.regions.Region; import software.amazon.awssdk.services.dynamodb.DynamoDbClient; +import software.amazon.awssdk.services.dynamodb.DynamoDbClientBuilder; import software.amazon.awssdk.services.dynamodb.model.AttributeValue; import software.amazon.awssdk.services.dynamodb.model.GetItemRequest; import software.amazon.awssdk.services.dynamodb.model.GetItemResponse; import software.amazon.awssdk.services.dynamodb.model.QueryRequest; import software.amazon.awssdk.services.dynamodb.model.QueryResponse; +import software.amazon.lambda.powertools.core.internal.LambdaConstants; import software.amazon.lambda.powertools.parameters.cache.CacheManager; import software.amazon.lambda.powertools.parameters.exception.DynamoDbProviderSchemaException; import software.amazon.lambda.powertools.parameters.transform.TransformationManager; @@ -18,6 +20,8 @@ import java.util.Map; import java.util.stream.Collectors; +import static software.amazon.lambda.powertools.core.internal.LambdaConstants.AWS_LAMBDA_INITIALIZATION_TYPE; + /** * Implements a {@link ParamProvider} on top of DynamoDB. The schema of the table * is described in the Powertools for AWS Lambda (Java) documentation. @@ -30,23 +34,16 @@ public class DynamoDbProvider extends BaseProvider { private final DynamoDbClient client; private final String tableName; - public DynamoDbProvider(CacheManager cacheManager, String tableName) { - this(cacheManager, DynamoDbClient.builder() - .httpClientBuilder(UrlConnectionHttpClient.builder()) - .credentialsProvider(EnvironmentVariableCredentialsProvider.create()) - .region(Region.of(System.getenv(SdkSystemSetting.AWS_REGION.environmentVariable()))) - .build(), - tableName - ); - - } - DynamoDbProvider(CacheManager cacheManager, DynamoDbClient client, String tableName) { super(cacheManager); this.client = client; this.tableName = tableName; } + DynamoDbProvider(CacheManager cacheManager, String tableName) { + this(cacheManager, Builder.createClient(), tableName); + } + /** * Return a single value from the DynamoDB parameter provider. * @@ -136,11 +133,11 @@ public DynamoDbProvider build() { throw new IllegalStateException("No DynamoDB table name provided; please provide one"); } DynamoDbProvider provider; - if (client != null) { - provider = new DynamoDbProvider(cacheManager, client, table); - } else { - provider = new DynamoDbProvider(cacheManager, table); + if (client == null) { + client = createClient(); } + provider = new DynamoDbProvider(cacheManager, client, table); + if (transformationManager != null) { provider.setTransformationManager(transformationManager); } @@ -191,5 +188,21 @@ public DynamoDbProvider.Builder withTransformationManager(TransformationManager this.transformationManager = transformationManager; return this; } + + private static DynamoDbClient createClient() { + DynamoDbClientBuilder dynamoDbClientBuilder = DynamoDbClient.builder() + .httpClientBuilder(UrlConnectionHttpClient.builder()) + .region(Region.of(System.getenv(SdkSystemSetting.AWS_REGION.environmentVariable()))); + + // AWS_LAMBDA_INITIALIZATION_TYPE has two values on-demand and snap-start + // when using snap-start mode, the env var creds provider isn't used and causes a fatal error if set + // fall back to the default provider chain if the mode is anything other than on-demand. + String initializationType = System.getenv().get(AWS_LAMBDA_INITIALIZATION_TYPE); + if (initializationType != null && initializationType.equals(LambdaConstants.ON_DEMAND)) { + dynamoDbClientBuilder.credentialsProvider(EnvironmentVariableCredentialsProvider.create()); + } + + return dynamoDbClientBuilder.build(); + } } } diff --git a/powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/ParamManager.java b/powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/ParamManager.java index b2e541c43..829ce336c 100644 --- a/powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/ParamManager.java +++ b/powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/ParamManager.java @@ -37,14 +37,19 @@ public final class ParamManager { /** * Get a concrete implementation of {@link BaseProvider}.
- * You can specify {@link SecretsProvider}, {@link SSMProvider}, {@link DynamoDbProvider}, or create your + * You can specify {@link SecretsProvider}, {@link SSMProvider} or create your * custom provider by extending {@link BaseProvider} if you need to integrate with a different parameter store. + * @deprecated You should not use this method directly but a typed one (getSecretsProvider, getSsmProvider, getDynamoDbProvider, getAppConfigProvider), will be removed in v2 * @return a {@link SecretsProvider} */ + // TODO in v2: remove public access to this and review how we get providers (it was not designed for DDB and AppConfig in mind initially) public static T getProvider(Class providerClass) { if (providerClass == null) { throw new IllegalStateException("providerClass cannot be null."); } + if (providerClass == DynamoDbProvider.class || providerClass == AppConfigProvider.class) { + throw new IllegalArgumentException(providerClass + " cannot be instantiated like this, additional parameters are required"); + } return (T) providers.computeIfAbsent(providerClass, ParamManager::createProvider); } @@ -166,7 +171,7 @@ public static TransformationManager getTransformationManager() { static T createProvider(Class providerClass) { try { Constructor constructor = providerClass.getDeclaredConstructor(CacheManager.class); - T provider = constructor.newInstance(cacheManager); + T provider = constructor.newInstance(cacheManager); // FIXME: avoid reflection here as we may have issues (# provider.setTransformationManager(transformationManager); return provider; } catch (ReflectiveOperationException e) { diff --git a/powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/SSMProvider.java b/powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/SSMProvider.java index d6d87747b..2eb2d4199 100644 --- a/powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/SSMProvider.java +++ b/powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/SSMProvider.java @@ -74,8 +74,7 @@ */ public class SSMProvider extends BaseProvider { - private SsmClient client; - + private final SsmClient client; private boolean decrypt = false; private boolean recursive = false; @@ -93,12 +92,13 @@ public class SSMProvider extends BaseProvider { } /** - * Constructor + * Constructor with only a CacheManager
* + * Used in {@link ParamManager#createProvider(Class)} * @param cacheManager handles the parameter caching */ SSMProvider(CacheManager cacheManager) { - super(cacheManager); + this(cacheManager, Builder.createClient()); } /** @@ -228,6 +228,11 @@ protected void resetToDefaults() { decrypt = false; } + // For tests purpose only + SsmClient getClient() { + return client; + } + /** * Create a builder that can be used to configure and create a {@link SSMProvider}. * @@ -237,6 +242,7 @@ public static SSMProvider.Builder builder() { return new SSMProvider.Builder(); } + static class Builder { private SsmClient client; private CacheManager cacheManager; @@ -253,19 +259,7 @@ public SSMProvider build() { } SSMProvider provider; if (client == null) { - SsmClientBuilder ssmClientBuilder = SsmClient.builder() - .httpClientBuilder(UrlConnectionHttpClient.builder()) - .region(Region.of(System.getenv(SdkSystemSetting.AWS_REGION.environmentVariable()))); - - // AWS_LAMBDA_INITIALIZATION_TYPE has two values on-demand and snap-start - // when using snap-start mode, the env var creds provider isn't used and causes a fatal error if set - // fall back to the default provider chain if the mode is anything other than on-demand. - String initializationType = System.getenv().get(AWS_LAMBDA_INITIALIZATION_TYPE); - if (initializationType != null && initializationType.equals(LambdaConstants.ON_DEMAND)) { - ssmClientBuilder.credentialsProvider(EnvironmentVariableCredentialsProvider.create()); - } - - client = ssmClientBuilder.build(); + client = createClient(); } provider = new SSMProvider(cacheManager, client); @@ -288,6 +282,22 @@ public SSMProvider.Builder withClient(SsmClient client) { return this; } + private static SsmClient createClient() { + SsmClientBuilder ssmClientBuilder = SsmClient.builder() + .httpClientBuilder(UrlConnectionHttpClient.builder()) + .region(Region.of(System.getenv(SdkSystemSetting.AWS_REGION.environmentVariable()))); + + // AWS_LAMBDA_INITIALIZATION_TYPE has two values on-demand and snap-start + // when using snap-start mode, the env var creds provider isn't used and causes a fatal error if set + // fall back to the default provider chain if the mode is anything other than on-demand. + String initializationType = System.getenv().get(AWS_LAMBDA_INITIALIZATION_TYPE); + if (initializationType != null && initializationType.equals(LambdaConstants.ON_DEMAND)) { + ssmClientBuilder.credentialsProvider(EnvironmentVariableCredentialsProvider.create()); + } + + return ssmClientBuilder.build(); + } + /** * Mandatory. Provide a CacheManager to the {@link SSMProvider} * diff --git a/powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/SecretsProvider.java b/powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/SecretsProvider.java index 54d0daee3..ea8b5a9d0 100644 --- a/powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/SecretsProvider.java +++ b/powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/SecretsProvider.java @@ -58,7 +58,7 @@ */ public class SecretsProvider extends BaseProvider { - private SecretsManagerClient client; + private final SecretsManagerClient client; /** * Constructor with custom {@link SecretsManagerClient}.
@@ -74,12 +74,13 @@ public class SecretsProvider extends BaseProvider { } /** - * Constructor + * Constructor with only a CacheManager
* + * Used in {@link ParamManager#createProvider(Class)} * @param cacheManager handles the parameter caching */ SecretsProvider(CacheManager cacheManager) { - super(cacheManager); + this(cacheManager, Builder.createClient()); } /** @@ -135,6 +136,11 @@ public SecretsProvider withTransformation(Class transform return this; } + // For test purpose only + SecretsManagerClient getClient() { + return client; + } + /** * Create a builder that can be used to configure and create a {@link SecretsProvider}. * @@ -161,19 +167,7 @@ public SecretsProvider build() { } SecretsProvider provider; if (client == null) { - SecretsManagerClientBuilder secretsManagerClientBuilder = SecretsManagerClient.builder() - .httpClientBuilder(UrlConnectionHttpClient.builder()) - .region(Region.of(System.getenv(SdkSystemSetting.AWS_REGION.environmentVariable()))); - - // AWS_LAMBDA_INITIALIZATION_TYPE has two values on-demand and snap-start - // when using snap-start mode, the env var creds provider isn't used and causes a fatal error if set - // fall back to the default provider chain if the mode is anything other than on-demand. - String initializationType = System.getenv().get(AWS_LAMBDA_INITIALIZATION_TYPE); - if (initializationType != null && initializationType.equals(LambdaConstants.ON_DEMAND)) { - secretsManagerClientBuilder.credentialsProvider(EnvironmentVariableCredentialsProvider.create()); - } - - client = secretsManagerClientBuilder.build(); + client = createClient(); } provider = new SecretsProvider(cacheManager, client); @@ -196,6 +190,22 @@ public Builder withClient(SecretsManagerClient client) { return this; } + private static SecretsManagerClient createClient() { + SecretsManagerClientBuilder secretsManagerClientBuilder = SecretsManagerClient.builder() + .httpClientBuilder(UrlConnectionHttpClient.builder()) + .region(Region.of(System.getenv(SdkSystemSetting.AWS_REGION.environmentVariable()))); + + // AWS_LAMBDA_INITIALIZATION_TYPE has two values on-demand and snap-start + // when using snap-start mode, the env var creds provider isn't used and causes a fatal error if set + // fall back to the default provider chain if the mode is anything other than on-demand. + String initializationType = System.getenv().get(AWS_LAMBDA_INITIALIZATION_TYPE); + if (initializationType != null && initializationType.equals(LambdaConstants.ON_DEMAND)) { + secretsManagerClientBuilder.credentialsProvider(EnvironmentVariableCredentialsProvider.create()); + } + + return secretsManagerClientBuilder.build(); + } + /** * Mandatory. Provide a CacheManager to the {@link SecretsProvider} * diff --git a/powertools-parameters/src/test/java/software/amazon/lambda/powertools/parameters/ParamManagerTest.java b/powertools-parameters/src/test/java/software/amazon/lambda/powertools/parameters/ParamManagerTest.java index ee61691c1..a21a6082c 100644 --- a/powertools-parameters/src/test/java/software/amazon/lambda/powertools/parameters/ParamManagerTest.java +++ b/powertools-parameters/src/test/java/software/amazon/lambda/powertools/parameters/ParamManagerTest.java @@ -18,6 +18,7 @@ import software.amazon.lambda.powertools.parameters.internal.CustomProvider; import software.amazon.lambda.powertools.parameters.transform.TransformationManager; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; import static org.assertj.core.api.Assertions.assertThatRuntimeException; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -79,23 +80,38 @@ public void testGetProviderWithProviderClass_throwsException() { } @Test - public void testGetSecretsProvider() { + public void testGetSecretsProvider_withoutParameter_shouldCreateDefaultClient() { // Act SecretsProvider secretsProvider = ParamManager.getSecretsProvider(); // Assert assertNotNull(secretsProvider); + assertNotNull(secretsProvider.getClient()); } @Test - public void testGetSSMProvider() { + public void testGetSSMProvider_withoutParameter_shouldCreateDefaultClient() { // Act SSMProvider ssmProvider = ParamManager.getSsmProvider(); // Assert assertNotNull(ssmProvider); + assertNotNull(ssmProvider.getClient()); } + @Test + public void testGetDynamoDBProvider_requireOtherParameters_throwException() { + + // Act & Assert + assertThatIllegalArgumentException().isThrownBy(() -> ParamManager.getProvider(DynamoDbProvider.class)); + } + + @Test + public void testGetAppConfigProvider_requireOtherParameters_throwException() { + + // Act & Assert + assertThatIllegalArgumentException().isThrownBy(() -> ParamManager.getProvider(AppConfigProvider.class)); + } } From 9140cf2e585386cac03046d65ae10816a389ac2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Van=20Der=20Linden?= <117538+jeromevdl@users.noreply.github.com> Date: Wed, 12 Jul 2023 17:01:14 +0200 Subject: [PATCH 2/2] Update powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/ParamManager.java missing issue number --- .../amazon/lambda/powertools/parameters/ParamManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/ParamManager.java b/powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/ParamManager.java index 829ce336c..c8abedf06 100644 --- a/powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/ParamManager.java +++ b/powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/ParamManager.java @@ -171,7 +171,7 @@ public static TransformationManager getTransformationManager() { static T createProvider(Class providerClass) { try { Constructor constructor = providerClass.getDeclaredConstructor(CacheManager.class); - T provider = constructor.newInstance(cacheManager); // FIXME: avoid reflection here as we may have issues (# + T provider = constructor.newInstance(cacheManager); // FIXME: avoid reflection here as we may have issues (#1280) provider.setTransformationManager(transformationManager); return provider; } catch (ReflectiveOperationException e) {