-
Notifications
You must be signed in to change notification settings - Fork 402
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
RFC: Setting SSM Parameters and Secret Manager Secrets #3040
Comments
hey @stephenbawks! Congrats on writing your first RFC, this is an incredible work of ideas and solutions, and I really appreciate this work. I will comment on each block you wrote and leave some new ideas. From here on, I will use
|
I think the As for
|
Yeah, they are pretty close to what they are doing. I think one question I still have is if we are using Wondering if we should make those both |
In regards to the random password, those are all optional parameters in the command. In terms of making a password compliant with certain standards, you could just abstract that all way into a simple ENUM variable. For example, "Give me a FIPS complaint password" and it is coded with those standards but that may be more than we want to maintain since those do change over time. |
@leandrodamascena What do you think of adding '*' in order to enforce key value args, this will help users to avoid mistakes when using this capability def set(name: str, value: str, *, description: str = None, overwrite: bool = False, kms_key_id: str = None, type: str = "String", tier: str = "Standard", **kwargs) |
Reviewing tomorrow 👊 |
Firstly, thank you @stephenbawks on your first RFC 🎉 Quick thoughts to complement what others shared earlier:
One of the things I wish we've done in the early design of high-level functions in Parameters was to accept a For example, If we were doing Parameters today, I wouldn't add a higher-level function for AppConfig like |
One more thing I have noticed is that the So I thinking of removing the transform option on
String is obviously just a string. StringList is just a comma separated set of string values. Secure String is just how SSM encodes the value on the Parameter Store side. Another option I can think of is adding another option to the transform that accepts a Wondering what feedback others might have on this. |
Here is what I am leaning towards.
|
Overall This looks pretty got IMO.
I suggest the following API
|
UX tips
- Write a complete example for each parameter type that a customer would
read
- Be mindful of “Optional[]”, you don’t need it if the parameter can have a
default value
- Code will be read by others, make that easier and intuitive to read and
to write
The minute you try writing an example for each parameter type it’ll become
easier to realize improvements you can make here, including set vs
put_parameter, cache, transform.
Readme driven development is king for an ergonomic and accessible API
…On Sat, 16 Sep 2023 at 08:53, aradyaron ***@***.***> wrote:
@aradyaron <https://github.com/aradyaron>
Here is what I am leaning towards.
def set_parameter(
name: str,
value: str,
description: Optional[str] = None,
type: Optional[Literal["String", "StringList", "SecureString"]] = "String",
overwrite: bool = False,
tier: Optional[Literal["Standard", "Advanced", "Intelligent-Tiering"]] = "Standard",
max_age: Optional[int] = None,
kms_key_id: Optional[str] = None,
**sdk_options,
) -> str:
Overall This looks pretty got IMO.
- Is there a need for max_age parameter in set parameter?
- the get_parameter utilities has a transform ability. Do you think
you API should enable the transformation to string from dict/bytes?
I suggest the following API
def set_parameter(
name: str,
value: Union[str, dict, bytes],
*,
description: Optional[str] = None,
type: Optional[Literal["String", "StringList", "SecureString"]] = "String",
overwrite: bool = False,
tier: Optional[Literal["Standard", "Advanced", "Intelligent-Tiering"]] = "Standard",
kms_key_id: Optional[str] = None,
**sdk_options,
) -> str:
—
Reply to this email directly, view it on GitHub
<#3040 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAZPQBFFL377C25NU3VIJ6DX2VEHLANCNFSM6AAAAAA4KU5T2E>
.
You are receiving this because you were mentioned.Message ID:
***@***.***
com>
|
@stephenbawks What do you think of adding an option to control the default upon constructing the SSMProvider?
This for example will allow to use a secure by default of parameter_type="SecureString" |
Hey everyone! I was attending an external event and I'm reading the new comments on this RFC. I hope to provide feedback tomorrow and move forward to finish this RFC and work on the opened PR. |
Thank you everyone for the wealth of context. I've just enumerated the activities we need to get done to release it in the next two weeks or so. |
|
Is this related to an existing feature request or issue?
#2826
Which Powertools for AWS Lambda (Python) utility does this relate to?
Parameters
Summary
There are many use cases where the Parameter store is used with PowerTools everywhere. It's an invaluable tool for getting SSM Parameters and the ability to cache them has been fantastic to save time going back to SSM repeatedly. There is one thing that I find to be missing and that is the ability to put SSM parameters as well as the ability to write secrets back to Secrets Manager as well.
Use case
Parameter Store
Adding the function to
put_parameter
back to SSM with the ability to make sure it's also being cached when it is written back. This should also include the ability to have theoverwrite
flag as optional. When parameters are being over-written to Parameter store you need to set the overwrite flag because it's set tofalse
by default.Secrets Manager
Would also like to add the ability to write values to the Secrets Store as well. There are many uses, especially in Lambda where I need to cache/write something secret, (JWT Tokens for example) so that multiple Lambda's are not all going back to get a new token every time a new one is invoked. "Caching" that value in Secret Manager (or as a SecureString in Parameter Store) is a good use case for this.
Proposal
Tasks
Parameter Store
For getting values in SSM, Powertools is using
get_parameter
. I am proposing that we add a new option forput_paramter
for the ability to write a value back to a parameter in SSM.By default, you have to specify the
overwrite=true
to write a value to a parameter if there is already an existing value. I am wondering if this should be set totrue
by default or if we should be setting that on every time you need to overwrite a value. When it is written to Parameter store theput_parameter
will return a response that includes the new version of the parameter. So in theory there is already the ability to go back versions and it will hold up to 100 of the previous versions before it will start rolling over the oldest.Secret Manager
Adding the ability to create, set, and update secrets in Secret Manager.
I think
create()
makes sense for creating a new one.set()
might be translated into boto3 asput_secret_value
. The newly created secret would become the latest version of the secret.update()
sounds very similar toset()
but could be used for changing the version of the secret that would be retrieved.update(secret_name="mySuperSecret", secret_value="toomanysecrets")
update(secret_name="mySuperSecret", secret_value="toomanysecrets", stage="development")
get()
would be nice to have with the optional way to get different versions of a secret.generate_random_secret()
is also a nice feature for Secrets Manager. This might be a really nice way for people to generate random secrets that can be added to Powertools and saving the need for our fellow Powertools-ers to write custom code to generate random strings or secrets.Out of scope
Secrets Manager
I think being able to cache that value would be great but I may have to think through if they are updating a secret with a different version.
I have been thinking about how this might eventually be used for versioning around being able to do "canary" or even cutovers to new secrets. But that is probably future work....
Potential challenges
Secrets Manager
When it comes to having the SecretsManager class also cache the new value, I think it would be great but I may have to think through if they are updating a secret with a different version. Do we cache the new version AND the old version if it exists? What would be the behavior around this?
Dependencies and Integrations
No response
Alternative solutions
No response
Acknowledgment
The text was updated successfully, but these errors were encountered: