BIND 10 master, updated. e8e45c21b3a6a6c8169259ea5d66d8d475bfbfa6 [master] Update changelog for merge of #2710

BIND 10 source code commits bind10-changes at lists.isc.org
Tue Mar 12 09:52:59 UTC 2013


The branch, master has been updated
       via  e8e45c21b3a6a6c8169259ea5d66d8d475bfbfa6 (commit)
       via  16e8be506f32de668699e6954f5de60ca9d14ddf (commit)
       via  3c243b229dd01343e387ee9a729bce54711c5a50 (commit)
       via  3c1fc036f79481a10628e3f841666c35a0b7b823 (commit)
       via  1090b92f9a37d021c445b046b9245861df6ca36f (commit)
       via  38faa03d1c37053d1c4ffe5e689716d42191a59e (commit)
       via  d27e68b1e1a5f124be2c02e0e6976afaabf0520b (commit)
       via  0a55e2f325a76a08c3feec9911c146bbf9e017a1 (commit)
       via  a2108be0aee261fe1046f96a94543c25937b55a4 (commit)
      from  95218da8e5bfd78889639e7cac68b4afcd44d264 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit e8e45c21b3a6a6c8169259ea5d66d8d475bfbfa6
Author: Jelte Jansen <jelte at isc.org>
Date:   Tue Mar 12 10:52:45 2013 +0100

    [master] Update changelog for merge of #2710

commit 16e8be506f32de668699e6954f5de60ca9d14ddf
Merge: 95218da 3c243b2
Author: Jelte Jansen <jelte at isc.org>
Date:   Tue Mar 12 10:32:03 2013 +0100

    [master] Merge branch 'trac2710'
    
    Conflicts:
    	src/bin/cmdctl/tests/cmdctl_test.py

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog                           |    5 ++
 src/bin/cmdctl/cmdctl.py.in         |   20 +++++--
 src/bin/cmdctl/tests/cmdctl_test.py |   98 +++++++++++++++++++++++++++++++++++
 3 files changed, 120 insertions(+), 3 deletions(-)

