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(assets): enable local tooling scenarios such as lambda debugging #1433

Merged
merged 7 commits into from
Dec 27, 2018

Conversation

eladb
Copy link
Contributor

@eladb eladb commented Dec 26, 2018

Adds CloudFormation resource metadata which enables tools such as SAM
CLI to find local assets used by resources in the template.

See design document under design/code-asset-metadata.md

Fixes #1432

Initial RFC for design of #1432
@eladb eladb requested a review from a team as a code owner December 26, 2018 17:46
@eladb
Copy link
Contributor Author

eladb commented Dec 26, 2018

@jfuss please review this and let me know if this looks good

Elad Ben-Israel added 2 commits December 26, 2018 21:48
Adds CloudFormation resource metadata which enables tools such as SAM
CLI to find local assets used by resources in the template.

See design document under [design/code-asset-metadata.md](./design/code-asset-metadata.md)

Fixes #1432
@eladb eladb changed the title doc: RFC for code asset metadata feat(assets): enable local tooling scenarios such as lambda debugging Dec 27, 2018
@jfuss
Copy link
Contributor

jfuss commented Dec 27, 2018

Looking now.


## Approach

We will automatically embed CloudFormation metadata on `AWS::Lambda::Function` resources which use
Copy link
Contributor

Choose a reason for hiding this comment

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

Why only AWS::Lambda::Functions? What about AWS::Lambda::LayerVersion, AWS::Serverless::Function, and AWS::Serverless::LayerVersion?

We are purposefully excluding API Gateway from this correct?

Is there a generic way this could work to allow any resource using an Asset to get this metadata? Trying to think broader here to make less work in the future of manually adding this metadata to every resource.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The implementation allows anyone who uses an asset to attach this metadata to the resource by calling asset.addResourceMetadata(resource, prop). I decided against doing this automatically because this problem is basically confined to only L2 constructs. Any higher level constructs shouldn't care about this at all, so I opted for a more manual solution.

Layers are currently in PR (#1411) and @RomainMuller should probably.

Copy link
Contributor

Choose a reason for hiding this comment

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

@eladb What about AWS::Serverless::Function?

Copy link
Contributor Author

@eladb eladb Dec 27, 2018

Choose a reason for hiding this comment

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

@jfuss the AWS::Serverless::Function is currently only supported as an L1 construct (serverless.CfnFunction), and I suspect people who use it will likely not try to use it with CDK assets but rather with CodeUri. For example:

new serverless.CfnFunction(this, 'Func', {
  codeUri: 'file://mycode',
   runtime: 'nodejs8.10',
   handler: 'index.handler'
});

If users wish to use CDK assets (and invoke them locally through SAM CLI), this is what they will have to do:

const asset = new assets.ZipDirectoryAsset(this, 'Foo', {
  path: '/foo/boom'
});

const resource = new serverless.CfnFunction(this, 'Func', {
    codeUri: {
    bucket: asset.s3BucketName,
    key: asset.s3ObjectKey
  },
  runtime: 'nodejs8.10',
  handler: 'index.handler'
});

resource.addResourceMetadata(resource, 'CodeUri');

But TBH, I doubt that this makes sense. If you are already writing "CDK native" code, the L2 constructs for Lambda, API Gateway, etc provides a much richer API then the AWS::Serverless::Function resource.

What do you think?

Copy link
Contributor

@jfuss jfuss Dec 27, 2018

Choose a reason for hiding this comment

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

@eladb I see. So it's more like Assets aren't directly supported within AWS::Serverless::Function like it is with AWS::Lambda::Functions (over simplifying here due to the Construct level difference). I see Assets being powerful and something we should consider supporting directly but out of scope for this PR.

Thanks for clarifying this.

@eladb eladb merged commit 0d2b633 into master Dec 27, 2018
@eladb eladb deleted the benisrae/sam-cli-metadata branch December 27, 2018 18:26
@NGL321 NGL321 added the contribution/core This is a PR that came from AWS. label Sep 23, 2019
@mergify
Copy link
Contributor

mergify bot commented Sep 23, 2019

Thanks so much for taking the time to contribute to the AWS CDK ❤️

We will shortly assign someone to review this pull request and help get it
merged. In the meantime, please take a minute to make sure you follow this
checklist
:

  • PR title type(scope): text
    • type: fix, feat, refactor go into CHANGELOG, chore is hidden
    • scope: name of module without aws- or cdk- prefix or postfix (e.g. s3 instead of aws-s3-deployment)
    • text: use all lower-case, do not end with a period, do not include issue refs
  • PR Description
    • Rationale: describe rationale of change and approach taken
    • Issues: indicate issues fixed via: fixes #xxx or closes #xxx
    • Breaking?: last paragraph: BREAKING CHANGE: <describe what changed + link for details>
  • Testing
    • Unit test added. Prefer to add a new test rather than modify existing tests
    • CLI or init templates change? Re-run/add CLI integration tests
  • Documentation
    • README: update module README to describe new features
    • API docs: public APIs must be documented. Copy from official AWS docs when possible
    • Design: for significant features, follow design process

mergify bot pushed a commit that referenced this pull request Nov 11, 2021
…al tooling (#17343)

----
Reference issue #14593
Building on this initial PR: #1433

We're looking to add asset metadata to the NestedStack resource. The implementation is similar to this one [design/code-asset-metadata.md](https://github.com/aws/aws-cdk/pull/design/code-asset-metadata.md). This will allow SAM CLI to support CDK-synthed templates nested function resources.

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
mergify bot pushed a commit that referenced this pull request Nov 15, 2021
)

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*
mpvosseller pushed a commit to mpvosseller/aws-cdk that referenced this pull request Nov 16, 2021
…#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*
mergify bot pushed a commit that referenced this pull request Nov 19, 2021
… resource provider functions (#17551)

----
Following up on issue #14593 and PR #1433. 
It seems that log retention and customer resource provider constructs create the corresponding lambda functions using ```CfnResource()``` which means that the asset metadata isn't appended to the output template. 

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this pull request Feb 21, 2022
…al tooling (aws#17343)

----
Reference issue aws#14593
Building on this initial PR: aws#1433

We're looking to add asset metadata to the NestedStack resource. The implementation is similar to this one [design/code-asset-metadata.md](https://github.com/aws/aws-cdk/pull/design/code-asset-metadata.md). This will allow SAM CLI to support CDK-synthed templates nested function resources.

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this pull request Feb 21, 2022
…#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*
TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this pull request Feb 21, 2022
… resource provider functions (aws#17551)

----
Following up on issue aws#14593 and PR aws#1433. 
It seems that log retention and customer resource provider constructs create the corresponding lambda functions using ```CfnResource()``` which means that the asset metadata isn't appended to the output template. 

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lambda code: add asset metadata to support SAM CLI
4 participants