Skip to content

Commit

Permalink
Merge branch 'warnings-are-errors' into test-everything-warnings
Browse files Browse the repository at this point in the history
  • Loading branch information
ohemorange committed Oct 31, 2018
2 parents 825879e + aad2663 commit 2904ff3
Show file tree
Hide file tree
Showing 133 changed files with 3,635 additions and 139 deletions.
26 changes: 25 additions & 1 deletion acme/acme/client.py
Expand Up @@ -50,7 +50,6 @@ class ClientBase(object): # pylint: disable=too-many-instance-attributes
:ivar .ClientNetwork net: Client network.
:ivar int acme_version: ACME protocol version. 1 or 2.
"""

def __init__(self, directory, net, acme_version):
"""Initialize.
Expand Down Expand Up @@ -588,6 +587,30 @@ def new_account(self, new_account):
self.net.account = regr
return regr

def update_registration(self, regr, update=None):
"""Update registration.
:param messages.RegistrationResource regr: Registration Resource.
:param messages.Registration update: Updated body of the
resource. If not provided, body will be taken from `regr`.
:returns: Updated Registration Resource.
:rtype: `.RegistrationResource`
"""
# https://github.com/certbot/certbot/issues/6155
new_regr = self._get_v2_account(regr)
return super(ClientV2, self).update_registration(new_regr, update)

def _get_v2_account(self, regr):
self.net.account = None
only_existing_reg = regr.body.update(only_return_existing=True)
response = self._post(self.directory['newAccount'], only_existing_reg)
updated_uri = response.headers['Location']
new_regr = regr.update(uri=updated_uri)
self.net.account = new_regr
return new_regr

