diff --git a/charmhelpers/core/hookenv.py b/charmhelpers/core/hookenv.py index 2e2876596..4744eb433 100644 --- a/charmhelpers/core/hookenv.py +++ b/charmhelpers/core/hookenv.py @@ -50,6 +50,11 @@ MARKER = object() SH_MAX_ARG = 131071 + +RANGE_WARNING = ('Passing NO_PROXY string that includes a cidr. ' + 'This may not be compatible with software you are ' + 'running in your shell.') + cache = {} @@ -1414,3 +1419,72 @@ def unit_doomed(unit=None): # I don't think 'dead' units ever show up in the goal-state, but # check anyway in addition to 'dying'. return units[unit]['status'] in ('dying', 'dead') + + +def env_proxy_settings(selected_settings=None): + """Get proxy settings from process environment variables. + + Get charm proxy settings from environment variables that correspond to + juju-http-proxy, juju-https-proxy and juju-no-proxy (available as of 2.4.2, + see lp:1782236) in a format suitable for passing to an application that + reacts to proxy settings passed as environment variables. Some applications + support lowercase or uppercase notation (e.g. curl), some support only + lowercase (e.g. wget), there are also subjectively rare cases of only + uppercase notation support. no_proxy CIDR and wildcard support also varies + between runtimes and applications as there is no enforced standard. + + Some applications may connect to multiple destinations and expose config + options that would affect only proxy settings for a specific destination + these should be handled in charms in an application-specific manner. + + :param selected_settings: format only a subset of possible settings + :type selected_settings: list + :rtype: Option(None, dict[str, str]) + """ + SUPPORTED_SETTINGS = { + 'http': 'HTTP_PROXY', + 'https': 'HTTPS_PROXY', + 'no_proxy': 'NO_PROXY', + 'ftp': 'FTP_PROXY' + } + if selected_settings is None: + selected_settings = SUPPORTED_SETTINGS + + selected_vars = [v for k, v in SUPPORTED_SETTINGS.items() + if k in selected_settings] + proxy_settings = {} + for var in selected_vars: + var_val = os.getenv(var) + if var_val: + proxy_settings[var] = var_val + proxy_settings[var.lower()] = var_val + # Now handle juju-prefixed environment variables. The legacy vs new + # environment variable usage is mutually exclusive + charm_var_val = os.getenv('JUJU_CHARM_{}'.format(var)) + if charm_var_val: + proxy_settings[var] = charm_var_val + proxy_settings[var.lower()] = charm_var_val + if 'no_proxy' in proxy_settings: + if _contains_range(proxy_settings['no_proxy']): + log(RANGE_WARNING, level=WARNING) + return proxy_settings if proxy_settings else None + + +def _contains_range(addresses): + """Check for cidr or wildcard domain in a string. + + Given a string comprising a comma seperated list of ip addresses + and domain names, determine whether the string contains IP ranges + or wildcard domains. + + :param addresses: comma seperated list of domains and ip addresses. + :type addresses: str + """ + return ( + # Test for cidr (e.g. 10.20.20.0/24) + "/" in addresses or + # Test for wildcard domains (*.foo.com or .foo.com) + "*" in addresses or + addresses.startswith(".") or + ",." in addresses or + " ." in addresses) diff --git a/charmhelpers/fetch/ubuntu.py b/charmhelpers/fetch/ubuntu.py index 84fff1c70..2394caf30 100644 --- a/charmhelpers/fetch/ubuntu.py +++ b/charmhelpers/fetch/ubuntu.py @@ -28,6 +28,7 @@ log, DEBUG, WARNING, + env_proxy_settings, ) from charmhelpers.fetch import SourceConfigError, GPGKeyError @@ -181,10 +182,6 @@ CMD_RETRY_DELAY = 10 # Wait 10 seconds between command retries. CMD_RETRY_COUNT = 3 # Retry a failing fatal command X times. -RANGE_WARNING = ('Passing NO_PROXY string that includes a cidr. ' - 'This may not be compatible with software you are ' - 'running in your shell.') - def filter_installed_packages(packages): """Return a list of packages that require installation.""" @@ -420,7 +417,7 @@ def _get_key_by_keyid(keyid): curl_cmd = ['curl', keyserver_url.format(keyid)] # use proxy server settings in order to retrieve the key return subprocess.check_output(curl_cmd, - env=_env_proxy_settings(['https'])) + env=env_proxy_settings(['https'])) def _dearmor_gpg_key(key_asc): @@ -581,81 +578,7 @@ def _add_apt_repository(spec): # passed as environment variables (See lp:1433761). This is not the case # LTS and non-LTS releases below bionic. _run_with_retries(['add-apt-repository', '--yes', spec], - cmd_env=_env_proxy_settings(['https'])) - - -def _contains_range(addresses): - """Check for cidr or wildcard domain in a string. - - Given a string comprising a comma seperated list of ip addresses - and domain names, determine whether the string contains IP ranges - or wildcard domains. - - :param addresses: comma seperated list of domains and ip addresses. - :type addresses: str - """ - # Test for cidr (e.g. 10.20.20.0/24) - if "/" in addresses: - return True - # Test for wildcard domains (*.foo.com or .foo.com) - if "*" in addresses: - return True - if addresses.startswith("."): - return True - if ",." in addresses: - return True - if " ." in addresses: - return True - return False - - -def _env_proxy_settings(selected_settings=None): - """Get proxy settings from process environment variables. - - Get charm proxy settings from environment variables that correspond to - juju-http-proxy, juju-https-proxy and juju-no-proxy (available as of 2.4.2, - see lp:1782236) in a format suitable for passing to an application that - reacts to proxy settings passed as environment variables. Some applications - support lowercase or uppercase notation (e.g. curl), some support only - lowercase (e.g. wget), there are also subjectively rare cases of only - uppercase notation support. no_proxy CIDR and wildcard support also varies - between runtimes and applications as there is no enforced standard. - - Some applications may connect to multiple destinations and expose config - options that would affect only proxy settings for a specific destination - these should be handled in charms in an application-specific manner. - - :param selected_settings: format only a subset of possible settings - :type selected_settings: dict - :rtype: Option(None, dict[str, str]) - """ - SUPPORTED_SETTINGS = { - 'http': 'HTTP_PROXY', - 'https': 'HTTPS_PROXY', - 'no_proxy': 'NO_PROXY', - 'ftp': 'FTP_PROXY' - } - if selected_settings is None: - selected_settings = SUPPORTED_SETTINGS - - selected_vars = [v for k, v in SUPPORTED_SETTINGS.items() - if k in selected_settings] - proxy_settings = {} - for var in selected_vars: - var_val = os.getenv(var) - if var_val: - proxy_settings[var] = var_val - proxy_settings[var.lower()] = var_val - # Now handle juju-prefixed environment variables. The legacy vs new - # environment variable usage is mutually exclusive - charm_var_val = os.getenv('JUJU_CHARM_{}'.format(var)) - if charm_var_val: - proxy_settings[var] = charm_var_val - proxy_settings[var.lower()] = charm_var_val - if 'no_proxy' in proxy_settings: - if _contains_range(proxy_settings['no_proxy']): - log(RANGE_WARNING, level=WARNING) - return proxy_settings if proxy_settings else None + cmd_env=env_proxy_settings(['https'])) def _add_cloud_pocket(pocket): diff --git a/tests/core/test_hookenv.py b/tests/core/test_hookenv.py index 8645b84cc..6d45eecac 100644 --- a/tests/core/test_hookenv.py +++ b/tests/core/test_hookenv.py @@ -1518,6 +1518,129 @@ def test_application_version_set(self, check_call_): hookenv.application_version_set('v1.2.3') check_call_.assert_called_with(['application-version-set', 'v1.2.3']) + @patch.object(os, 'getenv') + @patch.object(hookenv, 'log') + def test_env_proxy_settings_juju_charm_all_selected(self, faux_log, + get_env): + expected_settings = { + 'HTTP_PROXY': 'http://squid.internal:3128', + 'http_proxy': 'http://squid.internal:3128', + 'HTTPS_PROXY': 'https://squid.internals:3128', + 'https_proxy': 'https://squid.internals:3128', + 'NO_PROXY': '192.0.2.0/24,198.51.100.0/24,.bar.com', + 'no_proxy': '192.0.2.0/24,198.51.100.0/24,.bar.com', + 'FTP_PROXY': 'ftp://ftp.internal:21', + 'ftp_proxy': 'ftp://ftp.internal:21', + } + + def get_env_side_effect(var): + return { + 'HTTP_PROXY': None, + 'HTTPS_PROXY': None, + 'NO_PROXY': None, + 'FTP_PROXY': None, + 'JUJU_CHARM_HTTP_PROXY': 'http://squid.internal:3128', + 'JUJU_CHARM_HTTPS_PROXY': 'https://squid.internals:3128', + 'JUJU_CHARM_FTP_PROXY': 'ftp://ftp.internal:21', + 'JUJU_CHARM_NO_PROXY': '192.0.2.0/24,198.51.100.0/24,.bar.com' + }[var] + get_env.side_effect = get_env_side_effect + + proxy_settings = hookenv.env_proxy_settings() + get_env.assert_has_calls([call("HTTP_PROXY"), + call("HTTPS_PROXY"), + call("NO_PROXY"), + call("FTP_PROXY"), + call("JUJU_CHARM_HTTP_PROXY"), + call("JUJU_CHARM_HTTPS_PROXY"), + call("JUJU_CHARM_FTP_PROXY"), + call("JUJU_CHARM_NO_PROXY")], + any_order=True) + self.assertEqual(expected_settings, proxy_settings) + # Verify that we logged a warning about the cidr in NO_PROXY. + faux_log.assert_called_with(hookenv.RANGE_WARNING, + level=hookenv.WARNING) + + @patch.object(os, 'getenv') + def test_env_proxy_settings_legacy_https(self, get_env): + expected_settings = { + 'HTTPS_PROXY': 'http://squid.internal:3128', + 'https_proxy': 'http://squid.internal:3128', + } + + def get_env_side_effect(var): + return { + 'HTTPS_PROXY': 'http://squid.internal:3128', + 'JUJU_CHARM_HTTPS_PROXY': None, + }[var] + get_env.side_effect = get_env_side_effect + + proxy_settings = hookenv.env_proxy_settings(['https']) + get_env.assert_has_calls([call("HTTPS_PROXY"), + call("JUJU_CHARM_HTTPS_PROXY")], + any_order=True) + self.assertEqual(expected_settings, proxy_settings) + + @patch.object(os, 'getenv') + def test_env_proxy_settings_juju_charm_https(self, get_env): + expected_settings = { + 'HTTPS_PROXY': 'http://squid.internal:3128', + 'https_proxy': 'http://squid.internal:3128', + } + + def get_env_side_effect(var): + return { + 'HTTPS_PROXY': None, + 'JUJU_CHARM_HTTPS_PROXY': 'http://squid.internal:3128', + }[var] + get_env.side_effect = get_env_side_effect + + proxy_settings = hookenv.env_proxy_settings(['https']) + get_env.assert_has_calls([call("HTTPS_PROXY"), + call("JUJU_CHARM_HTTPS_PROXY")], + any_order=True) + self.assertEqual(expected_settings, proxy_settings) + + @patch.object(os, 'getenv') + def test_env_proxy_settings_legacy_http(self, get_env): + expected_settings = { + 'HTTP_PROXY': 'http://squid.internal:3128', + 'http_proxy': 'http://squid.internal:3128', + } + + def get_env_side_effect(var): + return { + 'HTTP_PROXY': 'http://squid.internal:3128', + 'JUJU_CHARM_HTTP_PROXY': None, + }[var] + get_env.side_effect = get_env_side_effect + + proxy_settings = hookenv.env_proxy_settings(['http']) + get_env.assert_has_calls([call("HTTP_PROXY"), + call("JUJU_CHARM_HTTP_PROXY")], + any_order=True) + self.assertEqual(expected_settings, proxy_settings) + + @patch.object(os, 'getenv') + def test_env_proxy_settings_juju_charm_http(self, get_env): + expected_settings = { + 'HTTP_PROXY': 'http://squid.internal:3128', + 'http_proxy': 'http://squid.internal:3128', + } + + def get_env_side_effect(var): + return { + 'HTTP_PROXY': None, + 'JUJU_CHARM_HTTP_PROXY': 'http://squid.internal:3128', + }[var] + get_env.side_effect = get_env_side_effect + + proxy_settings = hookenv.env_proxy_settings(['http']) + get_env.assert_has_calls([call("HTTP_PROXY"), + call("JUJU_CHARM_HTTP_PROXY")], + any_order=True) + self.assertEqual(expected_settings, proxy_settings) + class HooksTest(TestCase): def setUp(self): @@ -2142,3 +2265,25 @@ def test_unit_doomed(self, has_juju_version, goal_state, local_unit): local_unit.return_value = 'postgresql/0' self.assertTrue(hookenv.unit_doomed()) + + def test_contains_addr_range(self): + # Contains cidr + self.assertTrue(hookenv._contains_range("192.168.1/20")) + self.assertTrue(hookenv._contains_range("192.168.0/24")) + self.assertTrue( + hookenv._contains_range("10.40.50.1,192.168.1/20,10.56.78.9")) + self.assertTrue(hookenv._contains_range("192.168.22/24")) + self.assertTrue(hookenv._contains_range("2001:db8::/32")) + self.assertTrue(hookenv._contains_range("*.foo.com")) + self.assertTrue(hookenv._contains_range(".foo.com")) + self.assertTrue( + hookenv._contains_range("192.168.1.20,.foo.com")) + self.assertTrue( + hookenv._contains_range("192.168.1.20, .foo.com")) + self.assertTrue( + hookenv._contains_range("192.168.1.20,*.foo.com")) + + # Doesn't contain cidr + self.assertFalse(hookenv._contains_range("192.168.1")) + self.assertFalse(hookenv._contains_range("192.168.145")) + self.assertFalse(hookenv._contains_range("192.16.14")) diff --git a/tests/fetch/test_fetch_ubuntu.py b/tests/fetch/test_fetch_ubuntu.py index 333377f4f..e0fb73ab0 100644 --- a/tests/fetch/test_fetch_ubuntu.py +++ b/tests/fetch/test_fetch_ubuntu.py @@ -981,147 +981,3 @@ def test_apt_autoremove_nonfatal(self, run_apt_command): ['apt-get', '--assume-yes', 'autoremove'], False ) - - @patch.object(os, 'getenv') - @patch('charmhelpers.fetch.ubuntu.log') - def test_env_proxy_settings_juju_charm_all_selected(self, faux_log, - get_env): - expected_settings = { - 'HTTP_PROXY': 'http://squid.internal:3128', - 'http_proxy': 'http://squid.internal:3128', - 'HTTPS_PROXY': 'https://squid.internals:3128', - 'https_proxy': 'https://squid.internals:3128', - 'NO_PROXY': '192.0.2.0/24,198.51.100.0/24,.bar.com', - 'no_proxy': '192.0.2.0/24,198.51.100.0/24,.bar.com', - 'FTP_PROXY': 'ftp://ftp.internal:21', - 'ftp_proxy': 'ftp://ftp.internal:21', - } - - def get_env_side_effect(var): - return { - 'HTTP_PROXY': None, - 'HTTPS_PROXY': None, - 'NO_PROXY': None, - 'FTP_PROXY': None, - 'JUJU_CHARM_HTTP_PROXY': 'http://squid.internal:3128', - 'JUJU_CHARM_HTTPS_PROXY': 'https://squid.internals:3128', - 'JUJU_CHARM_FTP_PROXY': 'ftp://ftp.internal:21', - 'JUJU_CHARM_NO_PROXY': '192.0.2.0/24,198.51.100.0/24,.bar.com' - }[var] - get_env.side_effect = get_env_side_effect - - proxy_settings = fetch._env_proxy_settings() - get_env.assert_has_calls([call("HTTP_PROXY"), - call("HTTPS_PROXY"), - call("NO_PROXY"), - call("FTP_PROXY"), - call("JUJU_CHARM_HTTP_PROXY"), - call("JUJU_CHARM_HTTPS_PROXY"), - call("JUJU_CHARM_FTP_PROXY"), - call("JUJU_CHARM_NO_PROXY")], - any_order=True) - self.assertEqual(expected_settings, proxy_settings) - # Verify that we logged a warning about the cidr in NO_PROXY. - faux_log.assert_called_with(fetch.RANGE_WARNING, level=fetch.WARNING) - - @patch.object(os, 'getenv') - def test_env_proxy_settings_legacy_https(self, get_env): - expected_settings = { - 'HTTPS_PROXY': 'http://squid.internal:3128', - 'https_proxy': 'http://squid.internal:3128', - } - - def get_env_side_effect(var): - return { - 'HTTPS_PROXY': 'http://squid.internal:3128', - 'JUJU_CHARM_HTTPS_PROXY': None, - }[var] - get_env.side_effect = get_env_side_effect - - proxy_settings = fetch._env_proxy_settings(['https']) - get_env.assert_has_calls([call("HTTPS_PROXY"), - call("JUJU_CHARM_HTTPS_PROXY")], - any_order=True) - self.assertEqual(expected_settings, proxy_settings) - - @patch.object(os, 'getenv') - def test_env_proxy_settings_juju_charm_https(self, get_env): - expected_settings = { - 'HTTPS_PROXY': 'http://squid.internal:3128', - 'https_proxy': 'http://squid.internal:3128', - } - - def get_env_side_effect(var): - return { - 'HTTPS_PROXY': None, - 'JUJU_CHARM_HTTPS_PROXY': 'http://squid.internal:3128', - }[var] - get_env.side_effect = get_env_side_effect - - proxy_settings = fetch._env_proxy_settings(['https']) - get_env.assert_has_calls([call("HTTPS_PROXY"), - call("JUJU_CHARM_HTTPS_PROXY")], - any_order=True) - self.assertEqual(expected_settings, proxy_settings) - - @patch.object(os, 'getenv') - def test_env_proxy_settings_legacy_http(self, get_env): - expected_settings = { - 'HTTP_PROXY': 'http://squid.internal:3128', - 'http_proxy': 'http://squid.internal:3128', - } - - def get_env_side_effect(var): - return { - 'HTTP_PROXY': 'http://squid.internal:3128', - 'JUJU_CHARM_HTTP_PROXY': None, - }[var] - get_env.side_effect = get_env_side_effect - - proxy_settings = fetch._env_proxy_settings(['http']) - get_env.assert_has_calls([call("HTTP_PROXY"), - call("JUJU_CHARM_HTTP_PROXY")], - any_order=True) - self.assertEqual(expected_settings, proxy_settings) - - @patch.object(os, 'getenv') - def test_env_proxy_settings_juju_charm_http(self, get_env): - expected_settings = { - 'HTTP_PROXY': 'http://squid.internal:3128', - 'http_proxy': 'http://squid.internal:3128', - } - - def get_env_side_effect(var): - return { - 'HTTP_PROXY': None, - 'JUJU_CHARM_HTTP_PROXY': 'http://squid.internal:3128', - }[var] - get_env.side_effect = get_env_side_effect - - proxy_settings = fetch._env_proxy_settings(['http']) - get_env.assert_has_calls([call("HTTP_PROXY"), - call("JUJU_CHARM_HTTP_PROXY")], - any_order=True) - self.assertEqual(expected_settings, proxy_settings) - - def test_contains_addr_range(self): - # Contains cidr - self.assertTrue(fetch._contains_range("192.168.1/20")) - self.assertTrue(fetch._contains_range("192.168.0/24")) - self.assertTrue( - fetch._contains_range("10.40.50.1,192.168.1/20,10.56.78.9")) - self.assertTrue(fetch._contains_range("192.168.22/24")) - self.assertTrue(fetch._contains_range("2001:db8::/32")) - self.assertTrue(fetch._contains_range("*.foo.com")) - self.assertTrue(fetch._contains_range(".foo.com")) - self.assertTrue( - fetch._contains_range("192.168.1.20,.foo.com")) - self.assertTrue( - fetch._contains_range("192.168.1.20, .foo.com")) - self.assertTrue( - fetch._contains_range("192.168.1.20,*.foo.com")) - - # Doesn't contain cidr - self.assertFalse(fetch._contains_range("192.168.1")) - self.assertFalse(fetch._contains_range("192.168.145")) - self.assertFalse(fetch._contains_range("192.16.14"))