Merge "Remove preprov provider dependencies from CONF"
diff --git a/tempest/common/credentials_factory.py b/tempest/common/credentials_factory.py
index 38bde2e..95dcafc 100644
--- a/tempest/common/credentials_factory.py
+++ b/tempest/common/credentials_factory.py
@@ -13,6 +13,7 @@
 
 import os
 
+from oslo_concurrency import lockutils
 from oslo_log import log as logging
 from tempest_lib import auth
 
@@ -42,6 +43,29 @@
 # === Credential Providers
 
 
+# Subset of the parameters of credential providers that depend on configuration
+def _get_common_provider_params():
+    return {
+        'credentials_domain': CONF.auth.default_credentials_domain_name,
+        'admin_role': CONF.identity.admin_role
+    }
+
+
+def _get_dynamic_provider_params():
+    return _get_common_provider_params()
+
+
+def _get_preprov_provider_params():
+    _common_params = _get_common_provider_params()
+    reseller_admin_role = CONF.object_storage.reseller_admin_role
+    return dict(_common_params, **dict([
+        ('accounts_lock_dir', lockutils.get_lock_path(CONF)),
+        ('test_accounts_file', CONF.auth.test_accounts_file),
+        ('object_storage_operator_role', CONF.object_storage.operator_role),
+        ('object_storage_reseller_admin_role', reseller_admin_role)
+    ]))
+
+
 class LegacyCredentialProvider(cred_provider.CredentialProvider):
 
     def __init__(self, identity_version):
@@ -143,17 +167,15 @@
             name=name,
             network_resources=network_resources,
             identity_version=identity_version,
-            credentials_domain=CONF.auth.default_credentials_domain_name,
-            admin_role=CONF.identity.admin_role,
-            admin_creds=admin_creds)
+            admin_creds=admin_creds,
+            **_get_dynamic_provider_params())
     else:
         if (CONF.auth.test_accounts_file and
                 os.path.isfile(CONF.auth.test_accounts_file)):
             # Most params are not relevant for pre-created accounts
             return preprov_creds.PreProvisionedCredentialProvider(
                 name=name, identity_version=identity_version,
-                credentials_domain=CONF.auth.default_credentials_domain_name,
-                admin_role=CONF.identity.admin_role)
+                **_get_preprov_provider_params())
         else:
             # Dynamic credentials are disabled, and the account file is not
             # defined - we fall back on credentials configured in tempest.conf
@@ -175,7 +197,7 @@
             os.path.isfile(CONF.auth.test_accounts_file)):
         check_accounts = preprov_creds.PreProvisionedCredentialProvider(
             identity_version=identity_version, name='check_admin',
-            admin_role=CONF.identity.admin_role)
+            **_get_preprov_provider_params())
         if not check_accounts.admin_available():
             is_admin = False
     else:
@@ -201,7 +223,7 @@
             os.path.isfile(CONF.auth.test_accounts_file)):
         check_accounts = preprov_creds.PreProvisionedCredentialProvider(
             identity_version=identity_version, name='check_alt',
-            admin_role=CONF.identity.admin_role)
+            **_get_preprov_provider_params())
     else:
         check_accounts = LegacyCredentialProvider(identity_version)
     try:
diff --git a/tempest/common/preprov_creds.py b/tempest/common/preprov_creds.py
index f711302..5b81148 100644
--- a/tempest/common/preprov_creds.py
+++ b/tempest/common/preprov_creds.py
@@ -19,15 +19,14 @@
 from oslo_log import log as logging
 import six
 from tempest_lib import auth
+from tempest_lib import exceptions as lib_exc
 import yaml
 
 from tempest import clients
 from tempest.common import cred_provider
 from tempest.common import fixed_network
-from tempest import config
 from tempest import exceptions
 
-CONF = config.CONF
 LOG = logging.getLogger(__name__)
 
 
@@ -39,21 +38,52 @@
 
 class PreProvisionedCredentialProvider(cred_provider.CredentialProvider):
 
