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(core): move all types from "aws-cloudformation" to "core" #7736

Merged
merged 22 commits into from
May 4, 2020

Conversation

eladb
Copy link
Contributor

@eladb eladb commented May 1, 2020

Commit Message

feat(core): move all types from "aws-cloudformation" to "core" (#7736)

This commit folds the CustomResource and NestedStack types from @aws-cdk/aws-cloudformation into @aws-cdk/core in order to allow code in core and other lower layers to use capabilities such as nested stacks and custom resources.

This comes at a minor sacrifice to API fidelity: the provider's service token is for custom resources is now passed as a simple string instead of a strongly typed ICustomResourceProvider. But this is negligible for this type of resource given the high involvement users require to use it anyway. Additionally, the NestedStack class accepts a notificationArns as a string[] instead of an sns.ITopic[]. In both cases the API in @aws-cdk/aws-cloudformation (which is considered a stable module) remains unchanged with a compatibility layer added.

We took this opportunity to change the behavior of custom resources so that it won't pascal-case property names by default. This resolves #4896 and resolves #7035 and supersedes #7034.

The API in the aws-cloudformation module are still supported for backwards compatibility but marked as deprecated.

End Commit Message


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

Elad Ben-Israel added 5 commits April 30, 2020 19:22
Fold the "assets" module, which includes the Staging construct that takes care of staging asset files into the cloud assembly during synthesis into "core". This is in order to allow implementing custom resources that leverage assets throughout the framework.

A subsequent commit will add a mini-framework for custom resources that leverages this capability.
This commit folds the `CustomResource` and `NestedStack` types from `@aws-cdk/aws-cloudformation` into `@aws-cdk/core` in order to allow code in `core` and other lower layers to use capabilities such as nested stacks and custom resources.

This comes at a minor sacrifice to API fidelity: the provider's service token is for custom resources is now passed as a simple `string` instead of a strongly typed `ICustomResourceProvider`. But this is negligible for this type of resource given the high involvement users require to use it anyway. Additionally, the `NestedStack` class accepts a `notificationArns` as a `string[]` instead of an `sns.ITopic[]`. In both cases the API in `@aws-cdk/aws-cloudformation` (which is considered a stable module) remains unchanged with a compatibility layer added.

We took this opportunity to change the behavior of custom resources so that it won't pascal-case property names by default. This resolves #4896 and resolves #7035 and supersedes #7034.

The API in the aws-cloudformation module are still supported for backwards compatibility but marked as deprecated.
@eladb eladb added the pr/do-not-merge This PR should not be merged at this time. label May 1, 2020
@eladb eladb requested a review from a team May 1, 2020 11:52
@eladb eladb changed the title feat(core): fold "aws-cloudformation" into "core" feat(core): move all types from "aws-cloudformation" into "core" May 1, 2020
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label May 1, 2020
@eladb eladb changed the title feat(core): move all types from "aws-cloudformation" into "core" feat(core): move all types from "aws-cloudformation" to "core" May 1, 2020
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 81d80ea
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 093f300
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: a3b0b8f
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 94beaf0
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@eladb eladb changed the base branch from benisrae/refactor/assets to master May 3, 2020 09:50
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: ce4e43b
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 60c51f6
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 3877e65
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: c03e31e
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@eladb eladb removed the pr/do-not-merge This PR should not be merged at this time. label May 4, 2020
@eladb eladb self-assigned this May 4, 2020
Elad Ben-Israel added 2 commits May 4, 2020 14:22
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: b027158
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 79c8419
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

* serviceToken: myTopic.topicArn
* ```
*/
readonly serviceToken: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

Make this an IServiceToken or something with a single string attribute?

We can still have it typed, even if the dependency goes the other way. We can have class Function implements IServiceToken, for example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will "pollute" the public APIs of lambda.Function and sns.Topic in an unwanted way, and I don't think this is warranted for this use case. The documentation is clear and easy enough to follow and we expect people who implement custom resources to be able to easily follow it.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you think it's polluting Function then we could easily have migrated the ResourceProvider as well.

But fine, strings it is.

Copy link
Contributor Author

@eladb eladb May 4, 2020

Choose a reason for hiding this comment

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

If you think it's polluting Function then we could easily have migrated the ResourceProvider as well.

How? ResourceProvider.fromLambda references a IFunction which does not exist in core.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: d190ad9
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: b73f68f
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 867a534
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: c7a6815
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 2b1a4ae
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: b8f7ee3
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify
Copy link
Contributor

mergify bot commented May 4, 2020

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-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 48c759f
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: fd18b61
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify
Copy link
Contributor

mergify bot commented May 4, 2020

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).

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.

Custom Resource Properties Keys Are Being Uppercased Custom resource capitalises properties - undocumented
3 participants