From 3fbf3b51ff64976360d71e1ea760685afc390fd0 Mon Sep 17 00:00:00 2001 From: Tom Melendez Date: Thu, 18 May 2017 09:07:16 -0700 Subject: [PATCH] [GCP] remove ansible.utils.display for deprecations (#24738) * [GCP] remove ansible.utils.display for deprecations, use module.deprecate instead. * removed test file from legacy files --- lib/ansible/module_utils/gcp.py | 23 +++++------- test/sanity/pep8/legacy-files.txt | 1 - test/units/module_utils/gcp/test_auth.py | 46 +++++++++++++++--------- 3 files changed, 38 insertions(+), 32 deletions(-) diff --git a/lib/ansible/module_utils/gcp.py b/lib/ansible/module_utils/gcp.py index 441462bab7..2ec38476bc 100644 --- a/lib/ansible/module_utils/gcp.py +++ b/lib/ansible/module_utils/gcp.py @@ -60,13 +60,6 @@ except ImportError: HAS_GOOGLE_API_LIB = False -# Ansible Display object for warnings -try: - from __main__ import display -except ImportError: - from ansible.utils.display import Display - display = Display() - import ansible.module_utils.six.moves.urllib.parse as urlparse GCP_DEFAULT_SCOPES = ['https://www.googleapis.com/auth/cloud-platform'] @@ -106,7 +99,7 @@ def _get_gcp_environment_credentials(service_account_email, credentials_file, pr return (service_account_email, credentials_file, project_id) -def _get_gcp_libcloud_credentials(service_account_email=None, credentials_file=None, project_id=None): +def _get_gcp_libcloud_credentials(module, service_account_email=None, credentials_file=None, project_id=None): """ Helper to look for libcloud secrets.py file. @@ -130,9 +123,9 @@ def _get_gcp_libcloud_credentials(service_account_email=None, credentials_file=N if service_account_email is None or credentials_file is None: try: import secrets - display.deprecated(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) + 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'): @@ -199,7 +192,7 @@ def _get_gcp_credentials(module, require_valid_json=True, check_libcloud=False): # get the remaining values from the libcloud secrets file. (service_account_email, credentials_file, - project_id) = _get_gcp_libcloud_credentials(service_account_email, + 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: @@ -274,9 +267,9 @@ def _validate_credentials_file(module, credentials_file, require_valid_json=True module.fail_json( msg='GCP Credentials File %s invalid. Must be valid JSON.' % credentials_file, changed=False) else: - display.deprecated(msg=("Non-JSON credentials file provided. This format is deprecated. " - " Please generate a new JSON key from the Google Cloud console"), - version=2.5) + module.deprecate(msg=("Non-JSON credentials file provided. This format is deprecated. " + " Please generate a new JSON key from the Google Cloud console"), + version=2.5) return True diff --git a/test/sanity/pep8/legacy-files.txt b/test/sanity/pep8/legacy-files.txt index 686f2a07f5..da861040dc 100644 --- a/test/sanity/pep8/legacy-files.txt +++ b/test/sanity/pep8/legacy-files.txt @@ -883,7 +883,6 @@ test/units/module_utils/basic/test_run_command.py test/units/module_utils/basic/test_safe_eval.py test/units/module_utils/basic/test_set_mode_if_different.py test/units/module_utils/ec2/test_aws.py -test/units/module_utils/gcp/test_auth.py test/units/module_utils/json_utils/test_filter_non_json_lines.py test/units/module_utils/test_basic.py test/units/module_utils/test_distribution_version.py diff --git a/test/units/module_utils/gcp/test_auth.py b/test/units/module_utils/gcp/test_auth.py index 0e3adcd903..1d3685249d 100644 --- a/test/units/module_utils/gcp/test_auth.py +++ b/test/units/module_utils/gcp/test_auth.py @@ -27,6 +27,8 @@ from ansible.module_utils.gcp import (_get_gcp_ansible_credentials, _get_gcp_cre # Fake data/function used for testing fake_env_data = {'GCE_EMAIL': 'gce-email'} + + def fake_get_gcp_environ_var(var_name, default_value): if var_name not in fake_env_data: return default_value @@ -34,6 +36,8 @@ def fake_get_gcp_environ_var(var_name, default_value): return fake_env_data[var_name] # Fake AnsibleModule for use in tests + + class FakeModule(object): class Params(): data = {} @@ -51,6 +55,10 @@ class FakeModule(object): def fail_json(self, **kwargs): raise ValueError("fail_json") + def deprecate(self, **kwargs): + return None + + class GCPAuthTestCase(unittest.TestCase): """Tests to verify different Auth mechanisms.""" @@ -74,28 +82,30 @@ class GCPAuthTestCase(unittest.TestCase): existing_var_name = 'gcp_ansible_auth_test_54321' non_existing_var_name = 'doesnt_exist_gcp_ansible_auth_test_12345' os.environ[existing_var_name] = 'foobar' - self.assertEqual('foobar', _get_gcp_environ_var(existing_var_name, None)) + self.assertEqual('foobar', _get_gcp_environ_var( + existing_var_name, None)) del os.environ[existing_var_name] 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.""" - actual = _get_gcp_libcloud_credentials(service_account_email=None, + 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(service_account_email='sa-email', + 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) - @mock.patch("ansible.utils.display.Display.deprecated", - name='mock_deprecated', return_value=None) - def test_get_gcp_libcloud_credentials_import(self, mock_deprecated): + 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. @@ -109,21 +119,25 @@ class GCPAuthTestCase(unittest.TestCase): # patch in module fake_secrets = FakeSecrets() - patcher = mock.patch.dict(sys.modules,{'secrets': fake_secrets}) + patcher = mock.patch.dict(sys.modules, {'secrets': fake_secrets}) patcher.start() # obtain sa and creds from secrets - actual = _get_gcp_libcloud_credentials(service_account_email=None, + 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. + # 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' - - actual = _get_gcp_libcloud_credentials(service_account_email='my-sa', + 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') @@ -132,9 +146,7 @@ class GCPAuthTestCase(unittest.TestCase): # stop patching patcher.stop() - @mock.patch("ansible.utils.display.Display.deprecated", - name='mock_deprecated', return_value=None) - def test_validate_credentials_file(self, mock_deprecated): + def test_validate_credentials_file(self): # TODO(supertom): Only dealing with p12 here, check the other states # of this function module = mock.MagicMock() @@ -190,7 +202,8 @@ class GCPAuthTestCase(unittest.TestCase): # data passed in, picking up project id only fake_env_data = {'GOOGLE_CLOUD_PROJECT': 'my-project'} expected = tuple(['my-sa-email', '/path/to/creds.json', 'my-project']) - actual = _get_gcp_environment_credentials('my-sa-email', '/path/to/creds.json', None) + actual = _get_gcp_environment_credentials( + 'my-sa-email', '/path/to/creds.json', None) self.assertEqual(expected, actual) @mock.patch('ansible.module_utils.gcp._get_gcp_environ_var', @@ -207,7 +220,8 @@ class GCPAuthTestCase(unittest.TestCase): # project_id (only) is set from Ansible params. module.params.data['project_id'] = 'my-project' - actual = _get_gcp_credentials(module, require_valid_json=True, check_libcloud=False) + actual = _get_gcp_credentials( + module, require_valid_json=True, check_libcloud=False) expected = {'service_account_email': '', 'project_id': 'my-project', 'credentials_file': ''}