search

aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.33k stars 3.76k forks source link

chore(ssm): update simple name description and documentation #30653

Closed GavinZZ closed 1 day ago

GavinZZ commented 3 days ago

Issue # (if applicable)

Closes #28778.

Reason for this change

There are issues with SSM StringParameter where the parameter ARN would have missing / or duplicate / depending on the setup of simpleName with unresolved tokens in the parameter name.

Description of changes

Update README and docstring to explain to users when and how to correctly use simpleName parameter.

Description of how you validated changes

No code changes made.

Checklist

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

GavinZZ commented 3 days ago

Ideally it would be nice to throw a warning at synthesize time, but that's not possible because this issue only happens with unresolved tokens in parameter name. We can't determine whether the unresolved token contains / or not until later stage in the app life cycle. Thus, updating the docstring and README.

GavinZZ commented 1 day ago

Just a few minor changes. Is there any validation present that checks if the simpleName boolean value is appropriate for the given parameterName? If not should we add some to enforce it or are there cases where we want to allow it?

There're codes to verify is simpleName is appropriate to use. If not will raise exceptions. But this is to handle the case of unresolved token so we can't do much verifications.

paulhcsun commented 1 day ago

Ideally it would be nice to throw a warning at synthesize time, but that's not possible because this issue only happens with unresolved tokens in parameter name. We can't determine whether the unresolved token contains / or not until later stage in the app life cycle. Thus, updating the docstring and README.

Ohh gotcha, ok that should be fine then.

paulhcsun commented 1 day ago

@merigyfio update

GavinZZ commented 1 day ago

@mergify update

mergify[bot] commented 1 day ago

update

✅ Branch has been successfully updated

mergify[bot] commented 1 day ago

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation commented 1 day ago

AWS CodeBuild CI Report

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify[bot] commented 1 day ago

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).