[svn] commit: r2305 - in /branches/trac127/src/bin: bindctl/bindcmd.py cmdctl/TODO cmdctl/cmdctl.py.in cmdctl/tests/cmdctl_test.py

BIND 10 source code commits bind10-changes at lists.isc.org
Mon Jun 28 09:46:52 UTC 2010


Author: zhanglikun
Date: Mon Jun 28 09:46:52 2010
New Revision: 2305

Log:
Update the code according Jelte's review. Fix some hardcode string in code. Raise correct exception if key/certificate file doesn't exist. Fix docstring error. 

Modified:
    branches/trac127/src/bin/bindctl/bindcmd.py
    branches/trac127/src/bin/cmdctl/TODO
    branches/trac127/src/bin/cmdctl/cmdctl.py.in
    branches/trac127/src/bin/cmdctl/tests/cmdctl_test.py

Modified: branches/trac127/src/bin/bindctl/bindcmd.py
==============================================================================
--- branches/trac127/src/bin/bindctl/bindcmd.py (original)
+++ branches/trac127/src/bin/bindctl/bindcmd.py Mon Jun 28 09:46:52 2010
@@ -108,7 +108,7 @@
         return digest
     
     def run(self):
-        '''Parse commands inputted from user and send them to cmdctl. '''
+        '''Parse commands from user and send them to cmdctl. '''
         try:
             if not self.login_to_cmdctl():
                 return False

Modified: branches/trac127/src/bin/cmdctl/TODO
==============================================================================
--- branches/trac127/src/bin/cmdctl/TODO (original)
+++ branches/trac127/src/bin/cmdctl/TODO Mon Jun 28 09:46:52 2010
@@ -2,4 +2,5 @@
 . Update man page for b10-cmdctl?
 . Add check for the content of key/certificate file
   (when cmdctl starts or is configured by bindctl).
+. Use only one msgq/session to communicate with other modules?
 

Modified: branches/trac127/src/bin/cmdctl/cmdctl.py.in
==============================================================================
--- branches/trac127/src/bin/cmdctl/cmdctl.py.in (original)
+++ branches/trac127/src/bin/cmdctl/cmdctl.py.in Mon Jun 28 09:46:52 2010
@@ -51,7 +51,9 @@
 
 __version__ = 'BIND10'
 URL_PATTERN = re.compile('/([\w]+)(?:/([\w]+))?/?')
-MODULE_NAME = 'Cmdctl'
+CONFIG_DATA_URL = 'config_data'
+MODULE_SPEC_URL = 'module_spec'
+
 
 # If B10_FROM_SOURCE is set in the environment, we use data files
 # from a directory relative to that, otherwise we use the ones
@@ -63,6 +65,9 @@
     DATAROOTDIR = "@datarootdir@"
     SPECFILE_PATH = "@datadir@/@PACKAGE@".replace("${datarootdir}", DATAROOTDIR).replace("${prefix}", PREFIX)
 SPECFILE_LOCATION = SPECFILE_PATH + os.sep + "cmdctl.spec"
+
+class CmdctlException(Exception):
+    pass
        
 class SecureHTTPRequestHandler(http.server.BaseHTTPRequestHandler):
     '''https connection request handler.
@@ -206,8 +211,8 @@
     receive command from client and resend it to proper module.
     '''
     def __init__(self, httpserver, verbose = False):
-        '''When cmdctl get 'shutdown' command from boss, 
-        shutdown the outer httpserver. '''
+        ''' httpserver: the http server which use the object of
+        CommandControl to communicate with other modules. '''
         self._verbose = verbose
         self._httpserver = httpserver
         self._lock = threading.Lock()
@@ -228,6 +233,7 @@
         self._module_cc = isc.config.ModuleCCSession(SPECFILE_LOCATION,
                                               self.config_handler,
                                               self.command_handler)
+        self._module_name = self._module_cc.get_module_spec().get_module_name()
         self._cmdctl_config_data = self._module_cc.get_full_config()
         self._module_cc.start()
     
@@ -293,6 +299,8 @@
                 self.modules_spec[args[0]] = args[1]
 
         elif command == ccsession.COMMAND_SHUTDOWN:
+            #When cmdctl get 'shutdown' command from boss, 
+            #shutdown the outer httpserver.
             self._httpserver.shutdown()
             self._serving = False
 
@@ -375,7 +383,7 @@
         if self._verbose:
             self.log_info("Begin send command '%s' to module '%s'" %(command_name, module_name))
 
-        if module_name == MODULE_NAME:
+        if module_name == self._module_name:
             # Process the command sent to cmdctl directly. 
             answer = self.command_handler(command_name, params)
         else:
@@ -481,24 +489,22 @@
     def _check_key_and_cert(self, key, cert):
         # TODO, check the content of key/certificate file 
         if not os.path.exists(key):
-            self.log_info("Deny client's connection since key file doesn't exist " + key)
-            raise socket.error
+            raise CmdctlException("key file '%s' doesn't exist " % key)
 
         if not os.path.exists(cert):
-            self.log_info("Deny client's connection since certificate file doesn't exist " + cert)
-            raise socket.error
+            raise CmdctlException("certificate file '%s' doesn't exist " % cert)
 
     def _wrap_socket_in_ssl_context(self, sock, key, cert):
-        self._check_key_and_cert(key, cert)
         try:
