Pass user_creds_id into nested stacks
To avoid creating additional user_creds entries for every nested stack, pass in the parent/owner stack user_creds_id. This means updates will work with trust-scoped tokens, and will remove the overhead of creating a new trust for every nested resource. This will also avoid the potential situation where a stack has been updated (adding nested stacks) by a user other than the original stack owner - if that happened, it would be impossible to delete the stacks (owner and nested) as any user other than an admin, due to restrictions on who can delete the trust. Note the delete logic is modified so nested stacks no longer delete the user_creds ID. If we stick with this simple logic, it will mean it's necessary to either delete all stacks before upgrading or do a manual DB query to clean up any orphaned user_creds rows when existing nested stacks are deleted (such a cleanup is probably already necessary due to the previous patch fixing user_creds handling for backup stacks) Also modifies the stub_keystoneclient test call to make stubbing the client in the parser test easier. Change-Id: Id1fb89c92af48777b0487015d5048af34b33af76 Closes-Bug: #1342593
This commit is contained in:
parent
a199f46137
commit
88568e6400
@ -780,7 +780,9 @@ class Stack(collections.Mapping):
|
||||
stack_status = self.FAILED
|
||||
reason = '%s timed out' % action.title()
|
||||
|
||||
if stack_status != self.FAILED and not backup:
|
||||
# If the stack delete suceeded, this is not a backup stack and it's
|
||||
# not a nested stack, we should delete the credentials
|
||||
if stack_status != self.FAILED and not backup and not self.owner_id:
|
||||
# Cleanup stored user_creds so they aren't accessible via
|
||||
# the soft-deleted stack which remains in the DB
|
||||
if self.user_creds_id:
|
||||
|
@ -114,7 +114,8 @@ class StackResource(resource.Resource):
|
||||
self._nested_environment(params),
|
||||
disable_rollback=True,
|
||||
parent_resource=self,
|
||||
owner_id=self.stack.id)
|
||||
owner_id=self.stack.id,
|
||||
user_creds_id=self.stack.user_creds_id)
|
||||
|
||||
return nested.preview_resources()
|
||||
|
||||
@ -173,6 +174,7 @@ class StackResource(resource.Resource):
|
||||
disable_rollback=True,
|
||||
parent_resource=self,
|
||||
owner_id=self.stack.id,
|
||||
user_creds_id=self.stack.user_creds_id,
|
||||
adopt_stack_data=adopt_data)
|
||||
nested.validate()
|
||||
self._nested = nested
|
||||
@ -229,7 +231,8 @@ class StackResource(resource.Resource):
|
||||
timeout_mins=timeout_mins,
|
||||
disable_rollback=True,
|
||||
parent_resource=self,
|
||||
owner_id=self.stack.id)
|
||||
owner_id=self.stack.id,
|
||||
user_creds_id=self.stack.user_creds_id)
|
||||
stack.parameters.set_stack_id(nested_stack.identifier())
|
||||
stack.validate()
|
||||
|
||||
|
@ -114,8 +114,8 @@ class HeatTestCase(testscenarios.WithScenarios,
|
||||
mockfixture = self.useFixture(mockpatch.PatchObject(obj, attr))
|
||||
return mockfixture.mock
|
||||
|
||||
def stub_keystoneclient(self, **kwargs):
|
||||
def stub_keystoneclient(self, fake_client=None, **kwargs):
|
||||
client = self.patchobject(keystone.KeystoneClientPlugin, "_create")
|
||||
fkc = fakes.FakeKeystoneClient(**kwargs)
|
||||
fkc = fake_client or fakes.FakeKeystoneClient(**kwargs)
|
||||
client.return_value = fkc
|
||||
return fkc
|
||||
|
@ -1382,6 +1382,36 @@ class StackTest(HeatTestCase):
|
||||
self.assertEqual(self.stack.state,
|
||||
(parser.Stack.DELETE, parser.Stack.COMPLETE))
|
||||
|
||||
def test_delete_trust_nested(self):
|
||||
cfg.CONF.set_override('deferred_auth_method', 'trusts')
|
||||
|
||||
class FakeKeystoneClientFail(FakeKeystoneClient):
|
||||
def delete_trust(self, trust_id):
|
||||
raise Exception("Shouldn't delete")
|
||||
|
||||
self.stub_keystoneclient(fake_client=FakeKeystoneClientFail())
|
||||
|
||||
self.stack = parser.Stack(
|
||||
self.ctx, 'delete_trust_nested', self.tmpl,
|
||||
owner_id='owner123')
|
||||
stack_id = self.stack.store()
|
||||
|
||||
db_s = db_api.stack_get(self.ctx, stack_id)
|
||||
self.assertIsNotNone(db_s)
|
||||
user_creds_id = db_s.user_creds_id
|
||||
self.assertIsNotNone(user_creds_id)
|
||||
user_creds = db_api.user_creds_get(user_creds_id)
|
||||
self.assertIsNotNone(user_creds)
|
||||
|
||||
self.stack.delete()
|
||||
|
||||
db_s = db_api.stack_get(self.ctx, stack_id)
|
||||
self.assertIsNone(db_s)
|
||||
user_creds = db_api.user_creds_get(user_creds_id)
|
||||
self.assertIsNotNone(user_creds)
|
||||
self.assertEqual(self.stack.state,
|
||||
(parser.Stack.DELETE, parser.Stack.COMPLETE))
|
||||
|
||||
def test_delete_trust_fail(self):
|
||||
cfg.CONF.set_override('deferred_auth_method', 'trusts')
|
||||
|
||||
|
@ -111,7 +111,8 @@ class StackResourceTest(HeatTestCase):
|
||||
'Resources':
|
||||
{self.ws_resname: ws_res_snippet}})
|
||||
self.parent_stack = parser.Stack(utils.dummy_context(), 'test_stack',
|
||||
t, stack_id=str(uuid.uuid4()))
|
||||
t, stack_id=str(uuid.uuid4()),
|
||||
user_creds_id='uc123')
|
||||
resource_defns = t.resource_definitions(self.parent_stack)
|
||||
self.parent_resource = MyStackResource('test',
|
||||
resource_defns[self.ws_resname],
|
||||
@ -172,7 +173,8 @@ class StackResourceTest(HeatTestCase):
|
||||
'environment',
|
||||
disable_rollback=mock.ANY,
|
||||
parent_resource=parent_resource,
|
||||
owner_id=mock.ANY
|
||||
owner_id=self.parent_stack.id,
|
||||
user_creds_id=self.parent_stack.user_creds_id
|
||||
)
|
||||
|
||||
def test_preview_validates_nested_resources(self):
|
||||
@ -500,6 +502,7 @@ class StackResourceTest(HeatTestCase):
|
||||
disable_rollback=True,
|
||||
parent_resource=self.parent_resource,
|
||||
owner_id=self.parent_stack.id,
|
||||
user_creds_id=self.parent_stack.user_creds_id,
|
||||
adopt_stack_data=None).AndReturn(self.stack)
|
||||
|
||||
st_set = self.stack.state_set
|
||||
|
Loading…
Reference in New Issue
Block a user