diff --git a/README.md b/README.md index 41a04fa4f..41b1052b0 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,6 @@ 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 cloudformation-cli-typescript-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 @@ -103,12 +102,6 @@ If you're creating a resource type, you will also need to install a language plu pip install -e ../cloudformation-cli-java-plugin ``` -Linting and running unit tests is done via [pre-commit](https://pre-commit.com/), and so is performed automatically on commit. The continuous integration also runs these checks. Manual options are available, so you don't have to commit: - -These one-time steps are required to ensure all unit tests are passing: - * Ensure npm is installed by [following these steps](https://www.npmjs.com/get-npm) - * Install jsonata: `npm install jsonata` - ```bash # run all hooks on all files, mirrors what the CI runs pre-commit run --all-files diff --git a/src/rpdk/core/cli.py b/src/rpdk/core/cli.py index 3d27e178e..8a4d677cb 100644 --- a/src/rpdk/core/cli.py +++ b/src/rpdk/core/cli.py @@ -106,7 +106,8 @@ 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)) - raise SystemExit(1) from e + # pylint: disable=W0707 + raise SystemExit(1) except DownstreamError as e: # For these operations, we don't want to swallow the exception log.debug("Caught downstream error", exc_info=e) @@ -126,8 +127,9 @@ def no_command(args): "https://github.com/aws-cloudformation/aws-cloudformation-rpdk/issues", file=sys.stderr, ) - raise SystemExit(2) from e - except Exception as e: # pylint: disable=broad-except + # pylint: disable=W0707 + raise SystemExit(2) + except Exception: # pylint: disable=broad-except print("=== Unhandled exception ===", file=sys.stderr) print("Please report this issue to the team.", file=sys.stderr) print( @@ -143,4 +145,5 @@ def no_command(args): import traceback # pylint: disable=import-outside-toplevel traceback.print_exc() - raise SystemExit(EXIT_UNHANDLED_EXCEPTION) from e + # pylint: disable=W0707 + raise SystemExit(EXIT_UNHANDLED_EXCEPTION) diff --git a/src/rpdk/core/contract/resource_client.py b/src/rpdk/core/contract/resource_client.py index 8404181cb..51b1182ab 100644 --- a/src/rpdk/core/contract/resource_client.py +++ b/src/rpdk/core/contract/resource_client.py @@ -1,12 +1,8 @@ # 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 @@ -14,7 +10,6 @@ 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 @@ -181,13 +176,6 @@ 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 @@ -198,14 +186,12 @@ 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( @@ -233,119 +219,6 @@ def get_metadata(self): and properties[prop]["insertionOrder"] == "false" } - def transform_model(self, input_model, output_model): # pylint: disable=R0914 - if self.property_transform_keys: - self.check_npm() - for prop in self.property_transform_keys: # pylint: disable=R1702 - try: - 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 not self.compare(document_input, document_output): - path = "/" + "/".join(prop) - property_transform_value = self.property_transform[path].replace( - '"', '\\"' - ) - if "$OR" in property_transform_value: - transform_functions = property_transform_value.split("$OR") - for transform_function in transform_functions: - transformed_property = self.transform( - transform_function, input_model - ) - value = self.convert_type( - document_input, transformed_property - ) - if self.compare(value, document_output): - self.update_transformed_property( - prop, value, input_model - ) - else: - transformed_property = self.transform( - property_transform_value, input_model - ) - value = self.convert_type(document_input, transformed_property) - self.update_transformed_property(prop, value, input_model) - - except KeyError: - pass - - @staticmethod - def convert_type(document_input, transformed_property): - if isinstance(document_input, bool): - return bool(transformed_property) - if isinstance(document_input, int): - return int(transformed_property) - if isinstance(document_input, float): - return float(transformed_property) - return transformed_property - - @staticmethod - def compare(document_input, document_output): - try: - if isinstance(document_input, str): - return bool(re.match(f"^{document_input}$", document_output)) - return document_input == document_output - except re.error: - return document_input == document_output - - def transform(self, property_transform_value, input_model): - LOG.debug("This is the transform %s", property_transform_value) - content = self.transformation_template.render( - input_model=input_model, jsonata_expression=property_transform_value - ) - LOG.debug("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 @@ -518,7 +391,7 @@ def make_request( creds, token, callback_context=None, - **kwargs, + **kwargs ): return { "requestData": { @@ -594,7 +467,7 @@ def _make_payload(self, action, current_model, previous_model=None, **kwargs): self._session, LOWER_CAMEL_CRED_KEYS, self._role_arn ), self.generate_token(), - **kwargs, + **kwargs ) def _call(self, payload): diff --git a/src/rpdk/core/contract/suite/handler_commons.py b/src/rpdk/core/contract/suite/handler_commons.py index b2259bf7d..198bf6a79 100644 --- a/src/rpdk/core/contract/suite/handler_commons.py +++ b/src/rpdk/core/contract/suite/handler_commons.py @@ -173,15 +173,14 @@ 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) for key in pruned_input_model: if key in resource_client.properties_without_insertion_order: assert test_unordered_list_match( pruned_input_model[key], pruned_output_model[key] ) else: - assert resource_client.compare( - pruned_input_model[key], pruned_output_model[key] + assert ( + pruned_input_model[key] == pruned_output_model[key] ), assertion_error_message except KeyError as e: raise AssertionError(assertion_error_message) from e diff --git a/src/rpdk/core/contract/templates/transformation.template b/src/rpdk/core/contract/templates/transformation.template deleted file mode 100644 index 84bc1f71e..000000000 --- a/src/rpdk/core/contract/templates/transformation.template +++ /dev/null @@ -1,5 +0,0 @@ -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 c50933ff1..7742df507 100644 --- a/src/rpdk/core/init.py +++ b/src/rpdk/core/init.py @@ -71,8 +71,9 @@ def __init__(self, choices): def __call__(self, value): try: choice = int(value) - except ValueError as value_error: - raise WizardValidationError("Please enter an integer") from value_error + except ValueError: + # pylint: disable=W0707 + raise WizardValidationError("Please enter an integer") choice -= 1 if choice < 0 or choice >= self.max: raise WizardValidationError("Please select a choice") @@ -148,9 +149,10 @@ def ignore_abort(function): def wrapper(args): try: function(args) - except (KeyboardInterrupt, WizardAbortError) as exception: + except (KeyboardInterrupt, WizardAbortError): print("\naborted") - raise SystemExit(1) from exception + # pylint: disable=W0707 + raise SystemExit(1) return wrapper diff --git a/src/rpdk/core/jsonutils/flattener.py b/src/rpdk/core/jsonutils/flattener.py index cfdac7df6..55963fe36 100644 --- a/src/rpdk/core/jsonutils/flattener.py +++ b/src/rpdk/core/jsonutils/flattener.py @@ -79,9 +79,10 @@ 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) @@ -185,5 +186,6 @@ def _find_subschema_by_ref(self, ref_path): """ try: return traverse(self._full_schema, ref_path) - except (LookupError, ValueError) as e: - raise FlatteningError("Invalid ref: {}".format(ref_path)) from e + except (LookupError, ValueError): + # pylint: disable=W0707 + raise FlatteningError("Invalid ref: {}".format(ref_path)) diff --git a/src/rpdk/core/jsonutils/utils.py b/src/rpdk/core/jsonutils/utils.py index 56cdf41de..3b2acca9e 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 as type_error: + except TypeError: if key in (TYPE, REF): # combining multiple $ref and types src_set = to_set(src_schema) @@ -222,8 +222,7 @@ def schema_merge(target, src, path): # noqa: C901 # pylint: disable=R0912 "Object at path '{path}' declared multiple values " "for '{}': found '{}' and '{}'" ) - raise ConstraintError( - msg, path, key, target_schema, src_schema - ) from type_error + # pylint: disable=W0707 + raise ConstraintError(msg, path, key, target_schema, src_schema) target[key] = src_schema return target diff --git a/src/rpdk/core/upload.py b/src/rpdk/core/upload.py index e5a3433ec..ca5c44b25 100644 --- a/src/rpdk/core/upload.py +++ b/src/rpdk/core/upload.py @@ -73,16 +73,15 @@ def _get_stack_output(self, stack_id, output_key): for output in outputs if output["OutputKey"] == output_key ) - except StopIteration as stop_iteration: + except StopIteration: LOG.debug( "Outputs from stack '%s' did not contain '%s':\n%s", stack_id, output_key, ", ".join(output["OutputKey"] for output in outputs), ) - raise InternalError( - "Required output not found on stack" - ) from stop_iteration + # pylint: disable=W0707 + raise InternalError("Required output not found on stack") def _create_or_update_stack(self, template, stack_name): args = {"StackName": stack_name, "TemplateBody": template} @@ -141,14 +140,15 @@ 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 as file_not_found: + except FileNotFoundError: 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." ) - raise InvalidProjectError() from file_not_found + # pylint: disable=W0707 + raise InvalidProjectError() 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 7e67607c2..499b64355 100644 --- a/tests/contract/test_resource_client.py +++ b/tests/contract/test_resource_client.py @@ -80,30 +80,6 @@ "primaryIdentifier": ["/properties/c", "/properties/d"], } -SCHEMA_WITH_TRANSFORM = { - "properties": { - "a": {"type": "string"}, - "b": {"type": "number"}, - "c": {"type": "number"}, - "d": {"type": "number"}, - "e": {"type": "string"}, - }, - "readOnlyProperties": ["/properties/b"], - "createOnlyProperties": ["/properties/c"], - "primaryIdentifier": ["/properties/b"], - "writeOnlyProperties": ["/properties/d"], - "propertyTransform": { - "/properties/a": '$join([a, "Test"])', - "/properties/c": "$power(2, 2)", - "/properties/e": '$join([e, "Test"]) $OR $join([e, "Value"])', - }, -} - -TRANSFORM_OUTPUT = {"a": "ValueATest", "c": 4} -TRANSFORM_OUTPUT_WITHOUT_C = {"a": "ValueATest", "c": 1} -INPUT = {"a": "ValueA", "c": 1} -INVALID_OUTPUT = {"a": "ValueB", "c": 1} - @pytest.fixture def resource_client(): @@ -126,18 +102,14 @@ def resource_client(): mock_sesh = mock_create_sesh.return_value mock_sesh.region_name = DEFAULT_REGION client = ResourceClient( - DEFAULT_FUNCTION, - endpoint, - DEFAULT_REGION, - SCHEMA_WITH_TRANSFORM, - EMPTY_OVERRIDE, + DEFAULT_FUNCTION, endpoint, DEFAULT_REGION, {}, 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 == SCHEMA_WITH_TRANSFORM + assert client._schema == {} assert client._overrides == EMPTY_OVERRIDE assert client.account == ACCOUNT @@ -419,7 +391,6 @@ 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"} @@ -856,11 +827,7 @@ def test_call_async(resource_client, action): ) mock_client.invoke.side_effect = [ - { - "Payload": StringIO( - '{"status": "IN_PROGRESS", "resourceModel": {"c": 3, "b": 1} }' - ) - }, + {"Payload": StringIO('{"status": "IN_PROGRESS", "resourceModel": {"c": 3} }')}, {"Payload": StringIO('{"status": "SUCCESS"}')}, ] @@ -1243,75 +1210,3 @@ 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 == output_model - - -@pytest.mark.parametrize( - "output", - [{"e": "newValue", "a": "ValueA", "c": 1}, {"e": "newTest", "a": "ValueA", "c": 1}], -) -def test_transform_model_equal_output_OR(resource_client, output): - input_model = {"e": "new", "a": "ValueA", "c": 1} - - resource_client.transform_model(input_model, output) - assert input_model == output - - -def test_transform_model_not_equal_output_OR(resource_client): - input_model = {"e": "new", "a": "ValueA", "c": 1} - output_model = {"e": "not-newValue", "a": "ValueA", "c": 4} - - resource_client.transform_model(input_model, output_model) - assert input_model != output_model - assert input_model == {"a": "ValueA", "c": 4, "e": "new"} - - -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_WITHOUT_C - - -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 - - -def test_compare_raise_exception(resource_client): - assert not resource_client.compare("he(lo", "hello") - - -@pytest.mark.parametrize("value", [1, 2.3, True, "test"]) -def test_convert_type(resource_client, value): - converted_value = resource_client.convert_type(value, str(value)) - assert isinstance(converted_value, type(value)) - assert converted_value == value