Hash credentials on user, project/tenant and pwd

Preprovision credential provider hashes credentials based on all
fields specified in the YAML. The same configured credentials can
be used to build both v2 and v3 credential objects, so we need to
hash on the fields that are common between v2 and v3 only.

Because v2 only understand tenants (and not project) the
intersection would be only user and password. Because of that, and
because we want to promote project against tenant, accept
project in v2 credentials as well, by translating it to tenant at
__init__ time.

Change-Id: Ib62c26cdffc2db6f6352d9889c689db3ff09aa5d

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,

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

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)

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'}
 
-    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.useFixture(lockutils_fixtures.ExternalLockFixture())
-        self.test_accounts = [
+    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', 'tenant_name': 'test_tenant2',
+            {'username': 'test_user2', 'project_name': 'test_tenant2',
              'password': 'p'},
             {'username': 'test_user3', 'tenant_name': 'test_tenant3',
              'password': 'p'},
-            {'username': 'test_user4', 'tenant_name': 'test_tenant4',
+            {'username': 'test_user4', 'project_name': 'test_tenant4',
              'password': 'p'},
             {'username': 'test_user5', 'tenant_name': 'test_tenant5',
              'password': 'p'},
-            {'username': 'test_user6', 'tenant_name': 'test_tenant6',
+            {'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', 'tenant_name': 'test_tenant8',
+            {'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', 'tenant_name': 'test_tenant10',
+            {'username': 'test_user10', 'project_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]},
-        ]
+             '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.patch(self.token_client, side_effect=self.identity_response)
+        self.useFixture(lockutils_fixtures.ExternalLockFixture())
+        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]}]

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))