From fa1abfec27050a4fb71cad950a17e42f9b43b478 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Tue, 3 Mar 2020 15:23:33 -0700 Subject: ec2: only redact token request headers in logs, avoid altering request (#230) Our header redact logic was redacting both logged request headers and the actual source request. This results in DataSourceEc2 sending the invalid header "X-aws-ec2-metadata-token-ttl-seconds: REDACTED" which gets an HTTP status response of 400. Cloud-init retries this failed token request for 2 minutes before falling back to IMDSv1. LP: #1865882 --- cloudinit/tests/test_url_helper.py | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) (limited to 'cloudinit/tests/test_url_helper.py') diff --git a/cloudinit/tests/test_url_helper.py b/cloudinit/tests/test_url_helper.py index 1674120f..29b39374 100644 --- a/cloudinit/tests/test_url_helper.py +++ b/cloudinit/tests/test_url_helper.py @@ -1,7 +1,8 @@ # This file is part of cloud-init. See LICENSE file for license information. from cloudinit.url_helper import ( - NOT_FOUND, UrlError, oauth_headers, read_file_or_url, retry_on_url_exc) + NOT_FOUND, UrlError, REDACTED, oauth_headers, read_file_or_url, + retry_on_url_exc) from cloudinit.tests.helpers import CiTestCase, mock, skipIf from cloudinit import util from cloudinit import version @@ -50,6 +51,9 @@ class TestOAuthHeaders(CiTestCase): class TestReadFileOrUrl(CiTestCase): + + with_logs = True + def test_read_file_or_url_str_from_file(self): """Test that str(result.contents) on file is text version of contents. It should not be "b'data'", but just "'data'" """ @@ -71,6 +75,34 @@ class TestReadFileOrUrl(CiTestCase): self.assertEqual(result.contents, data) self.assertEqual(str(result), data.decode('utf-8')) + @httpretty.activate + def test_read_file_or_url_str_from_url_redacting_headers_from_logs(self): + """Headers are redacted from logs but unredacted in requests.""" + url = 'http://hostname/path' + headers = {'sensitive': 'sekret', 'server': 'blah'} + httpretty.register_uri(httpretty.GET, url) + + read_file_or_url(url, headers=headers, headers_redact=['sensitive']) + logs = self.logs.getvalue() + for k in headers.keys(): + self.assertEqual(headers[k], httpretty.last_request().headers[k]) + self.assertIn(REDACTED, logs) + self.assertNotIn('sekret', logs) + + @httpretty.activate + def test_read_file_or_url_str_from_url_redacts_noheaders(self): + """When no headers_redact, header values are in logs and requests.""" + url = 'http://hostname/path' + headers = {'sensitive': 'sekret', 'server': 'blah'} + httpretty.register_uri(httpretty.GET, url) + + read_file_or_url(url, headers=headers) + for k in headers.keys(): + self.assertEqual(headers[k], httpretty.last_request().headers[k]) + logs = self.logs.getvalue() + self.assertNotIn(REDACTED, logs) + self.assertIn('sekret', logs) + @mock.patch(M_PATH + 'readurl') def test_read_file_or_url_passes_params_to_readurl(self, m_readurl): """read_file_or_url passes all params through to readurl.""" -- cgit v1.2.3 From 5f7825e22241423322dbe628de1b00289cf34114 Mon Sep 17 00:00:00 2001 From: Joshua Powers Date: Tue, 2 Jun 2020 08:24:29 -0700 Subject: test: fix all flake8 E241 (#403) Remove extra spaces after a ',' --- cloudinit/cmd/tests/test_query.py | 2 +- cloudinit/config/cc_apt_configure.py | 2 +- cloudinit/net/tests/test_dhcp.py | 2 +- cloudinit/sources/DataSourceGCE.py | 2 +- cloudinit/tests/test_url_helper.py | 2 +- tests/unittests/test_net.py | 4 ++-- tox.ini | 3 +-- 7 files changed, 8 insertions(+), 9 deletions(-) (limited to 'cloudinit/tests/test_url_helper.py') diff --git a/cloudinit/cmd/tests/test_query.py b/cloudinit/cmd/tests/test_query.py index 6d36a4ea..cb15b2d2 100644 --- a/cloudinit/cmd/tests/test_query.py +++ b/cloudinit/cmd/tests/test_query.py @@ -261,7 +261,7 @@ class TestQuery(CiTestCase): args = self.args( debug=False, dump_all=False, format=None, instance_data=self.instance_data, list_keys=True, user_data='ud', - vendor_data='vd', varname='top') + vendor_data='vd', varname='top') with mock.patch('sys.stderr', new_callable=StringIO) as m_stderr: with mock.patch('sys.stdout', new_callable=StringIO) as m_stdout: with mock.patch('os.getuid') as m_getuid: diff --git a/cloudinit/config/cc_apt_configure.py b/cloudinit/config/cc_apt_configure.py index 9a33451d..b1c7b471 100644 --- a/cloudinit/config/cc_apt_configure.py +++ b/cloudinit/config/cc_apt_configure.py @@ -297,7 +297,7 @@ schema = { }, 'conf': { 'type': 'string', - 'description': dedent("""\ + 'description': dedent("""\ Specify configuration for apt, such as proxy configuration. This configuration is specified as a string. For multiline apt configuration, make sure diff --git a/cloudinit/net/tests/test_dhcp.py b/cloudinit/net/tests/test_dhcp.py index bc7bef45..7768da7c 100644 --- a/cloudinit/net/tests/test_dhcp.py +++ b/cloudinit/net/tests/test_dhcp.py @@ -211,7 +211,7 @@ class TestDHCPParseStaticRoutes(CiTestCase): "class_b": "16,172,16,10", "class_a": "8,10,10", "gateway": "0,0", - "netlen": "33,0", + "netlen": "33,0", } for rfc3442 in bad_rfc3442.values(): self.assertEqual([], parse_static_routes(rfc3442)) diff --git a/cloudinit/sources/DataSourceGCE.py b/cloudinit/sources/DataSourceGCE.py index 6cbfbbac..0ec5f6ec 100644 --- a/cloudinit/sources/DataSourceGCE.py +++ b/cloudinit/sources/DataSourceGCE.py @@ -116,7 +116,7 @@ def _write_host_key_to_guest_attributes(key_type, key_value): resp = url_helper.readurl(url=url, data=key_value, headers=HEADERS, request_method='PUT', check_status=False) if resp.ok(): - LOG.debug('Wrote %s host key to guest attributes.', key_type) + LOG.debug('Wrote %s host key to guest attributes.', key_type) else: LOG.debug('Unable to write %s host key to guest attributes.', key_type) diff --git a/cloudinit/tests/test_url_helper.py b/cloudinit/tests/test_url_helper.py index 29b39374..364ec822 100644 --- a/cloudinit/tests/test_url_helper.py +++ b/cloudinit/tests/test_url_helper.py @@ -85,7 +85,7 @@ class TestReadFileOrUrl(CiTestCase): read_file_or_url(url, headers=headers, headers_redact=['sensitive']) logs = self.logs.getvalue() for k in headers.keys(): - self.assertEqual(headers[k], httpretty.last_request().headers[k]) + self.assertEqual(headers[k], httpretty.last_request().headers[k]) self.assertIn(REDACTED, logs) self.assertNotIn('sekret', logs) diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index e075a64c..84d3a5f0 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -944,7 +944,7 @@ NETWORK_CONFIGS = { dhcp6: true """).rstrip(' '), 'expected_sysconfig_opensuse': { - 'ifcfg-iface0': textwrap.dedent("""\ + 'ifcfg-iface0': textwrap.dedent("""\ BOOTPROTO=dhcp DHCLIENT6_MODE=managed STARTMODE=auto""") @@ -1028,7 +1028,7 @@ NETWORK_CONFIGS = { }, 'v6_and_v4': { 'expected_sysconfig_opensuse': { - 'ifcfg-iface0': textwrap.dedent("""\ + 'ifcfg-iface0': textwrap.dedent("""\ BOOTPROTO=dhcp DHCLIENT6_MODE=managed STARTMODE=auto""") diff --git a/tox.ini b/tox.ini index 725a0ca4..5ee67368 100644 --- a/tox.ini +++ b/tox.ini @@ -47,10 +47,9 @@ deps = -r{toxinidir}/test-requirements.txt # E123: closing bracket does not match indentation of opening bracket’s line # E126: continuation line over-indented for hanging indent # E226: missing whitespace around arithmetic operator -# E241: multiple spaces after ‘,’ # W503: line break before binary operator # W504: line break after binary operator -ignore=E121,E123,E126,E226,E241,W503,W504 +ignore=E121,E123,E126,E226,W503,W504 exclude = .venv,.tox,dist,doc,*egg,.git,build,tools per-file-ignores = cloudinit/cmd/main.py:E402 -- cgit v1.2.3