diff --git a/samcli/cli/types.py b/samcli/cli/types.py index a3743128ad..9d05d57940 100644 --- a/samcli/cli/types.py +++ b/samcli/cli/types.py @@ -6,10 +6,13 @@ import logging import re from json import JSONDecodeError +from pathlib import Path from typing import Dict, List, Optional, Union import click +from ruamel.yaml.comments import CommentedMap, CommentedSeq +from samcli.lib.config.file_manager import FILE_MANAGER_MAPPER from samcli.lib.package.ecr_utils import is_ecr_url PARAM_AND_METADATA_KEY_REGEX = """([A-Za-z0-9\\"\']+)""" @@ -61,6 +64,29 @@ def _unquote_wrapped_quotes(value): return value.replace("\\ ", " ").replace('\\"', '"').replace("\\'", "'") +def _flatten_list(data: Union[list, tuple]) -> list: + """ + Recursively flattens lists and other list-like types for easy sequential processing. + This also helps with lists combined with YAML anchors & aliases. + + Parameters + ---------- + value : data + List to flatten + + Returns + ------- + Flattened list + """ + flat_data = [] + for item in data: + if isinstance(item, (tuple, list, CommentedSeq)): + flat_data.extend(_flatten_list(item)) + else: + flat_data.append(item) + return flat_data + + class CfnParameterOverridesType(click.ParamType): """ Custom Click options type to accept values for CloudFormation template parameters. You can pass values for @@ -69,6 +95,7 @@ class CfnParameterOverridesType(click.ParamType): __EXAMPLE_1 = "ParameterKey=KeyPairName,ParameterValue=MyKey ParameterKey=InstanceType,ParameterValue=t1.micro" __EXAMPLE_2 = "KeyPairName=MyKey InstanceType=t1.micro" + __EXAMPLE_3 = "file://MyParams.yaml" # Regex that parses CloudFormation parameter key-value pairs: # https://regex101.com/r/xqfSjW/2 @@ -86,45 +113,69 @@ class CfnParameterOverridesType(click.ParamType): ordered_pattern_match = [_pattern_1, _pattern_2] - name = "string,list" - - def convert(self, value, param, ctx): - result = {} - - # Empty tuple - if value == ("",): - return result - - value = (value,) if isinstance(value, str) else value - for val in value: - # Add empty string to start of the string to help match `_pattern2` - normalized_val = " " + val.strip() + name = "list,object,string" - try: - # NOTE(TheSriram): find the first regex that matched. - # pylint is concerned that we are checking at the same `val` within the loop, - # but that is the point, so disabling it. - pattern = next( - i - for i in filter( - lambda item: re.findall(item, normalized_val), self.ordered_pattern_match - ) # pylint: disable=cell-var-from-loop - ) - except StopIteration: - return self.fail( - "{} is not in valid format. It must look something like '{}' or '{}'".format( - val, self.__EXAMPLE_1, self.__EXAMPLE_2 - ), + def convert(self, values, param, ctx): + """ + Normalizes different parameter overrides formats into key-value pairs of strings + """ + if values in (("",), "", None) or values == {}: + LOG.debug("Empty parameter set (%s)", values) + return {} + + LOG.debug("Input parameters: %s", values) + values = _flatten_list([values]) + LOG.debug("Flattened parameters: %s", values) + + parameters = {} + for value in values: + if isinstance(value, str): + if value.startswith("file://"): + filepath = Path(value[7:]) + if not filepath.is_file(): + self.fail(f"{value} was not found or is a directory", param, ctx) + file_manager = FILE_MANAGER_MAPPER.get(filepath.suffix, None) + if not file_manager: + self.fail(f"{value} uses an unsupported extension", param, ctx) + parameters.update(self.convert(file_manager.read(filepath), param, ctx)) + else: + # Legacy parameter matching + normalized_value = " " + value.strip() + for pattern in self.ordered_pattern_match: + groups = re.findall(pattern, normalized_value) + if groups: + parameters.update(groups) + break + else: + self.fail( + f"{value} is not a valid format. It must look something like " + f"'{self.__EXAMPLE_1}', '{self.__EXAMPLE_2}', or '{self.__EXAMPLE_3}'", + param, + ctx, + ) + elif isinstance(value, (dict, CommentedMap)): + # e.g. YAML key-value pairs + for k, v in value.items(): + if v is None: + parameters[str(k)] = "" + elif isinstance(v, (list, CommentedSeq)): + # Collapse list values to comma delimited, ignore empty strings and remove extra spaces + parameters[str(k)] = ",".join(str(x).strip() for x in v if x not in (None, "")) + else: + parameters[str(k)] = str(v) + elif value is None: + continue + else: + self.fail( + f"{value} is not valid in a way the code doesn't expect", param, ctx, ) - groups = re.findall(pattern, normalized_val) - - # 'groups' variable is a list of tuples ex: [(key1, value1), (key2, value2)] - for key, param_value in groups: - result[_unquote_wrapped_quotes(key)] = _unquote_wrapped_quotes(param_value) - + result = {} + for key, param_value in parameters.items(): + result[_unquote_wrapped_quotes(key)] = _unquote_wrapped_quotes(param_value) + LOG.debug("Output parameters: %s", result) return result diff --git a/schema/make_schema.py b/schema/make_schema.py index 8e1a8e1568..3905d2b2b3 100644 --- a/schema/make_schema.py +++ b/schema/make_schema.py @@ -45,7 +45,7 @@ class SamCliParameterSchema: type: Union[str, List[str]] description: str = "" default: Optional[Any] = None - items: Optional[str] = None + items: Optional[Union[str, Dict[str, str]]] = None choices: Optional[Any] = None def to_schema(self) -> Dict[str, Any]: @@ -55,7 +55,11 @@ def to_schema(self) -> Dict[str, Any]: if self.default: param.update({"default": self.default}) if self.items: - param.update({"items": {"type": self.items}}) + if isinstance(self.items, dict): + param.update({"items": self.items}) + else: + param.update({"items": {"type": self.items}}) + if self.choices: if isinstance(self.choices, list): self.choices.sort() @@ -143,11 +147,15 @@ def format_param(param: click.core.Option) -> SamCliParameterSchema: formatted_param_types.append(param_name or "string") formatted_param_types = sorted(list(set(formatted_param_types))) # deduplicate + # Allow nested arrays of objects and strings + parameter_overrides_ref = { "$ref": "#/$defs/parameter_overrides_items" } + formatted_param: SamCliParameterSchema = SamCliParameterSchema( param.name or "", formatted_param_types if len(formatted_param_types) > 1 else formatted_param_types[0], clean_text(param.help or ""), - items="string" if "array" in formatted_param_types else None, + items=parameter_overrides_ref if param.name == "parameter_overrides" + else "string" if "array" in formatted_param_types else None, ) if param.default and param.name not in PARAMS_TO_OMIT_DEFAULT_FIELD: @@ -231,6 +239,31 @@ def generate_schema() -> dict: } schema["required"] = ["version"] schema["additionalProperties"] = False + # Allows objects and strings beneath variably nested arrays + schema["$defs"]= { + "parameter_overrides_items": { + "anyOf": [ + { + "type": "array", + "items": { "$ref": "#/$defs/parameter_overrides_items" } + }, + { + "type": "object", + "additionalProperties": { + "anyOf": [ + { "type": "string" }, + { "type": "integer" }, # Allow cooler types if it's a dict + { "type": "boolean" }, + ] + }, + }, + { + "type": "string" + }, + ] + } + } + # Iterate through packages for command and parameter information for package_name in _SAM_CLI_COMMAND_PACKAGES: commands.extend(retrieve_command_structure(package_name)) diff --git a/schema/samcli.json b/schema/samcli.json index feb5d8de55..b0e60db8aa 100644 --- a/schema/samcli.json +++ b/schema/samcli.json @@ -13,6 +13,37 @@ "version" ], "additionalProperties": false, + "$defs": { + "parameter_overrides_items": { + "anyOf": [ + { + "type": "array", + "items": { + "$ref": "#/$defs/parameter_overrides_items" + } + }, + { + "type": "object", + "additionalProperties": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "integer" + }, + { + "type": "boolean" + } + ] + } + }, + { + "type": "string" + } + ] + } + }, "patternProperties": { "^.+$": { "title": "Environment", @@ -351,11 +382,12 @@ "title": "parameter_overrides", "type": [ "array", + "object", "string" ], "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs.", "items": { - "type": "string" + "$ref": "#/$defs/parameter_overrides_items" } }, "skip_pull_image": { @@ -455,11 +487,12 @@ "title": "parameter_overrides", "type": [ "array", + "object", "string" ], "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs.", "items": { - "type": "string" + "$ref": "#/$defs/parameter_overrides_items" } }, "debug_port": { @@ -651,11 +684,12 @@ "title": "parameter_overrides", "type": [ "array", + "object", "string" ], "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs.", "items": { - "type": "string" + "$ref": "#/$defs/parameter_overrides_items" } }, "debug_port": { @@ -855,11 +889,12 @@ "title": "parameter_overrides", "type": [ "array", + "object", "string" ], "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs.", "items": { - "type": "string" + "$ref": "#/$defs/parameter_overrides_items" } }, "debug_port": { @@ -1250,11 +1285,12 @@ "title": "parameter_overrides", "type": [ "array", + "object", "string" ], "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs.", "items": { - "type": "string" + "$ref": "#/$defs/parameter_overrides_items" } }, "signing_profiles": { @@ -1724,11 +1760,12 @@ "title": "parameter_overrides", "type": [ "array", + "object", "string" ], "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs.", "items": { - "type": "string" + "$ref": "#/$defs/parameter_overrides_items" } }, "beta_features": { @@ -2007,11 +2044,12 @@ "title": "parameter_overrides", "type": [ "array", + "object", "string" ], "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs.", "items": { - "type": "string" + "$ref": "#/$defs/parameter_overrides_items" } }, "stack_name": { @@ -2136,11 +2174,12 @@ "title": "parameter_overrides", "type": [ "array", + "object", "string" ], "description": "String that contains AWS CloudFormation parameter overrides encoded as key=value pairs.", "items": { - "type": "string" + "$ref": "#/$defs/parameter_overrides_items" } }, "stack_name": { diff --git a/tests/unit/cli/test_types.py b/tests/unit/cli/test_types.py index f6943107f6..bd7ffabfa8 100644 --- a/tests/unit/cli/test_types.py +++ b/tests/unit/cli/test_types.py @@ -150,7 +150,53 @@ def test_successful_parsing(self, input, expected): self.assertEqual(result, expected, msg="Failed with Input = " + str(input)) + @parameterized.expand( + [ + ( + ("file://params.toml",), + {"A": "toml", "B": "toml", "Toml": "toml"}, + ), + ( + ("file://params.toml", "D=4"), + {"A": "toml", "B": "toml", "Toml": "toml", "D": "4"}, + ), + ( + ("ParameterKey=Y,ParameterValue=y", "file://params.toml", "D=4", "file://params.yaml", "A=6"), + {"A": "6", "B": "yaml", "Y": "y", "Toml": "toml", "D": "4", "Yaml": "yaml"}, + ), + ( + ("file://params.yaml", "file://list.yaml",), + {"A": "a", "B": "yaml", "List": "1,2,3", "Yaml": "yaml"}, + ), + ( + ("file://nested.yaml",), + {"A": "yaml", "B": "yaml", "Toml": "toml", "Yaml": "yaml"}, + ), + ] + ) + def test_merge_file_parsing(self, inputs, expected): + mock_files = { + "params.toml": 'A = "toml"\nB = "toml"\nToml = "toml"', + "params.yaml": "A: yaml\nB: yaml\nYaml: yaml", + "list.yaml": "- - - - - - A: a\n- List:\n - 1\n - 2\n - 3\n", + "nested.yaml": "- file://params.toml\n- file://params.yaml", + } + + def mock_read_text(file_path): + file_name = file_path.name + return mock_files.get(file_name, "") + + def mock_is_file(file_path): + return file_path.name in mock_files + + with patch("pathlib.Path.is_file", new=mock_is_file), patch("pathlib.Path.read_text", new=mock_read_text): + result = self.param_type.convert(inputs, None, MagicMock()) + print(result) + self.assertEqual(result, expected, msg="Failed with Input = " + str(inputs)) + + class TestCfnMetadataType(TestCase): + def setUp(self): self.param_type = CfnMetadataType() diff --git a/tests/unit/schema/testdata/failing_tests/buildcmd_wrongtype_parameter_overrides_list.toml b/tests/unit/schema/testdata/failing_tests/buildcmd_wrongtype_parameter_overrides_list.toml new file mode 100644 index 0000000000..64e3a4805d --- /dev/null +++ b/tests/unit/schema/testdata/failing_tests/buildcmd_wrongtype_parameter_overrides_list.toml @@ -0,0 +1,4 @@ +version = 0.1 + +[default.build.parameters] +parameter_overrides = [ 10 ] \ No newline at end of file diff --git a/tests/unit/schema/testdata/failing_tests/buildcmd_wrongtype_parameter_overrides_list.yaml b/tests/unit/schema/testdata/failing_tests/buildcmd_wrongtype_parameter_overrides_list.yaml new file mode 100644 index 0000000000..7d477dd685 --- /dev/null +++ b/tests/unit/schema/testdata/failing_tests/buildcmd_wrongtype_parameter_overrides_list.yaml @@ -0,0 +1,7 @@ +version: 0.1 + +default: + build: + parameters: + parameter_overrides: + - 10 \ No newline at end of file diff --git a/tests/unit/schema/testdata/passing_tests/parameter_overrides_features.yaml b/tests/unit/schema/testdata/passing_tests/parameter_overrides_features.yaml new file mode 100644 index 0000000000..ede1025358 --- /dev/null +++ b/tests/unit/schema/testdata/passing_tests/parameter_overrides_features.yaml @@ -0,0 +1,19 @@ +version: 0.1 +prod-a: + deploy: + parameters: + stack_name: sam-app + region: us-east-1 + parameter_overrides: &prod_a_overrides + - Name: prod-a + - ParamA: 1 + - ParamB: True + +prod-b: + deploy: + parameters: + stack_name: sam-app-2 + region: us-east-1 + parameter_overrides: + - *prod_a_overrides + - Name: prod-b \ No newline at end of file diff --git a/tests/unit/schema/testdata/passing_tests/sample.toml b/tests/unit/schema/testdata/passing_tests/sample.toml index 4b95a2c644..03726773f1 100644 --- a/tests/unit/schema/testdata/passing_tests/sample.toml +++ b/tests/unit/schema/testdata/passing_tests/sample.toml @@ -14,6 +14,10 @@ capabilities = "CAPABILITY_IAM" confirm_changeset = true resolve_s3 = true +[default.deploy.parameters.parameter_overrides] +A = "foo" +B = "bar" + [default.sync.parameters] watch = true