Merge "agent_client: support custom TLS certificates"

8 files changed, 93 insertions, 22 deletions

diff --git a/ironic/drivers/modules/agent_base.py b/ironic/drivers/modules/agent_base.py
index 49debc5a9..ee9753dcb 100644
--- a/ironic/drivers/modules/agent_base.py
+++ b/ironic/drivers/modules/agent_base.py
@@ -72,7 +72,14 @@ VENDOR_PROPERTIES = {
     'deploy_forces_oob_reboot': _(
         'Whether Ironic should force a reboot of the Node via the out-of-band '
         'channel after deployment is complete. Provides compatibility with '
-        'older deploy ramdisks. Defaults to False. Optional.')
+        'older deploy ramdisks. Defaults to False. Optional.'),
+    'agent_verify_ca': _(
+        'Either a Boolean value, a path to a CA_BUNDLE file or directory with '
+        'certificates of trusted CAs. If set to True ironic will verify '
+        'the agent\'s certificate; if False the driver will ignore verifying '
+        'the SSL certificate. If it\'s a path the driver will use the '
+        'specified certificate or one of the certificates in the '
+        'directory. Defaults to True. Optional'),
 }
 
 __HEARTBEAT_RECORD_ONLY = (states.ENROLL, states.MANAGEABLE, states.AVAILABLE,
diff --git a/ironic/drivers/modules/agent_client.py b/ironic/drivers/modules/agent_client.py
index d1f3d6759..090007d8f 100644
--- a/ironic/drivers/modules/agent_client.py
+++ b/ironic/drivers/modules/agent_client.py
@@ -76,6 +76,9 @@ class AgentClient(object):
             'params': params,
         })
 
+    def _get_verify(self, node):
+        return node.driver_info.get('agent_verify_ca', True)
+
     def _raise_if_typeerror(self, result, node, method):
         error = result.get('command_error')
         if error and error.get('type') == 'TypeError':
