Revert "support http/https proxy for discovery url"
This reverts commit f86de684d2
.
This commit is reverted for two reasons:
* It is undesirable that the end user can inject proxy config into
the magnum-conductor service via the cluster template.
* The proxy settings for the magnum-conductor service may not be
the same as those which are required in the cluster template for
the end user VM.
Systemd, docker and podman all include native mechanisms for setting
environment variables for proecesses, and this should be used by the
cloud operator / deployment tooling to configure the required proxy
settings for the magnum-conductor service.
In particular this patch makes it impossible for the cloud operator
to specify their own http_proxy via the environment, the user supplied
cluster template setting will always be used.
Change-Id: I33da19ad6764bedcf15f2a08381063e2471f8991
This commit is contained in:
parent
ee653e0fed
commit
4ecbda5ab1
|
@ -84,16 +84,6 @@ EOF
|
|||
|
||||
fi
|
||||
|
||||
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
|
||||
if [ -n "$HTTP_PROXY" ]; then
|
||||
echo "ETCD_DISCOVERY_PROXY=$HTTP_PROXY" >> /etc/etcd/etcd.conf
|
||||
fi
|
||||
|
|
|
@ -98,8 +98,7 @@ class K8sTemplateDefinition(template_def.BaseTemplateDefinition):
|
|||
extra_params['minions_to_remove'] = (
|
||||
scale_mgr.get_removal_nodes(hosts))
|
||||
|
||||
extra_params['discovery_url'] = \
|
||||
self.get_discovery_url(cluster, cluster_template=cluster_template)
|
||||
extra_params['discovery_url'] = self.get_discovery_url(cluster)
|
||||
osc = self.get_osc(context)
|
||||
extra_params['magnum_url'] = osc.magnum_url()
|
||||
|
||||
|
|
|
@ -90,8 +90,7 @@ 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, cluster_template=cluster_template)
|
||||
extra_params['discovery_url'] = self.get_discovery_url(cluster)
|
||||
# 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.
|
||||
|
|
|
@ -15,10 +15,8 @@ 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
|
||||
|
@ -302,26 +300,7 @@ class BaseTemplateDefinition(TemplateDefinition):
|
|||
size=int(value),
|
||||
discovery_url=discovery_url)
|
||||
|
||||
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):
|
||||
def get_discovery_url(self, cluster):
|
||||
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,
|
||||
|
@ -334,14 +313,7 @@ class BaseTemplateDefinition(TemplateDefinition):
|
|||
CONF.cluster.etcd_discovery_service_endpoint_format %
|
||||
{'size': cluster.master_count})
|
||||
try:
|
||||
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)
|
||||
discovery_url = discovery_request.text
|
||||
discovery_url = requests.get(discovery_endpoint).text
|
||||
except req_exceptions.RequestException as err:
|
||||
LOG.error(six.text_type(err))
|
||||
raise exception.GetDiscoveryUrlFailed(
|
||||
|
|
|
@ -581,7 +581,6 @@ class TestClusterConductorWithK8s(base.TestCase):
|
|||
self.cluster_template_dict['cluster_distro'] = 'coreos'
|
||||
self.cluster_dict['discovery_url'] = None
|
||||
mock_req = mock.MagicMock(text='http://tokentest/h1/h2/h3')
|
||||
mock_req.status_code = 200
|
||||
reqget.return_value = mock_req
|
||||
cluster_template = objects.ClusterTemplate(
|
||||
self.context, **self.cluster_template_dict)
|
||||
|
@ -765,7 +764,6 @@ class TestClusterConductorWithK8s(base.TestCase):
|
|||
'http://etcd/test?size=%(size)d',
|
||||
group='cluster')
|
||||
mock_req = mock.MagicMock(text='https://address/token')
|
||||
mock_req.status_code = 200
|
||||
reqget.return_value = mock_req
|
||||
|
||||
(template_path,
|
||||
|
@ -841,8 +839,7 @@ class TestClusterConductorWithK8s(base.TestCase):
|
|||
'../../common/templates/environments/disable_floating_ip.yaml',
|
||||
],
|
||||
env_files)
|
||||
reqget.assert_called_once_with('http://etcd/test?size=1', proxies={
|
||||
'http': 'http_proxy', 'https': 'https_proxy'})
|
||||
reqget.assert_called_once_with('http://etcd/test?size=1')
|
||||
|
||||
@patch('magnum.common.short_id.generate_id')
|
||||
@patch('heatclient.common.template_utils.get_template_contents')
|
||||
|
|
|
@ -634,7 +634,6 @@ class AtomicK8sTemplateDefinitionTestCase(BaseK8sTemplateDefinitionTestCase):
|
|||
expected_discovery_url = 'http://etcd/token'
|
||||
mock_resp = mock.MagicMock()
|
||||
mock_resp.text = expected_discovery_url
|
||||
mock_resp.status_code = 200
|
||||
mock_get.return_value = mock_resp
|
||||
mock_cluster = mock.MagicMock()
|
||||
mock_cluster.master_count = 10
|
||||
|
@ -643,92 +642,7 @@ 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',
|
||||
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={})
|
||||
mock_get.assert_called_once_with('http://etcd/test?size=10')
|
||||
self.assertEqual(expected_discovery_url, mock_cluster.discovery_url)
|
||||
self.assertEqual(expected_discovery_url, discovery_url)
|
||||
|
||||
|
@ -757,7 +671,6 @@ class AtomicK8sTemplateDefinitionTestCase(BaseK8sTemplateDefinitionTestCase):
|
|||
def test_k8s_get_discovery_url_not_found(self, mock_get):
|
||||
mock_resp = mock.MagicMock()
|
||||
mock_resp.text = ''
|
||||
mock_resp.status_code = 200
|
||||
mock_get.return_value = mock_resp
|
||||
|
||||
fake_cluster = mock.MagicMock()
|
||||
|
@ -1293,7 +1206,6 @@ class AtomicSwarmTemplateDefinitionTestCase(base.TestCase):
|
|||
expected_discovery_url = 'http://etcd/token'
|
||||
mock_resp = mock.MagicMock()
|
||||
mock_resp.text = expected_discovery_url
|
||||
mock_resp.status_code = 200
|
||||
mock_get.return_value = mock_resp
|
||||
mock_cluster = mock.MagicMock()
|
||||
mock_cluster.discovery_url = None
|
||||
|
@ -1301,84 +1213,7 @@ 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', 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={})
|
||||
mock_get.assert_called_once_with('http://etcd/test?size=1')
|
||||
self.assertEqual(mock_cluster.discovery_url, expected_discovery_url)
|
||||
self.assertEqual(discovery_url, expected_discovery_url)
|
||||
|
||||
|
@ -1386,7 +1221,6 @@ class AtomicSwarmTemplateDefinitionTestCase(base.TestCase):
|
|||
def test_swarm_get_discovery_url_not_found(self, mock_get):
|
||||
mock_resp = mock.MagicMock()
|
||||
mock_resp.text = ''
|
||||
mock_resp.status_code = 200
|
||||
mock_get.return_value = mock_resp
|
||||
|
||||
fake_cluster = mock.MagicMock()
|
||||
|
|
Loading…
Reference in New Issue