From d2399593e3a0eceb59b71ba144dd0dae6a9caa6b Mon Sep 17 00:00:00 2001
From: Felipe Monteiro <felipe.monteiro@att.com>
Date: Wed, 24 Jan 2018 11:06:12 -0500
Subject: [PATCH] Improve secret substitution logging and look up runtime

This PS adds multiple log statements to the secrets manager
module to assist with debugging and also uses a dictionary keyed
with (schema, name) => document to quickly retrieve substitution
sources; rather than doing a linear-time lookup per iteration,
a constant-time lookup is used instead, with only one linear-time
initialization done during the constructor to initialize the
substitution source dictionary.

Change-Id: I209430c48312be621fdc3787009346d3e9c12ac6
---
 deckhand/engine/layering.py        | 11 ++++++--
 deckhand/engine/secrets_manager.py | 41 +++++++++++++++++++++++++-----
 2 files changed, 43 insertions(+), 9 deletions(-)

diff --git a/deckhand/engine/layering.py b/deckhand/engine/layering.py
index 2c72d99a..e3d835f2 100644
--- a/deckhand/engine/layering.py
+++ b/deckhand/engine/layering.py
@@ -156,6 +156,11 @@ class DocumentLayering(object):
                          'discarded from the layerOrder during the layering '
                          'process.', layer)
                 layer_order.remove(layer)
+        if not layer_order:
+            LOG.info('Either the layerOrder in the LayeringPolicy was empty '
+                     'to begin with or no document layers were found in the '
+                     'layerOrder, causing it to become empty. No layering '
+                     'will be performed.')
         return layer_order
 
     def _extract_layering_policy(self, documents):
@@ -317,8 +322,10 @@ class DocumentLayering(object):
         # NOTE(fmontei): ``global_docs`` represents the topmost documents in
         # the system. It should probably be impossible for more than 1
         # top-level doc to exist, but handle multiple for now.
-        global_docs = [doc for doc in self._layered_documents
-                       if doc.layer == self._layer_order[0]]
+        global_docs = [
+            doc for doc in self._layered_documents
+            if self._layer_order and doc.layer == self._layer_order[0]
+        ]
 
         for doc in global_docs:
             layer_idx = self._layer_order.index(doc.layer)
diff --git a/deckhand/engine/secrets_manager.py b/deckhand/engine/secrets_manager.py
index 52c481fa..fe4f81dc 100644
--- a/deckhand/engine/secrets_manager.py
+++ b/deckhand/engine/secrets_manager.py
@@ -116,7 +116,7 @@ class SecretsSubstitution(object):
             documents = [documents]
 
         self._documents = []
-        self._substitution_sources = substitution_sources or []
+        self._substitution_sources = {}
 
         for document in documents:
             if not isinstance(document, document_wrapper.DocumentDict):
@@ -125,6 +125,13 @@ class SecretsSubstitution(object):
             if document.substitutions:
                 self._documents.append(document)
 
+        for document in substitution_sources:
+            if not isinstance(document, document_wrapper.DocumentDict):
+                document = document_wrapper.DocumentDict(document)
+            if document.schema and document.name:
+                self._substitution_sources.setdefault(
+                    (document.schema, document.name), document)
+
     def substitute_all(self):
         """Substitute all documents that have a `metadata.substitutions` field.
 
@@ -151,13 +158,28 @@ class SecretsSubstitution(object):
                 src_name = sub['src']['name']
                 src_path = sub['src']['path']
 
-                is_match = (lambda x: x['schema'] == src_schema and
-                                      x['metadata']['name'] == src_name)
-                try:
-                    src_doc = next(
-                        iter(filter(is_match, self._substitution_sources)))
-                except StopIteration:
+                if not src_schema:
+                    LOG.warning('Source document schema "%s" is unspecified '
+                                'under substitutions for document [%s] %s.',
+                                src_schema, document.schema, document.name)
+                if not src_name:
+                    LOG.warning('Source document name "%s" is unspecified'
+                                ' under substitutions for document [%s] %s.',
+                                src_name, document.schema, document.name)
+                if not src_path:
+                    LOG.warning('Source document path "%s" is unspecified '
+                                'under substitutions for document [%s] %s.',
+                                src_path, document.schema, document.name)
+
+                if (src_schema, src_name) in self._substitution_sources:
+                    src_doc = self._substitution_sources[
+                        (src_schema, src_name)]
+                else:
                     src_doc = {}
+                    LOG.warning('Could not find substitution source document '
+                                '[%s] %s among the provided '
+                                '`substitution_sources`.', src_schema,
+                                                           src_name)
 
                 # If the data is a dictionary, retrieve the nested secret
                 # via jsonpath_parse, else the secret is the primitive/string
@@ -171,6 +193,11 @@ class SecretsSubstitution(object):
                 dest_path = sub['dest']['path']
                 dest_pattern = sub['dest'].get('pattern', None)
 
+                if not dest_path:
+                    LOG.warning('Destination document path "%s" is unspecified'
+                                ' under substitutions for document [%s] %s.',
+                                dest_path, document.schema, document.name)
+
                 LOG.debug('Substituting from schema=%s name=%s src_path=%s '
                           'into dest_path=%s, dest_pattern=%s', src_schema,
                           src_name, src_path, dest_path, dest_pattern)