[2020] updated ZoneConfig ctor so the 2ndary zones are a set, not a list.

this will simplify the constructor, and would make more sense because the
caller should be able to build the set beforehand anyway.  b10-ddns is
updated accordingly.

src/bin/ddns/ddns.py.in

@@ -182,7 +182,7 @@ class DDNSServer:
             self._cc.get_default_value('zones'))
         self._cc.start()
 
-        # A list of secondary zones, retrieved from zonemgr configuration.
+        # A set of secondary zones, retrieved from zonemgr configuration.
         self._secondary_zones = None
 
         # Get necessary configurations from remote modules.
@@ -281,7 +281,7 @@ class DDNSServer:
         # it can work for both the initial default case and updates.
         sec_zones, _ = self._cc.get_remote_config_value(ZONEMGR_MODULE_NAME,
                                                         'secondary_zones')
-        new_secondary_zones = []
+        new_secondary_zones = set()
         try:
             # Parse the new config and build a new list of secondary zones.
             # Note that validation should have been done by zonemgr, so
@@ -299,7 +299,7 @@ class DDNSServer:
                 else:
                     zclass = RRClass(module_config.get_default_value(
                             'secondary_zones/class'))
-                new_secondary_zones.append((zname, zclass))
+                new_secondary_zones.add((zname, zclass))
             self._secondary_zones = new_secondary_zones
             logger.info(DDNS_SECONDARY_ZONES_UPDATE, len(self._secondary_zones))
         except Exception as ex:

src/bin/ddns/ddns_messages.mes

@@ -185,4 +185,5 @@ update its internal copy of that configuration.
 % DDNS_SECONDARY_ZONES_UPDATE updated secondary zone list (%1 zones are listed)
 b10-ddns has successfully updated the internal copy of secondary zones
 obtained from b10-zonemgr, based on a latest update to zonemgr's
-configuration.  The number of newly configured secondary zones is logged.
+configuration.  The number of newly configured (unique) secondary
+zones is logged.

src/bin/ddns/tests/ddns_test.py

@@ -451,7 +451,7 @@ class TestDDNSServer(unittest.TestCase):
 
     def test_secondary_zones_config(self):
         # By default it should be an empty list
-        self.assertEqual([], self.ddns_server._secondary_zones)
+        self.assertEqual(set(), self.ddns_server._secondary_zones)
 
         # emulating an update.  calling add_remote_config_by_name is a
         # convenient faked way to invoke the callback.
@@ -460,14 +460,22 @@ class TestDDNSServer(unittest.TestCase):
         self.__cc_session.add_remote_config_by_name('Zonemgr')
 
         # The new set of secondary zones should be stored.
