search

aws-powertools / powertools-lambda-java

Powertools is a developer toolkit to implement Serverless best practices and increase developer velocity.
https://docs.powertools.aws.dev/lambda/java/
MIT No Attribution
289 stars 88 forks source link

Support batch secrets retrieval in Parameters module #1530

Open jeromevdl opened 11 months ago

jeromevdl commented 11 months ago

Is your feature request related to a problem? Please describe. N/A

Describe the solution you'd like Add support for https://aws.amazon.com/about-aws/whats-new/2023/11/aws-secrets-manager-batch-retrieval-secrets/ in Parameters module.

scottgerring commented 11 months ago

This can be done in the v2 branch once #1403 is merged. It will be a good first issue at that point.

jreijn commented 9 months ago

@scottgerring @jeromevdl I would like to pick this up if possible. It seems #1403 has been merged, so good to go?

jreijn commented 9 months ago

I took a look at the code base and started with some minor improvements. I noticed that the getValue and the existing getMultipleValues exists, however the later on is now based on path now. I'm thinking of renaming the getMultipleValues to getMultipleValuesByPath and introduce a new method on the BaseProvider called getMultipleValuesByKey(List keys). That way we can support both use cases. For the new method, I'll see if I can implement a solution in all the providers.

scottgerring commented 9 months ago

Hey @jreijn happy for you to pick this up - I will assign it to you. I'll take some time tomorrow morning to look at the current ...multipleValues interface and write back!

scottgerring commented 9 months ago

v2 getMultipleValues impls

1. AppConfigProvider - unsupported

https://github.com/aws-powertools/powertools-lambda-java/blob/3963d6d1f58c9618959f7a7044189a4a32d9b3fc/powertools-parameters/powertools-parameters-appconfig/src/main/java/software/amazon/lambda/powertools/parameters/appconfig/AppConfigProvider.java#L113-L117

2. DynamoDbProvider

Retrieves all values that share the same partition key.

https://github.com/aws-powertools/powertools-lambda-java/blob/3963d6d1f58c9618959f7a7044189a4a32d9b3fc/powertools-parameters/powertools-parameters-dynamodb/src/main/java/software/amazon/lambda/powertools/parameters/dynamodb/DynamoDbProvider.java#L93-L99

3. SSMProvider

Retrieves either everything at the given level of the /parameter/hierarchy, or everything at the given level plus /parameter/hierarchy/nested/levels when using withRecursive.

https://github.com/aws-powertools/powertools-lambda-java/blob/3963d6d1f58c9618959f7a7044189a4a32d9b3fc/powertools-parameters/powertools-parameters-ssm/src/main/java/software/amazon/lambda/powertools/parameters/ssm/SSMProvider.java#L160-L166

Both the existing implementations - DDB and SSM provider - already vary a little - SSM is path-based, DDB is more like, common key. I'm a bit hesitant to change the existing method name because 1/ its not path-based in both cases and 2/ it's a breaking change against v1 which wouldn't add much. What do you think?

Maybe rather than changing the existing interface, we could add getMultipleValuesByKey(keys) on the BaseProvider, provide a default impl that simply maps getValue over the keys, and then override in SSMProvider ?

Thoughts @jreijn / @jeromevdl ?

jreijn commented 9 months ago

@scottgerring sounds reasonable. I also noticed that lambda powertools in other languages do something similar. I'll make some small commits while I go along, so I can get some feedback along the way.