From 02b90388497fd525ea15bc5f7d6ad27653320818 Mon Sep 17 00:00:00 2001 From: Gavin Zhang Date: Wed, 14 Dec 2022 14:13:58 -0800 Subject: [PATCH 1/5] Fix warning when using default stage name and FailOnWarnings is on --- samtranslator/model/api/http_api_generator.py | 14 ++++++++++++++ .../output/aws-cn/explicit_http_api.json | 2 +- .../output/aws-cn/http_api_explicit_stage.json | 2 +- ...th_default_stage_name_and_fail_on_warnings.json | 2 +- .../output/aws-us-gov/explicit_http_api.json | 2 +- .../output/aws-us-gov/http_api_explicit_stage.json | 2 +- ...th_default_stage_name_and_fail_on_warnings.json | 2 +- tests/translator/output/explicit_http_api.json | 2 +- .../translator/output/http_api_explicit_stage.json | 2 +- ...th_default_stage_name_and_fail_on_warnings.json | 2 +- 10 files changed, 23 insertions(+), 9 deletions(-) diff --git a/samtranslator/model/api/http_api_generator.py b/samtranslator/model/api/http_api_generator.py index 54cf31ed27..adb81558b2 100644 --- a/samtranslator/model/api/http_api_generator.py +++ b/samtranslator/model/api/http_api_generator.py @@ -120,6 +120,7 @@ def _construct_http_api(self) -> ApiGatewayV2HttpApi: self._add_title() self._add_description() + self._update_paths() if self.definition_uri: http_api.BodyS3Location = self._construct_body_s3_dict(self.definition_uri) @@ -224,6 +225,19 @@ def _add_cors(self) -> None: # Assign the OpenApi back to template self.definition_body = editor.openapi + def _update_paths(self): + if not self.fail_on_warnings: + return + + # Using default stage name generate warning during deployment + # Warnings found during import: Parse issue: attribute paths. + # Resource $default should start with / (Service: AmazonApiGatewayV2; Status Code: 400; + # Deployment fails when FailOnWarnings is true + paths: Optional[Dict[str, Any]] = self.definition_body.get("paths") + if DefaultStageName in paths: + paths[f"/{DefaultStageName}"] = paths.pop(DefaultStageName) + + def _construct_api_domain( self, http_api: ApiGatewayV2HttpApi, route53_record_set_groups: Dict[str, Route53RecordSetGroup] ) -> Tuple[ diff --git a/tests/translator/output/aws-cn/explicit_http_api.json b/tests/translator/output/aws-cn/explicit_http_api.json index 31d0cee889..7773386cbc 100644 --- a/tests/translator/output/aws-cn/explicit_http_api.json +++ b/tests/translator/output/aws-cn/explicit_http_api.json @@ -123,7 +123,7 @@ }, "openapi": "3.0.1", "paths": { - "$default": { + "/$default": { "x-amazon-apigateway-any-method": { "isDefaultRoute": true, "responses": {}, diff --git a/tests/translator/output/aws-cn/http_api_explicit_stage.json b/tests/translator/output/aws-cn/http_api_explicit_stage.json index 4cf02a5c45..8493132490 100644 --- a/tests/translator/output/aws-cn/http_api_explicit_stage.json +++ b/tests/translator/output/aws-cn/http_api_explicit_stage.json @@ -95,7 +95,7 @@ }, "openapi": "3.0.1", "paths": { - "$default": { + "/$default": { "x-amazon-apigateway-any-method": { "isDefaultRoute": true, "responses": {}, diff --git a/tests/translator/output/aws-cn/http_api_with_default_stage_name_and_fail_on_warnings.json b/tests/translator/output/aws-cn/http_api_with_default_stage_name_and_fail_on_warnings.json index 15c1990aa8..3c536e79a2 100644 --- a/tests/translator/output/aws-cn/http_api_with_default_stage_name_and_fail_on_warnings.json +++ b/tests/translator/output/aws-cn/http_api_with_default_stage_name_and_fail_on_warnings.json @@ -11,7 +11,7 @@ }, "openapi": "3.0.1", "paths": { - "$default": { + "/$default": { "x-amazon-apigateway-any-method": { "isDefaultRoute": true, "responses": {}, diff --git a/tests/translator/output/aws-us-gov/explicit_http_api.json b/tests/translator/output/aws-us-gov/explicit_http_api.json index d996571760..be307679a5 100644 --- a/tests/translator/output/aws-us-gov/explicit_http_api.json +++ b/tests/translator/output/aws-us-gov/explicit_http_api.json @@ -123,7 +123,7 @@ }, "openapi": "3.0.1", "paths": { - "$default": { + "/$default": { "x-amazon-apigateway-any-method": { "isDefaultRoute": true, "responses": {}, diff --git a/tests/translator/output/aws-us-gov/http_api_explicit_stage.json b/tests/translator/output/aws-us-gov/http_api_explicit_stage.json index 1b07f1df84..78bebc9c5b 100644 --- a/tests/translator/output/aws-us-gov/http_api_explicit_stage.json +++ b/tests/translator/output/aws-us-gov/http_api_explicit_stage.json @@ -95,7 +95,7 @@ }, "openapi": "3.0.1", "paths": { - "$default": { + "/$default": { "x-amazon-apigateway-any-method": { "isDefaultRoute": true, "responses": {}, diff --git a/tests/translator/output/aws-us-gov/http_api_with_default_stage_name_and_fail_on_warnings.json b/tests/translator/output/aws-us-gov/http_api_with_default_stage_name_and_fail_on_warnings.json index afd8071cd1..b93fb4f4fb 100644 --- a/tests/translator/output/aws-us-gov/http_api_with_default_stage_name_and_fail_on_warnings.json +++ b/tests/translator/output/aws-us-gov/http_api_with_default_stage_name_and_fail_on_warnings.json @@ -11,7 +11,7 @@ }, "openapi": "3.0.1", "paths": { - "$default": { + "/$default": { "x-amazon-apigateway-any-method": { "isDefaultRoute": true, "responses": {}, diff --git a/tests/translator/output/explicit_http_api.json b/tests/translator/output/explicit_http_api.json index 852b31e08f..dc3077feb5 100644 --- a/tests/translator/output/explicit_http_api.json +++ b/tests/translator/output/explicit_http_api.json @@ -123,7 +123,7 @@ }, "openapi": "3.0.1", "paths": { - "$default": { + "/$default": { "x-amazon-apigateway-any-method": { "isDefaultRoute": true, "responses": {}, diff --git a/tests/translator/output/http_api_explicit_stage.json b/tests/translator/output/http_api_explicit_stage.json index ee59ef6ea1..380ca09378 100644 --- a/tests/translator/output/http_api_explicit_stage.json +++ b/tests/translator/output/http_api_explicit_stage.json @@ -95,7 +95,7 @@ }, "openapi": "3.0.1", "paths": { - "$default": { + "/$default": { "x-amazon-apigateway-any-method": { "isDefaultRoute": true, "responses": {}, diff --git a/tests/translator/output/http_api_with_default_stage_name_and_fail_on_warnings.json b/tests/translator/output/http_api_with_default_stage_name_and_fail_on_warnings.json index 7c3c6241d1..97b131db27 100644 --- a/tests/translator/output/http_api_with_default_stage_name_and_fail_on_warnings.json +++ b/tests/translator/output/http_api_with_default_stage_name_and_fail_on_warnings.json @@ -11,7 +11,7 @@ }, "openapi": "3.0.1", "paths": { - "$default": { + "/$default": { "x-amazon-apigateway-any-method": { "isDefaultRoute": true, "responses": {}, From 680c7fe699419d1c671c25fdc6ba83ae2db82cda Mon Sep 17 00:00:00 2001 From: Gavin Zhang Date: Wed, 14 Dec 2022 14:38:07 -0800 Subject: [PATCH 2/5] Added integration test to make sure we can now deploy --- .../test_http_api_with_fail_on_warnings.py | 33 +++++++++++++++++++ ...il_on_warnings_and_default_stage_name.json | 22 +++++++++++++ ...il_on_warnings_and_default_stage_name.yaml | 29 ++++++++++++++++ 3 files changed, 84 insertions(+) create mode 100644 integration/combination/test_http_api_with_fail_on_warnings.py create mode 100644 integration/resources/expected/combination/http_api_with_fail_on_warnings_and_default_stage_name.json create mode 100644 integration/resources/templates/combination/http_api_with_fail_on_warnings_and_default_stage_name.yaml diff --git a/integration/combination/test_http_api_with_fail_on_warnings.py b/integration/combination/test_http_api_with_fail_on_warnings.py new file mode 100644 index 0000000000..f93e1272a6 --- /dev/null +++ b/integration/combination/test_http_api_with_fail_on_warnings.py @@ -0,0 +1,33 @@ +from unittest.case import skipIf +from parameterized import parameterized + +from integration.helpers.base_test import BaseTest +from integration.helpers.resource import current_region_does_not_support +from integration.config.service_names import HTTP_API + + +@skipIf(current_region_does_not_support([HTTP_API]), "HttpApi is not supported in this testing region") +class TestHttpApiWithFailOnWarnings(BaseTest): + @parameterized.expand( + [ + ("combination/http_api_with_fail_on_warnings_and_default_stage_name", True), + ("combination/http_api_with_fail_on_warnings_and_default_stage_name", False) + ] + ) + def test_http_api_with_fail_on_warnings(self, file_name, disable_value): + parameters = [ + { + "ParameterKey": "FailOnWarningsValue", + "ParameterValue": "true" if disable_value else "false", + "UsePreviousValue": False, + "ResolvedValue": "string", + } + ] + + self.create_and_verify_stack(file_name, parameters) + + http_api_id = self.get_physical_id_by_type("AWS::ApiGatewayV2::Api") + apigw_v2_client = self.client_provider.api_v2_client + + api_result = apigw_v2_client.get_api(ApiId=http_api_id) + self.assertEqual(api_result["ResponseMetadata"]["HTTPStatusCode"], 200) diff --git a/integration/resources/expected/combination/http_api_with_fail_on_warnings_and_default_stage_name.json b/integration/resources/expected/combination/http_api_with_fail_on_warnings_and_default_stage_name.json new file mode 100644 index 0000000000..fc4622babd --- /dev/null +++ b/integration/resources/expected/combination/http_api_with_fail_on_warnings_and_default_stage_name.json @@ -0,0 +1,22 @@ +[ + { + "LogicalResourceId": "AppFunctionAppHandlerPermission", + "ResourceType": "AWS::Lambda::Permission" + }, + { + "LogicalResourceId": "AppApi", + "ResourceType": "AWS::ApiGatewayV2::Api" + }, + { + "LogicalResourceId": "AppApiApiGatewayDefaultStage", + "ResourceType": "AWS::ApiGatewayV2::Stage" + }, + { + "LogicalResourceId": "AppFunction", + "ResourceType": "AWS::Lambda::Function" + }, + { + "LogicalResourceId": "AppFunctionRole", + "ResourceType": "AWS::IAM::Role" + } +] diff --git a/integration/resources/templates/combination/http_api_with_fail_on_warnings_and_default_stage_name.yaml b/integration/resources/templates/combination/http_api_with_fail_on_warnings_and_default_stage_name.yaml new file mode 100644 index 0000000000..f3e88c5a78 --- /dev/null +++ b/integration/resources/templates/combination/http_api_with_fail_on_warnings_and_default_stage_name.yaml @@ -0,0 +1,29 @@ +Parameters: + FailOnWarningsValue: + Type: String + AllowedValues: [true, false] + +Resources: + AppApi: + Type: AWS::Serverless::HttpApi + Properties: + FailOnWarnings: !Ref FailOnWarningsValue + StageName: $default + AppFunction: + Type: AWS::Serverless::Function + Properties: + Handler: index.handler + InlineCode: | + def handler(event, context): + print("Hello, world!") + Runtime: python3.8 + Architectures: + - x86_64 + Events: + AppHandler: + Type: HttpApi + Properties: + ApiId: !Ref AppApi + +Metadata: + SamTransformTest: true From ca14c4cd5859506d72fe48ee5368e8b23d3f2a32 Mon Sep 17 00:00:00 2001 From: Gavin Zhang Date: Wed, 14 Dec 2022 14:42:07 -0800 Subject: [PATCH 3/5] Format file --- .../test_http_api_with_fail_on_warnings.py | 2 +- ..._with_fail_on_warnings_and_default_stage_name.yaml | 2 +- samtranslator/model/api/http_api_generator.py | 11 +++++------ 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/integration/combination/test_http_api_with_fail_on_warnings.py b/integration/combination/test_http_api_with_fail_on_warnings.py index f93e1272a6..7c82224e0c 100644 --- a/integration/combination/test_http_api_with_fail_on_warnings.py +++ b/integration/combination/test_http_api_with_fail_on_warnings.py @@ -11,7 +11,7 @@ class TestHttpApiWithFailOnWarnings(BaseTest): @parameterized.expand( [ ("combination/http_api_with_fail_on_warnings_and_default_stage_name", True), - ("combination/http_api_with_fail_on_warnings_and_default_stage_name", False) + ("combination/http_api_with_fail_on_warnings_and_default_stage_name", False), ] ) def test_http_api_with_fail_on_warnings(self, file_name, disable_value): diff --git a/integration/resources/templates/combination/http_api_with_fail_on_warnings_and_default_stage_name.yaml b/integration/resources/templates/combination/http_api_with_fail_on_warnings_and_default_stage_name.yaml index f3e88c5a78..a9b1800a0a 100644 --- a/integration/resources/templates/combination/http_api_with_fail_on_warnings_and_default_stage_name.yaml +++ b/integration/resources/templates/combination/http_api_with_fail_on_warnings_and_default_stage_name.yaml @@ -18,7 +18,7 @@ Resources: print("Hello, world!") Runtime: python3.8 Architectures: - - x86_64 + - x86_64 Events: AppHandler: Type: HttpApi diff --git a/samtranslator/model/api/http_api_generator.py b/samtranslator/model/api/http_api_generator.py index adb81558b2..20ee4a4317 100644 --- a/samtranslator/model/api/http_api_generator.py +++ b/samtranslator/model/api/http_api_generator.py @@ -225,19 +225,18 @@ def _add_cors(self) -> None: # Assign the OpenApi back to template self.definition_body = editor.openapi - def _update_paths(self): - if not self.fail_on_warnings: + def _update_paths(self) -> None: + if not self.fail_on_warnings or not self.definition_body: return # Using default stage name generate warning during deployment - # Warnings found during import: Parse issue: attribute paths. - # Resource $default should start with / (Service: AmazonApiGatewayV2; Status Code: 400; + # Warnings found during import: Parse issue: attribute paths. + # Resource $default should start with / (Service: AmazonApiGatewayV2; Status Code: 400; # Deployment fails when FailOnWarnings is true - paths: Optional[Dict[str, Any]] = self.definition_body.get("paths") + paths: Dict[str, Any] = self.definition_body.get("paths", {}) if DefaultStageName in paths: paths[f"/{DefaultStageName}"] = paths.pop(DefaultStageName) - def _construct_api_domain( self, http_api: ApiGatewayV2HttpApi, route53_record_set_groups: Dict[str, Route53RecordSetGroup] ) -> Tuple[ From a343ad04a611ea910725127f64ec83b62129c5e0 Mon Sep 17 00:00:00 2001 From: Gavin Zhang Date: Wed, 14 Dec 2022 14:50:48 -0800 Subject: [PATCH 4/5] Update naming --- samtranslator/model/api/http_api_generator.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/samtranslator/model/api/http_api_generator.py b/samtranslator/model/api/http_api_generator.py index 20ee4a4317..025f31a20c 100644 --- a/samtranslator/model/api/http_api_generator.py +++ b/samtranslator/model/api/http_api_generator.py @@ -120,7 +120,7 @@ def _construct_http_api(self) -> ApiGatewayV2HttpApi: self._add_title() self._add_description() - self._update_paths() + self._update_default_path() if self.definition_uri: http_api.BodyS3Location = self._construct_body_s3_dict(self.definition_uri) @@ -225,14 +225,14 @@ def _add_cors(self) -> None: # Assign the OpenApi back to template self.definition_body = editor.openapi - def _update_paths(self) -> None: + def _update_default_path(self) -> None: if not self.fail_on_warnings or not self.definition_body: return # Using default stage name generate warning during deployment # Warnings found during import: Parse issue: attribute paths. # Resource $default should start with / (Service: AmazonApiGatewayV2; Status Code: 400; - # Deployment fails when FailOnWarnings is true + # Deployment fails when FailOnWarnings is true: https://github.com/aws/serverless-application-model/issues/2297 paths: Dict[str, Any] = self.definition_body.get("paths", {}) if DefaultStageName in paths: paths[f"/{DefaultStageName}"] = paths.pop(DefaultStageName) From ef3bbe766f0a4661c772d84c7f6ffccf85aede39 Mon Sep 17 00:00:00 2001 From: Gavin Zhang Date: Wed, 14 Dec 2022 15:16:07 -0800 Subject: [PATCH 5/5] Added comment explaining backward compatibility --- samtranslator/model/api/http_api_generator.py | 1 + 1 file changed, 1 insertion(+) diff --git a/samtranslator/model/api/http_api_generator.py b/samtranslator/model/api/http_api_generator.py index 025f31a20c..e332e40d03 100644 --- a/samtranslator/model/api/http_api_generator.py +++ b/samtranslator/model/api/http_api_generator.py @@ -226,6 +226,7 @@ def _add_cors(self) -> None: self.definition_body = editor.openapi def _update_default_path(self) -> None: + # Only do the following if FailOnWarnings is enabled for backward compatibility. if not self.fail_on_warnings or not self.definition_body: return