Skip to content

Commit b6b118f

Browse files
jeromevdljeromevdl
authored
fix: ParamManager cannot provide default SSM & Secrets providers (aws-powertools#1282)
* fix issue 1280 * Update powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/ParamManager.java missing issue number
1 parent a19def3 commit b6b118f

File tree

6 files changed

+120
-52
lines changed

6 files changed

+120
-52
lines changed

‎powertools-parameters/pom.xml‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,4 +133,18 @@
133133
</dependency>
134134
</dependencies>
135135

136+
<build>
137+
<plugins>
138+
<plugin>
139+
<artifactId>maven-surefire-plugin</artifactId>
140+
<version>3.1.2</version>
141+
<configuration>
142+
<environmentVariables>
143+
<AWS_REGION>eu-central-1</AWS_REGION>
144+
</environmentVariables>
145+
</configuration>
146+
</plugin>
147+
</plugins>
148+
</build>
149+
136150
</project>

‎powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/DynamoDbProvider.java‎

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@
55
import software.amazon.awssdk.http.urlconnection.UrlConnectionHttpClient;
66
import software.amazon.awssdk.regions.Region;
77
import software.amazon.awssdk.services.dynamodb.DynamoDbClient;
8+
import software.amazon.awssdk.services.dynamodb.DynamoDbClientBuilder;
89
import software.amazon.awssdk.services.dynamodb.model.AttributeValue;
910
import software.amazon.awssdk.services.dynamodb.model.GetItemRequest;
1011
import software.amazon.awssdk.services.dynamodb.model.GetItemResponse;
1112
import software.amazon.awssdk.services.dynamodb.model.QueryRequest;
1213
import software.amazon.awssdk.services.dynamodb.model.QueryResponse;
14+
import software.amazon.lambda.powertools.core.internal.LambdaConstants;
1315
import software.amazon.lambda.powertools.parameters.cache.CacheManager;
1416
import software.amazon.lambda.powertools.parameters.exception.DynamoDbProviderSchemaException;
1517
import software.amazon.lambda.powertools.parameters.transform.TransformationManager;
@@ -18,6 +20,8 @@
1820
import java.util.Map;
1921
import java.util.stream.Collectors;
2022

23+
import static software.amazon.lambda.powertools.core.internal.LambdaConstants.AWS_LAMBDA_INITIALIZATION_TYPE;
24+
2125
/**
2226
* Implements a {@link ParamProvider} on top of DynamoDB. The schema of the table
2327
* is described in the Powertools for AWS Lambda (Java) documentation.
@@ -30,23 +34,16 @@ public class DynamoDbProvider extends BaseProvider {
3034
private final DynamoDbClient client;
3135
private final String tableName;
3236

33-
public DynamoDbProvider(CacheManager cacheManager, String tableName) {
34-
this(cacheManager, DynamoDbClient.builder()
35-
.httpClientBuilder(UrlConnectionHttpClient.builder())
36-
.credentialsProvider(EnvironmentVariableCredentialsProvider.create())
37-
.region(Region.of(System.getenv(SdkSystemSetting.AWS_REGION.environmentVariable())))
38-
.build(),
39-
tableName
40-
);
41-
42-
}
43-
4437
DynamoDbProvider(CacheManager cacheManager, DynamoDbClient client, String tableName) {
4538
super(cacheManager);
4639
this.client = client;
4740
this.tableName = tableName;
4841
}
4942

43+
DynamoDbProvider(CacheManager cacheManager, String tableName) {
44+
this(cacheManager, Builder.createClient(), tableName);
45+
}
46+
5047
/**
5148
* Return a single value from the DynamoDB parameter provider.
5249
*
@@ -136,11 +133,11 @@ public DynamoDbProvider build() {
136133
throw new IllegalStateException("No DynamoDB table name provided; please provide one");
137134
}
138135
DynamoDbProvider provider;
139-
if (client != null) {
140-
provider = new DynamoDbProvider(cacheManager, client, table);
141-
} else {
142-
provider = new DynamoDbProvider(cacheManager, table);
136+
if (client == null) {
137+
client = createClient();
143138
}
139+
provider = new DynamoDbProvider(cacheManager, client, table);
140+
144141
if (transformationManager != null) {
145142
provider.setTransformationManager(transformationManager);
146143
}
@@ -191,5 +188,21 @@ public DynamoDbProvider.Builder withTransformationManager(TransformationManager
191188
this.transformationManager = transformationManager;
192189
return this;
193190
}
191+
192+
private static DynamoDbClient createClient() {
193+
DynamoDbClientBuilder dynamoDbClientBuilder = DynamoDbClient.builder()
194+
.httpClientBuilder(UrlConnectionHttpClient.builder())
195+
.region(Region.of(System.getenv(SdkSystemSetting.AWS_REGION.environmentVariable())));
196+
197+
// AWS_LAMBDA_INITIALIZATION_TYPE has two values on-demand and snap-start
198+
// when using snap-start mode, the env var creds provider isn't used and causes a fatal error if set
199+
// fall back to the default provider chain if the mode is anything other than on-demand.
200+
String initializationType = System.getenv().get(AWS_LAMBDA_INITIALIZATION_TYPE);
201+
if (initializationType != null && initializationType.equals(LambdaConstants.ON_DEMAND)) {
202+
dynamoDbClientBuilder.credentialsProvider(EnvironmentVariableCredentialsProvider.create());
203+
}
204+
205+
return dynamoDbClientBuilder.build();
206+
}
194207
}
195208
}

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,19 @@ public final class ParamManager {
3737

3838
/**
3939
* Get a concrete implementation of {@link BaseProvider}.<br/>
40-
* You can specify {@link SecretsProvider}, {@link SSMProvider}, {@link DynamoDbProvider}, or create your
40+
* You can specify {@link SecretsProvider}, {@link SSMProvider} or create your
4141
* custom provider by extending {@link BaseProvider} if you need to integrate with a different parameter store.
42+
* @deprecated You should not use this method directly but a typed one (getSecretsProvider, getSsmProvider, getDynamoDbProvider, getAppConfigProvider), will be removed in v2
4243
* @return a {@link SecretsProvider}
4344
*/
45+
// 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)
4446
public static <T extends BaseProvider> T getProvider(Class<T> providerClass) {
4547
if (providerClass == null) {
4648
throw new IllegalStateException("providerClass cannot be null.");
4749
}
50+
if (providerClass == DynamoDbProvider.class || providerClass == AppConfigProvider.class) {
51+
throw new IllegalArgumentException(providerClass + " cannot be instantiated like this, additional parameters are required");
52+
}
4853
return (T) providers.computeIfAbsent(providerClass, ParamManager::createProvider);
4954
}
5055

@@ -166,7 +171,7 @@ public static TransformationManager getTransformationManager() {
166171
static <T extends BaseProvider> T createProvider(Class<T> providerClass) {
167172
try {
168173
Constructor<T> constructor = providerClass.getDeclaredConstructor(CacheManager.class);
169-
T provider = constructor.newInstance(cacheManager);
174+
T provider = constructor.newInstance(cacheManager); // FIXME: avoid reflection here as we may have issues (#1280)
170175
provider.setTransformationManager(transformationManager);
171176
return provider;
172177
} catch (ReflectiveOperationException e) {

‎powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/SSMProvider.java‎

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,7 @@
7474
*/
7575
public class SSMProvider extends BaseProvider {
7676

77-
private SsmClient client;
78-
77+
private final SsmClient client;
7978
private boolean decrypt = false;
8079
private boolean recursive = false;
8180

@@ -93,12 +92,13 @@ public class SSMProvider extends BaseProvider {
9392
}
9493

9594
/**
96-
* Constructor
95+
* Constructor with only a CacheManager<br/>
9796
*
97+
* Used in {@link ParamManager#createProvider(Class)}
9898
* @param cacheManager handles the parameter caching
9999
*/
100100
SSMProvider(CacheManager cacheManager) {
101-
super(cacheManager);
101+
this(cacheManager, Builder.createClient());
102102
}
103103

104104
/**
@@ -228,6 +228,11 @@ protected void resetToDefaults() {
228228
decrypt = false;
229229
}
230230

231+
// For tests purpose only
232+
SsmClient getClient() {
233+
return client;
234+
}
235+
231236
/**
232237
* Create a builder that can be used to configure and create a {@link SSMProvider}.
233238
*
@@ -237,6 +242,7 @@ public static SSMProvider.Builder builder() {
237242
return new SSMProvider.Builder();
238243
}
239244

245+
240246
static class Builder {
241247
private SsmClient client;
242248
private CacheManager cacheManager;
@@ -253,19 +259,7 @@ public SSMProvider build() {
253259
}
254260
SSMProvider provider;
255261
if (client == null) {
256-
SsmClientBuilder ssmClientBuilder = SsmClient.builder()
257-
.httpClientBuilder(UrlConnectionHttpClient.builder())
258-
.region(Region.of(System.getenv(SdkSystemSetting.AWS_REGION.environmentVariable())));
259-
260-
// AWS_LAMBDA_INITIALIZATION_TYPE has two values on-demand and snap-start
261-
// when using snap-start mode, the env var creds provider isn't used and causes a fatal error if set
262-
// fall back to the default provider chain if the mode is anything other than on-demand.
263-
String initializationType = System.getenv().get(AWS_LAMBDA_INITIALIZATION_TYPE);
264-
if (initializationType != null && initializationType.equals(LambdaConstants.ON_DEMAND)) {
265-
ssmClientBuilder.credentialsProvider(EnvironmentVariableCredentialsProvider.create());
266-
}
267-
268-
client = ssmClientBuilder.build();
262+
client = createClient();
269263
}
270264

271265
provider = new SSMProvider(cacheManager, client);
@@ -288,6 +282,22 @@ public SSMProvider.Builder withClient(SsmClient client) {
288282
return this;
289283
}
290284

285+
private static SsmClient createClient() {
286+
SsmClientBuilder ssmClientBuilder = SsmClient.builder()
287+
.httpClientBuilder(UrlConnectionHttpClient.builder())
288+
.region(Region.of(System.getenv(SdkSystemSetting.AWS_REGION.environmentVariable())));
289+
290+
// AWS_LAMBDA_INITIALIZATION_TYPE has two values on-demand and snap-start
291+
// when using snap-start mode, the env var creds provider isn't used and causes a fatal error if set
292+
// fall back to the default provider chain if the mode is anything other than on-demand.
293+
String initializationType = System.getenv().get(AWS_LAMBDA_INITIALIZATION_TYPE);
294+
if (initializationType != null && initializationType.equals(LambdaConstants.ON_DEMAND)) {
295+
ssmClientBuilder.credentialsProvider(EnvironmentVariableCredentialsProvider.create());
296+
}
297+
298+
return ssmClientBuilder.build();
299+
}
300+
291301
/**
292302
* <b>Mandatory</b>. Provide a CacheManager to the {@link SSMProvider}
293303
*

‎powertools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/SecretsProvider.java‎

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@
5858
*/
5959
public class SecretsProvider extends BaseProvider {
6060

61-
private SecretsManagerClient client;
61+
private final SecretsManagerClient client;
6262

6363
/**
6464
* Constructor with custom {@link SecretsManagerClient}. <br/>
@@ -74,12 +74,13 @@ public class SecretsProvider extends BaseProvider {
7474
}
7575

7676
/**
77-
* Constructor
77+
* Constructor with only a CacheManager<br/>
7878
*
79+
* Used in {@link ParamManager#createProvider(Class)}
7980
* @param cacheManager handles the parameter caching
8081
*/
8182
SecretsProvider(CacheManager cacheManager) {
82-
super(cacheManager);
83+
this(cacheManager, Builder.createClient());
8384
}
8485

8586
/**
@@ -135,6 +136,11 @@ public SecretsProvider withTransformation(Class<? extends Transformer> transform
135136
return this;
136137
}
137138

139+
// For test purpose only
140+
SecretsManagerClient getClient() {
141+
return client;
142+
}
143+
138144
/**
139145
* Create a builder that can be used to configure and create a {@link SecretsProvider}.
140146
*
@@ -161,19 +167,7 @@ public SecretsProvider build() {
161167
}
162168
SecretsProvider provider;
163169
if (client == null) {
164-
SecretsManagerClientBuilder secretsManagerClientBuilder = SecretsManagerClient.builder()
165-
.httpClientBuilder(UrlConnectionHttpClient.builder())
166-
.region(Region.of(System.getenv(SdkSystemSetting.AWS_REGION.environmentVariable())));
167-
168-
// AWS_LAMBDA_INITIALIZATION_TYPE has two values on-demand and snap-start
169-
// when using snap-start mode, the env var creds provider isn't used and causes a fatal error if set
170-
// fall back to the default provider chain if the mode is anything other than on-demand.
171-
String initializationType = System.getenv().get(AWS_LAMBDA_INITIALIZATION_TYPE);
172-
if (initializationType != null && initializationType.equals(LambdaConstants.ON_DEMAND)) {
173-
secretsManagerClientBuilder.credentialsProvider(EnvironmentVariableCredentialsProvider.create());
174-
}
175-
176-
client = secretsManagerClientBuilder.build();
170+
client = createClient();
177171
}
178172

179173
provider = new SecretsProvider(cacheManager, client);
@@ -196,6 +190,22 @@ public Builder withClient(SecretsManagerClient client) {
196190
return this;
197191
}
198192

193+
private static SecretsManagerClient createClient() {
194+
SecretsManagerClientBuilder secretsManagerClientBuilder = SecretsManagerClient.builder()
195+
.httpClientBuilder(UrlConnectionHttpClient.builder())
196+
.region(Region.of(System.getenv(SdkSystemSetting.AWS_REGION.environmentVariable())));
197+
198+
// AWS_LAMBDA_INITIALIZATION_TYPE has two values on-demand and snap-start
199+
// when using snap-start mode, the env var creds provider isn't used and causes a fatal error if set
200+
// fall back to the default provider chain if the mode is anything other than on-demand.
201+
String initializationType = System.getenv().get(AWS_LAMBDA_INITIALIZATION_TYPE);
202+
if (initializationType != null && initializationType.equals(LambdaConstants.ON_DEMAND)) {
203+
secretsManagerClientBuilder.credentialsProvider(EnvironmentVariableCredentialsProvider.create());
204+
}
205+
206+
return secretsManagerClientBuilder.build();
207+
}
208+
199209
/**
200210
* <b>Mandatory</b>. Provide a CacheManager to the {@link SecretsProvider}
201211
*

‎powertools-parameters/src/test/java/software/amazon/lambda/powertools/parameters/ParamManagerTest.java‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import software.amazon.lambda.powertools.parameters.internal.CustomProvider;
1919
import software.amazon.lambda.powertools.parameters.transform.TransformationManager;
2020

21+
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
2122
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
2223
import static org.assertj.core.api.Assertions.assertThatRuntimeException;
2324
import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -79,23 +80,38 @@ public void testGetProviderWithProviderClass_throwsException() {
7980
}
8081

8182
@Test
82-
public void testGetSecretsProvider() {
83+
public void testGetSecretsProvider_withoutParameter_shouldCreateDefaultClient() {
8384

8485
// Act
8586
SecretsProvider secretsProvider = ParamManager.getSecretsProvider();
8687

8788
// Assert
8889
assertNotNull(secretsProvider);
90+
assertNotNull(secretsProvider.getClient());
8991
}
9092

9193
@Test
92-
public void testGetSSMProvider() {
94+
public void testGetSSMProvider_withoutParameter_shouldCreateDefaultClient() {
9395

9496
// Act
9597
SSMProvider ssmProvider = ParamManager.getSsmProvider();
9698

9799
// Assert
98100
assertNotNull(ssmProvider);
101+
assertNotNull(ssmProvider.getClient());
99102
}
100103

104+
@Test
105+
public void testGetDynamoDBProvider_requireOtherParameters_throwException() {
106+
107+
// Act & Assert
108+
assertThatIllegalArgumentException().isThrownBy(() -> ParamManager.getProvider(DynamoDbProvider.class));
109+
}
110+
111+
@Test
112+
public void testGetAppConfigProvider_requireOtherParameters_throwException() {
113+
114+
// Act & Assert
115+
assertThatIllegalArgumentException().isThrownBy(() -> ParamManager.getProvider(AppConfigProvider.class));
116+
}
101117
}

0 commit comments

Comments
 (0)