-    def __init__(self, identity_version, name=None, credentials_domain=None,
-                 admin_role=None):
+    def __init__(self, identity_version, test_accounts_file,
+                 accounts_lock_dir, name=None, credentials_domain=None,
+                 admin_role=None, object_storage_operator_role=None,
+                 object_storage_reseller_admin_role=None):
+        """Credentials provider using pre-provisioned accounts
+
+        This credentials provider loads the details of pre-provisioned
+        accounts from a YAML file, in the format specified by
+        `etc/accounts.yaml.sample`. It locks accounts while in use, using the
+        external locking mechanism, allowing for multiple python processes
+        to share a single account file, and thus running tests in parallel.
+
+        The accounts_lock_dir must be generated using `lockutils.get_lock_path`
+        from the oslo.concurrency library. For instance:
+
+            accounts_lock_dir = os.path.join(lockutils.get_lock_path(CONF),
+                                             'test_accounts')
+
+        Role names for object storage are optional as long as the
+        `operator` and `reseller_admin` credential types are not used in the
+        accounts file.
+
+        :param identity_version: identity version of the credentials
+        :param admin_role: name of the admin role
+        :param test_accounts_file: path to the accounts YAML file
+        :param accounts_lock_dir: the directory for external locking
+        :param name: name of the hash file (optional)
+        :param credentials_domain: name of the domain credentials belong to
+                                   (if no domain is configured)
+        :param object_storage_operator_role: name of the role
+        :param object_storage_reseller_admin_role: name of the role
+        """
         super(PreProvisionedCredentialProvider, self).__init__(
             identity_version=identity_version, name=name,
-            credentials_domain=credentials_domain, admin_role=admin_role)
-        if (CONF.auth.test_accounts_file and
-                os.path.isfile(CONF.auth.test_accounts_file)):
-            accounts = read_accounts_yaml(CONF.auth.test_accounts_file)
+            admin_role=admin_role, credentials_domain=credentials_domain)
+        self.test_accounts_file = test_accounts_file
+        if test_accounts_file and os.path.isfile(test_accounts_file):
+            accounts = read_accounts_yaml(self.test_accounts_file)
             self.use_default_creds = False
         else:
             accounts = {}
             self.use_default_creds = True
-        self.hash_dict = self.get_hash_dict(accounts, admin_role)
-        self.accounts_dir = os.path.join(lockutils.get_lock_path(CONF),
-                                         'test_accounts')
+        self.hash_dict = self.get_hash_dict(
+            accounts, admin_role, object_storage_operator_role,
+            object_storage_reseller_admin_role)
+        self.accounts_dir = accounts_lock_dir
         self._creds = {}
 
     @classmethod
@@ -65,7 +95,9 @@
         return hash_dict
 
     @classmethod
