Skip to content

Commit

Permalink
Refactor to use jsonpatch and improve naming.
Browse files Browse the repository at this point in the history
  • Loading branch information
@marc-1010
marc-1010 committed Jun 7, 2024
1 parent d7ae482 commit c7f7103
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 89 deletions.
2 changes: 1 addition & 1 deletion setup.cfg
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions setup.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
78 changes: 31 additions & 47 deletions src/rpdk/core/project.py
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# pylint: disable=too-many-lines
import copy
import json
import logging
import os
Expand All @@ -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
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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(
Expand All @@ -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():
Expand Down
65 changes: 24 additions & 41 deletions tests/test_project.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
}
],
Expand Down Expand Up @@ -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,
},
{
Expand All @@ -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],
},
],
Expand Down Expand Up @@ -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,
},
],
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
},
],
Expand Down Expand Up @@ -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 = {
Expand All @@ -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,
},
],
Expand Down Expand Up @@ -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):
Expand All @@ -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],
},
],
Expand Down Expand Up @@ -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",
},
],
}
Expand Down Expand Up @@ -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,
},
],
Expand Down

0 comments on commit c7f7103

Please sign in to comment.