Skip to content

Commit cc79140

Browse files
Zuulopenstack-gerrit
Zuul
authored and
openstack-gerrit
committed
Merge "Respect global parameters when downloading a configdrive" into stable/xena
2 parents f9ed56e + 442fc43 commit cc79140

File tree

3 files changed

+90
-7
lines changed

3 files changed

+90
-7
lines changed

‎ironic_python_agent/partition_utils.py

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,17 @@
2929
from ironic_lib import exception
3030
from ironic_lib import utils
3131
from oslo_concurrency import processutils
32+
from oslo_config import cfg
3233
from oslo_log import log
3334
from oslo_utils import excutils
3435
from oslo_utils import units
3536
import requests
3637

38+
from ironic_python_agent import utils as ipa_utils
39+
3740

3841
LOG = log.getLogger()
42+
CONF = cfg.CONF
3943

4044
MAX_CONFIG_DRIVE_SIZE_MB = 64
4145

@@ -59,13 +63,27 @@ def get_configdrive(configdrive, node_uuid, tempdir=None):
5963
# Check if the configdrive option is a HTTP URL or the content directly
6064
is_url = utils.is_http_url(configdrive)
6165
if is_url:
66+
verify, cert = ipa_utils.get_ssl_client_options(CONF)
67+
timeout = CONF.image_download_connection_timeout
68+
# TODO(dtantsur): support proxy parameters from instance_info
6269
try:
63-
data = requests.get(configdrive).content
70+
resp = requests.get(configdrive, verify=verify, cert=cert,
71+
timeout=timeout)
6472
except requests.exceptions.RequestException as e:
6573
raise exception.InstanceDeployFailure(
6674
"Can't download the configdrive content for node %(node)s "
6775
"from '%(url)s'. Reason: %(reason)s" %
6876
{'node': node_uuid, 'url': configdrive, 'reason': e})
77+
78+
if resp.status_code >= 400:
79+
raise exception.InstanceDeployFailure(
80+
"Can't download the configdrive content for node %(node)s "
81+
"from '%(url)s'. Got status code %(code)s, response "
82+
"body %(body)s" %
83+
{'node': node_uuid, 'url': configdrive,
84+
'code': resp.status_code, 'body': resp.text})
85+
86+
data = resp.content
6987
else:
7088
data = configdrive
7189

‎ironic_python_agent/tests/unit/test_partition_utils.py

Lines changed: 59 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,26 +32,68 @@ class GetConfigdriveTestCase(base.IronicAgentTest):
3232

3333
@mock.patch.object(gzip, 'GzipFile', autospec=True)
3434
def test_get_configdrive(self, mock_gzip, mock_requests, mock_copy):
35-
mock_requests.return_value = mock.MagicMock(content='Zm9vYmFy')
35+
mock_requests.return_value = mock.MagicMock(content='Zm9vYmFy',
36+
status_code=200)
3637
tempdir = tempfile.mkdtemp()
3738
(size, path) = partition_utils.get_configdrive('http://1.2.3.4/cd',
3839
'fake-node-uuid',
3940
tempdir=tempdir)
4041
self.assertTrue(path.startswith(tempdir))
41-
mock_requests.assert_called_once_with('http://1.2.3.4/cd')
42+
mock_requests.assert_called_once_with('http://1.2.3.4/cd',
43+
verify=True, cert=None,
44+
timeout=60)
45+
mock_gzip.assert_called_once_with('configdrive', 'rb',
46+
fileobj=mock.ANY)
47+
mock_copy.assert_called_once_with(mock.ANY, mock.ANY)
48+
49+
@mock.patch.object(gzip, 'GzipFile', autospec=True)
50+
def test_get_configdrive_insecure(self, mock_gzip, mock_requests,
51+
mock_copy):
52+
self.config(insecure=True)
53+
mock_requests.return_value = mock.MagicMock(content='Zm9vYmFy',
54+
status_code=200)
55+
tempdir = tempfile.mkdtemp()
56+
(size, path) = partition_utils.get_configdrive('http://1.2.3.4/cd',
57+
'fake-node-uuid',
58+
tempdir=tempdir)
59+
self.assertTrue(path.startswith(tempdir))
60+
mock_requests.assert_called_once_with('http://1.2.3.4/cd',
61+
verify=False, cert=None,
62+
timeout=60)
63+
mock_gzip.assert_called_once_with('configdrive', 'rb',
64+
fileobj=mock.ANY)
65+
mock_copy.assert_called_once_with(mock.ANY, mock.ANY)
66+
67+
@mock.patch.object(gzip, 'GzipFile', autospec=True)
68+
def test_get_configdrive_ssl(self, mock_gzip, mock_requests, mock_copy):
69+
self.config(cafile='cafile', keyfile='keyfile', certfile='certfile')
70+
mock_requests.return_value = mock.MagicMock(content='Zm9vYmFy',
71+
status_code=200)
72+
tempdir = tempfile.mkdtemp()
73+
(size, path) = partition_utils.get_configdrive('http://1.2.3.4/cd',
74+
'fake-node-uuid',
75+
tempdir=tempdir)
76+
self.assertTrue(path.startswith(tempdir))
77+
mock_requests.assert_called_once_with('http://1.2.3.4/cd',
78+
verify='cafile',
79+
cert=('certfile', 'keyfile'),
80+
timeout=60)
4281
mock_gzip.assert_called_once_with('configdrive', 'rb',
4382
fileobj=mock.ANY)
4483
mock_copy.assert_called_once_with(mock.ANY, mock.ANY)
4584

