Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Jul 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions powertools-parameters/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand All @@ -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.
*
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
jeromevdl marked this conversation as resolved.
Show resolved Hide resolved
throw new IllegalArgumentException(providerClass + " cannot be instantiated like this, additional parameters are required");
}
return (T) providers.computeIfAbsent(providerClass, ParamManager::createProvider);
}

Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,7 @@
*/
public class SSMProvider extends BaseProvider {

private SsmClient client;

private final SsmClient client;
private boolean decrypt = false;
private boolean recursive = false;

Expand All @@ -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());
}

/**
Expand Down Expand Up @@ -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}.
*
Expand All @@ -237,6 +242,7 @@ public static SSMProvider.Builder builder() {
return new SSMProvider.Builder();
}


static class Builder {
private SsmClient client;
private CacheManager cacheManager;
Expand All @@ -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);
Expand All @@ -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}
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
*/
public class SecretsProvider extends BaseProvider {

private SecretsManagerClient client;
private final SecretsManagerClient client;

/**
* Constructor with custom {@link SecretsManagerClient}. <br/>
Expand All @@ -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());
}

/**
Expand Down Expand Up @@ -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}.
*
Expand All @@ -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);
Expand All @@ -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}
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));
}
}