]> review.fuel-infra Code Review - openstack-build/heat-build.git/commitdiff
Only validate credentials on create based on resources
authorSteve Baker <sbaker@redhat.com>
Fri, 30 Aug 2013 05:21:56 +0000 (17:21 +1200)
committerSteve Baker <sbaker@redhat.com>
Thu, 5 Sep 2013 23:42:36 +0000 (11:42 +1200)
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

heat/engine/parser.py
heat/engine/resource.py
heat/engine/service.py
heat/engine/signal_responder.py
heat/tests/test_engine_service.py
heat/tests/test_parser.py
heat/tests/test_signal.py

index f715a6c0c5f93de9e54ddab149c61c8cc481f807..492d2c3bfd4ce40451a01bd45f8c8472c7a89101 100644 (file)
@@ -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:
index 39d5562cf93dd78fb16dad71bf55be46c579b5f5..87bcb9c71596325f605ed5b6d05ba6de000cdf8f 100644 (file)
@@ -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.'''
 
index 5b35bdcb004f70d242a976f381c106d82c169b2f..61a414b78cc0cee4a4be507f489eb13efeb24f82 100644 (file)
@@ -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)
index 8d005fcab7551e615be915ecbab4973510fec88f..694d8caca9d9c1dd3046ebb74cc44785ae74558e 100644 (file)
@@ -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(
index bd60bbd3c1ff87eadf08b097f87b789e31b382c5..ea40369eafc6c53c532cf48fdf5ffadd1795ebff 100644 (file)
@@ -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):
index ac2a71fa7746c2c8f13b7b8e8f6bd4489c274607..a9c0908f8f442549bca59d94a788aad9da4fe0cb 100644 (file)
@@ -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())
index 364ecf253cb0fe715bbe8ee9894479a54bef3ad7..bb417e7b4d5a24558cba4f3e625389ba27f85c28 100644 (file)
@@ -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)