Merge "Add proxy related parameters to agent driver"

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)
 

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.