From 848c4a14885c3501789271ceb4a03202855662c3 Mon Sep 17 00:00:00 2001 From: "Andrea Frittoli (andreaf)" Date: Thu, 9 Jun 2016 11:09:02 +0100 Subject: [PATCH] Stop passing TestResources to Managers Dynamic credential providers generate TestResource objects, which behave like Credentials but also include network resources. We used to pass those directly into client managers, but that has several issues: the interface of the client manager is confusing, because it accepts two different type of objects in one parameter. Having the client manager depending on the dynamic credential provider is a problem because in turn that depends on the client manager, which may lead to circular dependencies when trying to make those stable interfaces for tempest.lib. Finally TestResources seem to confused developers, since a number of tests ignored the possibility of getting Credential fields from TestResources directly, and went through the inner credentials atttribute. I think the best approach for now is to pass auth.Credentials objects only into the client manager. The client manager does not use the network resources anyways. We plan to refactor the link between credential provider and network resources anyways, so it's better to have that outside of client managers. Change-Id: I71d4587bf0b640d05d0f78fb84364ce4d2d021f3 Partially-implements: bp client-manager-refactor --- .../api/identity/v2/test_ec2_credentials.py | 40 +++++++++---------- tempest/api/identity/v2/test_tenants.py | 2 +- tempest/api/identity/v2/test_tokens.py | 4 +- tempest/api/identity/v2/test_users.py | 4 +- tempest/api/identity/v3/test_projects.py | 2 +- tempest/api/identity/v3/test_users.py | 4 +- tempest/cmd/verify_tempest_config.py | 2 +- tempest/manager.py | 13 ++---- tempest/scenario/utils.py | 3 +- tempest/test.py | 5 ++- tempest/tests/test_base_test.py | 2 +- 11 files changed, 38 insertions(+), 43 deletions(-) diff --git a/tempest/api/identity/v2/test_ec2_credentials.py b/tempest/api/identity/v2/test_ec2_credentials.py index 860098039..5902196b7 100644 --- a/tempest/api/identity/v2/test_ec2_credentials.py +++ b/tempest/api/identity/v2/test_ec2_credentials.py @@ -36,16 +36,16 @@ class EC2CredentialsTest(base.BaseIdentityV2Test): def test_create_ec2_credentials(self): """Create user ec2 credentials.""" resp = self.non_admin_users_client.create_user_ec2_credentials( - self.creds.credentials.user_id, - tenant_id=self.creds.credentials.tenant_id)["credential"] + self.creds.user_id, + tenant_id=self.creds.tenant_id)["credential"] access = resp['access'] self.addCleanup( self.non_admin_users_client.delete_user_ec2_credentials, - self.creds.credentials.user_id, access) + self.creds.user_id, access) self.assertNotEmpty(resp['access']) self.assertNotEmpty(resp['secret']) - self.assertEqual(self.creds.credentials.user_id, resp['user_id']) - self.assertEqual(self.creds.credentials.tenant_id, resp['tenant_id']) + self.assertEqual(self.creds.user_id, resp['user_id']) + self.assertEqual(self.creds.tenant_id, resp['tenant_id']) @test.idempotent_id('9e2ea42f-0a4f-468c-a768-51859ce492e0') def test_list_ec2_credentials(self): @@ -54,24 +54,24 @@ class EC2CredentialsTest(base.BaseIdentityV2Test): fetched_creds = [] # create first ec2 credentials creds1 = self.non_admin_users_client.create_user_ec2_credentials( - self.creds.credentials.user_id, - tenant_id=self.creds.credentials.tenant_id)["credential"] + self.creds.user_id, + tenant_id=self.creds.tenant_id)["credential"] created_creds.append(creds1['access']) # create second ec2 credentials creds2 = self.non_admin_users_client.create_user_ec2_credentials( - self.creds.credentials.user_id, - tenant_id=self.creds.credentials.tenant_id)["credential"] + self.creds.user_id, + tenant_id=self.creds.tenant_id)["credential"] created_creds.append(creds2['access']) # add credentials to be cleaned up self.addCleanup( self.non_admin_users_client.delete_user_ec2_credentials, - self.creds.credentials.user_id, creds1['access']) + self.creds.user_id, creds1['access']) self.addCleanup( self.non_admin_users_client.delete_user_ec2_credentials, - self.creds.credentials.user_id, creds2['access']) + self.creds.user_id, creds2['access']) # get the list of user ec2 credentials resp = self.non_admin_users_client.list_user_ec2_credentials( - self.creds.credentials.user_id)["credentials"] + self.creds.user_id)["credentials"] fetched_creds = [cred['access'] for cred in resp] # created credentials should be in a fetched list missing = [cred for cred in created_creds @@ -84,14 +84,14 @@ class EC2CredentialsTest(base.BaseIdentityV2Test): def test_show_ec2_credentials(self): """Get the definite user ec2 credentials.""" resp = self.non_admin_users_client.create_user_ec2_credentials( - self.creds.credentials.user_id, - tenant_id=self.creds.credentials.tenant_id)["credential"] + self.creds.user_id, + tenant_id=self.creds.tenant_id)["credential"] self.addCleanup( self.non_admin_users_client.delete_user_ec2_credentials, - self.creds.credentials.user_id, resp['access']) + self.creds.user_id, resp['access']) ec2_creds = self.non_admin_users_client.show_user_ec2_credentials( - self.creds.credentials.user_id, resp['access'] + self.creds.user_id, resp['access'] )["credential"] for key in ['access', 'secret', 'user_id', 'tenant_id']: self.assertEqual(ec2_creds[key], resp[key]) @@ -100,13 +100,13 @@ class EC2CredentialsTest(base.BaseIdentityV2Test): def test_delete_ec2_credentials(self): """Delete user ec2 credentials.""" resp = self.non_admin_users_client.create_user_ec2_credentials( - self.creds.credentials.user_id, - tenant_id=self.creds.credentials.tenant_id)["credential"] + self.creds.user_id, + tenant_id=self.creds.tenant_id)["credential"] access = resp['access'] self.non_admin_users_client.delete_user_ec2_credentials( - self.creds.credentials.user_id, access) + self.creds.user_id, access) self.assertRaises( lib_exc.NotFound, self.non_admin_users_client.show_user_ec2_credentials, - self.creds.credentials.user_id, + self.creds.user_id, access) diff --git a/tempest/api/identity/v2/test_tenants.py b/tempest/api/identity/v2/test_tenants.py index b742e692d..cc6de4713 100644 --- a/tempest/api/identity/v2/test_tenants.py +++ b/tempest/api/identity/v2/test_tenants.py @@ -24,7 +24,7 @@ class IdentityTenantsTest(base.BaseIdentityV2Test): @test.idempotent_id('ecae2459-243d-4ba1-ad02-65f15dc82b78') def test_list_tenants_returns_only_authorized_tenants(self): - alt_tenant_name = self.alt_manager.credentials.credentials.tenant_name + alt_tenant_name = self.alt_manager.credentials.tenant_name resp = self.non_admin_tenants_client.list_tenants() # check that user can see only that tenants that he presents in so user diff --git a/tempest/api/identity/v2/test_tokens.py b/tempest/api/identity/v2/test_tokens.py index 3b508f420..bdca1e070 100644 --- a/tempest/api/identity/v2/test_tokens.py +++ b/tempest/api/identity/v2/test_tokens.py @@ -43,8 +43,8 @@ class TokensTest(base.BaseIdentityV2Test): self.assertGreater(expires_at, now) self.assertEqual(body['token']['tenant']['id'], - creds.credentials.tenant_id) + creds.tenant_id) self.assertEqual(body['token']['tenant']['name'], tenant_name) - self.assertEqual(body['user']['id'], creds.credentials.user_id) + self.assertEqual(body['user']['id'], creds.user_id) diff --git a/tempest/api/identity/v2/test_users.py b/tempest/api/identity/v2/test_users.py index 79f2576f6..4833f9e2a 100644 --- a/tempest/api/identity/v2/test_users.py +++ b/tempest/api/identity/v2/test_users.py @@ -46,9 +46,9 @@ class IdentityUsersTest(base.BaseIdentityV2Test): client.auth_provider.clear_auth() client.auth_provider.set_auth() - old_pass = self.creds.credentials.password + old_pass = self.creds.password new_pass = data_utils.rand_password() - user_id = self.creds.credentials.user_id + user_id = self.creds.user_id # to change password back. important for allow_tenant_isolation = false self.addCleanup(_restore_password, self.non_admin_users_client, user_id, old_pass=old_pass, new_pass=new_pass) diff --git a/tempest/api/identity/v3/test_projects.py b/tempest/api/identity/v3/test_projects.py index 1574ab772..26cb90b60 100644 --- a/tempest/api/identity/v3/test_projects.py +++ b/tempest/api/identity/v3/test_projects.py @@ -25,7 +25,7 @@ class IdentityV3ProjectsTest(base.BaseIdentityV3Test): @test.idempotent_id('86128d46-e170-4644-866a-cc487f699e1d') def test_list_projects_returns_only_authorized_projects(self): alt_project_name =\ - self.alt_manager.credentials.credentials.project_name + self.alt_manager.credentials.project_name resp = self.non_admin_users_client.list_user_projects( self.os.credentials.user_id) diff --git a/tempest/api/identity/v3/test_users.py b/tempest/api/identity/v3/test_users.py index 76b46c0e9..c92e750ca 100644 --- a/tempest/api/identity/v3/test_users.py +++ b/tempest/api/identity/v3/test_users.py @@ -46,9 +46,9 @@ class IdentityV3UsersTest(base.BaseIdentityV3Test): client.auth_provider.clear_auth() client.auth_provider.set_auth() - old_pass = self.creds.credentials.password + old_pass = self.creds.password new_pass = data_utils.rand_password() - user_id = self.creds.credentials.user_id + user_id = self.creds.user_id # to change password back. important for allow_tenant_isolation = false self.addCleanup(_restore_password, self.non_admin_users_client, user_id, old_pass=old_pass, new_pass=new_pass) diff --git a/tempest/cmd/verify_tempest_config.py b/tempest/cmd/verify_tempest_config.py index 72ea894a4..f5587405c 100644 --- a/tempest/cmd/verify_tempest_config.py +++ b/tempest/cmd/verify_tempest_config.py @@ -384,7 +384,7 @@ def main(opts=None): icreds = credentials.get_credentials_provider( 'verify_tempest_config', network_resources=net_resources) try: - os = clients.Manager(icreds.get_primary_creds()) + os = clients.Manager(icreds.get_primary_creds().credentials) services = check_service_availability(os, update) results = {} for service in ['nova', 'cinder', 'neutron', 'swift']: diff --git a/tempest/manager.py b/tempest/manager.py index cafa5b9db..72762ab0a 100644 --- a/tempest/manager.py +++ b/tempest/manager.py @@ -13,7 +13,6 @@ # License for the specific language governing permissions and limitations # under the License. -from tempest.common import cred_provider from tempest import config from tempest import exceptions from tempest.lib import auth @@ -34,7 +33,7 @@ class Manager(object): Credentials to be used within the various client classes managed by the Manager object must be defined. - :param credentials: type Credentials or TestResources + :param credentials: An instance of `auth.Credentials` :param scope: default scope for tokens produced by the auth provider """ self.credentials = credentials @@ -42,15 +41,9 @@ class Manager(object): if not self.credentials.is_valid(): raise exceptions.InvalidCredentials() self.auth_version = CONF.identity.auth_version - # Tenant isolation creates TestResources, but - # PreProvisionedCredentialProvider and some tests create Credentials - if isinstance(credentials, cred_provider.TestResources): - creds = self.credentials.credentials - else: - creds = self.credentials # Creates an auth provider for the credentials - self.auth_provider = get_auth_provider(creds, pre_auth=True, - scope=scope) + self.auth_provider = get_auth_provider( + self.credentials, pre_auth=True, scope=scope) def get_auth_provider_class(credentials): diff --git a/tempest/scenario/utils.py b/tempest/scenario/utils.py index 75fd00036..c7ba65936 100644 --- a/tempest/scenario/utils.py +++ b/tempest/scenario/utils.py @@ -107,7 +107,8 @@ class InputScenarioUtils(object): name='InputScenarioUtils', identity_version=CONF.identity.auth_version, network_resources=network_resources) - os = clients.Manager(self.cred_provider.get_primary_creds()) + os = clients.Manager( + self.cred_provider.get_primary_creds().credentials) self.compute_images_client = os.compute_images_client self.flavors_client = os.flavors_client self.image_pattern = CONF.input_scenario.image_regex diff --git a/tempest/test.py b/tempest/test.py index d31c509c2..4c0d0bb03 100644 --- a/tempest/test.py +++ b/tempest/test.py @@ -541,7 +541,8 @@ class BaseTestCase(testtools.testcase.WithAttributes, else: raise exceptions.InvalidCredentials( "Invalid credentials type %s" % credential_type) - return cls.client_manager(credentials=creds, service=cls._service) + return cls.client_manager(credentials=creds.credentials, + service=cls._service) @classmethod def clear_credentials(cls): @@ -640,7 +641,7 @@ class BaseTestCase(testtools.testcase.WithAttributes, credentials.is_admin_available( identity_version=cls.get_identity_version())): admin_creds = cred_provider.get_admin_creds() - admin_manager = clients.Manager(admin_creds) + admin_manager = clients.Manager(admin_creds.credentials) networks_client = admin_manager.compute_networks_client return fixed_network.get_tenant_network( cred_provider, networks_client, CONF.compute.fixed_network_name) diff --git a/tempest/tests/test_base_test.py b/tempest/tests/test_base_test.py index dc355b428..01b8a72fd 100644 --- a/tempest/tests/test_base_test.py +++ b/tempest/tests/test_base_test.py @@ -66,7 +66,7 @@ class TestBaseTestCase(base.TestCase): test.BaseTestCase.get_tenant_network() mock_man.assert_called_once_with( - mock_prov.get_admin_creds.return_value) + mock_prov.get_admin_creds.return_value.credentials) mock_iaa.assert_called_once_with( identity_version=mock_giv.return_value) mock_gcp.assert_called_once_with()