4685
def test_get_configdrive_binary(self, mock_requests, mock_copy):
47-
mock_requests.return_value = mock.MagicMock(content=b'content')
86+
mock_requests.return_value = mock.MagicMock(content=b'content',
87+
status_code=200)
4888
tempdir = tempfile.mkdtemp()
4989
(size, path) = partition_utils.get_configdrive('http://1.2.3.4/cd',
5090
'fake-node-uuid',
5191
tempdir=tempdir)
5292
self.assertTrue(path.startswith(tempdir))
5393
self.assertEqual(b'content', open(path, 'rb').read())
54-
mock_requests.assert_called_once_with('http://1.2.3.4/cd')
94+
mock_requests.assert_called_once_with('http://1.2.3.4/cd',
95+
verify=True, cert=None,
96+
timeout=60)
5597
self.assertFalse(mock_copy.called)
5698

5799
@mock.patch.object(gzip, 'GzipFile', autospec=True)
@@ -70,6 +112,14 @@ def test_get_configdrive_bad_url(self, mock_requests, mock_copy):
70112
'http://1.2.3.4/cd', 'fake-node-uuid')
71113
self.assertFalse(mock_copy.called)
72114

115+
def test_get_configdrive_bad_status_code(self, mock_requests, mock_copy):
116+
mock_requests.return_value = mock.MagicMock(text='Not found',
117+
status_code=404)
118+
self.assertRaises(exception.InstanceDeployFailure,
119+
partition_utils.get_configdrive,
120+
'http://1.2.3.4/cd', 'fake-node-uuid')
121+
self.assertFalse(mock_copy.called)
122+
73123
def test_get_configdrive_base64_error(self, mock_requests, mock_copy):
74124
self.assertRaises(exception.InstanceDeployFailure,
75125
partition_utils.get_configdrive,
@@ -79,12 +129,15 @@ def test_get_configdrive_base64_error(self, mock_requests, mock_copy):
79129
@mock.patch.object(gzip, 'GzipFile', autospec=True)
80130
def test_get_configdrive_gzip_error(self, mock_gzip, mock_requests,
81131
mock_copy):
82-
mock_requests.return_value = mock.MagicMock(content='Zm9vYmFy')
132+
mock_requests.return_value = mock.MagicMock(content='Zm9vYmFy',
133+
status_code=200)
83134
mock_copy.side_effect = IOError
84135
self.assertRaises(exception.InstanceDeployFailure,
85136
partition_utils.get_configdrive,
86137
'http://1.2.3.4/cd', 'fake-node-uuid')
87-
mock_requests.assert_called_once_with('http://1.2.3.4/cd')
138+
mock_requests.assert_called_once_with('http://1.2.3.4/cd',
139+
verify=True, cert=None,
140+
timeout=60)
88141
mock_gzip.assert_called_once_with('configdrive', 'rb',
89142
fileobj=mock.ANY)
90143
mock_copy.assert_called_once_with(mock.ANY, mock.ANY)

‎releasenotes/notes/configdrive-ssl-02b069948dfef814.yaml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
fixes:
3+
- |
4+
No longer ignores global TLS configuration options (``ipa-insecure``, etc)
5+
when downloading a configdrive via a URL.
6+
- |
7+
No longer ignores error status codes from the server when downloading
8+
a configdrive via a URL.
9+
- |
10+
The configdrive downloading code now respects the
11+
``ipa-image-download-connection-timeout`` option and will no longer hang
12+
for a long time if the server does not respond.

0 commit comments

Comments
 (0)