From b4661a888392abc35db7cc52a7c75b181f8b94c2 Mon Sep 17 00:00:00 2001 From: Gavin Zhang Date: Tue, 22 Nov 2022 13:27:01 -0800 Subject: [PATCH 1/6] Run schema validation as unit test --- .gitignore | 2 - Makefile | 6 -- tests/schema/__init__.py | 0 tests/schema/test_validate_schema.py | 95 +++++++++++++++++++ .../input/error_schema_validation.yaml | 25 +++++ .../output/error_schema_validation.json | 8 ++ 6 files changed, 128 insertions(+), 8 deletions(-) create mode 100644 tests/schema/__init__.py create mode 100644 tests/schema/test_validate_schema.py create mode 100644 tests/translator/input/error_schema_validation.yaml create mode 100644 tests/translator/output/error_schema_validation.json diff --git a/.gitignore b/.gitignore index c284b99f5e..a4f20f387b 100644 --- a/.gitignore +++ b/.gitignore @@ -117,5 +117,3 @@ venv.bak/ # Companion stack config integration/config/file_to_s3_map_modified.json - -.tmp_schema.json diff --git a/Makefile b/Makefile index a074f65ba9..8d21cb0a8d 100755 --- a/Makefile +++ b/Makefile @@ -24,10 +24,6 @@ black: bin/yaml-format.py --write integration --add-test-metadata black-check: - # Checking latest schema was generated (run `make schema` if this fails) - python samtranslator/schema/schema.py > .tmp_schema.json - diff -u samtranslator/schema/schema.json .tmp_schema.json - rm .tmp_schema.json black --check setup.py samtranslator/* tests/* integration/* bin/*.py bin/json-format.py --check tests integration bin/yaml-format.py --check tests @@ -38,8 +34,6 @@ lint: mypy --strict samtranslator bin # Linter performs static analysis to catch latent bugs pylint --rcfile .pylintrc samtranslator - # Ensure templates adhere to JSON schema - bin/validate_schema.py prepare-companion-stack: pytest -v --no-cov integration/setup -m setup diff --git a/tests/schema/__init__.py b/tests/schema/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/schema/test_validate_schema.py b/tests/schema/test_validate_schema.py new file mode 100644 index 0000000000..c95b0eafe2 --- /dev/null +++ b/tests/schema/test_validate_schema.py @@ -0,0 +1,95 @@ +import json +import pytest +import os +import itertools + +from pathlib import Path +from typing import Iterator +from unittest import TestCase +from cfn_flip import to_json # type: ignore +from jsonschema import validate +from jsonschema.exceptions import ValidationError +from parameterized import parameterized + + +SCHEMA = json.loads(Path("samtranslator/schema/schema.json").read_bytes()) + +# TODO: Enable (most likely) everything but 'error_*' and 'basic_schema_validation_failure' +SKIPPED_TESTS = [ + "error_", + "basic_schema_validation_failure", # Designed to fail schema validation + "unsupported_resources", + "resource_with_invalid_type", + "state_machine_with_null_events", + "state_machine_with_cwe", # Doesn't match schema at all... + "function_with_null_events", + "function_with_event_bridge_rule_state", # Doesn't match schema at all... + "sns_existing_sqs", # 8 is not of type string + "eventbridgerule_with_dlq", # Doesn't match schema at all... + "sns_outside_sqs", # 8 is not of type string + "function_with_cwe_dlq_and_retry_policy", # Doesn't match schema at all... + "function_with_cwe_dlq_generated", # Doesn't match schema at all... + "function_with_request_parameters", # RequestParameters don't match documentation. Documentation and its example don't match either + "api_with_request_parameters_openapi", # RequestParameters don't match documentation. Documentation and its example don't match either + "api_with_aws_iam_auth_overrides", # null for invokeRole + "eventbridgerule", # missing required field 'Patterns' + "self_managed_kafka_with_intrinsics", # 'EnableValue' is of type bool but defined as string + "api_with_resource_policy_global", # 'ResourcePolicy CustomStatements' output expects a List + "api_with_resource_policy", # 'ResourcePolicy CustomStatements' output expects a List + "api_with_if_conditional_with_resource_policy", # 'ResourcePolicy CustomStatements' output expects a List + "api_rest_paths_with_if_condition_swagger", # 'EnableSimpleResponses' and 'AuthorizerPayloadFormatVersion' not defined in documentation + "api_rest_paths_with_if_condition_openapi", # 'EnableSimpleResponses' and 'AuthorizerPayloadFormatVersion' not defined in documentation + "state_machine_with_api_authorizer_maximum", # 'UserPoolArn' expects to be a string, but received list + "api_with_auth_all_maximum", # 'UserPoolArn' expects to be a string, but received list + "api_with_auth_and_conditions_all_max", # 'UserPoolArn' expects to be a string, but received list + "api_with_auth_all_maximum_openapi_3", # 'UserPoolArn' expects to be a string, but received list + "api_with_authorizers_max_openapi", # 'UserPoolArn' expects to be a string, but received list + "api_with_authorizers_max", # 'UserPoolArn' expects to be a string, but received list + "api_with_any_method_in_swagger", # Missing required field 'FunctionArn' + "api_with_cors_and_only_headers", # 'AllowOrigins' is required field + "api_with_cors_and_only_methods", # 'AllowOrigins' is required field + "implicit_api_with_auth_and_conditions_max", # 'UserPoolArn' expects to be a string, but received list +] + +def should_skip_test(s: str) -> bool: + for test in SKIPPED_TESTS: + if test in s: + return True + return False + + +def get_all_test_templates(): + return ( + list(Path("tests/translator/input").glob("**/*.yaml")) + + list(Path("tests/translator/input").glob("**/*.yml")) + + list(Path("tests/validator/input").glob("**/*.yaml")) + + list(Path("tests/validator/input").glob("**/*.yml")) + + list(Path("integration/resources/templates").glob("**/*.yaml")) + + list(Path("integration/resources/templates").glob("**/*.yml")) + ) + + +SCHEMA_VALIDATION_TESTS = [ + os.path.splitext(f)[0] + for f in get_all_test_templates() + if not should_skip_test(str(f)) +] + + +class TestValidateSchema(TestCase): + @parameterized.expand( + itertools.product( + SCHEMA_VALIDATION_TESTS + ) + ) + def test_validate_schema(self, testcase): + file_name = testcase + ".yaml" + obj = json.loads(to_json(Path(file_name).read_bytes())) + validate(obj, schema=SCHEMA) + + + def test_validate_schema_error(self): + failure_file_name = "tests/translator/input/error_schema_validation.yaml" + obj = json.loads(to_json(Path(failure_file_name).read_bytes())) + with pytest.raises(ValidationError): + validate(obj, schema=SCHEMA) \ No newline at end of file diff --git a/tests/translator/input/error_schema_validation.yaml b/tests/translator/input/error_schema_validation.yaml new file mode 100644 index 0000000000..8733bdbd3a --- /dev/null +++ b/tests/translator/input/error_schema_validation.yaml @@ -0,0 +1,25 @@ +Resources: + MyFunction: + Type: AWS::Serverless::Function + Properties: + Runtime: nodejs14.x + Handler: index.handler + InlineCode: | + const AWS = require('aws-sdk'); + exports.handler = async (event) => { + console.log(JSON.stringify(event)); + }; + + MyBucket: + Type: AWS::S3::Bucket + + MyConnector: + Type: AWS::Serverless::Connector + Properties: + Source: + Id: MyFunction + Desitation: # wrong spelling + Id: MyBucket + Permissions: + - Read + - Write diff --git a/tests/translator/output/error_schema_validation.json b/tests/translator/output/error_schema_validation.json new file mode 100644 index 0000000000..59f8291785 --- /dev/null +++ b/tests/translator/output/error_schema_validation.json @@ -0,0 +1,8 @@ +{ + "errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 1. Resource with id [MyConnector] is invalid. property Desitation not defined for resource of type AWS::Serverless::Connector", + "errors": [ + { + "errorMessage": "Resource with id [MyConnector] is invalid. property Desitation not defined for resource of type AWS::Serverless::Connector" + } + ] +} From ebf282da8ed4c42647c839e959cd8d0ea30bab59 Mon Sep 17 00:00:00 2001 From: Gavin Zhang Date: Tue, 22 Nov 2022 13:28:34 -0800 Subject: [PATCH 2/6] Format file --- .gitignore | 2 ++ Makefile | 4 ++++ tests/schema/test_validate_schema.py | 17 ++++------------- 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/.gitignore b/.gitignore index a4f20f387b..c284b99f5e 100644 --- a/.gitignore +++ b/.gitignore @@ -117,3 +117,5 @@ venv.bak/ # Companion stack config integration/config/file_to_s3_map_modified.json + +.tmp_schema.json diff --git a/Makefile b/Makefile index 8d21cb0a8d..2b2ef25bb4 100755 --- a/Makefile +++ b/Makefile @@ -24,6 +24,10 @@ black: bin/yaml-format.py --write integration --add-test-metadata black-check: + # Checking latest schema was generated (run `make schema` if this fails) + python samtranslator/schema/schema.py > .tmp_schema.json + diff -u samtranslator/schema/schema.json .tmp_schema.json + rm .tmp_schema.json black --check setup.py samtranslator/* tests/* integration/* bin/*.py bin/json-format.py --check tests integration bin/yaml-format.py --check tests diff --git a/tests/schema/test_validate_schema.py b/tests/schema/test_validate_schema.py index c95b0eafe2..167a3dd596 100644 --- a/tests/schema/test_validate_schema.py +++ b/tests/schema/test_validate_schema.py @@ -17,7 +17,6 @@ # TODO: Enable (most likely) everything but 'error_*' and 'basic_schema_validation_failure' SKIPPED_TESTS = [ "error_", - "basic_schema_validation_failure", # Designed to fail schema validation "unsupported_resources", "resource_with_invalid_type", "state_machine_with_null_events", @@ -51,6 +50,7 @@ "implicit_api_with_auth_and_conditions_max", # 'UserPoolArn' expects to be a string, but received list ] + def should_skip_test(s: str) -> bool: for test in SKIPPED_TESTS: if test in s: @@ -69,27 +69,18 @@ def get_all_test_templates(): ) -SCHEMA_VALIDATION_TESTS = [ - os.path.splitext(f)[0] - for f in get_all_test_templates() - if not should_skip_test(str(f)) -] +SCHEMA_VALIDATION_TESTS = [os.path.splitext(f)[0] for f in get_all_test_templates() if not should_skip_test(str(f))] class TestValidateSchema(TestCase): - @parameterized.expand( - itertools.product( - SCHEMA_VALIDATION_TESTS - ) - ) + @parameterized.expand(itertools.product(SCHEMA_VALIDATION_TESTS)) def test_validate_schema(self, testcase): file_name = testcase + ".yaml" obj = json.loads(to_json(Path(file_name).read_bytes())) validate(obj, schema=SCHEMA) - def test_validate_schema_error(self): failure_file_name = "tests/translator/input/error_schema_validation.yaml" obj = json.loads(to_json(Path(failure_file_name).read_bytes())) with pytest.raises(ValidationError): - validate(obj, schema=SCHEMA) \ No newline at end of file + validate(obj, schema=SCHEMA) From 4b83882dbf5e6ed2f718300afc958c279386b6d9 Mon Sep 17 00:00:00 2001 From: Gavin Zhang Date: Tue, 22 Nov 2022 14:21:56 -0800 Subject: [PATCH 3/6] Address Feedback --- bin/validate_schema.py | 77 ------------------- tests/schema/test_validate_schema.py | 10 ++- ...ror_schema_validation_wrong_property.yaml} | 0 .../error_schema_validation_wrong_type.yaml | 24 ++++++ ...ror_schema_validation_wrong_property.json} | 0 .../error_schema_validation_wrong_type.json | 8 ++ 6 files changed, 39 insertions(+), 80 deletions(-) delete mode 100755 bin/validate_schema.py rename tests/translator/input/{error_schema_validation.yaml => error_schema_validation_wrong_property.yaml} (100%) create mode 100644 tests/translator/input/error_schema_validation_wrong_type.yaml rename tests/translator/output/{error_schema_validation.json => error_schema_validation_wrong_property.json} (100%) create mode 100644 tests/translator/output/error_schema_validation_wrong_type.json diff --git a/bin/validate_schema.py b/bin/validate_schema.py deleted file mode 100755 index 1a31384f03..0000000000 --- a/bin/validate_schema.py +++ /dev/null @@ -1,77 +0,0 @@ -#!/usr/bin/env python - -import json -from pathlib import Path -from typing import Iterator - -from cfn_flip import to_json # type: ignore -from jsonschema import validate - -SCHEMA = json.loads(Path("samtranslator/schema/schema.json").read_bytes()) - - -def get_templates() -> Iterator[Path]: - paths = ( - list(Path("tests/translator/input").glob("**/*.yaml")) - + list(Path("tests/translator/input").glob("**/*.yml")) - + list(Path("tests/validator/input").glob("**/*.yaml")) - + list(Path("tests/validator/input").glob("**/*.yml")) - + list(Path("integration/resources/templates").glob("**/*.yaml")) - + list(Path("integration/resources/templates").glob("**/*.yml")) - ) - # TODO: Enable (most likely) everything but error_ - skips = [ - "error_", - "unsupported_resources", - "resource_with_invalid_type", - "state_machine_with_null_events", - "state_machine_with_cwe", # Doesn't match schema at all... - "function_with_null_events", - "function_with_event_bridge_rule_state", # Doesn't match schema at all... - "sns_existing_sqs", # 8 is not of type string - "eventbridgerule_with_dlq", # Doesn't match schema at all... - "sns_outside_sqs", # 8 is not of type string - "function_with_cwe_dlq_and_retry_policy", # Doesn't match schema at all... - "function_with_cwe_dlq_generated", # Doesn't match schema at all... - "function_with_request_parameters", # RequestParameters don't match documentation. Documentation and its example don't match either - "api_with_request_parameters_openapi", # RequestParameters don't match documentation. Documentation and its example don't match either - "api_with_aws_iam_auth_overrides", # null for invokeRole - "eventbridgerule", # missing required field 'Patterns' - "self_managed_kafka_with_intrinsics", # 'EnableValue' is of type bool but defined as string - "api_with_resource_policy_global", # 'ResourcePolicy CustomStatements' output expects a List - "api_with_resource_policy", # 'ResourcePolicy CustomStatements' output expects a List - "api_with_if_conditional_with_resource_policy", # 'ResourcePolicy CustomStatements' output expects a List - "api_rest_paths_with_if_condition_swagger", # 'EnableSimpleResponses' and 'AuthorizerPayloadFormatVersion' not defined in documentation - "api_rest_paths_with_if_condition_openapi", # 'EnableSimpleResponses' and 'AuthorizerPayloadFormatVersion' not defined in documentation - "state_machine_with_api_authorizer_maximum", # 'UserPoolArn' expects to be a string, but received list - "api_with_auth_all_maximum", # 'UserPoolArn' expects to be a string, but received list - "api_with_auth_and_conditions_all_max", # 'UserPoolArn' expects to be a string, but received list - "api_with_auth_all_maximum_openapi_3", # 'UserPoolArn' expects to be a string, but received list - "api_with_authorizers_max_openapi", # 'UserPoolArn' expects to be a string, but received list - "api_with_authorizers_max", # 'UserPoolArn' expects to be a string, but received list - "api_with_any_method_in_swagger", # Missing required field 'FunctionArn' - "api_with_cors_and_only_headers", # 'AllowOrigins' is required field - "api_with_cors_and_only_methods", # 'AllowOrigins' is required field - "implicit_api_with_auth_and_conditions_max", # 'UserPoolArn' expects to be a string, but received list - ] - - def should_skip(s: str) -> bool: - for skip in skips: - if skip in s: - return True - return False - - for path in paths: - if not should_skip(str(path)): - yield path - - -def main() -> None: - for path in get_templates(): - print(f"Checking {path}") - obj = json.loads(to_json(path.read_bytes())) - validate(obj, schema=SCHEMA) - - -if __name__ == "__main__": - main() diff --git a/tests/schema/test_validate_schema.py b/tests/schema/test_validate_schema.py index 167a3dd596..933934e6ae 100644 --- a/tests/schema/test_validate_schema.py +++ b/tests/schema/test_validate_schema.py @@ -79,8 +79,12 @@ def test_validate_schema(self, testcase): obj = json.loads(to_json(Path(file_name).read_bytes())) validate(obj, schema=SCHEMA) - def test_validate_schema_error(self): - failure_file_name = "tests/translator/input/error_schema_validation.yaml" - obj = json.loads(to_json(Path(failure_file_name).read_bytes())) + @parameterized.expand([ + "tests/translator/input/error_schema_validation_wrong_property", + "tests/translator/input/error_schema_validation_wrong_type", + ]) + def test_validate_schema_error(self, testcase): + file_name = testcase + ".yaml" + obj = json.loads(to_json(Path(file_name).read_bytes())) with pytest.raises(ValidationError): validate(obj, schema=SCHEMA) diff --git a/tests/translator/input/error_schema_validation.yaml b/tests/translator/input/error_schema_validation_wrong_property.yaml similarity index 100% rename from tests/translator/input/error_schema_validation.yaml rename to tests/translator/input/error_schema_validation_wrong_property.yaml diff --git a/tests/translator/input/error_schema_validation_wrong_type.yaml b/tests/translator/input/error_schema_validation_wrong_type.yaml new file mode 100644 index 0000000000..74d8861e0e --- /dev/null +++ b/tests/translator/input/error_schema_validation_wrong_type.yaml @@ -0,0 +1,24 @@ +Resources: + MyFunction: + Type: AWS::Serverless::Function + Properties: + Runtime: nodejs14.x + Handler: index.handler + InlineCode: | + const AWS = require('aws-sdk'); + exports.handler = async (event) => { + console.log(JSON.stringify(event)); + }; + MyBucket: + Type: AWS::S3::Bucket + + MyConnector: + Type: AWS::Serverless::Connector + Properties: + Source: + Id: MyFunction + Destination: + Id: 42 # wrong type + Permissions: + - Read + - Write \ No newline at end of file diff --git a/tests/translator/output/error_schema_validation.json b/tests/translator/output/error_schema_validation_wrong_property.json similarity index 100% rename from tests/translator/output/error_schema_validation.json rename to tests/translator/output/error_schema_validation_wrong_property.json diff --git a/tests/translator/output/error_schema_validation_wrong_type.json b/tests/translator/output/error_schema_validation_wrong_type.json new file mode 100644 index 0000000000..e1a5ce740e --- /dev/null +++ b/tests/translator/output/error_schema_validation_wrong_type.json @@ -0,0 +1,8 @@ +{ + "errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 1. Resource with id [MyConnector] is invalid. 'Id' is missing or not a string.", + "errors": [ + { + "errorMessage": "Resource with id [MyConnector] is invalid. 'Id' is missing or not a string." + } + ] +} From 5a37a1ce37274c2795e57bb2ec0210477a374ecb Mon Sep 17 00:00:00 2001 From: Gavin Zhang Date: Tue, 22 Nov 2022 14:24:42 -0800 Subject: [PATCH 4/6] Format file --- tests/schema/test_validate_schema.py | 10 ++++++---- .../input/error_schema_validation_wrong_type.yaml | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/schema/test_validate_schema.py b/tests/schema/test_validate_schema.py index 933934e6ae..66768d3b89 100644 --- a/tests/schema/test_validate_schema.py +++ b/tests/schema/test_validate_schema.py @@ -79,10 +79,12 @@ def test_validate_schema(self, testcase): obj = json.loads(to_json(Path(file_name).read_bytes())) validate(obj, schema=SCHEMA) - @parameterized.expand([ - "tests/translator/input/error_schema_validation_wrong_property", - "tests/translator/input/error_schema_validation_wrong_type", - ]) + @parameterized.expand( + [ + "tests/translator/input/error_schema_validation_wrong_property", + "tests/translator/input/error_schema_validation_wrong_type", + ] + ) def test_validate_schema_error(self, testcase): file_name = testcase + ".yaml" obj = json.loads(to_json(Path(file_name).read_bytes())) diff --git a/tests/translator/input/error_schema_validation_wrong_type.yaml b/tests/translator/input/error_schema_validation_wrong_type.yaml index 74d8861e0e..d30bf89263 100644 --- a/tests/translator/input/error_schema_validation_wrong_type.yaml +++ b/tests/translator/input/error_schema_validation_wrong_type.yaml @@ -21,4 +21,4 @@ Resources: Id: 42 # wrong type Permissions: - Read - - Write \ No newline at end of file + - Write From cca8fd81f3068af7cf2fc32f735fb339f7a90147 Mon Sep 17 00:00:00 2001 From: Gavin Zhang Date: Tue, 22 Nov 2022 14:29:07 -0800 Subject: [PATCH 5/6] Address Feedback --- tests/schema/test_validate_schema.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/schema/test_validate_schema.py b/tests/schema/test_validate_schema.py index 66768d3b89..f405dda477 100644 --- a/tests/schema/test_validate_schema.py +++ b/tests/schema/test_validate_schema.py @@ -69,24 +69,22 @@ def get_all_test_templates(): ) -SCHEMA_VALIDATION_TESTS = [os.path.splitext(f)[0] for f in get_all_test_templates() if not should_skip_test(str(f))] +SCHEMA_VALIDATION_TESTS = [str(f) for f in get_all_test_templates() if not should_skip_test(str(f))] class TestValidateSchema(TestCase): @parameterized.expand(itertools.product(SCHEMA_VALIDATION_TESTS)) def test_validate_schema(self, testcase): - file_name = testcase + ".yaml" - obj = json.loads(to_json(Path(file_name).read_bytes())) + obj = json.loads(to_json(Path(testcase).read_bytes())) validate(obj, schema=SCHEMA) @parameterized.expand( [ - "tests/translator/input/error_schema_validation_wrong_property", - "tests/translator/input/error_schema_validation_wrong_type", + "tests/translator/input/error_schema_validation_wrong_property.yaml", + "tests/translator/input/error_schema_validation_wrong_type.yaml", ] ) def test_validate_schema_error(self, testcase): - file_name = testcase + ".yaml" - obj = json.loads(to_json(Path(file_name).read_bytes())) + obj = json.loads(to_json(Path(testcase).read_bytes())) with pytest.raises(ValidationError): validate(obj, schema=SCHEMA) From 57b7713d622d87dcd35c365af4209f68c0ee15e3 Mon Sep 17 00:00:00 2001 From: Gavin Zhang Date: Tue, 22 Nov 2022 14:33:16 -0800 Subject: [PATCH 6/6] Address Feedback --- .../output/error_schema_validation_wrong_property.json | 7 +------ .../output/error_schema_validation_wrong_type.json | 7 +------ 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/tests/translator/output/error_schema_validation_wrong_property.json b/tests/translator/output/error_schema_validation_wrong_property.json index 59f8291785..dfd06f4ace 100644 --- a/tests/translator/output/error_schema_validation_wrong_property.json +++ b/tests/translator/output/error_schema_validation_wrong_property.json @@ -1,8 +1,3 @@ { - "errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 1. Resource with id [MyConnector] is invalid. property Desitation not defined for resource of type AWS::Serverless::Connector", - "errors": [ - { - "errorMessage": "Resource with id [MyConnector] is invalid. property Desitation not defined for resource of type AWS::Serverless::Connector" - } - ] + "errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 1. Resource with id [MyConnector] is invalid. property Desitation not defined for resource of type AWS::Serverless::Connector" } diff --git a/tests/translator/output/error_schema_validation_wrong_type.json b/tests/translator/output/error_schema_validation_wrong_type.json index e1a5ce740e..d2ef275b33 100644 --- a/tests/translator/output/error_schema_validation_wrong_type.json +++ b/tests/translator/output/error_schema_validation_wrong_type.json @@ -1,8 +1,3 @@ { - "errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 1. Resource with id [MyConnector] is invalid. 'Id' is missing or not a string.", - "errors": [ - { - "errorMessage": "Resource with id [MyConnector] is invalid. 'Id' is missing or not a string." - } - ] + "errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 1. Resource with id [MyConnector] is invalid. 'Id' is missing or not a string." }