-        self.assertEqual([(TEST_ZONE_NAME, TEST_RRCLASS)],
+        self.assertEqual({(TEST_ZONE_NAME, TEST_RRCLASS)},
                          self.ddns_server._secondary_zones)
 
         # Similar to the above, but the optional 'class' is missing.
         self.__cc_session._zonemgr_config = {'secondary_zones': [
                 {'name': TEST_ZONE_NAME_STR}]}
         self.__cc_session.add_remote_config_by_name('Zonemgr')
-        self.assertEqual([(TEST_ZONE_NAME, TEST_RRCLASS)],
+        self.assertEqual({(TEST_ZONE_NAME, TEST_RRCLASS)},
+                         self.ddns_server._secondary_zones)
+
+        # The given list has a duplicate.  The resulting set should unify them.
+        self.__cc_session._zonemgr_config = {'secondary_zones': [
+                {'name': TEST_ZONE_NAME_STR, 'class': TEST_RRCLASS_STR},
+                {'name': TEST_ZONE_NAME_STR, 'class': TEST_RRCLASS_STR}]}
+        self.__cc_session.add_remote_config_by_name('Zonemgr')
+        self.assertEqual({(TEST_ZONE_NAME, TEST_RRCLASS)},
                          self.ddns_server._secondary_zones)
 
         # Check the 2ndary zones aren't changed if the new config doesn't
@@ -479,9 +487,13 @@ class TestDDNSServer(unittest.TestCase):
         self.assertEqual(42, self.ddns_server._secondary_zones)
         self.ddns_server._secondary_zones = seczones_orig
 
+        # If the update config is broken, the existing set should be intact.
         self.__cc_session._zonemgr_config = {'secondary_zones': [
+                {'name': 'good.example', 'class': TEST_RRCLASS_STR},
                 {'name': 'badd..example', 'class': TEST_RRCLASS_STR}]}
         self.__cc_session.add_remote_config_by_name('Zonemgr')
+        self.assertEqual({(TEST_ZONE_NAME, TEST_RRCLASS)},
+                         self.ddns_server._secondary_zones)
 
     def test_shutdown_command(self):
         '''Test whether the shutdown command works'''

src/lib/python/isc/ddns/tests/session_tests.py

@@ -200,7 +200,7 @@ class SessionTestBase(unittest.TestCase):
         self._acl_map = {(TEST_ZONE_NAME, TEST_RRCLASS):
                              REQUEST_LOADER.load([{"action": "ACCEPT"}])}
         self._session = UpdateSession(self._update_msg, TEST_CLIENT4,
-                                      ZoneConfig([], TEST_RRCLASS,
+                                      ZoneConfig(set(), TEST_RRCLASS,
                                                  self._datasrc_client,
                                                  self._acl_map))
         self._session._get_update_zone()
@@ -327,7 +327,7 @@ class SessionTest(SessionTestBase):
         msg = create_update_msg(zones=[Question(TEST_ZONE_NAME, TEST_RRCLASS,
                                                 RRType.SOA())])
         session = UpdateSession(msg, TEST_CLIENT4,
-                                ZoneConfig([(TEST_ZONE_NAME, TEST_RRCLASS)],
+                                ZoneConfig({(TEST_ZONE_NAME, TEST_RRCLASS)},
                                            TEST_RRCLASS, self._datasrc_client))
         self.assertEqual(UPDATE_ERROR, session.handle()[0])
         self.check_response(session.get_message(), Rcode.NOTIMP())
@@ -336,7 +336,7 @@ class SessionTest(SessionTestBase):
         '''Common test sequence for the 'notauth' test'''
         msg = create_update_msg(zones=[Question(zname, zclass, RRType.SOA())])
         session = UpdateSession(msg, TEST_CLIENT4,
-                                ZoneConfig([(TEST_ZONE_NAME, TEST_RRCLASS)],
+                                ZoneConfig({(TEST_ZONE_NAME, TEST_RRCLASS)},
                                            TEST_RRCLASS, self._datasrc_client))
         self.assertEqual(UPDATE_ERROR, session.handle()[0])
         self.check_response(session.get_message(), Rcode.NOTAUTH())
@@ -360,7 +360,7 @@ class SessionTest(SessionTestBase):
         msg = create_update_msg(zones=[Question(TEST_ZONE_NAME, TEST_RRCLASS,
                                                 RRType.SOA())])
         session = UpdateSession(msg, TEST_CLIENT4,
-                                ZoneConfig([(TEST_ZONE_NAME, TEST_RRCLASS)],
+                                ZoneConfig({(TEST_ZONE_NAME, TEST_RRCLASS)},
                                            TEST_RRCLASS,
                                            BadDataSourceClient()))
         self.assertEqual(UPDATE_ERROR, session.handle()[0])
@@ -617,7 +617,7 @@ class SessionTest(SessionTestBase):
            from 'prerequisites'. Then checks if __check_prerequisites()
            returns the Rcode specified in 'expected'.'''
         msg = create_update_msg([TEST_ZONE_RECORD], prerequisites)
-        zconfig = ZoneConfig([], TEST_RRCLASS, self._datasrc_client,
+        zconfig = ZoneConfig(set(), TEST_RRCLASS, self._datasrc_client,
                              self._acl_map)
         session = UpdateSession(msg, TEST_CLIENT4, zconfig)
         session._get_update_zone()
@@ -643,7 +643,7 @@ class SessionTest(SessionTestBase):
            from 'updates'. Then checks if __do_prescan()
            returns the Rcode specified in 'expected'.'''
         msg = create_update_msg([TEST_ZONE_RECORD], [], updates)
-        zconfig = ZoneConfig([], TEST_RRCLASS, self._datasrc_client,
+        zconfig = ZoneConfig(set(), TEST_RRCLASS, self._datasrc_client,
                              self._acl_map)
         session = UpdateSession(msg, TEST_CLIENT4, zconfig)
         session._get_update_zone()
@@ -663,7 +663,7 @@ class SessionTest(SessionTestBase):
            from 'updates'. Then checks if __handle()
            results in a response with rcode 'expected'.'''
         msg = create_update_msg([TEST_ZONE_RECORD], [], updates)
-        zconfig = ZoneConfig([], TEST_RRCLASS, self._datasrc_client,
+        zconfig = ZoneConfig(set(), TEST_RRCLASS, self._datasrc_client,
                              self._acl_map)
         session = UpdateSession(msg, TEST_CLIENT4, zconfig)
 
@@ -1406,7 +1406,7 @@ class SessionACLTest(SessionTestBase):
         '''
         # create a separate session, with default (empty) ACL map.
         session = UpdateSession(self._update_msg,
-                                TEST_CLIENT4, ZoneConfig([], TEST_RRCLASS,
+                                TEST_CLIENT4, ZoneConfig(set(), TEST_RRCLASS,
                                                          self._datasrc_client))
         # then the request should be rejected.
         self.assertEqual((UPDATE_ERROR, None, None), session.handle())
@@ -1435,7 +1435,7 @@ class SessionACLTest(SessionTestBase):
         # If the message doesn't contain TSIG, it doesn't match the ACCEPT
         # ACL entry, and the request should be rejected.
         session = UpdateSession(self._update_msg,
-                                TEST_CLIENT4, ZoneConfig([], TEST_RRCLASS,
+                                TEST_CLIENT4, ZoneConfig(set(), TEST_RRCLASS,
                                                          self._datasrc_client,
                                                          acl_map))
         self.assertEqual((UPDATE_ERROR, None, None), session.handle())
@@ -1444,7 +1444,7 @@ class SessionACLTest(SessionTestBase):
         # If the message contains TSIG, it should match the ACCEPT
         # ACL entry, and the request should be granted.
         session = UpdateSession(create_update_msg(tsig_key=TEST_TSIG_KEY),
-                                TEST_CLIENT4, ZoneConfig([], TEST_RRCLASS,
+                                TEST_CLIENT4, ZoneConfig(set(), TEST_RRCLASS,
                                                          self._datasrc_client,
                                                          acl_map))
         self.assertEqual((UPDATE_SUCCESS, TEST_ZONE_NAME, TEST_RRCLASS),

src/lib/python/isc/ddns/tests/zone_config_tests.py

@@ -55,7 +55,7 @@ class ZoneConfigTest(unittest.TestCase):
     '''Some basic tests for the ZoneConfig class.'''
     def setUp(self):
         self.__datasrc_client = FakeDataSourceClient()
-        self.zconfig = ZoneConfig([(TEST_SECONDARY_ZONE_NAME, TEST_RRCLASS)],
+        self.zconfig = ZoneConfig({(TEST_SECONDARY_ZONE_NAME, TEST_RRCLASS)},
                                   TEST_RRCLASS, self.__datasrc_client)
 
     def test_find_zone(self):
@@ -87,34 +87,27 @@ class ZoneConfigTest(unittest.TestCase):
                                                  TEST_RRCLASS)))
         # zone class doesn't match (but zone name matches)
         self.__datasrc_client.set_find_result(DataSourceClient.SUCCESS)
-        zconfig = ZoneConfig([(TEST_SECONDARY_ZONE_NAME, TEST_RRCLASS)],
+        zconfig = ZoneConfig({(TEST_SECONDARY_ZONE_NAME, TEST_RRCLASS)},
                              RRClass.CH(), self.__datasrc_client)
         self.assertEqual((ZONE_NOTFOUND, None),
                          (zconfig.find_zone(TEST_ZONE_NAME, TEST_RRCLASS)))
         # similar to the previous case, but also in the secondary list
-        zconfig = ZoneConfig([(TEST_ZONE_NAME, TEST_RRCLASS)],
+        zconfig = ZoneConfig({(TEST_ZONE_NAME, TEST_RRCLASS)},
                              RRClass.CH(), self.__datasrc_client)
         self.assertEqual((ZONE_NOTFOUND, None),
                          (zconfig.find_zone(TEST_ZONE_NAME, TEST_RRCLASS)))
 
         # check some basic tests varying the secondary list.
         # empty secondary list doesn't cause any disruption.
-        zconfig = ZoneConfig([], TEST_RRCLASS, self.__datasrc_client)
+        zconfig = ZoneConfig(set(), TEST_RRCLASS, self.__datasrc_client)
         self.assertEqual((ZONE_PRIMARY, self.__datasrc_client),
                          self.zconfig.find_zone(TEST_ZONE_NAME, TEST_RRCLASS))
-        # adding some mulitle tuples, including subdomainof the test zone name,
-        # and the same zone name but a different class
-        zconfig = ZoneConfig([(TEST_SECONDARY_ZONE_NAME, TEST_RRCLASS),
+        # adding some mulitle tuples, including subdomain of the test zone
+        # name, and the same zone name but a different class
+        zconfig = ZoneConfig({(TEST_SECONDARY_ZONE_NAME, TEST_RRCLASS),
                               (Name('example'), TEST_RRCLASS),
                               (Name('sub.example.org'), TEST_RRCLASS),
-                              (TEST_ZONE_NAME, RRClass.CH())],
-                             TEST_RRCLASS, self.__datasrc_client)
-        self.assertEqual((ZONE_PRIMARY, self.__datasrc_client),
-                         self.zconfig.find_zone(TEST_ZONE_NAME, TEST_RRCLASS))
-        # secondary zone list has a duplicate entry, which is just
-        # (effecitivey) ignored
-        zconfig = ZoneConfig([(TEST_SECONDARY_ZONE_NAME, TEST_RRCLASS),
-                              (TEST_SECONDARY_ZONE_NAME, TEST_RRCLASS)],
+                              (TEST_ZONE_NAME, RRClass.CH())},
                              TEST_RRCLASS, self.__datasrc_client)
         self.assertEqual((ZONE_PRIMARY, self.__datasrc_client),
                          self.zconfig.find_zone(TEST_ZONE_NAME, TEST_RRCLASS))
@@ -122,7 +115,7 @@ class ZoneConfigTest(unittest.TestCase):
 class ACLConfigTest(unittest.TestCase):
     def setUp(self):
         self.__datasrc_client = FakeDataSourceClient()
-        self.__zconfig = ZoneConfig([(TEST_SECONDARY_ZONE_NAME, TEST_RRCLASS)],
+        self.__zconfig = ZoneConfig({(TEST_SECONDARY_ZONE_NAME, TEST_RRCLASS)},
                                     TEST_RRCLASS, self.__datasrc_client)
 
     def test_get_update_acl(self):

src/lib/python/isc/ddns/zone_config.py

@@ -38,7 +38,7 @@ class ZoneConfig:
         '''Constructor.
 
         Parameters:
-        - secondaries: a list of 2-element tuples.  Each element is a pair
+        - secondaries: a set of 2-element tuples.  Each element is a pair
           of isc.dns.Name and isc.dns.RRClass, and identifies a single
           secondary zone.
         - datasrc_class: isc.dns.RRClass object.  Specifies the RR class
@@ -53,9 +53,7 @@ class ZoneConfig:
           ACL will be applied to all zones, which is to reject any requests.
 
         '''
-        self.__secondaries = set()
-        for (zname, zclass) in secondaries:
-            self.__secondaries.add((zname, zclass))
+        self.__secondaries = secondaries
         self.__datasrc_class = datasrc_class
         self.__datasrc_client = datasrc_client
         self.__default_acl = REQUEST_LOADER.load([{"action": "REJECT"}])