From 1edb90805ec6f3af61908c91a521ea810cfe5799 Mon Sep 17 00:00:00 2001 From: Michael Barrett Date: Sun, 8 Jan 2017 15:10:22 -0800 Subject: [PATCH] Remove --var flag --variables comes from the days when we didn't have environments, so the only way to change things between environments was to pass arguments on the command line. In general, environments are a better use case (reproducibility is much better) and continuing to support the CLI --var was getting difficult due to complex variable types. Removing for 1.0 after discussing on #stacker. If needed in the future, we can reconsider. --- stacker/blueprints/variables/types.py | 1 + stacker/commands/stacker/__init__.py | 2 -- stacker/commands/stacker/base.py | 9 ------ stacker/context.py | 5 --- stacker/lookups/registry.py | 1 + stacker/stack.py | 23 ++------------ stacker/tests/blueprints/test_base.py | 1 + stacker/tests/test_stack.py | 46 +-------------------------- stacker/tests/test_stacker.py | 30 ++++++++--------- 9 files changed, 19 insertions(+), 99 deletions(-) diff --git a/stacker/blueprints/variables/types.py b/stacker/blueprints/variables/types.py index a322a8da9..dbe681540 100644 --- a/stacker/blueprints/variables/types.py +++ b/stacker/blueprints/variables/types.py @@ -64,6 +64,7 @@ def __init__(self, parameter_type): """ self.parameter_type = parameter_type + CFNString = CFNType("String") CFNNumber = CFNType("Number") CFNNumberList = CFNType("List") diff --git a/stacker/commands/stacker/__init__.py b/stacker/commands/stacker/__init__.py index e4a2d6719..88a925905 100644 --- a/stacker/commands/stacker/__init__.py +++ b/stacker/commands/stacker/__init__.py @@ -1,4 +1,3 @@ -import copy import logging from .build import Build @@ -34,7 +33,6 @@ def configure(self, options, **kwargs): options.provider = default.Provider(region=options.region) options.context = Context( environment=options.environment, - variables=copy.deepcopy(options.variables), logger_type=self.logger_type, # Allow subcommands to provide any specific kwargs to the Context # that it wants. diff --git a/stacker/commands/stacker/base.py b/stacker/commands/stacker/base.py index 628b86937..53c39a784 100644 --- a/stacker/commands/stacker/base.py +++ b/stacker/commands/stacker/base.py @@ -119,15 +119,6 @@ def get_context_kwargs(self, options, **kwargs): return {} def add_arguments(self, parser): - # global arguments that should be available on all stacker subcommands - parser.add_argument("-var", "--variable", dest="variables", - metavar="BLUEPRINT_VARIABLE=VALUE", - type=key_value_arg, action=KeyValueAction, - default={}, - help="Adds variables from the command line " - "that can be used inside any of the stacks " - "being built. Can be specified more than " - "once.") parser.add_argument("-e", "--env", dest="cli_envs", metavar="ENV=VALUE", type=key_value_arg, action=KeyValueAction, default={}, diff --git a/stacker/context.py b/stacker/context.py index 93d55c1e7..adcfb797a 100644 --- a/stacker/context.py +++ b/stacker/context.py @@ -31,8 +31,6 @@ class Context(object): the environment. Useful for templating. stack_names (list): A list of stack_names to operate on. If not passed, usually all stacks defined in the config will be operated on. - variables (dict): Variables from the command line passed down to each - blueprint to parameterize the templates. mappings (dict): Used as Cloudformation mappings for the blueprint. config (dict): The configuration being operated on, containing the stack definitions. @@ -43,7 +41,6 @@ class Context(object): def __init__(self, environment, # pylint: disable-msg=too-many-arguments stack_names=None, - variables=None, mappings=None, config=None, logger_type=None, @@ -55,7 +52,6 @@ def __init__(self, environment, # pylint: disable-msg=too-many-arguments self.environment = environment self.stack_names = stack_names or [] - self.variables = variables or {} self.mappings = mappings or {} self.logger_type = logger_type self.namespace_delimiter = "-" @@ -108,7 +104,6 @@ def get_stacks(self): stack = Stack( definition=stack_def, context=self, - variables=self.variables, mappings=self.mappings, force=stack_def["name"] in self.force_stacks, locked=stack_def.get("locked", False), diff --git a/stacker/lookups/registry.py b/stacker/lookups/registry.py index 643c8919f..1b950b0b3 100644 --- a/stacker/lookups/registry.py +++ b/stacker/lookups/registry.py @@ -64,6 +64,7 @@ def resolve_lookups(lookups, context, provider): ) return resolved_lookups + register_lookup_handler(output.TYPE_NAME, output.handler) register_lookup_handler(kms.TYPE_NAME, kms.handler) register_lookup_handler(xref.TYPE_NAME, xref.handler) diff --git a/stacker/stack.py b/stacker/stack.py index d80176f7f..90446f206 100644 --- a/stacker/stack.py +++ b/stacker/stack.py @@ -11,7 +11,7 @@ ) -def _gather_variables(stack_def, context_variables): +def _gather_variables(stack_def): """Merges context provided & stack defined variables. If multiple stacks have a variable with the same name, we can specify the @@ -27,8 +27,6 @@ def _gather_variables(stack_def, context_variables): Args: stack_def (dict): The stack definition being worked on. - context_variables (dict): A dictionary of variables passed in - through the Context, usually from the CLI. Returns: dict: Contains key/value pairs of the collected variables. @@ -44,22 +42,6 @@ def _gather_variables(stack_def, context_variables): "'parameters', rather than 'variables'. Please " "update your config." % stack_name) variable_values = copy.deepcopy(stack_def.get('variables', {})) - stack_specific_variables = {} - for key, value in context_variables.iteritems(): - stack = None - if "::" in key: - stack, key = key.split("::", 1) - if not stack: - # Non-stack specific, go ahead and add it - variable_values[key] = value - continue - # Gather stack specific params for later - if stack == stack_name: - stack_specific_variables[key] = value - - # Now update stack definition variables with the stack specific variables - # ensuring they override generic variables - variable_values.update(stack_specific_variables) return [Variable(k, v) for k, v in variable_values.iteritems()] @@ -70,7 +52,6 @@ class Stack(object): definition (dict): A stack definition. context (:class:`stacker.context.Context`): Current context for building the stack. - variables (dict, optional): Context provided variables. mappings (dict, optional): Cloudformation mappings passed to the blueprint. locked (bool, optional): Whether or not the stack is locked. @@ -84,7 +65,7 @@ def __init__(self, definition, context, variables=None, mappings=None, self.name = definition["name"] self.fqn = context.get_fqn(self.name) self.definition = definition - self.variables = _gather_variables(definition, variables or {}) + self.variables = _gather_variables(definition) self.mappings = mappings self.locked = locked self.force = force diff --git a/stacker/tests/blueprints/test_base.py b/stacker/tests/blueprints/test_base.py index 69cd13d25..c73281aa7 100644 --- a/stacker/tests/blueprints/test_base.py +++ b/stacker/tests/blueprints/test_base.py @@ -39,6 +39,7 @@ def mock_lookup_handler(value, provider=None, context=None, fqn=False, **kwargs): return value + register_lookup_handler("mock", mock_lookup_handler) diff --git a/stacker/tests/test_stack.py b/stacker/tests/test_stack.py index a79e7e840..345e2b22d 100644 --- a/stacker/tests/test_stack.py +++ b/stacker/tests/test_stack.py @@ -71,52 +71,8 @@ def test_stack_cfn_parameters(self): param = stack.parameter_values["Param2"] self.assertEqual(param, "Some Resolved Value") - def test_empty_variables(self): - build_action_variables = {} - self.assertEqual([], _gather_variables(self.sd, - build_action_variables)) - - def test_generic_build_action_override(self): - sdef = self.sd - sdef["variables"] = {"Address": "10.0.0.1", "Foo": "BAR"} - build_action_variables = {"Address": "192.168.1.1"} - result = _gather_variables(sdef, build_action_variables) - variable_dict = dict((v.name, v.value) for v in result) - self.assertEqual(variable_dict["Address"], "192.168.1.1") - self.assertEqual(variable_dict["Foo"], "BAR") - - def test_stack_specific_override(self): - sdef = self.sd - sdef["variables"] = {"Address": "10.0.0.1", "Foo": "BAR"} - build_action_variables = {"test::Address": "192.168.1.1"} - result = _gather_variables(sdef, build_action_variables) - variable_dict = dict((v.name, v.value) for v in result) - self.assertEqual(variable_dict["Address"], "192.168.1.1") - self.assertEqual(variable_dict["Foo"], "BAR") - - def test_invalid_stack_specific_override(self): - sdef = self.sd - sdef["variables"] = {"Address": "10.0.0.1", "Foo": "BAR"} - build_action_variables = {"FAKE::Address": "192.168.1.1"} - result = _gather_variables(sdef, build_action_variables) - variable_dict = dict((v.name, v.value) for v in result) - self.assertEqual(variable_dict["Address"], "10.0.0.1") - self.assertEqual(variable_dict["Foo"], "BAR") - - def test_specific_vs_generic_build_action_override(self): - sdef = self.sd - sdef["variables"] = {"Address": "10.0.0.1", "Foo": "BAR"} - build_action_variables = { - "test::Address": "192.168.1.1", - "Address": "10.0.0.1"} - result = _gather_variables(sdef, build_action_variables) - variable_dict = dict((v.name, v.value) for v in result) - self.assertEqual(variable_dict["Address"], "192.168.1.1") - self.assertEqual(variable_dict["Foo"], "BAR") - def test_gather_variables_fails_on_parameters_in_stack_def(self): sdef = self.sd sdef["parameters"] = {"Address": "10.0.0.1", "Foo": "BAR"} - build_action_variables = {"Address": "192.168.1.1"} with self.assertRaises(AttributeError): - _gather_variables(sdef, build_action_variables) + _gather_variables(sdef) diff --git a/stacker/tests/test_stacker.py b/stacker/tests/test_stacker.py index b41b62049..222e82fff 100644 --- a/stacker/tests/test_stacker.py +++ b/stacker/tests/test_stacker.py @@ -8,17 +8,12 @@ class TestStacker(unittest.TestCase): def test_stacker_build_parse_args(self): stacker = Stacker() args = stacker.parse_args( - ["build", "-var", "BaseDomain=mike.com", "-r", "us-west-2", "-var", - "AZCount=2", "-var", "CidrBlock=10.128.0.0/16", + ["build", + "-r", "us-west-2", "-e", "namespace=test.override", "stacker/tests/fixtures/basic.env", "stacker/tests/fixtures/vpc-bastion-db-web.yaml"] ) - # verify variables - variables = args.variables - self.assertEqual(variables["BaseDomain"], "mike.com") - self.assertEqual(variables["CidrBlock"], "10.128.0.0/16") - self.assertEqual(variables["AZCount"], "2") self.assertEqual(args.region, "us-west-2") self.assertFalse(args.outline) # verify namespace was modified @@ -27,8 +22,8 @@ def test_stacker_build_parse_args(self): def test_stacker_build_context_passed_to_blueprint(self): stacker = Stacker() args = stacker.parse_args( - ["build", "-var", "BaseDomain=mike.com", "-r", "us-west-2", "-var", - "AZCount=2", "-var", "CidrBlock=10.128.0.0/16", + ["build", + "-r", "us-west-2", "stacker/tests/fixtures/basic.env", "stacker/tests/fixtures/vpc-bastion-db-web.yaml"] ) @@ -47,8 +42,8 @@ def test_stacker_build_context_passed_to_blueprint(self): def test_stacker_blueprint_property_access_does_not_reset_blueprint(self): stacker = Stacker() args = stacker.parse_args( - ["build", "-var", "BaseDomain=mike.com", "-r", "us-west-2", "-var", - "AZCount=2", "-var", "CidrBlock=10.128.0.0/16", + ["build", + "-r", "us-west-2", "stacker/tests/fixtures/basic.env", "stacker/tests/fixtures/vpc-bastion-db-web.yaml"] ) @@ -61,11 +56,12 @@ def test_stacker_blueprint_property_access_does_not_reset_blueprint(self): def test_stacker_build_context_stack_names_specified(self): stacker = Stacker() args = stacker.parse_args( - ["build", "-var", "BaseDomain=mike.com", "-r", "us-west-2", "-var", - "AZCount=2", "-var", "CidrBlock=10.128.0.0/16", + ["build", + "-r", "us-west-2", "stacker/tests/fixtures/basic.env", - "stacker/tests/fixtures/vpc-bastion-db-web.yaml", "--stacks", - "vpc", "--stacks", "bastion"] + "stacker/tests/fixtures/vpc-bastion-db-web.yaml", + "--stacks", "vpc", + "--stacks", "bastion"] ) stacker.configure(args) stacks = args.context.get_stacks() @@ -74,8 +70,8 @@ def test_stacker_build_context_stack_names_specified(self): def test_stacker_build_fail_when_parameters_in_stack_def(self): stacker = Stacker() args = stacker.parse_args( - ["build", "-var", "BaseDomain=mike.com", "-r", "us-west-2", "-var", - "AZCount=2", "-var", "CidrBlock=10.128.0.0/16", + ["build", + "-r", "us-west-2", "stacker/tests/fixtures/basic.env", "stacker/tests/fixtures/vpc-bastion-db-web-pre-1.0.yaml"] )