From 789730542b4a2b9ef0b7779fc66dd0c37dc44bad Mon Sep 17 00:00:00 2001 From: Chris Rehn <1280602+hoffa@users.noreply.github.com> Date: Mon, 12 Dec 2022 17:13:20 -0800 Subject: [PATCH 1/5] Update PULL_REQUEST_TEMPLATE.md --- .github/PULL_REQUEST_TEMPLATE.md | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index c1aa420f56..a0ab4d8d95 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -7,20 +7,14 @@ ### Checklist - [ ] Adheres to the [development guidelines](https://github.com/aws/serverless-application-model/blob/develop/DEVELOPMENT_GUIDE.md#development-guidelines) -- [ ] Add/update [unit tests](https://github.com/aws/serverless-application-model/blob/develop/DEVELOPMENT_GUIDE.md#unit-testing-with-multiple-python-versions) using: - - [ ] Correct values - - [ ] Bad/wrong values (None, empty, wrong type, length, etc.) - - [ ] [Intrinsic Functions](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference.html) +- [ ] Add/update [transform tests](https://github.com/aws/serverless-application-model/blob/develop/DEVELOPMENT_GUIDE.md#unit-testing-with-multiple-python-versions) + - [ ] Using correct values + - [ ] Using wrong values - [ ] Add/update [integration tests](https://github.com/aws/serverless-application-model/blob/develop/INTEGRATION_TESTS.md) -- [ ] Update documentation -- [ ] Verify transformed template deploys and application functions as expected -- [ ] Do these changes include any template validations? - - [ ] Did the newly validated properties support intrinsics prior to adding the validations? (If unsure, please review [Intrinsic Functions](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference.html) before proceeding). - - [ ] Does the pull request ensure that intrinsics remain functional with the new validations? ### Examples? -Please reach out in the comments, if you want to add an example. Examples will be -added to `sam init` through https://github.com/awslabs/aws-sam-cli-app-templates/ +Please reach out in the comments if you want to add an example. Examples will be +added to `sam init` through [aws/aws-sam-cli-app-templates](https://github.com/aws/aws-sam-cli-app-templates). By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. From 4fc74a8041f51f4340d0ff1ea8464e79c74d4083 Mon Sep 17 00:00:00 2001 From: Chris Rehn <1280602+hoffa@users.noreply.github.com> Date: Mon, 12 Dec 2022 17:14:25 -0800 Subject: [PATCH 2/5] Update PULL_REQUEST_TEMPLATE.md --- .github/PULL_REQUEST_TEMPLATE.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index a0ab4d8d95..aa83bf4ddc 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -7,10 +7,10 @@ ### Checklist - [ ] Adheres to the [development guidelines](https://github.com/aws/serverless-application-model/blob/develop/DEVELOPMENT_GUIDE.md#development-guidelines) -- [ ] Add/update [transform tests](https://github.com/aws/serverless-application-model/blob/develop/DEVELOPMENT_GUIDE.md#unit-testing-with-multiple-python-versions) +- [ ] Add [transform tests](https://github.com/aws/serverless-application-model/blob/develop/DEVELOPMENT_GUIDE.md#unit-testing-with-multiple-python-versions) - [ ] Using correct values - [ ] Using wrong values -- [ ] Add/update [integration tests](https://github.com/aws/serverless-application-model/blob/develop/INTEGRATION_TESTS.md) +- [ ] Add [integration tests](https://github.com/aws/serverless-application-model/blob/develop/INTEGRATION_TESTS.md) ### Examples? From a11aaa333f10b1ba25f16498e3f69d91c6e4f166 Mon Sep 17 00:00:00 2001 From: Chris Rehn <1280602+hoffa@users.noreply.github.com> Date: Mon, 12 Dec 2022 17:15:11 -0800 Subject: [PATCH 3/5] Update PULL_REQUEST_TEMPLATE.md --- .github/PULL_REQUEST_TEMPLATE.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index aa83bf4ddc..a0ab4d8d95 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -7,10 +7,10 @@ ### Checklist - [ ] Adheres to the [development guidelines](https://github.com/aws/serverless-application-model/blob/develop/DEVELOPMENT_GUIDE.md#development-guidelines) -- [ ] Add [transform tests](https://github.com/aws/serverless-application-model/blob/develop/DEVELOPMENT_GUIDE.md#unit-testing-with-multiple-python-versions) +- [ ] Add/update [transform tests](https://github.com/aws/serverless-application-model/blob/develop/DEVELOPMENT_GUIDE.md#unit-testing-with-multiple-python-versions) - [ ] Using correct values - [ ] Using wrong values -- [ ] Add [integration tests](https://github.com/aws/serverless-application-model/blob/develop/INTEGRATION_TESTS.md) +- [ ] Add/update [integration tests](https://github.com/aws/serverless-application-model/blob/develop/INTEGRATION_TESTS.md) ### Examples? From d115090435c7cba1e95f5c31d1a658c028fc2bfd Mon Sep 17 00:00:00 2001 From: Chris Rehn <1280602+hoffa@users.noreply.github.com> Date: Mon, 12 Dec 2022 17:17:23 -0800 Subject: [PATCH 4/5] Update DEVELOPMENT_GUIDE.md --- DEVELOPMENT_GUIDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DEVELOPMENT_GUIDE.md b/DEVELOPMENT_GUIDE.md index df13925944..33c13ae973 100644 --- a/DEVELOPMENT_GUIDE.md +++ b/DEVELOPMENT_GUIDE.md @@ -157,7 +157,7 @@ Integration tests are covered in detail in the [INTEGRATION_TESTS.md file](INTEG ## Development guidelines -1. **Do not resolve [intrinsic functions](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference.html).** Adding [`AWS::LanguageExtensions`](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-languageextension-transform.html) before the `AWS::Serverless-2016-10-31` transform resolves most of them (see https://github.com/aws/serverless-application-model/issues/2533). Use [`Property`](https://github.com/aws/serverless-application-model/blob/c5830b63857f52e540fec13b29f029458edc539a/samtranslator/model/__init__.py#L36-L45) or [`PassThroughProperty`](https://github.com/aws/serverless-application-model/blob/dd79f535500158baa8e367f081d6a12113497e45/samtranslator/model/__init__.py#L48-L56) instead of [`PropertyType`](https://github.com/aws/serverless-application-model/blob/develop/samtranslator/model/__init__.py#L13-L33). +1. **Do not resolve [intrinsic functions](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference.html).** Adding [`AWS::LanguageExtensions`](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-languageextension-transform.html) before the `AWS::Serverless-2016-10-31` transform resolves most of them (see https://github.com/aws/serverless-application-model/issues/2533). Use [`Property`](https://github.com/aws/serverless-application-model/blob/c5830b63857f52e540fec13b29f029458edc539a/samtranslator/model/__init__.py#L36-L45) or [`PassThroughProperty`](https://github.com/aws/serverless-application-model/blob/dd79f535500158baa8e367f081d6a12113497e45/samtranslator/model/__init__.py#L48-L56) instead of [`PropertyType`](https://github.com/aws/serverless-application-model/blob/c39c2807bbf327255de8abed8b8150b18c60f053/samtranslator/model/__init__.py#L13-L33) for new properties. 2. **Do not break backward compatibility.** As rule of thumb, a specific SAM template should always transform into the same CloudFormation template. Do not change logical IDs. Add opt-in properties for breaking changes. There are some exceptions, such as changes that do not impact resources (e.g. [`Metadata`](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/metadata-section-structure.html)) or abstractions that can by design change over time. 3. **Stick as close as possible to the underlying CloudFormation properties.** This includes both property names and values. This ensures we can pass values to CloudFormation and let it handle any intrinsic functions. In some cases, it also allows us to pass all properties as-is to a resource, which means customers can always use the newest properties, and we don’t spend effort maintaining a duplicate set of properties. 4. **Only validate what’s necessary.** Do not validate properties if they’re passed directly to the underlying CloudFormation resource. From 963135ebf77013ff9c0b7905c97c8440deaaf315 Mon Sep 17 00:00:00 2001 From: Chris Rehn <1280602+hoffa@users.noreply.github.com> Date: Mon, 12 Dec 2022 17:17:53 -0800 Subject: [PATCH 5/5] Update DEVELOPMENT_GUIDE.md --- DEVELOPMENT_GUIDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DEVELOPMENT_GUIDE.md b/DEVELOPMENT_GUIDE.md index 33c13ae973..93c82b604d 100644 --- a/DEVELOPMENT_GUIDE.md +++ b/DEVELOPMENT_GUIDE.md @@ -157,7 +157,7 @@ Integration tests are covered in detail in the [INTEGRATION_TESTS.md file](INTEG ## Development guidelines -1. **Do not resolve [intrinsic functions](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference.html).** Adding [`AWS::LanguageExtensions`](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-languageextension-transform.html) before the `AWS::Serverless-2016-10-31` transform resolves most of them (see https://github.com/aws/serverless-application-model/issues/2533). Use [`Property`](https://github.com/aws/serverless-application-model/blob/c5830b63857f52e540fec13b29f029458edc539a/samtranslator/model/__init__.py#L36-L45) or [`PassThroughProperty`](https://github.com/aws/serverless-application-model/blob/dd79f535500158baa8e367f081d6a12113497e45/samtranslator/model/__init__.py#L48-L56) instead of [`PropertyType`](https://github.com/aws/serverless-application-model/blob/c39c2807bbf327255de8abed8b8150b18c60f053/samtranslator/model/__init__.py#L13-L33) for new properties. +1. **Do not resolve [intrinsic functions](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference.html).** Adding [`AWS::LanguageExtensions`](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-languageextension-transform.html) before the `AWS::Serverless-2016-10-31` transform resolves most of them (see https://github.com/aws/serverless-application-model/issues/2533). For new properties, use [`Property`](https://github.com/aws/serverless-application-model/blob/c5830b63857f52e540fec13b29f029458edc539a/samtranslator/model/__init__.py#L36-L45) or [`PassThroughProperty`](https://github.com/aws/serverless-application-model/blob/dd79f535500158baa8e367f081d6a12113497e45/samtranslator/model/__init__.py#L48-L56) instead of [`PropertyType`](https://github.com/aws/serverless-application-model/blob/c39c2807bbf327255de8abed8b8150b18c60f053/samtranslator/model/__init__.py#L13-L33). 2. **Do not break backward compatibility.** As rule of thumb, a specific SAM template should always transform into the same CloudFormation template. Do not change logical IDs. Add opt-in properties for breaking changes. There are some exceptions, such as changes that do not impact resources (e.g. [`Metadata`](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/metadata-section-structure.html)) or abstractions that can by design change over time. 3. **Stick as close as possible to the underlying CloudFormation properties.** This includes both property names and values. This ensures we can pass values to CloudFormation and let it handle any intrinsic functions. In some cases, it also allows us to pass all properties as-is to a resource, which means customers can always use the newest properties, and we don’t spend effort maintaining a duplicate set of properties. 4. **Only validate what’s necessary.** Do not validate properties if they’re passed directly to the underlying CloudFormation resource.