From 38b8a2d9f1d539f677fad351e114b7a306a6c786 Mon Sep 17 00:00:00 2001 From: Adrien Ferrand Date: Thu, 7 Mar 2019 01:13:29 +0100 Subject: [PATCH] Improve names --- .../certbot_tests/assertions.py | 1 - .../certbot_tests/context.py | 34 ++++-- .../certbot_tests/test_main.py | 113 +++++++++--------- 3 files changed, 84 insertions(+), 64 deletions(-) diff --git a/certbot-ci/certbot_integration_tests/certbot_tests/assertions.py b/certbot-ci/certbot_integration_tests/certbot_tests/assertions.py index 311262fb0..eecbc0622 100644 --- a/certbot-ci/certbot_integration_tests/certbot_tests/assertions.py +++ b/certbot-ci/certbot_integration_tests/certbot_tests/assertions.py @@ -1,5 +1,4 @@ """This module contains advanced assertions for the certbot integration tests.""" - import os import grp diff --git a/certbot-ci/certbot_integration_tests/certbot_tests/context.py b/certbot-ci/certbot_integration_tests/certbot_tests/context.py index 2a30607f1..880acd08f 100644 --- a/certbot-ci/certbot_integration_tests/certbot_tests/context.py +++ b/certbot-ci/certbot_integration_tests/certbot_tests/context.py @@ -1,3 +1,4 @@ +"""Module do handle the context of integration tests.""" import os import tempfile import subprocess @@ -51,9 +52,14 @@ class IntegrationTestsContext(object): ).format(sys.executable, self.challtestsrv_mgt_port) def cleanup(self): + """Cleanup the integration test context.""" shutil.rmtree(self.workspace) - def certbot_test_no_force_renew(self, args): + def _common_test_no_force_renew(self, args): + """ + Base command to execute certbot in a distributed integration test context, + not renewing certificates by default. + """ new_environ = os.environ.copy() new_environ['TMPDIR'] = self.workspace @@ -86,20 +92,34 @@ class IntegrationTestsContext(object): return subprocess.check_output(command, universal_newlines=True, cwd=self.workspace, env=new_environ) - def certbot_test(self, args): + def _common_test(self, args): + """ + Base command to execute certbot in a distributed integration test context, + renewing certificates by default. + """ command = ['--renew-by-default'] command.extend(args) - return self.certbot_test_no_force_renew(command) + return self._common_test_no_force_renew(command) - def common_no_force_renew(self, args): + def certbot_no_force_renew(self, args): + """ + Execute certbot with given args, not renewing certificates by default. + :param args: args to pass to certbot + :return: output of certbot execution + """ command = ['--authenticator', 'standalone', '--installer', 'null'] command.extend(args) - return self.certbot_test_no_force_renew(command) + return self._common_test_no_force_renew(command) - def common(self, args): + def certbot(self, args): + """ + Execute certbot with given args, renewing certificates by default. + :param args: args to pass to certbot + :return: output of certbot execution + """ command = ['--renew-by-default'] command.extend(args) - return self.common_no_force_renew(command) + return self.certbot_no_force_renew(command) def wtf(self, subdomain='le'): """ diff --git a/certbot-ci/certbot_integration_tests/certbot_tests/test_main.py b/certbot-ci/certbot_integration_tests/certbot_tests/test_main.py index af8587b65..d1d9642c9 100644 --- a/certbot-ci/certbot_integration_tests/certbot_tests/test_main.py +++ b/certbot-ci/certbot_integration_tests/certbot_tests/test_main.py @@ -1,3 +1,4 @@ +"""Module executing integration tests against certbot core.""" from __future__ import print_function import subprocess import shutil @@ -29,12 +30,12 @@ def test_basic_commands(context): # and its content can be tested to check correct certbot cleanup. initial_count_tmpfiles = len(os.listdir(context.workspace)) - context.common(['--help']) - context.common(['--help', 'all']) - context.common(['--version']) + context.certbot(['--help']) + context.certbot(['--help', 'all']) + context.certbot(['--version']) with pytest.raises(subprocess.CalledProcessError): - context.common(['--csr']) + context.certbot(['--csr']) new_count_tmpfiles = len(os.listdir(context.workspace)) assert initial_count_tmpfiles == new_count_tmpfiles @@ -42,7 +43,7 @@ def test_basic_commands(context): def test_hook_dirs_creation(context): """Test all hooks directory are created during Certbot startup.""" - context.common(['register']) + context.certbot(['register']) for hook_dir in misc.list_renewal_hooks_dirs(context.config_dir): assert os.path.isdir(hook_dir) @@ -50,22 +51,22 @@ def test_hook_dirs_creation(context): def test_registration_override(context): """Test correct register/unregister, and registration override.""" - context.common(['register']) - context.common(['unregister']) - context.common(['register', '--email', 'ex1@domain.org,ex2@domain.org']) + context.certbot(['register']) + context.certbot(['unregister']) + context.certbot(['register', '--email', 'ex1@domain.org,ex2@domain.org']) # TODO: When `certbot register --update-registration` is fully deprecated, # delete the two following deprecated uses - context.common(['register', '--update-registration', '--email', 'ex1@domain.org']) - context.common(['register', '--update-registration', '--email', 'ex1@domain.org,ex2@domain.org']) + context.certbot(['register', '--update-registration', '--email', 'ex1@domain.org']) + context.certbot(['register', '--update-registration', '--email', 'ex1@domain.org,ex2@domain.org']) - context.common(['update_account', '--email', 'example@domain.org']) - context.common(['update_account', '--email', 'ex1@domain.org,ex2@domain.org']) + context.certbot(['update_account', '--email', 'example@domain.org']) + context.certbot(['update_account', '--email', 'ex1@domain.org,ex2@domain.org']) def test_prepare_plugins(context): """Test that plugins are correctly instantiated and displayed.""" - output = context.common(['plugins', '--init', '--prepare']) + output = context.certbot(['plugins', '--init', '--prepare']) assert 'webroot' in output @@ -76,7 +77,7 @@ def test_http_01(context): # TLS-SNI challenge to prevent regressions in #3601. with misc.create_tcp_server(context.tls_alpn_01_port): certname = context.wtf('le2') - context.common([ + context.certbot([ '--domains', certname, '--preferred-challenges', 'http-01', 'run', '--cert-name', certname, '--pre-hook', 'echo wtf.pre >> "{0}"'.format(context.hook_probe), @@ -94,7 +95,7 @@ def test_manual_http_auth(context): manual_http_hooks = misc.manual_http_hooks(webroot) certname = context.wtf() - context.common([ + context.certbot([ 'certonly', '-a', 'manual', '-d', certname, '--cert-name', certname, '--manual-auth-hook', manual_http_hooks[0], @@ -112,7 +113,7 @@ def test_manual_http_auth(context): def test_manual_dns_auth(context): """Test the DNS-01 challenge using manual plugin.""" certname = context.wtf('dns') - context.common([ + context.certbot([ '-a', 'manual', '-d', certname, '--preferred-challenges', 'dns', 'run', '--cert-name', certname, '--manual-auth-hook', context.manual_dns_auth_hook, @@ -129,7 +130,7 @@ def test_manual_dns_auth(context): def test_certonly(context): """Test the certonly verb on certbot.""" - context.common(['certonly', '--cert-name', 'newname', '-d', context.wtf('newname')]) + context.certbot(['certonly', '--cert-name', 'newname', '-d', context.wtf('newname')]) def test_auth_and_install_with_csr(context): @@ -143,7 +144,7 @@ def test_auth_and_install_with_csr(context): cert_path = join(context.workspace, 'csr/cert.pem') chain_path = join(context.workspace, 'csr/chain.pem') - context.common([ + context.certbot([ 'auth', '--csr', csr_path, '--cert-path', cert_path, '--chain-path', chain_path @@ -152,7 +153,7 @@ def test_auth_and_install_with_csr(context): print(misc.read_certificate(cert_path)) print(misc.read_certificate(chain_path)) - context.common([ + context.certbot([ '--domains', certname, 'install', '--cert-path', cert_path, '--key-path', key_path @@ -165,7 +166,7 @@ def test_renew(context): # We should have a new certificate, with hooks executed. # Check also file permissions. certname = context.wtf('renew') - context.common([ + context.certbot([ 'certonly', '-d', certname, '--rsa-key-size', '4096', '--preferred-challenges', 'http-01' ]) @@ -177,7 +178,7 @@ def test_renew(context): # Second, we force renew, and ensure that renewal hooks files are executed. # Also check that file permissions are correct. misc.generate_test_file_hooks(context.config_dir, context.hook_probe) - context.common(['renew']) + context.certbot(['renew']) assert_certs_count_for_lineage(context.config_dir, certname, 2) assert_hook_execution(context.hook_probe, 'deploy') @@ -196,7 +197,7 @@ def test_renew(context): # It is not time, so no renew should occur, and no hooks should be executed. open(context.hook_probe, 'w').close() misc.generate_test_file_hooks(context.config_dir, context.hook_probe) - context.common_no_force_renew(['renew']) + context.certbot_no_force_renew(['renew']) assert_certs_count_for_lineage(context.config_dir, certname, 2) with pytest.raises(AssertionError): @@ -213,7 +214,7 @@ def test_renew(context): lines.insert(4, 'renew_before_expiry = 100 years{0}'.format(os.linesep)) with open(join(context.config_dir, 'renewal/{0}.conf'.format(certname)), 'w') as file: file.writelines(lines) - context.common_no_force_renew(['renew', '--no-directory-hooks', + context.certbot_no_force_renew(['renew', '--no-directory-hooks', '--rsa-key-size', '2048']) assert_certs_count_for_lineage(context.config_dir, certname, 3) @@ -239,7 +240,7 @@ def test_renew(context): shutil.rmtree(hook_dir) os.makedirs(join(hook_dir, 'dir')) open(join(hook_dir, 'file'), 'w').close() - context.common(['renew']) + context.certbot(['renew']) assert_certs_count_for_lineage(context.config_dir, certname, 4) @@ -247,7 +248,7 @@ def test_renew(context): def test_hook_override(context): """Test correct hook override on renew.""" certname = context.wtf('override') - context.common([ + context.certbot([ 'certonly', '-d', certname, '--preferred-challenges', 'http-01', '--pre-hook', 'echo pre >> "{0}"'.format(context.hook_probe), @@ -261,7 +262,7 @@ def test_hook_override(context): # Now we override all previous hooks during next renew. open(context.hook_probe, 'w').close() - context.common([ + context.certbot([ 'renew', '--cert-name', certname, '--pre-hook', 'echo pre-override >> "{0}"'.format(context.hook_probe), '--post-hook', 'echo post-override >> "{0}"'.format(context.hook_probe), @@ -280,7 +281,7 @@ def test_hook_override(context): # Expect that this renew will reuse new hooks registered in the previous renew. open(context.hook_probe, 'w').close() - context.common(['renew', '--cert-name', certname]) + context.certbot(['renew', '--cert-name', certname]) assert_hook_execution(context.hook_probe, 'pre-override') assert_hook_execution(context.hook_probe, 'post-override') @@ -291,7 +292,7 @@ def test_invalid_domain_with_dns_challenge(context): """Test certificate issuance failure with DNS-01 challenge.""" # Manual dns auth hooks from misc are designed to fail if the domain contains 'fail-*'. certs = ','.join([context.wtf('dns1'), context.wtf('fail-dns1')]) - context.common([ + context.certbot([ '-a', 'manual', '-d', certs, '--allow-subset-of-names', '--preferred-challenges', 'dns', @@ -299,7 +300,7 @@ def test_invalid_domain_with_dns_challenge(context): '--manual-cleanup-hook', context.manual_dns_cleanup_hook ]) - output = context.common(['certificates']) + output = context.certbot(['certificates']) assert context.wtf('fail-dns1') not in output @@ -307,8 +308,8 @@ def test_invalid_domain_with_dns_challenge(context): def test_reuse_key(context): """Test various scenarios where a key is reused.""" certname = context.wtf('reusekey') - context.common(['--domains', certname, '--reuse-key']) - context.common(['renew', '--cert-name', certname]) + context.certbot(['--domains', certname, '--reuse-key']) + context.certbot(['renew', '--cert-name', certname]) with open(join(context.config_dir, 'archive/{0}/privkey1.pem').format(certname), 'r') as file: privkey1 = file.read() @@ -316,7 +317,7 @@ def test_reuse_key(context): privkey2 = file.read() assert privkey1 == privkey2 - context.common(['--cert-name', certname, '--domains', certname, '--force-renewal']) + context.certbot(['--cert-name', certname, '--domains', certname, '--force-renewal']) with open(join(context.config_dir, 'archive/{0}/privkey3.pem').format(certname), 'r') as file: privkey3 = file.read() @@ -340,7 +341,7 @@ def test_ecdsa(context): chain_path = join(context.workspace, 'chain-p384.pem') misc.generate_csr([context.wtf('ecdsa')], key_path, csr_path, key_type='ECDSA') - context.common(['auth', '--csr', csr_path, '--cert-path', cert_path, '--chain-path', chain_path]) + context.certbot(['auth', '--csr', csr_path, '--cert-path', cert_path, '--chain-path', chain_path]) certificate = misc.read_certificate(cert_path) assert 'ASN1 OID: secp384r1' in certificate @@ -349,7 +350,7 @@ def test_ecdsa(context): def test_ocsp_must_staple(context): """Test that OCSP Must-Staple is correctly set in the generated certificate.""" certname = context.wtf('must-staple') - context.common(['auth', '--must-staple', '--domains', certname]) + context.certbot(['auth', '--must-staple', '--domains', certname]) certificate = misc.read_certificate(join(context.config_dir, 'live/{0}/cert.pem').format(certname)) @@ -361,20 +362,20 @@ def test_revoke_simple(context): # Default action after revoke is to delete the certificate. certname = context.wtf() cert_path = join(context.config_dir, 'live/{0}/cert.pem'.format(certname)) - context.common(['-d', certname]) - context.common(['revoke', '--cert-path', cert_path, '--delete-after-revoke']) + context.certbot(['-d', certname]) + context.certbot(['revoke', '--cert-path', cert_path, '--delete-after-revoke']) assert not exists(cert_path) # Check default deletion is overridden. certname = context.wtf('le1') cert_path = join(context.config_dir, 'live/{0}/cert.pem'.format(certname)) - context.common(['-d', certname]) - context.common(['revoke', '--cert-path', cert_path, '--no-delete-after-revoke']) + context.certbot(['-d', certname]) + context.certbot(['revoke', '--cert-path', cert_path, '--no-delete-after-revoke']) assert exists(cert_path) - context.common(['delete', '--cert-name', certname]) + context.certbot(['delete', '--cert-name', certname]) assert not exists(join(context.config_dir, 'archive/{0}'.format(certname))) assert not exists(join(context.config_dir, 'live/{0}'.format(certname))) @@ -383,8 +384,8 @@ def test_revoke_simple(context): certname = context.wtf('le2') key_path = join(context.config_dir, 'live/{0}/privkey.pem'.format(certname)) cert_path = join(context.config_dir, 'live/{0}/cert.pem'.format(certname)) - context.common(['-d', certname]) - context.common(['revoke', '--cert-path', cert_path, '--key-path', key_path]) + context.certbot(['-d', certname]) + context.certbot(['revoke', '--cert-path', cert_path, '--key-path', key_path]) def test_revoke_and_unregister(context): @@ -397,18 +398,18 @@ def test_revoke_and_unregister(context): key_path2 = join(context.config_dir, 'live/{0}/privkey.pem'.format(cert2)) cert_path2 = join(context.config_dir, 'live/{0}/cert.pem'.format(cert2)) - context.common(['-d', cert1]) - context.common(['-d', cert2]) - context.common(['-d', cert3]) + context.certbot(['-d', cert1]) + context.certbot(['-d', cert2]) + context.certbot(['-d', cert3]) - context.common(['revoke', '--cert-path', cert_path1, + context.certbot(['revoke', '--cert-path', cert_path1, '--reason', 'cessationOfOperation']) - context.common(['revoke', '--cert-path', cert_path2, '--key-path', key_path2, + context.certbot(['revoke', '--cert-path', cert_path2, '--key-path', key_path2, '--reason', 'keyCompromise']) - context.common(['unregister']) + context.certbot(['unregister']) - output = context.common(['certificates']) + output = context.certbot(['certificates']) assert cert1 not in output assert cert2 not in output @@ -419,9 +420,9 @@ def test_revoke_corner_cases(context): """Test specific revoke corner case.""" # Cannot use --cert-path and --cert-name during a revoke. cert1 = context.wtf('le1') - context.common(['-d', cert1]) + context.certbot(['-d', cert1]) with pytest.raises(subprocess.CalledProcessError) as error: - context.common([ + context.certbot([ 'revoke', '--cert-name', cert1, '--cert-path', join(context.config_dir, 'live/{0}/fullchain.pem'.format(cert1)) ]) @@ -431,7 +432,7 @@ def test_revoke_corner_cases(context): # Revocation should not delete if multiple lineages share an archive dir cert2 = context.wtf('le2') - context.common(['-d', cert2]) + context.certbot(['-d', cert2]) with open(join(context.config_dir, 'renewal/{0}.conf'.format(cert2)), 'r') as file: data = file.read() @@ -443,7 +444,7 @@ def test_revoke_corner_cases(context): with open(join(context.config_dir, 'renewal/{0}.conf'.format(cert2)), 'w') as file: file.write(data) - output = context.common([ + output = context.certbot([ 'revoke', '--cert-path', join(context.config_dir, 'live/{0}/cert.pem'.format(cert1)) ]) @@ -457,7 +458,7 @@ def test_wildcard_certificates(context): certname = context.wtf('wild') - context.common([ + context.certbot([ '-a', 'manual', '-d', '*.{0},{0}'.format(certname), '--preferred-challenge', 'dns', '--manual-auth-hook', context.manual_dns_auth_hook, @@ -474,7 +475,7 @@ def test_ocsp_status(context): # OCSP 1: Check stale OCSP status sample_data_path = misc.load_sample_data_path(context.workspace) - output = context.common(['certificates', '--config-dir', sample_data_path]) + output = context.certbot(['certificates', '--config-dir', sample_data_path]) assert output.count('TEST_CERT') == 2, ('Did not find two test certs as expected ({0})' .format(output.count('TEST_CERT'))) @@ -482,13 +483,13 @@ def test_ocsp_status(context): .format(output.count('EXPIRED'))) # OSCP 2: Check live certificate OCSP status (VALID) - output = context.common(['--domains', 'le-ocsp-check.wtf']) + output = context.certbot(['--domains', 'le-ocsp-check.wtf']) assert output.count('VALID') == 1, 'Expected le-ocsp-check.wtf to be VALID' assert output.count('EXPIRED') == 0, 'Did not expect le-ocsp-check.wtf to be EXPIRED' # OSCP 3: Check live certificate OCSP status (REVOKED) - output = context.common(['certificates']) + output = context.certbot(['certificates']) assert output.count('INVALID') == 1, 'Expected le-ocsp-check.wtf to be INVALID' assert output.count('REVOKED') == 1, 'Expected le-ocsp-check.wtf to be REVOKED'