-----------------------------------------------------------------------
diff --git a/ChangeLog b/ChangeLog
index 5e5ee1c..b1541bb 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+589.	[bug]		jelte
+	b10-cmdctl now automatically re-reads the user accounts file when
+	it is updated.
+	(Trac #2710, git 16e8be506f32de668699e6954f5de60ca9d14ddf)
+
 588.	[bug]*		jreed
 	b10-xfrout: Log message id XFROUT_QUERY_QUOTA_EXCCEEDED
 	changed to XFROUT_QUERY_QUOTA_EXCEEDED.
diff --git a/src/bin/cmdctl/cmdctl.py.in b/src/bin/cmdctl/cmdctl.py.in
index f9670cf..bca0cd9 100755
--- a/src/bin/cmdctl/cmdctl.py.in
+++ b/src/bin/cmdctl/cmdctl.py.in
@@ -507,12 +507,25 @@ class SecureHTTPServer(socketserver_mixin.NoPollMixIn,
         self._verbose = verbose
         self._lock = threading.Lock()
         self._user_infos = {}
-        self._accounts_file = None
+        self.__accounts_file = None
+        self.__accounts_file_mtime = 0
 
     def _create_user_info(self, accounts_file):
         '''Read all user's name and its' salt, hashed password
         from accounts file.'''
-        if (self._accounts_file == accounts_file) and (len(self._user_infos) > 0):
+
+        # If the file does not exist, set accounts to empty, and return
+        if not os.path.exists(accounts_file):
+            self._user_infos = {}
+            self.__accounts_file = None
+            self.__accounts_file_mtime = 0
+            return
+
+        # If the filename hasn't changed, and the file itself
+        # has neither, do nothing
+        accounts_file_mtime = os.stat(accounts_file).st_mtime
+        if self.__accounts_file == accounts_file and\
+           accounts_file_mtime <= self.__accounts_file_mtime:
             return
 
         with self._lock:
@@ -530,7 +543,8 @@ class SecureHTTPServer(socketserver_mixin.NoPollMixIn,
                 if csvfile:
                     csvfile.close()
 
-        self._accounts_file = accounts_file
+        self.__accounts_file = accounts_file
+        self.__accounts_file_mtime = accounts_file_mtime
         if len(self._user_infos) == 0:
             logger.error(CMDCTL_NO_USER_ENTRIES_READ)
 
diff --git a/src/bin/cmdctl/tests/cmdctl_test.py b/src/bin/cmdctl/tests/cmdctl_test.py
index 82042fe..16d76c2 100644
--- a/src/bin/cmdctl/tests/cmdctl_test.py
+++ b/src/bin/cmdctl/tests/cmdctl_test.py
@@ -17,6 +17,7 @@
 import unittest
 import socket
 import tempfile
+import time
 import stat
 import sys
 from cmdctl import *
@@ -71,6 +72,26 @@ class UnreadableFile:
     def __exit__(self, type, value, traceback):
         os.chmod(self.file_name, self.orig_mode)
 
+class TmpTextFile:
+    """
+    Context class for temporarily creating a text file with some
+    lines of content.
+
+    The file is automatically deleted if the context is left, so
+    make sure to not use the path of an existing file!
+    """
+    def __init__(self, path, contents):
+        self.__path = path
+        self.__contents = contents
+
+    def __enter__(self):
+        with open(self.__path, 'w') as f:
+            f.write("\n".join(self.__contents) + "\n")
+
+    def __exit__(self, type, value, traceback):
+        os.unlink(self.__path)
+
+
 class TestSecureHTTPRequestHandler(unittest.TestCase):
     def setUp(self):
         self.old_stdout = sys.stdout
@@ -534,6 +555,83 @@ class TestSecureHTTPServer(unittest.TestCase):
         self.assertIn('6f0c73bd33101a5ec0294b3ca39fec90ef4717fe',
                       self.server.get_user_info('root'))
 
+        # When the file is not changed calling _create_user_info() again
+        # should have no effect. In order to test this, we overwrite the
+        # user-infos that were just set and make sure it isn't touched by
+        # the call (so make sure it isn't set to some empty value)
+        fake_users_val = { 'notinfile': [] }
+        self.server._user_infos = fake_users_val
+        self.server._create_user_info(SRC_FILE_PATH + 'cmdctl-accounts.csv')
+        self.assertEqual(fake_users_val, self.server._user_infos)
+
+    def test_create_user_info_changing_file_time(self):
+        self.assertEqual(0, len(self.server._user_infos))
+
+        # Create a file
+        accounts_file = BUILD_FILE_PATH + 'new_file.csv'
+        with TmpTextFile(accounts_file, ['root,foo,bar']):
+            self.server._create_user_info(accounts_file)
+            self.assertEqual(1, len(self.server._user_infos))
+            self.assertTrue('root' in self.server._user_infos)
+
+            # Make sure re-reading is a noop if file was not modified
+            fake_users_val = { 'notinfile': [] }
+            self.server._user_infos = fake_users_val
+            self.server._create_user_info(accounts_file)
+            self.assertEqual(fake_users_val, self.server._user_infos)
+
+        # Yes sleep sucks, but in this case we need it to check for
+        # a changed mtime, not for some thread to do its work
+        # (do we run these tests on systems with 1+ secs mtimes?)
+        time.sleep(0.1)
+        # create the file again, this time read should not be a noop
+        with TmpTextFile(accounts_file, ['otherroot,foo,bar']):
+            self.server._create_user_info(accounts_file)
+            self.assertEqual(1, len(self.server._user_infos))
+            self.assertTrue('otherroot' in self.server._user_infos)
+
+    def test_create_user_info_changing_file_name(self):
+        """
+        Check that the accounts file is re-read if the file name is different
+        """
+        self.assertEqual(0, len(self.server._user_infos))
+
+        # Create two files
+        accounts_file1 = BUILD_FILE_PATH + 'new_file.csv'
+        accounts_file2 = BUILD_FILE_PATH + 'new_file2.csv'
+        with TmpTextFile(accounts_file2, ['otherroot,foo,bar']):
+            with TmpTextFile(accounts_file1, ['root,foo,bar']):
+                self.server._create_user_info(accounts_file1)
+                self.assertEqual(1, len(self.server._user_infos))
+                self.assertTrue('root' in self.server._user_infos)
+
+                # Make sure re-reading is a noop if file was not modified
+                fake_users_val = { 'notinfile': [] }
+                self.server._user_infos = fake_users_val
+                self.server._create_user_info(accounts_file1)
+                self.assertEqual(fake_users_val, self.server._user_infos)
+
+                # But a different file should be read
+                self.server._create_user_info(accounts_file2)
+                self.assertEqual(1, len(self.server._user_infos))
+                self.assertTrue('otherroot' in self.server._user_infos)
+
+    def test_create_user_info_nonexistent_file(self):
+        # Even if there was data initially, if set to a nonexistent
+        # file it should result in no users
+        accounts_file = BUILD_FILE_PATH + 'new_file.csv'
+        self.assertFalse(os.path.exists(accounts_file))
+        fake_users_val = { 'notinfile': [] }
+        self.server._user_infos = fake_users_val
+        self.server._create_user_info(accounts_file)
+        self.assertEqual({}, self.server._user_infos)
+
+        # Should it now be created it should be read
+        with TmpTextFile(accounts_file, ['root,foo,bar']):
+            self.server._create_user_info(accounts_file)
+            self.assertEqual(1, len(self.server._user_infos))
+            self.assertTrue('root' in self.server._user_infos)
+
     def test_check_file(self):
         # Just some file that we know exists
         file_name = BUILD_FILE_PATH + 'cmdctl-keyfile.pem'



More information about the bind10-changes mailing list