From 276ad32a4535c262b9aa42c08981656344959f53 Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Fri, 31 Aug 2018 08:28:32 -0400 Subject: [PATCH] removing libcloud secrets support for legacy gcp modules (#44932) * removing libcloud secrets support for legacy gcp modules * test fixes --- lib/ansible/module_utils/gcp.py | 47 ------------------- test/units/module_utils/gcp/test_auth.py | 60 +----------------------- 2 files changed, 1 insertion(+), 106 deletions(-) diff --git a/lib/ansible/module_utils/gcp.py b/lib/ansible/module_utils/gcp.py index 8629a17494..508df44ab6 100644 --- a/lib/ansible/module_utils/gcp.py +++ b/lib/ansible/module_utils/gcp.py @@ -99,46 +99,6 @@ def _get_gcp_environment_credentials(service_account_email, credentials_file, pr return (service_account_email, credentials_file, project_id) -def _get_gcp_libcloud_credentials(module, service_account_email=None, credentials_file=None, project_id=None): - """ - Helper to look for libcloud secrets.py file. - - Note: This has an 'additive' effect right now, filling in - vars not specified elsewhere, in order to keep legacy functionality. - This method of specifying credentials will be deprecated, otherwise - we'd look to make it more restrictive with an all-vars-or-nothing approach. - - :param service_account: GCP service account email used to make requests - :type service_account: ``str`` or None - - :param credentials_file: Path on disk to credentials file - :type credentials_file: ``str`` or None - - :param project_id: GCP project ID. - :type project_id: ``str`` or None - - :return: tuple of (service_account, credentials_file, project_id) - :rtype: ``tuple`` of ``str`` - """ - if service_account_email is None or credentials_file is None: - try: - import secrets - module.deprecate(msg=("secrets file found at '%s'. This method of specifying " - "credentials is deprecated. Please use env vars or " - "Ansible YAML files instead" % (secrets.__file__)), version=2.5) - except ImportError: - secrets = None - if hasattr(secrets, 'GCE_PARAMS'): - if not service_account_email: - service_account_email = secrets.GCE_PARAMS[0] - if not credentials_file: - credentials_file = secrets.GCE_PARAMS[1] - keyword_params = getattr(secrets, 'GCE_KEYWORD_PARAMS', {}) - if not project_id: - project_id = keyword_params.get('project', None) - return (service_account_email, credentials_file, project_id) - - def _get_gcp_credentials(module, require_valid_json=True, check_libcloud=False): """ Obtain GCP credentials by trying various methods. @@ -188,13 +148,6 @@ def _get_gcp_credentials(module, require_valid_json=True, check_libcloud=False): project_id) = _get_gcp_environment_credentials(service_account_email, credentials_file, project_id) - # If we still don't have one or more of our credentials, attempt to - # get the remaining values from the libcloud secrets file. - (service_account_email, - credentials_file, - project_id) = _get_gcp_libcloud_credentials(module, service_account_email, - credentials_file, project_id) - if credentials_file is None or project_id is None or service_account_email is None: if check_libcloud is True: if project_id is None: diff --git a/test/units/module_utils/gcp/test_auth.py b/test/units/module_utils/gcp/test_auth.py index 0148be734a..aad732741d 100644 --- a/test/units/module_utils/gcp/test_auth.py +++ b/test/units/module_utils/gcp/test_auth.py @@ -22,7 +22,7 @@ import pytest from ansible.compat.tests import mock, unittest from ansible.module_utils.gcp import (_get_gcp_ansible_credentials, _get_gcp_credentials, _get_gcp_environ_var, - _get_gcp_libcloud_credentials, _get_gcp_environment_credentials, + _get_gcp_environment_credentials, _validate_credentials_file) # Fake data/function used for testing @@ -90,64 +90,6 @@ class GCPAuthTestCase(unittest.TestCase): self.assertEqual('default_value', _get_gcp_environ_var( non_existing_var_name, 'default_value')) - def test_get_gcp_libcloud_credentials_no_import(self): - """No secrets imported. Whatever is sent in should come out.""" - module = FakeModule() - actual = _get_gcp_libcloud_credentials(module, - service_account_email=None, - credentials_file=None, - project_id=None) - expected = (None, None, None) - self.assertEqual(expected, actual) - # no libcloud, with values - actual = _get_gcp_libcloud_credentials(module, - service_account_email='sa-email', - credentials_file='creds-file', - project_id='proj-id') - expected = ('sa-email', 'creds-file', 'proj-id') - self.assertEqual(expected, actual) - - def test_get_gcp_libcloud_credentials_import(self): - """secrets is imported and those values should be used.""" - # Note: Opted for a real class here rather than MagicMock as - # __getitem__ comes for free. - class FakeSecrets: - def __init__(self): - # 2 element list, service account email and creds file - self.GCE_PARAMS = ['secrets-sa', 'secrets-file.json'] - # dictionary with project_id, optionally auth_type - self.GCE_KEYWORD_PARAMS = {} - self.__file__ = 'THIS_IS_A_FAKEFILE_FOR_TESTING' - - # patch in module - fake_secrets = FakeSecrets() - patcher = mock.patch.dict(sys.modules, {'secrets': fake_secrets}) - patcher.start() - - # obtain sa and creds from secrets - module = FakeModule() - actual = _get_gcp_libcloud_credentials(module, - service_account_email=None, - credentials_file=None, - project_id='proj-id') - expected = ('secrets-sa', 'secrets-file.json', 'proj-id') - self.assertEqual(expected, actual) - - # fetch project id. Current logic requires sa-email or creds to be - # set. - fake_secrets.GCE_KEYWORD_PARAMS['project'] = 'new-proj-id' - fake_secrets.GCE_PARAMS[1] = 'my-creds.json' - module = FakeModule() - actual = _get_gcp_libcloud_credentials(module, - service_account_email='my-sa', - credentials_file=None, - project_id=None) - expected = ('my-sa', 'my-creds.json', 'new-proj-id') - self.assertEqual(expected, actual) - - # stop patching - patcher.stop() - def test_validate_credentials_file(self): # TODO(supertom): Only dealing with p12 here, check the other states # of this function