From d9a2dc6bc5212196c9ef5bd05907e93594d02584 Mon Sep 17 00:00:00 2001 From: rallytime Date: Tue, 29 Sep 2015 19:38:38 -0600 Subject: [PATCH 1/3] [2015.8] Clean up salt-cloud logging and make it more useful --- salt/cloud/clouds/linode.py | 13 +++++++++---- salt/utils/http.py | 26 +++++++++++++++++++++++--- 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/salt/cloud/clouds/linode.py b/salt/cloud/clouds/linode.py index 086d1a2461..1ebf9c9000 100644 --- a/salt/cloud/clouds/linode.py +++ b/salt/cloud/clouds/linode.py @@ -445,14 +445,16 @@ def create(vm_): swap_disk_id = create_swap_disk(vm_, node_id)['DiskID'] # Add private IP address if requested - if get_private_ip(vm_): + use_private_ip = get_private_ip(vm_) + if use_private_ip: create_private_ip(vm_, node_id) # Create a ConfigID using disk ids config_id = create_config(kwargs={'name': vm_['name'], 'linode_id': node_id, 'root_disk_id': root_disk_id, - 'swap_disk_id': swap_disk_id})['ConfigID'] + 'swap_disk_id': swap_disk_id, + 'helper_network': True})['ConfigID'] # Boot the Linode boot(kwargs={'linode_id': node_id, 'config_id': config_id, @@ -469,7 +471,10 @@ def create(vm_): data['private_ips'] = ips['private_ips'] data['public_ips'] = ips['public_ips'] - vm_['ssh_host'] = data['public_ips'][0] + if use_private_ip: + vm_['ssh_host'] = data['private_ips'][0] + else: + vm_['ssh_host'] = data['public_ips'][0] # If a password wasn't supplied in the profile or provider config, set it now. vm_['password'] = get_password(vm_) @@ -1385,7 +1390,7 @@ def _query(action=None, decode_type='json', text=True, status=True, - hide_fields=['api_key'], + hide_fields=['api_key', 'rootPass'], opts=__opts__, ) log.debug( diff --git a/salt/utils/http.py b/salt/utils/http.py index 341a33fcc4..f95395e8aa 100644 --- a/salt/utils/http.py +++ b/salt/utils/http.py @@ -805,11 +805,31 @@ def sanitize_url(url, hide_fields): if len(url_comps) > 1: log_url += '?' for pair in url_comps[1:]: + url_tmp = None for field in hide_fields: - if pair.startswith('{0}='.format(field)): - log_url += '{0}=XXXXXXXXXX&'.format(field) + comps_list = pair.split('&') + if url_tmp: + url_tmp = url_tmp.split('&') + url_tmp = _sanitize_components(url_tmp, field) else: - log_url += '{0}&'.format(pair) + url_tmp = _sanitize_components(comps_list, field) + log_url += url_tmp return log_url.rstrip('&') else: return str(url) + + +def _sanitize_components(comp_list, field): + ''' + Use a recursive method to sanitize each component of the url. + ''' + if len(comp_list) == 0: + return '' + elif comp_list[0].startswith('{0}='.format(field)): + ret = '{0}=XXXXXXXXXX&'.format(field) + comp_list.remove(comp_list[0]) + return ret + _sanitize_components(comp_list, field) + else: + ret = '{0}&'.format(comp_list[0]) + comp_list.remove(comp_list[0]) + return ret + _sanitize_components(comp_list, field) From 78c85fbb31729703452e9f77159ad48ca5a61bdb Mon Sep 17 00:00:00 2001 From: rallytime Date: Tue, 29 Sep 2015 21:25:47 -0600 Subject: [PATCH 2/3] Add unit tests for new recursive function --- salt/utils/http.py | 12 ++-- tests/unit/utils/http_test.py | 100 ++++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 6 deletions(-) create mode 100644 tests/unit/utils/http_test.py diff --git a/salt/utils/http.py b/salt/utils/http.py index f95395e8aa..79ffcf6469 100644 --- a/salt/utils/http.py +++ b/salt/utils/http.py @@ -810,26 +810,26 @@ def sanitize_url(url, hide_fields): comps_list = pair.split('&') if url_tmp: url_tmp = url_tmp.split('&') - url_tmp = _sanitize_components(url_tmp, field) + url_tmp = _sanitize_url_components(url_tmp, field) else: - url_tmp = _sanitize_components(comps_list, field) + url_tmp = _sanitize_url_components(comps_list, field) log_url += url_tmp return log_url.rstrip('&') else: return str(url) -def _sanitize_components(comp_list, field): +def _sanitize_url_components(comp_list, field): ''' - Use a recursive method to sanitize each component of the url. + Recursive function to sanitize each component of the url. ''' if len(comp_list) == 0: return '' elif comp_list[0].startswith('{0}='.format(field)): ret = '{0}=XXXXXXXXXX&'.format(field) comp_list.remove(comp_list[0]) - return ret + _sanitize_components(comp_list, field) + return ret + _sanitize_url_components(comp_list, field) else: ret = '{0}&'.format(comp_list[0]) comp_list.remove(comp_list[0]) - return ret + _sanitize_components(comp_list, field) + return ret + _sanitize_url_components(comp_list, field) diff --git a/tests/unit/utils/http_test.py b/tests/unit/utils/http_test.py new file mode 100644 index 0000000000..fe2ff1e931 --- /dev/null +++ b/tests/unit/utils/http_test.py @@ -0,0 +1,100 @@ +# -*- coding: utf-8 -*- +''' + :codeauthor: :email:`Nicole Thomas ` +''' + +# Import Salt Libs +from __future__ import absolute_import + +# Import Salt Testing Libs +from salttesting import TestCase, skipIf +from salttesting.mock import NO_MOCK, NO_MOCK_REASON +from salttesting.helpers import ensure_in_syspath + +ensure_in_syspath('../../') + +# Import Salt Libs +from salt.utils import http + + +@skipIf(NO_MOCK, NO_MOCK_REASON) +class HTTPTestCase(TestCase): + ''' + Unit TestCase for the salt.utils.http module. + ''' + + # sanitize_url tests + + def test_sanitize_url_hide_fields_none(self): + ''' + Tests sanitizing a url when the hide_fields kwarg is None. + ''' + mock_url = 'https://api.testing.com/?&foo=bar&test=testing' + ret = http.sanitize_url(mock_url, hide_fields=None) + self.assertEqual(ret, mock_url) + + def test_sanitize_url_no_elements(self): + ''' + Tests sanitizing a url when no elements should be sanitized. + ''' + mock_url = 'https://api.testing.com/?&foo=bar&test=testing' + ret = http.sanitize_url(mock_url, ['']) + self.assertEqual(ret, mock_url) + + def test_sanitize_url_single_element(self): + ''' + Tests sanitizing a url with only a single element to be sanitized. + ''' + mock_url = 'https://api.testing.com/?&keep_it_secret=abcdefghijklmn' \ + '&api_action=module.function' + mock_ret = 'https://api.testing.com/?&keep_it_secret=XXXXXXXXXX&' \ + 'api_action=module.function' + ret = http.sanitize_url(mock_url, ['keep_it_secret']) + self.assertEqual(ret, mock_ret) + + def test_sanitize_url_multiple_elements(self): + ''' + Tests sanitizing a url with multiple elements to be sanitized. + ''' + mock_url = 'https://api.testing.com/?rootPass=badpassword%21' \ + '&skipChecks=True&api_key=abcdefghijklmn' \ + '&NodeID=12345&api_action=module.function' + mock_ret = 'https://api.testing.com/?rootPass=XXXXXXXXXX' \ + '&skipChecks=True&api_key=XXXXXXXXXX' \ + '&NodeID=12345&api_action=module.function' + ret = http.sanitize_url(mock_url, ['api_key', 'rootPass']) + self.assertEqual(ret, mock_ret) + + # _sanitize_components tests + + def test_sanitize_components_no_elements(self): + ''' + Tests when zero elements need to be sanitized. + ''' + mock_component_list = ['foo=bar', 'bar=baz', 'hello=world'] + mock_ret = 'foo=bar&bar=baz&hello=world&' + ret = http._sanitize_url_components(mock_component_list, 'api_key') + self.assertEqual(ret, mock_ret) + + def test_sanitize_components_one_element(self): + ''' + Tests a single component to be sanitized. + ''' + mock_component_list = ['foo=bar', 'api_key=abcdefghijklmnop'] + mock_ret = 'foo=bar&api_key=XXXXXXXXXX&' + ret = http._sanitize_url_components(mock_component_list, 'api_key') + self.assertEqual(ret, mock_ret) + + def test_sanitize_components_multiple_elements(self): + ''' + Tests two componenets to be sanitized. + ''' + mock_component_list = ['foo=bar', 'foo=baz', 'api_key=testing'] + mock_ret = 'foo=XXXXXXXXXX&foo=XXXXXXXXXX&api_key=testing&' + ret = http._sanitize_url_components(mock_component_list, 'foo') + self.assertEqual(ret, mock_ret) + + +if __name__ == '__main__': + from integration import run_tests + run_tests(HTTPTestCase, needs_daemon=False) From 9e0fccd543ffc79d0f76ece91f3cf52336886edc Mon Sep 17 00:00:00 2001 From: rallytime Date: Tue, 29 Sep 2015 21:28:59 -0600 Subject: [PATCH 3/3] Don't commit private-ip changes from testing another bug... --- salt/cloud/clouds/linode.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/salt/cloud/clouds/linode.py b/salt/cloud/clouds/linode.py index 1ebf9c9000..cf90251d3b 100644 --- a/salt/cloud/clouds/linode.py +++ b/salt/cloud/clouds/linode.py @@ -445,16 +445,14 @@ def create(vm_): swap_disk_id = create_swap_disk(vm_, node_id)['DiskID'] # Add private IP address if requested - use_private_ip = get_private_ip(vm_) - if use_private_ip: + if get_private_ip(vm_): create_private_ip(vm_, node_id) # Create a ConfigID using disk ids config_id = create_config(kwargs={'name': vm_['name'], 'linode_id': node_id, 'root_disk_id': root_disk_id, - 'swap_disk_id': swap_disk_id, - 'helper_network': True})['ConfigID'] + 'swap_disk_id': swap_disk_id})['ConfigID'] # Boot the Linode boot(kwargs={'linode_id': node_id, 'config_id': config_id, @@ -471,10 +469,7 @@ def create(vm_): data['private_ips'] = ips['private_ips'] data['public_ips'] = ips['public_ips'] - if use_private_ip: - vm_['ssh_host'] = data['private_ips'][0] - else: - vm_['ssh_host'] = data['public_ips'][0] + vm_['ssh_host'] = data['public_ips'][0] # If a password wasn't supplied in the profile or provider config, set it now. vm_['password'] = get_password(vm_)