Skip to content

Commit

Permalink
Require explicit lookup name
Browse files Browse the repository at this point in the history
This removes the old functionality where if a lookup was called, but no
lookup handler was given, it would default to using `output`.  IE:

```
SomeVariable: ${vpc::PublicSubnets}
```

must now explicitly call to the output lookup handler:

```
SomeVariable: ${output vpc::PublicSubnets}
```

Fixes cloudtools#298
  • Loading branch information
phobologic committed Jan 29, 2017
1 parent fcec318 commit bfba2f9
Show file tree
Hide file tree
Showing 8 changed files with 81 additions and 50 deletions.
17 changes: 12 additions & 5 deletions stacker/lookups/__init__.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
from collections import namedtuple
import re

from .registry import DEFAULT_LOOKUP
# export resolve_lookups at this level
from .registry import resolve_lookups # NOQA
from .registry import register_lookup_handler # NOQA

# TODO: we can remove the optionality of of the type in a later release, it
# is only included to allow for an error to be thrown while people are
# converting their configuration files to 1.0

LOOKUP_REGEX = re.compile("""
\$\{ # opening brace for the lookup
((?P<type>[._\-a-zA-Z0-9]*(?=\s)) # type of lookup, must be followed by a
# space to allow for defaulting to
# "output" type
# space
?\s* # any number of spaces separating the
# type from the input
(?P<input>[@\+\/,\._\-a-zA-Z0-9\:\s=]+) # the input value to the lookup
Expand All @@ -34,8 +36,13 @@ def extract_lookups_from_string(value):
for match in LOOKUP_REGEX.finditer(value):
groupdict = match.groupdict()
raw = match.groups()[0]
lookup_type = groupdict.get("type") or DEFAULT_LOOKUP
lookup_input = groupdict.get("input")
lookup_type = groupdict["type"]
if not lookup_type:
raise ValueError("stacker only allows explicit lookup types "
"now, and no longer uses `output` as the "
"default if not specified. Please update your "
"output lookups.")
lookup_input = groupdict["input"]
lookups.add(Lookup(lookup_type, lookup_input, raw))
return lookups

Expand Down
8 changes: 5 additions & 3 deletions stacker/tests/actions/test_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,11 @@ def setUp(self):
def _get_context(self, **kwargs):
config = {"stacks": [
{"name": "vpc"},
{"name": "bastion", "variables": {"test": "${vpc::something}"}},
{"name": "db", "variables": {"test": "${vpc::something}",
"else": "${bastion::something}"}},
{"name": "bastion",
"variables": {"test": "${output vpc::something}"}},
{"name": "db",
"variables": {"test": "${output vpc::something}",
"else": "${output bastion::something}"}},
{"name": "other", "variables": {}}
]}
return Context({"namespace": "namespace"}, config=config, **kwargs)
Expand Down
6 changes: 3 additions & 3 deletions stacker/tests/blueprints/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -264,11 +264,11 @@ class TestBlueprint(Blueprint):
blueprint = TestBlueprint(name="test", context=MagicMock())
variables = [
Variable("Param1", 1),
Variable("Param2", "${other-stack::Output}"),
Variable("Param2", "${output other-stack::Output}"),
Variable("Param3", 3),
]
resolved_lookups = {
mock_lookup("other-stack::Output"): "Test Output",
mock_lookup("other-stack::Output", "output"): "Test Output",
}
for var in variables:
var.replace(resolved_lookups)
Expand Down Expand Up @@ -326,7 +326,7 @@ class TestBlueprint(Blueprint):
variables = [
Variable(
"Param1",
"${custom non-string-return-val},${some-stack::Output}",
"${custom non-string-return-val},${output some-stack::Output}",
)
]
lookup = mock_lookup("non-string-return-val", "custom",
Expand Down
4 changes: 2 additions & 2 deletions stacker/tests/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def generate_definition(base_name, stack_id, **overrides):
return definition


def mock_lookup(lookup_input, lookup_type='output', raw=None):
def mock_lookup(lookup_input, lookup_type, raw=None):
if raw is None:
raw = lookup_input
raw = "%s %s" % (lookup_type, lookup_input)
return Lookup(type=lookup_type, input=lookup_input, raw=raw)
34 changes: 27 additions & 7 deletions stacker/tests/test_lookups.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import unittest

from stacker.lookups import extract_lookups
from stacker.lookups import extract_lookups, extract_lookups_from_string


class TestLookupExtraction(unittest.TestCase):
Expand All @@ -15,29 +15,33 @@ def test_single_lookup_string(self):

def test_multiple_lookups_string(self):
lookups = extract_lookups(
"url://${fakeStack::FakeOutput}@${fakeStack::FakeOutput2}"
"url://${output fakeStack::FakeOutput}@"
"${output fakeStack::FakeOutput2}"
)
self.assertEqual(len(lookups), 2)
self.assertEqual(list(lookups)[0].type, "output")

def test_lookups_list(self):
lookups = extract_lookups(["something", "${fakeStack::FakeOutput}"])
lookups = extract_lookups([
"something",
"${output fakeStack::FakeOutput}"
])
self.assertEqual(len(lookups), 1)

def test_lookups_dict(self):
lookups = extract_lookups({
"something": "${fakeStack::FakeOutput}",
"something": "${output fakeStack::FakeOutput}",
"other": "value",
})
self.assertEqual(len(lookups), 1)

def test_lookups_mixed(self):
lookups = extract_lookups({
"something": "${fakeStack::FakeOutput}",
"list": ["value", "${fakeStack::FakeOutput2}"],
"something": "${output fakeStack::FakeOutput}",
"list": ["value", "${output fakeStack::FakeOutput2}"],
"dict": {
"other": "value",
"another": "${fakeStack::FakeOutput3}",
"another": "${output fakeStack::FakeOutput3}",
},
})
self.assertEqual(len(lookups), 3)
Expand Down Expand Up @@ -79,3 +83,19 @@ def test_kms_file_lookup(self):
lookup = list(lookups)[0]
self.assertEqual(lookup.type, "kms")
self.assertEqual(lookup.input, "file://path/to/some/file.txt")

def test_valid_extract_lookups_from_string(self):
_type = "output"
_input = "vpc::PublicSubnets"
value = "${%s %s}" % (_type, _input)
lookups = extract_lookups_from_string(value)
l = lookups.pop()
assert l.type == _type
assert l.input == _input
assert l.raw == "%s %s" % (_type, _input)

def test_invalid_extract_lookups_from_string(self):
_input = "vpc::PublicSubnets"
value = "${%s}" % (_input)
with self.assertRaises(ValueError):
extract_lookups_from_string(value)
2 changes: 1 addition & 1 deletion stacker/tests/test_plan.py
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ def test_dump_no_provider_lookups(self, *args):
for i in range(5):
overrides = {
"variables": {
"Var1": "${fakeStack::FakeOutput}",
"Var1": "${output fakeStack::FakeOutput}",
"Var2": "${xref fakeStack::FakeOutput2}",
},
}
Expand Down
10 changes: 5 additions & 5 deletions stacker/tests/test_stack.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ def test_stack_requires(self):
variables={
"Var1": "${noop fakeStack3::FakeOutput}",
"Var2": (
"some.template.value:${fakeStack2::FakeOutput}:"
"${fakeStack::FakeOutput}"
"some.template.value:${output fakeStack2::FakeOutput}:"
"${output fakeStack::FakeOutput}"
),
"Var3": "${fakeStack::FakeOutput},"
"Var3": "${output fakeStack::FakeOutput},"
"${output fakeStack2::FakeOutput}",
},
requires=[self.context.get_fqn("fakeStack")],
Expand All @@ -47,7 +47,7 @@ def test_stack_requires_circular_ref(self):
base_name="vpc",
stack_id=1,
variables={
"Var1": "${vpc.1::FakeOutput}",
"Var1": "${output vpc.1::FakeOutput}",
},
)
stack = Stack(definition=definition, context=self.context)
Expand All @@ -59,7 +59,7 @@ def test_stack_cfn_parameters(self):
base_name="vpc",
stack_id=1,
variables={
"Param1": "${fakeStack::FakeOutput}",
"Param1": "${output fakeStack::FakeOutput}",
},
)
stack = Stack(definition=definition, context=self.context)
Expand Down
50 changes: 26 additions & 24 deletions stacker/tests/test_variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def test_variable_replace_no_lookups(self):
var = Variable("Param1", "2")
self.assertEqual(len(var.lookups), 0)
resolved_lookups = {
mock_lookup("fakeStack::FakeOutput"): "resolved",
mock_lookup("fakeStack::FakeOutput", "output"): "resolved",
}
var.replace(resolved_lookups)
self.assertEqual(var.value, "2")
Expand All @@ -32,16 +32,16 @@ def test_variable_resolve_no_lookups(self):
self.assertEqual(var.value, "2")

def test_variable_replace_simple_lookup(self):
var = Variable("Param1", "${fakeStack::FakeOutput}")
var = Variable("Param1", "${output fakeStack::FakeOutput}")
self.assertEqual(len(var.lookups), 1)
resolved_lookups = {
mock_lookup("fakeStack::FakeOutput"): "resolved",
mock_lookup("fakeStack::FakeOutput", "output"): "resolved",
}
var.replace(resolved_lookups)
self.assertEqual(var.value, "resolved")

def test_variable_resolve_simple_lookup(self):
var = Variable("Param1", "${fakeStack::FakeOutput}")
var = Variable("Param1", "${output fakeStack::FakeOutput}")
self.assertEqual(len(var.lookups), 1)
self.provider.get_output.return_value = "resolved"
var.resolve(self.context, self.provider)
Expand All @@ -51,20 +51,22 @@ def test_variable_resolve_simple_lookup(self):
def test_variable_replace_multiple_lookups_string(self):
var = Variable(
"Param1",
"url://${fakeStack::FakeOutput}@${fakeStack::FakeOutput2}",
"url://${output fakeStack::FakeOutput}@"
"${output fakeStack::FakeOutput2}",
)
self.assertEqual(len(var.lookups), 2)
resolved_lookups = {
mock_lookup("fakeStack::FakeOutput"): "resolved",
mock_lookup("fakeStack::FakeOutput2"): "resolved2",
mock_lookup("fakeStack::FakeOutput", "output"): "resolved",
mock_lookup("fakeStack::FakeOutput2", "output"): "resolved2",
}
var.replace(resolved_lookups)
self.assertEqual(var.value, "url://resolved@resolved2")

def test_variable_resolve_multiple_lookups_string(self):
var = Variable(
"Param1",
"url://${fakeStack::FakeOutput}@${fakeStack::FakeOutput2}",
"url://${output fakeStack::FakeOutput}@"
"${output fakeStack::FakeOutput2}",
)
self.assertEqual(len(var.lookups), 2)

Expand All @@ -84,33 +86,33 @@ def test_variable_replace_no_lookups_list(self):
var = Variable("Param1", ["something", "here"])
self.assertEqual(len(var.lookups), 0)
resolved_lookups = {
mock_lookup("fakeStack::FakeOutput"): "resolved",
mock_lookup("fakeStack::FakeOutput", "output"): "resolved",
}
var.replace(resolved_lookups)
self.assertEqual(var.value, ["something", "here"])

def test_variable_replace_lookups_list(self):
value = ["something", "${fakeStack::FakeOutput}",
"${fakeStack::FakeOutput2}"]
value = ["something", "${output fakeStack::FakeOutput}",
"${output fakeStack::FakeOutput2}"]
var = Variable("Param1", value)
self.assertEqual(len(var.lookups), 2)
resolved_lookups = {
mock_lookup("fakeStack::FakeOutput"): "resolved",
mock_lookup("fakeStack::FakeOutput2"): "resolved2",
mock_lookup("fakeStack::FakeOutput", "output"): "resolved",
mock_lookup("fakeStack::FakeOutput2", "output"): "resolved2",
}
var.replace(resolved_lookups)
self.assertEqual(var.value, ["something", "resolved", "resolved2"])

def test_variable_replace_lookups_dict(self):
value = {
"something": "${fakeStack::FakeOutput}",
"other": "${fakeStack::FakeOutput2}",
"something": "${output fakeStack::FakeOutput}",
"other": "${output fakeStack::FakeOutput2}",
}
var = Variable("Param1", value)
self.assertEqual(len(var.lookups), 2)
resolved_lookups = {
mock_lookup("fakeStack::FakeOutput"): "resolved",
mock_lookup("fakeStack::FakeOutput2"): "resolved2",
mock_lookup("fakeStack::FakeOutput", "output"): "resolved",
mock_lookup("fakeStack::FakeOutput2", "output"): "resolved2",
}
var.replace(resolved_lookups)
self.assertEqual(var.value, {"something": "resolved", "other":
Expand All @@ -119,21 +121,21 @@ def test_variable_replace_lookups_dict(self):
def test_variable_replace_lookups_mixed(self):
value = {
"something": [
"${fakeStack::FakeOutput}",
"${output fakeStack::FakeOutput}",
"other",
],
"here": {
"other": "${fakeStack::FakeOutput2}",
"same": "${fakeStack::FakeOutput}",
"mixed": "something:${fakeStack::FakeOutput3}",
"other": "${output fakeStack::FakeOutput2}",
"same": "${output fakeStack::FakeOutput}",
"mixed": "something:${output fakeStack::FakeOutput3}",
},
}
var = Variable("Param1", value)
self.assertEqual(len(var.lookups), 3)
resolved_lookups = {
mock_lookup("fakeStack::FakeOutput"): "resolved",
mock_lookup("fakeStack::FakeOutput2"): "resolved2",
mock_lookup("fakeStack::FakeOutput3"): "resolved3",
mock_lookup("fakeStack::FakeOutput", "output"): "resolved",
mock_lookup("fakeStack::FakeOutput2", "output"): "resolved2",
mock_lookup("fakeStack::FakeOutput3", "output"): "resolved3",
}
var.replace(resolved_lookups)
self.assertEqual(var.value, {
Expand Down

0 comments on commit bfba2f9

Please sign in to comment.