From 36ab0d79cbb3489f6095f9dca65297a9a5f1451d Mon Sep 17 00:00:00 2001
From: Zane Bitter <zbitter@redhat.com>
Date: Mon, 8 Apr 2013 12:43:35 +0200
Subject: [PATCH] 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
---
 heat/engine/resources/user.py | 35 +++++++++++++++++++++++------------
 heat/tests/test_user.py       | 17 +++++++++++++++++
 2 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/heat/engine/resources/user.py b/heat/engine/resources/user.py
index 8342ce68..48b58326 100644
--- 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 14e41232..52fc6163 100644
--- 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()
 
 
-- 
2.45.2