change for auth: change the parameter name from 'rrclass' to 'zone_class', set the default master and port is '127.0.0.1' and 53, refactor the code for processing command 'notify' and 'refresh/retransfer', rename the command sent from zonemgr to 'refresh_from_zonemgr'

git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac289@2636 e5f2f494-b856-4b98-b285-d166d9295462

src/bin/auth/auth_srv.cc

@@ -421,7 +421,7 @@ AuthSrvImpl::processNotify(const IOMessage& io_message, Message& message,
     static const string command_template_start =
         "{\"command\": [\"notify\", {\"zone_name\" : \"";
     static const string command_template_master = "\", \"master\" : \"";
-    static const string command_template_rrclass = "\", \"rrclass\" : \"";
+    static const string command_template_rrclass = "\", \"zone_class\" : \"";
     static const string command_template_end = "\"}]}";
 
     try {

src/bin/xfrin/tests/xfrin_test.py

@@ -63,6 +63,9 @@ class MockXfrin(Xfrin):
 
     def _cc_setup(self):
         pass
+
+    def _get_db_file(self):
+        pass
     
     def _cc_check_command(self):
         self._shutdown_event.set()
@@ -408,11 +411,16 @@ class TestXfrin(unittest.TestCase):
     def tearDown(self):
         self.xfr.shutdown()
 
-    def _do_parse(self):
-        return self.xfr._parse_cmd_params(self.args)
+    def _do_parse_zone_name_class(self):
+        return self.xfr._parse_zone_name_and_class(self.args)
+
+    def _do_parse_master_port(self):
+        return self.xfr._parse_master_and_port(self.args)
 
     def test_parse_cmd_params(self):
-        name, rrclass, master_addrinfo, db_file = self._do_parse()
+        name, rrclass = self._do_parse_zone_name_class()
+        master_addrinfo = self._do_parse_master_port()
+        db_file = self.args.get('db_file')
         self.assertEqual(master_addrinfo[4][1], int(TEST_MASTER_PORT))
         self.assertEqual(name, TEST_ZONE_NAME)
         self.assertEqual(rrclass, TEST_RRCLASS)
@@ -421,49 +429,50 @@ class TestXfrin(unittest.TestCase):
 
     def test_parse_cmd_params_default_port(self):
         del self.args['port']
-        master_addrinfo = self._do_parse()[2]
+        master_addrinfo = self._do_parse_master_port()
         self.assertEqual(master_addrinfo[4][1], 53)
 
     def test_parse_cmd_params_ip6master(self):
         self.args['master'] = TEST_MASTER_IPV6_ADDRESS
-        master_addrinfo = self._do_parse()[2]
+        master_addrinfo = self._do_parse_master_port()
         self.assertEqual(master_addrinfo[4][0], TEST_MASTER_IPV6_ADDRESS)
 
     def test_parse_cmd_params_chclass(self):
         self.args['zone_class'] = 'CH'
-        self.assertEqual(self._do_parse()[1], RRClass.CH())
+        self.assertEqual(self._do_parse_zone_name_class()[1], RRClass.CH())
 
     def test_parse_cmd_params_bogusclass(self):
         self.args['zone_class'] = 'XXX'
-        self.assertRaises(XfrinException, self._do_parse)
+        self.assertRaises(XfrinException, self._do_parse_zone_name_class)
 
     def test_parse_cmd_params_nozone(self):
         # zone name is mandatory.
         del self.args['zone_name']
-        self.assertRaises(XfrinException, self._do_parse)
+        self.assertRaises(XfrinException, self._do_parse_zone_name_class)
 
     def test_parse_cmd_params_nomaster(self):
         # master address is mandatory.
         del self.args['master']
-        self.assertRaises(XfrinException, self._do_parse)
+        master_addrinfo = self._do_parse_master_port()
+        self.assertEqual(master_addrinfo[4][0], DEFAULT_MASTER)
 
     def test_parse_cmd_params_bad_ip4(self):
         self.args['master'] = '3.3.3.3.3'
-        self.assertRaises(XfrinException, self._do_parse)
+        self.assertRaises(XfrinException, self._do_parse_master_port)
 
     def test_parse_cmd_params_bad_ip6(self):
         self.args['master'] = '1::1::1'
-        self.assertRaises(XfrinException, self._do_parse)
+        self.assertRaises(XfrinException, self._do_parse_master_port)
 
     def test_parse_cmd_params_bad_port(self):
         self.args['port'] = '-1'
-        self.assertRaises(XfrinException, self._do_parse)
+        self.assertRaises(XfrinException, self._do_parse_master_port)
 
         self.args['port'] = '65536'
-        self.assertRaises(XfrinException, self._do_parse)
+        self.assertRaises(XfrinException, self._do_parse_master_port)
 
         self.args['port'] = 'http'
-        self.assertRaises(XfrinException, self._do_parse)
+        self.assertRaises(XfrinException, self._do_parse_master_port)
 
     def test_command_handler_shutdown(self):
         self.assertEqual(self.xfr.command_handler("shutdown",
@@ -529,19 +538,19 @@ class TestXfrin(unittest.TestCase):
         self.assertEqual(self.xfr._max_transfers_in, 3)
 
     def test_command_handler_masters(self):
-        master_info = {'masters': {'address': '1.1.1.1', 'port':53} }
+        master_info = {'master_addr': '1.1.1.1', 'master_port':53}
         self.assertEqual(self.xfr.config_handler(master_info)['result'][0], 0)
 
-        master_info = {'masters': {'address': '1111.1.1.1', 'port':53 } }
+        master_info = {'master_addr': '1111.1.1.1', 'master_port':53 }
         self.assertEqual(self.xfr.config_handler(master_info)['result'][0], 1)
 
-        master_info = {'masters': {'address': '2.2.2.2', 'port':530000 } }
+        master_info = {'master_addr': '2.2.2.2', 'master_port':530000 }
         self.assertEqual(self.xfr.config_handler(master_info)['result'][0], 1)
 
-        master_info = {'masters': {'address': '2.2.2.2', 'port':53 } }
+        master_info = {'master_addr': '2.2.2.2', 'master_port':53 } 
         self.xfr.config_handler(master_info)
-        self.assertEqual(self.xfr._masters['address'], '2.2.2.2')
-        self.assertEqual(self.xfr._masters['port'], 53)
+        self.assertEqual(self.xfr._master_addr, '2.2.2.2')
+        self.assertEqual(self.xfr._master_port, 53)
 
 
 def raise_interrupt():

src/bin/xfrin/xfrin.py.in

@@ -52,6 +52,7 @@ AUTH_SPECFILE_LOCATION = AUTH_SPECFILE_PATH + "/auth.spec"
 
 XFROUT_MODULE_NAME = 'Xfrout'
 ZONE_MANAGER_MODULE_NAME = 'Zonemgr'
+REFRESH_FROM_ZONEMGR = 'refresh_from_zonemgr'
 ZONE_XFRIN_FAILED = 'zone_xfrin_failed'
 __version__ = 'BIND10'
 # define xfrin rcode
@@ -59,6 +60,7 @@ XFRIN_OK = 0
 XFRIN_FAIL = 1
 
 DEFAULT_MASTER_PORT = '53'
+DEFAULT_MASTER = '127.0.0.1'
 
 def log_error(msg):
     sys.stderr.write("[b10-xfrin] %s\n" % str(msg))
@@ -327,7 +329,7 @@ def process_xfrin(server, xfrin_recorder, zone_name, rrclass, db_file,
                            shutdown_event, master_addrinfo, verbose)
     if conn.connect_to_master():
         ret = conn.do_xfrin(check_soa)
-        server.publish_xfrin_news(zone_name, ret)
+        server.publish_xfrin_news(zone_name, rrclass, ret)
 
     xfrin_recorder.decrement(zone_name)
 
@@ -364,7 +366,8 @@ class Xfrin:
     def __init__(self, verbose = False):
         self._max_transfers_in = 10
         #TODO, this is the temp way to set the zone's master.
-        self._masters = {} 
+        self._master_addr = DEFAULT_MASTER
+        self._master_port = DEFAULT_MASTER_PORT
         self._cc_setup()
         self.recorder = XfrinRecorder()
         self._shutdown_event = threading.Event()
@@ -384,7 +387,8 @@ class Xfrin:
         self._module_cc.start()
         config_data = self._module_cc.get_full_config()
         self._max_transfers_in = config_data.get("transfers_in")
-        self._masters = config_data.get('masters')
+        self._master_addr = config_data.get('master_addr') or self._master_addr
+        self._master_port = config_data.get('master_port') or self._master_port
 
     def _cc_check_command(self):
         '''This is a straightforward wrapper for cc.check_command, 
@@ -394,17 +398,17 @@ class Xfrin:
 
     def config_handler(self, new_config):
         self._max_transfers_in = new_config.get("transfers_in") or self._max_transfers_in
-        new_master = new_config.get('masters')
-        if new_master:
+        if ('master_addr' in new_config) or ('master_port' in new_config):
             # Check if the new master is valid, there should be library for check it.
             # and user should change the port and address together.
             try:
-                new_master['address'] = new_master.get('address') or self._masters.get('address')
-                new_master['port'] = new_master.get('port') or self._masters.get('port')
-                check_addr_port(new_master.get('address'), new_master.get('port'))
-                self._masters = new_master
+                addr = new_config.get('master_addr') or self._master_addr
+                port = new_config.get('master_port') or self._master_port
+                check_addr_port(addr, port)
+                self._master_addr = addr
+                self._master_port = port
             except:
-                errmsg = "bad format for zone's master: " + str(new_master)
+                errmsg = "bad format for zone's master: " + str(new_config)
                 log_error(errmsg)
                 return create_answer(1, errmsg)
 
@@ -426,28 +430,41 @@ class Xfrin:
         try:
             if command == 'shutdown':
                 self._shutdown_event.set()
-            elif command == 'notify' or \
-                 command == 'retransfer' or \
-                 command == 'refresh':
-                # Xfrin maybe receives the refresh/notify command from zone manager, or 
-                # the retransfer/refresh from cmdctl(sent by bindctl).
-                # If the command has specified master address, do transfer from the 
-                # master address, or else do transfer from the configured masters.                
+            elif command == 'notify' or command == REFRESH_FROM_ZONEMGR:
+                # Xfrin receives the refresh/notify command from zone manager. 
                 # notify command maybe has the parameters which 
                 # specify the notifyfrom address and port, according the RFC1996, zone
                 # transfer should starts first from the notifyfrom, but now, let 'TODO' it.
-                (zone_name, rrclass, master_addr, db_file) = self._parse_cmd_params(args)
-                ret = self.xfrin_start(zone_name, rrclass, 
-                                       db_file, master_addr,
+                (zone_name, rrclass) = self._parse_zone_name_and_class(args)
+                (master_addr) = check_addr_port(self._master_addr, self._master_port)
+                ret = self.xfrin_start(zone_name, 
+                                       rrclass, 
+                                       self._get_db_file(),
+                                       master_addr,
+                                       True)
+                answer = create_answer(ret[0], ret[1])
+
+            elif command == 'retransfer' or command == 'refresh':
+                # Xfrin receives the retransfer/refresh from cmdctl(sent by bindctl).
+                # If the command has specified master address, do transfer from the 
+                # master address, or else do transfer from the configured masters.                
+                (zone_name, rrclass) = self._parse_zone_name_and_class(args)
+                master_addr = self._parse_master_and_port(args)
+                db_file = args.get('db_file') or self._get_db_file()
+                ret = self.xfrin_start(zone_name, 
+                                       rrclass, 
+                                       db_file, 
+                                       master_addr,
                                        (False if command == 'retransfer' else True))
                 answer = create_answer(ret[0], ret[1])
             else:
                 answer = create_answer(1, 'unknown command: ' + command)
         except XfrinException as err:
+            log_error('error happened for command: %s, %s' % (command, str(err)) )
             answer = create_answer(1, str(err))
         return answer
 
-    def _parse_cmd_params(self, args):
+    def _parse_zone_name_and_class(self, args):
         zone_name = args.get('zone_name')
         if not zone_name:
             raise XfrinException('zone name should be provided')
@@ -460,45 +477,37 @@ class Xfrin:
                 rrclass = RRClass(rrclass)
             except InvalidRRClass as e:
                 raise XfrinException('invalid RRClass: ' + rrclass)
-
-        port_str = args.get('port')
-        if not port_str:
-            port_str = DEFAULT_MASTER_PORT
-
-        master = args.get('master')
-        if not master:
-            if len(self._masters) > 0:
-                master = self._masters.get('address')
-                port_str = self._masters.get('port')
-            else:
-                raise XfrinException("zone's master should be provided")
-
-        master_addrinfo = check_addr_port(master, port_str)
-        db_file = args.get('db_file')
-        if not db_file:
-            #TODO, the db file path should be got in auth server's configuration
-            # if we need access to this configuration more often, we
-            # should add it on start, and not remove it here
-            # (or, if we have writable ds, we might not need this in
-            # the first place)
-            self._module_cc.add_remote_config(AUTH_SPECFILE_LOCATION)
-            db_file, is_default = self._module_cc.get_remote_config_value("Auth", "database_file")
-            if is_default and "B10_FROM_BUILD" in os.environ:
-                # this too should be unnecessary, but currently the
-                # 'from build' override isn't stored in the config
-                # (and we don't have writable datasources yet)
-                db_file = os.environ["B10_FROM_BUILD"] + os.sep + "bind10_zones.sqlite3"
-            self._module_cc.remove_remote_config(AUTH_SPECFILE_LOCATION)
-
-        return (zone_name, rrclass, master_addrinfo, db_file)
-
-    def publish_xfrin_news(self, zone_name, xfr_result):
+        
+        return zone_name, rrclass
+
+    def _parse_master_and_port(self, args):
+        port = args.get('port') or self._master_port
+        master = args.get('master') or self._master_addr
+        return check_addr_port(master, port)
+ 
+    def _get_db_file(self):
+        #TODO, the db file path should be got in auth server's configuration
+        # if we need access to this configuration more often, we
+        # should add it on start, and not remove it here
+        # (or, if we have writable ds, we might not need this in
+        # the first place)
+        self._module_cc.add_remote_config(AUTH_SPECFILE_LOCATION)
+        db_file, is_default = self._module_cc.get_remote_config_value("Auth", "database_file")
+        if is_default and "B10_FROM_BUILD" in os.environ:
+            # this too should be unnecessary, but currently the
+            # 'from build' override isn't stored in the config
+            # (and we don't have writable datasources yet)
+            db_file = os.environ["B10_FROM_BUILD"] + os.sep + "bind10_zones.sqlite3"
+        self._module_cc.remove_remote_config(AUTH_SPECFILE_LOCATION)
+        return db_file
+       
+    def publish_xfrin_news(self, zone_name, zone_class,  xfr_result):
         '''Send command to xfrout/zone manager module.
         If xfrin has finished successfully for one zone, tell the good 
         news(command: zone_new_data_ready) to zone manager and xfrout.
         if xfrin failed, just tell the bad news to zone manager, so that 
         it can reset the refresh timer for that zone. '''
-        param = {'zone_name': zone_name}
+        param = {'zone_name': zone_name, 'zone_class': zone_class.to_text()}
         if xfr_result == XFRIN_OK:
             msg = create_command(notify_out.ZONE_NEW_DATA_READY_CMD, param)
             self._send_cc_session.group_sendmsg(msg, XFROUT_MODULE_NAME)

src/bin/xfrin/xfrin.spec.pre.in

@@ -10,23 +10,16 @@
         "item_default": 10
       },
       {
-        "item_name": "masters",
-        "item_type": "map",
-        "item_optional": true,
-        "item_default": {},
-        "map_item_spec": [
-           { "item_name": "address",
-             "item_type": "string",
-             "item_optional": false,
-             "item_default": ""
-           },
-           { "item_name": "port",
-             "item_type": "integer",
-             "item_optional": false,
-             "item_default": 53
-           }
-         ]
-       }
+        "item_name": "master_addr",
+        "item_type": "string",
+        "item_optional": false,
+        "item_default": ""
+      },
+      { "item_name": "master_port",
+        "item_type": "integer",
+        "item_optional": false,
+        "item_default": 53
+      }
     ],
     "commands": [
      {

src/bin/xfrout/xfrout.py.in

@@ -477,10 +477,10 @@ class XfroutServer:
             self.shutdown()
             answer = create_answer(0)
         
-        elif cmd == notify_out.ZONE_NOTIFY_CMD:
+        elif cmd == notify_out.ZONE_NEW_DATA_READY_CMD:
             zone_name = args.get('zone_name')
             if zone_name:
-                self._log.log_message("info", "Receive notify command for zone " + zone_name)
+                self._log.log_message("info", "Receive notify command for zone: '" + zone_name + "'")
                 self.send_notify(zone_name)
                 answer = create_answer(0)
             else:

src/bin/zonemgr/zonemgr.py.in

@@ -59,7 +59,7 @@ AUTH_MODULE_NAME = 'Auth'
 # define command name
 ZONE_XFRIN_FAILED_COMMAND = 'zone_xfrin_failed'
 ZONE_XFRIN_SUCCESS_COMMAND = 'zone_new_data_ready'
-ZONE_REFRESH_COMMAND = 'refresh'
+ZONE_REFRESH_COMMAND = 'refresh_from_zonemgr'
 ZONE_NOTIFY_COMMAND = 'notify'
 # define zone state
 ZONE_OK = 0