8000 Cloudformation implement ssm param read by cloutierMat · Pull Request #11962 · localstack/localstack · GitHub
[go: up one dir, main page]

Skip to content

Cloudformation implement ssm param read #11962

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

Merged
merged 5 commits into from
Nov 29, 2024

Conversation

cloutierMat
Copy link
Contributor

Motivation

Implementation of read along with fixing list for ssm parameters. While fixing I realised that the generated specs were out of date. So I recreated them, highlighting an issue with the previous list operation.

Changes

  • update create method to return the read operation. (how can we test this?)
  • added read operation
  • updated list operation

@cloutierMat cloutierMat added the semver: patch Non-breaking changes which can be included in patch releases label Nov 29, 2024
@cloutierMat cloutierMat self-assigned this Nov 29, 2024
@cloutierMat cloutierMat added this to the 4.1 milestone Nov 29, 2024
Copy link
github-actions bot commented Nov 29, 2024

LocalStack Community integration with Pro

    2 files  ±0      2 suites  ±0   1h 48m 41s ⏱️ -15s
3 775 tests ±0  3 429 ✅ ±0  346 💤 ±0  0 ❌ ±0 
3 777 runs  ±0  3 429 ✅ ±0  348 💤 ±0  0 ❌ ±0 

Results for commit ec88ba7. ± Comparison against base commit 892eb4d.

♻️ This comment has been updated with latest results.

@@ -264,6 +264,7 @@ class ServicePrincipal(str):
"""

apigateway = "apigateway"
cloudformation = "cloudformation"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cloutierMat cloutierMat marked this pull request as ready for review November 29, 2024 05:07
Comment on lines -206 to +227
SSMParameterProperties(Id=resource["Name"]) for resource in resources["Parameters"]
SSMParameterProperties(Name=resource["Name"])
for resource in resources["Parameters"]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Pive01 Will this create an issue with the resource overview? If so please let me know so that I don't break it for you! 😉

Copy link
Contributor Author
@cloutierMat cloutierMat Nov 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I was just watching your demo and realised that the ssm parameter didn't show. So I realised that they are actually broken and this changes fixes it. We should probably try to push to merge this before the patch release!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @cloutierMat , it does indeed fix an issue as the identifier was not set correctly...now is properly discovered 🚀

@cloutierMat cloutierMat added the aws:cloudformation AWS CloudFormation label Nov 29, 2024
@cloutierMat cloutierMat modified the milestones: 4.1, 4.0.3 Nov 29, 2024
Copy link
Member
@dominikschubert dominikschubert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for spotting and addressing this so quickly @cloutierMat !

It also adds some pressure on a fairly old TODO item regarding an ASF-update-like CFn spec update automation 😁

@dominikschubert dominikschubert merged commit be61d50 into master Nov 29, 2024
35 checks passed
@dominikschubert dominikschubert deleted the cloudformation-implement-ssm-param-get branch November 29, 2024 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws:cloudformation AWS CloudFormation semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0