From 4e36926d0161120e972e18ddfca31cf8a4a37e96 Mon Sep 17 00:00:00 2001 From: Steven Hardy Date: Tue, 11 Feb 2014 12:37:20 +0000 Subject: [PATCH] migrate User/AccessKey resources to StackUser base class migrate the User/AccessKey resources to use the StackUser base class, and therefore convert to stack domain users. Note that becuse the User resource inherits from StackUser, and the keypair is managed by that resource now, we have to update both resources at the same time, as it's not possible to update individually and keep them both working. This (finally! :) resolves bug #1089261 and allows all resources to be created even if the user creating the stack doesn't have the required roles to create keystone users. Closes-Bug: #1089261 blueprint: instance-users Change-Id: I854644e7669150521f8904cdb76c292519ef4167 --- heat/engine/resources/user.py | 66 ++----- heat/tests/fakes.py | 4 +- heat/tests/test_autoscaling_update_policy.py | 10 +- heat/tests/test_ceilometer_alarm.py | 5 +- heat/tests/test_loadbalancer.py | 11 +- heat/tests/test_user.py | 177 ++++++++++--------- 6 files changed, 112 insertions(+), 161 deletions(-) diff --git a/heat/engine/resources/user.py b/heat/engine/resources/user.py index 89b0ba3017..d1917bf613 100644 --- a/heat/engine/resources/user.py +++ b/heat/engine/resources/user.py @@ -17,10 +17,10 @@ from heat.db import api as db_api from heat.engine import constraints from heat.engine import properties from heat.engine import resource +from heat.engine import stack_user from heat.openstack.common import log as logging -import keystoneclient.exceptions as kc_exception logger = logging.getLogger(__name__) @@ -31,7 +31,7 @@ logger = logging.getLogger(__name__) # -class User(resource.Resource): +class User(stack_user.StackUser): PROPERTIES = ( PATH, GROUPS, LOGIN_PROFILE, POLICIES, ) = ( @@ -100,43 +100,17 @@ class User(resource.Resource): return True def handle_create(self): - passwd = '' profile = self.properties[self.LOGIN_PROFILE] if profile and self.LOGIN_PROFILE_PASSWORD in profile: - passwd = profile[self.LOGIN_PROFILE_PASSWORD] + self.password = profile[self.LOGIN_PROFILE_PASSWORD] if self.properties[self.POLICIES]: if not self._validate_policies(self.properties[self.POLICIES]): raise exception.InvalidTemplateAttribute(resource=self.name, key=self.POLICIES) - uid = self.keystone().create_stack_user(self.physical_resource_name(), - passwd) - self.resource_id_set(uid) - - def handle_delete(self): - if self.resource_id is None: - logger.error(_("Cannot delete User resource before user " - "created!")) - return - try: - self.keystone().delete_stack_user(self.resource_id) - except kc_exception.NotFound: - pass - - def handle_suspend(self): - if self.resource_id is None: - logger.error(_("Cannot suspend User resource before user " - "created!")) - return - self.keystone().disable_stack_user(self.resource_id) - - def handle_resume(self): - if self.resource_id is None: - logger.error(_("Cannot resume User resource before user " - "created!")) - return - self.keystone().enable_stack_user(self.resource_id) + super(User, self).handle_create() + self.resource_id_set(self._get_user_id()) def FnGetRefId(self): return unicode(self.physical_resource_name()) @@ -212,25 +186,16 @@ class AccessKey(resource.Resource): if user is None: raise exception.NotFound(_('could not find user %s') % self.properties[self.USER_NAME]) - - kp = self.keystone().create_ec2_keypair(user.resource_id) - if not kp: - raise exception.Error(_("Error creating ec2 keypair for user %s") % - user) - + # The keypair is actually created and owned by the User resource + kp = user._create_keypair() self.resource_id_set(kp.access) self._secret = kp.secret self._register_access_key() - # Store the secret key, encrypted, in the DB so we don't have to - # re-request it from keystone every time someone requests the - # SecretAccessKey attribute - db_api.resource_data_set(self, 'secret_key', kp.secret, - redact=True) - # Also store the credential ID as this should be used to manage - # the credential rather than the access key via v3/credentials - db_api.resource_data_set(self, 'credential_id', kp.id, - redact=True) + # Store the secret key, encrypted, in the DB so we don't have lookup + # the user every time someone requests the SecretAccessKey attribute + db_api.resource_data_set(self, 'secret_key', kp.secret, redact=True) + db_api.resource_data_set(self, 'credential_id', kp.id, redact=True) def handle_delete(self): self._secret = None @@ -241,14 +206,7 @@ class AccessKey(resource.Resource): 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 kc_exception.NotFound: - pass - - self.resource_id_set(None) + user._delete_keypair() def _secret_accesskey(self): ''' diff --git a/heat/tests/fakes.py b/heat/tests/fakes.py index a4c202141a..0914dc7d1e 100644 --- a/heat/tests/fakes.py +++ b/heat/tests/fakes.py @@ -114,8 +114,8 @@ class FakeKeystoneClient(object): if user_id == self.user_id: return self.creds - def delete_ec2_keypair(self, user_id=None, access=None, - credential_id=None): + def delete_ec2_keypair(self, credential_id=None, user_id=None, + access=None): if user_id == self.user_id and access == self.creds.access: self.creds = None else: diff --git a/heat/tests/test_autoscaling_update_policy.py b/heat/tests/test_autoscaling_update_policy.py index 83723d95b3..e5752298c0 100644 --- a/heat/tests/test_autoscaling_update_policy.py +++ b/heat/tests/test_autoscaling_update_policy.py @@ -22,7 +22,7 @@ from heat.common import template_format from heat.engine.notification import stack as notification from heat.engine import clients from heat.engine import parser -from heat.engine.resources import user +from heat.engine import stack_user from heat.engine.resources import instance from heat.engine.resources import loadbalancer as lb from heat.engine.resources import wait_condition as wc @@ -216,12 +216,8 @@ class AutoScalingGroupTest(HeatTestCase): parser.Stack.validate().MultipleTimes() def _stub_lb_create(self): - self.m.StubOutWithMock(user.User, 'keystone') - user.User.keystone().AndReturn(self.fkc) - self.m.StubOutWithMock(user.AccessKey, 'keystone') - user.AccessKey.keystone().AndReturn(self.fkc) - self.m.StubOutWithMock(wc.WaitConditionHandle, 'keystone') - wc.WaitConditionHandle.keystone().MultipleTimes().AndReturn(self.fkc) + self.m.StubOutWithMock(stack_user.StackUser, 'keystone') + stack_user.StackUser.keystone().MultipleTimes().AndReturn(self.fkc) self.m.StubOutWithMock(wc.WaitConditionHandle, 'get_status') wc.WaitConditionHandle.get_status().AndReturn(['SUCCESS']) diff --git a/heat/tests/test_ceilometer_alarm.py b/heat/tests/test_ceilometer_alarm.py index 123a1ea2e0..776a9b1084 100644 --- a/heat/tests/test_ceilometer_alarm.py +++ b/heat/tests/test_ceilometer_alarm.py @@ -33,6 +33,7 @@ from heat.engine import clients from heat.engine import parser from heat.engine import resource from heat.engine import scheduler +from heat.engine import stack_user from heat.engine.properties import schemata from heat.engine.resources.ceilometer import alarm @@ -158,8 +159,8 @@ class CeilometerAlarmTest(HeatTestCase): disable_rollback=True) stack.store() - self.m.StubOutWithMock(resource.Resource, 'keystone') - resource.Resource.keystone().MultipleTimes().AndReturn( + self.m.StubOutWithMock(stack_user.StackUser, 'keystone') + stack_user.StackUser.keystone().MultipleTimes().AndReturn( self.fc) self.m.StubOutWithMock(alarm.CeilometerAlarm, 'ceilometer') diff --git a/heat/tests/test_loadbalancer.py b/heat/tests/test_loadbalancer.py index 0871b24883..4cc2e95ce2 100644 --- a/heat/tests/test_loadbalancer.py +++ b/heat/tests/test_loadbalancer.py @@ -21,8 +21,8 @@ from heat.common import exception from heat.common import template_format from heat.engine import clients from heat.engine import scheduler +from heat.engine import stack_user from heat.engine.resources import instance -from heat.engine.resources import user from heat.engine.resources import loadbalancer as lb from heat.engine.resources import wait_condition as wc from heat.engine.resource import Metadata @@ -124,13 +124,8 @@ class LoadBalancerTest(HeatTestCase): def _create_stubs(self, key_name='test', stub_meta=True): - self.m.StubOutWithMock(user.User, 'keystone') - user.User.keystone().AndReturn(self.fkc) - self.m.StubOutWithMock(user.AccessKey, 'keystone') - user.AccessKey.keystone().AndReturn(self.fkc) - - self.m.StubOutWithMock(wc.WaitConditionHandle, 'keystone') - wc.WaitConditionHandle.keystone().MultipleTimes().AndReturn(self.fkc) + self.m.StubOutWithMock(stack_user.StackUser, 'keystone') + stack_user.StackUser.keystone().MultipleTimes().AndReturn(self.fkc) server_name = utils.PhysName( utils.PhysName('test_stack', 'LoadBalancer'), diff --git a/heat/tests/test_user.py b/heat/tests/test_user.py index e25ef4d61c..98f836594b 100644 --- a/heat/tests/test_user.py +++ b/heat/tests/test_user.py @@ -11,10 +11,12 @@ # License for the specific language governing permissions and limitations # under the License. +import uuid from oslo.config import cfg from heat.common import exception +from heat.common import short_id from heat.common import template_format from heat.db import api as db_api from heat.engine import resource @@ -24,7 +26,6 @@ from heat.tests.common import HeatTestCase from heat.tests import fakes from heat.tests import utils -import keystoneclient.exceptions user_template = ''' { @@ -39,6 +40,22 @@ user_template = ''' } ''' +user_template_password = ''' +{ + "AWSTemplateFormatVersion" : "2010-09-09", + "Description" : "Just a User", + "Parameters" : {}, + "Resources" : { + "CfnUser" : { + "Type" : "AWS::IAM::User", + "Properties": { + "LoginProfile": { "Password": "myP@ssW0rd" } + } + } + } +} +''' + user_accesskey_template = ''' { "AWSTemplateFormatVersion" : "2010-09-09", @@ -86,40 +103,52 @@ user_policy_template = ''' ''' -class UserPolicyTestCase(HeatTestCase): +class UserTest(HeatTestCase): def setUp(self): - super(UserPolicyTestCase, self).setUp() - username = utils.PhysName('test_stack', 'CfnUser') - self.fc = fakes.FakeKeystoneClient(username=username) + super(UserTest, self).setUp() + self.username = 'test_stack-CfnUser-aabbcc' + self.fc = fakes.FakeKeystoneClient(username=self.username) cfg.CONF.set_default('heat_stack_user_role', 'stack_user_role') utils.setup_dummy_db() + self.resource_id = str(uuid.uuid4()) + def create_user(self, t, stack, resource_name, + project_id='stackproject', user_id='dummy_user', + password=None): + self.m.StubOutWithMock(user.User, 'keystone') + user.User.keystone().MultipleTimes().AndReturn(self.fc) -class UserTest(UserPolicyTestCase): + self.m.StubOutWithMock(fakes.FakeKeystoneClient, + 'create_stack_domain_project') + fakes.FakeKeystoneClient.create_stack_domain_project( + stack_name=stack.name).AndReturn(project_id) + + self.m.StubOutWithMock(short_id, 'get_id') + short_id.get_id(self.resource_id).MultipleTimes().AndReturn('aabbcc') + + self.m.StubOutWithMock(fakes.FakeKeystoneClient, + 'create_stack_domain_user') + fakes.FakeKeystoneClient.create_stack_domain_user( + username=self.username, password=password, + project_id=project_id).AndReturn(user_id) + self.m.ReplayAll() - def create_user(self, t, stack, resource_name): rsrc = user.User(resource_name, t['Resources'][resource_name], stack) self.assertIsNone(rsrc.validate()) - scheduler.TaskRunner(rsrc.create)() + with utils.UUIDStub(self.resource_id): + scheduler.TaskRunner(rsrc.create)() self.assertEqual((rsrc.CREATE, rsrc.COMPLETE), rsrc.state) return rsrc def test_user(self): - - self.m.StubOutWithMock(user.User, 'keystone') - user.User.keystone().MultipleTimes().AndReturn(self.fc) - - self.m.ReplayAll() - t = template_format.parse(user_template) stack = utils.parse_stack(t) rsrc = self.create_user(t, stack, 'CfnUser') - self.assertEqual(self.fc.user_id, rsrc.resource_id) - self.assertEqual(utils.PhysName('test_stack', 'CfnUser'), - rsrc.FnGetRefId()) + self.assertEqual('dummy_user', rsrc.resource_id) + self.assertEqual(self.username, rsrc.FnGetRefId()) self.assertRaises(exception.InvalidTemplateAttribute, rsrc.FnGetAtt, 'Foo') @@ -149,28 +178,31 @@ class UserTest(UserPolicyTestCase): self.assertEqual((rsrc.DELETE, rsrc.COMPLETE), rsrc.state) self.m.VerifyAll() + def test_user_password(self): + t = template_format.parse(user_template_password) + stack = utils.parse_stack(t) + + rsrc = self.create_user(t, stack, 'CfnUser', password=u'myP@ssW0rd') + self.assertEqual('dummy_user', rsrc.resource_id) + self.assertEqual(self.username, rsrc.FnGetRefId()) + + self.assertEqual((rsrc.CREATE, rsrc.COMPLETE), rsrc.state) + self.m.VerifyAll() + def test_user_validate_policies(self): - - self.m.StubOutWithMock(user.User, 'keystone') - user.User.keystone().MultipleTimes().AndReturn(self.fc) - - self.m.ReplayAll() - t = template_format.parse(user_policy_template) stack = utils.parse_stack(t) rsrc = self.create_user(t, stack, 'CfnUser') - self.assertEqual(self.fc.user_id, rsrc.resource_id) - self.assertEqual(utils.PhysName('test_stack', 'CfnUser'), - rsrc.FnGetRefId()) + self.assertEqual('dummy_user', rsrc.resource_id) + self.assertEqual(self.username, rsrc.FnGetRefId()) self.assertEqual((rsrc.CREATE, rsrc.COMPLETE), rsrc.state) self.assertEqual([u'WebServerAccessPolicy'], rsrc.properties['Policies']) # OK - self.assertTrue( - rsrc._validate_policies([u'WebServerAccessPolicy'])) + self.assertTrue(rsrc._validate_policies([u'WebServerAccessPolicy'])) # Resource name doesn't exist in the stack self.assertFalse(rsrc._validate_policies([u'NoExistAccessPolicy'])) @@ -193,8 +225,6 @@ class UserTest(UserPolicyTestCase): self.m.VerifyAll() def test_user_create_bad_policies(self): - self.m.ReplayAll() - t = template_format.parse(user_policy_template) t['Resources']['CfnUser']['Properties']['Policies'] = ['NoExistBad'] stack = utils.parse_stack(t) @@ -204,13 +234,8 @@ class UserTest(UserPolicyTestCase): stack) self.assertRaises(exception.InvalidTemplateAttribute, rsrc.handle_create) - self.m.VerifyAll() def test_user_access_allowed(self): - - self.m.StubOutWithMock(user.User, 'keystone') - user.User.keystone().MultipleTimes().AndReturn(self.fc) - self.m.StubOutWithMock(user.AccessPolicy, 'access_allowed') user.AccessPolicy.access_allowed('a_resource').AndReturn(True) user.AccessPolicy.access_allowed('b_resource').AndReturn(False) @@ -221,9 +246,8 @@ class UserTest(UserPolicyTestCase): stack = utils.parse_stack(t) rsrc = self.create_user(t, stack, 'CfnUser') - self.assertEqual(self.fc.user_id, rsrc.resource_id) - self.assertEqual(utils.PhysName('test_stack', 'CfnUser'), - rsrc.FnGetRefId()) + self.assertEqual('dummy_user', rsrc.resource_id) + self.assertEqual(self.username, rsrc.FnGetRefId()) self.assertEqual((rsrc.CREATE, rsrc.COMPLETE), rsrc.state) self.assertTrue(rsrc.access_allowed('a_resource')) @@ -231,10 +255,6 @@ class UserTest(UserPolicyTestCase): self.m.VerifyAll() def test_user_access_allowed_ignorepolicy(self): - - self.m.StubOutWithMock(user.User, 'keystone') - user.User.keystone().MultipleTimes().AndReturn(self.fc) - self.m.StubOutWithMock(user.AccessPolicy, 'access_allowed') user.AccessPolicy.access_allowed('a_resource').AndReturn(True) user.AccessPolicy.access_allowed('b_resource').AndReturn(False) @@ -247,9 +267,8 @@ class UserTest(UserPolicyTestCase): stack = utils.parse_stack(t) rsrc = self.create_user(t, stack, 'CfnUser') - self.assertEqual(self.fc.user_id, rsrc.resource_id) - self.assertEqual(utils.PhysName('test_stack', 'CfnUser'), - rsrc.FnGetRefId()) + self.assertEqual('dummy_user', rsrc.resource_id) + self.assertEqual(self.username, rsrc.FnGetRefId()) self.assertEqual((rsrc.CREATE, rsrc.COMPLETE), rsrc.state) self.assertTrue(rsrc.access_allowed('a_resource')) @@ -257,7 +276,29 @@ class UserTest(UserPolicyTestCase): self.m.VerifyAll() -class AccessKeyTest(UserPolicyTestCase): +class AccessKeyTest(HeatTestCase): + def setUp(self): + super(AccessKeyTest, self).setUp() + utils.setup_dummy_db() + self.username = utils.PhysName('test_stack', 'CfnUser') + self.credential_id = 'acredential123' + self.fc = fakes.FakeKeystoneClient(username=self.username, + user_id='dummy_user', + credential_id=self.credential_id) + cfg.CONF.set_default('heat_stack_user_role', 'stack_user_role') + + def create_user(self, t, stack, resource_name, + project_id='stackproject', user_id='dummy_user', + password=None): + self.m.StubOutWithMock(user.User, 'keystone') + user.User.keystone().MultipleTimes().AndReturn(self.fc) + + self.m.ReplayAll() + rsrc = stack[resource_name] + self.assertIsNone(rsrc.validate()) + scheduler.TaskRunner(rsrc.create)() + self.assertEqual((rsrc.CREATE, rsrc.COMPLETE), rsrc.state) + return rsrc def create_access_key(self, t, stack, resource_name): rsrc = user.AccessKey(resource_name, @@ -268,28 +309,14 @@ class AccessKeyTest(UserPolicyTestCase): self.assertEqual((rsrc.CREATE, rsrc.COMPLETE), rsrc.state) return rsrc - def create_user(self, t, stack, resource_name): - rsrc = stack[resource_name] - self.assertIsNone(rsrc.validate()) - scheduler.TaskRunner(rsrc.create)() - self.assertEqual((rsrc.CREATE, rsrc.COMPLETE), rsrc.state) - return rsrc - def test_access_key(self): - self.m.StubOutWithMock(user.AccessKey, 'keystone') - self.m.StubOutWithMock(user.User, 'keystone') - user.AccessKey.keystone().MultipleTimes().AndReturn(self.fc) - user.User.keystone().MultipleTimes().AndReturn(self.fc) - - self.m.ReplayAll() - t = template_format.parse(user_accesskey_template) - stack = utils.parse_stack(t) self.create_user(t, stack, 'CfnUser') rsrc = self.create_access_key(t, stack, 'HostKeys') + self.m.VerifyAll() self.assertRaises(resource.UpdateReplace, rsrc.handle_update, {}, {}, {}) self.assertEqual(self.fc.access, @@ -319,9 +346,7 @@ class AccessKeyTest(UserPolicyTestCase): def test_access_key_get_from_keystone(self): self.m.StubOutWithMock(user.AccessKey, 'keystone') - self.m.StubOutWithMock(user.User, 'keystone') user.AccessKey.keystone().MultipleTimes().AndReturn(self.fc) - user.User.keystone().MultipleTimes().AndReturn(self.fc) self.m.ReplayAll() @@ -348,30 +373,6 @@ class AccessKeyTest(UserPolicyTestCase): self.assertEqual((rsrc.DELETE, rsrc.COMPLETE), rsrc.state) self.m.VerifyAll() - def test_access_key_deleted(self): - self.m.StubOutWithMock(user.AccessKey, 'keystone') - self.m.StubOutWithMock(user.User, 'keystone') - user.AccessKey.keystone().MultipleTimes().AndReturn(self.fc) - user.User.keystone().MultipleTimes().AndReturn(self.fc) - - self.m.ReplayAll() - - t = template_format.parse(user_accesskey_template) - stack = utils.parse_stack(t) - - self.create_user(t, stack, 'CfnUser') - rsrc = self.create_access_key(t, stack, 'HostKeys') - self.assertEqual((rsrc.CREATE, rsrc.COMPLETE), rsrc.state) - - self.m.StubOutWithMock(self.fc, 'delete_ec2_keypair') - NotFound = keystoneclient.exceptions.NotFound - self.fc.delete_ec2_keypair(self.fc.user_id, - rsrc.resource_id).AndRaise(NotFound('Gone')) - self.m.ReplayAll() - scheduler.TaskRunner(rsrc.delete)() - - self.m.VerifyAll() - def test_access_key_no_user(self): self.m.ReplayAll() @@ -394,7 +395,7 @@ class AccessKeyTest(UserPolicyTestCase): self.m.VerifyAll() -class AccessPolicyTest(UserPolicyTestCase): +class AccessPolicyTest(HeatTestCase): def test_accesspolicy_create_ok(self): t = template_format.parse(user_policy_template)