diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b1fba2f4..47365e90c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ ## Upcoming/Master - Add JSON and YAML codecs to file lookup +- Improve config. validation by only allowing unrecognized keys at the top level ## 1.3.0 (2018-05-03) diff --git a/stacker/config/__init__.py b/stacker/config/__init__.py index ae514c4ca..d1d10124d 100644 --- a/stacker/config/__init__.py +++ b/stacker/config/__init__.py @@ -13,7 +13,11 @@ from schematics import Model from schematics.exceptions import ValidationError -from schematics.exceptions import BaseError as SchematicsError +from schematics.exceptions import ( + BaseError as SchematicsError, + UndefinedValueError +) + from schematics.types import ( ModelType, ListType, @@ -137,19 +141,12 @@ def parse(raw_config): tmp_list.append(tmp_dict) config_dict[top_level_key] = tmp_list - # We have to enable non-strict mode, because people may be including top - # level keys for re-use with stacks (e.g. including something like - # `common_variables: &common_variables`). - # - # The unfortunate side effect of this is that it propagates down to every - # schematics model, and there doesn't seem to be a good way to only disable - # strict mode on a single model. - # - # If we enabled strict mode, it would break backwards compatibility, so we - # should consider enabling this in the future. - strict = False - - return Config(config_dict, strict=strict) + # Top-level excess keys are removed by Config._convert, so enabling strict + # mode is fine here. + try: + return Config(config_dict, strict=True) + except SchematicsError as e: + raise exceptions.InvalidConfig(e.errors) def load(config): @@ -407,9 +404,36 @@ class Config(Model): stacks = ListType( ModelType(Stack), default=[], validators=[not_empty_list]) - def validate(self): + def _remove_excess_keys(self, data): + excess_keys = set(data.keys()) + excess_keys -= self._schema.valid_input_keys + if not excess_keys: + return data + + logger.debug('Removing excess keys from config input: %s', + excess_keys) + clean_data = data.copy() + for key in excess_keys: + del clean_data[key] + + return clean_data + + def _convert(self, raw_data=None, context=None, **kwargs): + if raw_data is not None: + # Remove excess top-level keys, since we want to allow them to be + # used for custom user variables to be reference later. This is + # preferable to just disabling strict mode, as we can still + # disallow excess keys in the inner models. + raw_data = self._remove_excess_keys(raw_data) + + return super(Config, self)._convert(raw_data=raw_data, context=context, + **kwargs) + + def validate(self, *args, **kwargs): try: - super(Config, self).validate() + return super(Config, self).validate(*args, **kwargs) + except UndefinedValueError as e: + raise exceptions.InvalidConfig([e.message]) except SchematicsError as e: raise exceptions.InvalidConfig(e.errors) diff --git a/stacker/tests/test_config.py b/stacker/tests/test_config.py index 3c6de86b8..f1e7b25dd 100644 --- a/stacker/tests/test_config.py +++ b/stacker/tests/test_config.py @@ -537,6 +537,20 @@ def test_raise_construct_error_on_duplicate_stack_name_dict(self): with self.assertRaises(ConstructorError): parse(yaml_config) + def test_parse_invalid_inner_keys(self): + yaml_config = """ + namespace: prod + stacks: + - name: vpc + class_path: blueprints.VPC + garbage: yes + variables: + Foo: bar + """ + + with self.assertRaises(exceptions.InvalidConfig): + parse(yaml_config) + if __name__ == '__main__': unittest.main() diff --git a/tests/test_suite/10_stacker_build-simple_build.bats b/tests/test_suite/10_stacker_build-simple_build.bats index 2c92ca84d..57435d7d3 100644 --- a/tests/test_suite/10_stacker_build-simple_build.bats +++ b/tests/test_suite/10_stacker_build-simple_build.bats @@ -11,7 +11,7 @@ namespace: ${STACKER_NAMESPACE} stacks: - name: vpc class_path: stacker.tests.fixtures.mock_blueprints.VPC - stack_policy: tests/fixtures/stack_policies/default.json + stack_policy_path: ${PWD}/fixtures/stack_policies/default.json variables: PublicSubnets: 10.128.0.0/24,10.128.1.0/24,10.128.2.0/24,10.128.3.0/24 PrivateSubnets: 10.128.8.0/22,10.128.12.0/22,10.128.16.0/22,10.128.20.0/22