Skip to content

Commit

Permalink
Move _env_proxy_settings to hookenv
Browse files Browse the repository at this point in the history 
This function seems general enough for charms to use other than for
add-apt-repository (e.g. curl or wget invocations, application-specific
proxy config and so on). It also belongs to hook tools and helpers and,
therefore, is moved to hookenv by this change.
  • Loading branch information
Dmitrii Shcherbakov committed Feb 26, 2019
1 parent e52f05a commit 842e5e1
Show file tree
Hide file tree
Showing 4 changed files with 222 additions and 224 deletions.
74 changes: 74 additions & 0 deletions charmhelpers/core/hookenv.py
View file
Expand Up @@ -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 = {}


Expand Down Expand Up @@ -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)
83 changes: 3 additions & 80 deletions charmhelpers/fetch/ubuntu.py
View file
Expand Up @@ -28,6 +28,7 @@
log,
DEBUG,
WARNING,
env_proxy_settings,
)
from charmhelpers.fetch import SourceConfigError, GPGKeyError

Expand Down Expand Up @@ -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."""
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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):
Expand Down
145 changes: 145 additions & 0 deletions tests/core/test_hookenv.py
View file
Expand Up @@ -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):
Expand Down Expand Up @@ -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"))

0 comments on commit 842e5e1

Please sign in to comment.