diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index f2f944a440..2186f55297 100644 --- a/ironic/drivers/modules/agent.py +++ b/ironic/drivers/modules/agent.py @@ -16,6 +16,7 @@ from oslo_config import cfg from oslo_log import log from oslo_utils import excutils from oslo_utils import units +import six.moves.urllib_parse as urlparse from ironic.common import dhcp_factory from ironic.common import exception @@ -29,6 +30,7 @@ from ironic.common import images from ironic.common import paths from ironic.common import raid from ironic.common import states +from ironic.common import utils from ironic.conductor import task_manager from ironic.conductor import utils as manager_utils from ironic.drivers import base @@ -92,7 +94,22 @@ REQUIRED_PROPERTIES = { 'deploy_ramdisk': _('UUID (from Glance) of the ramdisk with agent that is ' 'used at deploy time. Required.'), } -COMMON_PROPERTIES = REQUIRED_PROPERTIES + +OPTIONAL_PROPERTIES = { + 'image_http_proxy': _('URL of a proxy server for HTTP connections. ' + 'Optional.'), + 'image_https_proxy': _('URL of a proxy server for HTTPS connections. ' + 'Optional.'), + 'image_no_proxy': _('A comma-separated list of host names, IP addresses ' + 'and domain names (with optional :port) that will be ' + 'excluded from proxying. To denote a doman name, use ' + 'a dot to prefix the domain name. This value will be ' + 'ignored if ``image_http_proxy`` and ' + '``image_https_proxy`` are not specified. Optional.'), +} + +COMMON_PROPERTIES = REQUIRED_PROPERTIES.copy() +COMMON_PROPERTIES.update(OPTIONAL_PROPERTIES) def build_instance_info_for_deploy(task): @@ -171,6 +188,42 @@ def check_image_size(task, image_source): raise exception.InvalidParameterValue(msg) +def validate_image_proxies(node): + """Check that the provided proxy parameters are valid. + + :param node: an Ironic node. + :raises: InvalidParameterValue if any of the provided proxy parameters are + incorrect. + """ + invalid_proxies = {} + for scheme in ('http', 'https'): + proxy_param = 'image_%s_proxy' % scheme + proxy = node.driver_info.get(proxy_param) + if proxy: + chunks = urlparse.urlparse(proxy) + # NOTE(vdrok) If no scheme specified, this is still a valid + # proxy address. It is also possible for a proxy to have a + # scheme different from the one specified in the image URL, + # e.g. it is possible to use https:// proxy for downloading + # http:// image. + if chunks.scheme not in ('', 'http', 'https'): + invalid_proxies[proxy_param] = proxy + msg = '' + if invalid_proxies: + msg += _("Proxy URL should either have HTTP(S) scheme " + "or no scheme at all, the following URLs are " + "invalid: %s.") % invalid_proxies + no_proxy = node.driver_info.get('image_no_proxy') + if no_proxy is not None and not utils.is_valid_no_proxy(no_proxy): + msg += _( + "image_no_proxy should be a list of host names, IP addresses " + "or domain names to exclude from proxying, the specified list " + "%s is incorrect. To denote a domain name, prefix it with a dot " + "(instead of e.g. '.*').") % no_proxy + if msg: + raise exception.InvalidParameterValue(msg) + + class AgentDeploy(base.DeployInterface): """Interface for deploy-related actions.""" @@ -227,6 +280,8 @@ class AgentDeploy(base.DeployInterface): # Validate node capabilities deploy_utils.validate_capabilities(node) + validate_image_proxies(node) + @task_manager.require_exclusive_lock def deploy(self, task): """Perform a deployment to a node. @@ -397,6 +452,18 @@ class AgentVendorInterface(agent_base_vendor.BaseAgentVendor): 'stream_raw_images': CONF.agent.stream_raw_images, } + proxies = {} + for scheme in ('http', 'https'): + proxy_param = 'image_%s_proxy' % scheme + proxy = node.driver_info.get(proxy_param) + if proxy: + proxies[scheme] = proxy + if proxies: + image_info['proxies'] = proxies + no_proxy = node.driver_info.get('image_no_proxy') + if no_proxy is not None: + image_info['no_proxy'] = no_proxy + # Tell the client to download and write the image with the given args self._client.prepare_image(node, image_info) diff --git a/ironic/tests/unit/drivers/modules/test_agent.py b/ironic/tests/unit/drivers/modules/test_agent.py index cc3c8e9d08..595322b2a8 100644 --- a/ironic/tests/unit/drivers/modules/test_agent.py +++ b/ironic/tests/unit/drivers/modules/test_agent.py @@ -313,6 +313,22 @@ class TestAgentDeploy(db_base.DbTestCase): task.driver.boot, task) show_mock.assert_called_once_with(self.context, 'fake-image') + @mock.patch.object(images, 'image_show', autospec=True) + @mock.patch.object(pxe.PXEBoot, 'validate', autospec=True) + def test_validate_invalid_proxies(self, pxe_boot_validate_mock, show_mock): + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + task.node.driver_info.update({ + 'image_https_proxy': 'git://spam.ni', + 'image_http_proxy': 'http://spam.ni', + 'image_no_proxy': '1' * 500}) + self.assertRaisesRegexp(exception.InvalidParameterValue, + 'image_https_proxy.*image_no_proxy', + task.driver.deploy.validate, task) + pxe_boot_validate_mock.assert_called_once_with( + task.driver.boot, task) + show_mock.assert_called_once_with(self.context, 'fake-image') + @mock.patch('ironic.conductor.utils.node_power_action', autospec=True) def test_deploy(self, power_mock): with task_manager.acquire( @@ -495,10 +511,13 @@ class TestAgentVendor(db_base.DbTestCase): } self.node = object_utils.create_test_node(self.context, **n) - def test_continue_deploy(self): - CONF.set_override('stream_raw_images', False, 'agent') + def _test_continue_deploy(self, additional_driver_info=None, + additional_expected_image_info=None): self.node.provision_state = states.DEPLOYWAIT self.node.target_provision_state = states.ACTIVE + driver_info = self.node.driver_info + driver_info.update(additional_driver_info or {}) + self.node.driver_info = driver_info self.node.save() test_temp_url = 'http://image' expected_image_info = { @@ -507,8 +526,9 @@ class TestAgentVendor(db_base.DbTestCase): 'checksum': 'checksum', 'disk_format': 'qcow2', 'container_format': 'bare', - 'stream_raw_images': False, + 'stream_raw_images': CONF.agent.stream_raw_images, } + expected_image_info.update(additional_expected_image_info or {}) client_mock = mock.MagicMock(spec_set=['prepare_image']) self.passthru._client = client_mock @@ -523,32 +543,34 @@ class TestAgentVendor(db_base.DbTestCase): self.assertEqual(states.ACTIVE, task.node.target_provision_state) + def test_continue_deploy(self): + self._test_continue_deploy() + + def test_continue_deploy_with_proxies(self): + self._test_continue_deploy( + additional_driver_info={'image_https_proxy': 'https://spam.ni', + 'image_http_proxy': 'spam.ni', + 'image_no_proxy': '.eggs.com'}, + additional_expected_image_info={ + 'proxies': {'https': 'https://spam.ni', + 'http': 'spam.ni'}, + 'no_proxy': '.eggs.com'} + ) + + def test_continue_deploy_with_no_proxy_without_proxies(self): + self._test_continue_deploy( + additional_driver_info={'image_no_proxy': '.eggs.com'} + ) + def test_continue_deploy_image_source_is_url(self): - self.node.provision_state = states.DEPLOYWAIT - self.node.target_provision_state = states.ACTIVE - self.node.save() - test_temp_url = 'http://image' - expected_image_info = { - 'urls': [test_temp_url], - 'id': self.node.instance_info['image_source'], - 'checksum': 'checksum', - 'disk_format': 'qcow2', - 'container_format': 'bare', - 'stream_raw_images': True, - } - - client_mock = mock.MagicMock(spec_set=['prepare_image']) - self.passthru._client = client_mock - - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - self.passthru.continue_deploy(task) - - client_mock.prepare_image.assert_called_with(task.node, - expected_image_info) - self.assertEqual(states.DEPLOYWAIT, task.node.provision_state) - self.assertEqual(states.ACTIVE, - task.node.target_provision_state) + instance_info = self.node.instance_info + instance_info['image_source'] = 'http://example.com/woof.img' + self.node.instance_info = instance_info + self._test_continue_deploy( + additional_expected_image_info={ + 'id': 'woof.img' + } + ) @mock.patch.object(manager_utils, 'node_power_action', autospec=True) @mock.patch.object(fake.FakePower, 'get_power_state', diff --git a/releasenotes/notes/add-agent-proxy-support-790e629634ca2eb7.yaml b/releasenotes/notes/add-agent-proxy-support-790e629634ca2eb7.yaml new file mode 100644 index 0000000000..b885efedd7 --- /dev/null +++ b/releasenotes/notes/add-agent-proxy-support-790e629634ca2eb7.yaml @@ -0,0 +1,20 @@ +--- +features: + - Pass proxy information from agent driver to IPA ramdisk, so that images + can be cached on the proxy server. +issues: + - When using caching proxy with ``agent_*`` drivers, caching the image on the + proxy server might involve increasing [glance]swift_temp_url_duration + config option value. This way, the cached entry will be valid for a period + of time long enough to see the benefits of caching. Large temporary URL + duration might become a security issue in some cases. +upgrade: + - Adds a [glance]swift_temp_url_cache_enabled configuration option to enable + Swift temporary URL caching. It is only useful if the caching proxy is + used. Also adds [glance]swift_temp_url_expected_download_start_delay, which + is used to check if the Swift temporary URL duration is long enough to let + the image download to start, and, if temporary URL caching is enabled, to + determine if a cached entry will be still valid when download starts. The + value of [glance]swift_temp_url_expected_download_start_delay must be less + than the value for the [glance]swift_temp_url_duration configuration + option. \ No newline at end of file