From ffc61816c8cf7d6d3f1129b0d520a661b3b8cddf Mon Sep 17 00:00:00 2001 From: Guang Yee Date: Wed, 2 Jan 2019 10:43:19 -0800 Subject: [PATCH] support http/https proxy for discovery url HTTP(S) proxy can be specified when creating the template. https://docs.openstack.org/magnum/latest/admin/magnum-proxy.html However, it is not being utilized when talking to a public etcd discovery service, which result in failed cluster creation. We need to be able to use HTTP(S) proxy when services are running behind a firewall. Change-Id: I13d86b0dc7c232a51149107f0412219388d8c2cd story: 2004664 --- .../kubernetes/fragments/configure-etcd.sh | 14 +- magnum/drivers/heat/k8s_template_def.py | 3 +- .../drivers/heat/swarm_fedora_template_def.py | 3 +- magnum/drivers/heat/template_def.py | 28 ++- .../handlers/test_k8s_cluster_conductor.py | 3 +- .../unit/drivers/test_template_definition.py | 166 +++++++++++++++++- 6 files changed, 208 insertions(+), 9 deletions(-) diff --git a/magnum/drivers/common/templates/kubernetes/fragments/configure-etcd.sh b/magnum/drivers/common/templates/kubernetes/fragments/configure-etcd.sh index 177ad9235f..00a81403aa 100644 --- a/magnum/drivers/common/templates/kubernetes/fragments/configure-etcd.sh +++ b/magnum/drivers/common/templates/kubernetes/fragments/configure-etcd.sh @@ -84,6 +84,16 @@ EOF fi -if [ -n "$HTTP_PROXY" ]; then - echo "ETCD_DISCOVERY_PROXY=$HTTP_PROXY" >> /etc/etcd/etcd.conf +if [ -n "$HTTP_PROXY" -o "$HTTPS_PROXY" ]; then + ETCD_DISCOVERY_PROTOCOL=$(python -c "from six.moves.urllib import parse as urlparse; print urlparse.urlparse('${ETCD_DISCOVERY_URL}').scheme") + ETCD_DISCOVERY_HOSTNAME=$(python -c "from six.moves.urllib import parse as urlparse; print urlparse.urlparse('${ETCD_DISCOVERY_URL}').netloc.partition(':')[0]") + # prints 1 if $ETCD_DISCOVERY_HOSTNAME is listed explicitly in $NO_PROXY, or $NO_PROXY is set to "*" + ETCD_DISCOVERY_PROXY_BYPASS=$(NO_PROXY="${NO_PROXY}" python -c "import requests; print requests.utils.proxy_bypass('${ETCD_DISCOVERY_HOSTNAME}')") + if [ $ETCD_DISCOVERY_PROXY_BYPASS == "0" ]; then + if [ -n "$HTTP_PROXY" -a "$ETCD_DISCOVERY_PROTOCOL" == "http" ]; then + echo "ETCD_DISCOVERY_PROXY=$HTTP_PROXY" >> /etc/etcd/etcd.conf + elif [ -n "$HTTPS_PROXY" -a "$ETCD_DISCOVERY_PROTOCOL" == "https" ]; then + echo "ETCD_DISCOVERY_PROXY=$HTTPS_PROXY" >> /etc/etcd/etcd.conf + fi + fi fi diff --git a/magnum/drivers/heat/k8s_template_def.py b/magnum/drivers/heat/k8s_template_def.py index e5f4f74009..6750791f84 100644 --- a/magnum/drivers/heat/k8s_template_def.py +++ b/magnum/drivers/heat/k8s_template_def.py @@ -93,7 +93,8 @@ class K8sTemplateDefinition(template_def.BaseTemplateDefinition): def get_params(self, context, cluster_template, cluster, **kwargs): extra_params = kwargs.pop('extra_params', {}) - extra_params['discovery_url'] = self.get_discovery_url(cluster) + extra_params['discovery_url'] = \ + self.get_discovery_url(cluster, cluster_template=cluster_template) osc = self.get_osc(context) extra_params['magnum_url'] = osc.magnum_url() diff --git a/magnum/drivers/heat/swarm_fedora_template_def.py b/magnum/drivers/heat/swarm_fedora_template_def.py index 00fbc497b2..eeb411fb0a 100644 --- a/magnum/drivers/heat/swarm_fedora_template_def.py +++ b/magnum/drivers/heat/swarm_fedora_template_def.py @@ -90,7 +90,8 @@ class SwarmFedoraTemplateDefinition(template_def.BaseTemplateDefinition): def get_params(self, context, cluster_template, cluster, **kwargs): extra_params = kwargs.pop('extra_params', {}) - extra_params['discovery_url'] = self.get_discovery_url(cluster) + extra_params['discovery_url'] = \ + self.get_discovery_url(cluster, cluster_template=cluster_template) # HACK(apmelton) - This uses the user's bearer token, ideally # it should be replaced with an actual trust token with only # access to do what the template needs it to do. diff --git a/magnum/drivers/heat/template_def.py b/magnum/drivers/heat/template_def.py index e7fb24c6b9..e90b3b9e81 100755 --- a/magnum/drivers/heat/template_def.py +++ b/magnum/drivers/heat/template_def.py @@ -15,8 +15,10 @@ import abc import ast from oslo_log import log as logging +import re import requests import six +from six.moves.urllib import parse as urlparse from magnum.common import clients from magnum.common import exception @@ -303,7 +305,26 @@ class BaseTemplateDefinition(TemplateDefinition): size=int(value), discovery_url=discovery_url) - def get_discovery_url(self, cluster): + def get_proxies(self, url, cluster_template): + proxies = dict() + if cluster_template is None: + return proxies + hostname = urlparse.urlparse(url).netloc.partition(":")[0] + if hasattr(cluster_template, 'no_proxy') and \ + cluster_template.no_proxy and \ + (cluster_template.no_proxy == '*' or + re.search('\\b%s\\b' % re.escape(hostname), + cluster_template.no_proxy, re.I)): + LOG.debug('Bypass proxy, because discovery hostname is listed in' + ' cluster template no_proxy variable') + else: + if hasattr(cluster_template, 'http_proxy'): + proxies['http'] = cluster_template.http_proxy + if hasattr(cluster_template, 'https_proxy'): + proxies['https'] = cluster_template.https_proxy + return proxies + + def get_discovery_url(self, cluster, cluster_template=None): if hasattr(cluster, 'discovery_url') and cluster.discovery_url: if getattr(cluster, 'master_count', None) is not None: self.validate_discovery_url(cluster.discovery_url, @@ -316,7 +337,10 @@ class BaseTemplateDefinition(TemplateDefinition): CONF.cluster.etcd_discovery_service_endpoint_format % {'size': cluster.master_count}) try: - discovery_request = requests.get(discovery_endpoint) + proxies = self.get_proxies(discovery_endpoint, + cluster_template) + discovery_request = requests.get(discovery_endpoint, + proxies=proxies) if discovery_request.status_code != requests.codes.ok: raise exception.GetDiscoveryUrlFailed( discovery_endpoint=discovery_endpoint) diff --git a/magnum/tests/unit/conductor/handlers/test_k8s_cluster_conductor.py b/magnum/tests/unit/conductor/handlers/test_k8s_cluster_conductor.py index 305e2c96e6..0db68604b9 100644 --- a/magnum/tests/unit/conductor/handlers/test_k8s_cluster_conductor.py +++ b/magnum/tests/unit/conductor/handlers/test_k8s_cluster_conductor.py @@ -961,7 +961,8 @@ class TestClusterConductorWithK8s(base.TestCase): '../../common/templates/environments/disable_floating_ip.yaml', ], env_files) - reqget.assert_called_once_with('http://etcd/test?size=1') + reqget.assert_called_once_with('http://etcd/test?size=1', proxies={ + 'http': 'http_proxy', 'https': 'https_proxy'}) @patch('magnum.common.short_id.generate_id') @patch('heatclient.common.template_utils.get_template_contents') diff --git a/magnum/tests/unit/drivers/test_template_definition.py b/magnum/tests/unit/drivers/test_template_definition.py index 825acec3e4..8e9ffed840 100644 --- a/magnum/tests/unit/drivers/test_template_definition.py +++ b/magnum/tests/unit/drivers/test_template_definition.py @@ -703,7 +703,92 @@ class AtomicK8sTemplateDefinitionTestCase(BaseK8sTemplateDefinitionTestCase): k8s_def = k8sa_tdef.AtomicK8sTemplateDefinition() discovery_url = k8s_def.get_discovery_url(mock_cluster) - mock_get.assert_called_once_with('http://etcd/test?size=10') + mock_get.assert_called_once_with('http://etcd/test?size=10', + proxies={}) + self.assertEqual(expected_discovery_url, mock_cluster.discovery_url) + self.assertEqual(expected_discovery_url, discovery_url) + + @mock.patch('requests.get') + def test_k8s_get_discovery_url_proxy(self, mock_get): + CONF.set_override('etcd_discovery_service_endpoint_format', + 'http://etcd/test?size=%(size)d', + group='cluster') + expected_discovery_url = 'http://etcd/token' + mock_resp = mock.MagicMock() + mock_resp.status_code = 200 + mock_resp.text = expected_discovery_url + mock_get.return_value = mock_resp + mock_cluster = mock.MagicMock() + mock_cluster.master_count = 10 + mock_cluster.discovery_url = None + + mock_cluster_template = mock.MagicMock() + mock_cluster_template.http_proxy = 'http_proxy' + mock_cluster_template.https_proxy = 'https_proxy' + mock_cluster_template.no_proxy = 'localhost,127.0.0.1' + + k8s_def = k8sa_tdef.AtomicK8sTemplateDefinition() + discovery_url = k8s_def.get_discovery_url(mock_cluster, + mock_cluster_template) + + mock_get.assert_called_once_with('http://etcd/test?size=10', proxies={ + 'http': 'http_proxy', 'https': 'https_proxy'}) + self.assertEqual(expected_discovery_url, mock_cluster.discovery_url) + self.assertEqual(expected_discovery_url, discovery_url) + + @mock.patch('requests.get') + def test_k8s_get_discovery_url_no_proxy(self, mock_get): + CONF.set_override('etcd_discovery_service_endpoint_format', + 'http://etcd/test?size=%(size)d', + group='cluster') + expected_discovery_url = 'http://etcd/token' + mock_resp = mock.MagicMock() + mock_resp.status_code = 200 + mock_resp.text = expected_discovery_url + mock_get.return_value = mock_resp + mock_cluster = mock.MagicMock() + mock_cluster.master_count = 10 + mock_cluster.discovery_url = None + + mock_cluster_template = mock.MagicMock() + mock_cluster_template.http_proxy = 'http_proxy' + mock_cluster_template.https_proxy = 'https_proxy' + mock_cluster_template.no_proxy = 'localhost,127.0.0.1,etcd' + + k8s_def = k8sa_tdef.AtomicK8sTemplateDefinition() + discovery_url = k8s_def.get_discovery_url(mock_cluster, + mock_cluster_template) + + mock_get.assert_called_once_with('http://etcd/test?size=10', + proxies={}) + self.assertEqual(expected_discovery_url, mock_cluster.discovery_url) + self.assertEqual(expected_discovery_url, discovery_url) + + @mock.patch('requests.get') + def test_k8s_get_discovery_url_no_proxy_wildcard(self, mock_get): + CONF.set_override('etcd_discovery_service_endpoint_format', + 'http://etcd/test?size=%(size)d', + group='cluster') + expected_discovery_url = 'http://etcd/token' + mock_resp = mock.MagicMock() + mock_resp.status_code = 200 + mock_resp.text = expected_discovery_url + mock_get.return_value = mock_resp + mock_cluster = mock.MagicMock() + mock_cluster.master_count = 10 + mock_cluster.discovery_url = None + + mock_cluster_template = mock.MagicMock() + mock_cluster_template.http_proxy = 'http_proxy' + mock_cluster_template.https_proxy = 'https_proxy' + mock_cluster_template.no_proxy = '*' + + k8s_def = k8sa_tdef.AtomicK8sTemplateDefinition() + discovery_url = k8s_def.get_discovery_url(mock_cluster, + mock_cluster_template) + + mock_get.assert_called_once_with('http://etcd/test?size=10', + proxies={}) self.assertEqual(expected_discovery_url, mock_cluster.discovery_url) self.assertEqual(expected_discovery_url, discovery_url) @@ -1276,7 +1361,84 @@ class AtomicSwarmTemplateDefinitionTestCase(base.TestCase): swarm_def = swarm_tdef.AtomicSwarmTemplateDefinition() discovery_url = swarm_def.get_discovery_url(mock_cluster) - mock_get.assert_called_once_with('http://etcd/test?size=1') + mock_get.assert_called_once_with('http://etcd/test?size=1', proxies={}) + self.assertEqual(mock_cluster.discovery_url, expected_discovery_url) + self.assertEqual(discovery_url, expected_discovery_url) + + @mock.patch('requests.get') + def test_swarm_get_discovery_url_proxy(self, mock_get): + CONF.set_override('etcd_discovery_service_endpoint_format', + 'http://etcd/test?size=%(size)d', + group='cluster') + expected_discovery_url = 'http://etcd/token' + mock_resp = mock.MagicMock() + mock_resp.status_code = 200 + mock_resp.text = expected_discovery_url + mock_get.return_value = mock_resp + mock_cluster = mock.MagicMock() + mock_cluster.discovery_url = None + + mock_cluster_template = mock.MagicMock() + mock_cluster_template.http_proxy = 'http_proxy' + mock_cluster_template.https_proxy = 'https_proxy' + mock_cluster_template.no_proxy = 'localhost,127.0.0.1' + + swarm_def = swarm_tdef.AtomicSwarmTemplateDefinition() + discovery_url = swarm_def.get_discovery_url(mock_cluster, + mock_cluster_template) + + mock_get.assert_called_once_with('http://etcd/test?size=1', proxies={ + 'http': 'http_proxy', 'https': 'https_proxy'}) + self.assertEqual(mock_cluster.discovery_url, expected_discovery_url) + self.assertEqual(discovery_url, expected_discovery_url) + + @mock.patch('requests.get') + def test_swarm_get_discovery_url_no_proxy(self, mock_get): + CONF.set_override('etcd_discovery_service_endpoint_format', + 'http://etcd/test?size=%(size)d', + group='cluster') + expected_discovery_url = 'http://etcd/token' + mock_resp = mock.MagicMock() + mock_resp.status_code = 200 + mock_resp.text = expected_discovery_url + mock_get.return_value = mock_resp + mock_cluster = mock.MagicMock() + mock_cluster.discovery_url = None + + mock_cluster_template = mock.MagicMock() + mock_cluster_template.http_proxy = 'http_proxy' + mock_cluster_template.https_proxy = 'https_proxy' + mock_cluster_template.no_proxy = 'etcd,localhost,127.0.0.1' + + swarm_def = swarm_tdef.AtomicSwarmTemplateDefinition() + discovery_url = swarm_def.get_discovery_url(mock_cluster) + + mock_get.assert_called_once_with('http://etcd/test?size=1', proxies={}) + self.assertEqual(mock_cluster.discovery_url, expected_discovery_url) + self.assertEqual(discovery_url, expected_discovery_url) + + @mock.patch('requests.get') + def test_swarm_get_discovery_url_no_proxy_wildcard(self, mock_get): + CONF.set_override('etcd_discovery_service_endpoint_format', + 'http://etcd/test?size=%(size)d', + group='cluster') + expected_discovery_url = 'http://etcd/token' + mock_resp = mock.MagicMock() + mock_resp.status_code = 200 + mock_resp.text = expected_discovery_url + mock_get.return_value = mock_resp + mock_cluster = mock.MagicMock() + mock_cluster.discovery_url = None + + mock_cluster_template = mock.MagicMock() + mock_cluster_template.http_proxy = 'http_proxy' + mock_cluster_template.https_proxy = 'https_proxy' + mock_cluster_template.no_proxy = '*' + + swarm_def = swarm_tdef.AtomicSwarmTemplateDefinition() + discovery_url = swarm_def.get_discovery_url(mock_cluster) + + mock_get.assert_called_once_with('http://etcd/test?size=1', proxies={}) self.assertEqual(mock_cluster.discovery_url, expected_discovery_url) self.assertEqual(discovery_url, expected_discovery_url)