[2759] Apply check_zone before commiting DDNS changes

The response code of SERVFAIL is not the most suitable, but when
check_zone() fails, we don't know what type of condition exactly caused
the failure. We cannot map this condition to the different RCODE values
in RFC2136. There is no generic code for validation failures.

diff --git a/src/lib/python/isc/ddns/libddns_messages.mes b/src/lib/python/isc/ddns/libddns_messages.mes
index ab982ad548ea8e293ed5ba38c2e9ba3155475810..d9bfbc93f4373ce769fc38451cc18663e9bb948f 100644 (file)
--- a/src/lib/python/isc/ddns/libddns_messages.mes
+++ b/src/lib/python/isc/ddns/libddns_messages.mes
@@ -212,3 +212,11 @@ To make sure DDNS service is not interrupted, this problem is caught instead
 of reraised; The update is aborted, and a SERVFAIL is sent back to the client.
 This is most probably a bug in the DDNS code, but *could* be caused by
 the data source.
+
+% LIBDDNS_ZONE_INVALID_ERROR Newly received zone %1/%2 fails validation: %3
+The zone was received successfully, but it failed validation.
+
+% LIBDDNS_ZONE_INVALID_WARN Newly received zone %1/%2 has a problem: %3
+The zone was received successfully, but when checking it, it was discovered
+there's some issue with it. It might be correct, but it should be checked
+and possibly fixed. The problem does not stop the zone from being used.

diff --git a/src/lib/python/isc/ddns/session.py b/src/lib/python/isc/ddns/session.py
index febdfdc8f4b6ca0288d25d3ed84d96dc56cf73e9..e0cae24f7818f8a33e58ad17e8c2fa4ced7dda5f 100644 (file)
--- a/src/lib/python/isc/ddns/session.py
+++ b/src/lib/python/isc/ddns/session.py
@@ -812,6 +812,20 @@ class UpdateSession:
         self.__diff.delete_data(old_soa)
         self.__diff.add_data(new_soa)
 
+    def __validate_error(self, reason):
+        '''
+        Used as error callback below.
+        '''
+        logger.error(LIBDDNS_ZONE_INVALID_ERROR, self.__zname, self.__zclass,
+                     reason)
+
+    def __validate_warning(self, reason):
+        '''
+        Used as warning callback below.
+        '''
+        logger.warn(LIBDDNS_ZONE_INVALID_WARN, self.__zname, self.__zclass,
+                    reason)
+
     def __do_update(self):
         '''Scan, check, and execute the Update section in the
            DDNS Update message.
@@ -849,6 +863,11 @@ class UpdateSession:
                 elif rrset.get_class() == RRClass.NONE:
                     self.__do_update_delete_rrs_from_rrset(rrset)
 
+            if not check_zone(self.__zname, self.__zclass,
+                              self.__diff.get_rrset_collection(),
+                              (self.__validate_error, self.__validate_warning)):
+                raise UpdateError('Validation of the new zone failed',
+                                  self.__zname, self.__zclass, Rcode.SERVFAIL)
             self.__diff.commit()
             return Rcode.NOERROR
         except isc.datasrc.Error as dse:

diff --git a/src/lib/python/isc/ddns/tests/session_tests.py b/src/lib/python/isc/ddns/tests/session_tests.py
index 4880d724d9f3bb13a08900d406f565c5e6c2358f..405abed1b955d5c40df6d5d5479696d3233906e8 100644 (file)
--- a/src/lib/python/isc/ddns/tests/session_tests.py
+++ b/src/lib/python/isc/ddns/tests/session_tests.py
@@ -1481,6 +1481,16 @@ class SessionTest(SessionTestBase):
         self.assertEqual(Rcode.SERVFAIL.to_text(),
                          self._session._UpdateSession__do_update().to_text())
 
+    def test_check_zone_failure(self):
+        # ns3.example.org. is an NS and should not have a CNAME
+        # record. This would cause check_zone() to fail.
+        self.__initialize_update_rrsets()
+        new_cname = create_rrset("ns3.example.org", TEST_RRCLASS,
+                                 RRType.CNAME, 3600,
+                                 [ "cname.example.org." ])
+
+        self.check_full_handle_result(Rcode.SERVFAIL, [ new_cname ])
+
 class SessionACLTest(SessionTestBase):
     '''ACL related tests for update session.'''
     def test_update_acl_check(self):