-
Notifications
You must be signed in to change notification settings - Fork 4k
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
fix(apigateway): SAM CLI asset metadata missing from SpecRestApi #17293
Conversation
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.
Can you explain what is going on with the changes to cdk-integ?
The integration between SAM and CDK was implemented before using this design. The idea is SAM needs to know the local path of the assets attached to Lambda Function so SAM can implement the local testing and debugging functionalities. The solution was to let CDK add metadata properties for the Lambda Function resources to let SAM know the code path. In This PR we want to extend this solution to SpecRestApi resource to add the assets metadata to it in case if it is using AssetApiDefinition. |
in this PR .. I also updated the Lambda function runtime from NodeJs10 to NodeJs14 to fix the integration testing failures. |
* Definition to bind to it. Specifically it's required to allow assets to add | ||
* metadata for tooling like SAM CLI to be able to find their origins. | ||
*/ | ||
public bindToResource(_resource: CfnResource) { |
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.
Can we make this: bindAfterCreate(scope: Construct, restApi: RestApi)
?
Call it like apiDefinition.bindAfterCreate(this, this)
, and you can access the CfnResource inside like:
const child = Node.of(restApi).defaultChild as CfnRestApi;
child.addMetadata(...);
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.
sure
Pull request has been modified.
Pull request has been modified.
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
) Adds asset metadata to image-type lambda functions. This will allow SAM CLI to support local invocation of image-type lambdas from CDK-synthed templates. It follows the same design and builds upon #1433 Fixes #14593 Uses some changes from #17293 to enable asset metadata generation in integration tests *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…#17368) Adds asset metadata to image-type lambda functions. This will allow SAM CLI to support local invocation of image-type lambdas from CDK-synthed templates. It follows the same design and builds upon aws#1433 Fixes aws#14593 Uses some changes from aws#17293 to enable asset metadata generation in integration tests *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…#17293) Adds Assets metadata to RestApi resource in case if AssetApiDefinition is used. This Metadata will enable SAM CLI to find local assets used by RestApi in the template. It follows the same design in document [design/code-asset-metadata.md](https://github.com/aws/aws-cdk/pull/design/code-asset-metadata.md) Fixes aws#14593 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…#17368) Adds asset metadata to image-type lambda functions. This will allow SAM CLI to support local invocation of image-type lambdas from CDK-synthed templates. It follows the same design and builds upon aws#1433 Fixes aws#14593 Uses some changes from aws#17293 to enable asset metadata generation in integration tests *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Adds Assets metadata to RestApi resource in case if AssetApiDefinition is used. This Metadata will enable SAM
CLI to find local assets used by RestApi in the template.
It follows the same design in document design/code-asset-metadata.md
Fixes #14593
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license