diff --git a/setup.cfg b/setup.cfg index 4933275c..d401f8a8 100644 --- a/setup.cfg +++ b/setup.cfg @@ -29,7 +29,7 @@ include_trailing_comma = true combine_as_imports = True force_grid_wrap = 0 known_first_party = rpdk -known_third_party = boto3,botocore,cfn_tools,cfnlint,colorama,docker,hypothesis,jinja2,jsonschema,nested_lookup,ordered_set,pkg_resources,pytest,pytest_localserver,requests,setuptools,yaml +known_third_party = boto3,botocore,cfn_tools,cfnlint,colorama,docker,hypothesis,jinja2,jsonpatch,jsonschema,nested_lookup,ordered_set,pkg_resources,pytest,pytest_localserver,requests,setuptools,yaml [tool:pytest] # can't do anything about 3rd part modules, so don't spam us diff --git a/setup.py b/setup.py index 03c9532d..83e6cd18 100644 --- a/setup.py +++ b/setup.py @@ -41,6 +41,7 @@ def find_version(*file_paths): "boto3>=1.10.20", "Jinja2>=3.1.2", "markupsafe>=2.1.0", + "jsonpatch", "jsonschema>=3.0.0,<=4.17.3", "pytest>=4.5.0", "pytest-random-order>=1.0.4", diff --git a/src/rpdk/core/project.py b/src/rpdk/core/project.py index bbd45b73..38c7ffd9 100644 --- a/src/rpdk/core/project.py +++ b/src/rpdk/core/project.py @@ -1,5 +1,4 @@ # pylint: disable=too-many-lines -import copy import json import logging import os @@ -12,6 +11,7 @@ from typing import Any, Dict from uuid import uuid4 +import jsonpatch import yaml from botocore.exceptions import ClientError, WaiterError from jinja2 import Environment, PackageLoader, select_autoescape @@ -64,6 +64,13 @@ TARGET_CANARY_FOLDER = "canary-bundle/canary" RPDK_CONFIG_FILE = ".rpdk-config" CANARY_FILE_PREFIX = "canary" +CANARY_FILE_CREATE_SUFFIX = "001" +CANARY_FILE_UPDATE_SUFFIX = "002" +CANARY_SUPPORTED_PATCH_INPUT_OPERATIONS = {"replace", "remove", "add"} +CREATE_INPUTS_KEY = "CreateInputs" +PATCH_INPUTS_KEY = "PatchInputs" +PATCH_VALUE_KEY = "value" +PATCH_OPERATION_KEY = "op" CONTRACT_TEST_DEPENDENCY_FILE_NAME = "dependencies.yml" CANARY_DEPENDENCY_FILE_NAME = "bootstrap.yaml" CANARY_SETTINGS = "canarySettings" @@ -1351,21 +1358,23 @@ def _generate_stack_template_files(self) -> None: count, stack_template_folder, self._replace_dynamic_values( - json_data["CreateInputs"], + json_data[CREATE_INPUTS_KEY], ), - "001", + CANARY_FILE_CREATE_SUFFIX, ) - if "PatchInputs" in json_data: - deepcopy_create_input = copy.deepcopy(json_data["CreateInputs"]) + if PATCH_INPUTS_KEY in json_data: + supported_patch_inputs = self._translate_supported_patch_inputs( + json_data[PATCH_INPUTS_KEY] + ) + patch_data = jsonpatch.apply_patch( + json_data[CREATE_INPUTS_KEY], supported_patch_inputs, in_place=False + ) self._save_stack_template_data( resource_name, count, stack_template_folder, - self._apply_patch_inputs_to_create_inputs( - json_data["PatchInputs"], - deepcopy_create_input, - ), - "002", + patch_data, + CANARY_FILE_UPDATE_SUFFIX, ) def _save_stack_template_data( @@ -1392,44 +1401,19 @@ def _save_stack_template_data( with stack_template_file_path.open("w") as stack_template_file: yaml.dump(stack_template_data, stack_template_file, indent=2) - def _apply_patch_inputs_to_create_inputs( - self, patch_inputs: Dict[str, Any], create_inputs: Dict[str, Any] - ) -> Dict[str, Any]: + def _translate_supported_patch_inputs(self, patch_inputs: Any) -> Any: + output = [] for patch_input in patch_inputs: - self._apply_patch_input_to_create_input(patch_input, create_inputs) - return create_inputs - - def _apply_patch_input_to_create_input( - self, patch_input: Any, create_inputs: Dict[str, Any] - ) -> Dict[str, Any]: - op = patch_input.get("op") - path = patch_input.get("path") - if op not in {"replace", "remove", "add"}: - return create_inputs - key_list = [self._translate_integer_key(key) for key in path.split("/") if key] - current_input = create_inputs - for key in key_list[:-1]: - try: - current_input = current_input[key] - except (KeyError, IndexError): - LOG.warning("Canary generation skipped for invalid path: %s", path) - return create_inputs - patch_key = key_list[-1] - if op == "remove": - del current_input[patch_key] - elif op == "add" and isinstance(current_input, list): - current_input.insert(patch_key, patch_input["value"]) - self._replace_dynamic_values_with_root_key(current_input, patch_key) - else: - # remaining use cases for both "add" and "replace" operations - current_input[patch_key] = patch_input["value"] - self._replace_dynamic_values_with_root_key(current_input, patch_key) - return create_inputs - - def _translate_integer_key(self, key: str) -> Any: - if key.isdigit(): - key = int(key) - return key + if ( + patch_input.get(PATCH_OPERATION_KEY) + in CANARY_SUPPORTED_PATCH_INPUT_OPERATIONS + ): + if PATCH_VALUE_KEY in patch_input: + self._replace_dynamic_values_with_root_key( + patch_input, PATCH_VALUE_KEY + ) + output.append(patch_input) + return output def _replace_dynamic_values(self, properties: Dict[str, Any]) -> Dict[str, Any]: for key, value in properties.items(): diff --git a/tests/test_project.py b/tests/test_project.py index 43f07fd4..3789b51f 100644 --- a/tests/test_project.py +++ b/tests/test_project.py @@ -17,6 +17,7 @@ from shutil import copyfile from unittest.mock import ANY, MagicMock, Mock, call, patch +import jsonpatch import pytest import yaml from botocore.exceptions import ClientError, WaiterError @@ -3045,7 +3046,7 @@ def test_generate_canary_files_with_patch_inputs(mock_yaml_dump, project): "PatchInputs": [ { "op": "replace", - "path": "Property1", + "path": "/Property1", "value": update_value_1, } ], @@ -3111,7 +3112,7 @@ def test_create_template_file_with_patch_inputs(mock_yaml_dump, project): "PatchInputs": [ { "op": "replace", - "path": "Property1", + "path": "/Property1", "value": update_value_1, }, { @@ -3121,12 +3122,12 @@ def test_create_template_file_with_patch_inputs(mock_yaml_dump, project): }, { "op": "replace", - "path": "Property3", + "path": "/Property3", "value": {"Nested": "{{partition}}"}, }, { "op": "replace", - "path": "Property4", + "path": "/Property4", "value": ["{{region}}", update_value_2], }, ], @@ -3223,12 +3224,12 @@ def test_create_template_file_by_list_index(mock_yaml_dump, project): "PatchInputs": [ { "op": "replace", - "path": "Property1/1", + "path": "/Property1/1", "value": update_value_1, }, { "op": "add", - "path": "Property2/1", + "path": "/Property2/1", "value": update_value_2, }, ], @@ -3300,15 +3301,15 @@ def test_create_template_file_with_skipped_patch_operation(mock_yaml_dump, proje "PatchInputs": [ { "op": "test", - "path": "Property1", + "path": "/Property1", "value": update_value_1, }, { "op": "move", - "path": "Property4", + "path": "/Property4", "value": update_value_2, }, - {"op": "copy", "from": "Property4", "path": "Property2"}, + {"op": "copy", "from": "Property4", "path": "/Property2"}, ], } setup_contract_test_data(project.root, contract_test_data) @@ -3379,13 +3380,13 @@ def test_create_template_file_with_patch_inputs_missing_from_create( }, "PatchInputs": [ { - "op": "replace", - "path": "Property4", + "op": "add", + "path": "/Property4", "value": ["{{region}}", update_value_2], }, { "op": "add", - "path": "Property8", + "path": "/Property8", "value": update_value_8, }, ], @@ -3466,9 +3467,7 @@ def test_create_template_file_with_patch_inputs_missing_from_create( @patch("rpdk.core.project.yaml.dump") -def test_create_template_file_with_skipping_patch_inputs_with_invalid_path( - mock_yaml_dump, project -): +def test_create_template_file_throws_error_with_invalid_path(mock_yaml_dump, project): update_value1 = "Value1b" update_value_2 = "Value2b" contract_test_data = { @@ -3478,12 +3477,12 @@ def test_create_template_file_with_skipping_patch_inputs_with_invalid_path( "PatchInputs": [ { "op": "replace", - "path": "Property1", + "path": "/Property1", "value": update_value1, }, { - "op": "replace", - "path": "Property4/SubProperty4", + "op": "add", + "path": "/Property4/SubProperty4", "value": update_value_2, }, ], @@ -3511,27 +3510,11 @@ def test_create_template_file_with_skipping_patch_inputs_with_invalid_path( with patch_settings(project, data) as mock_open, patch_load as mock_load: project.load_settings() - project.generate_canary_files() + with pytest.raises(jsonpatch.JsonPointerException): + project.generate_canary_files() mock_open.assert_called_once_with("r", encoding="utf-8") mock_load.assert_called_once_with(LANGUAGE) - expected_template_data = { - "Description": "Template for AWS::Example::Resource", - "Resources": { - "Resource": { - "Type": "AWS::Example::Resource", - "Properties": { - "Property1": update_value1, - }, - } - }, - } - args, kwargs = _get_mock_yaml_dump_call_arg( - mock_yaml_dump.call_args_list, CANARY_PATCH_FILE_SUFFIX - ) - assert args[0] == expected_template_data - assert kwargs - @patch("rpdk.core.project.yaml.dump") def test_create_template_file_with_nested_replace_patch_inputs(mock_yaml_dump, project): @@ -3550,12 +3533,12 @@ def test_create_template_file_with_nested_replace_patch_inputs(mock_yaml_dump, p "PatchInputs": [ { "op": "replace", - "path": "Property8/Nested/PropertyA", + "path": "/Property8/Nested/PropertyA", "value": update_value_1, }, { "op": "replace", - "path": "Property8/Nested/PropertyB", + "path": "/Property8/Nested/PropertyB", "value": ["{{region}}", update_value_2], }, ], @@ -3657,12 +3640,12 @@ def test_create_template_file_with_nested_remove_patch_inputs(mock_yaml_dump, pr "PatchInputs": [ { "op": "replace", - "path": "Property8/Nested/PropertyA", + "path": "/Property8/Nested/PropertyA", "value": update_value_1, }, { "op": "remove", - "path": "Property8/Nested/PropertyB/1", + "path": "/Property8/Nested/PropertyB/1", }, ], } @@ -3760,7 +3743,7 @@ def test_create_template_file_with_nested_add_patch_inputs(mock_yaml_dump, proje "PatchInputs": [ { "op": "add", - "path": "Property8/Nested/PropertyB/2", + "path": "/Property8/Nested/PropertyB/2", "value": update_value_2, }, ],