def new_order(self, csr_pem):
"""Request a new Order object from the server.
Expand Down Expand Up @@ -910,6 +933,7 @@ def _wrap_in_jws(self, obj, nonce, url, acme_version):
if acme_version == 2:
kwargs["url"] = url
# newAccount and revokeCert work without the kid
# newAccount must not have kid
if self.account is not None:
kwargs["kid"] = self.account["uri"]
kwargs["key"] = self.key
Expand Down
24 changes: 22 additions & 2 deletions acme/acme/client_test.py
Expand Up @@ -139,7 +139,7 @@ def test_forwarding(self):
client = self._init()
self.assertEqual(client.directory, client.client.directory)
self.assertEqual(client.key, KEY)
self.assertEqual(client.update_registration, client.client.update_registration)
self.assertEqual(client.deactivate_registration, client.client.deactivate_registration)
self.assertRaises(AttributeError, client.__getattr__, 'nonexistent')
self.assertRaises(AttributeError, client.__getattr__, 'new_account_and_tos')
self.assertRaises(AttributeError, client.__getattr__, 'new_account')
Expand Down Expand Up @@ -270,6 +270,13 @@ def test_revoke(self):
client.revoke(messages_test.CERT, self.rsn)
mock_client().revoke.assert_called_once_with(messages_test.CERT, self.rsn)

def test_update_registration(self):
self.response.json.return_value = DIRECTORY_V1.to_json()
with mock.patch('acme.client.Client') as mock_client:
client = self._init()
client.update_registration(mock.sentinel.regr, None)
mock_client().update_registration.assert_called_once_with(mock.sentinel.regr, None)


class ClientTest(ClientTestBase):
"""Tests for acme.client.Client."""
Expand Down Expand Up @@ -652,7 +659,7 @@ def test_revoke(self):
def test_revocation_payload(self):
obj = messages.Revocation(certificate=self.certr.body, reason=self.rsn)
self.assertTrue('reason' in obj.to_partial_json().keys())
self.assertEquals(self.rsn, obj.to_partial_json()['reason'])
self.assertEqual(self.rsn, obj.to_partial_json()['reason'])

def test_revoke_bad_status_raises_error(self):
self.response.status_code = http_client.METHOD_NOT_ALLOWED
Expand Down Expand Up @@ -789,6 +796,19 @@ def test_revoke(self):
self.net.post.assert_called_once_with(
self.directory["revokeCert"], mock.ANY, acme_version=2)

def test_update_registration(self):
# "Instance of 'Field' has no to_json/update member" bug:
# pylint: disable=no-member
self.response.headers['Location'] = self.regr.uri
self.response.json.return_value = self.regr.body.to_json()
self.assertEqual(self.regr, self.client.update_registration(self.regr))
self.assertNotEqual(self.client.net.account, None)
self.assertEqual(self.client.net.post.call_count, 2)
self.assertTrue(DIRECTORY_V2.newAccount in self.net.post.call_args_list[0][0])

self.response.json.return_value = self.regr.body.update(
contact=()).to_json()


class MockJSONDeSerializable(jose.JSONDeSerializable):
# pylint: disable=missing-docstring
Expand Down
6 changes: 3 additions & 3 deletions acme/acme/crypto_util_test.py
Expand Up @@ -209,8 +209,8 @@ def test_make_csr(self):
# have a get_extensions() method, so we skip this test if the method
# isn't available.
if hasattr(csr, 'get_extensions'):
self.assertEquals(len(csr.get_extensions()), 1)
self.assertEquals(csr.get_extensions()[0].get_data(),
self.assertEqual(len(csr.get_extensions()), 1)
self.assertEqual(csr.get_extensions()[0].get_data(),
OpenSSL.crypto.X509Extension(
b'subjectAltName',
critical=False,
Expand All @@ -227,7 +227,7 @@ def test_make_csr_must_staple(self):
# have a get_extensions() method, so we skip this test if the method
# isn't available.
if hasattr(csr, 'get_extensions'):
self.assertEquals(len(csr.get_extensions()), 2)
self.assertEqual(len(csr.get_extensions()), 2)
# NOTE: Ideally we would filter by the TLS Feature OID, but
# OpenSSL.crypto.X509Extension doesn't give us the extension's raw OID,
# and the shortname field is just "UNDEF"
Expand Down
1 change: 1 addition & 0 deletions acme/acme/messages.py
Expand Up @@ -274,6 +274,7 @@ class Registration(ResourceBody):
agreement = jose.Field('agreement', omitempty=True)
status = jose.Field('status', omitempty=True)
terms_of_service_agreed = jose.Field('termsOfServiceAgreed', omitempty=True)
only_return_existing = jose.Field('onlyReturnExisting', omitempty=True)

phone_prefix = 'tel:'
email_prefix = 'mailto:'
Expand Down
3 changes: 2 additions & 1 deletion acme/setup.py
Expand Up @@ -58,7 +58,7 @@ def run_tests(self):
license='Apache License 2.0',
python_requires='>=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*',
classifiers=[
'Development Status :: 3 - Alpha',
'Development Status :: 5 - Production/Stable',
'Intended Audience :: Developers',
'License :: OSI Approved :: Apache Software License',
'Programming Language :: Python',
Expand All @@ -68,6 +68,7 @@ def run_tests(self):
'Programming Language :: Python :: 3.4',
'Programming Language :: Python :: 3.5',
'Programming Language :: Python :: 3.6',
'Programming Language :: Python :: 3.7',
'Topic :: Internet :: WWW/HTTP',
'Topic :: Security',
],
Expand Down
5 changes: 5 additions & 0 deletions certbot-apache/certbot_apache/configurator.py
Expand Up @@ -165,6 +165,7 @@ def __init__(self, *args, **kwargs):
self._autohsts = {} # type: Dict[str, Dict[str, Union[int, float]]]

# These will be set in the prepare function
self._prepared = False
self.parser = None
self.version = version
self.vhosts = None
Expand Down Expand Up @@ -249,6 +250,7 @@ def prepare(self):
logger.debug("Encountered error:", exc_info=True)
raise errors.PluginError(
"Unable to lock %s", self.conf("server-root"))
self._prepared = True

def _check_aug_version(self):
""" Checks that we have recent enough version of libaugeas.
Expand Down Expand Up @@ -2394,6 +2396,9 @@ def update_autohsts(self, _unused_domain):
continue
nextstep = config["laststep"] + 1
if nextstep < len(constants.AUTOHSTS_STEPS):
# If installer hasn't been prepared yet, do it now
if not self._prepared:
self.prepare()
# Have not reached the max value yet
try:
vhost = self.find_vhost_by_id(id_str)
Expand Down
17 changes: 10 additions & 7 deletions certbot-apache/certbot_apache/tests/autohsts_test.py
Expand Up @@ -55,20 +55,23 @@ def test_autohsts_deploy_already_exists(self, _restart):

@mock.patch("certbot_apache.constants.AUTOHSTS_FREQ", 0)
@mock.patch("certbot_apache.configurator.ApacheConfigurator.restart")
def test_autohsts_increase(self, _mock_restart):
@mock.patch("certbot_apache.configurator.ApacheConfigurator.prepare")
def test_autohsts_increase(self, mock_prepare, _mock_restart):
self.config._prepared = False
maxage = "\"max-age={0}\""
initial_val = maxage.format(constants.AUTOHSTS_STEPS[0])
inc_val = maxage.format(constants.AUTOHSTS_STEPS[1])

self.config.enable_autohsts(mock.MagicMock(), ["ocspvhost.com"])
# Verify initial value
self.assertEquals(self.get_autohsts_value(self.vh_truth[7].path),
self.assertEqual(self.get_autohsts_value(self.vh_truth[7].path),
initial_val)
# Increase
self.config.update_autohsts(mock.MagicMock())
# Verify increased value
self.assertEquals(self.get_autohsts_value(self.vh_truth[7].path),
self.assertEqual(self.get_autohsts_value(self.vh_truth[7].path),
inc_val)
self.assertTrue(mock_prepare.called)

@mock.patch("certbot_apache.configurator.ApacheConfigurator.restart")
@mock.patch("certbot_apache.configurator.ApacheConfigurator._autohsts_increase")
Expand All @@ -77,7 +80,7 @@ def test_autohsts_increase_noop(self, mock_increase, _restart):
initial_val = maxage.format(constants.AUTOHSTS_STEPS[0])
self.config.enable_autohsts(mock.MagicMock(), ["ocspvhost.com"])
# Verify initial value
self.assertEquals(self.get_autohsts_value(self.vh_truth[7].path),
self.assertEqual(self.get_autohsts_value(self.vh_truth[7].path),
initial_val)

self.config.update_autohsts(mock.MagicMock())
Expand Down Expand Up @@ -114,11 +117,11 @@ def test_autohsts_increase_and_make_permanent(self, _mock_restart):
self.config.update_autohsts(mock.MagicMock())
# Value should match pre-permanent increment step
cur_val = maxage.format(constants.AUTOHSTS_STEPS[i+1])
self.assertEquals(self.get_autohsts_value(self.vh_truth[7].path),
self.assertEqual(self.get_autohsts_value(self.vh_truth[7].path),
cur_val)
# Make permanent
self.config.deploy_autohsts(mock_lineage)
self.assertEquals(self.get_autohsts_value(self.vh_truth[7].path),
self.assertEqual(self.get_autohsts_value(self.vh_truth[7].path),
max_val)

def test_autohsts_update_noop(self):
Expand Down Expand Up @@ -150,7 +153,7 @@ def test_autohsts_dont_enhance_twice(self, mock_id, _restart):
mock_id.return_value = "1234567"
self.config.enable_autohsts(mock.MagicMock(),
["ocspvhost.com", "ocspvhost.com"])
self.assertEquals(mock_id.call_count, 1)
self.assertEqual(mock_id.call_count, 1)

def test_autohsts_remove_orphaned(self):
# pylint: disable=protected-access
Expand Down
8 changes: 4 additions & 4 deletions certbot-apache/certbot_apache/tests/centos_test.py
Expand Up @@ -81,9 +81,9 @@ def mock_get_cfg(command):
mock_osi.return_value = ("centos", "7")
self.config.parser.update_runtime_variables()

self.assertEquals(mock_get.call_count, 3)
self.assertEquals(len(self.config.parser.modules), 4)
self.assertEquals(len(self.config.parser.variables), 2)
self.assertEqual(mock_get.call_count, 3)
self.assertEqual(len(self.config.parser.modules), 4)
self.assertEqual(len(self.config.parser.variables), 2)
self.assertTrue("TEST2" in self.config.parser.variables.keys())
self.assertTrue("mod_another.c" in self.config.parser.modules)

Expand Down Expand Up @@ -127,7 +127,7 @@ def test_get_sysconfig_vars(self, mock_cfg):
def test_alt_restart_works(self, mock_run_script):
mock_run_script.side_effect = [None, errors.SubprocessError, None]
self.config.restart()
self.assertEquals(mock_run_script.call_count, 3)
self.assertEqual(mock_run_script.call_count, 3)

@mock.patch("certbot_apache.configurator.util.run_script")
def test_alt_restart_errors(self, mock_run_script):
Expand Down
18 changes: 9 additions & 9 deletions certbot-apache/certbot_apache/tests/configurator_test.py
Expand Up @@ -1401,11 +1401,11 @@ def test_choose_vhosts_wildcard(self):
vhs = self.config._choose_vhosts_wildcard("*.certbot.demo",
create_ssl=True)
# Check that the dialog was called with one vh: certbot.demo
self.assertEquals(mock_select_vhs.call_args[0][0][0], self.vh_truth[3])
self.assertEquals(len(mock_select_vhs.call_args_list), 1)
self.assertEqual(mock_select_vhs.call_args[0][0][0], self.vh_truth[3])
self.assertEqual(len(mock_select_vhs.call_args_list), 1)

# And the actual returned values
self.assertEquals(len(vhs), 1)
self.assertEqual(len(vhs), 1)
self.assertTrue(vhs[0].name == "certbot.demo")
self.assertTrue(vhs[0].ssl)

Expand All @@ -1420,7 +1420,7 @@ def test_choose_vhosts_wildcard_no_ssl(self, mock_makessl):
vhs = self.config._choose_vhosts_wildcard("*.certbot.demo",
create_ssl=False)
self.assertFalse(mock_makessl.called)
self.assertEquals(vhs[0], self.vh_truth[1])
self.assertEqual(vhs[0], self.vh_truth[1])

@mock.patch("certbot_apache.configurator.ApacheConfigurator._vhosts_for_wildcard")
@mock.patch("certbot_apache.configurator.ApacheConfigurator.make_vhost_ssl")
Expand All @@ -1433,15 +1433,15 @@ def test_choose_vhosts_wildcard_already_ssl(self, mock_makessl, mock_vh_for_w):
mock_select_vhs.return_value = [self.vh_truth[7]]
vhs = self.config._choose_vhosts_wildcard("whatever",
create_ssl=True)
self.assertEquals(mock_select_vhs.call_args[0][0][0], self.vh_truth[7])
self.assertEquals(len(mock_select_vhs.call_args_list), 1)
self.assertEqual(mock_select_vhs.call_args[0][0][0], self.vh_truth[7])
self.assertEqual(len(mock_select_vhs.call_args_list), 1)
# Ensure that make_vhost_ssl was not called, vhost.ssl == true
self.assertFalse(mock_makessl.called)

# And the actual returned values
self.assertEquals(len(vhs), 1)
self.assertEqual(len(vhs), 1)
self.assertTrue(vhs[0].ssl)
self.assertEquals(vhs[0], self.vh_truth[7])
self.assertEqual(vhs[0], self.vh_truth[7])


def test_deploy_cert_wildcard(self):
Expand All @@ -1454,7 +1454,7 @@ def test_deploy_cert_wildcard(self):
self.config.deploy_cert("*.wildcard.example.org", "/tmp/path",
"/tmp/path", "/tmp/path", "/tmp/path")
self.assertTrue(mock_dep.called)
self.assertEquals(len(mock_dep.call_args_list), 1)
self.assertEqual(len(mock_dep.call_args_list), 1)
self.assertEqual(self.vh_truth[7], mock_dep.call_args_list[0][0][0])

@mock.patch("certbot_apache.display_ops.select_vhost_multiple")
Expand Down
6 changes: 3 additions & 3 deletions certbot-apache/certbot_apache/tests/gentoo_test.py
Expand Up @@ -121,15 +121,15 @@ def mock_get_cfg(command):
mock_osi.return_value = ("gentoo", "123")
self.config.parser.update_runtime_variables()

self.assertEquals(mock_get.call_count, 1)
self.assertEquals(len(self.config.parser.modules), 4)
self.assertEqual(mock_get.call_count, 1)
self.assertEqual(len(self.config.parser.modules), 4)
self.assertTrue("mod_another.c" in self.config.parser.modules)

@mock.patch("certbot_apache.configurator.util.run_script")
def test_alt_restart_works(self, mock_run_script):
mock_run_script.side_effect = [None, errors.SubprocessError, None]
self.config.restart()
self.assertEquals(mock_run_script.call_count, 3)
self.assertEqual(mock_run_script.call_count, 3)

if __name__ == "__main__":
unittest.main() # pragma: no cover
4 changes: 2 additions & 2 deletions certbot-apache/certbot_apache/tests/parser_test.py
Expand Up @@ -84,7 +84,7 @@ def test_add_dir_beginning(self):
self.assertEqual(self.parser.aug.get(match), str(i + 1))

def test_empty_arg(self):
self.assertEquals(None,
self.assertEqual(None,
self.parser.get_arg("/files/whatever/nonexistent"))

def test_add_dir_to_ifmodssl(self):
Expand Down Expand Up @@ -303,7 +303,7 @@ def test_add_comment(self):
from certbot_apache.parser import get_aug_path
self.parser.add_comment(get_aug_path(self.parser.loc["name"]), "123456")
comm = self.parser.find_comments("123456")
self.assertEquals(len(comm), 1)
self.assertEqual(len(comm), 1)
self.assertTrue(self.parser.loc["name"] in comm[0])


Expand Down
3 changes: 2 additions & 1 deletion certbot-apache/setup.py
Expand Up @@ -31,7 +31,7 @@
license='Apache License 2.0',
python_requires='>=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*',
classifiers=[
'Development Status :: 3 - Alpha',
'Development Status :: 5 - Production/Stable',
'Environment :: Plugins',
'Intended Audience :: System Administrators',
'License :: OSI Approved :: Apache Software License',
Expand All @@ -43,6 +43,7 @@
'Programming Language :: Python :: 3.4',
'Programming Language :: Python :: 3.5',
'Programming Language :: Python :: 3.6',
'Programming Language :: Python :: 3.7',
'Topic :: Internet :: WWW/HTTP',
'Topic :: Security',
'Topic :: System :: Installation/Setup',
Expand Down
1 change: 1 addition & 0 deletions certbot-compatibility-test/setup.py
Expand Up @@ -46,6 +46,7 @@
'Programming Language :: Python :: 3.4',
'Programming Language :: Python :: 3.5',
'Programming Language :: Python :: 3.6',
'Programming Language :: Python :: 3.7',
'Topic :: Internet :: WWW/HTTP',
'Topic :: Security',
],
Expand Down
1 change: 1 addition & 0 deletions certbot-dns-cloudflare/setup.py
Expand Up @@ -42,6 +42,7 @@
'Programming Language :: Python :: 3.4',
'Programming Language :: Python :: 3.5',
'Programming Language :: Python :: 3.6',
'Programming Language :: Python :: 3.7',
'Topic :: Internet :: WWW/HTTP',
'Topic :: Security',
'Topic :: System :: Installation/Setup',
Expand Down
1 change: 1 addition & 0 deletions certbot-dns-cloudxns/setup.py
Expand Up @@ -42,6 +42,7 @@
'Programming Language :: Python :: 3.4',
'Programming Language :: Python :: 3.5',
'Programming Language :: Python :: 3.6',
'Programming Language :: Python :: 3.7',
'Topic :: Internet :: WWW/HTTP',
'Topic :: Security',
'Topic :: System :: Installation/Setup',
Expand Down
1 change: 1 addition & 0 deletions certbot-dns-digitalocean/setup.py
Expand Up @@ -43,6 +43,7 @@
'Programming Language :: Python :: 3.4',
'Programming Language :: Python :: 3.5',
'Programming Language :: Python :: 3.6',
'Programming Language :: Python :: 3.7',
'Topic :: Internet :: WWW/HTTP',
'Topic :: Security',
'Topic :: System :: Installation/Setup',
Expand Down
1 change: 1 addition & 0 deletions certbot-dns-dnsimple/setup.py
Expand Up @@ -42,6 +42,7 @@
'Programming Language :: Python :: 3.4',
'Programming Language :: Python :: 3.5',
'Programming Language :: Python :: 3.6',
'Programming Language :: Python :: 3.7',
'Topic :: Internet :: WWW/HTTP',
'Topic :: Security',
'Topic :: System :: Installation/Setup',
Expand Down

0 comments on commit 2904ff3

Please sign in to comment.