@@ -168,6 +171,7 @@ class AgentClient(object):
         try:
             response = self.session.post(
                 url, params=request_params, data=body,
+                verify=self._get_verify(node),
                 timeout=CONF.agent.command_timeout)
         except (requests.ConnectionError, requests.Timeout) as e:
             msg = (_('Failed to connect to the agent running on node %(node)s '
@@ -261,6 +265,7 @@ class AgentClient(object):
         def _get():
             try:
                 return self.session.get(url,
+                                        verify=self._get_verify(node),
                                         timeout=CONF.agent.command_timeout)
             except (requests.ConnectionError, requests.Timeout) as e:
                 msg = (_('Failed to connect to the agent running on node '
diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py
index 74f334068..7ab03e175 100644
--- a/ironic/tests/unit/conductor/test_manager.py
+++ b/ironic/tests/unit/conductor/test_manager.py
@@ -5769,7 +5769,7 @@ class ManagerTestProperties(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
                     enabled_power_interfaces=['ipmitool'],
                     enabled_management_interfaces=['ipmitool'],
                     enabled_console_interfaces=['ipmitool-socat'])
-        expected = ['ipmi_address', 'ipmi_terminal_port',
+        expected = ['agent_verify_ca', 'ipmi_address', 'ipmi_terminal_port',
                     'ipmi_password', 'ipmi_port', 'ipmi_priv_level',
                     'ipmi_username', 'ipmi_bridging', 'ipmi_transit_channel',
                     'ipmi_transit_address', 'ipmi_target_channel',
@@ -5784,7 +5784,7 @@ class ManagerTestProperties(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
     def test_driver_properties_snmp(self):
         self.config(enabled_hardware_types='snmp',
                     enabled_power_interfaces=['snmp'])
-        expected = ['deploy_kernel', 'deploy_ramdisk',
+        expected = ['agent_verify_ca', 'deploy_kernel', 'deploy_ramdisk',
                     'force_persistent_boot_device',
                     'rescue_kernel', 'rescue_ramdisk',
                     'snmp_driver', 'snmp_address', 'snmp_port', 'snmp_version',
@@ -5805,9 +5805,9 @@ class ManagerTestProperties(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
                     enabled_boot_interfaces=['ilo-virtual-media'],
                     enabled_inspect_interfaces=['ilo'],
                     enabled_console_interfaces=['ilo'])
-        expected = ['ilo_address', 'ilo_username', 'ilo_password',
-                    'client_port', 'client_timeout', 'ilo_deploy_iso',
-                    'console_port', 'ilo_change_password',
+        expected = ['agent_verify_ca', 'ilo_address', 'ilo_username',
+                    'ilo_password', 'client_port', 'client_timeout',
+                    'ilo_deploy_iso', 'console_port', 'ilo_change_password',
                     'ca_file', 'snmp_auth_user', 'snmp_auth_prot_password',
                     'snmp_auth_priv_password', 'snmp_auth_protocol',
                     'snmp_auth_priv_protocol', 'deploy_forces_oob_reboot']
@@ -5835,7 +5835,7 @@ class ManagerTestHardwareTypeProperties(mgr_utils.ServiceSetUpMixin,
         self.assertEqual(sorted(expected), sorted(properties))
 
     def test_hardware_type_properties_manual_management(self):
-        expected = ['deploy_kernel', 'deploy_ramdisk',
+        expected = ['agent_verify_ca', 'deploy_kernel', 'deploy_ramdisk',
                     'force_persistent_boot_device', 'deploy_forces_oob_reboot',
                     'rescue_kernel', 'rescue_ramdisk']
         self._check_hardware_type_properties('manual-management', expected)
diff --git a/ironic/tests/unit/drivers/modules/ansible/test_deploy.py b/ironic/tests/unit/drivers/modules/ansible/test_deploy.py
index cef1fabd4..17ab45786 100644
--- a/ironic/tests/unit/drivers/modules/ansible/test_deploy.py
+++ b/ironic/tests/unit/drivers/modules/ansible/test_deploy.py
@@ -576,7 +576,7 @@ class TestAnsibleDeploy(AnsibleDeployTestCaseBase):
     def test_get_properties(self):
         self.assertEqual(
             set(list(ansible_deploy.COMMON_PROPERTIES)
-                + ['deploy_forces_oob_reboot']),
+                + ['agent_verify_ca', 'deploy_forces_oob_reboot']),
             set(self.driver.get_properties()))
 
     @mock.patch.object(deploy_utils, 'check_for_missing_params',
diff --git a/ironic/tests/unit/drivers/modules/test_agent_client.py b/ironic/tests/unit/drivers/modules/test_agent_client.py
index 69aa03495..1f8cc1f6a 100644
--- a/ironic/tests/unit/drivers/modules/test_agent_client.py
+++ b/ironic/tests/unit/drivers/modules/test_agent_client.py
@@ -62,13 +62,15 @@ class MockNode(object):
             'hardware_manager_version': {'generic': '1'}
         }
         self.instance_info = {}
+        self.driver_info = {}
 
     def as_dict(self, secure=False):
         assert secure, 'agent_client must pass secure=True'
         return {
             'uuid': self.uuid,
             'driver_internal_info': self.driver_internal_info,
-            'instance_info': self.instance_info
+            'instance_info': self.instance_info,
+            'driver_info': self.driver_info,
         }
 
 
@@ -117,7 +119,8 @@ class TestAgentClient(base.TestCase):
             url,
             data=body,
             params={'wait': 'false'},
-            timeout=60)
+            timeout=60,
+            verify=True)
 
     def test__command_fail_json(self):
         response_text = 'this be not json matey!'
@@ -137,7 +140,8 @@ class TestAgentClient(base.TestCase):
             url,
             data=body,
             params={'wait': 'false'},
-            timeout=60)
+            timeout=60,
+            verify=True)
 
     def test__command_fail_post(self):
         error = 'Boom'
@@ -177,7 +181,8 @@ class TestAgentClient(base.TestCase):
             url,
             data=mock.ANY,
             params={'wait': 'false'},
-            timeout=60)
+            timeout=60,
+            verify=True)
         self.assertEqual(3, self.client.session.post.call_count)
 
     def test__command_error_code(self):
@@ -198,7 +203,8 @@ class TestAgentClient(base.TestCase):
             url,
             data=body,
             params={'wait': 'false'},
-            timeout=60)
+            timeout=60,
+            verify=True)
 
     def test__command_error_code_okay_error_typeerror_embedded(self):
         response_data = {"faultstring": "you dun goofd",
@@ -218,7 +224,29 @@ class TestAgentClient(base.TestCase):
             url,
             data=body,
             params={'wait': 'false'},
-            timeout=60)
+            timeout=60,
+            verify=True)
+
+    def test__command_verify(self):
+        response_data = {'status': 'ok'}
+        self.client.session.post.return_value = MockResponse(response_data)
+        method = 'standby.run_image'
+        image_info = {'image_id': 'test_image'}
+        params = {'image_info': image_info}
+
+        self.node.driver_info['agent_verify_ca'] = '/path/to/agent.crt'
+
+        url = self.client._get_command_url(self.node)
+        body = self.client._get_command_body(method, params)
+
+        response = self.client._command(self.node, method, params)
+        self.assertEqual(response, response_data)
+        self.client.session.post.assert_called_once_with(
+            url,
+            data=body,
+            params={'wait': 'false'},
+            timeout=60,
+            verify='/path/to/agent.crt')
 
     @mock.patch('time.sleep', lambda seconds: None)
     def test__command_poll(self):
@@ -247,8 +275,10 @@ class TestAgentClient(base.TestCase):
             url,
             data=body,
             params={'wait': 'false'},
-            timeout=60)
-        self.client.session.get.assert_called_with(url, timeout=60)
+            timeout=60,
+            verify=True)
+        self.client.session.get.assert_called_with(url, timeout=60,
+                                                   verify=True)
 
     def test_get_commands_status(self):
         with mock.patch.object(self.client.session, 'get',
@@ -262,7 +292,8 @@ class TestAgentClient(base.TestCase):
                 '%(agent_url)s/%(api_version)s/commands' % {
                     'agent_url': agent_url,
                     'api_version': CONF.agent.agent_api_version},
-                timeout=CONF.agent.command_timeout)
+                timeout=CONF.agent.command_timeout,
+                verify=True)
 
     def test_get_commands_status_retries(self):
         res = mock.MagicMock(spec_set=['json'])
