Fix AccessKey deletion with bad credentials

If an AccessKey could not be created, it also could not be deleted. This
change allows deletion of the resource to proceed even if the user does not
exist.

Fixes bug 1155824

Also fixes a bug that the actual key would never be deleted, because the
resource_id is set to None beforehand.

Change-Id: I34886410e3e25ad62e4eba537063f922b011e25a

diff --git a/heat/engine/resources/user.py b/heat/engine/resources/user.py
index 8342ce683caf9d9371c47d3e455918b3c21b2239..48b58326022073910d1b3cf53065dd4e15346b95 100644 (file)
--- a/heat/engine/resources/user.py
+++ b/heat/engine/resources/user.py
@@ -14,6 +14,7 @@
 #    under the License.
 
 from heat.common import exception
+from heat.engine import clients
 from heat.engine import resource
 
 from heat.openstack.common import log as logging
@@ -141,29 +142,39 @@ class AccessKey(resource.Resource):
         return self.stack.resource_by_refid(self.properties['UserName'])
 
     def handle_create(self):
-        try:
-            user_id = self._get_user().resource_id
-        except AttributeError:
+        user = self._get_user()
+        if user is None:
             raise exception.NotFound('could not find user %s' %
                                      self.properties['UserName'])
 
-        kp = self.keystone().get_ec2_keypair(user_id)
+        kp = self.keystone().get_ec2_keypair(user.resource_id)
         if not kp:
             raise exception.Error("Error creating ec2 keypair for user %s" %
-                                  user_id)
-        else:
-            self.resource_id_set(kp.access)
-            self._secret = kp.secret
+                                  user)
+
+        self.resource_id_set(kp.access)
+        self._secret = kp.secret
 
     def handle_update(self, json_snippet):
         return self.UPDATE_REPLACE
 
     def handle_delete(self):
-        self.resource_id_set(None)
         self._secret = None
-        user_id = self._get_user().resource_id
-        if user_id and self.resource_id:
-            self.keystone().delete_ec2_keypair(user_id, self.resource_id)
+        if self.resource_id is None:
+            return
+
+        user = self._get_user()
+        if user is None:
+            logger.warning('Error deleting %s - user not found' % str(self))
+            return
+        user_id = user.resource_id
+        if user_id:
+            try:
+                self.keystone().delete_ec2_keypair(user_id, self.resource_id)
+            except clients.hkc.kc.exceptions.NotFound:
+                pass
+
+        self.resource_id_set(None)
 
     def _secret_accesskey(self):
         '''

diff --git a/heat/tests/test_user.py b/heat/tests/test_user.py
index 14e41232e6ac72ca9bf56e8d4cb29740ec4ea8f3..52fc616364df62c6d4349935a05d0e5f47928154 100644 (file)
--- a/heat/tests/test_user.py
+++ b/heat/tests/test_user.py
@@ -27,6 +27,8 @@ from heat.engine import parser
 from heat.engine.resources import user
 from heat.tests import fakes
 
+import keystoneclient.exceptions
+
 
 @attr(tag=['unit', 'resource', 'User'])
 @attr(speed='fast')
@@ -301,6 +303,19 @@ class AccessKeyTest(unittest.TestCase):
         self.assertEqual(None, resource.delete())
         self.m.VerifyAll()
 
+        # Check for double delete
+        test_key = object()
+        self.m.StubOutWithMock(self.fc, 'delete_ec2_keypair')
+        NotFound = keystoneclient.exceptions.NotFound
+        self.fc.delete_ec2_keypair(self.fc.user_id,
+                                   test_key).AndRaise(NotFound('Gone'))
+
+        self.m.ReplayAll()
+        resource.state = resource.CREATE_COMPLETE
+        resource.resource_id = test_key
+        self.assertEqual(None, resource.delete())
+        self.m.VerifyAll()
+
     def test_access_key_no_user(self):
         self.m.ReplayAll()
 
@@ -319,6 +334,8 @@ class AccessKeyTest(unittest.TestCase):
         self.assertEqual(user.AccessKey.CREATE_FAILED,
                          resource.state)
 
+        self.assertEqual(None, resource.delete())
+
         self.m.VerifyAll()