diff --git a/src/ipahealthcheck/ipa/idns.py b/src/ipahealthcheck/ipa/idns.py index 48a979d4..e294db22 100644 --- a/src/ipahealthcheck/ipa/idns.py +++ b/src/ipahealthcheck/ipa/idns.py @@ -171,72 +171,72 @@ def check(self): key=realm, msg='expected realm missing') - if a_rec: - # Look up the ipa-ca records - qname = "ipa-ca." + api.env.domain + "." - logger.debug("Search DNS for A record of %s", qname) + # Verify that all of the ipa-ca record IPs match those of + # servers with a CA role. Report any missing or unexpected. + qname = "ipa-ca." + api.env.domain + "." + ipa_ca_records = [] + for dtype in (rdatatype.A, rdatatype.AAAA): + logger.debug("Search DNS for %s records of %s", dtype.name, qname) try: - answers = resolve(qname, rdatatype.A) + answers = resolve(qname, dtype) except DNSException as e: logger.debug("DNS record not found: %s", e.__class__.__name__) answers = [] - - for answer in answers: - logger.debug("DNS record found: %s", answer) - ipaddr = answer.to_text() - try: - yield Result(self, constants.SUCCESS, - key=ipaddr) - except ValueError: - yield Result(self, constants.WARNING, - key=ipaddr, - msg='expected ipa-ca IPv4 address missing') - - ca_count = 0 - for server in system_records.servers_data: - master = system_records.servers_data.get(server) - if 'CA server' in master.get('roles'): - ca_count += 1 - - if len(answers) != ca_count: - yield Result( - self, constants.WARNING, - key='ca_count_a_rec', - msg='Got {count} ipa-ca A records, expected {expected}', - count=len(answers), - expected=ca_count) - - if aaaa_rec: - # Look up the ipa-ca records - qname = "ipa-ca." + api.env.domain + "." - logger.debug("Search DNS for AAAA record of %s", qname) - try: - answers = resolve(qname, rdatatype.AAAA) - except DNSException as e: - logger.debug("DNS record not found: %s", e.__class__.__name__) - answers = [] - for answer in answers: - logger.debug("DNS record found: %s", answer) - ipaddr = answer.to_text() - try: - yield Result(self, constants.SUCCESS, - key=ipaddr) - except ValueError: - yield Result(self, constants.WARNING, - key=ipaddr, - msg='expected ipa-ca IPv6 address missing') - - ca_count = 0 - for server in system_records.servers_data: - master = system_records.servers_data.get(server) - if 'CA server' in master.get('roles'): - ca_count += 1 - - if len(answers) != ca_count: - yield Result( - self, constants.WARNING, - key='ca_count_aaaa_rec', - msg='Got {count} ipa-ca AAAA records, expected {expected}', - count=len(answers), - expected=ca_count) + ipa_ca_records.append(answer.to_text()) + + # Get the set of servers with the 'CA server' role + ca_servers = {} + for server in system_records.servers_data: + host = system_records.servers_data.get(server) + if 'CA server' in host.get('roles'): + for dtype in (rdatatype.A, rdatatype.AAAA): + try: + a = resolve(server + '.', dtype) + except DNSException: + pass + else: + for answer in a: + if server in ca_servers: + ca_servers[server].append(answer.to_text()) + else: + ca_servers[server] = [answer.to_text()] + + all_ca_ipaddr = [] + for server, ipaddrs in ca_servers.items(): + for ipaddr in ipaddrs: + all_ca_ipaddr.append(ipaddr) + + # Loop through the ipa-ca records to determine if any are not + # in the collection of all the reported CA server IPs. + errors = 0 + for ipaddr in ipa_ca_records: + if ipaddr not in all_ca_ipaddr: + errors += 1 + yield Result(self, constants.WARNING, + key='ipa_ca_non_server_%s' % ipaddr, + ipaddr=ipaddr, + msg='Unexpected ipa-ca address {ipaddr}') + + # Remove any IP addresses we found for ipa-ca from the set of + # IP addresses for all the IPA servers. Any remaining ones + # are not in the ipa-ca A/AAAA record. We're only looking at + # the DNS advertised servers so hidden ones should not be + # here. + for server, ipaddrs in ca_servers.items(): + for ipaddr in ipa_ca_records: + if ipaddr in ipaddrs: + ipaddrs.remove(ipaddr) + + for server, ipaddrs in ca_servers.items(): + if ipaddrs: + errors += 1 + yield Result(self, constants.WARNING, + key='ipa_ca_missing_%s' % server, + server=server, + ipaddr=', '.join(ipaddrs), + msg='expected ipa-ca to contain {ipaddr} for ' + '{server}') + + if errors == 0: + yield Result(self, constants.SUCCESS, key='ipa_ca_check') diff --git a/tests/test_ipa_dns.py b/tests/test_ipa_dns.py index 8fc267e6..787ac95d 100644 --- a/tests/test_ipa_dns.py +++ b/tests/test_ipa_dns.py @@ -12,6 +12,7 @@ version, ) from dns.resolver import Answer +from dns.exception import DNSException from base import BaseTest from util import capture_results, m_api @@ -45,6 +46,12 @@ else: resolve_rrsets_import = 'ipaserver.install.installutils.resolve_rrsets_nss' +# Global for test_dnsrecords_missing_ca() +no_ca_count = 0 + +# Global for test_dnsrecords_extra_ca() +extra_ca_count = 0 + def add_srv_records(qname, port_map, priority=0, weight=100): rdlist = [] @@ -229,6 +236,41 @@ def fake_query_one_txt(qname, rdtype=rdatatype.A, rdclass=rdataclass.IN, return fake_query(qname, rdtype, rdclass, count, fake_txt=True) +def fake_query_one_no_ca(qname, rdtype=rdatatype.A, + rdclass=rdataclass.IN, count=1): + """This is the singular side for resolve(). Since I use the + side_effect as a callable I need to maintain my own state + so I can still take in arguments from the check. + + The idea here is to start raising exceptions when the + CA servers are resolved in the system records such that + a CA is droppped. + """ + global no_ca_count + no_ca_count += 1 + if no_ca_count == 3: + # This drops the AAAA ipa-ca record + raise DNSException() + return fake_query(qname, rdtype, rdclass, count=1) + + +def fake_query_one_extra_ca(qname, rdtype=rdatatype.A, + rdclass=rdataclass.IN, count=1): + """This is the singular side for resolve(). Since I use the + side_effect as a callable I need to maintain my own state + so I can still take in arguments from the check. + + This adds an extra record during the ipa-ca AAAA + lookup. + """ + global extra_ca_count + extra_ca_count += 1 + if extra_ca_count == 3: + return fake_query(qname, rdtype, rdclass, count=2) + else: + return fake_query(qname, rdtype, rdclass, count=1) + + def get_results_by_severity(results, severity): """Return the results with a matching severity""" new_results = [] @@ -285,9 +327,9 @@ def test_dnsrecords_single(self, mock_query, mock_query_uri, self.results = capture_results(f) if has_uri_support: - expected = 14 + expected = 13 else: - expected = 10 + expected = 9 assert len(self.results) == expected for result in self.results.results: @@ -343,9 +385,9 @@ def test_dnsrecords_two(self, mock_query, mock_query_uri, self.results = capture_results(f) if has_uri_support: - expected = 27 + expected = 24 else: - expected = 19 + expected = 16 assert len(self.results) == expected for result in self.results.results: @@ -412,9 +454,9 @@ def test_dnsrecords_three(self, mock_query, mock_query_uri, self.results = capture_results(f) if has_uri_support: - expected = 40 + expected = 35 else: - expected = 28 + expected = 23 assert len(self.results) == expected for result in self.results.results: @@ -479,9 +521,9 @@ def test_dnsrecords_three_mixed(self, mock_query, mock_query_uri, self.results = capture_results(f) if has_uri_support: - expected = 36 + expected = 35 else: - expected = 24 + expected = 23 assert len(self.results) == expected for result in self.results.results: @@ -551,18 +593,18 @@ def test_dnsrecords_missing_server(self, mock_query, mock_query_uri, self.results = capture_results(f) if has_uri_support: - expected = 40 + expected = 35 else: - expected = 28 + expected = 23 assert len(self.results) == expected ok = get_results_by_severity(self.results.results, constants.SUCCESS) warn = get_results_by_severity(self.results.results, constants.WARNING) if has_uri_support: - assert len(ok) == 33 + assert len(ok) == 28 assert len(warn) == 7 else: - assert len(ok) == 21 + assert len(ok) == 16 assert len(warn) == 7 for result in warn: @@ -631,19 +673,19 @@ def test_dnsrecords_missing_ipa_ca(self, mock_query, mock_query_uri, self.results = capture_results(f) if has_uri_support: - expected = 40 + expected = 35 else: - expected = 28 + expected = 23 assert len(self.results) == expected ok = get_results_by_severity(self.results.results, constants.SUCCESS) warn = get_results_by_severity(self.results.results, constants.WARNING) if has_uri_support: - assert len(ok) == 38 - assert len(warn) == 2 + assert len(ok) == 35 + assert len(warn) == 0 else: - assert len(ok) == 26 - assert len(warn) == 2 + assert len(ok) == 23 + assert len(warn) == 0 for result in warn: assert re.match( @@ -719,18 +761,18 @@ def test_dnsrecords_extra_srv(self, mock_query, mock_query_uri, self.results = capture_results(f) if has_uri_support: - expected = 47 + expected = 42 else: - expected = 35 + expected = 30 assert len(self.results) == expected ok = get_results_by_severity(self.results.results, constants.SUCCESS) warn = get_results_by_severity(self.results.results, constants.WARNING) if has_uri_support: - assert len(ok) == 40 + assert len(ok) == 35 assert len(warn) == 7 else: - assert len(ok) == 28 + assert len(ok) == 23 assert len(warn) == 7 for result in warn: @@ -769,18 +811,18 @@ def test_dnsrecords_bad_realm(self, mock_query, mock_query_uri, self.results = capture_results(f) if has_uri_support: - expected = 14 + expected = 13 else: - expected = 10 + expected = 9 assert len(self.results) == expected ok = get_results_by_severity(self.results.results, constants.SUCCESS) warn = get_results_by_severity(self.results.results, constants.WARNING) if has_uri_support: - assert len(ok) == 13 + assert len(ok) == 12 assert len(warn) == 1 else: - assert len(ok) == 9 + assert len(ok) == 8 assert len(warn) == 1 result = warn[0] @@ -819,12 +861,116 @@ def test_dnsrecords_one_with_ad(self, mock_query, mock_query_uri, self.results = capture_results(f) if has_uri_support: - expected = 20 + expected = 19 else: - expected = 16 + expected = 15 assert len(self.results) == expected for result in self.results.results: assert result.result == constants.SUCCESS assert result.source == 'ipahealthcheck.ipa.idns' assert result.check == 'IPADNSSystemRecordsCheck' + + @patch(resolve_rrsets_import) + @patch('ipapython.dnsutil.query_srv') + @patch('ipahealthcheck.ipa.idns.query_uri') + @patch('ipahealthcheck.ipa.idns.resolve') + def test_dnsrecords_missing_ca(self, mock_query, mock_query_uri, + mock_query_srv, mock_rrset): + """Missing AAAA record in ipa-ca""" + mock_query.side_effect = fake_query_one_no_ca + mock_query_srv.side_effect = query_srv([m_api.env.host]) + mock_query_uri.side_effect = query_uri([m_api.env.host]) + mock_rrset.side_effect = [ + resolve_rrsets(m_api.env.host, (rdatatype.A, rdatatype.AAAA,)) + ] + + m_api.Command.server_find.side_effect = [{ + 'result': [ + { + 'cn': [m_api.env.host], + 'enabled_role_servrole': [ + 'CA server', + 'IPA master' + ], + }, + ] + }] + framework = object() + registry.initialize(framework, config.Config) + f = IPADNSSystemRecordsCheck(registry) + + self.results = capture_results(f) + + if has_uri_support: + expected = 13 + else: + expected = 8 + assert len(self.results) == expected + + ok = get_results_by_severity(self.results.results, constants.SUCCESS) + warn = get_results_by_severity(self.results.results, constants.WARNING) + if has_uri_support: + assert len(ok) == 12 + assert len(warn) == 1 + else: + assert len(ok) == 8 + assert len(warn) == 1 + + result = warn[0] + assert result.kw.get('msg') == ( + 'expected ipa-ca to contain {ipaddr} for {server}' + ) + assert result.kw.get('ipaddr') == '2001:db8:1::1' + + @patch(resolve_rrsets_import) + @patch('ipapython.dnsutil.query_srv') + @patch('ipahealthcheck.ipa.idns.query_uri') + @patch('ipahealthcheck.ipa.idns.resolve') + def test_dnsrecords_extra_ca(self, mock_query, mock_query_uri, + mock_query_srv, mock_rrset): + """Extra AAAA record in ipa-ca""" + mock_query.side_effect = fake_query_one_extra_ca + mock_query_srv.side_effect = query_srv([m_api.env.host]) + mock_query_uri.side_effect = query_uri([m_api.env.host]) + mock_rrset.side_effect = [ + resolve_rrsets(m_api.env.host, (rdatatype.A, rdatatype.AAAA,)) + ] + + m_api.Command.server_find.side_effect = [{ + 'result': [ + { + 'cn': [m_api.env.host], + 'enabled_role_servrole': [ + 'CA server', + 'IPA master' + ], + }, + ] + }] + framework = object() + registry.initialize(framework, config.Config) + f = IPADNSSystemRecordsCheck(registry) + + self.results = capture_results(f) + + if has_uri_support: + expected = 13 + else: + expected = 8 + assert len(self.results) == expected + + ok = get_results_by_severity(self.results.results, constants.SUCCESS) + warn = get_results_by_severity(self.results.results, constants.WARNING) + if has_uri_support: + assert len(ok) == 12 + assert len(warn) == 1 + else: + assert len(ok) == 8 + assert len(warn) == 1 + + result = warn[0] + assert result.kw.get('msg') == ( + 'Unexpected ipa-ca address {ipaddr}' + ) + assert result.kw.get('ipaddr') == '2001:db8:1::2'