Add proxy related parameters to agent driver

This change adds image_http_proxy, image_https_proxy, image_no_proxy
parameters to a node's driver_info for use by the agent driver. If set,
they will be passed to ironic python agent ramdisk and used during
image download.

Also this change updates the unit test
TestAgentVendor.test_continue_deploy_image_source_is_url() as it was was
incorrect.

Partial-bug: #1526222

Change-Id: I7890f761f4adbe768f76831ef5b48b691a70b2d1

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
@@ -93,7 +95,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):
@@ -172,6 +189,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."""
 
@@ -228,6 +281,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.
@@ -398,6 +453,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)
 

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',

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.