Skip to content

Commit

Permalink
Revert npm dependency (aws-cloudformation#737)
Browse files Browse the repository at this point in the history
  • Loading branch information
anshikg authored and kddejong committed Oct 24, 2022
1 parent 6b000e3 commit 44b2450
Show file tree
Hide file tree
Showing 10 changed files with 35 additions and 274 deletions.
7 changes: 0 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
11 changes: 7 additions & 4 deletions src/rpdk/core/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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(
Expand All @@ -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)
133 changes: 3 additions & 130 deletions src/rpdk/core/contract/resource_client.py
Original file line number Diff line number Diff line change
@@ -1,20 +1,15 @@
# 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

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
Expand Down Expand Up @@ -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
Expand All @@ -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(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -518,7 +391,7 @@ def make_request(
creds,
token,
callback_context=None,
**kwargs,
**kwargs
):
return {
"requestData": {
Expand Down Expand Up @@ -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):
Expand Down
5 changes: 2 additions & 3 deletions src/rpdk/core/contract/suite/handler_commons.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 0 additions & 5 deletions src/rpdk/core/contract/templates/transformation.template

This file was deleted.

10 changes: 6 additions & 4 deletions src/rpdk/core/init.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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

Expand Down
8 changes: 5 additions & 3 deletions src/rpdk/core/jsonutils/flattener.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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))
7 changes: 3 additions & 4 deletions src/rpdk/core/jsonutils/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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
12 changes: 6 additions & 6 deletions src/rpdk/core/upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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)
)
Expand Down
Loading

0 comments on commit 44b2450

Please sign in to comment.