update hvac error handling (Open todo) (#391)

* Clean up old import that did not raise an exception when hvac was not installed * Check HVAC error and try to gracefully shutdown * Removing traceback and editing exception message; using the original exception * adding self.hvac import to make hvac available to other modules * sanity * restore code accidentally removed in merge conflict resolution * space common code properly * write base, failure and success test for importing the error class * Remove try catch block on vault helper test since it should always succeed add test for hashivaultplugin to test exception forward to ansible * move over to the new HVAC error system for KV modules * move over to the new HVAC error system for KV modules * remove unused imports * create a dedicated function to return the exceptions from HVAC (better to mock/monitor) * Implement new HVAC error system in all non-database modules * removed a little too much, reworked the DEFAULT_MOUNT_POINT variable * return get_hvac_exceptions back to get_hvac().exceptions * implement get_hvac().exceptions to all other plugins * removed HVAC_IMPORT_ERROR from the lookup vault write and adjusted the test accordingly * removed HVAC_IMPORT_ERROR from the last few lookup functions * extended the pki test to include authentication exception * Create tests for HashiVaultModule * validate has an exception and is before authenticate so authenticate should not be triggered and could do with an empty mock * Update plugins/module_utils/_hashi_vault_module.py * Update plugins/module_utils/_hashi_vault_module.py --------- Co-authored-by: Brian Scholer <[email protected]> Co-authored-by: mathijswesterhof <[email protected]>
ansible-collections · Oct 23, 2024 · ff5d334 · ff5d334
1 parent cdca188
commit ff5d334
Show file tree

Hide file tree

Showing 69 changed files with 340 additions and 1,039 deletions.
diff --git a/plugins/lookup/hashi_vault.py b/plugins/lookup/hashi_vault.py
@@ -244,18 +244,9 @@
 
 display = Display()
 
-HAS_HVAC = False
-try:
-    import hvac
-    HAS_HVAC = True
-except ImportError:
-    HAS_HVAC = False
-
 
 class LookupModule(HashiVaultLookupBase):
     def run(self, terms, variables=None, **kwargs):
-        if not HAS_HVAC:
-            raise AnsibleError("Please pip install hvac to use the hashi_vault lookup module.")
 
         ret = []
 
@@ -314,10 +305,11 @@ def get(self):
         field = self._secret_field
         secret = self.get_option('secret')
         return_as = self.get_option('return_format')
+        hvac_exceptions = self.helper.get_hvac().exceptions
 
         try:
             data = self.client.read(secret)
-        except hvac.exceptions.Forbidden:
+        except hvac_exceptions.Forbidden:
             raise AnsibleError("Forbidden: Permission Denied to secret '%s'." % secret)
 
         if data is None:

diff --git a/plugins/lookup/vault_kv1_get.py b/plugins/lookup/vault_kv1_get.py
@@ -165,22 +165,9 @@
 
 display = Display()
 
-try:
-    import hvac
-except ImportError as imp_exc:
-    HVAC_IMPORT_ERROR = imp_exc
-else:
-    HVAC_IMPORT_ERROR = None
-
 
 class LookupModule(HashiVaultLookupBase):
     def run(self, terms, variables=None, **kwargs):
-        if HVAC_IMPORT_ERROR:
-            raise_from(
-                AnsibleError("This plugin requires the 'hvac' Python library"),
-                HVAC_IMPORT_ERROR
-            )
-
         ret = []
 
         self.set_options(direct=kwargs, var_options=variables)
@@ -190,6 +177,7 @@ def run(self, terms, variables=None, **kwargs):
         self.connection_options.process_connection_options()
         client_args = self.connection_options.get_hvac_connection_options()
         client = self.helper.get_vault_client(**client_args)
+        hvac_exceptions = self.helper.get_hvac().exceptions
 
         engine_mount_point = self._options_adapter.get_option('engine_mount_point')
 
@@ -202,9 +190,9 @@ def run(self, terms, variables=None, **kwargs):
         for term in terms:
             try:
                 raw = client.secrets.kv.v1.read_secret(path=term, mount_point=engine_mount_point)
-            except hvac.exceptions.Forbidden as e:
+            except hvac_exceptions.Forbidden as e:
                 raise_from(AnsibleError("Forbidden: Permission Denied to path ['%s']." % term), e)
-            except hvac.exceptions.InvalidPath as e:
+            except hvac_exceptions.InvalidPath as e:
                 if 'Invalid path for a versioned K/V secrets engine' in str(e):
                     msg = "Invalid path for a versioned K/V secrets engine ['%s']. If this is a KV version 2 path, use community.hashi_vault.vault_kv2_get."
                 else:

diff --git a/plugins/lookup/vault_kv2_get.py b/plugins/lookup/vault_kv2_get.py
@@ -178,22 +178,9 @@
 
 display = Display()
 
-try:
-    import hvac
-except ImportError as imp_exc:
-    HVAC_IMPORT_ERROR = imp_exc
-else:
-    HVAC_IMPORT_ERROR = None
-
 
 class LookupModule(HashiVaultLookupBase):
     def run(self, terms, variables=None, **kwargs):
-        if HVAC_IMPORT_ERROR:
-            raise_from(
-                AnsibleError("This plugin requires the 'hvac' Python library"),
-                HVAC_IMPORT_ERROR
-            )
-
         ret = []
 
         self.set_options(direct=kwargs, var_options=variables)
@@ -203,6 +190,7 @@ def run(self, terms, variables=None, **kwargs):
         self.connection_options.process_connection_options()
         client_args = self.connection_options.get_hvac_connection_options()
         client = self.helper.get_vault_client(**client_args)
+        hvac_exceptions = self.helper.get_hvac().exceptions
 
         version = self._options_adapter.get_option_default('version')
         engine_mount_point = self._options_adapter.get_option('engine_mount_point')
@@ -216,9 +204,9 @@ def run(self, terms, variables=None, **kwargs):
         for term in terms:
             try:
                 raw = client.secrets.kv.v2.read_secret_version(path=term, version=version, mount_point=engine_mount_point)
-            except hvac.exceptions.Forbidden as e:
+            except hvac_exceptions.Forbidden as e:
                 raise_from(AnsibleError("Forbidden: Permission Denied to path ['%s']." % term), e)
-            except hvac.exceptions.InvalidPath as e:
+            except hvac_exceptions.InvalidPath as e:
                 raise_from(
                     AnsibleError("Invalid or missing path ['%s'] with secret version '%s'. Check the path or secret version." % (term, version or 'latest')),
                     e

diff --git a/plugins/lookup/vault_list.py b/plugins/lookup/vault_list.py
@@ -130,29 +130,14 @@
 from ansible.errors import AnsibleError
 from ansible.utils.display import Display
 
-from ansible.module_utils.six import raise_from
-
 from ansible_collections.community.hashi_vault.plugins.plugin_utils._hashi_vault_lookup_base import HashiVaultLookupBase
 from ansible_collections.community.hashi_vault.plugins.module_utils._hashi_vault_common import HashiVaultValueError
 
 display = Display()
 
-try:
-    import hvac
-except ImportError as imp_exc:
-    HVAC_IMPORT_ERROR = imp_exc
-else:
-    HVAC_IMPORT_ERROR = None
-
 
 class LookupModule(HashiVaultLookupBase):
     def run(self, terms, variables=None, **kwargs):
-        if HVAC_IMPORT_ERROR:
-            raise_from(
-                AnsibleError("This plugin requires the 'hvac' Python library"),
-                HVAC_IMPORT_ERROR
-            )
-
         ret = []
 
         self.set_options(direct=kwargs, var_options=variables)
@@ -162,6 +147,7 @@ def run(self, terms, variables=None, **kwargs):
         self.connection_options.process_connection_options()
         client_args = self.connection_options.get_hvac_connection_options()
         client = self.helper.get_vault_client(**client_args)
+        hvac_exceptions = self.helper.get_hvac().exceptions
 
         try:
             self.authenticator.validate()
@@ -172,7 +158,7 @@ def run(self, terms, variables=None, **kwargs):
         for term in terms:
             try:
                 data = client.list(term)
-            except hvac.exceptions.Forbidden:
+            except hvac_exceptions.Forbidden:
                 raise AnsibleError("Forbidden: Permission Denied to path '%s'." % term)
 
             if data is None:

diff --git a/plugins/lookup/vault_login.py b/plugins/lookup/vault_login.py
@@ -92,29 +92,14 @@
 from ansible.errors import AnsibleError
 from ansible.utils.display import Display
 
-from ansible.module_utils.six import raise_from
-
 from ...plugins.plugin_utils._hashi_vault_lookup_base import HashiVaultLookupBase
 from ...plugins.module_utils._hashi_vault_common import HashiVaultValueError
 
 display = Display()
 
-try:
-    import hvac  # pylint: disable=unused-import
-except ImportError as imp_exc:
-    HVAC_IMPORT_ERROR = imp_exc
-else:
-    HVAC_IMPORT_ERROR = None
-
 
 class LookupModule(HashiVaultLookupBase):
     def run(self, terms, variables=None, **kwargs):
-        if HVAC_IMPORT_ERROR:
-            raise_from(
-                AnsibleError("This plugin requires the 'hvac' Python library"),
-                HVAC_IMPORT_ERROR
-            )
-
         self.set_options(direct=kwargs, var_options=variables)
         # TODO: remove process_deprecations() if backported fix is available (see method definition)
         self.process_deprecations()

diff --git a/plugins/lookup/vault_read.py b/plugins/lookup/vault_read.py
@@ -84,29 +84,14 @@
 from ansible.errors import AnsibleError
 from ansible.utils.display import Display
 
-from ansible.module_utils.six import raise_from
-
 from ansible_collections.community.hashi_vault.plugins.plugin_utils._hashi_vault_lookup_base import HashiVaultLookupBase
 from ansible_collections.community.hashi_vault.plugins.module_utils._hashi_vault_common import HashiVaultValueError
 
 display = Display()
 
-try:
-    import hvac
-except ImportError as imp_exc:
-    HVAC_IMPORT_ERROR = imp_exc
-else:
-    HVAC_IMPORT_ERROR = None
-
 
 class LookupModule(HashiVaultLookupBase):
     def run(self, terms, variables=None, **kwargs):
-        if HVAC_IMPORT_ERROR:
-            raise_from(
-                AnsibleError("This plugin requires the 'hvac' Python library"),
-                HVAC_IMPORT_ERROR
-            )
-
         ret = []
 
         self.set_options(direct=kwargs, var_options=variables)
@@ -116,6 +101,7 @@ def run(self, terms, variables=None, **kwargs):
         self.connection_options.process_connection_options()
         client_args = self.connection_options.get_hvac_connection_options()
         client = self.helper.get_vault_client(**client_args)
+        hvac_exceptions = self.helper.get_hvac().exceptions
 
         try:
             self.authenticator.validate()
@@ -126,7 +112,7 @@ def run(self, terms, variables=None, **kwargs):
         for term in terms:
             try:
                 data = client.read(term)
-            except hvac.exceptions.Forbidden:
+            except hvac_exceptions.Forbidden:
                 raise AnsibleError("Forbidden: Permission Denied to path '%s'." % term)
 
             if data is None:

diff --git a/plugins/lookup/vault_token_create.py b/plugins/lookup/vault_token_create.py
@@ -100,20 +100,11 @@
 from ansible.errors import AnsibleError
 from ansible.utils.display import Display
 
-from ansible.module_utils.six import raise_from
-
 from ...plugins.plugin_utils._hashi_vault_lookup_base import HashiVaultLookupBase
 from ...plugins.module_utils._hashi_vault_common import HashiVaultValueError
 
 display = Display()
 
-try:
-    import hvac  # pylint: disable=unused-import
-except ImportError as imp_exc:
-    HVAC_IMPORT_ERROR = imp_exc
-else:
-    HVAC_IMPORT_ERROR = None
-
 
 class LookupModule(HashiVaultLookupBase):
     PASS_THRU_OPTION_NAMES = [
@@ -141,12 +132,6 @@ class LookupModule(HashiVaultLookupBase):
     }
 
     def run(self, terms, variables=None, **kwargs):
-        if HVAC_IMPORT_ERROR:
-            raise_from(
-                AnsibleError("This plugin requires the 'hvac' Python library"),
-                HVAC_IMPORT_ERROR
-            )
-
         self.set_options(direct=kwargs, var_options=variables)
         # TODO: remove process_deprecations() if backported fix is available (see method definition)
         self.process_deprecations()

diff --git a/plugins/lookup/vault_write.py b/plugins/lookup/vault_write.py
@@ -129,22 +129,9 @@
 
 display = Display()
 
-try:
-    import hvac
-except ImportError as imp_exc:
-    HVAC_IMPORT_ERROR = imp_exc
-else:
-    HVAC_IMPORT_ERROR = None
-
 
 class LookupModule(HashiVaultLookupBase):
     def run(self, terms, variables=None, **kwargs):
-        if HVAC_IMPORT_ERROR:
-            raise_from(
-                AnsibleError("This plugin requires the 'hvac' Python library"),
-                HVAC_IMPORT_ERROR
-            )
-
         ret = []
 
         self.set_options(direct=kwargs, var_options=variables)
@@ -154,6 +141,7 @@ def run(self, terms, variables=None, **kwargs):
         self.connection_options.process_connection_options()
         client_args = self.connection_options.get_hvac_connection_options()
         client = self.helper.get_vault_client(**client_args)
+        hvac_exceptions = self.helper.get_hvac().exceptions
 
         data = self._options_adapter.get_option('data')
         wrap_ttl = self._options_adapter.get_option_default('wrap_ttl')
@@ -176,11 +164,11 @@ def run(self, terms, variables=None, **kwargs):
                         raise_from(AnsibleError("To use 'path' or 'wrap_ttl' as data keys, use hvac >= 1.2"), e)
                     else:
                         response = client.write(path=term, wrap_ttl=wrap_ttl, **data)
-            except hvac.exceptions.Forbidden as e:
+            except hvac_exceptions.Forbidden as e:
                 raise_from(AnsibleError("Forbidden: Permission Denied to path '%s'." % term), e)
-            except hvac.exceptions.InvalidPath as e:
+            except hvac_exceptions.InvalidPath as e:
                 raise_from(AnsibleError("The path '%s' doesn't seem to exist." % term), e)
-            except hvac.exceptions.InternalServerError as e:
+            except hvac_exceptions.InternalServerError as e:
                 raise_from(AnsibleError("Internal Server Error: %s" % str(e)), e)
 
             # https://github.com/hvac/hvac/issues/797

diff --git a/plugins/module_utils/_hashi_vault_common.py b/plugins/module_utils/_hashi_vault_common.py
@@ -17,22 +17,29 @@
 import os
 
 
-HAS_HVAC = False
-try:
-    import hvac
-    HAS_HVAC = True
-except ImportError:
-    HAS_HVAC = False
-
-
 class HashiVaultValueError(ValueError):
     '''Use in common code to raise an Exception that can be turned into AnsibleError or used to fail_json()'''
 
 
+class HashiVaultHVACError(ImportError):
+    '''Use in common code to signal HVAC is missing.'''
+    def __init__(self, error, msg):
+        super().__init__(error)
+        self.msg = msg
+        self.error = error
+
+
 class HashiVaultHelper():
     def __init__(self):
-        # TODO move hvac checking here?
-        pass
+        try:
+            import hvac
+            self.hvac = hvac
+        except ImportError as e:
+            from ansible.module_utils.basic import missing_required_lib
+            raise HashiVaultHVACError(error=str(e), msg=missing_required_lib('hvac'))
+
+    def get_hvac(self):
+        return self.hvac
 
     def get_vault_client(
         self,
@@ -50,7 +57,7 @@ def get_vault_client(
         :type hashi_vault_revoke_on_logout: bool
         '''
 
-        client = hvac.Client(**kwargs)
+        client = self.hvac.Client(**kwargs)
 
         # logout to prevent accidental use of inferred tokens
         # https://github.com/ansible-collections/community.hashi_vault/issues/13

diff --git a/plugins/module_utils/_hashi_vault_module.py b/plugins/module_utils/_hashi_vault_module.py
@@ -16,6 +16,7 @@
 
 from ansible_collections.community.hashi_vault.plugins.module_utils._hashi_vault_common import (
     HashiVaultHelper,
+    HashiVaultHVACError,
     HashiVaultOptionAdapter,
 )
 from ansible_collections.community.hashi_vault.plugins.module_utils._connection_options import HashiVaultConnectionOptions
@@ -31,7 +32,14 @@ def __init__(self, *args, **kwargs):
 
         super(HashiVaultModule, self).__init__(*args, **kwargs)
 
-        self.helper = HashiVaultHelper()
+        try:
+            self.helper = HashiVaultHelper()
+        except HashiVaultHVACError as exc:
+            self.fail_json(
+                msg=exc.msg,
+                exception=exc.error
+            )
+
         self.adapter = HashiVaultOptionAdapter.from_dict(self.params)
         self.connection_options = HashiVaultConnectionOptions(option_adapter=self.adapter, retry_callback_generator=callback)
         self.authenticator = HashiVaultAuthenticator(option_adapter=self.adapter, warning_callback=self.warn, deprecate_callback=self.deprecate)