Address cert expiry parsing review comments

2 files changed, 7 insertions, 8 deletions

diff --git a/roles/openshift_certificate_expiry/library/openshift_cert_expiry.py b/roles/openshift_certificate_expiry/library/openshift_cert_expiry.py
index 33930c0c1..b093d84fe 100644
--- a/roles/openshift_certificate_expiry/library/openshift_cert_expiry.py
+++ b/roles/openshift_certificate_expiry/library/openshift_cert_expiry.py
@@ -242,6 +242,8 @@ will be returned
 
 
 # pylint: disable=too-many-locals,too-many-branches
+#
+# TODO: Break this function down into smaller chunks
 def load_and_handle_cert(cert_string, now, base64decode=False, ans_module=None):
     """Load a certificate, split off the good parts, and return some
 useful data
@@ -254,8 +256,8 @@ Params:
 - `ans_module` (AnsibleModule) - The AnsibleModule object for this module (so we can raise errors)
 
 Returns:
-A 3-tuple of the form: (certificate_common_name, certificate_expiry_date, certificate_time_remaining)
-
+A tuple of the form:
+    (cert_subject, cert_expiry_date, time_remaining, cert_serial_number)
     """
     if base64decode:
         _cert_string = cert_string.decode('base-64')
@@ -287,8 +289,9 @@ A 3-tuple of the form: (certificate_common_name, certificate_expiry_date, certif
             ans_module.fail_json(msg="Error: The 'OpenSSL' python library and CLI command were not found on the target host. Unable to parse any certificates. This host will not be included in generated reports.")
         else:
             openssl_decoded = openssl_decoded.communicate()[0]
-            os.remove(path)
             cert_loaded = FakeOpenSSLCertificate(openssl_decoded)
+        finally:
+            os.remove(path)
 
     ######################################################################
     # Read all possible names from the cert
diff --git a/roles/openshift_certificate_expiry/test/test_fakeopensslclasses.py b/roles/openshift_certificate_expiry/test/test_fakeopensslclasses.py
index e98d6ac64..2e245191f 100644
--- a/roles/openshift_certificate_expiry/test/test_fakeopensslclasses.py
+++ b/roles/openshift_certificate_expiry/test/test_fakeopensslclasses.py
@@ -11,7 +11,7 @@ import pytest
 # Disable import-error b/c our libraries aren't loaded in jenkins
 # pylint: disable=import-error,wrong-import-position
 # place class in our python path
-module_path = os.path.join('/'.join(os.path.realpath(__file__).split('/')[:-1]), 'library')
+module_path = os.path.join('/'.join(os.path.realpath(__file__).split(os.path.sep)[:-1]), 'library')
 sys.path.insert(0, module_path)
 openshift_cert_expiry = pytest.importorskip("openshift_cert_expiry")
 
@@ -77,10 +77,6 @@ class TestFakeOpenSSLClasses(unittest.TestCase):
 
         self.assertEqual('CN=172.30.0.1', ', '.join(subjects))
 
-    def tearDown(self):
-        '''TearDown method'''
-        pass
-
 
 if __name__ == "__main__":
     unittest.main()