From 8b24bb5b47edb65f96cb6de595a6beb234bfac93 Mon Sep 17 00:00:00 2001 From: Anshika Garg Date: Thu, 11 Feb 2021 14:22:46 -0800 Subject: [PATCH] Add property transform to contract test (#667) * Add property Transform check in contract test --- README.md | 1 + src/rpdk/core/cli.py | 11 +-- src/rpdk/core/contract/resource_client.py | 92 ++++++++++++++++++- .../core/contract/suite/handler_commons.py | 3 + .../templates/transformation.template | 5 + src/rpdk/core/init.py | 10 +- src/rpdk/core/jsonutils/flattener.py | 8 +- src/rpdk/core/jsonutils/utils.py | 7 +- src/rpdk/core/upload.py | 12 +-- tests/contract/test_resource_client.py | 68 +++++++++++++- 10 files changed, 187 insertions(+), 30 deletions(-) create mode 100644 src/rpdk/core/contract/templates/transformation.template diff --git a/README.md b/README.md index d8094d45..ad6f0769 100644 --- a/README.md +++ b/README.md @@ -15,6 +15,7 @@ This tool can be installed using [pip](https://pypi.org/project/pip/) from the P ```bash pip install cloudformation-cli cloudformation-cli-java-plugin cloudformation-cli-go-plugin cloudformation-cli-python-plugin ``` +You will need npm to run contract tests (cfn test) if your resource schema requires property transform. Please ensure you have npm installed. Follow the instructions: https://www.npmjs.com/get-npm to get npm installed on your machine. ### Command: init diff --git a/src/rpdk/core/cli.py b/src/rpdk/core/cli.py index 930bbf08..1a423c74 100644 --- a/src/rpdk/core/cli.py +++ b/src/rpdk/core/cli.py @@ -106,8 +106,7 @@ def no_command(args): # some cases where it makes sense for the commands to raise SystemExit.) log.debug("Caught exit recommendation", exc_info=e) log.critical(str(e)) - # pylint: disable=W0707 - raise SystemExit(1) + raise SystemExit(1) from e except DownstreamError as e: # For these operations, we don't want to swallow the exception log.debug("Caught downstream error", exc_info=e) @@ -127,9 +126,8 @@ def no_command(args): "https://github.com/aws-cloudformation/aws-cloudformation-rpdk/issues", file=sys.stderr, ) - # pylint: disable=W0707 - raise SystemExit(2) - except Exception: # pylint: disable=broad-except + raise SystemExit(2) from e + except Exception as e: # pylint: disable=broad-except print("=== Unhandled exception ===", file=sys.stderr) print("Please report this issue to the team.", file=sys.stderr) print( @@ -146,5 +144,4 @@ def no_command(args): import traceback # pylint: disable=import-outside-toplevel traceback.print_exc() - # pylint: disable=W0707 - raise SystemExit(EXIT_UNHANDLED_EXCEPTION) + raise SystemExit(EXIT_UNHANDLED_EXCEPTION) from e diff --git a/src/rpdk/core/contract/resource_client.py b/src/rpdk/core/contract/resource_client.py index 51ba3291..e76eb631 100644 --- a/src/rpdk/core/contract/resource_client.py +++ b/src/rpdk/core/contract/resource_client.py @@ -1,8 +1,12 @@ # pylint: disable=import-outside-toplevel # pylint: disable=R0904 +# have to skip B404, import_subprocess is required for executing typescript +# have to skip B60*, to allow typescript code to be executed using subprocess import json import logging import re +import subprocess # nosec +import tempfile import time from time import sleep from uuid import uuid4 @@ -10,6 +14,7 @@ import docker from botocore import UNSIGNED from botocore.config import Config +from jinja2 import Environment, PackageLoader, select_autoescape from rpdk.core.contract.interface import Action, HandlerErrorCode, OperationStatus from rpdk.core.exceptions import InvalidProjectError @@ -158,6 +163,13 @@ def _properties_to_paths(self, key): def _update_schema(self, schema): # TODO: resolve $ref + self.env = Environment( + trim_blocks=True, + lstrip_blocks=True, + keep_trailing_newline=True, + loader=PackageLoader(__name__, "templates/"), + autoescape=select_autoescape(["html", "htm", "xml", "md"]), + ) self._schema = schema self._strategy = None self._update_strategy = None @@ -168,12 +180,14 @@ def _update_schema(self, schema): self.write_only_paths = self._properties_to_paths("writeOnlyProperties") self.create_only_paths = self._properties_to_paths("createOnlyProperties") self.properties_without_insertion_order = self.get_metadata() - + self.property_transform_keys = self._properties_to_paths("propertyTransform") + self.property_transform = self._schema.get("propertyTransform") additional_identifiers = self._schema.get("additionalIdentifiers", []) self._additional_identifiers_paths = [ {fragment_decode(prop, prefix="") for prop in identifier} for identifier in additional_identifiers ] + self.transformation_template = self.env.get_template("transformation.template") def has_only_writable_identifiers(self): return all( @@ -201,6 +215,82 @@ def get_metadata(self): and properties[prop]["insertionOrder"] == "false" } + def transform_model(self, input_model, output_model): + if self.property_transform_keys: + self.check_npm() + for prop in self.property_transform_keys: + document_input, _path_input, _parent_input = traverse( + input_model, list(prop)[1:] + ) + document_output, _path_output, _parent_output = traverse( + output_model, list(prop)[1:] + ) + if document_input != document_output: + transformed_property = self.transform(prop, input_model) + self.update_transformed_property( + prop, transformed_property, input_model + ) + + def transform(self, property_path, input_model): + + path = "/" + "/".join(property_path) + property_transform_value = json.dumps(self.property_transform[path]) + # self.property_transform[path].replace('"', '\\"') + + LOG.warning("This is the transform %s", property_transform_value) + content = self.transformation_template.render( + input_model=input_model, jsonata_expression=property_transform_value + ) + LOG.warning("This is the content %s", content) + file = tempfile.NamedTemporaryFile( + mode="w+b", + buffering=-1, + encoding=None, + newline=None, + suffix=".js", + prefix=None, + dir=".", + delete=True, + ) + file.write(str.encode(content)) + + LOG.debug("Jsonata transformation content %s", file.read().decode()) + jsonata_output = subprocess.getoutput("node " + file.name) + + file.close() + return jsonata_output + + # removing coverage for this method as it is not possible to check both npm + # exists and does not exist condition in unit test + @staticmethod + def check_npm(): # pragma: no cover + output = subprocess.getoutput("npm list jsonata") + if "npm: command not found" not in output: + if "jsonata@" not in output: + subprocess.getoutput("npm install jsonata") + else: + err_msg = ( + "NPM is required to support propertyTransform. " + "Please install npm using the following link: https://www.npmjs.com/get-npm" + ) + LOG.error(err_msg) + raise AssertionError(err_msg) + + @staticmethod + def update_transformed_property(property_path, transformed_property, input_model): + try: + _prop, resolved_path, parent = traverse( + input_model, list(property_path)[1:] + ) + except LookupError: + LOG.debug( + "Override failed.\nPath %s\nDocument %s", property_path, input_model + ) + LOG.warning("Override with path %s not found, skipping", property_path) + else: + key = resolved_path[-1] + parent[key] = transformed_property + @property def strategy(self): # an empty strategy (i.e. false-y) is valid diff --git a/src/rpdk/core/contract/suite/handler_commons.py b/src/rpdk/core/contract/suite/handler_commons.py index 94ffa085..162e2b5b 100644 --- a/src/rpdk/core/contract/suite/handler_commons.py +++ b/src/rpdk/core/contract/suite/handler_commons.py @@ -170,6 +170,9 @@ def test_input_equals_output(resource_client, input_model, output_model): # only comparing properties in input model to those in output model and # ignoring extraneous properties that maybe present in output model. try: + resource_client.transform_model( + pruned_input_model, pruned_output_model, resource_client + ) for key in pruned_input_model: if key in resource_client.properties_without_insertion_order: assert test_unordered_list_match( diff --git a/src/rpdk/core/contract/templates/transformation.template b/src/rpdk/core/contract/templates/transformation.template new file mode 100644 index 00000000..586b3038 --- /dev/null +++ b/src/rpdk/core/contract/templates/transformation.template @@ -0,0 +1,5 @@ +var jsonata = require('jsonata') +var model = {{input_model}} +var expression = jsonata({{jsonata_expression}}); +var result = expression.evaluate(model); +console.log(result); diff --git a/src/rpdk/core/init.py b/src/rpdk/core/init.py index 7742df50..c50933ff 100644 --- a/src/rpdk/core/init.py +++ b/src/rpdk/core/init.py @@ -71,9 +71,8 @@ def __init__(self, choices): def __call__(self, value): try: choice = int(value) - except ValueError: - # pylint: disable=W0707 - raise WizardValidationError("Please enter an integer") + except ValueError as value_error: + raise WizardValidationError("Please enter an integer") from value_error choice -= 1 if choice < 0 or choice >= self.max: raise WizardValidationError("Please select a choice") @@ -149,10 +148,9 @@ def ignore_abort(function): def wrapper(args): try: function(args) - except (KeyboardInterrupt, WizardAbortError): + except (KeyboardInterrupt, WizardAbortError) as exception: print("\naborted") - # pylint: disable=W0707 - raise SystemExit(1) + raise SystemExit(1) from exception return wrapper diff --git a/src/rpdk/core/jsonutils/flattener.py b/src/rpdk/core/jsonutils/flattener.py index 55963fe3..cfdac7df 100644 --- a/src/rpdk/core/jsonutils/flattener.py +++ b/src/rpdk/core/jsonutils/flattener.py @@ -79,10 +79,9 @@ def _flatten_ref_type(self, ref_path): try: ref_parts = fragment_decode(ref_path) except ValueError as e: - # pylint: disable=W0707 raise FlatteningError( "Invalid ref at path '{}': {}".format(ref_path, str(e)) - ) + ) from e ref_schema, ref_parts, _ref_parent = self._find_subschema_by_ref(ref_parts) return self._walk(ref_schema, ref_parts) @@ -186,6 +185,5 @@ def _find_subschema_by_ref(self, ref_path): """ try: return traverse(self._full_schema, ref_path) - except (LookupError, ValueError): - # pylint: disable=W0707 - raise FlatteningError("Invalid ref: {}".format(ref_path)) + except (LookupError, ValueError) as e: + raise FlatteningError("Invalid ref: {}".format(ref_path)) from e diff --git a/src/rpdk/core/jsonutils/utils.py b/src/rpdk/core/jsonutils/utils.py index 3b2acca9..56cdf41d 100644 --- a/src/rpdk/core/jsonutils/utils.py +++ b/src/rpdk/core/jsonutils/utils.py @@ -189,7 +189,7 @@ def schema_merge(target, src, path): # noqa: C901 # pylint: disable=R0912 next_path = path + (key,) try: target[key] = schema_merge(target_schema, src_schema, next_path) - except TypeError: + except TypeError as type_error: if key in (TYPE, REF): # combining multiple $ref and types src_set = to_set(src_schema) @@ -222,7 +222,8 @@ def schema_merge(target, src, path): # noqa: C901 # pylint: disable=R0912 "Object at path '{path}' declared multiple values " "for '{}': found '{}' and '{}'" ) - # pylint: disable=W0707 - raise ConstraintError(msg, path, key, target_schema, src_schema) + raise ConstraintError( + msg, path, key, target_schema, src_schema + ) from type_error target[key] = src_schema return target diff --git a/src/rpdk/core/upload.py b/src/rpdk/core/upload.py index ca5c44b2..e5a3433e 100644 --- a/src/rpdk/core/upload.py +++ b/src/rpdk/core/upload.py @@ -73,15 +73,16 @@ def _get_stack_output(self, stack_id, output_key): for output in outputs if output["OutputKey"] == output_key ) - except StopIteration: + except StopIteration as stop_iteration: LOG.debug( "Outputs from stack '%s' did not contain '%s':\n%s", stack_id, output_key, ", ".join(output["OutputKey"] for output in outputs), ) - # pylint: disable=W0707 - raise InternalError("Required output not found on stack") + raise InternalError( + "Required output not found on stack" + ) from stop_iteration def _create_or_update_stack(self, template, stack_name): args = {"StackName": stack_name, "TemplateBody": template} @@ -140,15 +141,14 @@ def create_or_update_role(self, template_path, resource_type): try: with template_path.open("r", encoding="utf-8") as f: template = f.read() - except FileNotFoundError: + except FileNotFoundError as file_not_found: LOG.critical( "CloudFormation template 'resource-role.yaml' " "for execution role not found. " "Please run `generate` or " "provide an execution role via the --role-arn parameter." ) - # pylint: disable=W0707 - raise InvalidProjectError() + raise InvalidProjectError() from file_not_found stack_id = self._create_or_update_stack( template, "{}-role-stack".format(resource_type) ) diff --git a/tests/contract/test_resource_client.py b/tests/contract/test_resource_client.py index 6c06f4cb..3fb2da20 100644 --- a/tests/contract/test_resource_client.py +++ b/tests/contract/test_resource_client.py @@ -79,6 +79,24 @@ "primaryIdentifier": ["/properties/c", "/properties/d"], } +SCHEMA_WITH_TRANSFORM = { + "properties": { + "a": {"type": "string"}, + "b": {"type": "number"}, + "c": {"type": "number"}, + "d": {"type": "number"}, + }, + "readOnlyProperties": ["/properties/b"], + "createOnlyProperties": ["/properties/c"], + "primaryIdentifier": ["/properties/c"], + "writeOnlyProperties": ["/properties/d"], + "propertyTransform": {"/properties/a": '$join([a, "Test"])'}, +} + +TRANSFORM_OUTPUT = {"a": "ValueATest", "c": 1} +INPUT = {"a": "ValueA", "c": 1} +INVALID_OUTPUT = {"a": "ValueB", "c": 1} + @pytest.fixture def resource_client(): @@ -101,14 +119,18 @@ def resource_client(): mock_sesh = mock_create_sesh.return_value mock_sesh.region_name = DEFAULT_REGION client = ResourceClient( - DEFAULT_FUNCTION, endpoint, DEFAULT_REGION, {}, EMPTY_OVERRIDE + DEFAULT_FUNCTION, + endpoint, + DEFAULT_REGION, + SCHEMA_WITH_TRANSFORM, + EMPTY_OVERRIDE, ) mock_sesh.client.assert_called_once_with("lambda", endpoint_url=endpoint) mock_creds.assert_called_once_with(mock_sesh, LOWER_CAMEL_CRED_KEYS, None) mock_account.assert_called_once_with(mock_sesh, {}) assert client._function_name == DEFAULT_FUNCTION - assert client._schema == {} + assert client._schema == SCHEMA_WITH_TRANSFORM assert client._overrides == EMPTY_OVERRIDE assert client.account == ACCOUNT @@ -375,6 +397,7 @@ def test_get_metadata(resource_client): }, "readOnlyProperties": ["/properties/c"], "createOnlyProperties": ["/properties/d"], + "propertyTransform": {"/properties/a": "test"}, } resource_client._update_schema(schema) assert resource_client.get_metadata() == {"b"} @@ -1192,3 +1215,44 @@ def test_generate_update_example_with_composite_key( created_resource ) assert updated_resource == {"a": 1, "c": 2, "d": 3} + + +def test_update_transformed_property_loookup_error(resource_client): + input_model = INPUT.copy() + document = resource_client.update_transformed_property( + ("properties", "d"), "", input_model + ) + assert document is None + + +def test_transform_model_equal_input_and_output(resource_client): + input_model = INPUT.copy() + output_model = INPUT.copy() + + resource_client.transform_model(input_model, output_model) + assert input_model == INPUT + + +def test_transform_model_equal_output(resource_client): + input_model = INPUT.copy() + output_model = TRANSFORM_OUTPUT.copy() + + resource_client.transform_model(input_model, output_model) + assert input_model == TRANSFORM_OUTPUT + + +def test_transform_model_unequal_models(resource_client): + input_model = INPUT.copy() + output_model = INVALID_OUTPUT.copy() + + resource_client.transform_model(input_model, output_model) + assert input_model != output_model + assert input_model == TRANSFORM_OUTPUT + + +def test_non_transform_model_not_equal(resource_client_inputs_schema): + input_model = INPUT.copy() + output_model = INVALID_OUTPUT.copy() + + resource_client_inputs_schema.transform_model(input_model, output_model) + assert input_model != output_model