Refactor to use jsonpatch and improve naming.

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

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",

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():

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,
  },
  ],