Repository: remind101/stacker
Pull request #557 (2018-03-14 01:15:17)

Don't require class_path or template_path for locked stacks
Technically, "locked" stacks don't actually need a `class_path` or a `template_path`, since they're readonly. This change also fixes a bug introduced in https://github.com/remind101/stacker/pull/550, where the `output` plugin would fail with the following when looking up outputs on locked stacks:

```
FailedVariableLookup: Couldn't resolve lookups in variable `StringVariable`. 'NoneType' object has no attribute '__getitem__'
```

Tests are gonna fail right now until https://github.com/remind101/stacker/pull/556 is merged.


Patch:
diff --git a/docs/config.rst b/docs/config.rst
index 3fbaab9..0848a46 100644
--- a/docs/config.rst
+++ b/docs/config.rst
@@ -334,7 +334,8 @@ A stack has the following keys:
   updated unless the stack is passed to stacker via the *--force* flag.
   This is useful for *risky* stacks that you don't want to take the
   risk of allowing CloudFormation to update, but still want to make
-  sure get launched when the environment is first created.
+  sure get launched when the environment is first created. When ``locked``,
+  it's not necessary to specify a ``class_path`` or ``template_path``.
 **enabled:**
   (optional) If set to false, the stack is disabled, and will not be
   built or updated. This can allow you to disable stacks in different
diff --git a/stacker/actions/build.py b/stacker/actions/build.py
index 492ad98..0b3fb35 100644
--- a/stacker/actions/build.py
+++ b/stacker/actions/build.py
@@ -228,6 +228,11 @@ class Action(BaseAction):
         except StackDoesNotExist:
             provider_stack = None
 
