[blivet:rhel7/master 1/4] Make crypt tests runnable.
David Lehman
dlehman at redhat.com
Mon Mar 24 23:14:58 UTC 2014
On Mon, 2014-03-24 at 19:08 -0400, Anne Mulhern wrote:
> What I would like to do for this and the subsequent patch in
> this patch set is:
>
> 1) Commit the working tests.
> 2) Put in a FIXME comment in the test file indicating all the things that
> we believe need to be fixed, as thoroughly as we can.
> 3) Fix all the listed things in due course.
>
> I think that it is important to get to the point where our tests are
> actually running and are expected to pass, and to not allow the fact that
> some of the code does not work as we believe that it should to result
> in an indefinite delay.
None of the things mentioned as being broken are critical, so I'd be
fine with a single bug to track the followups and a comment in the test
code.
David
>
> - mulhern
>
> ----- Original Message -----
> > From: "David Lehman" <dlehman at redhat.com>
> > To: "anaconda patch review" <anaconda-patches at lists.fedorahosted.org>
> > Sent: Monday, March 24, 2014 2:34:35 PM
> > Subject: Re: [blivet:rhel7/master 1/4] Make crypt tests runnable.
> >
> > On Fri, 2014-03-21 at 15:29 -0400, mulhern wrote:
> > > If any of these tests seem wrong then the code can be fixed up and the test
> > > changed.
> > >
> > > A tearDown method was added to close the device in case a test failed.
> > >
> > > Some instance fields were used for the names of the devices.
> > >
> > > Some aspects of the code that brought about these changes are:
> > > * All crypto.luks_* methods raise a ValueError if they are not given a
> > > passphrase although they accept a key_file argument.
> >
> > This should only be the case when flags.installer_mode is True.
> >
> > > * If a device does not exist, CryptSetup raises an IOError.
> > >
> > > Some methods that did suprising things were.
> > > * is_luks --- it raises an IOError for an unknown device
> >
> > I'm not sure what's the best way to handle this. Return False or raise
> > an exception? If exception, which one? If we were checking it ourselves
> > we could raise ValueError instead of IOError.
> >
> > > * luks_format --- raises a ValueError if it gets a keyfile instead of a
> > > passphrase
> >
> > That's a blivet bug -- it is assuming installer mode is enabled.
> >
> > > * luks_remove_key --- raises a CryptoError instead of a RuntimeError
> > > * luks_status --- returns ints, not booleans
> >
> > It would be nice to fix this in cryptsetup-python, but we could fix it
> > in blivet if necessary.
> >
> > >
> > > Signed-off-by: mulhern <amulhern at redhat.com>
> > > ---
> > > tests/devicelibs_test/crypto_test.py | 140
> > > +++++++++++++++++++++++++----------
> > > 1 file changed, 100 insertions(+), 40 deletions(-)
> > >
> > > diff --git a/tests/devicelibs_test/crypto_test.py
> > > b/tests/devicelibs_test/crypto_test.py
> > > index 7ef49fe..5b33a07 100755
> > > --- a/tests/devicelibs_test/crypto_test.py
> > > +++ b/tests/devicelibs_test/crypto_test.py
> > > @@ -8,7 +8,7 @@ import os
> > > class CryptoTestCase(baseclass.DevicelibsTestCase):
> > >
> > > @unittest.skipUnless(os.geteuid() == 0, "requires root privileges")
> > > - def testCrypto(self):
> > > + def testCryptoMisc(self):
> > > _LOOP_DEV0 = self._loopMap[self._LOOP_DEVICES[0]]
> > > _LOOP_DEV1 = self._loopMap[self._LOOP_DEVICES[1]]
> > >
> > > @@ -19,13 +19,17 @@ class CryptoTestCase(baseclass.DevicelibsTestCase):
> > > ##
> > > # pass
> > > self.assertEqual(crypto.is_luks(_LOOP_DEV0), -22)
> > > - self.assertEqual(crypto.is_luks("/not/existing/device"), -22)
> > > + self.assertRaisesRegexp(IOError,
> > > + "Device cannot be opened",
> > > + crypto.is_luks,
> > > + "/not/existing/device")
> > >
> > > ##
> > > ## luks_format
> > > ##
> > > # pass
> > > self.assertEqual(crypto.luks_format(_LOOP_DEV0,
> > > passphrase="secret", cipher="aes-cbc-essiv:sha256",
> > > key_size=256), None)
> > > + self.assertEqual(crypto.is_luks(_LOOP_DEV0), 0)
> > >
> > > # make a key file
> > > handle, keyfile = tempfile.mkstemp(prefix="key", text=False)
> > > @@ -33,19 +37,29 @@ class CryptoTestCase(baseclass.DevicelibsTestCase):
> > > os.close(handle)
> > >
> > > # format with key file
> > > - self.assertEqual(crypto.luks_format(_LOOP_DEV1, key_file=keyfile),
> > > None)
> > > + self.assertRaisesRegexp(ValueError,
> > > + "requires passphrase",
> > > + crypto.luks_format,
> > > + _LOOP_DEV1, key_file=keyfile)
> > >
> > > # fail
> > > - self.assertRaises(crypto.CryptoError, crypto.luks_format,
> > > "/not/existing/device", passphrase="secret",
> > > cipher="aes-cbc-essiv:sha256", key_size=256)
> > > + self.assertRaisesRegexp(IOError,
> > > + "Device cannot be opened",
> > > + crypto.luks_format,
> > > + "/not/existing/device", passphrase="secret",
> > > cipher="aes-cbc-essiv:sha256", key_size=256)
> > > +
> > > # no passhprase or key file
> > > - self.assertRaises(ValueError, crypto.luks_format, _LOOP_DEV1,
> > > cipher="aes-cbc-essiv:sha256", key_size=256)
> > > + self.assertRaisesRegexp(ValueError,
> > > + "requires passphrase",
> > > + crypto.luks_format,
> > > + _LOOP_DEV1, cipher="aes-cbc-essiv:sha256", key_size=256)
> > >
> > > ##
> > > ## is_luks
> > > ##
> > > # pass
> > > self.assertEqual(crypto.is_luks(_LOOP_DEV0), 0) # 0 = is luks
> > > - self.assertEqual(crypto.is_luks(_LOOP_DEV1), 0)
> > > + self.assertEqual(crypto.is_luks(_LOOP_DEV1), -22)
> > >
> > > ##
> > > ## luks_add_key
> > > @@ -53,49 +67,96 @@ class CryptoTestCase(baseclass.DevicelibsTestCase):
> > > # pass
> > > self.assertEqual(crypto.luks_add_key(_LOOP_DEV0,
> > > new_passphrase="another-secret", passphrase="secret"), None)
> > >
> > > - # make another key file
> > > - handle, new_keyfile = tempfile.mkstemp(prefix="key", text=False)
> > > - os.write(handle, "area51")
> > > - os.close(handle)
> > > -
> > > - # add new key file
> > > - self.assertEqual(crypto.luks_add_key(_LOOP_DEV1,
> > > new_key_file=new_keyfile, key_file=keyfile), None)
> > > -
> > > # fail
> > > - self.assertRaises(crypto.CryptoError, crypto.luks_add_key,
> > > _LOOP_DEV0, new_passphrase="another-secret",
> > > passphrase="wrong-passphrase")
> > > + self.assertRaisesRegexp(crypto.CryptoError,
> > > + "luks add key failed",
> > > + crypto.luks_add_key,
> > > + _LOOP_DEV0, new_passphrase="another-secret",
> > > passphrase="wrong-passphrase")
> > >
> > > ##
> > > ## luks_remove_key
> > > ##
> > > # fail
> > > - self.assertRaises(RuntimeError, crypto.luks_remove_key,
> > > _LOOP_DEV0, del_passphrase="another-secret", passphrase="wrong-pasphrase")
> > > + self.assertRaisesRegexp(crypto.CryptoError,
> > > + "luks remove key failed",
> > > + crypto.luks_remove_key,
> > > + _LOOP_DEV0, del_passphrase="another-secret",
> > > passphrase="wrong-pasphrase")
> > >
> > > # pass
> > > self.assertEqual(crypto.luks_remove_key(_LOOP_DEV0,
> > > del_passphrase="another-secret", passphrase="secret"), None)
> > >
> > > - # remove key file
> > > - self.assertEqual(crypto.luks_remove_key(LOOP_DEV1,
> > > del_key_file=new_keyfile, key_file=keyfile), None)
> > > + # fail
> > > + self.assertRaisesRegexp(IOError,
> > > + "Device cannot be opened",
> > > + crypto.luks_open,
> > > + "/not/existing/device", "another-crypted", passphrase="secret")
> > > +
> > > + # no passhprase or key file
> > > + self.assertRaisesRegexp(ValueError,
> > > + "luks_format requires passphrase",
> > > + crypto.luks_open,
> > > + _LOOP_DEV1, "another-crypted")
> > > +
> > > + self.assertRaisesRegexp(IOError,
> > > + "Device cannot be opened",
> > > + crypto.luks_status,
> > > + "another-crypted")
> > > + self.assertRaisesRegexp(IOError,
> > > + "Device cannot be opened",
> > > + crypto.luks_close,
> > > + "wrong-name")
> > > +
> > > + # cleanup
> > > + os.unlink(keyfile)
> > > +
> > > +class CryptoTestCase2(baseclass.DevicelibsTestCase):
> > > +
> > > + def __init__(self, *args, **kwargs):
> > > + """Set up the names by which luks knows these devices."""
> > > + super(CryptoTestCase2, self).__init__(*args, **kwargs)
> > > + self._names = { self._LOOP_DEVICES[0]: "crypted",
> > > + self._LOOP_DEVICES[1]: "encrypted" }
> > > +
> > > + def tearDown(self):
> > > + """Close all devices just in case they are still open."""
> > > + import blivet.devicelibs.crypto as crypto
> > > + for name in self._names.values():
> > > + try:
> > > + crypto.luks_close(name)
> > > + except (IOError, crypto.CryptoError):
> > > + pass
> > > + super(CryptoTestCase2, self).tearDown()
> > > +
> > > + @unittest.skipUnless(os.geteuid() == 0, "requires root privileges")
> > > + def testCryptoOpen(self):
> > > + _LOOP_DEV0 = self._loopMap[self._LOOP_DEVICES[0]]
> > > + _LOOP_DEV1 = self._loopMap[self._LOOP_DEVICES[1]]
> > > +
> > > + _name0 = self._names[self._LOOP_DEVICES[0]]
> > > + _name1 = self._names[self._LOOP_DEVICES[1]]
> > > +
> > > + import blivet.devicelibs.crypto as crypto
> > >
> > > ##
> > > - ## luks_open
> > > + ## luks_format
> > > ##
> > > # pass
> > > - self.assertEqual(crypto.luks_open(_LOOP_DEV0, "crypted",
> > > passphrase="secret"), None)
> > > - self.assertEqual(crypto.luks_open(_LOOP_DEV1, "encrypted",
> > > key_file=keyfile), None)
> > > + self.assertEqual(crypto.luks_format(_LOOP_DEV0,
> > > passphrase="secret", cipher="aes-cbc-essiv:sha256", key_size=256), None)
> > > + self.assertEqual(crypto.luks_format(_LOOP_DEV1,
> > > passphrase="hidden", cipher="aes-cbc-essiv:sha256", key_size=256), None)
> > >
> > > - # fail
> > > - self.assertRaises(crypto.CryptoError, crypto.luks_open,
> > > "/not/existing/device", "another-crypted", passphrase="secret")
> > > - self.assertRaises(crypto.CryptoError, crypto.luks_open,
> > > "/not/existing/device", "another-crypted", key_file=keyfile)
> > > - # no passhprase or key file
> > > - self.assertRaises(ValueError, crypto.luks_open, _LOOP_DEV1,
> > > "another-crypted")
> > > + ##
> > > + ## luks_open
> > > + ##
> > > + # pass
> > > + self.assertEqual(crypto.luks_open(_LOOP_DEV0, _name0,
> > > passphrase="secret"), None)
> > > + self.assertEqual(crypto.luks_open(_LOOP_DEV1, _name1,
> > > passphrase="hidden"), None)
> > >
> > > ##
> > > ## luks_status
> > > ##
> > > # pass
> > > - self.assertEqual(crypto.luks_status("crypted"), True)
> > > - self.assertEqual(crypto.luks_status("encrypted"), True)
> > > - self.assertEqual(crypto.luks_status("another-crypted"), False)
> > > + self.assertEqual(crypto.luks_status(_name0), 2)
> > > + self.assertEqual(crypto.luks_status(_name1), 2)
> > >
> > > ##
> > > ## luks_uuid
> > > @@ -110,19 +171,18 @@ class CryptoTestCase(baseclass.DevicelibsTestCase):
> > > ## luks_close
> > > ##
> > > # pass
> > > - self.assertEqual(crypto.luks_close("crypted"), None)
> > > - self.assertEqual(crypto.luks_close("encrypted"), None)
> > > + self.assertEqual(crypto.luks_close(_name0), None)
> > > + self.assertEqual(crypto.luks_close(_name1), None)
> > >
> > > - # fail
> > > - self.assertRaises(crypto.CryptoError, crypto.luks_close,
> > > "wrong-name")
> > > # already closed
> > > - self.assertRaises(crypto.CryptoError, crypto.luks_close,
> > > "crypted")
> > > - self.assertRaises(crypto.CryptoError, crypto.luks_close,
> > > "encrypted")
> > > -
> > > - # cleanup
> > > - os.unlink(keyfile)
> > > - os.unlink(new_keyfile)
> > > -
> > > + self.assertRaisesRegexp(IOError,
> > > + "Device cannot be opened",
> > > + crypto.luks_close,
> > > + "crypted")
> > > + self.assertRaisesRegexp(IOError,
> > > + "Device cannot be opened",
> > > + crypto.luks_close,
> > > + "encrypted")
> > >
> > > def suite():
> > > return unittest.TestLoader().loadTestsFromTestCase(CryptoTestCase)
> >
> >
> > _______________________________________________
> > anaconda-patches mailing list
> > anaconda-patches at lists.fedorahosted.org
> > https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> >
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
More information about the anaconda-patches
mailing list