openstack
/
nova
Code Issues Proposed changes

Fix broken cert revocation 

Cert revocation was broken by
32b0adb591.  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 c8538208da)
This commit is contained in:
Russell Bryant 2014-10-08 12:14:31 +00:00 committed by Thierry Carrez
parent ed188559c8
commit 3f9003270e
2 changed files with 5 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
nova/crypto.py
View File

@ -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

4
nova/tests/test_crypto.py
View File

@ -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')