Merge pull request #20 from Venafi/release-fix

Release fix

library/venafi_certificate.py

@@ -270,7 +270,8 @@
 '''
 # Some strings variables
 STRING_FAILED_TO_CHECK_CERT_VALIDITY = "Certificate is not yet valid, " \
-    "has expired, or has CN or SANs that differ from the request"
+                                       "has expired, or has CN or SANs " \
+                                       "that differ from the request"
 STRING_PKEY_NOT_MATCHED = "Private key does not match certificate public key"
 STRING_BAD_PKEY = "Private key file does not contain a valid private key"
 STRING_CERT_FILE_NOT_EXISTS = "Certificate file does not exist"
@@ -450,26 +451,33 @@ def _check_certificate_validity(self, cert, validate):
                 % (cn, self.common_name)
             )
             return False
-        if cert.not_valid_after - datetime.timedelta(
-                hours=self.before_expired_hours) < datetime.datetime.now():
+        # Check if certificate not already expired
+        if cert.not_valid_after < datetime.datetime.now():
             self.changed_message.append(
-                'Hours before certificate expiration date %s '
-                'is less than before_expired_hours value %s'
+                'Certificate expiration date %s '
+                'is less than current time %s (certificate expired)'
                 % (cert.not_valid_after, self.before_expired_hours)
             )
-            # Do not return false if we're just validating existing certificate
-            if validate:
-                return True
-            else:
+            return False
+        # Check if certificate expiring time is greater than
+        # before_expired_hours (only for creating new certificate)
+        if not validate:
+            if cert.not_valid_after - datetime.timedelta(
+                    hours=self.before_expired_hours) < datetime.datetime.now():
+                self.changed_message.append(
+                    'Hours before certificate expiration date %s '
+                    'is less than before_expired_hours value %s'
+                    % (cert.not_valid_after, self.before_expired_hours)
+                )
                 return False
         if cert.not_valid_before - datetime.timedelta(
                 hours=24) > datetime.datetime.now():
             self.changed_message.append(
                 "Certificate expiration date %s "
                 "is set to future from server time %s."
                 % (cert.not_valid_before -
-                    datetime.timedelta(hours=24),
-                    (datetime.datetime.now()))
+                   datetime.timedelta(hours=24),
+                   (datetime.datetime.now()))
             )
             return False
         ips = []
@@ -482,7 +490,7 @@ def _check_certificate_validity(self, cert, validate):
             elif isinstance(e, x509.general_name.IPAddress):
                 ips.append(e.value.exploded)
         if self.ip_addresses and sorted(self.ip_addresses) != sorted(ips):
-            self.changed_message.append("IP addresses in request: %s and in "
+            self.changed_message.append("IP address in request: %s and in"
                                         "certificate: %s are different"
                                         % (sorted(self.ip_addresses), ips))
             self.changed_message.append("CN is %s" % cn)
@@ -533,7 +541,7 @@ def _check_files_permissions(self):
     def _check_file_permissions(self, path, update=False):
         return True  # todo: write
 
-    def check(self,validate):
+    def check(self, validate):
         """Return true if running will change anything"""
         result = {
             'cert_file_exists': True,
@@ -544,7 +552,7 @@ def check(self,validate):
                 'cert_file_exists': False,
                 'changed': True,
                 'changed_msg':
-                self.changed_message.append(STRING_CERT_FILE_NOT_EXISTS),
+                    self.changed_message.append(STRING_CERT_FILE_NOT_EXISTS),
             }
         else:
             try:

molecule/default/playbook.yml

@@ -15,6 +15,7 @@
     certificate_cert_dir: "/tmp/ansible/etc/ssl/{{ certificate_common_name }}"
     certificate_cert_path: "{{ certificate_cert_dir }}/{{ certificate_common_name }}.pem"
     certificate_chain_path: "{{ certificate_cert_dir }}/{{ certificate_common_name }}.chain.pem"
+    certificate_privatekey_size: "4096"
     certificate_privatekey_path: "{{ certificate_cert_dir }}/{{ certificate_common_name }}.key"
     certificate_csr_path: "{{ certificate_cert_dir }}/{{ certificate_common_name }}.csr"
 

tasks/local-certificate.yml

@@ -22,8 +22,9 @@
     cert_path: "{{ certificate_cert_path }}"
     chain_path: "{{ certificate_chain_path if certificate_chain_path is defined else None }}"
     privatekey_path: "{{ certificate_privatekey_path if certificate_privatekey_path is defined else None }}"
+    privatekey_size: "{{ certificate_privatekey_size if certificate_privatekey_size is defined else None }}"
     common_name: "{{ certificate_common_name }}"
-    alt_name: "{{ certificate_alt_name if certificate_alt_name is defined else None }}"
+    alt_name: "{{ certificate_alt_name | default([]) }}"
     before_expired_hours: "{{ certificate_before_expired_hours if certificate_before_expired_hours is defined else 72 }}"
     force: "{{ certificate_force if certificate_force is defined else false }}"
   register: certout

tasks/remote-certificate.yml

@@ -12,8 +12,9 @@
     cert_path: "{{ certificate_cert_path }}"
     chain_path: "{{ certificate_chain_path if certificate_chain_path is defined else None }}"
     privatekey_path: "{{ certificate_privatekey_path if certificate_privatekey_path is defined else None }}"
+    privatekey_size: "{{ certificate_privatekey_size if certificate_privatekey_size is defined else None }}"
     common_name: "{{ certificate_common_name }}"
-    alt_name: "{{ certificate_alt_name if certificate_alt_name is defined else None }}"
+    alt_name: "{{ certificate_alt_name | default([]) }}"
     before_expired_hours: "{{ certificate_before_expired_hours if certificate_before_expired_hours is defined else None }}"
     force: "{{ certificate_force if certificate_force is defined else false }}"
   register: certout

tests/jeremy-playbook.yml

@@ -6,15 +6,15 @@
       certificate_common_name: "ansible-test.se.venafi.com"
       certificate_cert_dir: "/tmp/etc/ssl/{{ certificate_common_name }}"
 
-      certificate_alt_name: "IP:192.168.1.1,DNS:san-example.se.com"
+      certificate_alt_name: "IP:192.168.0.15,DNS:ansible-test-ext.se.venafi.com"
       #certificate_alt_name: "IP:192.168.1.1,DNS:www.venafi.example.com,DNS:m.venafi.example.com,email:e@venafi.com,email:e2@venafi.com,IP Address:192.168.2.2"
 
       certificate_privatekey_type: "RSA"
       certificate_privatekey_size: "2048"
       #certificate_privatekey_curve: "P251"
       #certificate_privatekey_passphrase: "password"
       #certificate_chain_option: "last"
-      certificate_before_expired_hours: 72
+      certificate_before_expired_hours: 2000
 
       #certificate_cert_dir: "/etc/ssl/{{ certificate_common_name }}"
       certificate_cert_path: "{{ certificate_cert_dir }}/{{ certificate_common_name }}.pem"

tests/test_venafi_certificate.py

@@ -7,7 +7,7 @@
 CURRENT_DIR = os.path.dirname(os.path.abspath(__file__))
 
 
-testAsset = namedtuple("testAssert", "is_valid cert chain private_key password common_name alt_name")
+testAsset = namedtuple("testAssert", "is_valid cert chain private_key password common_name alt_name id")
 
 CERT_PATH = "/tmp/cert.pem"
 CHAIN_PATH = "/tmp/chain.pem"
@@ -41,6 +41,7 @@ def fail_json(self, **kwargs):
 class TestVcertificate(unittest.TestCase):
     def test_validate(self):
         for asset in TEST_ASSETS:
+            print("testing asset id %s" % asset.id)
             create_testfiles(asset)
             module = FakeModule(asset)
             vcert = VCertificate(module)
@@ -62,26 +63,34 @@ def create_testfiles(asset):
 
 TEST_ASSETS = [
     #simple valid
-    testAsset(is_valid=True,  cert="valid_rsa2048_cert.pem", chain="valid_rsa2048_chain.pem", private_key="valid_rsa2048_key.pem", password=None, common_name="test111.venafi.example.com", alt_name=None),
+    testAsset(is_valid=True,  cert="valid_rsa2048_cert.pem", chain="valid_rsa2048_chain.pem",
+              private_key="valid_rsa2048_key.pem", password=None, common_name="test111.venafi.example.com",
+              alt_name=None,id=1),
     #another cn
-    testAsset(is_valid=False, cert="valid_rsa2048_cert.pem", chain="valid_rsa2048_chain.pem", private_key="valid_rsa2048_key.pem", password=None, common_name="test1111.venafi.example.com", alt_name=None),
+    testAsset(is_valid=False, cert="valid_rsa2048_cert.pem", chain="valid_rsa2048_chain.pem",
+              private_key="valid_rsa2048_key.pem", password=None, common_name="test1111.venafi.example.com", alt_name=None,id=2),
     #corrupted file
-    testAsset(is_valid=False, cert="invalid_cert.pem", chain="valid_rsa2048_chain.pem", private_key="valid_rsa2048_key.pem", password=None, common_name="test111.venafi.example.com", alt_name=None),
+    testAsset(is_valid=False, cert="invalid_cert.pem", chain="valid_rsa2048_chain.pem",
+              private_key="valid_rsa2048_key.pem", password=None, common_name="test111.venafi.example.com", alt_name=None,id=3),
     #unmactched cn
-    testAsset(is_valid=False, cert="invalid_cn_rsa2048_cert.pem", chain="valid_rsa2048_chain.pem", private_key="valid_rsa2048_key.pem", password=None, common_name="test111.venafi.example.com", alt_name=None),
+    testAsset(is_valid=False, cert="invalid_cn_rsa2048_cert.pem", chain="valid_rsa2048_chain.pem",
+              private_key="valid_rsa2048_key.pem", password=None, common_name="test111.venafi.example.com", alt_name=None,id=4),
     # unmatched key type
-    testAsset(is_valid=False, cert="valid_rsa2048_cert.pem", chain="valid_rsa2048_chain.pem", private_key="valid_ec_key.pem", password=None, common_name="test1111.venafi.example.com", alt_name=None),
-
+    testAsset(is_valid=False, cert="valid_rsa2048_cert.pem", chain="valid_rsa2048_chain.pem",
+              private_key="valid_ec_key.pem", password=None, common_name="test1111.venafi.example.com", alt_name=None,id=5),
     #valid with dns
     testAsset(is_valid=True, cert="valid_alt_rsa2048_cert.pem", chain="valid_rsa2048_chain.pem",
               private_key="valid_rsa2048_key.pem", password=None, common_name="test123.venafi.example.com",
-              alt_name="IP:192.168.1.1,DNS:www.venafi.example.com,DNS:m.venafi.example.com,email:e@venafi.com,email:e2@venafi.com,IP Address:192.168.2.2"),
+              alt_name="IP:192.168.1.1,DNS:www.venafi.example.com,DNS:m.venafi.example.com,email:e@venafi.com,"
+                       "email:e2@venafi.com,IP Address:192.168.2.2",id=6),
     #invalid with dns
     testAsset(is_valid=False, cert="valid_alt_rsa2048_cert.pem", chain="valid_rsa2048_chain.pem",
               private_key="valid_rsa2048_key.pem", password=None, common_name="test123.venafi.example.com",
-              alt_name="IP:192.168.1.1,DNS:www.venafi.example.com,DNS:m.venafi.example.com,email:e@venafi.com,email:e2@venafi.com"),
+              alt_name="IP:192.168.1.1,DNS:www.venafi.example.com,DNS:m.venafi.example.com,email:e@venafi.com,"
+                       "email:e2@venafi.com",id=7),
     #expired
     testAsset(is_valid=False, cert="invalid_date_rsa2048_cert.pem", chain="valid_rsa2048_chain.pem",
-              private_key="valid_rsa2048_key.pem", password=None, common_name="test123.venafi.example.com", alt_name=None)
+              private_key="valid_rsa2048_key.pem", password=None, common_name="test123.venafi.example.com",
+              alt_name=None,id=8)
 ]
 

tests/venafi-playbook-example.yml

@@ -74,6 +74,7 @@
         cert_path: "{{ certificate_cert_path }}"
         chain_path: "{{ certificate_chain_path if certificate_chain_path is defined else None }}"
         privatekey_path: "{{ certificate_privatekey_path if certificate_privatekey_path is defined else None }}"
+        privatekey_size: "{{ certificate_privatekey_size if certificate_privatekey_size is defined else None }}"
         common_name: "{{ certificate_common_name }}"
       register: certout
     - name: "Certificate is in following state:"