From f8e3f6d6785638f6049e5a9c084ddbf164538b96 Mon Sep 17 00:00:00 2001 From: Andrey Tuzhilin Date: Thu, 19 Jul 2018 09:23:24 +0300 Subject: [PATCH] + lint errors fixed --- salt/modules/vault.py | 12 ++--- salt/runners/vault.py | 12 +++-- tests/unit/runners/test_vault.py | 90 ++++++++++++++++++++------------ 3 files changed, 72 insertions(+), 42 deletions(-) diff --git a/salt/modules/vault.py b/salt/modules/vault.py index 737fdd5f21..18f7d127f5 100644 --- a/salt/modules/vault.py +++ b/salt/modules/vault.py @@ -51,10 +51,10 @@ Functions to interact with Hashicorp Vault. .. versionadded:: 2018.3.0 role_name - Role name for minion tokens created. If omitted, minion tokens will be - created without any role, thus being able to inherit any master token + Role name for minion tokens created. If omitted, minion tokens will be + created without any role, thus being able to inherit any master token policy (including token creation capabilities). Optional. - + For details please see: https://www.vaultproject.io/api/auth/token/index.html#create-token Example configuration: @@ -62,12 +62,12 @@ Functions to interact with Hashicorp Vault. auth Currently only token and approle auth types are supported. Required. - + Approle is the preferred way to authenticate with Vault as it provide - some advanced options to control authentication process. + some advanced options to control authentication process. Please visit Vault documentation for more info: https://www.vaultproject.io/docs/auth/approle.html - + The token must be able to create tokens with the policies that should be assigned to minions. diff --git a/salt/runners/vault.py b/salt/runners/vault.py index 547e626ec6..89e5b8bb9f 100644 --- a/salt/runners/vault.py +++ b/salt/runners/vault.py @@ -24,6 +24,7 @@ from salt.ext import six log = logging.getLogger(__name__) + def generate_token(minion_id, signature, impersonated_by_master=False): ''' Generate a Vault token for minion minion_id @@ -54,7 +55,7 @@ def generate_token(minion_id, signature, impersonated_by_master=False): log.debug('Vault token expired. Recreating one') # Requesting a short ttl token url = '{0}/v1/auth/approle/login'.format(config['url']) - + payload = {'role_id': config['auth']['role_id']} if 'secret_id' in config['auth']: payload['secret_id'] = config['auth']['secret_id'] @@ -85,9 +86,9 @@ def generate_token(minion_id, signature, impersonated_by_master=False): if response.status_code != 200: return {'error': response.reason} - authData = response.json()['auth'] + auth_data = response.json()['auth'] return { - 'token': authData['client_token'], + 'token': auth_data['client_token'], 'url': config['url'], 'verify': verify, } @@ -257,9 +258,12 @@ def _selftoken_expired(): 'Error while looking up self token : {0}'.format(six.text_type(e)) ) + def _get_token_create_url(config): + ''' + Create Vault url for token creation + ''' role_name = config.get('role_name', None) auth_path = '/v1/auth/token/create' base_url = config['url'] return '/'.join(x.strip('/') for x in (base_url, auth_path, role_name) if x) - diff --git a/tests/unit/runners/test_vault.py b/tests/unit/runners/test_vault.py index e7397ef38a..2fd4446d6c 100644 --- a/tests/unit/runners/test_vault.py +++ b/tests/unit/runners/test_vault.py @@ -17,8 +17,7 @@ from tests.support.mock import ( NO_MOCK, NO_MOCK_REASON, ANY, - call, - patch + call ) # Import salt libs @@ -27,6 +26,7 @@ import salt.runners.vault as vault log = logging.getLogger(__name__) + class VaultTest(TestCase, LoaderModuleMockMixin): ''' Tests for the runner module of the Vault integration @@ -87,7 +87,7 @@ class VaultTest(TestCase, LoaderModuleMockMixin): for case, correct_output in six.iteritems(cases): output = vault._expand_pattern_lists(case, **mappings) # pylint: disable=protected-access diff = set(output).symmetric_difference(set(correct_output)) - if len(diff) != 0: + if diff: log.debug('Test %s failed', case) log.debug('Expected:\n\t%s\nGot\n\t%s', output, correct_output) log.debug('Difference:\n\t%s', diff) @@ -107,7 +107,7 @@ class VaultTest(TestCase, LoaderModuleMockMixin): test_config = {'policies': [case]} output = vault._get_policies(minion_id, test_config) # pylint: disable=protected-access diff = set(output).symmetric_difference(set(correct_output)) - if len(diff) != 0: + if diff: log.debug('Test %s failed', case) log.debug('Expected:\n\t%s\nGot\n\t%s', output, correct_output) log.debug('Difference:\n\t%s', diff) @@ -148,54 +148,68 @@ class VaultTest(TestCase, LoaderModuleMockMixin): test_config = {'policies': [case]} output = vault._get_policies('test-minion', test_config) # pylint: disable=protected-access diff = set(output).symmetric_difference(set(correct_output)) - if len(diff) != 0: + if diff: log.debug('Test %s failed', case) log.debug('Expected:\n\t%s\nGot\n\t%s', output, correct_output) log.debug('Difference:\n\t%s', diff) self.assertEqual(output, correct_output) def test_get_token_create_url(self): - self.assertEqual(vault._get_token_create_url( + ''' + Ensure _get_token_create_url parses config correctly + ''' + self.assertEqual(vault._get_token_create_url( # pylint: disable=protected-access {"url": "http://127.0.0.1"}), "http://127.0.0.1/v1/auth/token/create") - self.assertEqual(vault._get_token_create_url( + self.assertEqual(vault._get_token_create_url( # pylint: disable=protected-access {"url": "https://127.0.0.1/"}), "https://127.0.0.1/v1/auth/token/create") - self.assertEqual(vault._get_token_create_url( + self.assertEqual(vault._get_token_create_url( # pylint: disable=protected-access {"url": "http://127.0.0.1:8200", "role_name": "therole"}), "http://127.0.0.1:8200/v1/auth/token/create/therole") - self.assertEqual(vault._get_token_create_url( + self.assertEqual(vault._get_token_create_url( # pylint: disable=protected-access {"url": "https://127.0.0.1/test", "role_name": "therole"}), "https://127.0.0.1/test/v1/auth/token/create/therole") -class _Helpers: +def _mock_json_response(data, status_code=200, reason=""): ''' - Mock helpers + Mock helper for http response ''' - def _mock_json_response(self, data, status_code = 200, reason = ""): - response = MagicMock() - response.json = MagicMock(return_value=data) - response.status_code = status_code - response.reason = reason - return Mock(return_value=response) + response = MagicMock() + response.json = MagicMock(return_value=data) + response.status_code = status_code + response.reason = reason + return Mock(return_value=response) -class VaultTokenAuthTest(TestCase, LoaderModuleMockMixin, _Helpers): + +class VaultTokenAuthTest(TestCase, LoaderModuleMockMixin): ''' - Tests for the runner module of the Vault with approle setup + Tests for the runner module of the Vault with token setup ''' def setup_loader_modules(self): return { vault: { - '__opts__': {'vault': {'url': "http://127.0.0.1", "auth": {'token': 'test', 'method': 'token'}}} + '__opts__': { + 'vault': { + 'url': "http://127.0.0.1", + "auth": { + 'token': 'test', + 'method': 'token' + } + } + } } } @patch('salt.runners.vault._validate_signature', MagicMock(return_value=None)) @patch('salt.runners.vault._get_token_create_url', MagicMock(return_value="http://fake_url")) def test_generate_token(self): - mock = self._mock_json_response({'auth': {'client_token': 'test'}}) + ''' + Basic tests for test_generate_token: all exits + ''' + mock = _mock_json_response({'auth': {'client_token': 'test'}}) with patch('requests.post', mock): result = vault.generate_token('test-minion', 'signature') log.debug('generate_token result: %s', result) @@ -204,35 +218,35 @@ class VaultTokenAuthTest(TestCase, LoaderModuleMockMixin, _Helpers): self.assertTrue('token' in result) self.assertEqual(result['token'], 'test') mock.assert_called_with("http://fake_url", headers=ANY, json=ANY, verify=ANY) - - mock = self._mock_json_response({}, status_code=403, reason="no reason") + + mock = _mock_json_response({}, status_code=403, reason="no reason") with patch('requests.post', mock): result = vault.generate_token('test-minion', 'signature') self.assertTrue(isinstance(result, dict)) self.assertTrue('error' in result) self.assertEqual(result['error'], "no reason") - mock = self._mock_json_response({}, status_code=403, reason="no reason") + mock = _mock_json_response({}, status_code=403, reason="no reason") with patch('requests.post', mock): result = vault.generate_token('test-minion', 'signature') self.assertTrue(isinstance(result, dict)) self.assertTrue('error' in result) self.assertEqual(result['error'], 'no reason') - + with patch('salt.runners.vault._get_policies', MagicMock(return_value=[])): - result = vault.generate_token('test-minion', 'signature') + result = vault.generate_token('test-minion', 'signature') self.assertTrue(isinstance(result, dict)) self.assertTrue('error' in result) self.assertEqual(result['error'], 'No policies matched minion') - with patch('salt.runners.vault._get_policies', MagicMock(side_effect = Exception('Test Exception Reason'))): - result = vault.generate_token('test-minion', 'signature') + with patch('salt.runners.vault._get_policies', MagicMock(side_effect=Exception('Test Exception Reason'))): + result = vault.generate_token('test-minion', 'signature') self.assertTrue(isinstance(result, dict)) self.assertTrue('error' in result) self.assertEqual(result['error'], 'Test Exception Reason') -class VaultAppRoleAuthTest(TestCase, LoaderModuleMockMixin, _Helpers): +class VaultAppRoleAuthTest(TestCase, LoaderModuleMockMixin): ''' Tests for the runner module of the Vault with approle setup ''' @@ -240,14 +254,26 @@ class VaultAppRoleAuthTest(TestCase, LoaderModuleMockMixin, _Helpers): def setup_loader_modules(self): return { vault: { - '__opts__': {'vault': {'url': "http://127.0.0.1", "auth": {'method': 'approle', 'role_id': 'role', 'secret_id': 'secret'}}} + '__opts__': { + 'vault': { + 'url': "http://127.0.0.1", + "auth": { + 'method': 'approle', + 'role_id': 'role', + 'secret_id': 'secret' + } + } + } } } @patch('salt.runners.vault._validate_signature', MagicMock(return_value=None)) @patch('salt.runners.vault._get_token_create_url', MagicMock(return_value="http://fake_url")) def test_generate_token(self): - mock = self._mock_json_response({'auth': {'client_token': 'test'}}) + ''' + Basic test for test_generate_token with approle (two vault calls) + ''' + mock = _mock_json_response({'auth': {'client_token': 'test'}}) with patch('requests.post', mock): result = vault.generate_token('test-minion', 'signature') log.debug('generate_token result: %s', result) @@ -256,7 +282,7 @@ class VaultAppRoleAuthTest(TestCase, LoaderModuleMockMixin, _Helpers): self.assertTrue('token' in result) self.assertEqual(result['token'], 'test') calls = [ - call("http://127.0.0.1/v1/auth/approle/login", json=ANY, verify=ANY), + call("http://127.0.0.1/v1/auth/approle/login", json=ANY, verify=ANY), call("http://fake_url", headers=ANY, json=ANY, verify=ANY) ] mock.assert_has_calls(calls)