diff options
author | Brett Holman <brett.holman@canonical.com> | 2023-03-14 08:23:15 -0600 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-03-14 08:23:15 -0600 |
commit | fc6c1d3780cd1480b7819df8fbc08eb9247e4eec (patch) | |
tree | 035018e4c83da77c686cd2df9009beac8cbd2244 | |
parent | 8a0feb1ec04b211f9444e0eeaf473b670db475bd (diff) | |
download | cloud-init-git-fc6c1d3780cd1480b7819df8fbc08eb9247e4eec.tar.gz |
do not attempt dns resolution on ip addresses (#2040)
-rw-r--r-- | cloudinit/util.py | 20 | ||||
-rw-r--r-- | tests/unittests/config/test_apt_configure_sources_list_v1.py | 10 | ||||
-rw-r--r-- | tests/unittests/config/test_apt_source_v3.py | 5 | ||||
-rw-r--r-- | tests/unittests/test_util.py | 15 |
4 files changed, 35 insertions, 15 deletions
diff --git a/cloudinit/util.py b/cloudinit/util.py index aca82878..fc777b82 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -34,6 +34,7 @@ import sys import time from base64 import b64decode, b64encode from collections import deque, namedtuple +from contextlib import suppress from errno import EACCES, ENOENT from functools import lru_cache, total_ordering from pathlib import Path @@ -44,6 +45,7 @@ from cloudinit import features, importer from cloudinit import log as logging from cloudinit import ( mergers, + net, safeyaml, subp, temp_utils, @@ -1233,8 +1235,8 @@ def get_fqdn_from_hosts(hostname, filename="/etc/hosts"): return fqdn -def is_resolvable(name): - """determine if a url is resolvable, return a boolean +def is_resolvable(url) -> bool: + """determine if a url's network address is resolvable, return a boolean This also attempts to be resilent against dns redirection. Note, that normal nsswitch resolution is used here. So in order @@ -1246,6 +1248,8 @@ def is_resolvable(name): be resolved inside the search list. """ global _DNS_REDIRECT_IP + parsed_url = parse.urlparse(url) + name = parsed_url.hostname if _DNS_REDIRECT_IP is None: badips = set() badnames = ( @@ -1253,7 +1257,7 @@ def is_resolvable(name): "example.invalid.", "__cloud_init_expected_not_found__", ) - badresults = {} + badresults: dict = {} for iname in badnames: try: result = socket.getaddrinfo( @@ -1270,12 +1274,14 @@ def is_resolvable(name): LOG.debug("detected dns redirection: %s", badresults) try: + # ip addresses need no resolution + with suppress(ValueError): + if net.is_ip_address(parsed_url.netloc.strip("[]")): + return True result = socket.getaddrinfo(name, None) # check first result's sockaddr field addr = result[0][4][0] - if addr in _DNS_REDIRECT_IP: - return False - return True + return addr not in _DNS_REDIRECT_IP except (socket.gaierror, socket.error): return False @@ -1298,7 +1304,7 @@ def is_resolvable_url(url): logfunc=LOG.debug, msg="Resolving URL: " + url, func=is_resolvable, - args=(parse.urlparse(url).hostname,), + args=(url,), ) diff --git a/tests/unittests/config/test_apt_configure_sources_list_v1.py b/tests/unittests/config/test_apt_configure_sources_list_v1.py index 52964e10..b0bf54f4 100644 --- a/tests/unittests/config/test_apt_configure_sources_list_v1.py +++ b/tests/unittests/config/test_apt_configure_sources_list_v1.py @@ -135,7 +135,7 @@ class TestAptSourceConfigSourceList(t_help.FilesystemMockingTestCase): @staticmethod def myresolve(name): """Fake util.is_resolvable for mirrorfail tests""" - if name == "does.not.exist": + if "does.not.exist" in name: print("Faking FAIL for '%s'" % name) return False else: @@ -155,8 +155,8 @@ class TestAptSourceConfigSourceList(t_help.FilesystemMockingTestCase): ], "http://httpredir.debian.org/debian", ) - mockresolve.assert_any_call("does.not.exist") - mockresolve.assert_any_call("httpredir.debian.org") + mockresolve.assert_any_call("http://does.not.exist") + mockresolve.assert_any_call("http://httpredir.debian.org/debian") def test_apt_v1_srcl_ubuntu_mirrorfail(self): """Test rendering of a source.list from template for ubuntu""" @@ -168,8 +168,8 @@ class TestAptSourceConfigSourceList(t_help.FilesystemMockingTestCase): ["http://does.not.exist", "http://archive.ubuntu.com/ubuntu/"], "http://archive.ubuntu.com/ubuntu/", ) - mockresolve.assert_any_call("does.not.exist") - mockresolve.assert_any_call("archive.ubuntu.com") + mockresolve.assert_any_call("http://does.not.exist") + mockresolve.assert_any_call("http://archive.ubuntu.com/ubuntu/") def test_apt_v1_srcl_custom(self): """Test rendering from a custom source.list template""" diff --git a/tests/unittests/config/test_apt_source_v3.py b/tests/unittests/config/test_apt_source_v3.py index 8d7ba5dc..1813000e 100644 --- a/tests/unittests/config/test_apt_source_v3.py +++ b/tests/unittests/config/test_apt_source_v3.py @@ -963,11 +963,11 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase): with mock.patch.object(util, "is_resolvable") as mockresolve: util.is_resolvable_url("http://1.2.3.4/ubuntu") - mockresolve.assert_called_with("1.2.3.4") + mockresolve.assert_called_with("http://1.2.3.4/ubuntu") with mock.patch.object(util, "is_resolvable") as mockresolve: util.is_resolvable_url("http://us.archive.ubuntu.com/ubuntu") - mockresolve.assert_called_with("us.archive.ubuntu.com") + mockresolve.assert_called_with("http://us.archive.ubuntu.com/ubuntu") # former tests can leave this set (or not if the test is ran directly) # do a hard reset to ensure a stable result @@ -984,7 +984,6 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase): ) mocksock.assert_any_call("example.invalid.", None, 0, 0, 1, 2) mocksock.assert_any_call("us.archive.ubuntu.com", None) - mocksock.assert_any_call("1.2.3.4", None) self.assertTrue(ret) self.assertTrue(ret2) diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py index ecf8cf5c..865f202a 100644 --- a/tests/unittests/test_util.py +++ b/tests/unittests/test_util.py @@ -3026,3 +3026,18 @@ class TestVersion: ) def test_from_str(self, str_ver, cls_ver): assert util.Version.from_str(str_ver) == cls_ver + + +@pytest.mark.allow_dns_lookup +class TestResolvable: + @mock.patch.object(util, "_DNS_REDIRECT_IP", return_value=True) + @mock.patch.object(util.socket, "getaddrinfo") + def test_ips_need_not_be_resolved(self, m_getaddr, m_dns): + """Optimization test: dns resolution may timeout during early boot, and + often the urls being checked use IP addresses rather than dns names. + Therefore, the fast path checks if the address contains an IP and exits + early if the path is a valid IP. + """ + assert util.is_resolvable("http://169.254.169.254/") is True + assert util.is_resolvable("http://[fd00:ec2::254]/") is True + assert not m_getaddr.called |