diff --git a/tempest/common/credentials_factory.py b/tempest/common/credentials_factory.py index 7c73ada430..82db9cc05a 100644 --- a/tempest/common/credentials_factory.py +++ b/tempest/common/credentials_factory.py @@ -186,11 +186,6 @@ def get_configured_credentials(credential_type, fill_in=True, params[attr] = getattr(_section, attr) else: params[attr] = getattr(_section, prefix + "_" + attr) - # NOTE(andreaf) v2 API still uses tenants, so we must translate project - # to tenant before building the Credentials object - if identity_version == 'v2': - params['tenant_name'] = params.get('project_name') - params.pop('project_name', None) # Build and validate credentials. We are reading configured credentials, # so validate them even if fill_in is False credentials = get_credentials(fill_in=fill_in, diff --git a/tempest/common/preprov_creds.py b/tempest/common/preprov_creds.py index 51f723b106..7350222a89 100644 --- a/tempest/common/preprov_creds.py +++ b/tempest/common/preprov_creds.py @@ -43,6 +43,11 @@ def read_accounts_yaml(path): class PreProvisionedCredentialProvider(cred_provider.CredentialProvider): + # Exclude from the hash fields specific to v2 or v3 identity API + # i.e. only include user*, project*, tenant* and password + HASH_CRED_FIELDS = (set(auth.KeystoneV2Credentials.ATTRIBUTES) & + set(auth.KeystoneV3Credentials.ATTRIBUTES)) + def __init__(self, identity_version, test_accounts_file, accounts_lock_dir, name=None, credentials_domain=None, admin_role=None, object_storage_operator_role=None, @@ -104,6 +109,7 @@ class PreProvisionedCredentialProvider(cred_provider.CredentialProvider): object_storage_operator_role=None, object_storage_reseller_admin_role=None): hash_dict = {'roles': {}, 'creds': {}, 'networks': {}} + # Loop over the accounts read from the yaml file for account in accounts: roles = [] @@ -116,7 +122,9 @@ class PreProvisionedCredentialProvider(cred_provider.CredentialProvider): if 'resources' in account: resources = account.pop('resources') temp_hash = hashlib.md5() - temp_hash.update(six.text_type(account).encode('utf-8')) + account_for_hash = dict((k, v) for (k, v) in six.iteritems(account) + if k in cls.HASH_CRED_FIELDS) + temp_hash.update(six.text_type(account_for_hash).encode('utf-8')) temp_hash_key = temp_hash.hexdigest() hash_dict['creds'][temp_hash_key] = account for role in roles: @@ -262,13 +270,13 @@ class PreProvisionedCredentialProvider(cred_provider.CredentialProvider): for _hash in self.hash_dict['creds']: # Comparing on the attributes that are expected in the YAML init_attributes = creds.get_init_attributes() + # Only use the attributes initially used to calculate the hash + init_attributes = [x for x in init_attributes if + x in self.HASH_CRED_FIELDS] hash_attributes = self.hash_dict['creds'][_hash].copy() - if ('user_domain_name' in init_attributes and 'user_domain_name' - not in hash_attributes): - # Allow for the case of domain_name populated from config - domain_name = self.credentials_domain - hash_attributes['user_domain_name'] = domain_name - if all([getattr(creds, k) == hash_attributes[k] for + # NOTE(andreaf) Not all fields may be available on all credentials + # so defaulting to None for that case. + if all([getattr(creds, k, None) == hash_attributes.get(k, None) for k in init_attributes]): return _hash raise AttributeError('Invalid credentials %s' % creds) @@ -351,23 +359,20 @@ class PreProvisionedCredentialProvider(cred_provider.CredentialProvider): return net_creds def _extend_credentials(self, creds_dict): - # In case of v3, adds a user_domain_name field to the creds - # dict if not defined + # Add or remove credential domain fields to fit the identity version + domain_fields = set(x for x in auth.KeystoneV3Credentials.ATTRIBUTES + if 'domain' in x) + msg = 'Assuming they are valid in the default domain.' if self.identity_version == 'v3': - user_domain_fields = set(['user_domain_name', 'user_domain_id']) - if not user_domain_fields.intersection(set(creds_dict.keys())): - creds_dict['user_domain_name'] = self.credentials_domain - # NOTE(andreaf) In case of v2, replace project with tenant if project - # is provided and tenant is not + if not domain_fields.intersection(set(creds_dict.keys())): + msg = 'Using credentials %s for v3 API calls. ' + msg + LOG.warning(msg, self._sanitize_creds(creds_dict)) + creds_dict['domain_name'] = self.credentials_domain if self.identity_version == 'v2': - if ('project_name' in creds_dict and - 'tenant_name' in creds_dict and - creds_dict['project_name'] != creds_dict['tenant_name']): - clean_creds = self._sanitize_creds(creds_dict) - msg = 'Cannot specify project and tenant at the same time %s' - raise exceptions.InvalidCredentials(msg % clean_creds) - if ('project_name' in creds_dict and - 'tenant_name' not in creds_dict): - creds_dict['tenant_name'] = creds_dict['project_name'] - creds_dict.pop('project_name') + if domain_fields.intersection(set(creds_dict.keys())): + msg = 'Using credentials %s for v2 API calls. ' + msg + LOG.warning(msg, self._sanitize_creds(creds_dict)) + # Remove all valid domain attributes + for attr in domain_fields.intersection(set(creds_dict.keys())): + creds_dict.pop(attr) return creds_dict diff --git a/tempest/lib/auth.py b/tempest/lib/auth.py index a6833bede3..c2ccf7757a 100644 --- a/tempest/lib/auth.py +++ b/tempest/lib/auth.py @@ -532,6 +532,7 @@ class Credentials(object): """ ATTRIBUTES = [] + COLLISIONS = [] def __init__(self, **kwargs): """Enforce the available attributes at init time (only). @@ -543,6 +544,13 @@ class Credentials(object): self._apply_credentials(kwargs) def _apply_credentials(self, attr): + for (key1, key2) in self.COLLISIONS: + val1 = attr.get(key1) + val2 = attr.get(key2) + if val1 and val2 and val1 != val2: + msg = ('Cannot have conflicting values for %s and %s' % + (key1, key2)) + raise exceptions.InvalidCredentials(msg) for key in attr.keys(): if key in self.ATTRIBUTES: setattr(self, key, attr[key]) @@ -600,7 +608,33 @@ class Credentials(object): class KeystoneV2Credentials(Credentials): ATTRIBUTES = ['username', 'password', 'tenant_name', 'user_id', - 'tenant_id'] + 'tenant_id', 'project_id', 'project_name'] + COLLISIONS = [('project_name', 'tenant_name'), ('project_id', 'tenant_id')] + + def __str__(self): + """Represent only attributes included in self.ATTRIBUTES""" + attrs = [attr for attr in self.ATTRIBUTES if attr is not 'password'] + _repr = dict((k, getattr(self, k)) for k in attrs) + return str(_repr) + + def __setattr__(self, key, value): + # NOTE(andreaf) In order to ease the migration towards 'project' we + # support v2 credentials configured with 'project' and translate it + # to tenant on the fly. The original kwargs are stored for clients + # that may rely on them. We also set project when tenant is defined + # so clients can rely on project being part of credentials. + parent = super(KeystoneV2Credentials, self) + # for project_* set tenant only + if key == 'project_id': + parent.__setattr__('tenant_id', value) + elif key == 'project_name': + parent.__setattr__('tenant_name', value) + if key == 'tenant_id': + parent.__setattr__('project_id', value) + elif key == 'tenant_name': + parent.__setattr__('project_name', value) + # trigger default behaviour for all attributes + parent.__setattr__(key, value) def is_valid(self): """Check of credentials (no API call) @@ -611,9 +645,6 @@ class KeystoneV2Credentials(Credentials): return None not in (self.username, self.password) -COLLISIONS = [('project_name', 'tenant_name'), ('project_id', 'tenant_id')] - - class KeystoneV3Credentials(Credentials): """Credentials suitable for the Keystone Identity V3 API""" @@ -621,16 +652,7 @@ class KeystoneV3Credentials(Credentials): 'project_domain_id', 'project_domain_name', 'project_id', 'project_name', 'tenant_id', 'tenant_name', 'user_domain_id', 'user_domain_name', 'user_id'] - - def _apply_credentials(self, attr): - for (key1, key2) in COLLISIONS: - val1 = attr.get(key1) - val2 = attr.get(key2) - if val1 and val2 and val1 != val2: - msg = ('Cannot have conflicting values for %s and %s' % - (key1, key2)) - raise exceptions.InvalidCredentials(msg) - super(KeystoneV3Credentials, self)._apply_credentials(attr) + COLLISIONS = [('project_name', 'tenant_name'), ('project_id', 'tenant_id')] def __setattr__(self, key, value): parent = super(KeystoneV3Credentials, self) diff --git a/tempest/tests/common/test_preprov_creds.py b/tempest/tests/common/test_preprov_creds.py index b595c8889b..22bbdd3bfb 100644 --- a/tempest/tests/common/test_preprov_creds.py +++ b/tempest/tests/common/test_preprov_creds.py @@ -27,7 +27,6 @@ from tempest.common import preprov_creds from tempest import config from tempest.lib import auth from tempest.lib import exceptions as lib_exc -from tempest.lib.services.identity.v2 import token_client from tempest.tests import base from tempest.tests import fake_config from tempest.tests.lib import fake_identity @@ -43,40 +42,46 @@ class TestPreProvisionedCredentials(base.TestCase): 'object_storage_operator_role': 'operator', 'object_storage_reseller_admin_role': 'reseller'} + identity_response = fake_identity._fake_v2_response + token_client = ('tempest.lib.services.identity.v2.token_client' + '.TokenClient.raw_request') + + @classmethod + def _fake_accounts(cls, admin_role): + return [ + {'username': 'test_user1', 'tenant_name': 'test_tenant1', + 'password': 'p'}, + {'username': 'test_user2', 'project_name': 'test_tenant2', + 'password': 'p'}, + {'username': 'test_user3', 'tenant_name': 'test_tenant3', + 'password': 'p'}, + {'username': 'test_user4', 'project_name': 'test_tenant4', + 'password': 'p'}, + {'username': 'test_user5', 'tenant_name': 'test_tenant5', + 'password': 'p'}, + {'username': 'test_user6', 'project_name': 'test_tenant6', + 'password': 'p', 'roles': ['role1', 'role2']}, + {'username': 'test_user7', 'tenant_name': 'test_tenant7', + 'password': 'p', 'roles': ['role2', 'role3']}, + {'username': 'test_user8', 'project_name': 'test_tenant8', + 'password': 'p', 'roles': ['role4', 'role1']}, + {'username': 'test_user9', 'tenant_name': 'test_tenant9', + 'password': 'p', 'roles': ['role1', 'role2', 'role3', 'role4']}, + {'username': 'test_user10', 'project_name': 'test_tenant10', + 'password': 'p', 'roles': ['role1', 'role2', 'role3', 'role4']}, + {'username': 'test_user11', 'tenant_name': 'test_tenant11', + 'password': 'p', 'roles': [admin_role]}, + {'username': 'test_user12', 'project_name': 'test_tenant12', + 'password': 'p', 'roles': [admin_role]}] + def setUp(self): super(TestPreProvisionedCredentials, self).setUp() self.useFixture(fake_config.ConfigFixture()) self.patchobject(config, 'TempestConfigPrivate', fake_config.FakePrivate) - self.patchobject(token_client.TokenClient, 'raw_request', - fake_identity._fake_v2_response) + self.patch(self.token_client, side_effect=self.identity_response) self.useFixture(lockutils_fixtures.ExternalLockFixture()) - self.test_accounts = [ - {'username': 'test_user1', 'tenant_name': 'test_tenant1', - 'password': 'p'}, - {'username': 'test_user2', 'tenant_name': 'test_tenant2', - 'password': 'p'}, - {'username': 'test_user3', 'tenant_name': 'test_tenant3', - 'password': 'p'}, - {'username': 'test_user4', 'tenant_name': 'test_tenant4', - 'password': 'p'}, - {'username': 'test_user5', 'tenant_name': 'test_tenant5', - 'password': 'p'}, - {'username': 'test_user6', 'tenant_name': 'test_tenant6', - 'password': 'p', 'roles': ['role1', 'role2']}, - {'username': 'test_user7', 'tenant_name': 'test_tenant7', - 'password': 'p', 'roles': ['role2', 'role3']}, - {'username': 'test_user8', 'tenant_name': 'test_tenant8', - 'password': 'p', 'roles': ['role4', 'role1']}, - {'username': 'test_user9', 'tenant_name': 'test_tenant9', - 'password': 'p', 'roles': ['role1', 'role2', 'role3', 'role4']}, - {'username': 'test_user10', 'tenant_name': 'test_tenant10', - 'password': 'p', 'roles': ['role1', 'role2', 'role3', 'role4']}, - {'username': 'test_user11', 'tenant_name': 'test_tenant11', - 'password': 'p', 'roles': [cfg.CONF.identity.admin_role]}, - {'username': 'test_user12', 'tenant_name': 'test_tenant12', - 'password': 'p', 'roles': [cfg.CONF.identity.admin_role]}, - ] + self.test_accounts = self._fake_accounts(cfg.CONF.identity.admin_role) self.accounts_mock = self.useFixture(mockpatch.Patch( 'tempest.common.preprov_creds.read_accounts_yaml', return_value=self.test_accounts)) @@ -89,24 +94,33 @@ class TestPreProvisionedCredentials(base.TestCase): def _get_hash_list(self, accounts_list): hash_list = [] + hash_fields = ( + preprov_creds.PreProvisionedCredentialProvider.HASH_CRED_FIELDS) for account in accounts_list: hash = hashlib.md5() - hash.update(six.text_type(account).encode('utf-8')) + account_for_hash = dict((k, v) for (k, v) in six.iteritems(account) + if k in hash_fields) + hash.update(six.text_type(account_for_hash).encode('utf-8')) temp_hash = hash.hexdigest() hash_list.append(temp_hash) return hash_list def test_get_hash(self): - self.patchobject(token_client.TokenClient, 'raw_request', - fake_identity._fake_v2_response) - test_account_class = preprov_creds.PreProvisionedCredentialProvider( - **self.fixed_params) - hash_list = self._get_hash_list(self.test_accounts) - test_cred_dict = self.test_accounts[3] - test_creds = auth.get_credentials(fake_identity.FAKE_AUTH_URL, - **test_cred_dict) - results = test_account_class.get_hash(test_creds) - self.assertEqual(hash_list[3], results) + # Test with all accounts to make sure we try all combinations + # and hide no race conditions + hash_index = 0 + for test_cred_dict in self.test_accounts: + test_account_class = ( + preprov_creds.PreProvisionedCredentialProvider( + **self.fixed_params)) + hash_list = self._get_hash_list(self.test_accounts) + test_creds = auth.get_credentials( + fake_identity.FAKE_AUTH_URL, + identity_version=self.fixed_params['identity_version'], + **test_cred_dict) + results = test_account_class.get_hash(test_creds) + self.assertEqual(hash_list[hash_index], results) + hash_index += 1 def test_get_hash_dict(self): test_account_class = preprov_creds.PreProvisionedCredentialProvider( @@ -331,3 +345,53 @@ class TestPreProvisionedCredentials(base.TestCase): self.assertIn('id', network) self.assertEqual('fake-id', network['id']) self.assertEqual('network-2', network['name']) + + +class TestPreProvisionedCredentialsV3(TestPreProvisionedCredentials): + + fixed_params = {'name': 'test class', + 'identity_version': 'v3', + 'test_accounts_file': 'fake_accounts_file', + 'accounts_lock_dir': 'fake_locks_dir', + 'admin_role': 'admin', + 'object_storage_operator_role': 'operator', + 'object_storage_reseller_admin_role': 'reseller'} + + identity_response = fake_identity._fake_v3_response + token_client = ('tempest.lib.services.identity.v3.token_client' + '.V3TokenClient.raw_request') + + @classmethod + def _fake_accounts(cls, admin_role): + return [ + {'username': 'test_user1', 'project_name': 'test_project1', + 'domain_name': 'domain', 'password': 'p'}, + {'username': 'test_user2', 'project_name': 'test_project2', + 'domain_name': 'domain', 'password': 'p'}, + {'username': 'test_user3', 'project_name': 'test_project3', + 'domain_name': 'domain', 'password': 'p'}, + {'username': 'test_user4', 'project_name': 'test_project4', + 'domain_name': 'domain', 'password': 'p'}, + {'username': 'test_user5', 'project_name': 'test_project5', + 'domain_name': 'domain', 'password': 'p'}, + {'username': 'test_user6', 'project_name': 'test_project6', + 'domain_name': 'domain', 'password': 'p', + 'roles': ['role1', 'role2']}, + {'username': 'test_user7', 'project_name': 'test_project7', + 'domain_name': 'domain', 'password': 'p', + 'roles': ['role2', 'role3']}, + {'username': 'test_user8', 'project_name': 'test_project8', + 'domain_name': 'domain', 'password': 'p', + 'roles': ['role4', 'role1']}, + {'username': 'test_user9', 'project_name': 'test_project9', + 'domain_name': 'domain', 'password': 'p', + 'roles': ['role1', 'role2', 'role3', 'role4']}, + {'username': 'test_user10', 'project_name': 'test_project10', + 'domain_name': 'domain', 'password': 'p', + 'roles': ['role1', 'role2', 'role3', 'role4']}, + {'username': 'test_user11', 'project_name': 'test_project11', + 'domain_name': 'domain', 'password': 'p', + 'roles': [admin_role]}, + {'username': 'test_user12', 'project_name': 'test_project12', + 'domain_name': 'domain', 'password': 'p', + 'roles': [admin_role]}] diff --git a/tempest/tests/lib/test_credentials.py b/tempest/tests/lib/test_credentials.py index ca3baa135c..b6f2cf602a 100644 --- a/tempest/tests/lib/test_credentials.py +++ b/tempest/tests/lib/test_credentials.py @@ -36,8 +36,10 @@ class CredentialsTests(base.TestCase): # Check the right version of credentials has been returned self.assertIsInstance(credentials, credentials_class) # Check the id attributes are filled in + # NOTE(andreaf) project_* attributes are accepted as input but + # never set on the credentials object attributes = [x for x in credentials.ATTRIBUTES if ( - '_id' in x and x != 'domain_id')] + '_id' in x and x != 'domain_id' and x != 'project_id')] for attr in attributes: if filled: self.assertIsNotNone(getattr(credentials, attr))