From: Steve Baker Date: Fri, 30 Aug 2013 05:21:56 +0000 (+1200) Subject: Only validate credentials on create based on resources X-Git-Tag: 2014.1~56^2 X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=af238fbd081f7c14016d923c3924a648963fdeca;p=openstack-build%2Fheat-build.git Only validate credentials on create based on resources This change relaxes the validation which checked for credentials on stack create and update. As implemented, having any of the following resources in the template will result in credentials being mandatory on create and update: * AWS::AutoScaling::ScalingPolicy * OS::Heat::HARestarter * AWS::CloudFormation::WaitConditionHandle For all other templates, credentials are not needed. When trusts are merged, this logic could also be used to decide whether a trust token needs to be created at all. Fixes bug: #1217617 Change-Id: I3e4b8698d3712053dc3c0851433ef0cbbadbdfed --- diff --git a/heat/engine/parser.py b/heat/engine/parser.py index f715a6c0..492d2c3b 100644 --- a/heat/engine/parser.py +++ b/heat/engine/parser.py @@ -271,6 +271,14 @@ class Stack(object): if result: raise StackValidationFailed(message=result) + def requires_deferred_auth(self): + ''' + Returns whether this stack may need to perform API requests + during its lifecycle using the configured deferred authentication + method. + ''' + return any(res.requires_deferred_auth for res in self) + def state_set(self, action, status, reason): '''Update the stack state in the database.''' if action not in self.ACTIONS: diff --git a/heat/engine/resource.py b/heat/engine/resource.py index 39d5562c..87bcb9c7 100644 --- a/heat/engine/resource.py +++ b/heat/engine/resource.py @@ -116,6 +116,10 @@ class Resource(object): # that describes the appropriate resource attributes attributes_schema = {} + # If True, this resource may perform authenticated API requests + # throughout its lifecycle + requires_deferred_auth = False + def __new__(cls, name, json, stack): '''Create a new Resource of the appropriate class for its type.''' diff --git a/heat/engine/service.py b/heat/engine/service.py index 5b35bdcb..61a414b7 100644 --- a/heat/engine/service.py +++ b/heat/engine/service.py @@ -206,7 +206,13 @@ class EngineService(service.Service): stacks = db_api.stack_get_all_by_tenant(cnxt) or [] return list(format_stack_details(stacks)) - def _validate_mandatory_credentials(self, cnxt): + def _validate_deferred_auth_context(self, cnxt, stack): + if cfg.CONF.deferred_auth_method != 'password': + return + + if not stack.requires_deferred_auth(): + return + if cnxt.username is None: raise exception.MissingCredentialError(required='X-Auth-User') if cnxt.password is None: @@ -229,8 +235,6 @@ class EngineService(service.Service): """ logger.info('template is %s' % template) - self._validate_mandatory_credentials(cnxt) - def _stack_create(stack): # Create the stack, and create the periodic task if successful stack.create() @@ -251,6 +255,8 @@ class EngineService(service.Service): stack = parser.Stack(cnxt, stack_name, tmpl, env, **common_params) + self._validate_deferred_auth_context(cnxt, stack) + stack.validate() # Creates a trust and sets the trust_id and trustor_user_id in @@ -280,8 +286,6 @@ class EngineService(service.Service): """ logger.info('template is %s' % template) - self._validate_mandatory_credentials(cnxt) - # Get the database representation of the existing stack db_stack = self._get_stack(cnxt, stack_identity) @@ -296,6 +300,7 @@ class EngineService(service.Service): updated_stack = parser.Stack(cnxt, stack_name, tmpl, env, **common_params) + self._validate_deferred_auth_context(cnxt, updated_stack) updated_stack.validate() self._start_in_thread(db_stack.id, current_stack.update, updated_stack) diff --git a/heat/engine/signal_responder.py b/heat/engine/signal_responder.py index 8d005fca..694d8cac 100644 --- a/heat/engine/signal_responder.py +++ b/heat/engine/signal_responder.py @@ -39,6 +39,10 @@ SIGNAL_VERB = {WAITCONDITION: 'PUT', class SignalResponder(resource.Resource): + # Anything which subclasses this may trigger authenticated + # API operations as a consequence of handling a signal + requires_deferred_auth = True + def handle_create(self): # Create a keystone user so we can create a signed URL via FnGetRefId user_id = self.keystone().create_stack_user( diff --git a/heat/tests/test_engine_service.py b/heat/tests/test_engine_service.py index bd60bbd3..ea40369e 100644 --- a/heat/tests/test_engine_service.py +++ b/heat/tests/test_engine_service.py @@ -372,19 +372,46 @@ class StackServiceCreateUpdateDeleteTest(HeatTestCase): stack.t, {}, None, {}) def test_stack_create_no_credentials(self): - stack_name = 'service_create_test_stack' + stack_name = 'test_stack_create_no_credentials' params = {'foo': 'bar'} template = '{ "Template": "data" }' - ctx = self.ctx = utils.dummy_context(password=None) - self.assertRaises(exception.MissingCredentialError, - self.man.create_stack, ctx, stack_name, template, - params, None, {}) + stack = get_wordpress_stack(stack_name, self.ctx) + # force check for credentials on create + stack.resources['WebServer'].requires_deferred_auth = True + + self.m.StubOutWithMock(parser, 'Template') + self.m.StubOutWithMock(environment, 'Environment') + self.m.StubOutWithMock(parser, 'Stack') - ctx = self.ctx = utils.dummy_context(user=None) - self.assertRaises(exception.MissingCredentialError, - self.man.create_stack, ctx, stack_name, template, - params, None, {}) + ctx_no_pwd = utils.dummy_context(password=None) + ctx_no_user = utils.dummy_context(user=None) + + parser.Template(template, files=None).AndReturn(stack.t) + environment.Environment(params).AndReturn(stack.env) + parser.Stack(ctx_no_pwd, stack.name, + stack.t, stack.env).AndReturn(stack) + + parser.Template(template, files=None).AndReturn(stack.t) + environment.Environment(params).AndReturn(stack.env) + parser.Stack(ctx_no_user, stack.name, + stack.t, stack.env).AndReturn(stack) + + self.m.ReplayAll() + + ex = self.assertRaises(exception.MissingCredentialError, + self.man.create_stack, + ctx_no_pwd, stack_name, + template, params, None, {}) + self.assertEqual( + 'Missing required credential: X-Auth-Key', ex.message) + + ex = self.assertRaises(exception.MissingCredentialError, + self.man.create_stack, + ctx_no_user, stack_name, + template, params, None, {}) + self.assertEqual( + 'Missing required credential: X-Auth-User', ex.message) def test_stack_validate(self): stack_name = 'service_create_test_validate' @@ -543,23 +570,82 @@ class StackServiceCreateUpdateDeleteTest(HeatTestCase): self.m.VerifyAll() def test_stack_update_no_credentials(self): - stack_name = 'service_update_nonexist_test_stack' + stack_name = 'test_stack_update_no_credentials' params = {'foo': 'bar'} template = '{ "Template": "data" }' - stack = get_wordpress_stack(stack_name, self.ctx) + old_stack = get_wordpress_stack(stack_name, self.ctx) + # force check for credentials on create + old_stack.resources['WebServer'].requires_deferred_auth = True - ctx = self.ctx = utils.dummy_context(password=None) - self.assertRaises(exception.MissingCredentialError, - self.man.update_stack, - ctx, stack.identifier(), template, params, - None, {}) + sid = old_stack.store() + s = db_api.stack_get(self.ctx, sid) - ctx = self.ctx = utils.dummy_context(user=None) - self.assertRaises(exception.MissingCredentialError, - self.man.update_stack, - ctx, stack.identifier(), template, params, - None, {}) + self.ctx = utils.dummy_context(password=None) + + self.m.StubOutWithMock(parser, 'Stack') + self.m.StubOutWithMock(parser.Stack, 'load') + self.m.StubOutWithMock(parser, 'Template') + self.m.StubOutWithMock(environment, 'Environment') + + parser.Stack.load(self.ctx, stack=s).AndReturn(old_stack) + + parser.Template(template, files=None).AndReturn(old_stack.t) + environment.Environment(params).AndReturn(old_stack.env) + parser.Stack(self.ctx, old_stack.name, + old_stack.t, old_stack.env).AndReturn(old_stack) + + self.m.ReplayAll() + + ex = self.assertRaises(exception.MissingCredentialError, + self.man.update_stack, self.ctx, + old_stack.identifier(), + template, params, None, {}) + + self.assertEqual( + 'Missing required credential: X-Auth-Key', ex.message) + + self.m.VerifyAll() + + def test_validate_deferred_auth_context_trusts(self): + stack = get_wordpress_stack('test_deferred_auth', self.ctx) + stack.resources['WebServer'].requires_deferred_auth = True + ctx = utils.dummy_context(user=None, password=None) + cfg.CONF.set_default('deferred_auth_method', 'trusts') + + # using trusts, no username or password required + self.man._validate_deferred_auth_context(ctx, stack) + + def test_validate_deferred_auth_context_not_required(self): + stack = get_wordpress_stack('test_deferred_auth', self.ctx) + stack.resources['WebServer'].requires_deferred_auth = False + ctx = utils.dummy_context(user=None, password=None) + cfg.CONF.set_default('deferred_auth_method', 'password') + + # stack performs no deferred operations, so no username or + # password required + self.man._validate_deferred_auth_context(ctx, stack) + + def test_validate_deferred_auth_context_missing_credentials(self): + stack = get_wordpress_stack('test_deferred_auth', self.ctx) + stack.resources['WebServer'].requires_deferred_auth = True + cfg.CONF.set_default('deferred_auth_method', 'password') + + # missing username + ctx = utils.dummy_context(user=None) + ex = self.assertRaises(exception.MissingCredentialError, + self.man._validate_deferred_auth_context, + ctx, stack) + self.assertEqual( + 'Missing required credential: X-Auth-User', ex.message) + + # missing password + ctx = utils.dummy_context(password=None) + ex = self.assertRaises(exception.MissingCredentialError, + self.man._validate_deferred_auth_context, + ctx, stack) + self.assertEqual( + 'Missing required credential: X-Auth-Key', ex.message) class StackServiceSuspendResumeTest(HeatTestCase): diff --git a/heat/tests/test_parser.py b/heat/tests/test_parser.py index ac2a71fa..a9c0908f 100644 --- a/heat/tests/test_parser.py +++ b/heat/tests/test_parser.py @@ -1717,3 +1717,18 @@ class StackTest(HeatTestCase): saved_stack = parser.Stack.load(self.ctx, stack_id=stack_ownee.id) self.assertEqual(saved_stack.owner_id, self.stack.id) + + @utils.stack_delete_after + def test_requires_deferred_auth(self): + tmpl = {'Resources': {'AResource': {'Type': 'GenericResourceType'}, + 'BResource': {'Type': 'GenericResourceType'}, + 'CResource': {'Type': 'GenericResourceType'}}} + + self.stack = parser.Stack(self.ctx, 'update_test_stack', + template.Template(tmpl), + disable_rollback=False) + + self.assertFalse(self.stack.requires_deferred_auth()) + + self.stack['CResource'].requires_deferred_auth = True + self.assertTrue(self.stack.requires_deferred_auth()) diff --git a/heat/tests/test_signal.py b/heat/tests/test_signal.py index 364ecf25..bb417e7b 100644 --- a/heat/tests/test_signal.py +++ b/heat/tests/test_signal.py @@ -164,6 +164,7 @@ class SignalTest(HeatTestCase): rsrc = self.stack.resources['signal_handler'] self.assertEqual(rsrc.state, (rsrc.CREATE, rsrc.COMPLETE)) + self.assertTrue(rsrc.requires_deferred_auth) rsrc.signal(details=test_d)