Commit 87c80992 authored by Michael Hanselmann's avatar Michael Hanselmann
Browse files

cfgupgrade: Fix critical bug overwriting RAPI users file



The cfgupgrade tool was designed to be idempotent, that means it could
be run several times and still give produce the correct result. Ganeti
2.4 moved the file containing the RAPI users to a separate directory
(…/lib/ganeti/rapi/users). If it exists, cfgupgrade would automatically
move an existing file from …/lib/ganeti/rapi_users and replace it with a
symlink.

Unfortunately one of the checks for this was incorrect and, when run
multiple times, replaces the users file at the new location with a
symlink created during a previous run.

In addition the “--dry-run” parameter to cfgupgrade was not respected.
Unittests are updated for all these cases.
Signed-off-by: default avatarMichael Hanselmann <hansmi@google.com>
Reviewed-by: default avatarIustin Pop <iustin@google.com>
parent 20203756
......@@ -161,12 +161,16 @@ class TestCfgupgrade(unittest.TestCase):
def testRapiUsers(self):
self.assertFalse(os.path.exists(self.rapi_users_path))
self.assertFalse(os.path.exists(self.rapi_users_path_pre24))
self.assertFalse(os.path.exists(os.path.dirname(self.rapi_users_path)))
utils.WriteFile(self.rapi_users_path_pre24, data="some user\n")
self._TestSimpleUpgrade(constants.BuildVersion(2, 3, 0), False)
self.assertTrue(os.path.isdir(os.path.dirname(self.rapi_users_path)))
self.assert_(os.path.islink(self.rapi_users_path_pre24))
self.assert_(os.path.isfile(self.rapi_users_path))
self.assertEqual(os.readlink(self.rapi_users_path_pre24),
self.rapi_users_path)
for path in [self.rapi_users_path, self.rapi_users_path_pre24]:
self.assertEqual(utils.ReadFile(path), "some user\n")
......@@ -180,6 +184,8 @@ class TestCfgupgrade(unittest.TestCase):
self.assert_(os.path.islink(self.rapi_users_path_pre24))
self.assert_(os.path.isfile(self.rapi_users_path))
self.assertEqual(os.readlink(self.rapi_users_path_pre24),
self.rapi_users_path)
for path in [self.rapi_users_path, self.rapi_users_path_pre24]:
self.assertEqual(utils.ReadFile(path), "other user\n")
......@@ -187,13 +193,77 @@ class TestCfgupgrade(unittest.TestCase):
self.assertFalse(os.path.exists(self.rapi_users_path))
self.assertFalse(os.path.exists(self.rapi_users_path_pre24))
os.mkdir(os.path.dirname(self.rapi_users_path))
os.symlink(self.rapi_users_path, self.rapi_users_path_pre24)
utils.WriteFile(self.rapi_users_path_pre24, data="hello world\n")
utils.WriteFile(self.rapi_users_path, data="hello world\n")
self._TestSimpleUpgrade(constants.BuildVersion(2, 2, 0), False)
self.assert_(os.path.isfile(self.rapi_users_path))
self.assert_(os.path.isfile(self.rapi_users_path) and
not os.path.islink(self.rapi_users_path))
self.assert_(os.path.islink(self.rapi_users_path_pre24))
self.assertEqual(os.readlink(self.rapi_users_path_pre24),
self.rapi_users_path)
for path in [self.rapi_users_path, self.rapi_users_path_pre24]:
self.assertEqual(utils.ReadFile(path), "hello world\n")
def testRapiUsersExistingTarget(self):
self.assertFalse(os.path.exists(self.rapi_users_path))
self.assertFalse(os.path.exists(self.rapi_users_path_pre24))
os.mkdir(os.path.dirname(self.rapi_users_path))
utils.WriteFile(self.rapi_users_path, data="other user\n")
utils.WriteFile(self.rapi_users_path_pre24, data="hello world\n")
self.assertRaises(Exception, self._TestSimpleUpgrade,
constants.BuildVersion(2, 2, 0), False)
for path in [self.rapi_users_path, self.rapi_users_path_pre24]:
self.assert_(os.path.isfile(path) and not os.path.islink(path))
self.assertEqual(utils.ReadFile(self.rapi_users_path), "other user\n")
self.assertEqual(utils.ReadFile(self.rapi_users_path_pre24),
"hello world\n")
def testRapiUsersDryRun(self):
self.assertFalse(os.path.exists(self.rapi_users_path))
self.assertFalse(os.path.exists(self.rapi_users_path_pre24))
utils.WriteFile(self.rapi_users_path_pre24, data="some user\n")
self._TestSimpleUpgrade(constants.BuildVersion(2, 3, 0), True)
self.assertFalse(os.path.isdir(os.path.dirname(self.rapi_users_path)))
self.assertTrue(os.path.isfile(self.rapi_users_path_pre24) and
not os.path.islink(self.rapi_users_path_pre24))
self.assertFalse(os.path.exists(self.rapi_users_path))
def testRapiUsers24AndAboveDryRun(self):
self.assertFalse(os.path.exists(self.rapi_users_path))
self.assertFalse(os.path.exists(self.rapi_users_path_pre24))
os.mkdir(os.path.dirname(self.rapi_users_path))
utils.WriteFile(self.rapi_users_path, data="other user\n")
self._TestSimpleUpgrade(constants.BuildVersion(2, 3, 0), True)
self.assertTrue(os.path.isfile(self.rapi_users_path) and
not os.path.islink(self.rapi_users_path))
self.assertFalse(os.path.exists(self.rapi_users_path_pre24))
self.assertEqual(utils.ReadFile(self.rapi_users_path), "other user\n")
def testRapiUsersExistingSymlinkDryRun(self):
self.assertFalse(os.path.exists(self.rapi_users_path))
self.assertFalse(os.path.exists(self.rapi_users_path_pre24))
os.mkdir(os.path.dirname(self.rapi_users_path))
os.symlink(self.rapi_users_path, self.rapi_users_path_pre24)
utils.WriteFile(self.rapi_users_path, data="hello world\n")
self._TestSimpleUpgrade(constants.BuildVersion(2, 2, 0), True)
self.assertTrue(os.path.islink(self.rapi_users_path_pre24))
self.assertTrue(os.path.isfile(self.rapi_users_path) and
not os.path.islink(self.rapi_users_path))
self.assertEqual(os.readlink(self.rapi_users_path_pre24),
self.rapi_users_path)
for path in [self.rapi_users_path, self.rapi_users_path_pre24]:
self.assertEqual(utils.ReadFile(path), "hello world\n")
......
......@@ -185,16 +185,25 @@ def main():
raise Error("Configuration version %d.%d.%d not supported by this tool" %
(config_major, config_minor, config_revision))
if os.path.isfile(options.RAPI_USERS_FILE_PRE24):
if (os.path.isfile(options.RAPI_USERS_FILE_PRE24) and
not os.path.islink(options.RAPI_USERS_FILE_PRE24)):
if os.path.exists(options.RAPI_USERS_FILE):
raise Error("Found pre-2.4 RAPI users file at %s, but another file"
" already exists at %s" %
(options.RAPI_USERS_FILE_PRE24, options.RAPI_USERS_FILE))
logging.info("Found pre-2.4 RAPI users file at %s, renaming to %s",
options.RAPI_USERS_FILE_PRE24, options.RAPI_USERS_FILE)
if not options.dry_run:
utils.RenameFile(options.RAPI_USERS_FILE_PRE24, options.RAPI_USERS_FILE,
mkdir=True, mkdir_mode=0750)
# Create a symlink for RAPI users file
if not os.path.islink(options.RAPI_USERS_FILE_PRE24):
if (not (os.path.islink(options.RAPI_USERS_FILE_PRE24) or
os.path.isfile(options.RAPI_USERS_FILE_PRE24)) and
os.path.isfile(options.RAPI_USERS_FILE)):
logging.info("Creating symlink from %s to %s",
options.RAPI_USERS_FILE_PRE24, options.RAPI_USERS_FILE)
if not options.dry_run:
os.symlink(options.RAPI_USERS_FILE, options.RAPI_USERS_FILE_PRE24)
try:
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment