fix: ParamManager cannot provide default SSM & Secrets providers (#1282)

* fix issue 1280

* Update powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/ParamManager.java

missing issue number

Author: jeromevdl

Diff for: powertools-parameters/pom.xml

@@ -133,4 +133,18 @@
         </dependency>
     </dependencies>
 
+    <build>
+        <plugins>
+            <plugin>
+                <artifactId>maven-surefire-plugin</artifactId>
+                <version>3.1.2</version>
+                <configuration>
+                    <environmentVariables>
+                        <AWS_REGION>eu-central-1</AWS_REGION>
+                    </environmentVariables>
+                </configuration>
+            </plugin>
+        </plugins>
+    </build>
+
 </project>

Diff for: 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 for: 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}.<br/>
-     * 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 extends BaseProvider> T getProvider(Class<T> 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 extends BaseProvider> T createProvider(Class<T> providerClass) {
         try {
             Constructor<T> constructor = providerClass.getDeclaredConstructor(CacheManager.class);
-            T provider = constructor.newInstance(cacheManager);
+            T provider = constructor.newInstance(cacheManager); // FIXME: avoid reflection here as we may have issues (#1280)
             provider.setTransformationManager(transformationManager);
             return provider;
         } catch (ReflectiveOperationException e) {

Diff for: 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<br/>
      *
+     * 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();
+        }
+
         /**
          * <b>Mandatory</b>. Provide a CacheManager to the {@link SSMProvider}
          *

Diff for: 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}. <br/>
@@ -74,12 +74,13 @@ public class SecretsProvider extends BaseProvider {
     }
 
     /**
-     * Constructor
+     * Constructor with only a CacheManager<br/>
      *
+     * 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<? extends Transformer> 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();
+        }
+
         /**
          * <b>Mandatory</b>. Provide a CacheManager to the {@link SecretsProvider}
          *

Diff for: 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));
+    }
 }