@@ -281,6 +312,23 @@ class TestAgentClient(base.TestCase):
                           retry_connection=False)
         self.assertEqual(1, self.client.session.get.call_count)
 
+    def test_get_commands_status_verify(self):
+        self.node.driver_info['agent_verify_ca'] = '/path/to/agent.crt'
+
+        with mock.patch.object(self.client.session, 'get',
+                               autospec=True) as mock_get:
+            res = mock.MagicMock(spec_set=['json'])
+            res.json.return_value = {'commands': []}
+            mock_get.return_value = res
+            self.assertEqual([], self.client.get_commands_status(self.node))
+            agent_url = self.node.driver_internal_info.get('agent_url')
+            mock_get.assert_called_once_with(
+                '%(agent_url)s/%(api_version)s/commands' % {
+                    'agent_url': agent_url,
+                    'api_version': CONF.agent.agent_api_version},
+                timeout=CONF.agent.command_timeout,
+                verify='/path/to/agent.crt')
+
     def test_prepare_image(self):
         self.client._command = mock.MagicMock(spec_set=[])
         image_info = {'image_id': 'image'}
@@ -500,7 +548,8 @@ class TestAgentClient(base.TestCase):
             data=body,
             params={'wait': 'false',
                     'agent_token': 'magical'},
-            timeout=60)
+            timeout=60,
+            verify=True)
 
 
 class TestAgentClientAttempts(base.TestCase):
@@ -553,7 +602,8 @@ class TestAgentClientAttempts(base.TestCase):
             self.client._get_command_url(self.node),
             data=self.client._get_command_body(method, params),
             params={'wait': 'false'},
-            timeout=60)
+            timeout=60,
+            verify=True)
 
     @mock.patch.object(retrying.time, 'sleep', autospec=True)
     def test__command_succeed_after_one_timeout(self, mock_sleep):
@@ -574,4 +624,5 @@ class TestAgentClientAttempts(base.TestCase):
             self.client._get_command_url(self.node),
             data=self.client._get_command_body(method, params),
             params={'wait': 'false'},
-            timeout=60)
+            timeout=60,
+            verify=True)
diff --git a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py
index 0923df714..b6eb12771 100644
--- a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py
+++ b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py
@@ -616,7 +616,8 @@ class ISCSIDeployTestCase(db_base.DbTestCase):
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=True) as task:
             props = task.driver.deploy.get_properties()
-            self.assertEqual(['deploy_forces_oob_reboot'], list(props))
+            self.assertEqual({'agent_verify_ca', 'deploy_forces_oob_reboot'},
+                             set(props))
 
     @mock.patch.object(iscsi_deploy, 'validate', autospec=True)
     @mock.patch.object(deploy_utils, 'validate_capabilities', autospec=True)
diff --git a/ironic/tests/unit/drivers/test_generic.py b/ironic/tests/unit/drivers/test_generic.py
index 6a1d07863..d1acb59b8 100644
--- a/ironic/tests/unit/drivers/test_generic.py
+++ b/ironic/tests/unit/drivers/test_generic.py
@@ -68,7 +68,8 @@ class ManualManagementHardwareTestCase(db_base.DbTestCase):
     def test_get_properties(self):
         # These properties are from vendor (agent) and boot (pxe) interfaces
         expected_prop_keys = [
-            'deploy_forces_oob_reboot', 'deploy_kernel', 'deploy_ramdisk',
+            'agent_verify_ca', 'deploy_forces_oob_reboot',
+            'deploy_kernel', 'deploy_ramdisk',
             'force_persistent_boot_device', 'rescue_kernel', 'rescue_ramdisk']
         hardware_type = driver_factory.get_hardware_type("manual-management")
         properties = hardware_type.get_properties()
diff --git a/releasenotes/notes/agent-verify-ca-ddbfbb0f27198d82.yaml b/releasenotes/notes/agent-verify-ca-ddbfbb0f27198d82.yaml
new file mode 100644
index 000000000..3f945f22d
--- /dev/null
+++ b/releasenotes/notes/agent-verify-ca-ddbfbb0f27198d82.yaml
@@ -0,0 +1,6 @@
+---
+features:
+  - |
+    Adds a new ``driver_info`` parameter ``agent_verify_ca`` that allows
+    specifying a file with certificates to use when accessing IPA. Set
+    to ``False`` to disable certificate validation.