-
Notifications
You must be signed in to change notification settings - Fork 17
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
Expose CloudFormation Custom Resource Emulator Resource #1807
Conversation
This change is part of the following stack: Change managed by git-spice. |
Does the PR have any schema changes?Looking good! No breaking changes found. New resources:
|
baf7ba4
to
1d4b118
Compare
adff217
to
8a8c517
Compare
c222a0e
to
a78e2d5
Compare
59357d1
to
94cfa06
Compare
a78e2d5
to
6464a5f
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1807 +/- ##
==========================================
+ Coverage 48.13% 49.65% +1.51%
==========================================
Files 43 43
Lines 6554 6559 +5
==========================================
+ Hits 3155 3257 +102
+ Misses 3156 3059 -97
Partials 243 243 ☔ View full report in Codecov by Sentry. |
94cfa06
to
19b72f2
Compare
6464a5f
to
939b0b3
Compare
@@ -1125,7 +1135,7 @@ func (p *cfnProvider) Delete(ctx context.Context, req *pulumirpc.DeleteRequest) | |||
KeepSecrets: true, | |||
}) | |||
if err != nil { | |||
return nil, errors.Wrapf(err, "failed to parse inputs for update") | |||
return nil, errors.Wrapf(err, "failed to parse inputs for delete") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found this copy&paste error in the message. We're in the delete
method here
19b72f2
to
bc9e116
Compare
ee9200c
to
ba6b3ba
Compare
|
||
CloudFormation Custom Resources allow you to write custom provisioning logic for resources that aren't directly available as AWS CloudFormation resource types. Common use cases include: | ||
|
||
- Managing resources outside of AWS (e.g., GitHub repositories, external APIs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we remove this one or add a comment saying that it would be better to use one of the other providers instead?
|
||
exports.handler = function(event, context) { | ||
|
||
console.log("REQUEST RECEIVED:\n" + JSON.stringify(event)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should remove the ResponseURL
when logging the event for security reasons.
}, | ||
}); | ||
|
||
const rpa1 = new awsClassic.iam.RolePolicyAttachment("lambdaRolePolicyAttachment1", { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as a side note it's hilarious to me that CCAPI doesn't support these basic iam resources. How are you supposed to do anything in AWS without these 🙃
|
||
// Create the Lambda function for the custom resource | ||
const lambdaFunction = new awsClassic.lambda.Function("ami-lookup-custom-resource", { | ||
runtime: awsClassic.types.enums.lambda.Runtime.NodeJS16dX, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use a newer version? This test will probably start failing sooner rather than later if we use this old of a runtime.
resourceType: 'Custom::MyResource', | ||
}, { customTimeouts: { create: '5m', update: '5m', delete: '5m' } }); | ||
|
||
const cloudformationStack = new awsClassic.cloudformation.Stack('stack', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
customResourceProperties: { | ||
hello: "world" | ||
}, | ||
serviceToken: "arn:aws:lambda:us-west-2:123456789012:function:my-custom-resource", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This lambda would be provisioned by CF, is that right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it is provisioned by the user in their Pulumi program or CloudFormation Stack.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This Example usage should probably have a link to how to do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Lambda Function Requirements
section explains how to do that. I don't think we should add a full Custom Resource implementation here. That would essentially be the cfn-custom-resource
example I've added (and that's multiple files)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes exactly, so perhaps consider adding a link to it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I can add that once it's merged into master :)
policyArn: awsClassic.iam.ManagedPolicies.AWSLambdaBasicExecutionRole, | ||
}); | ||
|
||
const bucket = new awsClassic.s3.Bucket('custom-resource-emulator', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BucketV2
Region: amiRegion, | ||
Architecture: 'HVM64', | ||
}, | ||
serviceToken: lambdaFunction.arn, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahhh so serviceToken is not called lambdaFunctionARN because some custom resources are backed by something else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, there's also SNS backed Custom Resources: #1812
They're not widely used, so we decided to skip support for them for now. It's not in scope of the CDK work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The naming makes a bit more sense now.
d64727a
to
5001132
Compare
cdfdb9b
to
914125c
Compare
This PR adds support for [CloudFormation Custom Resource](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/template-custom-resources.html) to the aws-native provider. It implements an emulator that enables Pulumi programs to interact with Lambda-backed CloudFormation Custom Resources. A CloudFormation custom resource is essentially an extension point to run arbitrary code as part of the CloudFormation lifecycle. It is similar in concept to the [Pulumi Command Provider](https://www.pulumi.com/registry/packages/command/), the difference being that CloudFormation CustomResources are executed in the Cloud; either through Lambda or SNS. For the first implementation we decided to limit the scope to Lambda backed Custom Resources, because the SNS variants are not widely used. ## Custom Resource Protocol The implementation follows the CloudFormation Custom Resource protocol. I derived the necessary parts by combining information from the [docs](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/crpg-ref.html), [CDKs CustomResource Framework](https://github.com/aws/aws-cdk/tree/main/packages/%40aws-cdk/custom-resource-handlers/lib/custom-resources-framework) and trial&error. Notable aspects of that protocol are: - primitive properties need to be string encoded when sending them to Custom Resource handlers. This includes deeply nested properties: aws-cloudformation/cloudformation-coverage-roadmap#1037 - The Lambda Function is invoked asynchronously. Lambda will retry the execution if the function fails unexpectedly (e.g. unhandled exception). - Due to the async invocation, the response is not returned from the Lambda Function, instead it's sent to a `ResponseURL` that needs to be included in the request payload. - Similarly to CloudFormation, we decided to implement this using S3 Buckets and presigned URLs. ### Custom Resource Lifecycle ```mermaid sequenceDiagram participant A as aws-native participant S3 as S3 Bucket participant L as Lambda %% Create Flow Note over A,L: Create Operation A->>S3: Generate presigned URL A->>L: Invoke with CREATE event activate L loop Until response found or timeout A->>S3: Poll for response L-->>S3: Upload response end deactivate L A->>S3: Fetch response alt Success A->>A: Store PhysicalId & outputs else Failure A->>A: Return error end %% Update Flow Note over A,L: Update Operation A->>S3: Generate presigned URL A->>L: Invoke with UPDATE event activate L loop Until response found or timeout A->>S3: Poll for response L-->>S3: Upload response end deactivate L A->>S3: Fetch response alt Success A->>A: Check PhysicalId alt ID Changed A->>S3: Generate presigned URL for cleanup A->>L: Invoke with DELETE event for old resource activate L loop Until cleanup response found or timeout A->>S3: Poll for cleanup response L-->>S3: Upload cleanup response end deactivate L A->>S3: Fetch cleanup response end else Failure A->>A: Return error end %% Delete Flow Note over A,L: Delete Operation A->>S3: Generate presigned URL A->>L: Invoke with DELETE event activate L loop Until response found or timeout A->>S3: Poll for response L-->>S3: Upload response end deactivate L A->>S3: Fetch response alt Success A->>A: Return success else Failure A->>A: Return error end ``` ## Reviewer Notes Key areas to review: 1. Error handling in the response collection mechanism 2. Timeout management, especially for the `Update` lifecycle 3. Documentation completeness and accuracy Exposing this resource and schematizing it is part of this PR #1807. Automatically cleaning up the response objects is not included in this PR in order to keep its size manageable. Implementing this is tracked here: #1813. Please pay special attention to: - S3 response collection mechanism security - State management during updates - Cleanup handling when physical resource IDs change ## Testing - Unit tests including error handling tests for various failure scenarios - Integration tests with actual Lambda functions are added in this stacked PR: #1807 ## Related Issues - pulumi/pulumi-cdk#109 - #1812 - #1813
8496d7d
to
670e340
Compare
// if the stack ID is not provided, we use the pulumi stack ID as the stack ID | ||
inputs[resource.PropertyKey("stackId")] = resource.NewStringProperty(urn.Stack().String()) | ||
} | ||
|
||
if typedInputs.CustomResourceProperties != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I decided to move stringifying
the customResourceProperties
into the Create/Update/Delete methods.
This is an implementation detail and shouldn't really be done in Check
. It would also complicate handling secrets and unknowns.
}, | ||
}, | ||
{ | ||
name: "Preserves Secrets", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
previewResult = test.Preview(t) | ||
assertpreview.HasNoChanges(t, previewResult) | ||
|
||
test.Destroy(t) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is automatically done and not necessary to call explicitly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it is, but I wanted to be explicit about this being one of the tested aspects. Happy to remove it though
} | ||
|
||
// Send response to the pre-signed S3 URL | ||
async function sendResponse(event, context, responseStatus, responseData) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am curious here, when people write lambdas to power custom resources in CloudFormation, do they need to take care of this functionality in user code or is there some library or framework that users can call in CF?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be part of the user code in the lambda function.
When using Inline Code in CloudFormation you can use the cfn-response
module if it's a node Lambda. That's automatically included in the Lambda Zip when using inline code.
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/cfn-lambda-function-code-cfnresponsemodule.html#cfn-lambda-function-code-cfnresponsemodule-source
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could use it in the example too presumably? https://www.npmjs.com/package/cfn-response seems available. No big deal though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's not the official version. Somebody just extracted it (from a lambda function) and published it on Lambda. I wouldn't wanna rely on this in-official version for the example.
Not worth it for a simple http put request IMO
This change exposes the new CloudFormation Custom Resource Emulator resource.
Additionally, it adds an integration test for it and makes
Check
correctly handle unknowns.One follow up item is to translate the code example to other languages.