From 4e0538e2145b4db79c2489cbb1fb5e286a05ecd3 Mon Sep 17 00:00:00 2001 From: Steven Hardy Date: Fri, 5 Sep 2014 16:51:03 +0100 Subject: [PATCH] Clarify NotFound error when creating trust If a user creates a stack without the heat_stack_owner role, we try to create the trust and then fail somewhat cryptically by letting the NotFound exception from keystoneclient get exposed to the user. This is confusing, as it doesn't even mention the role name (only the ID). Instead, we can catch the NotFound and propagate a MissingCredential error, with a list of the role names we need to create the trust. This error is already correctly mapped to a bad request in the native API, but not in the CFN exception map, so add it there to avoid 500 errors if this happens via heat-api-cfn. Now, if a user lacks the required role, they will see an error like: Missing required credential: roles ['heat_stack_owner'] Which is hopefully somewhat clearer. Change-Id: Ief4956bdb76ddf0cdb0a642721b63c63b0d007d8 Closes-Bug: #1306665 --- heat/api/aws/exception.py | 1 + heat/common/heat_keystoneclient.py | 16 +++++++++----- heat/tests/test_heatclient.py | 34 ++++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 5 deletions(-) diff --git a/heat/api/aws/exception.py b/heat/api/aws/exception.py index 7fc83994d..41407581f 100644 --- a/heat/api/aws/exception.py +++ b/heat/api/aws/exception.py @@ -284,6 +284,7 @@ def map_remote_error(ex): 'UnknownUserParameter', 'UserParameterMissing', 'InvalidTemplateParameter', + 'MissingCredentialError', ) denied_errors = ('Forbidden', 'NotAuthorized') already_exists_errors = ('StackExists') diff --git a/heat/common/heat_keystoneclient.py b/heat/common/heat_keystoneclient.py index 6bf1ec8b4..e2ce7df9a 100644 --- a/heat/common/heat_keystoneclient.py +++ b/heat/common/heat_keystoneclient.py @@ -273,11 +273,17 @@ class KeystoneClientV3(object): trustor_user_id = self.client.auth_ref.user_id trustor_project_id = self.client.auth_ref.project_id roles = cfg.CONF.trusts_delegated_roles - trust = self.client.trusts.create(trustor_user=trustor_user_id, - trustee_user=trustee_user_id, - project=trustor_project_id, - impersonation=True, - role_names=roles) + try: + trust = self.client.trusts.create(trustor_user=trustor_user_id, + trustee_user=trustee_user_id, + project=trustor_project_id, + impersonation=True, + role_names=roles) + except kc_exception.NotFound: + LOG.debug("Failed to find roles %s for user %s" + % (roles, trustor_user_id)) + raise exception.MissingCredentialError( + required=_("roles %s") % roles) trust_context = context.RequestContext.from_dict( self.context.to_dict()) diff --git a/heat/tests/test_heatclient.py b/heat/tests/test_heatclient.py index af1591aa1..679dda8c6 100644 --- a/heat/tests/test_heatclient.py +++ b/heat/tests/test_heatclient.py @@ -525,6 +525,40 @@ class KeystoneClientTest(HeatTestCase): self.assertEqual('atrust123', trust_context.trust_id) self.assertEqual('5678', trust_context.trustor_user_id) + def test_create_trust_context_trust_create_norole(self): + + """Test create_trust_context when creating a trust.""" + + class MockTrust(object): + id = 'atrust123' + + self._stub_admin_client() + + self._stubs_v3() + cfg.CONF.set_override('deferred_auth_method', 'trusts') + cfg.CONF.set_override('trusts_delegated_roles', ['heat_stack_owner']) + + self.mock_ks_v3_client.auth_ref = self.m.CreateMockAnything() + self.mock_ks_v3_client.auth_ref.user_id = '5678' + self.mock_ks_v3_client.auth_ref.project_id = '42' + self.mock_ks_v3_client.trusts = self.m.CreateMockAnything() + self.mock_ks_v3_client.trusts.create( + trustor_user='5678', + trustee_user='1234', + project='42', + impersonation=True, + role_names=['heat_stack_owner']).AndRaise(kc_exception.NotFound()) + + self.m.ReplayAll() + + ctx = utils.dummy_context() + ctx.trust_id = None + heat_ks_client = heat_keystoneclient.KeystoneClient(ctx) + exc = self.assertRaises(exception.MissingCredentialError, + heat_ks_client.create_trust_context) + expected = 'Missing required credential: roles [\'heat_stack_owner\']' + self.assertIn(expected, six.text_type(exc)) + def test_init_domain_cfg_not_set_fallback(self): """Test error path when config lacks domain config."""