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

feat(cloudformation): request and response models for custom resources #19

Merged

Conversation

renatoaguimaraes
Copy link
Contributor

@renatoaguimaraes renatoaguimaraes commented May 20, 2022

Mapping Request and Response of CustomResource used by Type: AWS::Lambda::Function.

Motivation:

Avoid to write repeated code for every Swift Lambda Function where is used a CustomResource to pass properties. Inspired on event.go and response.go from https://github.com/aws/aws-lambda-go.git.

Modifications:

Added two new structs: Request and Response for Lambda event triggered by CloudFormation.

Result:

New models to be shared between Swift Lambda Functions that use CustomResource.

@swift-server-bot
Copy link

Can one of the admins verify this patch?

5 similar comments
@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@tomerd
Copy link
Contributor

tomerd commented May 20, 2022

thank you so much @renatoaguimaraes, would you mind adding a unit test following the examples of the other events

@renatoaguimaraes
Copy link
Contributor Author

thank you so much @renatoaguimaraes, would you mind adding a unit test following the examples of the other events

Sure, I will follow the existing examples. Thanks.

@renatoaguimaraes renatoaguimaraes force-pushed the feat/cloudformation_custom_resource_lambda branch from 08b43de to 2eb6f63 Compare May 20, 2022 16:42
@renatoaguimaraes
Copy link
Contributor Author

renatoaguimaraes commented May 20, 2022

Hello @tomerd. Can you take a look? Thank you.

@tomerd
Copy link
Contributor

tomerd commented May 20, 2022

@swift-server-bot test this please

@tomerd
Copy link
Contributor

tomerd commented May 21, 2022

@renatoaguimaraes looks like the new tests are failing?

@renatoaguimaraes
Copy link
Contributor Author

renatoaguimaraes commented May 21, 2022

@renatoaguimaraes looks like the new tests are failing?

Yes, there is one scenario failing. I'm running the new unit tests on Mac OS 12.3.1 + Swift 5.6.1 with success and the project pipeline is running the tests on swift:5.4-amazonlinux2. It seems to me an implementation detail that depends on the environment, where the order of attributes is changing for some reason. But that's ok, I'll think in another way to test this specific scenario.

Running on swift:5.4-amazonlinux2.

Expected:

("Optional("{\"StackId\":\"arn:aws:cloudformation:us-east-1:123456789:stack\\/TestStack\",\"Reason\":\"See the details in CloudWatch Log Stream\",\"NoEcho\":false,\"Status\":\"SUCCESS\",\"LogicalResourceId\":\"TestLogicalResource\",\"RequestId\":\"cdc73f9d-aea9-11e3-9d5a-835b769c0d9c\",\"Data\":{\"property2\":\"\",\"property3\":[\"1\",\"2\",\"3\"],\"property1\":\"value1\"},\"PhysicalResourceId\":\"TestPhysicalResource\"}")")

Result:

("Optional("{\"NoEcho\":false,\"Data\":{\"property2\":\"\",\"property1\":\"value1\",\"property3\":[\"1\",\"2\",\"3\"]},\"StackId\":\"arn:aws:cloudformation:us-east-1:123456789:stack\\/TestStack\",\"LogicalResourceId\":\"TestLogicalResource\",\"Reason\":\"See the details in CloudWatch Log Stream\",\"Status\":\"SUCCESS\",\"RequestId\":\"cdc73f9d-aea9-11e3-9d5a-835b769c0d9c\",\"PhysicalResourceId\":\"TestPhysicalResource\"}")")

Mac OS 12.3.1 + Swift 5.6.1

Expected:

("Optional("{\"StackId\":\"arn:aws:cloudformation:us-east-1:123456789:stack\\/TestStack\",\"Reason\":\"See the details in CloudWatch Log Stream\",\"NoEcho\":false,\"Status\":\"SUCCESS\",\"LogicalResourceId\":\"TestLogicalResource\",\"RequestId\":\"cdc73f9d-aea9-11e3-9d5a-835b769c0d9c\",\"Data\":{\"property2\":\"\",\"property3\":[\"1\",\"2\",\"3\"],\"property1\":\"value1\"},\"PhysicalResourceId\":\"TestPhysicalResource\"}")")

Result:

("Optional("{\"StackId\":\"arn:aws:cloudformation:us-east-1:123456789:stack\\/TestStack\",\"Reason\":\"See the details in CloudWatch Log Stream\",\"NoEcho\":false,\"Status\":\"SUCCESS\",\"LogicalResourceId\":\"TestLogicalResource\",\"RequestId\":\"cdc73f9d-aea9-11e3-9d5a-835b769c0d9c\",\"Data\":{\"property2\":\"\",\"property3\":[\"1\",\"2\",\"3\"],\"property1\":\"value1\"},\"PhysicalResourceId\":\"TestPhysicalResource\"}")")

@tomerd
Copy link
Contributor

tomerd commented May 21, 2022

@swift-server-bot test this please

@tomerd
Copy link
Contributor

tomerd commented May 21, 2022

@renatoaguimaraes looks like some formatting issues left - you can fix that locally by running the formatting task in docker

@tomerd
Copy link
Contributor

tomerd commented May 21, 2022

@swift-server-bot test this please

@tomerd tomerd merged commit d6934fe into swift-server:main May 21, 2022
@tomerd
Copy link
Contributor

tomerd commented May 21, 2022

thank you @renatoaguimaraes

@renatoaguimaraes
Copy link
Contributor Author

thank you @renatoaguimaraes

Thank you. It is my pleasure to contribute to the project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants