Merge branch 'master' into update-test-everything

This commit is contained in:
Brad Warren 2018-03-07 15:57:07 -08:00
commit 937e27db28
15 changed files with 252 additions and 70 deletions

View file

@ -227,8 +227,7 @@ class ClientBase(object): # pylint: disable=too-many-instance-attributes
response = self._post(url,
messages.Revocation(
certificate=cert,
reason=rsn),
content_type=None)
reason=rsn))
if response.status_code != http_client.OK:
raise errors.ClientError(
'Successful revocation must return HTTP OK status')

View file

@ -635,8 +635,7 @@ class ClientTest(ClientTestBase):
def test_revoke(self):
self.client.revoke(self.certr.body, self.rsn)
self.net.post.assert_called_once_with(
self.directory[messages.Revocation], mock.ANY, content_type=None,
acme_version=1)
self.directory[messages.Revocation], mock.ANY, acme_version=1)
def test_revocation_payload(self):
obj = messages.Revocation(certificate=self.certr.body, reason=self.rsn)
@ -776,8 +775,7 @@ class ClientV2Test(ClientTestBase):
def test_revoke(self):
self.client.revoke(messages_test.CERT, self.rsn)
self.net.post.assert_called_once_with(
self.directory["revokeCert"], mock.ANY, content_type=None,
acme_version=2)
self.directory["revokeCert"], mock.ANY, acme_version=2)
class MockJSONDeSerializable(jose.JSONDeSerializable):

View file

@ -29,6 +29,8 @@ for an account with the following permissions:
* ``dns.managedZones.list``
* ``dns.resourceRecordSets.create``
* ``dns.resourceRecordSets.delete``
* ``dns.resourceRecordSets.list``
* ``dns.resourceRecordSets.update``
Google provides instructions for `creating a service account <https://developers
.google.com/identity/protocols/OAuth2ServiceAccount#creatinganaccount>`_ and

View file

@ -107,6 +107,17 @@ class _GoogleClient(object):
zone_id = self._find_managed_zone_id(domain)
record_contents = self.get_existing_txt_rrset(zone_id, record_name)
if record_contents is None:
record_contents = []
add_records = record_contents[:]
if "\""+record_content+"\"" in record_contents:
# The process was interrupted previously and validation token exists
return
add_records.append(record_content)
data = {
"kind": "dns#change",
"additions": [
@ -114,12 +125,24 @@ class _GoogleClient(object):
"kind": "dns#resourceRecordSet",
"type": "TXT",
"name": record_name + ".",
"rrdatas": [record_content, ],
"rrdatas": add_records,
"ttl": record_ttl,
},
],
}
if record_contents:
# 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,
},
]
changes = self.dns.changes() # changes | pylint: disable=no-member
try:
@ -154,6 +177,10 @@ class _GoogleClient(object):
logger.warn('Error finding zone. Skipping cleanup.')
return
record_contents = self.get_existing_txt_rrset(zone_id, record_name)
if record_contents is None:
record_contents = ["\"" + record_content + "\""]
data = {
"kind": "dns#change",
"deletions": [
@ -161,12 +188,26 @@ class _GoogleClient(object):
"kind": "dns#resourceRecordSet",
"type": "TXT",
"name": record_name + ".",
"rrdatas": [record_content, ],
"rrdatas": record_contents,
"ttl": record_ttl,
},
],
}
# Remove the record being deleted from the list
readd_contents = [r for r in record_contents if r != "\"" + record_content + "\""]
if readd_contents:
# We need to remove old records in the same request
data["additions"] = [
{
"kind": "dns#resourceRecordSet",
"type": "TXT",
"name": record_name + ".",
"rrdatas": readd_contents,
"ttl": record_ttl,
},
]
changes = self.dns.changes() # changes | pylint: disable=no-member
try:
@ -175,6 +216,37 @@ class _GoogleClient(object):
except googleapiclient_errors.Error as e:
logger.warn('Encountered error deleting TXT record: %s', e)
def get_existing_txt_rrset(self, zone_id, record_name):
"""
Get existing TXT records from the RRset for the record name.
If an error occurs while requesting the record set, it is suppressed
and None is returned.
: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`
"""
rrs_request = self.dns.resourceRecordSets() # pylint: disable=no-member
request = rrs_request.list(managedZone=zone_id, project=self.project_id)
# Add dot as the API returns absolute domains
record_name += "."
try:
response = request.execute()
except googleapiclient_errors.Error:
logger.info("Unable to list existing records. If you're "
"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"]
return None
def _find_managed_zone_id(self, domain):
"""
Find the managed zone for a given domain.

View file

@ -74,10 +74,15 @@ class GoogleClientTest(unittest.TestCase):
mock_mz = mock.MagicMock()
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
mock_changes = mock.MagicMock()
client.dns.managedZones = mock.MagicMock(return_value=mock_mz)
client.dns.changes = mock.MagicMock(return_value=mock_changes)
client.dns.resourceRecordSets = mock.MagicMock(return_value=mock_rrs)
return client, mock_changes
@ -137,6 +142,30 @@ class GoogleClientTest(unittest.TestCase):
managedZone=self.zone,
project=PROJECT_ID)
@mock.patch('oauth2client.service_account.ServiceAccountCredentials.from_json_keyfile_name')
@mock.patch('certbot_dns_google.dns_google.open',
mock.mock_open(read_data='{"project_id": "' + PROJECT_ID + '"}'), create=True)
def test_add_txt_record_delete_old(self, unused_credential_mock):
client, changes = self._setUp_client_with_mock(
[{'managedZones': [{'id': self.zone}]}])
mock_get_rrs = "certbot_dns_google.dns_google._GoogleClient.get_existing_txt_rrset"
with mock.patch(mock_get_rrs) as mock_rrs:
mock_rrs.return_value = ["sample-txt-contents"]
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"])
@mock.patch('oauth2client.service_account.ServiceAccountCredentials.from_json_keyfile_name')
@mock.patch('certbot_dns_google.dns_google.open',
mock.mock_open(read_data='{"project_id": "' + PROJECT_ID + '"}'), create=True)
def test_add_txt_record_noop(self, unused_credential_mock):
client, changes = self._setUp_client_with_mock(
[{'managedZones': [{'id': self.zone}]}])
client.add_txt_record(DOMAIN, "_acme-challenge.example.org",
"example-txt-contents", self.record_ttl)
self.assertFalse(changes.create.called)
@mock.patch('oauth2client.service_account.ServiceAccountCredentials.from_json_keyfile_name')
@mock.patch('certbot_dns_google.dns_google.open',
mock.mock_open(read_data='{"project_id": "' + PROJECT_ID + '"}'), create=True)
@ -172,7 +201,12 @@ class GoogleClientTest(unittest.TestCase):
def test_del_txt_record(self, unused_credential_mock):
client, changes = self._setUp_client_with_mock([{'managedZones': [{'id': self.zone}]}])
client.del_txt_record(DOMAIN, self.record_name, self.record_content, self.record_ttl)
mock_get_rrs = "certbot_dns_google.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\""]
client.del_txt_record(DOMAIN, "_acme-challenge.example.org",
"example-txt-contents", self.record_ttl)
expected_body = {
"kind": "dns#change",
@ -180,8 +214,17 @@ class GoogleClientTest(unittest.TestCase):
{
"kind": "dns#resourceRecordSet",
"type": "TXT",
"name": self.record_name + ".",
"rrdatas": [self.record_content, ],
"name": "_acme-challenge.example.org.",
"rrdatas": ["\"sample-txt-contents\"", "\"example-txt-contents\""],
"ttl": self.record_ttl,
},
],
"additions": [
{
"kind": "dns#resourceRecordSet",
"type": "TXT",
"name": "_acme-challenge.example.org.",
"rrdatas": ["\"sample-txt-contents\"", ],
"ttl": self.record_ttl,
},
],
@ -217,6 +260,30 @@ class GoogleClientTest(unittest.TestCase):
client.del_txt_record(DOMAIN, self.record_name, self.record_content, self.record_ttl)
@mock.patch('oauth2client.service_account.ServiceAccountCredentials.from_json_keyfile_name')
@mock.patch('certbot_dns_google.dns_google.open',
mock.mock_open(read_data='{"project_id": "' + PROJECT_ID + '"}'), create=True)
def test_get_existing(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.assertEquals(found, ["\"example-txt-contents\""])
not_found = client.get_existing_txt_rrset(self.zone, "nonexistent.tld")
self.assertEquals(not_found, None)
@mock.patch('oauth2client.service_account.ServiceAccountCredentials.from_json_keyfile_name')
@mock.patch('certbot_dns_google.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
rrset = client.get_existing_txt_rrset(self.zone, "_acme-challenge.example.org")
self.assertFalse(rrset)
def test_get_project_id(self):
from certbot_dns_google.dns_google import _GoogleClient

View file

@ -1,4 +1,5 @@
"""Certbot Route53 authenticator plugin."""
import collections
import logging
import time
@ -33,6 +34,7 @@ class Authenticator(dns_common.DNSAuthenticator):
def __init__(self, *args, **kwargs):
super(Authenticator, self).__init__(*args, **kwargs)
self.r53 = boto3.client("route53")
self._resource_records = collections.defaultdict(list)
def more_info(self): # pylint: disable=missing-docstring,no-self-use
return "Solve a DNS01 challenge using AWS Route53"
@ -88,6 +90,20 @@ class Authenticator(dns_common.DNSAuthenticator):
def _change_txt_record(self, action, validation_domain_name, validation):
zone_id = self._find_zone_id_for_domain(validation_domain_name)
rrecords = self._resource_records[validation_domain_name]
challenge = {"Value": '"{0}"'.format(validation)}
if action == "DELETE":
# Remove the record being deleted from the list of tracked records
rrecords.remove(challenge)
if rrecords:
# Need to update instead, as we're not deleting the rrset
action = "UPSERT"
else:
# Create a new list containing the record to use with DELETE
rrecords = [challenge]
else:
rrecords.append(challenge)
response = self.r53.change_resource_record_sets(
HostedZoneId=zone_id,
ChangeBatch={
@ -99,11 +115,7 @@ class Authenticator(dns_common.DNSAuthenticator):
"Name": validation_domain_name,
"Type": "TXT",
"TTL": self.ttl,
"ResourceRecords": [
# For some reason TXT records need to be
# manually quoted.
{"Value": '"{0}"'.format(validation)}
],
"ResourceRecords": rrecords,
}
}
]

View file

@ -186,6 +186,48 @@ class ClientTest(unittest.TestCase):
call_count = self.client.r53.change_resource_record_sets.call_count
self.assertEqual(call_count, 1)
def test_change_txt_record_delete(self):
self.client._find_zone_id_for_domain = mock.MagicMock()
self.client.r53.change_resource_record_sets = mock.MagicMock(
return_value={"ChangeInfo": {"Id": 1}})
validation = "some-value"
validation_record = {"Value": '"{0}"'.format(validation)}
self.client._resource_records[DOMAIN] = [validation_record]
self.client._change_txt_record("DELETE", DOMAIN, validation)
call_count = self.client.r53.change_resource_record_sets.call_count
self.assertEqual(call_count, 1)
call_args = self.client.r53.change_resource_record_sets.call_args_list[0][1]
call_args_batch = call_args["ChangeBatch"]["Changes"][0]
self.assertEqual(call_args_batch["Action"], "DELETE")
self.assertEqual(
call_args_batch["ResourceRecordSet"]["ResourceRecords"],
[validation_record])
def test_change_txt_record_multirecord(self):
self.client._find_zone_id_for_domain = mock.MagicMock()
self.client._get_validation_rrset = mock.MagicMock()
self.client._resource_records[DOMAIN] = [
{"Value": "\"pre-existing-value\""},
{"Value": "\"pre-existing-value-two\""},
]
self.client.r53.change_resource_record_sets = mock.MagicMock(
return_value={"ChangeInfo": {"Id": 1}})
self.client._change_txt_record("DELETE", DOMAIN, "pre-existing-value")
call_count = self.client.r53.change_resource_record_sets.call_count
call_args = self.client.r53.change_resource_record_sets.call_args_list[0][1]
call_args_batch = call_args["ChangeBatch"]["Changes"][0]
self.assertEqual(call_args_batch["Action"], "UPSERT")
self.assertEqual(
call_args_batch["ResourceRecordSet"]["ResourceRecords"],
[{"Value": "\"pre-existing-value-two\""}])
self.assertEqual(call_count, 1)
def test_wait_for_change(self):
self.client.r53.get_change = mock.MagicMock(
side_effect=[{"ChangeInfo": {"Status": "PENDING"}},

View file

@ -190,6 +190,7 @@ class PluginsRegistry(collections.Mapping):
def find_all(cls):
"""Find plugins using setuptools entry points."""
plugins = {}
# pylint: disable=not-callable
entry_points = itertools.chain(
pkg_resources.iter_entry_points(
constants.SETUPTOOLS_PLUGINS_ENTRY_POINT),

View file

@ -189,7 +189,7 @@ when it receives a TLS ClientHello with the SNI extension set to
os.environ.update(env)
_, out = hooks.execute(self.conf('auth-hook'))
env['CERTBOT_AUTH_OUTPUT'] = out.strip()
self.env[achall.domain] = env
self.env[achall] = env
def _perform_achall_manually(self, achall):
validation = achall.validation(achall.account_key)
@ -215,7 +215,7 @@ when it receives a TLS ClientHello with the SNI extension set to
def cleanup(self, achalls): # pylint: disable=missing-docstring
if self.conf('cleanup-hook'):
for achall in achalls:
env = self.env.pop(achall.domain)
env = self.env.pop(achall)
if 'CERTBOT_TOKEN' not in env:
os.environ.pop('CERTBOT_TOKEN', None)
os.environ.update(env)

View file

@ -93,10 +93,10 @@ class AuthenticatorTest(test_util.TempDirTestCase):
self.auth.perform(self.achalls),
[achall.response(achall.account_key) for achall in self.achalls])
self.assertEqual(
self.auth.env[self.dns_achall.domain]['CERTBOT_AUTH_OUTPUT'],
self.auth.env[self.dns_achall]['CERTBOT_AUTH_OUTPUT'],
dns_expected)
self.assertEqual(
self.auth.env[self.http_achall.domain]['CERTBOT_AUTH_OUTPUT'],
self.auth.env[self.http_achall]['CERTBOT_AUTH_OUTPUT'],
http_expected)
# tls_sni_01 challenge must be perform()ed above before we can
# get the cert_path and key_path.
@ -107,7 +107,7 @@ class AuthenticatorTest(test_util.TempDirTestCase):
self.auth.tls_sni_01.get_z_domain(self.tls_sni_achall),
'novalidation')
self.assertEqual(
self.auth.env[self.tls_sni_achall.domain]['CERTBOT_AUTH_OUTPUT'],
self.auth.env[self.tls_sni_achall]['CERTBOT_AUTH_OUTPUT'],
tls_sni_expected)
@test_util.patch_get_utility()

View file

@ -1216,7 +1216,7 @@ UNLIKELY_EOF
# -------------------------------------------------------------------------
cat << "UNLIKELY_EOF" > "$TEMP_DIR/pipstrap.py"
#!/usr/bin/env python
"""A small script that can act as a trust root for installing pip 8
"""A small script that can act as a trust root for installing pip >=8
Embed this in your project, and your VCS checkout is all you have to trust. In
a post-peep era, this lets you claw your way to a hash-checking version of pip,
@ -1274,7 +1274,7 @@ except ImportError:
from urllib.parse import urlparse # 3.4
__version__ = 1, 5, 0
__version__ = 1, 5, 1
PIP_VERSION = '9.0.1'
DEFAULT_INDEX_BASE = 'https://pypi.python.org'
@ -1287,14 +1287,11 @@ maybe_argparse = (
if version_info < (2, 7, 0) else [])
# Pip has no dependencies, as it vendors everything:
PIP_PACKAGE = [
PACKAGES = maybe_argparse + [
# Pip has no dependencies, as it vendors everything:
('11/b6/abcb525026a4be042b486df43905d6893fb04f05aac21c32c638e939e447/'
'pip-{0}.tar.gz'.format(PIP_VERSION),
'09f243e1a7b461f654c26a725fa373211bb7ff17a9300058b205c61658ca940d')]
OTHER_PACKAGES = maybe_argparse + [
'09f243e1a7b461f654c26a725fa373211bb7ff17a9300058b205c61658ca940d'),
# This version of setuptools has only optional dependencies:
('59/88/2f3990916931a5de6fa9706d6d75eb32ee8b78627bb2abaab7ed9e6d0622/'
'setuptools-29.0.1.tar.gz',
@ -1379,21 +1376,16 @@ def main():
index_base = get_index_base()
temp = mkdtemp(prefix='pipstrap-')
try:
# We download and install pip first, then the rest, to avoid the bug
# https://github.com/certbot/certbot/issues/4938.
pip_downloads, other_downloads = [
[hashed_download(index_base + '/packages/' + path,
temp,
digest)
for path, digest in packages]
for packages in (PIP_PACKAGE, OTHER_PACKAGES)]
for downloads in (pip_downloads, other_downloads):
check_output('pip install --no-index --no-deps -U ' +
# Disable cache since we're not using it and it
# otherwise sometimes throws permission warnings:
('--no-cache-dir ' if has_pip_cache else '') +
' '.join(quote(d) for d in downloads),
shell=True)
downloads = [hashed_download(index_base + '/packages/' + path,
temp,
digest)
for path, digest in PACKAGES]
check_output('pip install --no-index --no-deps -U ' +
# Disable cache since we're not using it and it otherwise
# sometimes throws permission warnings:
('--no-cache-dir ' if has_pip_cache else '') +
' '.join(quote(d) for d in downloads),
shell=True)
except HashError as exc:
print(exc)
except Exception:

View file

@ -1,5 +1,5 @@
#!/usr/bin/env python
"""A small script that can act as a trust root for installing pip 8
"""A small script that can act as a trust root for installing pip >=8
Embed this in your project, and your VCS checkout is all you have to trust. In
a post-peep era, this lets you claw your way to a hash-checking version of pip,
@ -57,7 +57,7 @@ except ImportError:
from urllib.parse import urlparse # 3.4
__version__ = 1, 5, 0
__version__ = 1, 5, 1
PIP_VERSION = '9.0.1'
DEFAULT_INDEX_BASE = 'https://pypi.python.org'
@ -70,14 +70,11 @@ maybe_argparse = (
if version_info < (2, 7, 0) else [])
# Pip has no dependencies, as it vendors everything:
PIP_PACKAGE = [
PACKAGES = maybe_argparse + [
# Pip has no dependencies, as it vendors everything:
('11/b6/abcb525026a4be042b486df43905d6893fb04f05aac21c32c638e939e447/'
'pip-{0}.tar.gz'.format(PIP_VERSION),
'09f243e1a7b461f654c26a725fa373211bb7ff17a9300058b205c61658ca940d')]
OTHER_PACKAGES = maybe_argparse + [
'09f243e1a7b461f654c26a725fa373211bb7ff17a9300058b205c61658ca940d'),
# This version of setuptools has only optional dependencies:
('59/88/2f3990916931a5de6fa9706d6d75eb32ee8b78627bb2abaab7ed9e6d0622/'
'setuptools-29.0.1.tar.gz',
@ -162,21 +159,16 @@ def main():
index_base = get_index_base()
temp = mkdtemp(prefix='pipstrap-')
try:
# We download and install pip first, then the rest, to avoid the bug
# https://github.com/certbot/certbot/issues/4938.
pip_downloads, other_downloads = [
[hashed_download(index_base + '/packages/' + path,
temp,
digest)
for path, digest in packages]
for packages in (PIP_PACKAGE, OTHER_PACKAGES)]
for downloads in (pip_downloads, other_downloads):
check_output('pip install --no-index --no-deps -U ' +
# Disable cache since we're not using it and it
# otherwise sometimes throws permission warnings:
('--no-cache-dir ' if has_pip_cache else '') +
' '.join(quote(d) for d in downloads),
shell=True)
downloads = [hashed_download(index_base + '/packages/' + path,
temp,
digest)
for path, digest in PACKAGES]
check_output('pip install --no-index --no-deps -U ' +
# Disable cache since we're not using it and it otherwise
# sometimes throws permission warnings:
('--no-cache-dir ' if has_pip_cache else '') +
' '.join(quote(d) for d in downloads),
shell=True)
except HashError as exc:
print(exc)
except Exception:

View file

@ -233,6 +233,7 @@ certname="dns.le.wtf"
common -a manual -d dns.le.wtf --preferred-challenges dns,tls-sni run \
--cert-name $certname \
--manual-auth-hook ./tests/manual-dns-auth.sh \
--manual-cleanup-hook ./tests/manual-dns-cleanup.sh \
--pre-hook 'echo wtf2.pre >> "$HOOK_TEST"' \
--post-hook 'echo wtf2.post >> "$HOOK_TEST"' \
--renew-hook 'echo deploy >> "$HOOK_TEST"'
@ -433,7 +434,8 @@ done
# Test ACMEv2-only features
if [ "${BOULDER_INTEGRATION:-v1}" = "v2" ]; then
common -a manual -d '*.le4.wtf,le4.wtf' --preferred-challenges dns \
--manual-auth-hook ./tests/manual-dns-auth.sh
--manual-auth-hook ./tests/manual-dns-auth.sh \
--manual-cleanup-hook ./tests/manual-dns-cleanup.sh
fi
# Most CI systems set this variable to true.
@ -443,4 +445,4 @@ then
. ./certbot-nginx/tests/boulder-integration.sh
fi
coverage report --fail-under 63 -m
coverage report --fail-under 67 -m

View file

@ -23,7 +23,7 @@ fi
certbot_test_no_force_renew () {
omit_patterns="*/*.egg-info/*,*/dns_common*,*/setup.py,*/test_*,*/tests/*"
omit_patterns="$omit_patterns,*_test.py,*_test_*,"
omit_patterns="$omit_patterns,*_test.py,*_test_*,certbot-apache/*"
omit_patterns="$omit_patterns,certbot-compatibility-test/*,certbot-dns*/"
coverage run \
--append \

3
tests/manual-dns-cleanup.sh Executable file
View file

@ -0,0 +1,3 @@
#!/bin/sh
curl -X POST 'http://localhost:8055/clear-txt' -d \
"{\"host\": \"_acme-challenge.$CERTBOT_DOMAIN.\"}"