+            self._check_key_and_cert(key, cert)
             ssl_sock = ssl.wrap_socket(sock,
                                       server_side = True,
                                       certfile = cert,
                                       keyfile = key,
                                       ssl_version = ssl.PROTOCOL_SSLv23)
             return ssl_sock 
-        except ssl.SSLError as e :
-            self.log_info("Deny client's connection:%s\n" % e)
+        except (ssl.SSLError, CmdctlException) as err :
+            self.log_info("Deny client's connection because %s" % str(err))
             self.close_request(sock)
             # raise socket error to finish the request
             raise socket.error
@@ -515,9 +521,9 @@
         '''Currently only support the following three url GET request '''
         rcode, reply = http.client.NO_CONTENT, []        
         if not module:
-            if id == 'config_data':
+            if id == CONFIG_DATA_URL:
                 rcode, reply = http.client.OK, self.cmdctl.get_config_data()
-            elif id == 'module_spec':
+            elif id == MODULE_SPEC_URL:
                 rcode, reply = http.client.OK, self.cmdctl.get_modules_spec()
         
         return rcode, reply 

Modified: branches/trac127/src/bin/cmdctl/tests/cmdctl_test.py
==============================================================================
--- branches/trac127/src/bin/cmdctl/tests/cmdctl_test.py (original)
+++ branches/trac127/src/bin/cmdctl/tests/cmdctl_test.py Mon Jun 28 09:46:52 2010
@@ -157,7 +157,7 @@
     def test_check_user_name_and_pwd(self):
         self.handler.headers = {}
         ret, msg = self.handler._check_user_name_and_pwd()
-        self.assertTrue(ret == False)
+        self.assertFalse(ret)
         self.assertEqual(msg, ['invalid username or password'])
 
     def test_check_user_name_and_pwd_1(self):
@@ -168,7 +168,7 @@
 
         self.handler.server._user_infos['root'] = ['aa', 'aaa']
         ret, msg = self.handler._check_user_name_and_pwd()
-        self.assertTrue(ret == False)
+        self.assertFalse(ret)
         self.assertEqual(msg, ['password doesn\'t match'])
 
     def test_check_user_name_and_pwd_2(self):
@@ -178,7 +178,7 @@
         self.handler.rfile.seek(0, 0)
 
         ret, msg = self.handler._check_user_name_and_pwd()
-        self.assertTrue(ret == False)
+        self.assertFalse(ret)
         self.assertEqual(msg, ['invalid username or password'])
 
     def test_check_user_name_and_pwd_3(self):
@@ -188,7 +188,7 @@
         self.handler.rfile.seek(0, 0)
 
         ret, msg = self.handler._check_user_name_and_pwd()
-        self.assertTrue(ret == False)
+        self.assertFalse(ret)
         self.assertEqual(msg, ['need user name'])
 
     def test_check_user_name_and_pwd_4(self):
@@ -199,7 +199,7 @@
 
         self.handler.server._user_infos['root'] = ['aa', 'aaa']
         ret, msg = self.handler._check_user_name_and_pwd()
-        self.assertTrue(ret == False)
+        self.assertFalse(ret)
         self.assertEqual(msg, ['need password'])
 
     def test_check_user_name_and_pwd_5(self):
@@ -209,7 +209,7 @@
         self.handler.rfile.seek(0, 0)
 
         ret, msg = self.handler._check_user_name_and_pwd()
-        self.assertTrue(ret == False)
+        self.assertFalse(ret)
         self.assertEqual(msg, ['user doesn\'t exist'])
 
     def test_do_POST(self):
@@ -287,6 +287,7 @@
         spec_file = FILE_PATH + 'cmdctl.spec'
         module_spec = isc.config.module_spec_from_file(spec_file)
         config = isc.config.config_data.ConfigData(module_spec)
+        self._module_name = 'Cmdctl'
         self._cmdctl_config_data = config.get_full_config()
 
     def _handle_msg_from_msgq(self): 
@@ -369,7 +370,7 @@
         os.remove(file_name)
     
     def test_send_command(self):
-        rcode, value = self.cmdctl.send_command(MODULE_NAME, 'print_settings', None)
+        rcode, value = self.cmdctl.send_command('Cmdctl', 'print_settings', None)
         self.assertEqual(rcode, 0)
 
 class MySecureHTTPServer(SecureHTTPServer):
@@ -395,6 +396,13 @@
         self.assertEqual(1, len(self.server._user_infos))
         self.assertTrue('root' in self.server._user_infos)
 
+    def test_check_key_and_cert(self):
+        self.assertRaises(CmdctlException, self.server._check_key_and_cert,
+                         '/local/not-exist', 'cmdctl-keyfile.pem')
+
+        self.server._check_key_and_cert(FILE_PATH + 'cmdctl-keyfile.pem',
+                                        FILE_PATH + 'cmdctl-certfile.pem')
+
     def test_wrap_sock_in_ssl_context(self):
         sock = socket.socket()
         self.assertRaises(socket.error, 
@@ -403,7 +411,8 @@
                           '../cmdctl-keyfile',
                           '../cmdctl-certfile')
 
-        self.server._wrap_socket_in_ssl_context(sock, 
+        sock1 = socket.socket()
+        self.server._wrap_socket_in_ssl_context(sock1, 
                           FILE_PATH + 'cmdctl-keyfile.pem',
                           FILE_PATH + 'cmdctl-certfile.pem')
 




More information about the bind10-changes mailing list