openstack
/
magnum
Code Issues Proposed changes

Revert "support http/https proxy for discovery url" 

This reverts commit ffc61816c8. A cherry
pick from master is not used here are there are conflicts to resolve
in magnum/drivers/heat/template_def.py

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:
Jonathan Rosser 2019-06-25 08:46:32 +01:00
parent 9f06126be7
commit de7b9a6a94
6 changed files with 9 additions and 208 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
magnum/drivers/common/templates/kubernetes/fragments/configure-etcd.sh
View File

@ -96,16 +96,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

3
magnum/drivers/heat/k8s_template_def.py
View File

@ -96,8 +96,7 @@ 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, 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()

3
magnum/drivers/heat/swarm_fedora_template_def.py
View File

@ -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.

28
magnum/drivers/heat/template_def.py
View File

@ -16,10 +16,8 @@ import ast
from oslo_log import log as logging
from oslo_utils import strutils
import re
import requests
import six
from six.moves.urllib import parse as urlparse
from magnum.common import clients
from magnum.common import exception
@ -306,26 +304,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,
@ -338,10 +317,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)
discovery_request = requests.get(discovery_endpoint)
if discovery_request.status_code != requests.codes.ok:
raise exception.GetDiscoveryUrlFailed(
discovery_endpoint=discovery_endpoint)

3
magnum/tests/unit/conductor/handlers/test_k8s_cluster_conductor.py
View File

@ -985,8 +985,7 @@ class TestClusterConductorWithK8s(base.TestCase):
'../../common/templates/environments/disable_lb_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')

166
magnum/tests/unit/drivers/test_template_definition.py
View File

@ -1034,92 +1034,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)
@ -1692,84 +1607,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)