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

Add event definitions for CloudFormation custom resources #695

Merged
merged 3 commits into from
Sep 11, 2023

Conversation

chris-leach
Copy link
Contributor

@chris-leach chris-leach commented Sep 10, 2023

Issue #, if available:
Related to #680 - see below

Description of changes:
This PR adds event definitions for CloudFormation custom resource request objects. Requests are either a Create, Update, or Delete event, implemented as enum variants with internally tagged representation.

Note that, in addition to the fields documented in the links above, in practice CloudFormation sends a ServiceToken field in every request. Since this isn't documented I've encoded it as an Option<String> with #[serde(default)], so deserialisation will tolerate it being missing, but I can change this if desired.


This is intended as an initial building block of a framework for implementing Lambda custom resource handlers, of the kind requested in #680. These work by invoking a Lambda with one of the above event types, all of which include an S3 presigned URL to which a response must be posted. Failing to post a response can result in long delays, so tools to make it easy to send a response, and a fallback mechanism to send a failure response if the Lambda is about to time out, are desirable.

I'd appreciate feedback on whether further PRs to add this functionality (probably in a lambda_cfn crate) would be appreciated, but either way I think the event definitions are a useful addition.

By submitting this pull request

  • I confirm that my contribution is made under the terms of the Apache 2.0 license.
  • I confirm that I've made a best effort attempt to update all relevant documentation.

@calavera
Copy link
Contributor

Thanks for adding those events!

I'd appreciate feedback on whether further PRs to add this functionality (probably in a lambda_cfn crate) would be appreciated, but either way I think the event definitions are a useful addition.

I'm hesitate to add more crates to this repository. I personally don't know much about CFN, so I would not be able to maintain that code. It'd probably be better if you want to create it and maintain it on your own.

Copy link
Contributor

@calavera calavera left a comment

Choose a reason for hiding this comment

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

Can you review the errors and fix them? 🙏

@chris-leach
Copy link
Contributor Author

@calavera Done - I've removed the let ... else statements that I now realise were stabilised one release after the MSRV.

Copy link
Contributor

@calavera calavera left a comment

Choose a reason for hiding this comment

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

Just one more fix:

error: use of deprecated method `chrono::TimeZone::datetime_from_str`: use `DateTime::parse_from_str` instead
  --> lambda-events/src/custom_serde/codebuild_time.rs:21:13
   |
21 |         Utc.datetime_from_str(val, CODEBUILD_TIME_FORMAT)
   |             ^^^^^^^^^^^^^^^^^

@chris-leach
Copy link
Contributor Author

@calavera This doesn't appear to be related to my changes - would you still like me to fix it in this PR?

@chris-leach
Copy link
Contributor Author

@calavera pushed a fix for this - please review

Copy link
Contributor

@calavera calavera left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@calavera calavera merged commit e2d51ad into awslabs:main Sep 11, 2023
5 checks passed
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.

2 participants