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

New Router: CloudFormation Custom Resource #1268

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

willfarrell
Copy link
Member

@willfarrell willfarrell commented Dec 13, 2024

#1260

Todo List:

  • Feature/Fix fully implemented
  • All commits are cryptographically signed
  • Updated relevant types
  • Added tests (if applicable)
    • Unit tests
    • Fuzz tests
    • Type tests
    • Benchmark tests
  • Updated relevant documentation
  • Updated relevant examples

Copy link

socket-security bot commented Dec 13, 2024

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher

View full report↗︎

@willfarrell willfarrell changed the title Feature/cloudformation custom resource router New Router: CloudFormation Custom Resource Dec 13, 2024
@willfarrell willfarrell marked this pull request as draft December 13, 2024 16:38
Comment on lines 32 to 39
if (typeof handler !== 'undefined') {
const response = handler(event, context, abort)
response.Status ??= 'SUCCESS'
response.RequestId ??= event.RequestId
response.LogicalResourceId ??= event.LogicalResourceId
response.StackId ??= event.StackId
return response
}

Choose a reason for hiding this comment

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

It would be nice if it also reported to the responseUrl, I feel like even though the abstraction is nice, it doesn't really add that much value currently as it's still the responsibility of the developer to correctly handle all errors and respond.

The auto reporting implementation is more what I was looking for honestly

return (event, context, abort) => {
const { RequestType: requestType } = event
if (!requestType) {
return notFoundResponse({ requestType })

Choose a reason for hiding this comment

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

I would for example expect that here we report to cloudformation that we have a failure, because if we don't do that cloudformation will simply wait x time until it timeouts whilst we already know that we won't be handling the request

@willfarrell willfarrell marked this pull request as ready for review January 3, 2025 01:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants