From: Miod Vallat Date: Wed, 2 Apr 2025 12:34:21 +0000 (+0200) Subject: Try to give more helpful information in checkRRSet errors. X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=bfdcfa62b67ea5ee0ba78bf28749f4523ea58759;p=thirdparty%2Fpdns.git Try to give more helpful information in checkRRSet errors. --- diff --git a/pdns/check-zone.cc b/pdns/check-zone.cc index f23a58d92..a8c1f839a 100644 --- a/pdns/check-zone.cc +++ b/pdns/check-zone.cc @@ -53,24 +53,27 @@ void checkRRSet(vector& records, const ZoneName& zone) if (previous.qname == rec.qname) { if (previous.qtype == rec.qtype) { if (onlyOneEntryTypes.count(rec.qtype.getCode()) != 0) { - rejectRecord(rec, " has more than one record"); + rejectRecord(rec, ": only one such record allowed"); } if (previous.content == rec.content) { - throw CheckException("Duplicate record in RRset " + rec.qname.toString() + " IN " + rec.qtype.toString() + " with content \"" + rec.content + "\""); + rejectRecord(rec, std::string{": duplicate record with content \""} + rec.content + "\""); } } - else if (QType::exclusiveEntryTypes.count(rec.qtype.getCode()) != 0 || QType::exclusiveEntryTypes.count(previous.qtype.getCode()) != 0) { - rejectRecord(rec, ": Conflicts with another RRset"); + else { + if (QType::exclusiveEntryTypes.count(rec.qtype.getCode()) != 0 + || QType::exclusiveEntryTypes.count(previous.qtype.getCode()) != 0) { + rejectRecord(rec, std::string{": conflicts with existing "} + previous.qtype.toString() + " RRset of the same name"); + } } } if (rec.qname == zone.operator const DNSName&()) { if (nonApexTypes.count(rec.qtype.getCode()) != 0) { - rejectRecord(rec, " is not allowed at apex"); + rejectRecord(rec, ": is not allowed at apex"); } } else if (atApexTypes.count(rec.qtype.getCode()) != 0) { - rejectRecord(rec, " is only allowed at apex"); + rejectRecord(rec, ": is only allowed at apex"); } // Check if the DNSNames that should be hostnames, are hostnames diff --git a/regression-tests.api/test_Zones.py b/regression-tests.api/test_Zones.py index f81180800..318a7e0fc 100644 --- a/regression-tests.api/test_Zones.py +++ b/regression-tests.api/test_Zones.py @@ -1203,7 +1203,7 @@ $ORIGIN %NAME% data=json.dumps(payload), headers={'content-type': 'application/json'}) self.assertEqual(r.status_code, 422) - self.assertIn('Conflicts with another RRset', r.json()['error']) + self.assertIn('conflicts with existing NS RRset', r.json()['error']) def test_export_zone_json(self): name, payload, zone = self.create_zone(nameservers=['ns1.foo.com.', 'ns2.foo.com.'], soa_edit_api='') @@ -1489,7 +1489,7 @@ $NAME$ 1D IN SOA ns1.example.org. hostmaster.example.org. ( data=json.dumps(payload), headers={'content-type': 'application/json'}) self.assertEqual(r.status_code, 422) - self.assertIn('Duplicate record in RRset', r.json()['error']) + self.assertIn('duplicate record with content', r.json()['error']) def test_zone_rr_update_duplicate_rrset(self): name, payload, zone = self.create_zone() @@ -1772,7 +1772,7 @@ $NAME$ 1D IN SOA ns1.example.org. hostmaster.example.org. ( r = self.session.patch(self.url("/api/v1/servers/localhost/zones/" + name), data=json.dumps(payload), headers={'content-type': 'application/json'}) self.assertEqual(r.status_code, 422) - self.assertIn('IN ' + qtype + ' has more than one record', r.json()['error']) + self.assertIn('IN ' + qtype + ': only one such record', r.json()['error']) def test_rrset_zone_apex(self): name, payload, zone = self.create_zone() @@ -2592,7 +2592,7 @@ $NAME$ 1D IN SOA ns1.example.org. hostmaster.example.org. ( ("out of zone", [{'name': 'not-in-zone.', 'type': 'NS', 'ttl': 3600, 'records': [{"content": "ns1.example.org."}]}]), ("contains unsupported characters", [{'name': 'test:.$NAME$', 'type': 'NS', 'ttl': 3600, 'records': [{"content": "ns1.example.org."}]}]), ("unknown type", [{'name': '$NAME$', 'type': 'INVALID', 'ttl': 3600, 'records': [{"content": "192.0.2.1"}]}]), - ("Conflicts with another RRset", [{'name': '$NAME$', 'type': 'CNAME', 'ttl': 3600, 'records': [{"content": "example.org."}]}]), + ("conflicts with existing NS RRset", [{'name': '$NAME$', 'type': 'CNAME', 'ttl': 3600, 'records': [{"content": "example.org."}]}]), ]) def test_zone_replace_rrsets_invalid(self, expected_error, invalid_rrsets): """Test validation of RRsets before replacing them""" diff --git a/regression-tests/tests/pdnsutil-zone-handling/expected_result b/regression-tests/tests/pdnsutil-zone-handling/expected_result index 5bc05c02c..93025fe5e 100644 --- a/regression-tests/tests/pdnsutil-zone-handling/expected_result +++ b/regression-tests/tests/pdnsutil-zone-handling/expected_result @@ -5,8 +5,8 @@ host.bug.less. 3600 IN A 127.0.0.1 Ignoring duplicate record content "127.0.0.2" New rrset: host2.bug.less. 3600 IN A 127.0.0.2 -RRset cname.bug.less. IN CNAME: Conflicts with another RRset -RRset host.bug.less. IN CNAME: Conflicts with another RRset +RRset cname.bug.less. IN CNAME: conflicts with existing A RRset of the same name +RRset host.bug.less. IN CNAME: conflicts with existing A RRset of the same name New rrset: host2.bug.less. 3600 IN A 127.0.0.2 host2.bug.less. 3600 IN A 127.0.0.3 diff --git a/regression-tests/tests/pdnsutil-zone-handling/expected_result.lmdb b/regression-tests/tests/pdnsutil-zone-handling/expected_result.lmdb index 655dddf4d..3c87ac01e 100644 --- a/regression-tests/tests/pdnsutil-zone-handling/expected_result.lmdb +++ b/regression-tests/tests/pdnsutil-zone-handling/expected_result.lmdb @@ -5,8 +5,8 @@ host.bug.less. 3600 IN A 127.0.0.1 Ignoring duplicate record content "127.0.0.2" New rrset: host2.bug.less. 3600 IN A 127.0.0.2 -Attempting to add record to cname.bug.less which already has a CNAME record -Attempting to add CNAME to host.bug.less which already has existing records +RRset cname.bug.less. IN CNAME: conflicts with existing A RRset of the same name +RRset host.bug.less. IN CNAME: conflicts with existing A RRset of the same name New rrset: host2.bug.less. 3600 IN A 127.0.0.2 host2.bug.less. 3600 IN A 127.0.0.3