diff options
| author | Russell Bryant <rbryant@redhat.com> | 2014-10-08 12:14:31 +0000 |
|---|---|---|
| committer | Thierry Carrez <thierry@openstack.org> | 2014-10-09 08:59:06 +0200 |
| commit | 3f9003270efd9ac036f3c229b36baa0bb05203bf (patch) | |
| tree | 0152878a01015bcf6ba17a72331a83930be64d61 | |
| parent | ed188559c8e5911e066fbad0c7d1d023ee4c09b2 (diff) | |
| download | nova-3f9003270efd9ac036f3c229b36baa0bb05203bf.tar.gz | |
Fix broken cert revocation
Cert revocation was broken by
32b0adb591f80ad2c5c19519b4ffc2b55dbea672. os.chdir() never returns
anything, so this method would always raise an exception. The proper
way to handle an error from os.chdir() is to catch OSError.
There were existing tests for this code, but they conveniently mocked
os.chdir() to return values that are never actually returned. The
tests were fixed to match the real behavior.
Change-Id: I7549bb60a7d43d53d6f81eecea31cbb9720cc8b6
Closes-bug: #1376368
(cherry picked from commit c8538208da00c3b0d0646629c9d668aa69944b85)
| -rw-r--r-- | nova/crypto.py | 4 | ||||
| -rw-r--r-- | nova/tests/test_crypto.py | 4 |
2 files changed, 5 insertions, 3 deletions
diff --git a/nova/crypto.py b/nova/crypto.py index cecd1846bc..c9f3030d48 100644 --- a/nova/crypto.py +++ b/nova/crypto.py @@ -274,7 +274,9 @@ def ssh_encrypt_text(ssh_public_key, text): def revoke_cert(project_id, file_name): """Revoke a cert by file name.""" start = os.getcwd() - if not os.chdir(ca_folder(project_id)): + try: + os.chdir(ca_folder(project_id)) + except OSError: raise exception.ProjectNotFound(project_id=project_id) try: # NOTE(vish): potential race condition here diff --git a/nova/tests/test_crypto.py b/nova/tests/test_crypto.py index b99f329a5c..392937f864 100644 --- a/nova/tests/test_crypto.py +++ b/nova/tests/test_crypto.py @@ -136,12 +136,12 @@ class RevokeCertsTest(test.TestCase): @mock.patch.object(utils, 'execute', side_effect=processutils.ProcessExecutionError) - @mock.patch.object(os, 'chdir', return_value=True) + @mock.patch.object(os, 'chdir', return_value=None) def test_revoke_cert_process_execution_error(self, *args, **kargs): self.assertRaises(exception.RevokeCertFailure, crypto.revoke_cert, 2, 'test_file') - @mock.patch.object(os, 'chdir', return_value=False) + @mock.patch.object(os, 'chdir', mock.Mock(side_effect=OSError)) def test_revoke_cert_project_not_found_chdir_fails(self, *args, **kargs): self.assertRaises(exception.ProjectNotFound, crypto.revoke_cert, 2, 'test_file') |