From d01088f9fe35223cf0547a3cbfd7712f94711beb Mon Sep 17 00:00:00 2001 From: Federico Bosi Date: Thu, 29 Feb 2024 21:46:41 +0100 Subject: [PATCH] Fix dynamic ipv6 address check & DNS tests get_ipv6_addr: the if condition was checking for the eui 64 mac address, but that has nothing to do with global ipv6 addresses. I added and fixed existing tests accordingly. The DNS resolver tests were not expecting a NoAnswer that is returned if the query is done against a non-authoritative recursor. Finally fixed a typo in a test name: loopup -> lookup. --- charmhelpers/contrib/network/ip.py | 8 +++---- tests/contrib/network/test_ip.py | 35 ++++++++++++++++++++++++++---- 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/charmhelpers/contrib/network/ip.py b/charmhelpers/contrib/network/ip.py index cf9926b95..b46d9900c 100644 --- a/charmhelpers/contrib/network/ip.py +++ b/charmhelpers/contrib/network/ip.py @@ -381,7 +381,6 @@ def get_ipv6_addr(iface=None, inc_aliases=False, fatal=True, exc_list=None, key_scope_link_local = re.compile("^fe80::..(.+)%(.+)") m = re.match(key_scope_link_local, addr) if m: - eui_64_mac = m.group(1) iface = m.group(2) else: global_addrs.append(addr) @@ -404,8 +403,7 @@ def get_ipv6_addr(iface=None, inc_aliases=False, fatal=True, exc_list=None, # Return the first valid address we find for addr in global_addrs: if m.group(1) == addr: - if not dynamic_only or \ - m.group(1).endswith(eui_64_mac): + if not dynamic_only or 'dynamic' in line: addrs.append(addr) if addrs: @@ -467,7 +465,9 @@ def ns_query(address): try: answers = dns.resolver.query(address, rtype) - except (dns.resolver.NXDOMAIN, dns.resolver.NoNameservers): + # If we hit a non-authoritative recursor it's going + # to raise a dns.resolver.NoAnswer exception + except (dns.resolver.NoAnswer, dns.resolver.NXDOMAIN, dns.resolver.NoNameservers): return None if answers: diff --git a/tests/contrib/network/test_ip.py b/tests/contrib/network/test_ip.py index 606fc8a39..989788d7e 100644 --- a/tests/contrib/network/test_ip.py +++ b/tests/contrib/network/test_ip.py @@ -345,7 +345,7 @@ def test_get_ipv6_addr(self, _interfaces, _ifaddresses, mock_check_out, mock_get_iface_from_addr): mock_get_iface_from_addr.return_value = 'eth0' mock_check_out.return_value = \ - b"inet6 2a01:348:2f4:0:685e:5748:ae62:209f/64 scope global dynamic" + b"inet6 2a01:348:2f4:0:685e:5748:ae62:209f/64 scope global" _interfaces.return_value = DUMMY_ADDRESSES.keys() _ifaddresses.side_effect = DUMMY_ADDRESSES.__getitem__ result = net_ip.get_ipv6_addr(dynamic_only=False) @@ -355,8 +355,7 @@ def test_get_ipv6_addr(self, _interfaces, _ifaddresses, mock_check_out, @patch('charmhelpers.contrib.network.ip.subprocess.check_output') @patch.object(netifaces, 'ifaddresses') @patch.object(netifaces, 'interfaces') - def test_get_ipv6_addr_global_dynamic(self, _interfaces, _ifaddresses, - mock_check_out, + def test_get_ipv6_addr_global_dynamic(self, _interfaces, _ifaddresses, mock_check_out, mock_get_iface_from_addr): mock_get_iface_from_addr.return_value = 'eth0' mock_check_out.return_value = \ @@ -366,6 +365,34 @@ def test_get_ipv6_addr_global_dynamic(self, _interfaces, _ifaddresses, result = net_ip.get_ipv6_addr(dynamic_only=False) self.assertEqual(['2a01:348:2f4:0:685e:5748:ae62:209f'], result) + @patch('charmhelpers.contrib.network.ip.get_iface_from_addr') + @patch('charmhelpers.contrib.network.ip.subprocess.check_output') + @patch.object(netifaces, 'ifaddresses') + @patch.object(netifaces, 'interfaces') + def test_get_ipv6_addr_global_dynamic_only(self, _interfaces, _ifaddresses, mock_check_out, + mock_get_iface_from_addr): + mock_get_iface_from_addr.return_value = 'eth0' + mock_check_out.return_value = \ + (b"inet6 2a01:348:2f4:0:685e:5748:ae62:209f/64 scope global\n" + + b"inet6 2a01:348:2f4:0:685e:5748:ae62:209f/64 scope global dynamic") + _interfaces.return_value = DUMMY_ADDRESSES.keys() + _ifaddresses.side_effect = DUMMY_ADDRESSES.__getitem__ + result = net_ip.get_ipv6_addr(dynamic_only=True) + self.assertEqual(['2a01:348:2f4:0:685e:5748:ae62:209f'], result) + + @patch('charmhelpers.contrib.network.ip.get_iface_from_addr') + @patch('charmhelpers.contrib.network.ip.subprocess.check_output') + @patch.object(netifaces, 'ifaddresses') + @patch.object(netifaces, 'interfaces') + def test_get_ipv6_addr_no_dynamic_addresses(self, _interfaces, _ifaddresses, mock_check_out, + mock_get_iface_from_addr): + mock_get_iface_from_addr.return_value = 'eth0' + mock_check_out.return_value = \ + b"inet6 2a01:348:2f4:0:685e:5748:ae62:209f/64 scope global" + _interfaces.return_value = DUMMY_ADDRESSES.keys() + _ifaddresses.side_effect = DUMMY_ADDRESSES.__getitem__ + self.assertRaises(Exception, net_ip.get_ipv6_addr, dynamic_only=True) + @patch.object(netifaces, 'interfaces') def test_get_ipv6_addr_invalid_nic(self, _interfaces): _interfaces.return_value = DUMMY_ADDRESSES.keys() @@ -709,7 +736,7 @@ def test_ns_query_lookup_fail(self, apt_install): self.assertEquals(nsq, None) @patch('charmhelpers.contrib.network.ip.apt_install') - def test_ns_query_loopup_fail_real_implementation(self, apt_install): + def test_ns_query_lookup_fail_real_implementation(self, apt_install): self.assertEqual(net_ip.ns_query('nonexistant'), None) apt_install.assert_not_called()