From b8f69bb0345a033ba44516637e5d8a42d87a1608 Mon Sep 17 00:00:00 2001 From: zjinmei <81066492+zjinmei@users.noreply.github.com> Date: Fri, 6 Jan 2023 13:21:44 -0800 Subject: [PATCH] Add property transform for contract tests (#843) * Add property transform for contract tests * Install JQ as dependency * test without windows * test ubuntu system * test macos system * Test windows system * Fix pylint and test windows * add more dependencies for jq * use jq instead of pyjq and test windows * test three system to install jq * update pr-ci yaml file * Using string for system * Use pyjq for macOS and Linux * Use apt instead of yum for Ubuntu system * remove apt for Ubuntu system * Update readme with Ubuntu system to install pyjq * fix whitespace issue * Reduce function parameters from 6 to 5 and update readme * Add comments and annotations, do copy inside transform function * don't install pyjq on windows, it is not supported Co-authored-by: Michael Maeng --- .github/workflows/pr-ci.yaml | 9 ++ README.md | 26 +++++- src/rpdk/core/contract/resource_client.py | 69 ++++++++++++++- .../suite/resource/handler_commons.py | 6 +- tests/contract/test_resource_client.py | 87 +++++++++++++++++++ 5 files changed, 188 insertions(+), 9 deletions(-) diff --git a/.github/workflows/pr-ci.yaml b/.github/workflows/pr-ci.yaml index 5ebb6783..c386a770 100644 --- a/.github/workflows/pr-ci.yaml +++ b/.github/workflows/pr-ci.yaml @@ -20,6 +20,15 @@ jobs: with: python-version: ${{ matrix.python }} cache: 'pip' + - name: Install PYJQ dependencies on Macos + run: | + brew install autoconf automake libtool + brew install jq + if: matrix.os == 'macos-latest' + - name: Install PYJQ on non-Windows + run: | + pip install pyjq + if: matrix.os != 'windows-latest' - name: Install dependencies run: | pip install --upgrade 'attrs==19.2.0' wheel -r requirements.txt diff --git a/README.md b/README.md index 7bba68ab..703c35fb 100644 --- a/README.md +++ b/README.md @@ -59,7 +59,31 @@ cfn test --enforce-timeout 60 -- -k contract_delete_update # combine arguments cfn test --log-group-name cw_log_group --log-role-arn log_delivery_role_arn # Handler logs generated by contract tests will be delivered to the specified cw_log_group using the credentials from log_delivery_role_arn ``` -Note: To use your type configuration in contract tests, you will need to save your type configuration json file in `~/.cfn-cli/typeConfiguration.json`. +Note: +* To use your type configuration in contract tests, you will need to save your type configuration json file in `~/.cfn-cli/typeConfiguration.json`. + +* To use `propertyTransform` in schema, you will need to install [PYJQ](https://pypi.org/project/pyjq/). This feature will not be available to use with contract tests on Windows OS + +Install PYJQ for Linux system + +```bash +yum install autoconf automake libtool +pip install pyjq +``` + +Install PYJQ for macOS system + +```bash +brew install autoconf automake libtool +brew install jq +pip install pyjq +``` + +Install PYJQ for Ubuntu system + +```bash +pip install pyjq +``` ### Command: validate diff --git a/src/rpdk/core/contract/resource_client.py b/src/rpdk/core/contract/resource_client.py index 13399a75..40e3937d 100644 --- a/src/rpdk/core/contract/resource_client.py +++ b/src/rpdk/core/contract/resource_client.py @@ -1,10 +1,13 @@ # pylint: disable=import-outside-toplevel # pylint: disable=R0904 +import copy import json import logging import re +import sys import time from time import sleep +from typing import Any, Dict, Tuple from uuid import uuid4 import docker @@ -226,6 +229,8 @@ 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 = [ @@ -233,6 +238,41 @@ def _update_schema(self, schema): for identifier in additional_identifiers ] + def transform_model(self, input_model): + if not self.property_transform: + return None + # When CT input and output not equal, and with property transform + # Need to check system as property transform for CT not supported on Windows + if sys.platform.startswith("win"): + raise EnvironmentError( + "Property transform not available with contract tests on Windows OS" + ) + + import pyjq + + transformed_input_model = copy.deepcopy(input_model) + for key in self.property_transform_keys: + path = "/" + "/".join(key) + expression = self.property_transform[path] + transformed_value = pyjq.first(expression, transformed_input_model) + # key is a tuple like ("properties", "A", "B") + # input model is like: {"A": {"B": "valueB"}} + # use key[1:] here to remove "properties" + transformed_input_model = self.update_property( + transformed_input_model, transformed_value, key[1:] + ) + + return transformed_input_model + + def update_property( + self, model: Dict[str, Any], value: Any, path: Tuple[str, ...] + ) -> Dict[str, Any]: + if len(path) > 1: + model[path[0]] = self.update_property(model[path[0]], value, path[1:]) + elif len(path) == 1: + model[path[0]] = value + return model + def has_only_writable_identifiers(self): return all( path in self.create_only_paths for path in self.primary_identifier_paths @@ -393,7 +433,17 @@ def generate_invalid_update_example(self, create_model): example = override_properties(self.invalid_strategy.example(), overrides) return {**create_model, **example} - def compare(self, inputs, outputs, path=()): + def compare(self, inputs, outputs): + try: + self.compare_model(inputs, outputs) + except AssertionError as exception: + transformed_inputs = self.transform_model(inputs) + if transformed_inputs: + self.compare_model(transformed_inputs, outputs) + else: + raise exception + + def compare_model(self, inputs, outputs, path=()): assertion_error_message = ( "All properties specified in the request MUST " "be present in the model returned, and they MUST" @@ -405,7 +455,11 @@ def compare(self, inputs, outputs, path=()): for key in inputs: new_path = path + (key,) if isinstance(inputs[key], dict): - self.compare(inputs[key], outputs[key], new_path) + self.compare_model( + inputs[key], + outputs[key], + new_path, + ) elif isinstance(inputs[key], list): assert len(inputs[key]) == len(outputs[key]) @@ -414,7 +468,10 @@ def compare(self, inputs, outputs, path=()): ) self.compare_collection( - inputs[key], outputs[key], is_ordered, new_path + inputs[key], + outputs[key], + is_ordered, + new_path, ) else: assert inputs[key] == outputs[key], assertion_error_message @@ -426,7 +483,11 @@ def compare(self, inputs, outputs, path=()): def compare_collection(self, inputs, outputs, is_ordered, path): if is_ordered: for index in range(len(inputs)): # pylint: disable=C0200 - self.compare(inputs[index], outputs[index], path) + self.compare_model( + inputs[index], + outputs[index], + path, + ) return assert {item_hash(item) for item in inputs} == { diff --git a/src/rpdk/core/contract/suite/resource/handler_commons.py b/src/rpdk/core/contract/suite/resource/handler_commons.py index 7e1b45f3..c6b21b76 100644 --- a/src/rpdk/core/contract/suite/resource/handler_commons.py +++ b/src/rpdk/core/contract/suite/resource/handler_commons.py @@ -164,7 +164,5 @@ def test_input_equals_output(resource_client, input_model, output_model): pruned_output_model = prune_properties_if_not_exist_in_path( pruned_output_model, pruned_input_model, resource_client.create_only_paths ) - resource_client.compare( - pruned_input_model, - pruned_output_model, - ) + + resource_client.compare(pruned_input_model, pruned_output_model) diff --git a/tests/contract/test_resource_client.py b/tests/contract/test_resource_client.py index e27c1d4d..8bb7958d 100644 --- a/tests/contract/test_resource_client.py +++ b/tests/contract/test_resource_client.py @@ -130,6 +130,23 @@ "handlers": {"create": {}, "delete": {}, "read": {}}, } +SCHEMA_WITH_PROPERTY_TRANSFORM = { + "properties": { + "a": {"type": "string"}, + "b": {"$ref": "#/definitions/c"}, + }, + "definitions": { + "c": { + "type": "object", + "properties": {"d": {"type": "String"}, "e": {"type": "integer"}}, + } + }, + "readOnlyProperties": ["/properties/a"], + "primaryIdentifier": ["/properties/a"], + "handlers": {"create": {}, "delete": {}, "read": {}}, + "propertyTransform": {"/properties/b/c/d": '.b.c.d + "Test"'}, +} + EMPTY_SCHEMA = {"handlers": {"create": [], "delete": [], "read": []}} @@ -334,6 +351,45 @@ def resource_client_inputs_composite_key(): return client +@pytest.fixture +def resource_client_inputs_property_transform(): + endpoint = "https://" + patch_sesh = patch( + "rpdk.core.contract.resource_client.create_sdk_session", autospec=True + ) + patch_creds = patch( + "rpdk.core.contract.resource_client.get_temporary_credentials", + autospec=True, + return_value={}, + ) + patch_account = patch( + "rpdk.core.contract.resource_client.get_account", + autospec=True, + return_value=ACCOUNT, + ) + with patch_sesh as mock_create_sesh, patch_creds as mock_creds: + with patch_account as mock_account: + mock_sesh = mock_create_sesh.return_value + mock_sesh.region_name = DEFAULT_REGION + client = ResourceClient( + DEFAULT_FUNCTION, + endpoint, + DEFAULT_REGION, + SCHEMA_WITH_PROPERTY_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 == SCHEMA_WITH_PROPERTY_TRANSFORM + assert client._overrides == EMPTY_OVERRIDE + assert client.account == ACCOUNT + + return client + + def test_prune_properties(): document = { "foo": "bar", @@ -692,6 +748,37 @@ def test_update_schema(resource_client): assert resource_client.create_only_paths == {("properties", "d")} +def test_transform_model(resource_client_inputs_property_transform): + inputs = {"a": "ValueA", "b": {"c": {"d": "ValueD", "e": 1}}} + expected_inputs = {"a": "ValueA", "b": {"c": {"d": "ValueDTest", "e": 1}}} + + transformed_inputs = resource_client_inputs_property_transform.transform_model( + inputs + ) + + assert transformed_inputs == expected_inputs + + +def test_compare_with_transform_should_pass(resource_client_inputs_property_transform): + inputs = {"a": "ValueA", "b": {"c": {"d": "ValueD", "e": 1}}} + # transformed_inputs = {"a": "ValueA", "b": {"c": {"d": "ValueDTest", "e": 1}}} + outputs = {"a": "ValueA", "b": {"c": {"d": "ValueDTest", "e": 1}}} + + resource_client_inputs_property_transform.compare(inputs, outputs) + + +def test_compare_with_transform_should_throw_exception( + resource_client_inputs_property_transform, +): + inputs = {"a": "ValueA", "b": {"c": {"d": "ValueD", "e": 1}}} + outputs = {"a": "ValueA", "b": {"c": {"d": "D", "e": 1}}} + + try: + resource_client_inputs_property_transform.compare(inputs, outputs) + except AssertionError: + logging.debug("This test expects Assertion Exception to be thrown") + + def test_strategy(resource_client): schema = { "properties": {