Address reviewer comments

CHANGELOG.md

@@ -1,7 +1,7 @@
 ## 0.22.0-rc.0:
 #### Breaking:
 - Added reveal secrets function, updated rsapublic function (#182)
-- Parameters.kapitan.secrets.recipients is deprecated, please use parameters.kapitan.secrets.gpg.recipients (#183)
+- parameters.kapitan.secrets.recipients is deprecated, please use parameters.kapitan.secrets.gpg.recipients (#183)
 
 #### Updates:
 - Added AWS KMS support as a secrets backend(#179)

kapitan/cli.py

@@ -206,8 +206,6 @@ def main():
         logging.basicConfig(level=logging.INFO, format="%(message)s")
 
     if cmd == 'eval':
-        logging.basicConfig(level=logging.WARNING)
-
         file_path = args.jsonnet_file
         search_paths = [os.path.abspath(path) for path in args.search_paths]
         ext_vars = {}
@@ -419,7 +417,7 @@ def secret_reveal(args, ref_controller):
             for rev_obj in revealer.reveal_path(file_name):
                 sys.stdout.write(rev_obj.content)
     except (RefHashMismatchError, KeyError):
-        logger.exception("Reveal failed for file {name}".format(name=file_name))
+        raise KapitanError("Reveal failed for file {name}".format(name=file_name))
 
 
 def secret_update_validate(args, ref_controller):
@@ -432,18 +430,26 @@ def secret_update_validate(args, ref_controller):
     target_token_paths = search_target_token_paths(secrets_path, targets)
     ret_code = 0
 
+    try:
+        recipients = kap_inv_params['secrets']['gpg']['recipients']
+    except KeyError:
+        recipients = None
+    try:
+        gkey = kap_inv_params['secrets']['gkms']['key']
+    except KeyError:
+        gkey = None
+    try:
+        awskey = kap_inv_params['secrets']['awskms']['key']
+    except KeyError:
+        awskey = None
+
     for target_name, token_paths in target_token_paths.items():
         kap_inv_params = inv['nodes'][target_name]['parameters']['kapitan']
         if 'secrets' not in kap_inv_params:
             raise KapitanError("parameters.kapitan.secrets not defined in {}".format(target_name))
 
         for token_path in token_paths:
             if token_path.startswith("?{gpg:"):
-                try:
-                    recipients = kap_inv_params['secrets']['gpg']['recipients']
-                except KeyError:
-                    recipients = None
-
                 if not recipients:
                     logger.debug("secret_update_validate: target: %s has no inventory gpg recipients, skipping %s", target_name, token_path)
                     continue
@@ -466,11 +472,6 @@ def secret_update_validate(args, ref_controller):
                         ref_controller[token_path] = secret_obj
 
             elif token_path.startswith("?{gkms:"):
-                try:
-                    gkey = kap_inv_params['secrets']['gkms']['key']
-                except KeyError:
-                    gkey = None
-
                 if not gkey:
                     logger.debug("secret_update_validate: target: %s has no inventory gkms key, skipping %s", target_name, token_path)
                     continue
@@ -484,11 +485,6 @@ def secret_update_validate(args, ref_controller):
                         ref_controller[token_path] = secret_obj
 
             elif token_path.startswith("?{awskms:"):
-                try:
-                    awskey = kap_inv_params['secrets']['awskms']['key']
-                except KeyError:
-                    awskey = None
-
                 if not awskey:
                     logger.debug("secret_update_validate: target: %s has no inventory awskms key, skipping %s", target_name, token_path)
                     continue

kapitan/refs/secrets/awskms.py

@@ -58,11 +58,11 @@ def from_params(cls, data, ref_params):
         """
         try:
             target_name = ref_params.kwargs['target_name']
-            if not target_name:
+            if target_name is None:
                 raise ValueError('target_name not set')
 
             target_inv = cached.inv['nodes'].get(target_name, None)
-            if not target_inv:
+            if target_inv is None:
                 raise ValueError('target_inv not set')
 
             key = target_inv['parameters']['kapitan']['secrets']['awskms']['key']
@@ -138,9 +138,8 @@ def dump(self):
         """
         Returns dict with keys/values to be serialised.
         """
-        orig = super().dump()
-        orig['key'] = self.key
-        return orig
+        return {"data": self.data, "encoding": self.encoding,
+                "key": self.key, "type": self.type_name}
 
 
 class AWSKMSBackend(RefBackend):

kapitan/refs/secrets/gkms.py

@@ -69,11 +69,11 @@ def from_params(cls, data, ref_params):
         """
         try:
             target_name = ref_params.kwargs['target_name']
-            if not target_name:
+            if target_name is None:
                 raise ValueError('target_name not set')
 
             target_inv = cached.inv['nodes'].get(target_name, None)
-            if not target_inv:
+            if target_inv is None:
                 raise ValueError('target_inv not set')
 
             key = target_inv['parameters']['kapitan']['secrets']['gkms']['key']
@@ -156,9 +156,8 @@ def dump(self):
         """
         Returns dict with keys/values to be serialised.
         """
-        orig = super().dump()
-        orig['key'] = self.key
-        return orig
+        return {"data": self.data, "encoding": self.encoding,
+                "key": self.key, "type": self.type_name}
 
 
 class GoogleKMSBackend(RefBackend):

kapitan/refs/secrets/gpg.py

@@ -79,11 +79,11 @@ def from_params(cls, data, ref_params):
                 return cls(data, _fingerprints, **ref_params.kwargs)
 
             target_name = ref_params.kwargs['target_name']
-            if not target_name:
+            if target_name is None:
                 raise ValueError('target_name not set')
 
             target_inv = cached.inv['nodes'].get(target_name, None)
-            if not target_inv:
+            if target_inv is None:
                 raise ValueError('target_inv not set')
 
             if 'secrets' not in target_inv['parameters']['kapitan']:
@@ -155,9 +155,8 @@ def dump(self):
         """
         Returns dict with keys/values to be serialised.
         """
-        orig = super().dump()
-        orig['recipients'] = self.recipients
-        return orig
+        return {"data": self.data, "encoding": self.encoding,
+                "recipients": self.recipients, "type": self.type_name}
 
 
 class GPGBackend(RefBackend):