+        if provider_stack and not should_update(stack):
+            stack.set_outputs(
+                self.provider.get_output_dict(provider_stack))
+            return NotUpdatedStatus()
+
         recreate = False
         if provider_stack and old_status == SUBMITTED:
             logger.debug(
@@ -289,8 +294,6 @@ class Action(BaseAction):
             provider.create_stack(stack.fqn, template, parameters, tags,
                                   force_change_set)
             return SubmittedStatus("creating new stack")
-        elif not should_update(stack):
-            return NotUpdatedStatus()
 
         try:
             if provider.prepare_stack_for_update(provider_stack, tags):
diff --git a/stacker/config/__init__.py b/stacker/config/__init__.py
index bca00f7..a37b9e6 100644
--- a/stacker/config/__init__.py
+++ b/stacker/config/__init__.py
@@ -312,6 +312,11 @@ class Stack(Model):
         self.validate_stack_source(data)
 
     def validate_stack_source(self, data):
+        # Locked stacks don't actually need a template, since they're
+        # read-only.
+        if data["locked"]:
+            return
+
         if not (data["class_path"] or data["template_path"]):
             raise ValidationError(
                 "class_path or template_path is required.")
diff --git a/stacker/session_cache.py b/stacker/session_cache.py
index e3382d6..593bbbc 100644
--- a/stacker/session_cache.py
+++ b/stacker/session_cache.py
@@ -1,17 +1,10 @@
+import json
+import os
 import boto3
 import logging
 from .ui import ui
 
 
-logger = logging.getLogger(__name__)
-
-
-# A global credential cache that can be shared among boto3 sessions. This is
-# inherently threadsafe thanks to the GIL:
-# https://docs.python.org/3/glossary.html#term-global-interpreter-lock
-credential_cache = {}
-
-
 def get_session(region, profile=None):
     """Creates a boto3 session with a cache
 
@@ -26,6 +19,60 @@ def get_session(region, profile=None):
     session = boto3.Session(region_name=region, profile_name=profile)
     c = session._session.get_component('credential_provider')
     provider = c.get_provider('assume-role')
-    provider.cache = credential_cache
+    provider.cache = CredentialCache()
     provider._prompter = ui.getpass
     return session
+
+
+logger = logging.getLogger(__name__)
+
+
+class CredentialCache(object):
+
+    """JSON file cache.
+    This provides a dict like interface that stores JSON serializable
+    objects.
+    The objects are serialized to JSON and stored in a file.  These
+    values can be retrieved at a later time.
+    """
+
+    CACHE_DIR = os.path.expanduser(
+        os.path.join('~', '.aws', 'cli', 'cache'))
+
+    def __init__(self, working_dir=CACHE_DIR):
+        self._working_dir = working_dir
+
+    def __contains__(self, cache_key):
+        actual_key = self._convert_cache_key(cache_key)
+        return os.path.isfile(actual_key)
+
+    def __getitem__(self, cache_key):
+        """Retrieve value from a cache key."""
+        logger.debug("Getting cached credentials from: %s", self.CACHE_DIR)
+        actual_key = self._convert_cache_key(cache_key)
+        try:
+            with open(actual_key) as f:
+                return json.load(f)
+        except (OSError, ValueError, IOError):
+            raise KeyError(cache_key)
+
+    def __setitem__(self, cache_key, value):
+        full_key = self._convert_cache_key(cache_key)
+        try:
+            file_content = json.dumps(value, default=str)
+        except (TypeError, ValueError):
+            raise ValueError("Value cannot be cached, must be "
+                             "JSON serializable: %s" % value)
+        if not os.path.isdir(self._working_dir):
+            os.makedirs(self._working_dir)
+        with os.fdopen(os.open(full_key,
+                               os.O_WRONLY | os.O_CREAT, 0o600), 'w') as f:
+            f.truncate()
+            f.write(file_content)
+            logger.debug(
+                "Updating cache with new obtained credentials: %s",
+                self.CACHE_DIR)
+
+    def _convert_cache_key(self, cache_key):
+        full_path = os.path.join(self._working_dir, cache_key + '.json')
+        return full_path


Test patch:
diff --git a/stacker/tests/fixtures/mock_blueprints.py b/stacker/tests/fixtures/mock_blueprints.py
index 7300098..d2b2394 100644
--- a/stacker/tests/fixtures/mock_blueprints.py
+++ b/stacker/tests/fixtures/mock_blueprints.py
@@ -154,7 +154,6 @@ class Dummy(Blueprint):
     def create_template(self):
         self.template.add_resource(WaitConditionHandle("Dummy"))
         self.template.add_output(Output("DummyId", Value="dummy-1234"))
-        self.template.add_output(Output("Region", Value=Ref("AWS::Region")))
 
 
 class Dummy2(Blueprint):
diff --git a/stacker/tests/test_config.py b/stacker/tests/test_config.py
index f5ee39a..8fcd011 100644
--- a/stacker/tests/test_config.py
+++ b/stacker/tests/test_config.py
@@ -55,7 +55,6 @@ class TestConfig(unittest.TestCase):
             config.validate()
 
         stack_errors = ex.exception.errors['stacks'][0]
-        print stack_errors
         self.assertEquals(
             stack_errors['template_path'][0].__str__(),
             "class_path or template_path is required.")
@@ -63,6 +62,15 @@ class TestConfig(unittest.TestCase):
             stack_errors['class_path'][0].__str__(),
             "class_path or template_path is required.")
 
+    def test_config_validate_missing_stack_source_when_locked(self):
+        config = Config({
+            "namespace": "prod",
+            "stacks": [
+                {
+                    "name": "bastion",
+                    "locked": True}]})
+        config.validate()
+
     def test_config_validate_stack_class_and_template_paths(self):
         config = Config({
             "namespace": "prod",
diff --git a/tests/suite.bats b/tests/suite.bats
index 32ca802..f297506 100755
--- a/tests/suite.bats
+++ b/tests/suite.bats
@@ -433,6 +433,51 @@ EOF
   assert_has_line "add-resource-test-with-replacements-only: complete (stack destroyed)"
 }
 
+@test "stacker build - locked stacks" {
+  needs_aws
+
+  config1() {
+    cat <<EOF
+namespace: ${STACKER_NAMESPACE}
+stacks:
+  - name: vpc
+    class_path: stacker.tests.fixtures.mock_blueprints.Dummy
+EOF
+  }
+
+  config2() {
+    cat <<EOF
+namespace: ${STACKER_NAMESPACE}
+stacks:
+  - name: vpc
+    locked: true
+  - name: bastion
+    class_path: stacker.tests.fixtures.mock_blueprints.Dummy
+    variables:
+      StringVariable: \${output vpc::DummyId}
+EOF
+  }
+
+  teardown() {
+    stacker destroy --force <(config2)
+  }
+
+  stacker build <(config2)
+  assert "$status" -eq 1
+  assert_has_line "AttributeError: Stack does not have a defined class or template path."
+
+  # Create the new stacks.
+  stacker build <(config1)
+  assert "$status" -eq 0
+
+  stacker build <(config2)
+  assert "$status" -eq 0
+  assert_has_line "Using default AWS provider mode"
+  assert_has_line "vpc: skipped (locked)"
+  assert_has_line "bastion: submitted (creating new stack)"
+  assert_has_line "bastion: complete (creating new stack)"
+}
+
 @test "stacker build - default mode, without & with protected stack" {
   needs_aws
 
@@ -908,21 +953,6 @@ EOF
   assert_has_line "vpc: complete (creating new stack)"
   assert_has_line "app: submitted (creating new stack)"
   assert_has_line "app: complete (creating new stack)"
-
-  config_simple() {
-    cat <<EOF
-namespace: ${STACKER_NAMESPACE}
-stacks:
-  - name: west/vpc
-    region: us-west-1
-    stack_name: vpc
-    class_path: stacker.tests.fixtures.mock_blueprints.Dummy
-EOF
-  }
-
-  # Assert that the vpc stack was built in us-west-1
-  stacker info <(config_simple)
-  assert_has_line "Region: us-west-1"
 }
 
 @test "stacker build - profiles" {