-    def get_hash_dict(cls, accounts, admin_role):
+    def get_hash_dict(cls, accounts, admin_role,
+                      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:
@@ -92,14 +124,24 @@
                     hash_dict = cls._append_role(admin_role, temp_hash_key,
                                                  hash_dict)
                 elif type == 'operator':
-                    hash_dict = cls._append_role(
-                        CONF.object_storage.operator_role, temp_hash_key,
-                        hash_dict)
+                    if object_storage_operator_role:
+                        hash_dict = cls._append_role(
+                            object_storage_operator_role, temp_hash_key,
+                            hash_dict)
+                    else:
+                        msg = ("Type 'operator' configured, but no "
+                               "object_storage_operator_role specified")
+                        raise lib_exc.InvalidCredentials(msg)
                 elif type == 'reseller_admin':
-                    hash_dict = cls._append_role(
-                        CONF.object_storage.reseller_admin_role,
-                        temp_hash_key,
-                        hash_dict)
+                    if object_storage_reseller_admin_role:
+                        hash_dict = cls._append_role(
+                            object_storage_reseller_admin_role,
+                            temp_hash_key,
+                            hash_dict)
+                    else:
+                        msg = ("Type 'reseller_admin' configured, but no "
+                               "object_storage_reseller_admin_role specified")
+                        raise lib_exc.InvalidCredentials(msg)
             # Populate the network subdict
             for resource in resources:
                 if resource == 'network':
@@ -112,8 +154,8 @@
     def is_multi_user(self):
         # Default credentials is not a valid option with locking Account
         if self.use_default_creds:
-            raise exceptions.InvalidConfiguration(
-                "Account file %s doesn't exist" % CONF.auth.test_accounts_file)
+            raise lib_exc.InvalidCredentials(
+                "Account file %s doesn't exist" % self.test_accounts_file)
         else:
             return len(self.hash_dict['creds']) > 1
 
@@ -149,7 +191,7 @@
                     names.append(fd.read())
         msg = ('Insufficient number of users provided. %s have allocated all '
                'the credentials for this allocation request' % ','.join(names))
-        raise exceptions.InvalidConfiguration(msg)
+        raise lib_exc.InvalidCredentials(msg)
 
     def _get_match_hash_list(self, roles=None):
         hashes = []
@@ -159,7 +201,7 @@
             for role in roles:
                 temp_hashes = self.hash_dict['roles'].get(role, None)
                 if not temp_hashes:
-                    raise exceptions.InvalidConfiguration(
+                    raise lib_exc.InvalidCredentials(
                         "No credentials with role: %s specified in the "
                         "accounts ""file" % role)
                 hashes.append(temp_hashes)
@@ -191,8 +233,8 @@
 
     def _get_creds(self, roles=None):
         if self.use_default_creds:
-            raise exceptions.InvalidConfiguration(
-                "Account file %s doesn't exist" % CONF.auth.test_accounts_file)
+            raise lib_exc.InvalidCredentials(
+                "Account file %s doesn't exist" % self.test_accounts_file)
         useable_hashes = self._get_match_hash_list(roles)
         free_hash = self._get_free_hash(useable_hashes)
         clean_creds = self._sanitize_creds(
diff --git a/tempest/tests/common/test_preprov_creds.py b/tempest/tests/common/test_preprov_creds.py
index 36dd976..2e0f82a 100644
--- a/tempest/tests/common/test_preprov_creds.py
+++ b/tempest/tests/common/test_preprov_creds.py
@@ -17,17 +17,17 @@
 
 import mock
 from oslo_concurrency.fixture import lockutils as lockutils_fixtures
-from oslo_concurrency import lockutils
 from oslo_config import cfg
 from oslotest import mockpatch
+import shutil
 import six
 from tempest_lib import auth
+from tempest_lib import exceptions as lib_exc
 from tempest_lib.services.identity.v2 import token_client
 
 from tempest.common import cred_provider
 from tempest.common import preprov_creds
 from tempest import config
-from tempest import exceptions
 from tempest.tests import base
 from tempest.tests import fake_config
 from tempest.tests import fake_http
@@ -38,7 +38,11 @@
 
     fixed_params = {'name': 'test class',
                     'identity_version': 'v2',
-                    'admin_role': 'admin'}
+                    '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'}
 
     def setUp(self):
         super(TestPreProvisionedCredentials, self).setUp()
@@ -77,9 +81,13 @@
         self.accounts_mock = self.useFixture(mockpatch.Patch(
             'tempest.common.preprov_creds.read_accounts_yaml',
             return_value=self.test_accounts))
-        cfg.CONF.set_default('test_accounts_file', 'fake_path', group='auth')
         self.useFixture(mockpatch.Patch('os.path.isfile', return_value=True))
 
+    def tearDown(self):
+        super(TestPreProvisionedCredentials, self).tearDown()
+        shutil.rmtree(self.fixed_params['accounts_lock_dir'],
+                      ignore_errors=True)
+
     def _get_hash_list(self, accounts_list):
         hash_list = []
         for account in accounts_list:
@@ -147,11 +155,10 @@
         with mock.patch('six.moves.builtins.open', mock.mock_open(),
                         create=True) as open_mock:
             test_account_class._get_free_hash(hash_list)
-            lock_path = os.path.join(lockutils.get_lock_path(
-                preprov_creds.CONF), 'test_accounts', hash_list[0])
+            lock_path = os.path.join(self.fixed_params['accounts_lock_dir'],
+                                     hash_list[0])
             open_mock.assert_called_once_with(lock_path, 'w')
-        mkdir_path = os.path.join(
-            preprov_creds.CONF.oslo_concurrency.lock_path, 'test_accounts')
+        mkdir_path = os.path.join(self.fixed_params['accounts_lock_dir'])
         mkdir_mock.mock.assert_called_once_with(mkdir_path)
 
     @mock.patch('oslo_concurrency.lockutils.lock')
@@ -165,7 +172,7 @@
             **self.fixed_params)
         with mock.patch('six.moves.builtins.open', mock.mock_open(),
                         create=True):
-            self.assertRaises(exceptions.InvalidConfiguration,
+            self.assertRaises(lib_exc.InvalidCredentials,
                               test_account_class._get_free_hash, hash_list)
 
     @mock.patch('oslo_concurrency.lockutils.lock')
@@ -187,9 +194,8 @@
         with mock.patch('six.moves.builtins.open', mock.mock_open(),
                         create=True) as open_mock:
             test_account_class._get_free_hash(hash_list)
-            lock_path = os.path.join(
-                lockutils.get_lock_path(preprov_creds.CONF),
-                'test_accounts', hash_list[3])
+            lock_path = os.path.join(self.fixed_params['accounts_lock_dir'],
+                                     hash_list[3])
             open_mock.assert_has_calls([mock.call(lock_path, 'w')])
 
     @mock.patch('oslo_concurrency.lockutils.lock')
@@ -204,11 +210,9 @@
         remove_mock = self.useFixture(mockpatch.Patch('os.remove'))
         rmdir_mock = self.useFixture(mockpatch.Patch('os.rmdir'))
         test_account_class.remove_hash(hash_list[2])
-        hash_path = os.path.join(lockutils.get_lock_path(preprov_creds.CONF),
-                                 'test_accounts',
+        hash_path = os.path.join(self.fixed_params['accounts_lock_dir'],
                                  hash_list[2])
-        lock_path = os.path.join(preprov_creds.CONF.oslo_concurrency.lock_path,
-                                 'test_accounts')
+        lock_path = self.fixed_params['accounts_lock_dir']
         remove_mock.mock.assert_called_once_with(hash_path)
         rmdir_mock.mock.assert_called_once_with(lock_path)
 
@@ -225,8 +229,7 @@
         remove_mock = self.useFixture(mockpatch.Patch('os.remove'))
         rmdir_mock = self.useFixture(mockpatch.Patch('os.rmdir'))
         test_account_class.remove_hash(hash_list[2])
-        hash_path = os.path.join(lockutils.get_lock_path(preprov_creds.CONF),
-                                 'test_accounts',
+        hash_path = os.path.join(self.fixed_params['accounts_lock_dir'],
                                  hash_list[2])
         remove_mock.mock.assert_called_once_with(hash_path)
         rmdir_mock.mock.assert_not_called()