diff --git a/.azure-pipelines/templates/jobs/packaging-jobs.yml b/.azure-pipelines/templates/jobs/packaging-jobs.yml index f0c6b6e49..900be9b2f 100644 --- a/.azure-pipelines/templates/jobs/packaging-jobs.yml +++ b/.azure-pipelines/templates/jobs/packaging-jobs.yml @@ -144,7 +144,7 @@ jobs: git config --global user.name "$(Build.RequestedFor)" mkdir -p ~/.local/share/snapcraft/provider/launchpad cp $(credentials.secureFilePath) ~/.local/share/snapcraft/provider/launchpad/credentials - python3 tools/snap/build_remote.py ALL --archs ${ARCHS} + python3 tools/snap/build_remote.py ALL --archs ${ARCHS} --timeout 19800 displayName: Build snaps - script: | set -e diff --git a/AUTHORS.md b/AUTHORS.md index f76c323a5..ff5c61613 100644 --- a/AUTHORS.md +++ b/AUTHORS.md @@ -149,6 +149,7 @@ Authors * [Lior Sabag](https://github.com/liorsbg) * [Lipis](https://github.com/lipis) * [lord63](https://github.com/lord63) +* [Lorenzo Fundaró](https://github.com/lfundaro) * [Luca Beltrame](https://github.com/lbeltrame) * [Luca Ebach](https://github.com/lucebac) * [Luca Olivetti](https://github.com/olivluca) diff --git a/acme/acme/__init__.py b/acme/acme/__init__.py index 8b6ce88c0..3ec5203bf 100644 --- a/acme/acme/__init__.py +++ b/acme/acme/__init__.py @@ -6,6 +6,7 @@ This module is an implementation of the `ACME protocol`_. """ import sys +import warnings # This code exists to keep backwards compatibility with people using acme.jose # before it became the standalone josepy package. @@ -19,3 +20,10 @@ for mod in list(sys.modules): # preserved (acme.jose.* is josepy.*) if mod == 'josepy' or mod.startswith('josepy.'): sys.modules['acme.' + mod.replace('josepy', 'jose', 1)] = sys.modules[mod] + +if sys.version_info[0] == 2: + warnings.warn( + "Python 2 support will be dropped in the next release of acme. " + "Please upgrade your Python version.", + PendingDeprecationWarning, + ) # pragma: no cover diff --git a/acme/acme/crypto_util.py b/acme/acme/crypto_util.py index f8b7e2b30..cabc7f4d1 100644 --- a/acme/acme/crypto_util.py +++ b/acme/acme/crypto_util.py @@ -186,6 +186,7 @@ def probe_sni(name, host, port=443, timeout=300, # pylint: disable=too-many-argu raise errors.Error(error) return client_ssl.get_peer_certificate() + def make_csr(private_key_pem, domains, must_staple=False): """Generate a CSR containing a list of domains as subjectAltNames. @@ -217,6 +218,7 @@ def make_csr(private_key_pem, domains, must_staple=False): return crypto.dump_certificate_request( crypto.FILETYPE_PEM, csr) + def _pyopenssl_cert_or_req_all_names(loaded_cert_or_req): common_name = loaded_cert_or_req.get_subject().CN sans = _pyopenssl_cert_or_req_san(loaded_cert_or_req) @@ -225,6 +227,7 @@ def _pyopenssl_cert_or_req_all_names(loaded_cert_or_req): return sans return [common_name] + [d for d in sans if d != common_name] + def _pyopenssl_cert_or_req_san(cert_or_req): """Get Subject Alternative Names from certificate or CSR using pyOpenSSL. @@ -317,6 +320,7 @@ def gen_ss_cert(key, domains, not_before=None, cert.sign(key, "sha256") return cert + def dump_pyopenssl_chain(chain, filetype=crypto.FILETYPE_PEM): """Dump certificate chain into a bundle. diff --git a/certbot-apache/certbot_apache/_internal/configurator.py b/certbot-apache/certbot_apache/_internal/configurator.py index c20a4fdd6..16def1998 100644 --- a/certbot-apache/certbot_apache/_internal/configurator.py +++ b/certbot-apache/certbot_apache/_internal/configurator.py @@ -327,6 +327,9 @@ class ApacheConfigurator(common.Installer): if self.version < (2, 2): raise errors.NotSupportedError( "Apache Version {0} not supported.".format(str(self.version))) + elif self.version < (2, 4): + logger.warning('Support for Apache 2.2 is deprecated and will be removed in a ' + 'future release.') # Recover from previous crash before Augeas initialization to have the # correct parse tree from the get go. diff --git a/certbot-ci/certbot_integration_tests/certbot_tests/__init__.py b/certbot-ci/certbot_integration_tests/certbot_tests/__init__.py index 60c2fcdd8..819cb3e78 100644 --- a/certbot-ci/certbot_integration_tests/certbot_tests/__init__.py +++ b/certbot-ci/certbot_integration_tests/certbot_tests/__init__.py @@ -1,3 +1,4 @@ +# pylint: disable=missing-module-docstring import pytest # Custom assertions defined in the following package need to be registered to be properly diff --git a/certbot-ci/certbot_integration_tests/certbot_tests/context.py b/certbot-ci/certbot_integration_tests/certbot_tests/context.py index e295aefd7..b9854b402 100644 --- a/certbot-ci/certbot_integration_tests/certbot_tests/context.py +++ b/certbot-ci/certbot_integration_tests/certbot_tests/context.py @@ -77,6 +77,6 @@ class IntegrationTestsContext(object): appending the pytest worker id to the subdomain, using this pattern: {subdomain}.{worker_id}.wtf :param subdomain: the subdomain to use in the generated domain (default 'le') - :return: the well-formed domain suitable for redirection on + :return: the well-formed domain suitable for redirection on """ return '{0}.{1}.wtf'.format(subdomain, self.worker_id) 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 a91819180..546f96305 100644 --- a/certbot-ci/certbot_integration_tests/certbot_tests/test_main.py +++ b/certbot-ci/certbot_integration_tests/certbot_tests/test_main.py @@ -29,8 +29,9 @@ from certbot_integration_tests.certbot_tests.assertions import EVERYBODY_SID from certbot_integration_tests.utils import misc -@pytest.fixture() -def context(request): +@pytest.fixture(name='context') +def test_context(request): + # pylint: disable=missing-function-docstring # Fixture request is a built-in pytest fixture describing current test request. integration_test_context = certbot_context.IntegrationTestsContext(request) try: @@ -222,14 +223,16 @@ def test_renew_files_propagate_permissions(context): if os.name != 'nt': os.chmod(privkey1, 0o444) else: - import win32security - import ntsecuritycon + import win32security # pylint: disable=import-error + import ntsecuritycon # pylint: disable=import-error # Get the current DACL of the private key security = win32security.GetFileSecurity(privkey1, win32security.DACL_SECURITY_INFORMATION) dacl = security.GetSecurityDescriptorDacl() # Create a read permission for Everybody group everybody = win32security.ConvertStringSidToSid(EVERYBODY_SID) - dacl.AddAccessAllowedAce(win32security.ACL_REVISION, ntsecuritycon.FILE_GENERIC_READ, everybody) + dacl.AddAccessAllowedAce( + win32security.ACL_REVISION, ntsecuritycon.FILE_GENERIC_READ, everybody + ) # Apply the updated DACL to the private key security.SetSecurityDescriptorDacl(1, dacl, 0) win32security.SetFileSecurity(privkey1, win32security.DACL_SECURITY_INFORMATION, security) @@ -238,12 +241,14 @@ def test_renew_files_propagate_permissions(context): assert_cert_count_for_lineage(context.config_dir, certname, 2) if os.name != 'nt': - # On Linux, read world permissions + all group permissions will be copied from the previous private key + # On Linux, read world permissions + all group permissions + # will be copied from the previous private key assert_world_read_permissions(privkey2) assert_equals_world_read_permissions(privkey1, privkey2) assert_equals_group_permissions(privkey1, privkey2) else: - # On Windows, world will never have any permissions, and group permission is irrelevant for this platform + # On Windows, world will never have any permissions, and + # group permission is irrelevant for this platform assert_world_no_permissions(privkey2) @@ -609,19 +614,22 @@ def test_revoke_multiple_lineages(context): with open(join(context.config_dir, 'renewal', '{0}.conf'.format(cert2)), 'r') as file: data = file.read() - data = re.sub('archive_dir = .*\n', - 'archive_dir = {0}\n'.format(join(context.config_dir, 'archive', cert1).replace('\\', '\\\\')), - data) + data = re.sub( + 'archive_dir = .*\n', + 'archive_dir = {0}\n'.format( + join(context.config_dir, 'archive', cert1).replace('\\', '\\\\') + ), data + ) with open(join(context.config_dir, 'renewal', '{0}.conf'.format(cert2)), 'w') as file: file.write(data) - output = context.certbot([ + context.certbot([ 'revoke', '--cert-path', join(context.config_dir, 'live', cert1, 'cert.pem') ]) with open(join(context.workspace, 'logs', 'letsencrypt.log'), 'r') as f: - assert 'Not deleting revoked certs due to overlapping archive dirs' in f.read() + assert 'Not deleting revoked certificates due to overlapping archive dirs' in f.read() def test_wildcard_certificates(context): diff --git a/certbot-ci/certbot_integration_tests/conftest.py b/certbot-ci/certbot_integration_tests/conftest.py index bb5c07dac..230fb0eda 100644 --- a/certbot-ci/certbot_integration_tests/conftest.py +++ b/certbot-ci/certbot_integration_tests/conftest.py @@ -13,7 +13,6 @@ import sys from certbot_integration_tests.utils import acme_server as acme_lib from certbot_integration_tests.utils import dns_server as dns_lib -from certbot_integration_tests.utils.dns_server import DNSServer def pytest_addoption(parser): @@ -92,8 +91,10 @@ def _setup_primary_node(config): try: subprocess.check_output(['docker-compose', '-v'], stderr=subprocess.STDOUT) except (subprocess.CalledProcessError, OSError): - raise ValueError('Error: docker-compose is required in PATH to launch the integration tests, ' - 'but is not installed or not available for current user.') + raise ValueError( + 'Error: docker-compose is required in PATH to launch the integration tests, ' + 'but is not installed or not available for current user.' + ) # Parameter numprocesses is added to option by pytest-xdist workers = ['primary'] if not config.option.numprocesses\ diff --git a/certbot-ci/certbot_integration_tests/nginx_tests/context.py b/certbot-ci/certbot_integration_tests/nginx_tests/context.py index 3a769840c..6f0f833a0 100644 --- a/certbot-ci/certbot_integration_tests/nginx_tests/context.py +++ b/certbot-ci/certbot_integration_tests/nginx_tests/context.py @@ -1,3 +1,4 @@ +"""Module to handle the context of nginx integration tests.""" import os import subprocess diff --git a/certbot-ci/certbot_integration_tests/nginx_tests/test_main.py b/certbot-ci/certbot_integration_tests/nginx_tests/test_main.py index 1a62ea8d7..8a2d48a50 100644 --- a/certbot-ci/certbot_integration_tests/nginx_tests/test_main.py +++ b/certbot-ci/certbot_integration_tests/nginx_tests/test_main.py @@ -2,13 +2,14 @@ import os import ssl +from typing import List import pytest from certbot_integration_tests.nginx_tests import context as nginx_context -@pytest.fixture() -def context(request): +@pytest.fixture(name='context') +def test_context(request): # Fixture request is a built-in pytest fixture describing current test request. integration_test_context = nginx_context.IntegrationTestsContext(request) try: @@ -27,10 +28,12 @@ def context(request): # No matching server block; default_server does not exist ('nginx5.{0}.wtf', ['--preferred-challenges', 'http'], {'default_server': False}), # Multiple domains, mix of matching and not - ('nginx6.{0}.wtf,nginx7.{0}.wtf', ['--preferred-challenges', 'http'], {'default_server': False}), + ('nginx6.{0}.wtf,nginx7.{0}.wtf', [ + '--preferred-challenges', 'http' + ], {'default_server': False}), ], indirect=['context']) def test_certificate_deployment(certname_pattern, params, context): - # type: (str, list, nginx_context.IntegrationTestsContext) -> None + # type: (str, List[str], nginx_context.IntegrationTestsContext) -> None """ Test various scenarios to deploy a certificate to nginx using certbot. """ @@ -41,7 +44,9 @@ def test_certificate_deployment(certname_pattern, params, context): lineage = domains.split(',')[0] server_cert = ssl.get_server_certificate(('localhost', context.tls_alpn_01_port)) - with open(os.path.join(context.workspace, 'conf/live/{0}/cert.pem'.format(lineage)), 'r') as file: + with open(os.path.join( + context.workspace, 'conf/live/{0}/cert.pem'.format(lineage)), 'r' + ) as file: certbot_cert = file.read() assert server_cert == certbot_cert diff --git a/certbot-ci/certbot_integration_tests/rfc2136_tests/context.py b/certbot-ci/certbot_integration_tests/rfc2136_tests/context.py index b9fe8b401..bdedee1fe 100644 --- a/certbot-ci/certbot_integration_tests/rfc2136_tests/context.py +++ b/certbot-ci/certbot_integration_tests/rfc2136_tests/context.py @@ -1,7 +1,10 @@ -from contextlib import contextmanager -from pytest import skip -from pkg_resources import resource_filename +"""Module to handle the context of RFC2136 integration tests.""" + import tempfile +from contextlib import contextmanager + +from pkg_resources import resource_filename +from pytest import skip from certbot_integration_tests.certbot_tests import context as certbot_context from certbot_integration_tests.utils import certbot_call @@ -33,7 +36,6 @@ class IntegrationTestsContext(certbot_context.IntegrationTestsContext): @contextmanager def rfc2136_credentials(self, label='default'): - # type: (str) -> str """ Produces the contents of a certbot-dns-rfc2136 credentials file. :param str label: which RFC2136 credential to use @@ -52,10 +54,10 @@ class IntegrationTestsContext(certbot_context.IntegrationTestsContext): ) with tempfile.NamedTemporaryFile('w+', prefix='rfc2136-creds-{}'.format(label), - suffix='.ini', dir=self.workspace) as f: - f.write(contents) - f.flush() - yield f.name + suffix='.ini', dir=self.workspace) as fp: + fp.write(contents) + fp.flush() + yield fp.name def skip_if_no_bind9_server(self): """Skips the test if there was no RFC2136-capable DNS server configured diff --git a/certbot-ci/certbot_integration_tests/rfc2136_tests/test_main.py b/certbot-ci/certbot_integration_tests/rfc2136_tests/test_main.py index 69996d533..ae6c0018e 100644 --- a/certbot-ci/certbot_integration_tests/rfc2136_tests/test_main.py +++ b/certbot-ci/certbot_integration_tests/rfc2136_tests/test_main.py @@ -4,8 +4,9 @@ import pytest from certbot_integration_tests.rfc2136_tests import context as rfc2136_context -@pytest.fixture() -def context(request): +@pytest.fixture(name="context") +def pytest_context(request): + # pylint: disable=missing-function-docstring # Fixture request is a built-in pytest fixture describing current test request. integration_test_context = rfc2136_context.IntegrationTestsContext(request) try: diff --git a/certbot-ci/certbot_integration_tests/utils/acme_server.py b/certbot-ci/certbot_integration_tests/utils/acme_server.py index aa501a279..bbbdd196b 100755 --- a/certbot-ci/certbot_integration_tests/utils/acme_server.py +++ b/certbot-ci/certbot_integration_tests/utils/acme_server.py @@ -7,18 +7,19 @@ import errno import json import os from os.path import join -import re import shutil import subprocess import sys import tempfile import time +from typing import List import requests from certbot_integration_tests.utils import misc from certbot_integration_tests.utils import pebble_artifacts from certbot_integration_tests.utils import proxy +# pylint: disable=wildcard-import,unused-wildcard-import from certbot_integration_tests.utils.constants import * @@ -31,10 +32,11 @@ class ACMEServer(object): ACMEServer gives access the acme_xdist parameter, listing the ports and directory url to use for each pytest node. It exposes also start and stop methods in order to start the stack, and stop it with proper resources cleanup. - ACMEServer is also a context manager, and so can be used to ensure ACME server is started/stopped - upon context enter/exit. + ACMEServer is also a context manager, and so can be used to ensure ACME server is + started/stopped upon context enter/exit. """ - def __init__(self, acme_server, nodes, http_proxy=True, stdout=False, dns_server=None): + def __init__(self, acme_server, nodes, http_proxy=True, stdout=False, + dns_server=None, http_01_port=DEFAULT_HTTP_01_PORT): """ Create an ACMEServer instance. :param str acme_server: the type of acme server used (boulder-v1, boulder-v2 or pebble) @@ -42,15 +44,22 @@ class ACMEServer(object): :param bool http_proxy: if False do not start the HTTP proxy :param bool stdout: if True stream all subprocesses stdout to standard stdout :param str dns_server: if set, Pebble/Boulder will use it to resolve domains + :param int http_01_port: port to use for http-01 validation; currently + only supported for pebble without an HTTP proxy """ self._construct_acme_xdist(acme_server, nodes) self._acme_type = 'pebble' if acme_server == 'pebble' else 'boulder' self._proxy = http_proxy self._workspace = tempfile.mkdtemp() - self._processes = [] + self._processes = [] # type: List[subprocess.Popen] self._stdout = sys.stdout if stdout else open(os.devnull, 'w') self._dns_server = dns_server + self._http_01_port = http_01_port + if http_01_port != DEFAULT_HTTP_01_PORT: + if self._acme_type != 'pebble' or self._proxy: + raise ValueError('setting http_01_port is not currently supported ' + 'with boulder or the HTTP proxy') def start(self): """Start the test stack""" @@ -107,26 +116,34 @@ class ACMEServer(object): """Generate and return the acme_xdist dict""" acme_xdist = {'acme_server': acme_server, 'challtestsrv_port': CHALLTESTSRV_PORT} - # Directory and ACME port are set implicitly in the docker-compose.yml files of Boulder/Pebble. + # Directory and ACME port are set implicitly in the docker-compose.yml + # files of Boulder/Pebble. if acme_server == 'pebble': acme_xdist['directory_url'] = PEBBLE_DIRECTORY_URL else: # boulder acme_xdist['directory_url'] = BOULDER_V2_DIRECTORY_URL \ if acme_server == 'boulder-v2' else BOULDER_V1_DIRECTORY_URL - acme_xdist['http_port'] = {node: port for (node, port) - in zip(nodes, range(5200, 5200 + len(nodes)))} - acme_xdist['https_port'] = {node: port for (node, port) - in zip(nodes, range(5100, 5100 + len(nodes)))} - acme_xdist['other_port'] = {node: port for (node, port) - in zip(nodes, range(5300, 5300 + len(nodes)))} + acme_xdist['http_port'] = { + node: port for (node, port) in # pylint: disable=unnecessary-comprehension + zip(nodes, range(5200, 5200 + len(nodes))) + } + acme_xdist['https_port'] = { + node: port for (node, port) in # pylint: disable=unnecessary-comprehension + zip(nodes, range(5100, 5100 + len(nodes))) + } + acme_xdist['other_port'] = { + node: port for (node, port) in # pylint: disable=unnecessary-comprehension + zip(nodes, range(5300, 5300 + len(nodes))) + } self.acme_xdist = acme_xdist def _prepare_pebble_server(self): """Configure and launch the Pebble server""" print('=> Starting pebble instance deployment...') - pebble_path, challtestsrv_path, pebble_config_path = pebble_artifacts.fetch(self._workspace) + pebble_artifacts_rv = pebble_artifacts.fetch(self._workspace, self._http_01_port) + pebble_path, challtestsrv_path, pebble_config_path = pebble_artifacts_rv # Configure Pebble at full speed (PEBBLE_VA_NOSLEEP=1) and not randomly refusing valid # nonce (PEBBLE_WFE_NONCEREJECT=0) to have a stable test environment. @@ -150,9 +167,9 @@ class ACMEServer(object): env=environ) # pebble_ocsp_server is imported here and not at the top of module in order to avoid a - # useless ImportError, in the case where cryptography dependency is too old to support ocsp, - # but Boulder is used instead of Pebble, so pebble_ocsp_server is not used. This is the - # typical situation of integration-certbot-oldest tox testenv. + # useless ImportError, in the case where cryptography dependency is too old to support + # ocsp, but Boulder is used instead of Pebble, so pebble_ocsp_server is not used. This is + # the typical situation of integration-certbot-oldest tox testenv. from certbot_integration_tests.utils import pebble_ocsp_server self._launch_process([sys.executable, pebble_ocsp_server.__file__]) @@ -195,13 +212,16 @@ class ACMEServer(object): if not self._dns_server: # Configure challtestsrv to answer any A record request with ip of the docker host. - response = requests.post('http://localhost:{0}/set-default-ipv4'.format(CHALLTESTSRV_PORT), - json={'ip': '10.77.77.1'}) + response = requests.post('http://localhost:{0}/set-default-ipv4'.format( + CHALLTESTSRV_PORT), json={'ip': '10.77.77.1'} + ) response.raise_for_status() except BaseException: # If we failed to set up boulder, print its logs. print('=> Boulder setup failed. Boulder logs are:') - process = self._launch_process(['docker-compose', 'logs'], cwd=instance_path, force_stderr=True) + process = self._launch_process([ + 'docker-compose', 'logs'], cwd=instance_path, force_stderr=True + ) process.wait() raise @@ -212,7 +232,7 @@ class ACMEServer(object): print('=> Configuring the HTTP proxy...') mapping = {r'.+\.{0}\.wtf'.format(node): 'http://127.0.0.1:{0}'.format(port) for node, port in self.acme_xdist['http_port'].items()} - command = [sys.executable, proxy.__file__, str(HTTP_01_PORT), json.dumps(mapping)] + command = [sys.executable, proxy.__file__, str(DEFAULT_HTTP_01_PORT), json.dumps(mapping)] self._launch_process(command) print('=> Finished configuring the HTTP proxy.') @@ -221,12 +241,15 @@ class ACMEServer(object): if not env: env = os.environ stdout = sys.stderr if force_stderr else self._stdout - process = subprocess.Popen(command, stdout=stdout, stderr=subprocess.STDOUT, cwd=cwd, env=env) + process = subprocess.Popen( + command, stdout=stdout, stderr=subprocess.STDOUT, cwd=cwd, env=env + ) self._processes.append(process) return process def main(): + # pylint: disable=missing-function-docstring parser = argparse.ArgumentParser( description='CLI tool to start a local instance of Pebble or Boulder CA server.') parser.add_argument('--server-type', '-s', @@ -237,9 +260,15 @@ def main(): help='specify the DNS server as `IP:PORT` to use by ' 'Pebble; if not specified, a local mock DNS server will be used to ' 'resolve domains to localhost.') + parser.add_argument('--http-01-port', type=int, default=DEFAULT_HTTP_01_PORT, + help='specify the port to use for http-01 validation; ' + 'this is currently only supported for Pebble.') args = parser.parse_args() - acme_server = ACMEServer(args.server_type, [], http_proxy=False, stdout=True, dns_server=args.dns_server) + acme_server = ACMEServer( + args.server_type, [], http_proxy=False, stdout=True, + dns_server=args.dns_server, http_01_port=args.http_01_port, + ) try: with acme_server as acme_xdist: diff --git a/certbot-ci/certbot_integration_tests/utils/certbot_call.py b/certbot-ci/certbot_integration_tests/utils/certbot_call.py index 2ddaa41c8..c9e46cdc7 100755 --- a/certbot-ci/certbot_integration_tests/utils/certbot_call.py +++ b/certbot-ci/certbot_integration_tests/utils/certbot_call.py @@ -2,12 +2,13 @@ """Module to call certbot in test mode""" from __future__ import absolute_import -from distutils.version import LooseVersion import os import subprocess import sys +from distutils.version import LooseVersion import certbot_integration_tests +# pylint: disable=wildcard-import,unused-wildcard-import from certbot_integration_tests.utils.constants import * @@ -35,6 +36,8 @@ def certbot_test(certbot_args, directory_url, http_01_port, tls_alpn_01_port, def _prepare_environ(workspace): + # pylint: disable=missing-function-docstring + new_environ = os.environ.copy() new_environ['TMPDIR'] = workspace @@ -58,8 +61,13 @@ def _prepare_environ(workspace): # certbot_integration_tests.__file__ is: # '/path/to/certbot/certbot-ci/certbot_integration_tests/__init__.pyc' # ... and we want '/path/to/certbot' - certbot_root = os.path.dirname(os.path.dirname(os.path.dirname(certbot_integration_tests.__file__))) - python_paths = [path for path in new_environ['PYTHONPATH'].split(':') if path != certbot_root] + certbot_root = os.path.dirname(os.path.dirname( + os.path.dirname(certbot_integration_tests.__file__)) + ) + python_paths = [ + path for path in new_environ['PYTHONPATH'].split(':') + if path != certbot_root + ] new_environ['PYTHONPATH'] = ':'.join(python_paths) return new_environ @@ -70,7 +78,8 @@ def _compute_additional_args(workspace, environ, force_renew): output = subprocess.check_output(['certbot', '--version'], universal_newlines=True, stderr=subprocess.STDOUT, cwd=workspace, env=environ) - version_str = output.split(' ')[1].strip() # Typical response is: output = 'certbot 0.31.0.dev0' + # Typical response is: output = 'certbot 0.31.0.dev0' + version_str = output.split(' ')[1].strip() if LooseVersion(version_str) >= LooseVersion('0.30.0'): additional_args.append('--no-random-sleep-on-renew') @@ -113,11 +122,12 @@ def _prepare_args_env(certbot_args, directory_url, http_01_port, tls_alpn_01_por def main(): + # pylint: disable=missing-function-docstring args = sys.argv[1:] # Default config is pebble directory_url = os.environ.get('SERVER', PEBBLE_DIRECTORY_URL) - http_01_port = int(os.environ.get('HTTP_01_PORT', HTTP_01_PORT)) + http_01_port = int(os.environ.get('HTTP_01_PORT', DEFAULT_HTTP_01_PORT)) tls_alpn_01_port = int(os.environ.get('TLS_ALPN_01_PORT', TLS_ALPN_01_PORT)) # Execution of certbot in a self-contained workspace diff --git a/certbot-ci/certbot_integration_tests/utils/constants.py b/certbot-ci/certbot_integration_tests/utils/constants.py index 8b002478e..b02c434db 100644 --- a/certbot-ci/certbot_integration_tests/utils/constants.py +++ b/certbot-ci/certbot_integration_tests/utils/constants.py @@ -1,5 +1,5 @@ """Some useful constants to use throughout certbot-ci integration tests""" -HTTP_01_PORT = 5002 +DEFAULT_HTTP_01_PORT = 5002 TLS_ALPN_01_PORT = 5001 CHALLTESTSRV_PORT = 8055 BOULDER_V1_DIRECTORY_URL = 'http://localhost:4000/directory' @@ -7,4 +7,4 @@ BOULDER_V2_DIRECTORY_URL = 'http://localhost:4001/directory' PEBBLE_DIRECTORY_URL = 'https://localhost:14000/dir' PEBBLE_MANAGEMENT_URL = 'https://localhost:15000' MOCK_OCSP_SERVER_PORT = 4002 -PEBBLE_ALTERNATE_ROOTS = 2 \ No newline at end of file +PEBBLE_ALTERNATE_ROOTS = 2 diff --git a/certbot-ci/certbot_integration_tests/utils/dns_server.py b/certbot-ci/certbot_integration_tests/utils/dns_server.py index 779d736e3..416f6567e 100644 --- a/certbot-ci/certbot_integration_tests/utils/dns_server.py +++ b/certbot-ci/certbot_integration_tests/utils/dns_server.py @@ -4,7 +4,6 @@ from __future__ import print_function import os import os.path -from pkg_resources import resource_filename import shutil import socket import subprocess @@ -12,13 +11,14 @@ import sys import tempfile import time +from pkg_resources import resource_filename -BIND_DOCKER_IMAGE = 'internetsystemsconsortium/bind9:9.16' -BIND_BIND_ADDRESS = ('127.0.0.1', 45953) +BIND_DOCKER_IMAGE = "internetsystemsconsortium/bind9:9.16" +BIND_BIND_ADDRESS = ("127.0.0.1", 45953) # A TCP DNS message which is a query for '. CH A' transaction ID 0xcb37. This is used # by _wait_until_ready to check that BIND is responding without depending on dnspython. -BIND_TEST_QUERY = bytearray.fromhex('0011cb37000000010000000000000000010003') +BIND_TEST_QUERY = bytearray.fromhex("0011cb37000000010000000000000000010003") class DNSServer(object): @@ -31,7 +31,7 @@ class DNSServer(object): future to support parallelization (https://github.com/certbot/certbot/issues/8455). """ - def __init__(self, nodes, show_output=False): + def __init__(self, unused_nodes, show_output=False): """ Create an DNSServer instance. :param list nodes: list of node names that will be setup by pytest xdist @@ -40,16 +40,13 @@ class DNSServer(object): self.bind_root = tempfile.mkdtemp() - self.process = None + self.process = None # type: subprocess.Popen - self.dns_xdist = { - 'address': BIND_BIND_ADDRESS[0], - 'port': BIND_BIND_ADDRESS[1] - } + self.dns_xdist = {"address": BIND_BIND_ADDRESS[0], "port": BIND_BIND_ADDRESS[1]} # Unfortunately the BIND9 image forces everything to stderr with -g and we can't # modify the verbosity. - self._output = sys.stderr if show_output else open(os.devnull, 'w') + self._output = sys.stderr if show_output else open(os.devnull, "w") def start(self): """Start the DNS server""" @@ -63,11 +60,11 @@ class DNSServer(object): def stop(self): """Stop the DNS server, and clean its resources""" if self.process: - try: - self.process.terminate() - self.process.wait() - except BaseException as e: - print("BIND9 did not stop cleanly: {}".format(e), file=sys.stderr) + try: + self.process.terminate() + self.process.wait() + except BaseException as e: + print("BIND9 did not stop cleanly: {}".format(e), file=sys.stderr) shutil.rmtree(self.bind_root, ignore_errors=True) @@ -76,65 +73,79 @@ class DNSServer(object): def _configure_bind(self): """Configure the BIND9 server based on the prebaked configuration""" - bind_conf_src = resource_filename('certbot_integration_tests', 'assets/bind-config') - for dir in ('conf', 'zones'): - shutil.copytree(os.path.join(bind_conf_src, dir), os.path.join(self.bind_root, dir)) + bind_conf_src = resource_filename( + "certbot_integration_tests", "assets/bind-config" + ) + for directory in ("conf", "zones"): + shutil.copytree( + os.path.join(bind_conf_src, directory), os.path.join(self.bind_root, directory) + ) def _start_bind(self): """Launch the BIND9 server as a Docker container""" - addr_str = '{}:{}'.format(BIND_BIND_ADDRESS[0], BIND_BIND_ADDRESS[1]) - self.process = subprocess.Popen([ - 'docker', 'run', '--rm', - '-p', '{}:53/udp'.format(addr_str), - '-p', '{}:53/tcp'.format(addr_str), - '-v', '{}/conf:/etc/bind'.format(self.bind_root), - '-v', '{}/zones:/var/lib/bind'.format(self.bind_root), - BIND_DOCKER_IMAGE - ], stdout=self._output, stderr=self._output) + addr_str = "{}:{}".format(BIND_BIND_ADDRESS[0], BIND_BIND_ADDRESS[1]) + self.process = subprocess.Popen( + [ + "docker", + "run", + "--rm", + "-p", + "{}:53/udp".format(addr_str), + "-p", + "{}:53/tcp".format(addr_str), + "-v", + "{}/conf:/etc/bind".format(self.bind_root), + "-v", + "{}/zones:/var/lib/bind".format(self.bind_root), + BIND_DOCKER_IMAGE, + ], + stdout=self._output, + stderr=self._output, + ) if self.process.poll(): - raise("BIND9 server stopped unexpectedly") + raise ValueError("BIND9 server stopped unexpectedly") try: - self._wait_until_ready() + self._wait_until_ready() except: - # The container might be running even if we think it isn't - self.stop() - raise + # The container might be running even if we think it isn't + self.stop() + raise def _wait_until_ready(self, attempts=30): - # type: (int) -> None - """ - Polls the DNS server over TCP until it gets a response, or until - it runs out of attempts and raises a ValueError. - The DNS response message must match the txn_id of the DNS query message, - but otherwise the contents are ignored. - :param int attempts: The number of attempts to make. - """ - for _ in range(attempts): - if self.process.poll(): - raise ValueError('BIND9 server stopped unexpectedly') + # type: (int) -> None + """ + Polls the DNS server over TCP until it gets a response, or until + it runs out of attempts and raises a ValueError. + The DNS response message must match the txn_id of the DNS query message, + but otherwise the contents are ignored. + :param int attempts: The number of attempts to make. + """ + for _ in range(attempts): + if self.process.poll(): + raise ValueError("BIND9 server stopped unexpectedly") - sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - sock.settimeout(5.0) - try: - sock.connect(BIND_BIND_ADDRESS) - sock.sendall(BIND_TEST_QUERY) - buf = sock.recv(1024) - # We should receive a DNS message with the same tx_id - if buf and len(buf) > 4 and buf[2:4] == BIND_TEST_QUERY[2:4]: - return - # If we got a response but it wasn't the one we wanted, wait a little - time.sleep(1) - except: - # If there was a network error, wait a little - time.sleep(1) - pass - finally: - sock.close() + sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + sock.settimeout(5.0) + try: + sock.connect(BIND_BIND_ADDRESS) + sock.sendall(BIND_TEST_QUERY) + buf = sock.recv(1024) + # We should receive a DNS message with the same tx_id + if buf and len(buf) > 4 and buf[2:4] == BIND_TEST_QUERY[2:4]: + return + # If we got a response but it wasn't the one we wanted, wait a little + time.sleep(1) + except: # pylint: disable=bare-except + # If there was a network error, wait a little + time.sleep(1) + finally: + sock.close() - raise ValueError( - 'Gave up waiting for DNS server {} to respond'.format(BIND_BIND_ADDRESS)) + raise ValueError( + "Gave up waiting for DNS server {} to respond".format(BIND_BIND_ADDRESS) + ) def __enter__(self): self.start() diff --git a/certbot-ci/certbot_integration_tests/utils/misc.py b/certbot-ci/certbot_integration_tests/utils/misc.py index d83f276ef..799b079fe 100644 --- a/certbot-ci/certbot_integration_tests/utils/misc.py +++ b/certbot-ci/certbot_integration_tests/utils/misc.py @@ -39,6 +39,7 @@ def _suppress_x509_verification_warnings(): urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) except ImportError: # Handle old versions of request with vendorized urllib3 + # pylint: disable=no-member from requests.packages.urllib3.exceptions import InsecureRequestWarning requests.packages.urllib3.disable_warnings(InsecureRequestWarning) @@ -256,7 +257,8 @@ def generate_csr(domains, key_path, csr_path, key_type=RSA_KEY_TYPE): def read_certificate(cert_path): """ - Load the certificate from the provided path, and return a human readable version of it (TEXT mode). + Load the certificate from the provided path, and return a human readable version + of it (TEXT mode). :param str cert_path: the path to the certificate :returns: the TEXT version of the certificate, as it would be displayed by openssl binary """ diff --git a/certbot-ci/certbot_integration_tests/utils/pebble_artifacts.py b/certbot-ci/certbot_integration_tests/utils/pebble_artifacts.py index 7fe03b990..cd62e1a7f 100644 --- a/certbot-ci/certbot_integration_tests/utils/pebble_artifacts.py +++ b/certbot-ci/certbot_integration_tests/utils/pebble_artifacts.py @@ -1,3 +1,5 @@ +# pylint: disable=missing-module-docstring + import json import os import stat @@ -5,18 +7,19 @@ import stat import pkg_resources import requests -from certbot_integration_tests.utils.constants import MOCK_OCSP_SERVER_PORT +from certbot_integration_tests.utils.constants import DEFAULT_HTTP_01_PORT, MOCK_OCSP_SERVER_PORT PEBBLE_VERSION = 'v2.3.0' ASSETS_PATH = pkg_resources.resource_filename('certbot_integration_tests', 'assets') -def fetch(workspace): +def fetch(workspace, http_01_port=DEFAULT_HTTP_01_PORT): + # pylint: disable=missing-function-docstring suffix = 'linux-amd64' if os.name != 'nt' else 'windows-amd64.exe' pebble_path = _fetch_asset('pebble', suffix) challtestsrv_path = _fetch_asset('pebble-challtestsrv', suffix) - pebble_config_path = _build_pebble_config(workspace) + pebble_config_path = _build_pebble_config(workspace, http_01_port) return pebble_path, challtestsrv_path, pebble_config_path @@ -35,7 +38,7 @@ def _fetch_asset(asset, suffix): return asset_path -def _build_pebble_config(workspace): +def _build_pebble_config(workspace, http_01_port): config_path = os.path.join(workspace, 'pebble-config.json') with open(config_path, 'w') as file_h: file_h.write(json.dumps({ @@ -44,7 +47,7 @@ def _build_pebble_config(workspace): 'managementListenAddress': '0.0.0.0:15000', 'certificate': os.path.join(ASSETS_PATH, 'cert.pem'), 'privateKey': os.path.join(ASSETS_PATH, 'key.pem'), - 'httpPort': 5002, + 'httpPort': http_01_port, 'tlsPort': 5001, 'ocspResponderURL': 'http://127.0.0.1:{0}'.format(MOCK_OCSP_SERVER_PORT), }, diff --git a/certbot-ci/certbot_integration_tests/utils/pebble_ocsp_server.py b/certbot-ci/certbot_integration_tests/utils/pebble_ocsp_server.py index 9458560e8..b86e1cbc9 100755 --- a/certbot-ci/certbot_integration_tests/utils/pebble_ocsp_server.py +++ b/certbot-ci/certbot_integration_tests/utils/pebble_ocsp_server.py @@ -21,6 +21,7 @@ from certbot_integration_tests.utils.misc import GracefulTCPServer class _ProxyHandler(BaseHTTPServer.BaseHTTPRequestHandler): + # pylint: disable=missing-function-docstring def do_POST(self): request = requests.get(PEBBLE_MANAGEMENT_URL + '/intermediate-keys/0', verify=False) issuer_key = serialization.load_pem_private_key(request.content, None, default_backend()) @@ -35,20 +36,28 @@ class _ProxyHandler(BaseHTTPServer.BaseHTTPRequestHandler): ocsp_request = ocsp.load_der_ocsp_request(self.rfile.read(content_len)) response = requests.get('{0}/cert-status-by-serial/{1}'.format( - PEBBLE_MANAGEMENT_URL, str(hex(ocsp_request.serial_number)).replace('0x', '')), verify=False) + PEBBLE_MANAGEMENT_URL, str(hex(ocsp_request.serial_number)).replace('0x', '')), + verify=False + ) if not response.ok: - ocsp_response = ocsp.OCSPResponseBuilder.build_unsuccessful(ocsp.OCSPResponseStatus.UNAUTHORIZED) + ocsp_response = ocsp.OCSPResponseBuilder.build_unsuccessful( + ocsp.OCSPResponseStatus.UNAUTHORIZED + ) else: data = response.json() now = datetime.datetime.utcnow() cert = x509.load_pem_x509_certificate(data['Certificate'].encode(), default_backend()) if data['Status'] != 'Revoked': - ocsp_status, revocation_time, revocation_reason = ocsp.OCSPCertStatus.GOOD, None, None + ocsp_status = ocsp.OCSPCertStatus.GOOD + revocation_time = None + revocation_reason = None else: - ocsp_status, revocation_reason = ocsp.OCSPCertStatus.REVOKED, x509.ReasonFlags.unspecified - revoked_at = re.sub(r'( \+\d{4}).*$', r'\1', data['RevokedAt']) # "... +0000 UTC" => "+0000" + ocsp_status = ocsp.OCSPCertStatus.REVOKED + revocation_reason = x509.ReasonFlags.unspecified + # "... +0000 UTC" => "+0000" + revoked_at = re.sub(r'( \+\d{4}).*$', r'\1', data['RevokedAt']) revocation_time = parser.parse(revoked_at) ocsp_response = ocsp.OCSPResponseBuilder().add_response( diff --git a/certbot-ci/certbot_integration_tests/utils/proxy.py b/certbot-ci/certbot_integration_tests/utils/proxy.py index 3a16adebf..225f98e6e 100644 --- a/certbot-ci/certbot_integration_tests/utils/proxy.py +++ b/certbot-ci/certbot_integration_tests/utils/proxy.py @@ -1,4 +1,6 @@ #!/usr/bin/env python +# pylint: disable=missing-module-docstring + import json import re import sys @@ -10,7 +12,9 @@ from certbot_integration_tests.utils.misc import GracefulTCPServer def _create_proxy(mapping): + # pylint: disable=missing-function-docstring class ProxyHandler(BaseHTTPServer.BaseHTTPRequestHandler): + # pylint: disable=missing-class-docstring def do_GET(self): headers = {key.lower(): value for key, value in self.headers.items()} backend = [backend for pattern, backend in mapping.items() diff --git a/certbot-ci/setup.py b/certbot-ci/setup.py index ce29fe45d..4d4557939 100644 --- a/certbot-ci/setup.py +++ b/certbot-ci/setup.py @@ -18,7 +18,7 @@ install_requires = [ 'python-dateutil', 'pyyaml', 'requests', - 'six', + 'six' ] # Add pywin32 on Windows platforms to handle low-level system calls. diff --git a/certbot-dns-cloudflare/certbot_dns_cloudflare/__init__.py b/certbot-dns-cloudflare/certbot_dns_cloudflare/__init__.py index d59862a3c..81c053c04 100644 --- a/certbot-dns-cloudflare/certbot_dns_cloudflare/__init__.py +++ b/certbot-dns-cloudflare/certbot_dns_cloudflare/__init__.py @@ -3,6 +3,10 @@ The `~certbot_dns_cloudflare.dns_cloudflare` plugin automates the process of completing a ``dns-01`` challenge (`~acme.challenges.DNS01`) by creating, and subsequently removing, TXT records using the Cloudflare API. +.. note:: + The plugin is not installed by default. It can be installed by heading to + `certbot.eff.org `_, choosing your system and + selecting the Wildcard tab. Named Arguments --------------- diff --git a/certbot-dns-cloudxns/certbot_dns_cloudxns/__init__.py b/certbot-dns-cloudxns/certbot_dns_cloudxns/__init__.py index 6ddbdfe5a..0d098445c 100644 --- a/certbot-dns-cloudxns/certbot_dns_cloudxns/__init__.py +++ b/certbot-dns-cloudxns/certbot_dns_cloudxns/__init__.py @@ -3,6 +3,10 @@ The `~certbot_dns_cloudxns.dns_cloudxns` plugin automates the process of completing a ``dns-01`` challenge (`~acme.challenges.DNS01`) by creating, and subsequently removing, TXT records using the CloudXNS API. +.. note:: + The plugin is not installed by default. It can be installed by heading to + `certbot.eff.org `_, choosing your system and + selecting the Wildcard tab. Named Arguments --------------- diff --git a/certbot-dns-digitalocean/certbot_dns_digitalocean/__init__.py b/certbot-dns-digitalocean/certbot_dns_digitalocean/__init__.py index 3ab8df041..2cb7a92de 100644 --- a/certbot-dns-digitalocean/certbot_dns_digitalocean/__init__.py +++ b/certbot-dns-digitalocean/certbot_dns_digitalocean/__init__.py @@ -3,6 +3,10 @@ The `~certbot_dns_digitalocean.dns_digitalocean` plugin automates the process of completing a ``dns-01`` challenge (`~acme.challenges.DNS01`) by creating, and subsequently removing, TXT records using the DigitalOcean API. +.. note:: + The plugin is not installed by default. It can be installed by heading to + `certbot.eff.org `_, choosing your system and + selecting the Wildcard tab. Named Arguments --------------- diff --git a/certbot-dns-digitalocean/certbot_dns_digitalocean/_internal/dns_digitalocean.py b/certbot-dns-digitalocean/certbot_dns_digitalocean/_internal/dns_digitalocean.py index 75e25a848..e0c9561a2 100644 --- a/certbot-dns-digitalocean/certbot_dns_digitalocean/_internal/dns_digitalocean.py +++ b/certbot-dns-digitalocean/certbot_dns_digitalocean/_internal/dns_digitalocean.py @@ -19,7 +19,8 @@ class Authenticator(dns_common.DNSAuthenticator): This Authenticator uses the DigitalOcean API to fulfill a dns-01 challenge. """ - description = 'Obtain certs using a DNS TXT record (if you are using DigitalOcean for DNS).' + description = 'Obtain certificates using a DNS TXT record (if you are ' + \ + 'using DigitalOcean for DNS).' def __init__(self, *args, **kwargs): super(Authenticator, self).__init__(*args, **kwargs) diff --git a/certbot-dns-dnsimple/certbot_dns_dnsimple/__init__.py b/certbot-dns-dnsimple/certbot_dns_dnsimple/__init__.py index f8a2e83aa..0f6168a13 100644 --- a/certbot-dns-dnsimple/certbot_dns_dnsimple/__init__.py +++ b/certbot-dns-dnsimple/certbot_dns_dnsimple/__init__.py @@ -3,6 +3,10 @@ The `~certbot_dns_dnsimple.dns_dnsimple` plugin automates the process of completing a ``dns-01`` challenge (`~acme.challenges.DNS01`) by creating, and subsequently removing, TXT records using the DNSimple API. +.. note:: + The plugin is not installed by default. It can be installed by heading to + `certbot.eff.org `_, choosing your system and + selecting the Wildcard tab. Named Arguments --------------- diff --git a/certbot-dns-dnsmadeeasy/certbot_dns_dnsmadeeasy/__init__.py b/certbot-dns-dnsmadeeasy/certbot_dns_dnsmadeeasy/__init__.py index 52f055237..fa49ee516 100644 --- a/certbot-dns-dnsmadeeasy/certbot_dns_dnsmadeeasy/__init__.py +++ b/certbot-dns-dnsmadeeasy/certbot_dns_dnsmadeeasy/__init__.py @@ -3,6 +3,10 @@ The `~certbot_dns_dnsmadeeasy.dns_dnsmadeeasy` plugin automates the process of completing a ``dns-01`` challenge (`~acme.challenges.DNS01`) by creating, and subsequently removing, TXT records using the DNS Made Easy API. +.. note:: + The plugin is not installed by default. It can be installed by heading to + `certbot.eff.org `_, choosing your system and + selecting the Wildcard tab. Named Arguments --------------- diff --git a/certbot-dns-gehirn/certbot_dns_gehirn/__init__.py b/certbot-dns-gehirn/certbot_dns_gehirn/__init__.py index fdcb8cd48..fd81d0712 100644 --- a/certbot-dns-gehirn/certbot_dns_gehirn/__init__.py +++ b/certbot-dns-gehirn/certbot_dns_gehirn/__init__.py @@ -3,6 +3,10 @@ The `~certbot_dns_gehirn.dns_gehirn` plugin automates the process of completing a ``dns-01`` challenge (`~acme.challenges.DNS01`) by creating, and subsequently removing, TXT records using the Gehirn Infrastructure Service DNS API. +.. note:: + The plugin is not installed by default. It can be installed by heading to + `certbot.eff.org `_, choosing your system and + selecting the Wildcard tab. Named Arguments --------------- diff --git a/certbot-dns-google/certbot_dns_google/__init__.py b/certbot-dns-google/certbot_dns_google/__init__.py index b88260b07..2d448c590 100644 --- a/certbot-dns-google/certbot_dns_google/__init__.py +++ b/certbot-dns-google/certbot_dns_google/__init__.py @@ -3,6 +3,10 @@ The `~certbot_dns_google.dns_google` plugin automates the process of completing a ``dns-01`` challenge (`~acme.challenges.DNS01`) by creating, and subsequently removing, TXT records using the Google Cloud DNS API. +.. note:: + The plugin is not installed by default. It can be installed by heading to + `certbot.eff.org `_, choosing your system and + selecting the Wildcard tab. Named Arguments --------------- diff --git a/certbot-dns-google/certbot_dns_google/_internal/dns_google.py b/certbot-dns-google/certbot_dns_google/_internal/dns_google.py index 1bd3468da..4b0d91463 100644 --- a/certbot-dns-google/certbot_dns_google/_internal/dns_google.py +++ b/certbot-dns-google/certbot_dns_google/_internal/dns_google.py @@ -118,10 +118,13 @@ class _GoogleClient(object): record_contents = self.get_existing_txt_rrset(zone_id, record_name) if record_contents is None: - record_contents = [] - add_records = record_contents[:] + # If it wasn't possible to fetch the records at this label (missing .list permission), + # assume there aren't any (#5678). If there are actually records here, this will fail + # with HTTP 409/412 API errors. + record_contents = {"rrdatas": []} + add_records = record_contents["rrdatas"][:] - if "\""+record_content+"\"" in record_contents: + if "\""+record_content+"\"" in record_contents["rrdatas"]: # The process was interrupted previously and validation token exists return @@ -140,15 +143,15 @@ class _GoogleClient(object): ], } - if record_contents: + if record_contents["rrdatas"]: # We need to remove old records in the same request data["deletions"] = [ { "kind": "dns#resourceRecordSet", "type": "TXT", "name": record_name + ".", - "rrdatas": record_contents, - "ttl": record_ttl, + "rrdatas": record_contents["rrdatas"], + "ttl": record_contents["ttl"], }, ] @@ -188,7 +191,10 @@ class _GoogleClient(object): record_contents = self.get_existing_txt_rrset(zone_id, record_name) if record_contents is None: - record_contents = ["\"" + record_content + "\""] + # If it wasn't possible to fetch the records at this label (missing .list permission), + # assume there aren't any (#5678). If there are actually records here, this will fail + # with HTTP 409/412 API errors. + record_contents = {"rrdatas": ["\"" + record_content + "\""], "ttl": record_ttl} data = { "kind": "dns#change", @@ -197,14 +203,15 @@ class _GoogleClient(object): "kind": "dns#resourceRecordSet", "type": "TXT", "name": record_name + ".", - "rrdatas": record_contents, - "ttl": record_ttl, + "rrdatas": record_contents["rrdatas"], + "ttl": record_contents["ttl"], }, ], } # Remove the record being deleted from the list - readd_contents = [r for r in record_contents if r != "\"" + record_content + "\""] + readd_contents = [r for r in record_contents["rrdatas"] + if r != "\"" + record_content + "\""] if readd_contents: # We need to remove old records in the same request data["additions"] = [ @@ -213,7 +220,7 @@ class _GoogleClient(object): "type": "TXT", "name": record_name + ".", "rrdatas": readd_contents, - "ttl": record_ttl, + "ttl": record_contents["ttl"], }, ] @@ -235,14 +242,15 @@ class _GoogleClient(object): :param str zone_id: The ID of the managed zone. :param str record_name: The record name (typically beginning with '_acme-challenge.'). - :returns: List of TXT record values or None - :rtype: `list` of `string` or `None` + :returns: The resourceRecordSet corresponding to `record_name` or None + :rtype: `resourceRecordSet ` or `None` # pylint: disable=line-too-long """ rrs_request = self.dns.resourceRecordSets() - request = rrs_request.list(managedZone=zone_id, project=self.project_id) # Add dot as the API returns absolute domains record_name += "." + request = rrs_request.list(project=self.project_id, managedZone=zone_id, name=record_name, + type="TXT") try: response = request.execute() except googleapiclient_errors.Error: @@ -250,10 +258,8 @@ class _GoogleClient(object): "requesting a wildcard certificate, this might not work.") logger.debug("Error was:", exc_info=True) else: - if response: - for rr in response["rrsets"]: - if rr["name"] == record_name and rr["type"] == "TXT": - return rr["rrdatas"] + if response and response["rrsets"]: + return response["rrsets"][0] return None def _find_managed_zone_id(self, domain): diff --git a/certbot-dns-google/tests/dns_google_test.py b/certbot-dns-google/tests/dns_google_test.py index 40002f143..396a6c8bd 100644 --- a/certbot-dns-google/tests/dns_google_test.py +++ b/certbot-dns-google/tests/dns_google_test.py @@ -70,7 +70,7 @@ class GoogleClientTest(unittest.TestCase): zone = "ZONE_ID" change = "an-id" - def _setUp_client_with_mock(self, zone_request_side_effect): + def _setUp_client_with_mock(self, zone_request_side_effect, rrs_list_side_effect=None): from certbot_dns_google._internal.dns_google import _GoogleClient pwd = os.path.dirname(__file__) @@ -86,9 +86,16 @@ class GoogleClientTest(unittest.TestCase): mock_mz.list.return_value.execute.side_effect = zone_request_side_effect mock_rrs = mock.MagicMock() - rrsets = {"rrsets": [{"name": "_acme-challenge.example.org.", "type": "TXT", - "rrdatas": ["\"example-txt-contents\""]}]} - mock_rrs.list.return_value.execute.return_value = rrsets + def rrs_list(project=None, managedZone=None, name=None, type=None): + response = {"rrsets": []} + if name == "_acme-challenge.example.org.": + response = {"rrsets": [{"name": "_acme-challenge.example.org.", "type": "TXT", + "rrdatas": ["\"example-txt-contents\""], "ttl": 60}]} + mock_return = mock.MagicMock() + mock_return.execute.return_value = response + mock_return.execute.side_effect = rrs_list_side_effect + return mock_return + mock_rrs.list.side_effect = rrs_list mock_changes = mock.MagicMock() client.dns.managedZones = mock.MagicMock(return_value=mock_mz) @@ -173,11 +180,29 @@ class GoogleClientTest(unittest.TestCase): # pylint: disable=line-too-long mock_get_rrs = "certbot_dns_google._internal.dns_google._GoogleClient.get_existing_txt_rrset" with mock.patch(mock_get_rrs) as mock_rrs: - mock_rrs.return_value = ["sample-txt-contents"] + mock_rrs.return_value = {"rrdatas": ["sample-txt-contents"], "ttl": self.record_ttl} client.add_txt_record(DOMAIN, self.record_name, self.record_content, self.record_ttl) self.assertTrue(changes.create.called) - self.assertTrue("sample-txt-contents" in - changes.create.call_args_list[0][1]["body"]["deletions"][0]["rrdatas"]) + deletions = changes.create.call_args_list[0][1]["body"]["deletions"][0] + self.assertTrue("sample-txt-contents" in deletions["rrdatas"]) + self.assertEqual(self.record_ttl, deletions["ttl"]) + + @mock.patch('oauth2client.service_account.ServiceAccountCredentials.from_json_keyfile_name') + @mock.patch('certbot_dns_google._internal.dns_google.open', + mock.mock_open(read_data='{"project_id": "' + PROJECT_ID + '"}'), create=True) + def test_add_txt_record_delete_old_ttl_case(self, unused_credential_mock): + client, changes = self._setUp_client_with_mock( + [{'managedZones': [{'id': self.zone}]}]) + # pylint: disable=line-too-long + mock_get_rrs = "certbot_dns_google._internal.dns_google._GoogleClient.get_existing_txt_rrset" + with mock.patch(mock_get_rrs) as mock_rrs: + custom_ttl = 300 + mock_rrs.return_value = {"rrdatas": ["sample-txt-contents"], "ttl": custom_ttl} + client.add_txt_record(DOMAIN, self.record_name, self.record_content, self.record_ttl) + self.assertTrue(changes.create.called) + deletions = changes.create.call_args_list[0][1]["body"]["deletions"][0] + self.assertTrue("sample-txt-contents" in deletions["rrdatas"]) + self.assertEqual(custom_ttl, deletions["ttl"]) #otherwise HTTP 412 @mock.patch('oauth2client.service_account.ServiceAccountCredentials.from_json_keyfile_name') @mock.patch('certbot_dns_google._internal.dns_google.open', @@ -221,14 +246,13 @@ class GoogleClientTest(unittest.TestCase): @mock.patch('oauth2client.service_account.ServiceAccountCredentials.from_json_keyfile_name') @mock.patch('certbot_dns_google._internal.dns_google.open', mock.mock_open(read_data='{"project_id": "' + PROJECT_ID + '"}'), create=True) - def test_del_txt_record(self, unused_credential_mock): + def test_del_txt_record_multi_rrdatas(self, unused_credential_mock): client, changes = self._setUp_client_with_mock([{'managedZones': [{'id': self.zone}]}]) - # pylint: disable=line-too-long mock_get_rrs = "certbot_dns_google._internal.dns_google._GoogleClient.get_existing_txt_rrset" with mock.patch(mock_get_rrs) as mock_rrs: - mock_rrs.return_value = ["\"sample-txt-contents\"", - "\"example-txt-contents\""] + mock_rrs.return_value = {"rrdatas": ["\"sample-txt-contents\"", + "\"example-txt-contents\""], "ttl": self.record_ttl} client.del_txt_record(DOMAIN, "_acme-challenge.example.org", "example-txt-contents", self.record_ttl) @@ -261,19 +285,48 @@ class GoogleClientTest(unittest.TestCase): @mock.patch('oauth2client.service_account.ServiceAccountCredentials.from_json_keyfile_name') @mock.patch('certbot_dns_google._internal.dns_google.open', mock.mock_open(read_data='{"project_id": "' + PROJECT_ID + '"}'), create=True) - def test_del_txt_record_error_during_zone_lookup(self, unused_credential_mock): - client, unused_changes = self._setUp_client_with_mock(API_ERROR) + def test_del_txt_record_single_rrdatas(self, unused_credential_mock): + client, changes = self._setUp_client_with_mock([{'managedZones': [{'id': self.zone}]}]) + # pylint: disable=line-too-long + mock_get_rrs = "certbot_dns_google._internal.dns_google._GoogleClient.get_existing_txt_rrset" + with mock.patch(mock_get_rrs) as mock_rrs: + mock_rrs.return_value = {"rrdatas": ["\"example-txt-contents\""], "ttl": self.record_ttl} + client.del_txt_record(DOMAIN, "_acme-challenge.example.org", + "example-txt-contents", self.record_ttl) + expected_body = { + "kind": "dns#change", + "deletions": [ + { + "kind": "dns#resourceRecordSet", + "type": "TXT", + "name": "_acme-challenge.example.org.", + "rrdatas": ["\"example-txt-contents\""], + "ttl": self.record_ttl, + }, + ], + } + + changes.create.assert_called_with(body=expected_body, + managedZone=self.zone, + project=PROJECT_ID) + + @mock.patch('oauth2client.service_account.ServiceAccountCredentials.from_json_keyfile_name') + @mock.patch('certbot_dns_google._internal.dns_google.open', + mock.mock_open(read_data='{"project_id": "' + PROJECT_ID + '"}'), create=True) + def test_del_txt_record_error_during_zone_lookup(self, unused_credential_mock): + client, changes = self._setUp_client_with_mock(API_ERROR) client.del_txt_record(DOMAIN, self.record_name, self.record_content, self.record_ttl) + changes.create.assert_not_called() @mock.patch('oauth2client.service_account.ServiceAccountCredentials.from_json_keyfile_name') @mock.patch('certbot_dns_google._internal.dns_google.open', mock.mock_open(read_data='{"project_id": "' + PROJECT_ID + '"}'), create=True) def test_del_txt_record_zone_not_found(self, unused_credential_mock): - client, unused_changes = self._setUp_client_with_mock([{'managedZones': []}, + client, changes = self._setUp_client_with_mock([{'managedZones': []}, {'managedZones': []}]) - client.del_txt_record(DOMAIN, self.record_name, self.record_content, self.record_ttl) + changes.create.assert_not_called() @mock.patch('oauth2client.service_account.ServiceAccountCredentials.from_json_keyfile_name') @mock.patch('certbot_dns_google._internal.dns_google.open', @@ -287,24 +340,39 @@ class GoogleClientTest(unittest.TestCase): @mock.patch('oauth2client.service_account.ServiceAccountCredentials.from_json_keyfile_name') @mock.patch('certbot_dns_google._internal.dns_google.open', mock.mock_open(read_data='{"project_id": "' + PROJECT_ID + '"}'), create=True) - def test_get_existing(self, unused_credential_mock): + def test_get_existing_found(self, unused_credential_mock): client, unused_changes = self._setUp_client_with_mock( [{'managedZones': [{'id': self.zone}]}]) # Record name mocked in setUp found = client.get_existing_txt_rrset(self.zone, "_acme-challenge.example.org") - self.assertEqual(found, ["\"example-txt-contents\""]) + self.assertEqual(found["rrdatas"], ["\"example-txt-contents\""]) + self.assertEqual(found["ttl"], 60) + + @mock.patch('oauth2client.service_account.ServiceAccountCredentials.from_json_keyfile_name') + @mock.patch('certbot_dns_google._internal.dns_google.open', + mock.mock_open(read_data='{"project_id": "' + PROJECT_ID + '"}'), create=True) + def test_get_existing_not_found(self, unused_credential_mock): + client, unused_changes = self._setUp_client_with_mock( + [{'managedZones': [{'id': self.zone}]}]) not_found = client.get_existing_txt_rrset(self.zone, "nonexistent.tld") self.assertEqual(not_found, None) + @mock.patch('oauth2client.service_account.ServiceAccountCredentials.from_json_keyfile_name') + @mock.patch('certbot_dns_google._internal.dns_google.open', + mock.mock_open(read_data='{"project_id": "' + PROJECT_ID + '"}'), create=True) + def test_get_existing_with_error(self, unused_credential_mock): + client, unused_changes = self._setUp_client_with_mock( + [{'managedZones': [{'id': self.zone}]}], API_ERROR) + # Record name mocked in setUp + found = client.get_existing_txt_rrset(self.zone, "_acme-challenge.example.org") + self.assertEqual(found, None) + @mock.patch('oauth2client.service_account.ServiceAccountCredentials.from_json_keyfile_name') @mock.patch('certbot_dns_google._internal.dns_google.open', mock.mock_open(read_data='{"project_id": "' + PROJECT_ID + '"}'), create=True) def test_get_existing_fallback(self, unused_credential_mock): client, unused_changes = self._setUp_client_with_mock( - [{'managedZones': [{'id': self.zone}]}]) - mock_execute = client.dns.resourceRecordSets.return_value.list.return_value.execute - mock_execute.side_effect = API_ERROR - + [{'managedZones': [{'id': self.zone}]}], API_ERROR) rrset = client.get_existing_txt_rrset(self.zone, "_acme-challenge.example.org") self.assertFalse(rrset) diff --git a/certbot-dns-linode/certbot_dns_linode/__init__.py b/certbot-dns-linode/certbot_dns_linode/__init__.py index 4bfd95573..bca15bdb2 100644 --- a/certbot-dns-linode/certbot_dns_linode/__init__.py +++ b/certbot-dns-linode/certbot_dns_linode/__init__.py @@ -3,6 +3,10 @@ The `~certbot_dns_linode.dns_linode` plugin automates the process of completing a ``dns-01`` challenge (`~acme.challenges.DNS01`) by creating, and subsequently removing, TXT records using the Linode API. +.. note:: + The plugin is not installed by default. It can be installed by heading to + `certbot.eff.org `_, choosing your system and + selecting the Wildcard tab. Named Arguments --------------- diff --git a/certbot-dns-linode/certbot_dns_linode/_internal/dns_linode.py b/certbot-dns-linode/certbot_dns_linode/_internal/dns_linode.py index f9450c02c..c1b5e066f 100644 --- a/certbot-dns-linode/certbot_dns_linode/_internal/dns_linode.py +++ b/certbot-dns-linode/certbot_dns_linode/_internal/dns_linode.py @@ -24,7 +24,7 @@ class Authenticator(dns_common.DNSAuthenticator): This Authenticator uses the Linode API to fulfill a dns-01 challenge. """ - description = 'Obtain certs using a DNS TXT record (if you are using Linode for DNS).' + description = 'Obtain certificates using a DNS TXT record (if you are using Linode for DNS).' def __init__(self, *args, **kwargs): super(Authenticator, self).__init__(*args, **kwargs) diff --git a/certbot-dns-luadns/certbot_dns_luadns/__init__.py b/certbot-dns-luadns/certbot_dns_luadns/__init__.py index e8e86f77c..302cb1392 100644 --- a/certbot-dns-luadns/certbot_dns_luadns/__init__.py +++ b/certbot-dns-luadns/certbot_dns_luadns/__init__.py @@ -3,6 +3,10 @@ The `~certbot_dns_luadns.dns_luadns` plugin automates the process of completing a ``dns-01`` challenge (`~acme.challenges.DNS01`) by creating, and subsequently removing, TXT records using the LuaDNS API. +.. note:: + The plugin is not installed by default. It can be installed by heading to + `certbot.eff.org `_, choosing your system and + selecting the Wildcard tab. Named Arguments --------------- diff --git a/certbot-dns-nsone/certbot_dns_nsone/__init__.py b/certbot-dns-nsone/certbot_dns_nsone/__init__.py index e59be74a7..6c7d41ba4 100644 --- a/certbot-dns-nsone/certbot_dns_nsone/__init__.py +++ b/certbot-dns-nsone/certbot_dns_nsone/__init__.py @@ -3,6 +3,10 @@ The `~certbot_dns_nsone.dns_nsone` plugin automates the process of completing a ``dns-01`` challenge (`~acme.challenges.DNS01`) by creating, and subsequently removing, TXT records using the NS1 API. +.. note:: + The plugin is not installed by default. It can be installed by heading to + `certbot.eff.org `_, choosing your system and + selecting the Wildcard tab. Named Arguments --------------- diff --git a/certbot-dns-ovh/certbot_dns_ovh/__init__.py b/certbot-dns-ovh/certbot_dns_ovh/__init__.py index d508fad1b..6a079e59f 100644 --- a/certbot-dns-ovh/certbot_dns_ovh/__init__.py +++ b/certbot-dns-ovh/certbot_dns_ovh/__init__.py @@ -3,6 +3,10 @@ The `~certbot_dns_ovh.dns_ovh` plugin automates the process of completing a ``dns-01`` challenge (`~acme.challenges.DNS01`) by creating, and subsequently removing, TXT records using the OVH API. +.. note:: + The plugin is not installed by default. It can be installed by heading to + `certbot.eff.org `_, choosing your system and + selecting the Wildcard tab. Named Arguments --------------- diff --git a/certbot-dns-rfc2136/certbot_dns_rfc2136/__init__.py b/certbot-dns-rfc2136/certbot_dns_rfc2136/__init__.py index da8ef3419..3c574835f 100644 --- a/certbot-dns-rfc2136/certbot_dns_rfc2136/__init__.py +++ b/certbot-dns-rfc2136/certbot_dns_rfc2136/__init__.py @@ -3,6 +3,10 @@ The `~certbot_dns_rfc2136.dns_rfc2136` plugin automates the process of completing a ``dns-01`` challenge (`~acme.challenges.DNS01`) by creating, and subsequently removing, TXT records using RFC 2136 Dynamic Updates. +.. note:: + The plugin is not installed by default. It can be installed by heading to + `certbot.eff.org `_, choosing your system and + selecting the Wildcard tab. Named Arguments --------------- diff --git a/certbot-dns-route53/certbot_dns_route53/__init__.py b/certbot-dns-route53/certbot_dns_route53/__init__.py index 8659617ef..1b59f5620 100644 --- a/certbot-dns-route53/certbot_dns_route53/__init__.py +++ b/certbot-dns-route53/certbot_dns_route53/__init__.py @@ -3,6 +3,10 @@ The `~certbot_dns_route53.dns_route53` plugin automates the process of completing a ``dns-01`` challenge (`~acme.challenges.DNS01`) by creating, and subsequently removing, TXT records using the Amazon Web Services Route 53 API. +.. note:: + The plugin is not installed by default. It can be installed by heading to + `certbot.eff.org `_, choosing your system and + selecting the Wildcard tab. Named Arguments --------------- diff --git a/certbot-dns-sakuracloud/certbot_dns_sakuracloud/__init__.py b/certbot-dns-sakuracloud/certbot_dns_sakuracloud/__init__.py index f18780c18..c16ee96ef 100644 --- a/certbot-dns-sakuracloud/certbot_dns_sakuracloud/__init__.py +++ b/certbot-dns-sakuracloud/certbot_dns_sakuracloud/__init__.py @@ -3,6 +3,10 @@ The `~certbot_dns_sakuracloud.dns_sakuracloud` plugin automates the process of c a ``dns-01`` challenge (`~acme.challenges.DNS01`) by creating, and subsequently removing, TXT records using the Sakura Cloud DNS API. +.. note:: + The plugin is not installed by default. It can be installed by heading to + `certbot.eff.org `_, choosing your system and + selecting the Wildcard tab. Named Arguments --------------- diff --git a/certbot-nginx/certbot_nginx/_internal/configurator.py b/certbot-nginx/certbot_nginx/_internal/configurator.py index 87afedd38..15fbe61f7 100644 --- a/certbot-nginx/certbot_nginx/_internal/configurator.py +++ b/certbot-nginx/certbot_nginx/_internal/configurator.py @@ -226,7 +226,7 @@ class NginxConfigurator(common.Installer): if not fullchain_path: raise errors.PluginError( "The nginx plugin currently requires --fullchain-path to " - "install a cert.") + "install a certificate.") vhosts = self.choose_vhosts(domain, create_if_no_match=True) for vhost in vhosts: diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index 82ba6121a..4c8de11af 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -10,12 +10,20 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). ### Changed -* +* We deprecated support for Python 2 in Certbot and its ACME library. + Support for Python 2 will be removed in the next planned release of Certbot. +* certbot-auto was deprecated on all systems. For more information about this + change, see + https://community.letsencrypt.org/t/certbot-auto-no-longer-works-on-debian-based-systems/139702/7. +* We deprecated support for Apache 2.2 in the certbot-apache plugin and it will + be removed in a future release of Certbot. ### Fixed * The Certbot snap no longer loads packages installed via `pip install --user`. This was unintended and DNS plugins should be installed via `snap` instead. +* `certbot-dns-google` would sometimes crash with HTTP 409/412 errors when used with very large zones. See [#6036](https://github.com/certbot/certbot/issues/6036). +* `certbot-dns-google` would sometimes crash with an HTTP 412 error if preexisting records had an unexpected TTL, i.e.: different than Certbot's default TTL for this plugin. See [#8551](https://github.com/certbot/certbot/issues/8551). More details about these changes can be found on our GitHub repo. diff --git a/certbot/README.rst b/certbot/README.rst index 2ff2d41be..40f6a52ec 100644 --- a/certbot/README.rst +++ b/certbot/README.rst @@ -18,10 +18,6 @@ systems. To see the changes made to Certbot between versions please refer to our `changelog `_. -Until May 2016, Certbot was named simply ``letsencrypt`` or ``letsencrypt-auto``, -depending on install method. Instructions on the Internet, and some pieces of the -software, may still refer to this older name. - Contributing ------------ @@ -96,7 +92,7 @@ Current Features - apache/2.x - nginx/0.8.48+ - webroot (adds files to webroot directories in order to prove control of - domains and obtain certs) + domains and obtain certificates) - standalone (runs its own simple webserver to prove you control a domain) - other server software via `third party plugins `_ diff --git a/certbot/certbot/__init__.py b/certbot/certbot/__init__.py index 11c97dfac..98009a71b 100644 --- a/certbot/certbot/__init__.py +++ b/certbot/certbot/__init__.py @@ -1,4 +1,13 @@ """Certbot client.""" +import warnings +import sys # version number like 1.2.3a0, must have at least 2 parts, like 1.2 __version__ = '1.11.0.dev0' + +if sys.version_info[0] == 2: + warnings.warn( + "Python 2 support will be dropped in the next release of Certbot. " + "Please upgrade your Python version.", + PendingDeprecationWarning, + ) # pragma: no cover diff --git a/certbot/certbot/_internal/cert_manager.py b/certbot/certbot/_internal/cert_manager.py index 80a98ab04..dfbe4b538 100644 --- a/certbot/certbot/_internal/cert_manager.py +++ b/certbot/certbot/_internal/cert_manager.py @@ -369,7 +369,7 @@ def _describe_certs(config, parsed_certs, parse_failures): notify = out.append if not parsed_certs and not parse_failures: - notify("No certs found.") + notify("No certificates found.") else: if parsed_certs: match = "matching " if config.certname or config.domains else "" diff --git a/certbot/certbot/_internal/main.py b/certbot/certbot/_internal/main.py index d268f3fd8..e96868b65 100644 --- a/certbot/certbot/_internal/main.py +++ b/certbot/certbot/_internal/main.py @@ -5,6 +5,7 @@ from __future__ import print_function import functools import logging.handlers import sys +import warnings import configobj import josepy as jose @@ -238,7 +239,7 @@ def _handle_identical_cert_request(config, # type: configuration.NamespaceConfi elif config.verb == "certonly": keep_opt = "Keep the existing certificate for now" choices = [keep_opt, - "Renew & replace the cert (may be subject to CA rate limits)"] + "Renew & replace the certificate (may be subject to CA rate limits)"] display = zope.component.getUtility(interfaces.IDisplay) response = display.menu(question, choices, @@ -418,8 +419,8 @@ def _ask_user_to_confirm_new_names(config, new_domains, certname, old_domains): _format_list("-", removed), br=os.linesep)) obj = zope.component.getUtility(interfaces.IDisplay) - if not obj.yesno(msg, "Update cert", "Cancel", default=True): - raise errors.ConfigurationError("Specified mismatched cert name and domains.") + if not obj.yesno(msg, "Update certificate", "Cancel", default=True): + raise errors.ConfigurationError("Specified mismatched certificate name and domains.") def _find_domains_or_certname(config, installer, question=None): @@ -621,8 +622,8 @@ def _delete_if_appropriate(config): attempt_deletion = config.delete_after_revoke if attempt_deletion is None: - msg = ("Would you like to delete the cert(s) you just revoked, along with all earlier and " - "later versions of the cert?") + msg = ("Would you like to delete the certificate(s) you just revoked, " + "along with all earlier and later versions of the certificate?") attempt_deletion = display.yesno(msg, yes_label="Yes (recommended)", no_label="No", force_interactive=True, default=True) @@ -644,8 +645,8 @@ def _delete_if_appropriate(config): cert_manager.match_and_check_overlaps(config, [lambda x: archive_dir], lambda x: x.archive_dir, lambda x: x) except errors.OverlappingMatchFound: - logger.warning("Not deleting revoked certs due to overlapping archive dirs. More than " - "one certificate is using %s", archive_dir) + logger.warning("Not deleting revoked certificates due to overlapping archive dirs. " + "More than one certificate is using %s", archive_dir) return except Exception as e: msg = ('config.default_archive_dir: {0}, config.live_dir: {1}, archive_dir: {2},' @@ -793,7 +794,7 @@ def update_account(config, unused_plugins): acc.regr = acc.regr.update(uri=prev_regr_uri) account_storage.update_regr(acc, cb_client.acme) - if config.email is None: + if not config.email: display_util.notify("Any contact information associated " "with this account has been removed.") else: @@ -1122,7 +1123,7 @@ def revoke(config, unused_plugins): raise errors.Error("Error! Exactly one of --cert-path or --cert-name must be specified!") if config.key_path is not None: # revocation by cert key - logger.debug("Revoking %s using cert key %s", + logger.debug("Revoking %s using certificate key %s", config.cert_path[0], config.key_path[0]) crypto_util.verify_cert_matches_priv_key(config.cert_path[0], config.key_path[0]) key = jose.JWK.load(config.key_path[1]) @@ -1416,6 +1417,7 @@ def main(cli_args=None): plugins = plugins_disco.PluginsRegistry.find_all() logger.debug("certbot version: %s", certbot.__version__) + logger.debug("Location of certbot entry point: %s", sys.argv[0]) # do not log `config`, as it contains sensitive data (e.g. revoke --key)! logger.debug("Arguments: %r", cli_args) logger.debug("Discovered plugins: %r", plugins) @@ -1437,6 +1439,13 @@ def main(cli_args=None): if config.func != plugins_cmd: # pylint: disable=comparison-with-callable raise + if sys.version_info[0] == 2: + warnings.warn( + "Python 2 support will be dropped in the next release of Certbot. " + "Please upgrade your Python version.", + PendingDeprecationWarning, + ) # pragma: no cover + set_displayer(config) # Reporter diff --git a/certbot/certbot/_internal/renewal.py b/certbot/certbot/_internal/renewal.py index 9b528cb6a..3a550d355 100644 --- a/certbot/certbot/_internal/renewal.py +++ b/certbot/certbot/_internal/renewal.py @@ -99,7 +99,7 @@ def _reconstitute(config, full_path): config.domains = [util.enforce_domain_sanity(d) for d in renewal_candidate.names()] except errors.ConfigurationError as error: - logger.warning("Renewal configuration file %s references a cert " + logger.warning("Renewal configuration file %s references a certificate " "that contains an invalid domain name. The problem " "was: %s. Skipping.", full_path, error) return None @@ -293,13 +293,13 @@ def should_renew(config, lineage): def _avoid_invalidating_lineage(config, lineage, original_server): "Do not renew a valid cert with one from a staging server!" - # Some lineages may have begun with --staging, but then had production certs - # added to them + # Some lineages may have begun with --staging, but then had production + # certificates added to them with open(lineage.cert) as the_file: contents = the_file.read() latest_cert = OpenSSL.crypto.load_certificate( OpenSSL.crypto.FILETYPE_PEM, contents) - # all our test certs are from happy hacker fake CA, though maybe one day + # all our test certificates are from happy hacker fake CA, though maybe one day # we should test more methodically now_valid = "fake" not in repr(latest_cert.get_issuer()).lower() @@ -366,7 +366,7 @@ def _renew_describe_results(config, renew_successes, renew_failures, renewal_noun = "simulated renewal" if config.dry_run else "renewal" if renew_skipped: - notify("The following certs are not due for renewal yet:") + notify("The following certificates are not due for renewal yet:") notify(report(renew_skipped, "skipped")) if not renew_successes and not renew_failures: notify("No {renewal}s were attempted.".format(renewal=renewal_noun)) @@ -377,7 +377,7 @@ def _renew_describe_results(config, renew_successes, renew_failures, notify("Congratulations, all {renewal}s succeeded: ".format(renewal=renewal_noun)) notify(report(renew_successes, "success")) elif renew_failures and not renew_successes: - notify_error("All %ss failed. The following certs could " + notify_error("All %ss failed. The following certificates could " "not be renewed:", renewal_noun) notify_error(report(renew_failures, "failure")) elif renew_failures and renew_successes: @@ -482,7 +482,7 @@ def handle_renewal_request(config): except Exception as e: # pylint: disable=broad-except # obtain_cert (presumably) encountered an unanticipated problem. logger.error( - "Failed to renew cert %s with error: %s", + "Failed to renew certificate %s with error: %s", lineagename, e ) logger.debug("Traceback was:\n%s", traceback.format_exc()) diff --git a/certbot/certbot/_internal/storage.py b/certbot/certbot/_internal/storage.py index b6c37a5ba..a7f319197 100644 --- a/certbot/certbot/_internal/storage.py +++ b/certbot/certbot/_internal/storage.py @@ -809,8 +809,8 @@ class RenewableCert(interfaces.RenewableCert): May need to recover from rare interrupted / crashed states.""" if self.has_pending_deployment(): - logger.warning("Found a new cert /archive/ that was not linked to in /live/; " - "fixing...") + logger.warning("Found a new certificate /archive/ that was not " + "linked to in /live/; fixing...") self.update_all_links_to(self.latest_common_version()) return False return True @@ -883,7 +883,7 @@ class RenewableCert(interfaces.RenewableCert): """ target = self.current_target("cert") if target is None: - raise errors.CertStorageError("could not find cert file") + raise errors.CertStorageError("could not find the certificate file") with open(target) as f: return crypto_util.get_names_from_cert(f.read()) diff --git a/certbot/certbot/crypto_util.py b/certbot/certbot/crypto_util.py index 19a83103a..2f3622d94 100644 --- a/certbot/certbot/crypto_util.py +++ b/certbot/certbot/crypto_util.py @@ -279,7 +279,7 @@ def verify_renewable_cert_sig(renewable_cert): verify_signed_payload(pk, cert.signature, cert.tbs_certificate_bytes, cert.signature_hash_algorithm) except (IOError, ValueError, InvalidSignature) as e: - error_str = "verifying the signature of the cert located at {0} has failed. \ + error_str = "verifying the signature of the certificate located at {0} has failed. \ Details: {1}".format(renewable_cert.cert_path, e) logger.exception(error_str) raise errors.Error(error_str) @@ -330,7 +330,7 @@ def verify_cert_matches_priv_key(cert_path, key_path): context.use_privatekey_file(key_path) context.check_privatekey() except (IOError, SSL.Error) as e: - error_str = "verifying the cert located at {0} matches the \ + error_str = "verifying the certificate located at {0} matches the \ private key located at {1} has failed. \ Details: {2}".format(cert_path, key_path, e) diff --git a/certbot/certbot/ocsp.py b/certbot/certbot/ocsp.py index 75ce9e2ff..b63338e2e 100644 --- a/certbot/certbot/ocsp.py +++ b/certbot/certbot/ocsp.py @@ -167,7 +167,7 @@ def _determine_ocsp_server(cert_path): if host: return url, host - logger.info("Cannot process OCSP host from URL (%s) in cert at %s", url, cert_path) + logger.info("Cannot process OCSP host from URL (%s) in certificate at %s", url, cert_path) return None, None @@ -222,7 +222,7 @@ def _check_ocsp_cryptography(cert_path, chain_path, url, timeout): def _check_ocsp_response(response_ocsp, request_ocsp, issuer_cert, cert_path): - """Verify that the OCSP is valid for serveral criteria""" + """Verify that the OCSP is valid for several criteria""" # Assert OCSP response corresponds to the certificate we are talking about if response_ocsp.serial_number != request_ocsp.serial_number: raise AssertionError('the certificate in response does not correspond ' diff --git a/certbot/docs/compatibility.rst b/certbot/docs/compatibility.rst index a511f36a2..a4f33c281 100644 --- a/certbot/docs/compatibility.rst +++ b/certbot/docs/compatibility.rst @@ -9,7 +9,7 @@ application itself. This means that we will not change behavior in a backwards incompatible way except in a new major version of the project. .. note:: None of this applies to the behavior of Certbot distribution - mechanisms such as :ref:`certbot-auto ` or OS packages whose + mechanisms such as :ref:`our snaps ` or OS packages whose behavior may change at any time. Semantic versioning only applies to the common Certbot components that are installed by various distribution methods. diff --git a/certbot/docs/contributing.rst b/certbot/docs/contributing.rst index eb90b05f4..e130f0548 100644 --- a/certbot/docs/contributing.rst +++ b/certbot/docs/contributing.rst @@ -282,8 +282,8 @@ support for IIS, Icecast and Plesk. Installers and Authenticators will oftentimes be the same class/object (because for instance both tasks can be performed by a webserver like nginx) though this is not always the case (the standalone plugin is an authenticator -that listens on port 80, but it cannot install certs; a postfix plugin would -be an installer but not an authenticator). +that listens on port 80, but it cannot install certificates; a postfix plugin +would be an installer but not an authenticator). Installers and Authenticators are kept separate because it should be possible to use the `~.StandaloneAuthenticator` (it sets @@ -516,11 +516,13 @@ Steps: 4. Run ``tox --skip-missing-interpreters`` to run the entire test suite including coverage. The ``--skip-missing-interpreters`` argument ignores missing versions of Python needed for running the tests. Fix any errors. -5. Submit the PR. Once your PR is open, please do not force push to the branch +5. If any documentation should be added or updated as part of the changes you + have made, please include the documentation changes in your PR. +6. Submit the PR. Once your PR is open, please do not force push to the branch containing your pull request to squash or amend commits. We use `squash merges `_ on PRs and rewriting commits makes changes harder to track between reviews. -6. Did your tests pass on Azure Pipelines? If they didn't, fix any errors. +7. Did your tests pass on Azure Pipelines? If they didn't, fix any errors. .. _ask for help: diff --git a/certbot/docs/install.rst b/certbot/docs/install.rst index c1d8cc403..c2d79dc33 100644 --- a/certbot/docs/install.rst +++ b/certbot/docs/install.rst @@ -44,17 +44,6 @@ supports `_ modern OSes based on Debian, Ubuntu, Fedora, SUSE, Gentoo and Darwin. - -Additional integrity verification of certbot-auto script can be done by verifying its digital signature. -This requires a local installation of gpg2, which comes packaged in many Linux distributions under name gnupg or gnupg2. - - -Installing with ``certbot-auto`` requires 512MB of RAM in order to build some -of the dependencies. Installing from pre-built OS packages avoids this -requirement. You can also temporarily set a swap file. See "Problems with -Python virtual environment" below for details. - - Alternate installation methods ================================ @@ -78,74 +67,6 @@ choosing "snapd" in the "System" dropdown menu. (You should select "snapd" regardless of your operating system, as our instructions are the same across all systems.) -.. _certbot-auto: - -Certbot-Auto ------------- - -The ``certbot-auto`` wrapper script installs Certbot, obtaining some dependencies -from your web server OS and putting others in a python virtual environment. You can -download and run it as follows:: - - wget https://dl.eff.org/certbot-auto - sudo mv certbot-auto /usr/local/bin/certbot-auto - sudo chown root /usr/local/bin/certbot-auto - sudo chmod 0755 /usr/local/bin/certbot-auto - /usr/local/bin/certbot-auto --help - -To remove certbot-auto, just delete it and the files it places under /opt/eff.org, along with any cronjob or systemd timer you may have created. - -To check the integrity of the ``certbot-auto`` script, -you can use these steps:: - - - user@webserver:~$ wget -N https://dl.eff.org/certbot-auto.asc - user@webserver:~$ gpg2 --keyserver pool.sks-keyservers.net --recv-key A2CFB51FA275A7286234E7B24D17C995CD9775F2 - user@webserver:~$ gpg2 --trusted-key 4D17C995CD9775F2 --verify certbot-auto.asc /usr/local/bin/certbot-auto - - - -The output of the last command should look something like:: - - - gpg: Signature made Wed 02 May 2018 05:29:12 AM IST - gpg: using RSA key A2CFB51FA275A7286234E7B24D17C995CD9775F2 - gpg: key 4D17C995CD9775F2 marked as ultimately trusted - gpg: checking the trustdb - gpg: marginals needed: 3 completes needed: 1 trust model: pgp - gpg: depth: 0 valid: 2 signed: 2 trust: 0-, 0q, 0n, 0m, 0f, 2u - gpg: depth: 1 valid: 2 signed: 0 trust: 2-, 0q, 0n, 0m, 0f, 0u - gpg: next trustdb check due at 2027-11-22 - gpg: Good signature from "Let's Encrypt Client Team " [ultimate] - - - -The ``certbot-auto`` command updates to the latest client release automatically. -Since ``certbot-auto`` is a wrapper to ``certbot``, it accepts exactly -the same command line flags and arguments. For more information, see -`Certbot command-line options `_. - -For full command line help, you can type:: - - /usr/local/bin/certbot-auto --help all - -Problems with Python virtual environment ----------------------------------------- - -On a low memory system such as VPS with less than 512MB of RAM, the required dependencies of Certbot will fail to build. -This can be identified if the pip outputs contains something like ``internal compiler error: Killed (program cc1)``. -You can workaround this restriction by creating a temporary swapfile:: - - user@webserver:~$ sudo fallocate -l 1G /tmp/swapfile - user@webserver:~$ sudo chmod 600 /tmp/swapfile - user@webserver:~$ sudo mkswap /tmp/swapfile - user@webserver:~$ sudo swapon /tmp/swapfile - -Disable and remove the swapfile once the virtual environment is constructed:: - - user@webserver:~$ sudo swapoff /tmp/swapfile - user@webserver:~$ sudo rm /tmp/swapfile - .. _docker-user: Running with Docker @@ -161,7 +82,7 @@ Docker if you are sure you know what you are doing and have a good reason to do so. You should definitely read the :ref:`where-certs` section, in order to -know how to manage the certs +know how to manage the certificates manually. `Our ciphersuites page `__ provides some information about recommended ciphersuites. If none of these make much sense to you, you should definitely use the installation method @@ -207,6 +128,18 @@ of the ``/etc/letsencrypt`` directory, see :ref:`where-certs`. Operating System Packages ------------------------- +.. warning:: While the Certbot team tries to keep the Certbot packages offered + by various operating systems working in the most basic sense, due to + distribution policies and/or the limited resources of distribution + maintainers, Certbot OS packages often have problems that other distribution + mechanisms do not. The packages are often old resulting in a lack of bug + fixes and features and a worse TLS configuration than is generated by newer + versions of Certbot. They also may not configure certificate renewal for you + or have all of Certbot's plugins available. For reasons like these, we + recommend most users follow the instructions at + https://certbot.eff.org/instructions and OS packages are only documented + here as an alternative. + **Arch Linux** .. code-block:: shell @@ -273,8 +206,8 @@ Optionally to install the Certbot Apache plugin, you can use: **Gentoo** -The official Certbot client is available in Gentoo Portage. From the -official Certbot plugins, three of them are also available in Portage. +The official Certbot client is available in Gentoo Portage. From the +official Certbot plugins, three of them are also available in Portage. They need to be installed separately if you require their functionality. .. code-block:: shell @@ -284,7 +217,7 @@ They need to be installed separately if you require their functionality. emerge -av app-crypt/certbot-nginx emerge -av app-crypt/certbot-dns-nsone -.. Note:: The ``app-crypt/certbot-dns-nsone`` package has a different +.. Note:: The ``app-crypt/certbot-dns-nsone`` package has a different maintainer than the other packages and can lag behind in version. **NetBSD** @@ -303,6 +236,35 @@ OS packaging is an ongoing effort. If you'd like to package Certbot for your distribution of choice please have a look at the :doc:`packaging`. +.. _certbot-auto: + +Certbot-Auto +------------ + +We used to have a shell script named ``certbot-auto`` to help people install +Certbot on UNIX operating systems, however, this script is no longer supported. +If you want to uninstall ``certbot-auto``, you can follow our instructions +:doc:`here `. + +Problems with Python virtual environment +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +When using ``certbot-auto`` on a low memory system such as VPS with less than +512MB of RAM, the required dependencies of Certbot may fail to build. This can +be identified if the pip outputs contains something like ``internal compiler +error: Killed (program cc1)``. You can workaround this restriction by creating +a temporary swapfile:: + + user@webserver:~$ sudo fallocate -l 1G /tmp/swapfile + user@webserver:~$ sudo chmod 600 /tmp/swapfile + user@webserver:~$ sudo mkswap /tmp/swapfile + user@webserver:~$ sudo swapon /tmp/swapfile + +Disable and remove the swapfile once the virtual environment is constructed:: + + user@webserver:~$ sudo swapoff /tmp/swapfile + user@webserver:~$ sudo rm /tmp/swapfile + Installing from source ---------------------- diff --git a/certbot/docs/uninstall.rst b/certbot/docs/uninstall.rst new file mode 100644 index 000000000..65151242c --- /dev/null +++ b/certbot/docs/uninstall.rst @@ -0,0 +1,16 @@ +========================= +Uninstalling certbot-auto +========================= + +To uninstall ``certbot-auto``, you need to do three things: + +1. If you added a cron job or systemd timer to automatically run + ``certbot-auto`` to renew your certificates, you should delete it. If you + did this by following our instructions, you can delete the entry added to + ``/etc/crontab`` by running a command like ``sudo sed -i '/certbot-auto/d' + /etc/crontab``. +2. Delete the ``certbot-auto`` script. If you placed it in ``/usr/local/bin`` + like we recommended, you can delete it by running ``sudo rm + /usr/local/bin/certbot-auto``. +3. Delete the Certbot installation created by ``certbot-auto`` by running + ``sudo rm -rf /opt/eff.org``. diff --git a/certbot/docs/using.rst b/certbot/docs/using.rst index 1912dafa4..ab8d64d79 100644 --- a/certbot/docs/using.rst +++ b/certbot/docs/using.rst @@ -179,10 +179,9 @@ If you'd like to obtain a wildcard certificate from Let's Encrypt or run Certbot's DNS plugins. These plugins are not included in a default Certbot installation and must be -installed separately. While the DNS plugins cannot currently be used with -``certbot-auto``, they are available in many OS package managers, as Docker -images, and as snaps. Visit https://certbot.eff.org to learn the best way to use -the DNS plugins on your system. +installed separately. They are available in many OS package managers, as Docker +images, and as snaps. Visit https://certbot.eff.org to learn the best way to +use the DNS plugins on your system. Once installed, you can find documentation on how to use each plugin at: @@ -314,7 +313,7 @@ the ``certificates`` subcommand: This returns information in the following format:: - Found the following certs: + Found the following certificates: Certificate Name: example.com Domains: example.com, www.example.com Expiry Date: 2017-02-19 19:53:00+00:00 (VALID: 30 days) @@ -913,7 +912,7 @@ Changing the ACME Server ======================== By default, Certbot uses Let's Encrypt's production server at -https://acme-v02.api.letsencrypt.org/. You can tell Certbot to use a +https://acme-v02.api.letsencrypt.org/directory. You can tell Certbot to use a different CA by providing ``--server`` on the command line or in a :ref:`configuration file ` with the URL of the server's ACME directory. For example, if you would like to use Let's Encrypt's diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py index 31b434ce4..c50bc6be1 100644 --- a/certbot/tests/main_test.py +++ b/certbot/tests/main_test.py @@ -1173,7 +1173,7 @@ class MainTest(test_util.ConfigTestCase): _, _, stdout = self._test_renewal_common(False, extra_args=None, should_renew=False, args=['renew'], expiry_date=expiry) self.assertTrue('No renewals were attempted.' in stdout.getvalue()) - self.assertTrue('The following certs are not due for renewal yet:' in stdout.getvalue()) + self.assertTrue('The following certificates are not due for renewal yet:' in stdout.getvalue()) @mock.patch('certbot._internal.log.post_arg_parse_setup') def test_quiet_renew(self, _): @@ -1450,85 +1450,6 @@ class MainTest(test_util.ConfigTestCase): x = self._call_no_clientmock(["register", "--email", "user@example.org"]) self.assertTrue("There is an existing account" in x[0]) - def test_update_account_no_existing_accounts(self): - # with mock.patch('certbot._internal.main.client') as mocked_client: - with mock.patch('certbot._internal.main.account') as mocked_account: - mocked_storage = mock.MagicMock() - mocked_account.AccountFileStorage.return_value = mocked_storage - mocked_storage.find_all.return_value = [] - x = self._call_no_clientmock( - ["update_account", "--email", - "user@example.org"]) - self.assertTrue("Could not find an existing account" in x[0]) - - @mock.patch('certbot._internal.main._determine_account') - @mock.patch('certbot._internal.eff.prepare_subscription') - @mock.patch('certbot._internal.main.account') - def test_update_account_remove_email(self, mocked_account_module, mock_prepare, mock_det_acc): - # Mock account storage and the account object returned - mocked_storage = mock.MagicMock() - mocked_account = mock.MagicMock() - - mocked_account_module.AccountFileStorage.return_value = mocked_storage - mocked_storage.find_all.return_value = [mocked_account] - mock_det_acc.return_value = (mocked_account, "foo") - - # Mock registration body to verify calls are made - mock_regr_body = mock.MagicMock() - - # mocked_account.regr is overwritten in update, requiring an odd mock setup - mocked_account.regr.body = mock_regr_body - - x = self._call( - ["update_account", "--register-unsafely-without-email"]) - - - # When update succeeds, the return value of update_account() is None - self.assertTrue(x[0] is None) - # and we got supposedly did update the registration from - # the server - client_mock = x[3] - self.assertTrue(client_mock.Client().acme.update_registration.called) - - self.assertTrue(mock_regr_body.update.called) - self.assertTrue('contact' in mock_regr_body.update.call_args[1]) - self.assertEqual(mock_regr_body.update.call_args[1]['contact'], ()) - # and we saved the updated registration on disk - self.assertTrue(mocked_storage.update_regr.called) - # ensure we didn't try to subscribe (no email to subscribe with) - self.assertFalse(mock_prepare.called) - - @mock.patch("certbot._internal.main.display_util.notify") - @mock.patch('certbot._internal.main.display_ops.get_email') - @test_util.patch_get_utility() - def test_update_account_with_email(self, mock_utility, mock_email, mock_notify): - email = "user@example.com" - mock_email.return_value = email - with mock.patch('certbot._internal.eff.prepare_subscription') as mock_prepare: - with mock.patch('certbot._internal.main._determine_account') as mocked_det: - with mock.patch('certbot._internal.main.account') as mocked_account: - with mock.patch('certbot._internal.main.client') as mocked_client: - mocked_storage = mock.MagicMock() - mocked_account.AccountFileStorage.return_value = mocked_storage - mocked_storage.find_all.return_value = ["an account"] - mocked_det.return_value = (mock.MagicMock(), "foo") - cb_client = mock.MagicMock() - mocked_client.Client.return_value = cb_client - x = self._call_no_clientmock( - ["update_account"]) - # When registration change succeeds, the return value - # of register() is None - self.assertTrue(x[0] is None) - # and we got supposedly did update the registration from - # the server - self.assertTrue( - cb_client.acme.update_registration.called) - # and we saved the updated registration on disk - self.assertTrue(mocked_storage.update_regr.called) - self.assertTrue( - email in mock_notify.call_args[0][0]) - self.assertTrue(mock_prepare.called) - @mock.patch('certbot._internal.plugins.selection.choose_configurator_plugins') @mock.patch('certbot._internal.updater._run_updaters') def test_plugin_selection_error(self, mock_run, mock_choose): @@ -1882,5 +1803,111 @@ class ReportNewCertTest(unittest.TestCase): ) +class UpdateAccountTest(test_util.ConfigTestCase): + """Tests for certbot._internal.main.update_account""" + + def setUp(self): + patches = { + 'account': mock.patch('certbot._internal.main.account'), + 'atexit': mock.patch('certbot.util.atexit'), + 'client': mock.patch('certbot._internal.main.client'), + 'determine_account': mock.patch('certbot._internal.main._determine_account'), + 'notify': mock.patch('certbot._internal.main.display_util.notify'), + 'prepare_sub': mock.patch('certbot._internal.eff.prepare_subscription'), + 'util': test_util.patch_get_utility() + } + self.mocks = { k: patches[k].start() for k in patches } + for patch in patches.values(): + self.addCleanup(patch.stop) + + return super(UpdateAccountTest, self).setUp() + + def _call(self, args): + with mock.patch('certbot._internal.main.sys.stdout'), \ + mock.patch('certbot._internal.main.sys.stderr'): + args = ['--config-dir', self.config.config_dir, + '--work-dir', self.config.work_dir, + '--logs-dir', self.config.logs_dir, '--text'] + args + return main.main(args[:]) # NOTE: parser can alter its args! + + def _prepare_mock_account(self): + mock_storage = mock.MagicMock() + mock_account = mock.MagicMock() + mock_regr = mock.MagicMock() + mock_storage.find_all.return_value = [mock_account] + self.mocks['account'].AccountFileStorage.return_value = mock_storage + mock_account.regr.body = mock_regr.body + self.mocks['determine_account'].return_value = (mock_account, mock.MagicMock()) + return (mock_account, mock_storage, mock_regr) + + def _test_update_no_contact(self, args): + """Utility to assert that email removal is handled correctly""" + (_, mock_storage, mock_regr) = self._prepare_mock_account() + result = self._call(args) + # When update succeeds, the return value of update_account() is None + self.assertIsNone(result) + # We submitted a registration to the server + self.assertEqual(self.mocks['client'].Client().acme.update_registration.call_count, 1) + mock_regr.body.update.assert_called_with(contact=()) + # We got an update from the server and persisted it + self.assertEqual(mock_storage.update_regr.call_count, 1) + # We should have notified the user + self.mocks['notify'].assert_called_with( + 'Any contact information associated with this account has been removed.' + ) + # We should not have called subscription because there's no email + self.mocks['prepare_sub'].assert_not_called() + + def test_no_existing_accounts(self): + """Test that no existing account is handled correctly""" + mock_storage = mock.MagicMock() + mock_storage.find_all.return_value = [] + self.mocks['account'].AccountFileStorage.return_value = mock_storage + self.assertEqual(self._call(['update_account', '--email', 'user@example.org']), + 'Could not find an existing account to update.') + + def test_update_account_remove_email(self): + """Test that --register-unsafely-without-email is handled as no email""" + self._test_update_no_contact(['update_account', '--register-unsafely-without-email']) + + def test_update_account_empty_email(self): + """Test that providing an empty email is handled as no email""" + self._test_update_no_contact(['update_account', '-m', '']) + + @mock.patch('certbot._internal.main.display_ops.get_email') + def test_update_account_with_email(self, mock_email): + """Test that updating with a singular email is handled correctly""" + mock_email.return_value = 'user@example.com' + (_, mock_storage, _) = self._prepare_mock_account() + mock_client = mock.MagicMock() + self.mocks['client'].Client.return_value = mock_client + + result = self._call(['update_account']) + # None if registration succeeds + self.assertIsNone(result) + # We should have updated the server + self.assertEqual(mock_client.acme.update_registration.call_count, 1) + # We should have updated the account on disk + self.assertEqual(mock_storage.update_regr.call_count, 1) + # Subscription should have been prompted + self.assertEqual(self.mocks['prepare_sub'].call_count, 1) + # Should have printed the email + self.mocks['notify'].assert_called_with( + 'Your e-mail address was updated to user@example.com.') + + def test_update_account_with_multiple_emails(self): + """Test that multiple email addresses are handled correctly""" + (_, mock_storage, mock_regr) = self._prepare_mock_account() + self.assertIsNone( + self._call(['update_account', '-m', 'user@example.com,user@example.org']) + ) + mock_regr.body.update.assert_called_with( + contact=['mailto:user@example.com', 'mailto:user@example.org'] + ) + self.assertEqual(mock_storage.update_regr.call_count, 1) + self.mocks['notify'].assert_called_with( + 'Your e-mail address was updated to user@example.com,user@example.org.') + + if __name__ == '__main__': unittest.main() # pragma: no cover diff --git a/certbot/tests/renewal_test.py b/certbot/tests/renewal_test.py index 44c78c701..4af8c6e7f 100644 --- a/certbot/tests/renewal_test.py +++ b/certbot/tests/renewal_test.py @@ -204,7 +204,7 @@ class DescribeResultsTest(unittest.TestCase): '- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -', ]) self.mock_error.assert_has_calls([ - mock.call('All %ss failed. The following certs could not be renewed:', 'renewal'), + mock.call('All %ss failed. The following certificates could not be renewed:', 'renewal'), mock.call(' bad.pem (failure)'), ]) @@ -214,7 +214,7 @@ class DescribeResultsTest(unittest.TestCase): ['foo.pem expires on 123'], ['errored.conf']) self._assert_success_output([ '\n- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -', - 'The following certs are not due for renewal yet:', + 'The following certificates are not due for renewal yet:', ' foo.pem expires on 123 (skipped)', 'The following simulated renewals succeeded:', ' good.pem (success)\n good2.pem (success)\n', diff --git a/certbot/tests/util_test.py b/certbot/tests/util_test.py index 4b93d2c38..7b510fbb6 100644 --- a/certbot/tests/util_test.py +++ b/certbot/tests/util_test.py @@ -547,8 +547,7 @@ class OsInfoTest(unittest.TestCase): m_distro.linux_distribution.return_value = ("something", "else") self.assertEqual(cbutil.get_os_info(), ("something", "else")) - @mock.patch("certbot.util.subprocess.Popen") - def test_non_systemd_os_info(self, popen_mock): + def test_non_systemd_os_info(self): import certbot.util as cbutil with mock.patch('certbot.util._USE_DISTRO', False): with mock.patch('platform.system_alias', @@ -557,13 +556,14 @@ class OsInfoTest(unittest.TestCase): with mock.patch('platform.system_alias', return_value=('darwin', '', '')): - comm_mock = mock.Mock() - comm_attrs = {'communicate.return_value': - ('42.42.42', 'error')} - comm_mock.configure_mock(**comm_attrs) - popen_mock.return_value = comm_mock - self.assertEqual(cbutil.get_python_os_info()[0], 'darwin') - self.assertEqual(cbutil.get_python_os_info()[1], '42.42.42') + with mock.patch("subprocess.Popen") as popen_mock: + comm_mock = mock.Mock() + comm_attrs = {'communicate.return_value': + ('42.42.42', 'error')} + comm_mock.configure_mock(**comm_attrs) + popen_mock.return_value = comm_mock + self.assertEqual(cbutil.get_python_os_info()[0], 'darwin') + self.assertEqual(cbutil.get_python_os_info()[1], '42.42.42') with mock.patch('platform.system_alias', return_value=('freebsd', '9.3-RC3-p1', '')): diff --git a/letsencrypt-auto-source/letsencrypt-auto b/letsencrypt-auto-source/letsencrypt-auto index 789904992..7f358f805 100755 --- a/letsencrypt-auto-source/letsencrypt-auto +++ b/letsencrypt-auto-source/letsencrypt-auto @@ -804,6 +804,7 @@ elif [ -f /etc/mageia-release ]; then # Mageia has both /etc/mageia-release and /etc/redhat-release DEPRECATED_OS=1 elif [ -f /etc/redhat-release ]; then + DEPRECATED_OS=1 # Run DeterminePythonVersion to decide on the basis of available Python versions # whether to use 2.x or 3.x on RedHat-like systems. # Then, revert LE_PYTHON to its previous state. @@ -836,12 +837,7 @@ elif [ -f /etc/redhat-release ]; then INTERACTIVE_BOOTSTRAP=1 fi - Bootstrap() { - BootstrapMessage "Legacy RedHat-based OSes that will use Python3" - BootstrapRpmPython3Legacy - } USE_PYTHON_3=1 - BOOTSTRAP_VERSION="BootstrapRpmPython3Legacy $BOOTSTRAP_RPM_PYTHON3_LEGACY_VERSION" # Try now to enable SCL rh-python36 for systems already bootstrapped # NB: EnablePython36SCL has been defined along with BootstrapRpmPython3Legacy in certbot-auto @@ -860,18 +856,7 @@ elif [ -f /etc/redhat-release ]; then fi if [ "$RPM_USE_PYTHON_3" = 1 ]; then - Bootstrap() { - BootstrapMessage "RedHat-based OSes that will use Python3" - BootstrapRpmPython3 - } USE_PYTHON_3=1 - BOOTSTRAP_VERSION="BootstrapRpmPython3 $BOOTSTRAP_RPM_PYTHON3_VERSION" - else - Bootstrap() { - BootstrapMessage "RedHat-based OSes" - BootstrapRpmCommon - } - BOOTSTRAP_VERSION="BootstrapRpmCommon $BOOTSTRAP_RPM_COMMON_VERSION" fi fi @@ -889,10 +874,7 @@ elif uname | grep -iq FreeBSD ; then elif uname | grep -iq Darwin ; then DEPRECATED_OS=1 elif [ -f /etc/issue ] && grep -iq "Amazon Linux" /etc/issue ; then - Bootstrap() { - ExperimentalBootstrap "Amazon Linux" BootstrapRpmCommon - } - BOOTSTRAP_VERSION="BootstrapRpmCommon $BOOTSTRAP_RPM_COMMON_VERSION" + DEPRECATED_OS=1 elif [ -f /etc/product ] && grep -q "Joyent Instance" /etc/product ; then DEPRECATED_OS=1 else diff --git a/letsencrypt-auto-source/letsencrypt-auto.template b/letsencrypt-auto-source/letsencrypt-auto.template index 5eb82b705..bc27469fb 100755 --- a/letsencrypt-auto-source/letsencrypt-auto.template +++ b/letsencrypt-auto-source/letsencrypt-auto.template @@ -326,6 +326,7 @@ elif [ -f /etc/mageia-release ]; then # Mageia has both /etc/mageia-release and /etc/redhat-release DEPRECATED_OS=1 elif [ -f /etc/redhat-release ]; then + DEPRECATED_OS=1 # Run DeterminePythonVersion to decide on the basis of available Python versions # whether to use 2.x or 3.x on RedHat-like systems. # Then, revert LE_PYTHON to its previous state. @@ -358,12 +359,7 @@ elif [ -f /etc/redhat-release ]; then INTERACTIVE_BOOTSTRAP=1 fi - Bootstrap() { - BootstrapMessage "Legacy RedHat-based OSes that will use Python3" - BootstrapRpmPython3Legacy - } USE_PYTHON_3=1 - BOOTSTRAP_VERSION="BootstrapRpmPython3Legacy $BOOTSTRAP_RPM_PYTHON3_LEGACY_VERSION" # Try now to enable SCL rh-python36 for systems already bootstrapped # NB: EnablePython36SCL has been defined along with BootstrapRpmPython3Legacy in certbot-auto @@ -382,18 +378,7 @@ elif [ -f /etc/redhat-release ]; then fi if [ "$RPM_USE_PYTHON_3" = 1 ]; then - Bootstrap() { - BootstrapMessage "RedHat-based OSes that will use Python3" - BootstrapRpmPython3 - } USE_PYTHON_3=1 - BOOTSTRAP_VERSION="BootstrapRpmPython3 $BOOTSTRAP_RPM_PYTHON3_VERSION" - else - Bootstrap() { - BootstrapMessage "RedHat-based OSes" - BootstrapRpmCommon - } - BOOTSTRAP_VERSION="BootstrapRpmCommon $BOOTSTRAP_RPM_COMMON_VERSION" fi fi @@ -411,10 +396,7 @@ elif uname | grep -iq FreeBSD ; then elif uname | grep -iq Darwin ; then DEPRECATED_OS=1 elif [ -f /etc/issue ] && grep -iq "Amazon Linux" /etc/issue ; then - Bootstrap() { - ExperimentalBootstrap "Amazon Linux" BootstrapRpmCommon - } - BOOTSTRAP_VERSION="BootstrapRpmCommon $BOOTSTRAP_RPM_COMMON_VERSION" + DEPRECATED_OS=1 elif [ -f /etc/product ] && grep -q "Joyent Instance" /etc/product ; then DEPRECATED_OS=1 else diff --git a/linter_plugin.py b/linter_plugin.py index 75879f73a..a19bf7df9 100644 --- a/linter_plugin.py +++ b/linter_plugin.py @@ -10,7 +10,9 @@ from pylint.checkers import BaseChecker from pylint.interfaces import IAstroidChecker # Modules in theses packages can import the os module. -WHITELIST_PACKAGES = ['acme', 'certbot_compatibility_test', 'lock_test'] +WHITELIST_PACKAGES = [ + 'acme', 'certbot_integration_tests', 'certbot_compatibility_test', 'lock_test' +] class ForbidStandardOsModule(BaseChecker): @@ -25,8 +27,8 @@ class ForbidStandardOsModule(BaseChecker): 'E5001': ( 'Forbidden use of os module, certbot.compat.os must be used instead', 'os-module-forbidden', - 'Some methods from the standard os module cannot be used for security reasons on Windows: ' - 'the safe wrapper certbot.compat.os must be used instead in Certbot.' + 'Some methods from the standard os module cannot be used for security reasons on ' + 'Windows: the safe wrapper certbot.compat.os must be used instead in Certbot.' ) } priority = -1 diff --git a/pull_request_template.md b/pull_request_template.md index c806d33e8..53298291b 100644 --- a/pull_request_template.md +++ b/pull_request_template.md @@ -1,4 +1,5 @@ ## Pull Request Checklist - [ ] If the change being made is to a [distributed component](https://certbot.eff.org/docs/contributing.html#code-components-and-layout), edit the `master` section of `certbot/CHANGELOG.md` to include a description of the change being made. +- [ ] Add or update any documentation as needed to support the changes in this PR. - [ ] Include your name in `AUTHORS.md` if you like. diff --git a/pytest.ini b/pytest.ini index 16aa9a193..b7a6928ea 100644 --- a/pytest.ini +++ b/pytest.ini @@ -4,6 +4,8 @@ [pytest] # In general, all warnings are treated as errors. Here are the exceptions: # 1- decodestring: https://github.com/rthalley/dnspython/issues/338 +# 2- Python 2 deprecation: https://github.com/certbot/certbot/issues/8388 +# (to be removed with Certbot 1.12.0 and its drop of Python 2 support) # Warnings being triggered by our plugins using deprecated features in # acme/certbot should be fixed by having our plugins no longer using the # deprecated code rather than adding them to the list of ignored warnings here. @@ -14,3 +16,4 @@ filterwarnings = error ignore:decodestring:DeprecationWarning + ignore:Python 2 support will be dropped:PendingDeprecationWarning diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml index 09d409d26..c9061ecb3 100644 --- a/snap/snapcraft.yaml +++ b/snap/snapcraft.yaml @@ -40,7 +40,6 @@ parts: certbot: plugin: python source: . - constraints: [$SNAPCRAFT_PART_SRC/snap-constraints.txt] python-packages: - git+https://github.com/certbot/python-augeas.git@certbot-patched - ./acme @@ -64,7 +63,6 @@ parts: - libpython3.8-stdlib - libpython3.8-minimal - python3-pip - - python3-setuptools - python3-wheel - python3-venv - python3-minimal @@ -74,13 +72,23 @@ parts: # To build cryptography and cffi if needed build-packages: [gcc, libffi-dev, libssl-dev, git, libaugeas-dev, python3-dev] build-environment: - - SNAPCRAFT_PYTHON_VENV_ARGS: --system-site-packages - - PIP_NO_BUILD_ISOLATION: "no" + - SNAPCRAFT_PYTHON_VENV_ARGS: --upgrade + # Constraints are passed through the environment variable PIP_CONSTRAINTS instead of using the + # parts.[part_name].constraints option available in snapcraft.yaml when the Python plugin is + # used. This is done to let these constraints be applied not only on the certbot package + # build, but also on any isolated build that pip could trigger when building wheels for + # dependencies. See https://github.com/certbot/certbot/pull/8443 for more info. + - PIP_CONSTRAINT: $SNAPCRAFT_PART_SRC/snap-constraints.txt + override-build: | + python3 -m venv "${SNAPCRAFT_PART_INSTALL}" + "${SNAPCRAFT_PART_INSTALL}/bin/python3" "${SNAPCRAFT_PART_SRC}/tools/pipstrap.py" + snapcraftctl build override-pull: | - snapcraftctl pull - cd $SNAPCRAFT_PART_SRC - python3 tools/strip_hashes.py letsencrypt-auto-source/pieces/dependency-requirements.txt | grep -v python-augeas > snap-constraints.txt - snapcraftctl set-version `grep -oP "__version__ = '\K.*(?=')" $SNAPCRAFT_PART_SRC/certbot/certbot/__init__.py` + snapcraftctl pull + python3 "${SNAPCRAFT_PART_SRC}/tools/strip_hashes.py" "${SNAPCRAFT_PART_SRC}/letsencrypt-auto-source/pieces/dependency-requirements.txt" | grep -v python-augeas >> "${SNAPCRAFT_PART_SRC}/snap-constraints.txt" + python3 "${SNAPCRAFT_PART_SRC}/tools/strip_hashes.py" "${SNAPCRAFT_PART_SRC}/tools/pipstrap_constraints.txt" >> "${SNAPCRAFT_PART_SRC}/snap-constraints.txt" + echo "$(python3 "${SNAPCRAFT_PART_SRC}/tools/merge_requirements.py" "${SNAPCRAFT_PART_SRC}/snap-constraints.txt")" > "${SNAPCRAFT_PART_SRC}/snap-constraints.txt" + snapcraftctl set-version `grep -oP "__version__ = '\K.*(?=')" "${SNAPCRAFT_PART_SRC}/certbot/certbot/__init__.py"` shared-metadata: plugin: dump source: . diff --git a/tests/letstest/README.md b/tests/letstest/README.md index 4cf6c83c3..76db57153 100644 --- a/tests/letstest/README.md +++ b/tests/letstest/README.md @@ -1,7 +1,6 @@ # letstest Simple AWS testfarm scripts for certbot client testing -- Configures (canned) boulder server - Launches EC2 instances with a given list of AMIs for different distros - Copies certbot repo and puts it on the instances - Runs certbot tests (bash scripts) on all of these @@ -56,11 +55,6 @@ It will take a minute for these instances to shut down and become available agai A folder named `letest-` is also created with a log file from each instance of the test and a file named "results" containing the output above. The tests take quite a while to run. -Also, the way all of the tests work is to check if there is already a boulder server running and if not start one. The boulder server is left running between tests, -and there are known issues if two instances of boulder attempt to be started. After starting your first test, wait until you see "Found existing boulder server:" or if you see output -about creating a boulder server, wait a minute before starting the 2nd test. You only have to do this after starting your first session of tests or after running -the `aws ec2 terminate-instances` command above. - ## Scripts Example scripts are in the 'scripts' directory, these are just bash scripts that have a few parameters passed to them at runtime via environment variables. test_apache2.sh is a useful reference. @@ -73,5 +67,4 @@ See: - https://docs.aws.amazon.com/cli/latest/userguide/cli-ec2-keypairs.html Main repos: -- https://github.com/letsencrypt/boulder - https://github.com/letsencrypt/letsencrypt diff --git a/tests/letstest/apache2_targets.yaml b/tests/letstest/apache2_targets.yaml index 8e8e23116..2663782ce 100644 --- a/tests/letstest/apache2_targets.yaml +++ b/tests/letstest/apache2_targets.yaml @@ -1,4 +1,7 @@ # These images are located in us-east-1. +# +# All machines must currently use x86_64 since Pebble does not currently +# publish images for other architectures. targets: #----------------------------------------------------------------------------- @@ -30,12 +33,6 @@ targets: type: ubuntu virt: hvm user: admin - - ami: ami-0dcd54b7d2fff584f - name: debian10_arm64 - type: ubuntu - virt: hvm - user: admin - machine_type: a1.medium - ami: ami-003f19e0e687de1cd name: debian9 type: ubuntu diff --git a/tests/letstest/auto_targets.yaml b/tests/letstest/auto_targets.yaml index 9d97c6a83..01d410227 100644 --- a/tests/letstest/auto_targets.yaml +++ b/tests/letstest/auto_targets.yaml @@ -31,10 +31,6 @@ targets: virt: hvm user: admin machine_type: a1.medium - # userdata: | - # #cloud-init - # runcmd: - # - [ apt-get, install, -y, curl ] #----------------------------------------------------------------------------- # Other Redhat Distros - ami: ami-0916c408cb02e310b diff --git a/tests/letstest/multitester.py b/tests/letstest/multitester.py index cf9f2899a..5ad1d8c15 100644 --- a/tests/letstest/multitester.py +++ b/tests/letstest/multitester.py @@ -1,7 +1,6 @@ """ Certbot Integration Test Tool -- Configures (canned) boulder server - Launches EC2 instances with a given list of AMIs for different distros - Copies certbot repo and puts it on the instances - Runs certbot tests (bash scripts) on all of these @@ -81,12 +80,6 @@ parser.add_argument('--saveinstances', parser.add_argument('--alt_pip', default='', help="server from which to pull candidate release packages") -parser.add_argument('--killboulder', - action='store_true', - help="do not leave a persistent boulder server running") -parser.add_argument('--boulderonly', - action='store_true', - help="only make a boulder server") cl_args = parser.parse_args() # Credential Variables @@ -98,7 +91,6 @@ PROFILE = None if cl_args.aws_profile == 'SET_BY_ENV' else cl_args.aws_profile # Globals #------------------------------------------------------------------------------- -BOULDER_AMI = 'ami-072a9534772bec854' # premade shared boulder AMI 18.04LTS us-east-1 SECURITY_GROUP_NAME = 'certbot-security-group' SENTINEL = None #queue kill signal SUBNET_NAME = 'certbot-subnet' @@ -133,10 +125,6 @@ def make_security_group(vpc): mysg = vpc.create_security_group(GroupName=SECURITY_GROUP_NAME, Description='security group for automated testing') mysg.authorize_ingress(IpProtocol="tcp", CidrIp="0.0.0.0/0", FromPort=22, ToPort=22) - mysg.authorize_ingress(IpProtocol="tcp", CidrIp="0.0.0.0/0", FromPort=80, ToPort=80) - mysg.authorize_ingress(IpProtocol="tcp", CidrIp="0.0.0.0/0", FromPort=443, ToPort=443) - # for boulder wfe (http) server - mysg.authorize_ingress(IpProtocol="tcp", CidrIp="0.0.0.0/0", FromPort=4000, ToPort=4000) # for mosh mysg.authorize_ingress(IpProtocol="udp", CidrIp="0.0.0.0/0", FromPort=60000, ToPort=61000) return mysg @@ -147,22 +135,32 @@ def make_instance(ec2_client, keyname, security_group_id, subnet_id, - machine_type='t2.micro', - userdata=""): #userdata contains bash or cloud-init script + self_destruct, + machine_type='t2.micro'): + """Creates an instance using the given parameters. + + If self_destruct is True, the instance will be configured to shutdown after + 1 hour and to terminate itself on shutdown. + + """ block_device_mappings = _get_block_device_mappings(ec2_client, ami_id) tags = [{'Key': 'Name', 'Value': instance_name}] tag_spec = [{'ResourceType': 'instance', 'Tags': tags}] - return ec2_client.create_instances( - BlockDeviceMappings=block_device_mappings, - ImageId=ami_id, - SecurityGroupIds=[security_group_id], - SubnetId=subnet_id, - KeyName=keyname, - MinCount=1, - MaxCount=1, - UserData=userdata, - InstanceType=machine_type, - TagSpecifications=tag_spec)[0] + kwargs = { + 'BlockDeviceMappings': block_device_mappings, + 'ImageId': ami_id, + 'SecurityGroupIds': [security_group_id], + 'SubnetId': subnet_id, + 'KeyName': keyname, + 'MinCount': 1, + 'MaxCount': 1, + 'InstanceType': machine_type, + 'TagSpecifications': tag_spec + } + if self_destruct: + kwargs['InstanceInitiatedShutdownBehavior'] = 'terminate' + kwargs['UserData'] = '#!/bin/bash\nshutdown -P +60\n' + return ec2_client.create_instances(**kwargs)[0] def _get_block_device_mappings(ec2_client, ami_id): """Returns the list of block device mappings to ensure cleanup. @@ -183,23 +181,6 @@ def _get_block_device_mappings(ec2_client, ami_id): # Helper Routines #------------------------------------------------------------------------------- -def block_until_http_ready(urlstring, wait_time=10, timeout=240): - "Blocks until server at urlstring can respond to http requests" - server_ready = False - t_elapsed = 0 - while not server_ready and t_elapsed < timeout: - try: - sys.stdout.write('.') - sys.stdout.flush() - req = urllib_request.Request(urlstring) - response = urllib_request.urlopen(req) - #if response.code == 200: - server_ready = True - except urllib_error.URLError: - pass - time.sleep(wait_time) - t_elapsed += wait_time - def block_until_ssh_open(ipstring, wait_time=10, timeout=120): "Blocks until server at ipstring has an open port 22" reached = False @@ -278,26 +259,15 @@ def deploy_script(cxn, scriptpath, *args): args_str = ' '.join(args) cxn.run('./'+scriptfile+' '+args_str) -def run_boulder(cxn): - boulder_path = '$GOPATH/src/github.com/letsencrypt/boulder' - cxn.run('cd %s && sudo docker-compose up -d' % boulder_path) - -def config_and_launch_boulder(cxn, instance): - # yes, we're hardcoding the gopath. it's a predetermined AMI. - with cxn.prefix('export GOPATH=/home/ubuntu/gopath'): - deploy_script(cxn, 'scripts/boulder_config.sh') - run_boulder(cxn) - -def install_and_launch_certbot(cxn, instance, boulder_url, target, log_dir): +def install_and_launch_certbot(cxn, instance, target, log_dir): local_repo_to_remote(cxn, log_dir) # This needs to be like this, I promise. 1) The env argument to run doesn't work. # See https://github.com/fabric/fabric/issues/1744. 2) prefix() sticks an && between # the commands, so it needs to be exports rather than no &&s in between for the script subshell. - with cxn.prefix('export BOULDER_URL=%s && export PUBLIC_IP=%s && export PRIVATE_IP=%s && ' + with cxn.prefix('export PUBLIC_IP=%s && export PRIVATE_IP=%s && ' 'export PUBLIC_HOSTNAME=%s && export PIP_EXTRA_INDEX_URL=%s && ' 'export OS_TYPE=%s' % - (boulder_url, - instance.public_ip_address, + (instance.public_ip_address, instance.private_ip_address, instance.public_dns_name, cl_args.alt_pip, @@ -313,7 +283,7 @@ def grab_certbot_log(cxn): 'cat ./certbot.log; else echo "[nolocallog]"; fi\'') -def create_client_instance(ec2_client, target, security_group_id, subnet_id): +def create_client_instance(ec2_client, target, security_group_id, subnet_id, self_destruct): """Create a single client instance for running tests.""" if 'machine_type' in target: machine_type = target['machine_type'] @@ -322,10 +292,6 @@ def create_client_instance(ec2_client, target, security_group_id, subnet_id): else: # 32 bit systems machine_type = 'c1.medium' - if 'userdata' in target: - userdata = target['userdata'] - else: - userdata = '' name = 'le-%s'%target['name'] print(name, end=" ") return make_instance(ec2_client, @@ -335,10 +301,10 @@ def create_client_instance(ec2_client, target, security_group_id, subnet_id): machine_type=machine_type, security_group_id=security_group_id, subnet_id=subnet_id, - userdata=userdata) + self_destruct=self_destruct) -def test_client_process(fab_config, inqueue, outqueue, boulder_url, log_dir): +def test_client_process(fab_config, inqueue, outqueue, log_dir): cur_proc = mp.current_process() for inreq in iter(inqueue.get, SENTINEL): ii, instance_id, target = inreq @@ -360,7 +326,7 @@ def test_client_process(fab_config, inqueue, outqueue, boulder_url, log_dir): with Connection(host_string, config=fab_config) as cxn: try: - install_and_launch_certbot(cxn, instance, boulder_url, target, log_dir) + install_and_launch_certbot(cxn, instance, target, log_dir) outqueue.put((ii, target, Status.PASS)) print("%s - %s SUCCESS"%(target['ami'], target['name'])) except: @@ -379,15 +345,13 @@ def test_client_process(fab_config, inqueue, outqueue, boulder_url, log_dir): pass -def cleanup(cl_args, instances, targetlist, boulder_server, log_dir): +def cleanup(cl_args, instances, targetlist, log_dir): print('Logs in ', log_dir) # If lengths of instances and targetlist aren't equal, instances failed to # start before running tests so leaving instances running for debugging # isn't very useful. Let's cleanup after ourselves instead. if len(instances) != len(targetlist) or not cl_args.saveinstances: print('Terminating EC2 Instances') - if cl_args.killboulder: - boulder_server.terminate() for instance in instances: instance.terminate() else: @@ -477,63 +441,18 @@ def main(): security_group_id = make_security_group(vpc).id time.sleep(30) - boulder_preexists = False - boulder_servers = ec2_client.instances.filter(Filters=[ - {'Name': 'tag:Name', 'Values': ['le-boulderserver']}, - {'Name': 'instance-state-name', 'Values': ['running']}]) - - boulder_server = next(iter(boulder_servers), None) - - print("Requesting Instances...") - if boulder_server: - print("Found existing boulder server:", boulder_server) - boulder_preexists = True - else: - print("Can't find a boulder server, starting one...") - boulder_server = make_instance(ec2_client, - 'le-boulderserver', - BOULDER_AMI, - KEYNAME, - machine_type='t2.micro', - #machine_type='t2.medium', - security_group_id=security_group_id, - subnet_id=subnet_id) - instances = [] try: - if not cl_args.boulderonly: - print("Creating instances: ", end="") - for target in targetlist: - instances.append( - create_client_instance(ec2_client, target, - security_group_id, subnet_id) - ) - print() - - # Configure and launch boulder server - #------------------------------------------------------------------------------- - print("Waiting on Boulder Server") - boulder_server = block_until_instance_ready(boulder_server) - print(" server %s"%boulder_server) - - - # host_string defines the ssh user and host for connection - host_string = "ubuntu@%s"%boulder_server.public_ip_address - print("Boulder Server at (SSH):", host_string) - if not boulder_preexists: - print("Configuring and Launching Boulder") - with Connection(host_string, config=fab_config) as boulder_cxn: - config_and_launch_boulder(boulder_cxn, boulder_server) - # blocking often unnecessary, but cheap EC2 VMs can get very slow - block_until_http_ready('http://%s:4000'%boulder_server.public_ip_address, - wait_time=10, timeout=500) - - boulder_url = "http://%s:4000/directory"%boulder_server.private_ip_address - print("Boulder Server at (public ip): http://%s:4000/directory"%boulder_server.public_ip_address) - print("Boulder Server at (EC2 private ip): %s"%boulder_url) - - if cl_args.boulderonly: - sys.exit(0) + print("Creating instances: ", end="") + # If we want to preserve instances, do not have them self-destruct. + self_destruct = not cl_args.saveinstances + for target in targetlist: + instances.append( + create_client_instance(ec2_client, target, + security_group_id, subnet_id, + self_destruct) + ) + print() # Install and launch client scripts in parallel #------------------------------------------------------------------------------- @@ -551,7 +470,7 @@ def main(): # initiate process execution - client_process_args=(fab_config, inqueue, outqueue, boulder_url, log_dir) + client_process_args=(fab_config, inqueue, outqueue, log_dir) for i in range(num_processes): p = mp.Process(target=test_client_process, args=client_process_args) jobs.append(p) @@ -602,7 +521,7 @@ def main(): sys.exit(1) finally: - cleanup(cl_args, instances, targetlist, boulder_server, log_dir) + cleanup(cl_args, instances, targetlist, log_dir) if __name__ == '__main__': diff --git a/tests/letstest/scripts/bootstrap_os_packages.sh b/tests/letstest/scripts/bootstrap_os_packages.sh index 96506282b..7ad93f63e 100755 --- a/tests/letstest/scripts/bootstrap_os_packages.sh +++ b/tests/letstest/scripts/bootstrap_os_packages.sh @@ -98,41 +98,6 @@ BootstrapRpmCommonBase() { fi } -# This bootstrap concerns old RedHat-based distributions that do not ship by default -# with Python 2.7, but only Python 2.6. We bootstrap them by enabling SCL and installing -# Python 3.6. Some of these distributions are: CentOS/RHEL/OL/SL 6. -BootstrapRpmPython3Legacy() { - # Tested with: - # - CentOS 6 - - InitializeRPMCommonBase - - if ! "${TOOL}" list rh-python36 >/dev/null 2>&1; then - echo "To use Certbot on this operating system, packages from the SCL repository need to be installed." - if ! "${TOOL}" list centos-release-scl >/dev/null 2>&1; then - error "Enable the SCL repository and try running Certbot again." - exit 1 - fi - if ! "${TOOL}" install -y centos-release-scl; then - error "Could not enable SCL. Aborting bootstrap!" - exit 1 - fi - fi - - # CentOS 6 must use rh-python36 from SCL - if "${TOOL}" list rh-python36 >/dev/null 2>&1; then - python_pkgs="rh-python36-python - rh-python36-python-virtualenv - rh-python36-python-devel - " - else - error "No supported Python package available to install. Aborting bootstrap!" - exit 1 - fi - - BootstrapRpmCommonBase "${python_pkgs}" -} - BootstrapRpmPython3() { InitializeRPMCommonBase @@ -154,16 +119,9 @@ if [ -f /etc/debian_version ]; then } elif [ -f /etc/redhat-release ]; then DeterminePythonVersion - # Handle legacy RPM distributions - if [ "$PYVER" -eq 26 ]; then - Bootstrap() { - BootstrapRpmPython3Legacy - } - else - Bootstrap() { - BootstrapRpmPython3 - } - fi + Bootstrap() { + BootstrapRpmPython3 + } fi diff --git a/tests/letstest/scripts/boulder_config.sh b/tests/letstest/scripts/boulder_config.sh deleted file mode 100755 index b99bbabbe..000000000 --- a/tests/letstest/scripts/boulder_config.sh +++ /dev/null @@ -1,24 +0,0 @@ -#!/bin/bash -x - -# Configures and Launches Boulder Server installed on -# us-east-1 ami-072a9534772bec854 bouldertestserver3 (boulder commit b24fe7c3ea4) - -# fetch instance data from EC2 metadata service -public_host=$(curl -s http://169.254.169.254/2014-11-05/meta-data/public-hostname) -public_ip=$(curl -s http://169.254.169.254/2014-11-05/meta-data/public-ipv4) -private_ip=$(curl -s http://169.254.169.254/2014-11-05/meta-data/local-ipv4) - -# set to public DNS resolver -resolver_ip=8.8.8.8 -resolver=$resolver_ip':53' - -# modifies integration testing boulder setup for local AWS VPC network -# connections instead of localhost -cd $GOPATH/src/github.com/letsencrypt/boulder -# change test ports to real -sed -i '/httpPort/ s/5002/80/' ./test/config/va.json -sed -i '/httpsPort/ s/5001/443/' ./test/config/va.json -sed -i '/tlsPort/ s/5001/443/' ./test/config/va.json -# set dns resolver -sed -i 's/"127.0.0.1:8053",/"'$resolver'"/' ./test/config/va.json -sed -i 's/"127.0.0.1:8054"//' ./test/config/va.json diff --git a/tests/letstest/scripts/boulder_install.sh b/tests/letstest/scripts/boulder_install.sh deleted file mode 100755 index 5161de374..000000000 --- a/tests/letstest/scripts/boulder_install.sh +++ /dev/null @@ -1,8 +0,0 @@ -#!/bin/bash -x - -# Check out special branch until latest docker changes land in Boulder master. -git clone -b docker-integration https://github.com/letsencrypt/boulder $BOULDERPATH -cd $BOULDERPATH -FAKE_DNS=$(ifconfig docker0 | grep "inet addr:" | cut -d: -f2 | awk '{ print $1}') -sed -i "s/FAKE_DNS: .*/FAKE_DNS: $FAKE_DNS/" docker-compose.yml -docker-compose up -d diff --git a/tests/letstest/scripts/test_apache2.sh b/tests/letstest/scripts/test_apache2.sh index ba3d94379..247191610 100755 --- a/tests/letstest/scripts/test_apache2.sh +++ b/tests/letstest/scripts/test_apache2.sh @@ -7,7 +7,7 @@ if [ "$OS_TYPE" = "ubuntu" ] then CONFFILE=/etc/apache2/sites-available/000-default.conf sudo apt-get update - sudo apt-get -y --no-upgrade install apache2 #curl + sudo apt-get -y --no-upgrade install apache2 curl sudo apt-get -y install realpath # needed for test-apache-conf # For apache 2.4, set up ServerName sudo sed -i '/ServerName/ s/#ServerName/ServerName/' $CONFFILE @@ -64,17 +64,41 @@ if [ $? -ne 0 ] ; then exit 1 fi -if command -v python && [ $(python -V 2>&1 | cut -d" " -f 2 | cut -d. -f1,2 | sed 's/\.//') -eq 26 ]; then - # RHEL/CentOS 6 will need a special treatment, so we need to detect that environment - # Enable the SCL Python 3.6 installed by letsencrypt-auto bootstrap - PATH="/opt/rh/rh-python36/root/usr/bin:$PATH" +tools/venv3.py -e acme[dev] -e certbot[dev,docs] -e certbot-apache -e certbot-ci +PEBBLE_LOGS="acme_server.log" +PEBBLE_URL="https://localhost:14000/dir" +# We configure Pebble to use port 80 for http-01 validation rather than an +# alternate port because: +# 1) It allows us to test with Apache configurations that are more realistic +# and closer to the default configuration on various OSes. +# 2) As of writing this, Certbot's Apache plugin requires there to be an +# existing virtual host for the port used for http-01 validation. +venv3/bin/run_acme_server --http-01-port 80 > "${PEBBLE_LOGS}" 2>&1 & + +DumpPebbleLogs() { + if [ -f "${PEBBLE_LOGS}" ] ; then + echo "Pebble's logs were:" + cat "${PEBBLE_LOGS}" + fi +} + +for n in $(seq 1 150) ; do + if curl --insecure "${PEBBLE_URL}" 2>/dev/null; then + break + else + echo "waiting for pebble" + sleep 1 + fi +done +if ! curl --insecure "${PEBBLE_URL}" 2>/dev/null; then + echo "timed out waiting for pebble to start" + DumpPebbleLogs + exit 1 fi -tools/venv3.py -e acme[dev] -e certbot[dev,docs] -e certbot-apache - -sudo "venv3/bin/certbot" -v --debug --text --agree-tos \ +sudo "venv3/bin/certbot" -v --debug --text --agree-tos --no-verify-ssl \ --renew-by-default --redirect --register-unsafely-without-email \ - --domain $PUBLIC_HOSTNAME --server $BOULDER_URL + --domain "${PUBLIC_HOSTNAME}" --server "${PEBBLE_URL}" if [ $? -ne 0 ] ; then FAIL=1 fi @@ -96,7 +120,7 @@ fi if [ "$OS_TYPE" = "ubuntu" ] ; then - export SERVER="$BOULDER_URL" + export SERVER="${PEBBLE_URL}" "venv3/bin/tox" -e apacheconftest else echo Not running hackish apache tests on $OS_TYPE @@ -108,5 +132,6 @@ fi # return error if any of the subtests failed if [ "$FAIL" = 1 ] ; then + DumpPebbleLogs exit 1 fi diff --git a/tests/letstest/scripts/test_leauto_upgrades.sh b/tests/letstest/scripts/test_leauto_upgrades.sh index 51ff640c5..c599623cb 100755 --- a/tests/letstest/scripts/test_leauto_upgrades.sh +++ b/tests/letstest/scripts/test_leauto_upgrades.sh @@ -1,7 +1,7 @@ #!/bin/bash -xe set -o pipefail -# $OS_TYPE $PUBLIC_IP $PRIVATE_IP $PUBLIC_HOSTNAME $BOULDER_URL +# $OS_TYPE $PUBLIC_IP $PRIVATE_IP $PUBLIC_HOSTNAME # are dynamically set at execution cd letsencrypt @@ -105,15 +105,10 @@ if ./letsencrypt-auto -v --debug --version | grep "WARNING: couldn't find Python exit 1 fi -# On systems like Debian where certbot-auto is deprecated, we expect it to -# leave existing Certbot installations unmodified so we check for the same -# version that was initially installed below. Once certbot-auto is deprecated -# on RHEL systems, we can unconditionally check for INITIAL_VERSION. -if [ -f /etc/debian_version ]; then - EXPECTED_VERSION="$INITIAL_VERSION" -else - EXPECTED_VERSION=$(grep -m1 LE_AUTO_VERSION certbot-auto | cut -d\" -f2) -fi +# Since certbot-auto is deprecated, we expect it to leave existing Certbot +# installations unmodified so we check for the same version that was initially +# installed below. +EXPECTED_VERSION="$INITIAL_VERSION" if ! /opt/eff.org/certbot/venv/bin/letsencrypt --version 2>&1 | tail -n1 | grep "^certbot $EXPECTED_VERSION$" ; then echo unexpected certbot version found @@ -124,22 +119,3 @@ if ! diff letsencrypt-auto letsencrypt-auto-source/letsencrypt-auto ; then echo letsencrypt-auto and letsencrypt-auto-source/letsencrypt-auto differ exit 1 fi - -if [ "$RUN_RHEL6_TESTS" = 1 ]; then - # Add the SCL python release to PATH in order to resolve python3 command - PATH="/opt/rh/rh-python36/root/usr/bin:$PATH" - if ! command -v python3; then - echo "Python3 wasn't properly installed" - exit 1 - fi - if [ "$(/opt/eff.org/certbot/venv/bin/python -V 2>&1 | cut -d" " -f 2 | cut -d. -f1)" != 3 ]; then - echo "Python3 wasn't used in venv!" - exit 1 - fi - - if [ "$("$PYTHON_NAME" tools/readlink.py $OLD_VENV_PATH)" != "/opt/eff.org/certbot/venv" ]; then - echo symlink from old venv path not properly created! - exit 1 - fi -fi -echo upgrade appeared to be successful diff --git a/tests/letstest/scripts/test_letsencrypt_auto_certonly_standalone.sh b/tests/letstest/scripts/test_letsencrypt_auto_certonly_standalone.sh index 15cf9ee1b..9573ab690 100755 --- a/tests/letstest/scripts/test_letsencrypt_auto_certonly_standalone.sh +++ b/tests/letstest/scripts/test_letsencrypt_auto_certonly_standalone.sh @@ -1,7 +1,7 @@ #!/bin/bash -x set -eo pipefail -# $PUBLIC_IP $PRIVATE_IP $PUBLIC_HOSTNAME $BOULDER_URL are dynamically set at execution +# $PUBLIC_IP $PRIVATE_IP $PUBLIC_HOSTNAME are dynamically set at execution # with curl, instance metadata available from EC2 metadata service: #public_host=$(curl -s http://169.254.169.254/2014-11-05/meta-data/public-hostname) @@ -16,58 +16,14 @@ sudo chown root "$LE_AUTO_PATH" sudo chmod 0755 "$LE_AUTO_PATH" export PATH="$LE_AUTO_DIR:$PATH" -# On systems like Debian where certbot-auto is deprecated, we expect -# certbot-auto to error and refuse to install Certbot. Once certbot-auto is -# deprecated on RHEL systems, we can unconditionally run this code. -if [ -f /etc/debian_version ]; then - set +o pipefail - if ! letsencrypt-auto --debug --version | grep "Certbot cannot be installed."; then - echo "letsencrypt-auto didn't report being uninstallable." - exit 1 - fi - if [ ${PIPESTATUS[0]} != 1 ]; then - echo "letsencrypt-auto didn't exit with status 1 as expected" - exit 1 - fi - # letsencrypt-auto is deprecated and cannot be installed on this system so - # we cannot run the rest of this test. - exit 0 -fi - -letsencrypt-auto --os-packages-only --debug --version - -# This script sets the environment variables PYTHON_NAME, VENV_PATH, and -# VENV_SCRIPT based on the version of Python available on the system. For -# instance, Fedora uses Python 3 and Python 2 is not installed. -. tests/letstest/scripts/set_python_envvars.sh - -# Create a venv-like layout at the old virtual environment path to test that a -# symlink is properly created when letsencrypt-auto runs. -HOME=${HOME:-~root} -XDG_DATA_HOME=${XDG_DATA_HOME:-~/.local/share} -OLD_VENV_BIN="$XDG_DATA_HOME/letsencrypt/bin" -mkdir -p "$OLD_VENV_BIN" -touch "$OLD_VENV_BIN/letsencrypt" - -letsencrypt-auto certonly --no-self-upgrade -v --standalone --debug \ - --text --agree-tos \ - --renew-by-default --redirect \ - --register-unsafely-without-email \ - --domain $PUBLIC_HOSTNAME --server $BOULDER_URL - -LINK_PATH=$("$PYTHON_NAME" tools/readlink.py ${XDG_DATA_HOME:-~/.local/share}/letsencrypt) -if [ "$LINK_PATH" != "/opt/eff.org/certbot/venv" ]; then - echo symlink from old venv path not properly created! +# Since certbot-auto is deprecated, we expect certbot-auto to error and +# refuse to install Certbot. +set +o pipefail +if ! letsencrypt-auto --debug --version | grep "Certbot cannot be installed."; then + echo "letsencrypt-auto didn't report being uninstallable." exit 1 fi - -if ! letsencrypt-auto --help --no-self-upgrade | grep -F "letsencrypt-auto [SUBCOMMAND]"; then - echo "letsencrypt-auto not included in help output!" - exit 1 -fi - -OUTPUT_LEN=$(letsencrypt-auto --install-only --no-self-upgrade --quiet 2>&1 | wc -c) -if [ "$OUTPUT_LEN" != 0 ]; then - echo letsencrypt-auto produced unexpected output! +if [ ${PIPESTATUS[0]} != 1 ]; then + echo "letsencrypt-auto didn't exit with status 1 as expected" exit 1 fi diff --git a/tests/letstest/scripts/test_sdists.sh b/tests/letstest/scripts/test_sdists.sh index e3d9a8b80..a038caff6 100755 --- a/tests/letstest/scripts/test_sdists.sh +++ b/tests/letstest/scripts/test_sdists.sh @@ -8,12 +8,6 @@ VENV_PATH=venv3 # install OS packages sudo $BOOTSTRAP_SCRIPT -if command -v python && [ $(python -V 2>&1 | cut -d" " -f 2 | cut -d. -f1,2 | sed 's/\.//') -eq 26 ]; then - # RHEL/CentOS 6 will need a special treatment, so we need to detect that environment - # Enable the SCL Python 3.6 installed by letsencrypt-auto bootstrap - PATH="/opt/rh/rh-python36/root/usr/bin:$PATH" -fi - # setup venv # We strip the hashes because the venv creation script includes unhashed # constraints in the commands given to pip and the mix of hashed and unhashed diff --git a/tests/letstest/scripts/test_tests.sh b/tests/letstest/scripts/test_tests.sh index f62584709..f07e3b78e 100755 --- a/tests/letstest/scripts/test_tests.sh +++ b/tests/letstest/scripts/test_tests.sh @@ -15,12 +15,6 @@ VENV_SCRIPT="tools/venv3.py" sudo $BOOTSTRAP_SCRIPT -if command -v python && [ $(python -V 2>&1 | cut -d" " -f 2 | cut -d. -f1,2 | sed 's/\.//') -eq 26 ]; then - # RHEL/CentOS 6 will need a special treatment, so we need to detect that environment - # Enable the SCL Python 3.6 installed by letsencrypt-auto bootstrap - PATH="/opt/rh/rh-python36/root/usr/bin:$PATH" -fi - cd $REPO_ROOT $VENV_SCRIPT . $VENV_NAME/bin/activate diff --git a/tools/docker/core/Dockerfile b/tools/docker/core/Dockerfile index 02222008b..0d3626853 100644 --- a/tools/docker/core/Dockerfile +++ b/tools/docker/core/Dockerfile @@ -42,9 +42,7 @@ RUN apk add --no-cache --virtual .build-deps \ musl-dev \ libffi-dev \ && python tools/pipstrap.py \ - && pip install --no-build-isolation \ - -r letsencrypt-auto-source/pieces/dependency-requirements.txt \ - && pip install --no-build-isolation --no-cache-dir --no-deps \ - --editable src/acme \ - --editable src/certbot \ -&& apk del .build-deps + && python tools/pip_install.py --no-cache-dir \ + --editable src/acme \ + --editable src/certbot \ + && apk del .build-deps diff --git a/tools/docker/plugin/Dockerfile b/tools/docker/plugin/Dockerfile index 5a6673e5b..863efd105 100644 --- a/tools/docker/plugin/Dockerfile +++ b/tools/docker/plugin/Dockerfile @@ -11,4 +11,4 @@ COPY qemu-${QEMU_ARCH}-static /usr/bin/ COPY . /opt/certbot/src/plugin # Install the DNS plugin -RUN tools/pip_install.py --no-cache-dir --editable /opt/certbot/src/plugin +RUN python tools/pip_install.py --no-cache-dir --editable /opt/certbot/src/plugin diff --git a/tools/finish_release.py b/tools/finish_release.py index bc8e832df..24e20987f 100755 --- a/tools/finish_release.py +++ b/tools/finish_release.py @@ -21,6 +21,7 @@ Run: python tools/finish_release.py ~/.ssh/githubpat.txt """ +import argparse import glob import os.path import re @@ -44,6 +45,34 @@ SNAPS = ['certbot'] + DNS_PLUGINS # for sanity checking. SNAP_ARCH_COUNT = 3 + +def parse_args(args): + """Parse command line arguments. + + :param args: command line arguments with the program name removed. This is + usually taken from sys.argv[1:]. + :type args: `list` of `str` + + :returns: parsed arguments + :rtype: argparse.Namespace + + """ + # Use the file's docstring for the help text and don't let argparse reformat it. + parser = argparse.ArgumentParser(description=__doc__, + formatter_class=argparse.RawDescriptionHelpFormatter) + parser.add_argument('githubpat', help='path to your GitHub personal access token') + group = parser.add_mutually_exclusive_group() + # We use 'store_false' and a destination related to the other type of + # artifact to cause the flag being set to disable publishing of the other + # artifact. This makes using the parsed arguments later on a little simpler + # and cleaner. + group.add_argument('--snaps-only', action='store_false', dest='publish_windows', + help='Skip publishing other artifacts and only publish the snaps') + group.add_argument('--windows-only', action='store_false', dest='publish_snaps', + help='Skip publishing other artifacts and only publish the Windows installer') + return parser.parse_args(args) + + def download_azure_artifacts(tempdir): """Download and unzip build artifacts from Azure pipelines. @@ -181,8 +210,9 @@ def promote_snaps(version): def main(args): - github_access_token_file = args[0] + parsed_args = parse_args(args) + github_access_token_file = parsed_args.githubpat github_access_token = open(github_access_token_file, 'r').read().rstrip() with tempfile.TemporaryDirectory() as tempdir: @@ -191,8 +221,10 @@ def main(args): # again fails. Publishing the snaps can be done multiple times though # so we do that first to make it easier to run the script again later # if something goes wrong. - promote_snaps(version) - create_github_release(github_access_token, tempdir, version) + if parsed_args.publish_snaps: + promote_snaps(version) + if parsed_args.publish_windows: + create_github_release(github_access_token, tempdir, version) if __name__ == "__main__": main(sys.argv[1:]) diff --git a/tools/pip_install.py b/tools/pip_install.py index f963e4660..c1c81482b 100755 --- a/tools/pip_install.py +++ b/tools/pip_install.py @@ -59,9 +59,13 @@ def certbot_normal_processing(tools_path, test_constraints): certbot_requirements = os.path.normpath(os.path.join( repo_path, 'letsencrypt-auto-source/pieces/dependency-requirements.txt')) with open(certbot_requirements, 'r') as fd: - data = fd.readlines() + certbot_reqs = fd.readlines() + with open(os.path.join(tools_path, 'pipstrap_constraints.txt'), 'r') as fd: + pipstrap_reqs = fd.readlines() with open(test_constraints, 'w') as fd: - data = "\n".join(strip_hashes.process_entries(data)) + data_certbot = "\n".join(strip_hashes.process_entries(certbot_reqs)) + data_pipstrap = "\n".join(strip_hashes.process_entries(pipstrap_reqs)) + data = "\n".join([data_certbot, data_pipstrap]) fd.write(data) @@ -72,7 +76,8 @@ def merge_requirements(tools_path, requirements, test_constraints, all_constrain # Here is the order by increasing priority: # 1) The general development constraints (tools/dev_constraints.txt) # 2) The general tests constraints (oldest_requirements.txt or - # certbot-auto's dependency-requirements.txt for the normal processing) + # certbot-auto's dependency-requirements.txt + pipstrap's constraints + # for the normal processing) # 3) The local requirement file, typically local-oldest-requirement in oldest tests files = [os.path.join(tools_path, 'dev_constraints.txt'), test_constraints] if requirements: @@ -82,17 +87,18 @@ def merge_requirements(tools_path, requirements, test_constraints, all_constrain fd.write(merged_requirements) -def call_with_print(command): +def call_with_print(command, env=None): + if not env: + env = os.environ print(command) - subprocess.check_call(command, shell=True) + subprocess.check_call(command, shell=True, env=env) -def pip_install_with_print(args_str, disable_build_isolation=True): - command = ['"', sys.executable, '" -m pip install --disable-pip-version-check '] - if disable_build_isolation: - command.append('--no-build-isolation ') - command.append(args_str) - call_with_print(''.join(command)) +def pip_install_with_print(args_str, env=None): + if not env: + env = os.environ + command = ['"', sys.executable, '" -m pip install --disable-pip-version-check ', args_str] + call_with_print(''.join(command), env=env) def main(args): @@ -113,20 +119,22 @@ def main(args): else: certbot_normal_processing(tools_path, test_constraints) + env = os.environ.copy() + env["PIP_CONSTRAINT"] = all_constraints + merge_requirements(tools_path, requirements, test_constraints, all_constraints) if requirements: # This branch is executed during the oldest tests # First step, install the transitive dependencies of oldest requirements # in respect with oldest constraints. - pip_install_with_print('--constraint "{0}" --requirement "{1}"' - .format(all_constraints, requirements)) + pip_install_with_print('--requirement "{0}"'.format(requirements), + env=env) # Second step, ensure that oldest requirements themselves are effectively # installed using --force-reinstall, and avoid corner cases like the one described # in https://github.com/certbot/certbot/issues/7014. pip_install_with_print('--force-reinstall --no-deps --requirement "{0}"' .format(requirements)) - pip_install_with_print('--constraint "{0}" {1}'.format( - all_constraints, ' '.join(args))) + pip_install_with_print(' '.join(args), env=env) if __name__ == '__main__': diff --git a/tools/pipstrap.py b/tools/pipstrap.py index 2f21a9a5f..e6b746916 100755 --- a/tools/pipstrap.py +++ b/tools/pipstrap.py @@ -1,46 +1,17 @@ #!/usr/bin/env python """Uses pip to upgrade Python packaging tools to pinned versions.""" from __future__ import absolute_import - import os -import shutil -import tempfile import pip_install -# We include the hashes of the packages here for extra verification of -# the packages downloaded from PyPI. This is especially valuable in our -# builds of Certbot that we ship to our users such as our Docker images. -# -# An older version of setuptools is currently used here in order to keep -# compatibility with Python 2 since newer versions of setuptools have dropped -# support for it. -REQUIREMENTS = r""" -pip==20.2.4 \ - --hash=sha256:51f1c7514530bd5c145d8f13ed936ad6b8bfcb8cf74e10403d0890bc986f0033 \ - --hash=sha256:85c99a857ea0fb0aedf23833d9be5c40cf253fe24443f0829c7b472e23c364a1 -setuptools==44.1.1 \ - --hash=sha256:27a714c09253134e60a6fa68130f78c7037e5562c4f21f8f318f2ae900d152d5 \ - --hash=sha256:c67aa55db532a0dadc4d2e20ba9961cbd3ccc84d544e9029699822542b5a476b -wheel==0.35.1 \ - --hash=sha256:497add53525d16c173c2c1c733b8f655510e909ea78cc0e29d374243544b77a2 \ - --hash=sha256:99a22d87add3f634ff917310a3d87e499f19e663413a52eb9232c447aa646c9f -""" +_REQUIREMENTS_PATH = os.path.join(os.path.dirname(__file__), "pipstrap_constraints.txt") def main(): - with pip_install.temporary_directory() as tempdir: - requirements_filepath = os.path.join(tempdir, 'reqs.txt') - with open(requirements_filepath, 'w') as f: - f.write(REQUIREMENTS) - pip_install_args = '--requirement ' + requirements_filepath - # We don't disable build isolation because we may have an older - # version of pip that doesn't support the flag disabling it. We - # expect these packages to already have usable wheels available - # anyway so no building should be required. - pip_install.pip_install_with_print(pip_install_args, - disable_build_isolation=False) + pip_install_args = '--requirement "{0}"'.format(_REQUIREMENTS_PATH) + pip_install.pip_install_with_print(pip_install_args) if __name__ == '__main__': diff --git a/tools/pipstrap_constraints.txt b/tools/pipstrap_constraints.txt new file mode 100644 index 000000000..5de9e147d --- /dev/null +++ b/tools/pipstrap_constraints.txt @@ -0,0 +1,18 @@ +# Constraints for pipstrap.py +# +# We include the hashes of the packages here for extra verification of +# the packages downloaded from PyPI. This is especially valuable in our +# builds of Certbot that we ship to our users such as our Docker images. +# +# An older version of setuptools is currently used here in order to keep +# compatibility with Python 2 since newer versions of setuptools have dropped +# support for it. +pip==20.2.4 \ + --hash=sha256:51f1c7514530bd5c145d8f13ed936ad6b8bfcb8cf74e10403d0890bc986f0033 \ + --hash=sha256:85c99a857ea0fb0aedf23833d9be5c40cf253fe24443f0829c7b472e23c364a1 +setuptools==44.1.1 \ + --hash=sha256:27a714c09253134e60a6fa68130f78c7037e5562c4f21f8f318f2ae900d152d5 \ + --hash=sha256:c67aa55db532a0dadc4d2e20ba9961cbd3ccc84d544e9029699822542b5a476b +wheel==0.35.1 \ + --hash=sha256:497add53525d16c173c2c1c733b8f655510e909ea78cc0e29d374243544b77a2 \ + --hash=sha256:99a22d87add3f634ff917310a3d87e499f19e663413a52eb9232c447aa646c9f diff --git a/tools/snap/build_remote.py b/tools/snap/build_remote.py index 285521190..e6a44240f 100755 --- a/tools/snap/build_remote.py +++ b/tools/snap/build_remote.py @@ -1,22 +1,22 @@ #!/usr/bin/env python3 import argparse -import glob import datetime -from multiprocessing import Pool, Process, Manager, Event +import glob import re import subprocess import sys -import tempfile +import time +from multiprocessing import Pool, Process, Manager from os.path import join, realpath, dirname, basename, exists - CERTBOT_DIR = dirname(dirname(dirname(realpath(__file__)))) PLUGINS = [basename(path) for path in glob.glob(join(CERTBOT_DIR, 'certbot-dns-*'))] def _execute_build(target, archs, status, workspace): process = subprocess.Popen([ - 'snapcraft', 'remote-build', '--launchpad-accept-public-upload', '--recover', '--build-on', ','.join(archs) + 'snapcraft', 'remote-build', '--launchpad-accept-public-upload', '--recover', + '--build-on', ','.join(archs) ], stdout=subprocess.PIPE, stderr=subprocess.STDOUT, universal_newlines=True, cwd=workspace) process_output = [] @@ -27,7 +27,7 @@ def _execute_build(target, archs, status, workspace): return process.wait(), process_output -def _build_snap(target, archs, status, lock): +def _build_snap(target, archs, status, running, lock): status[target] = {arch: '...' for arch in archs} if target == 'certbot': @@ -39,7 +39,8 @@ def _build_snap(target, archs, status, lock): while retry: exit_code, process_output = _execute_build(target, archs, status, workspace) - print(f'Build {target} for {",".join(archs)} (attempt {4-retry}/3) ended with exit code {exit_code}.') + print(f'Build {target} for {",".join(archs)} (attempt {4-retry}/3) ended with ' + f'exit code {exit_code}.') sys.stdout.flush() with lock: @@ -49,7 +50,8 @@ def _build_snap(target, archs, status, lock): # We expect to have all target snaps available, or something bad happened. snaps_list = glob.glob(join(workspace, '*.snap')) if not len(snaps_list) == len(archs): - print(f'Some of the expected snaps for a successful build are missing (current list: {snaps_list}).') + print('Some of the expected snaps for a successful build are missing ' + f'(current list: {snaps_list}).') dump_output = True else: break @@ -63,9 +65,12 @@ def _build_snap(target, archs, status, lock): print(f'Dumping snapcraft remote-build output build for {target}:') print('\n'.join(process_output)) - # Retry the remote build if it has been interrupted (non zero status code) or if some builds have failed. + # Retry the remote build if it has been interrupted (non zero status code) + # or if some builds have failed. retry = retry - 1 + running[target] = False + return {target: workspace} @@ -96,15 +101,11 @@ def _dump_status_helper(archs, status): sys.stdout.flush() -def _dump_status(archs, status, stop_event): - while not stop_event.wait(10): - print('Remote build status at {0}'.format(datetime.datetime.now())) +def _dump_status(archs, status, running): + while any(running.values()): + print(f'Remote build status at {datetime.datetime.now()}') _dump_status_helper(archs, status) - - -def _dump_status_final(archs, status): - print('Results for remote build finished at {0}'.format(datetime.datetime.now())) - _dump_status_helper(archs, status) + time.sleep(10) def _dump_results(targets, archs, status, workspaces): @@ -120,10 +121,10 @@ def _dump_results(targets, archs, status, workspaces): if not exists(build_output_path): build_output = f'No output has been dumped by snapcraft remote-build.' else: - with open(join(workspaces[target], '{0}_{1}.txt'.format(target, arch))) as file_h: + with open(join(workspaces[target], f'{target}_{arch}.txt')) as file_h: build_output = file_h.read() - print('Output for failed build target={0} arch={1}'.format(target, arch)) + print(f'Output for failed build target={target} arch={arch}') print('-------------------------------------------') print(build_output) print('-------------------------------------------') @@ -134,6 +135,10 @@ def _dump_results(targets, archs, status, workspaces): else: print('Some builds failed.') + print() + print(f'Results for remote build finished at {datetime.datetime.now()}') + _dump_status_helper(archs, status) + return failures @@ -143,6 +148,8 @@ def main(): help='the list of snaps to build') parser.add_argument('--archs', nargs='+', choices=['amd64', 'arm64', 'armhf'], default=['amd64'], help='the architectures for which snaps are built') + parser.add_argument('--timeout', type=int, default=None, + help='build process will fail after the provided timeout (in seconds)') args = parser.parse_args() archs = set(args.archs) @@ -158,7 +165,7 @@ def main(): # If we're building anything other than just Certbot, we need to # generate the snapcraft files for the DNS plugins. - if targets != set(('certbot',)): + if targets != {'certbot'}: subprocess.run(['tools/snap/generate_dnsplugins_all.sh'], check=True, cwd=CERTBOT_DIR) @@ -169,25 +176,29 @@ def main(): with Manager() as manager, Pool(processes=len(targets)) as pool: status = manager.dict() + running = manager.dict({target: True for target in targets}) lock = manager.Lock() - stop_event = Event() - state_process = Process(target=_dump_status, args=(archs, status, stop_event)) - state_process.start() + async_results = [pool.apply_async(_build_snap, (target, archs, status, running, lock)) + for target in targets] - async_results = [pool.apply_async(_build_snap, (target, archs, status, lock)) for target in targets] + process = Process(target=_dump_status, args=(archs, status, running)) + process.start() - workspaces = {} - for async_result in async_results: - workspaces.update(async_result.get()) + try: + process.join(args.timeout) - stop_event.set() - state_process.join() + if process.is_alive(): + raise ValueError(f"Timeout out reached ({args.timeout} seconds) during the build!") - failures = _dump_results(targets, archs, status, workspaces) - _dump_status_final(archs, status) + workspaces = {} + for async_result in async_results: + workspaces.update(async_result.get()) - return 1 if failures else 0 + if _dump_results(targets, archs, status, workspaces): + raise ValueError("There were failures during the build!") + finally: + process.terminate() if __name__ == '__main__': diff --git a/tools/snap/generate_dnsplugins_all.sh b/tools/snap/generate_dnsplugins_all.sh index 6c41a19cd..40404bf9b 100755 --- a/tools/snap/generate_dnsplugins_all.sh +++ b/tools/snap/generate_dnsplugins_all.sh @@ -11,5 +11,6 @@ for PLUGIN_PATH in "${CERTBOT_DIR}"/certbot-dns-*; do # Create constraints file "${CERTBOT_DIR}"/tools/merge_requirements.py tools/dev_constraints.txt \ <("${CERTBOT_DIR}"/tools/strip_hashes.py letsencrypt-auto-source/pieces/dependency-requirements.txt) \ + <("${CERTBOT_DIR}"/tools/strip_hashes.py tools/pipstrap_constraints.txt) \ > "${PLUGIN_PATH}"/snap-constraints.txt done diff --git a/tools/snap/generate_dnsplugins_snapcraft.sh b/tools/snap/generate_dnsplugins_snapcraft.sh index 06807ec48..d93d8ec73 100755 --- a/tools/snap/generate_dnsplugins_snapcraft.sh +++ b/tools/snap/generate_dnsplugins_snapcraft.sh @@ -23,11 +23,16 @@ parts: ${PLUGIN}: plugin: python source: . - constraints: [\$SNAPCRAFT_PART_SRC/snap-constraints.txt] override-pull: | snapcraftctl pull snapcraftctl set-version \`grep ^version \$SNAPCRAFT_PART_SRC/setup.py | cut -f2 -d= | tr -d "'[:space:]"\` build-environment: + # Constraints are passed through the environment variable PIP_CONSTRAINTS instead of using the + # parts.[part_name].constraints option available in snapcraft.yaml when the Python plugin is + # used. This is done to let these constraints be applied not only on the certbot package + # build, but also on any isolated build that pip could trigger when building wheels for + # dependencies. See https://github.com/certbot/certbot/pull/8443 for more info. + - PIP_CONSTRAINT: \$SNAPCRAFT_PART_SRC/snap-constraints.txt - SNAP_BUILD: "True" # To build cryptography and cffi if needed build-packages: [gcc, libffi-dev, libssl-dev, python3-dev] diff --git a/tox.ini b/tox.ini index 142e62a92..212d4ee76 100644 --- a/tox.ini +++ b/tox.ini @@ -40,6 +40,7 @@ install_packages = source_paths = acme/acme certbot/certbot + certbot-ci/certbot_integration_tests certbot-apache/certbot_apache certbot-compatibility-test/certbot_compatibility_test certbot-dns-cloudflare/certbot_dns_cloudflare diff --git a/windows-installer/construct.py b/windows-installer/construct.py index 14f770959..1ce4811ac 100644 --- a/windows-installer/construct.py +++ b/windows-installer/construct.py @@ -46,9 +46,11 @@ def _compile_wheels(repo_path, build_path, venv_python): wheels_project = [os.path.join(repo_path, package) for package in certbot_packages] with _prepare_constraints(repo_path) as constraints_file_path: - command = [venv_python, '-m', 'pip', 'wheel', '-w', wheels_path, '--constraint', constraints_file_path] + env = os.environ.copy() + env['PIP_CONSTRAINT'] = constraints_file_path + command = [venv_python, '-m', 'pip', 'wheel', '-w', wheels_path] command.extend(wheels_project) - subprocess.check_call(command) + subprocess.check_call(command, env=env) def _prepare_build_tools(venv_path, venv_python, repo_path): @@ -61,15 +63,20 @@ def _prepare_build_tools(venv_path, venv_python, repo_path): @contextlib.contextmanager def _prepare_constraints(repo_path): - requirements = os.path.join(repo_path, 'letsencrypt-auto-source', 'pieces', 'dependency-requirements.txt') - constraints = subprocess.check_output( - [sys.executable, os.path.join(repo_path, 'tools', 'strip_hashes.py'), requirements], + reqs_certbot = os.path.join(repo_path, 'letsencrypt-auto-source', 'pieces', 'dependency-requirements.txt') + reqs_pipstrap = os.path.join(repo_path, 'tools', 'pipstrap_constraints.txt') + constraints_certbot = subprocess.check_output( + [sys.executable, os.path.join(repo_path, 'tools', 'strip_hashes.py'), reqs_certbot], + universal_newlines=True) + constraints_pipstrap = subprocess.check_output( + [sys.executable, os.path.join(repo_path, 'tools', 'strip_hashes.py'), reqs_pipstrap], universal_newlines=True) workdir = tempfile.mkdtemp() try: constraints_file_path = os.path.join(workdir, 'constraints.txt') with open(constraints_file_path, 'a') as file_h: - file_h.write(constraints) + file_h.write(constraints_pipstrap) + file_h.write(constraints_certbot) file_h.write('pywin32=={0}'.format(PYWIN32_VERSION)) yield constraints_file_path finally: