From c6e8281f85f52d2708eb369e2c2fcf395443afdc Mon Sep 17 00:00:00 2001
From: Dmitry Tantsur <dtantsur@protonmail.com>
Date: Thu, 8 Apr 2021 13:42:25 +0200
Subject: [PATCH] Wipe agent tokens on inspection start and abort

Also make sure the pregenerated flag is always reset.

Change-Id: I73aaa803d3eb84ddac59a778e998836a645217eb
---
 ironic/conductor/manager.py                       |  7 ++++---
 ironic/conductor/utils.py                         |  2 ++
 ironic/tests/unit/conductor/test_manager.py       | 15 +++++++++++----
 .../notes/inspection-token-b3d9e8e34341d680.yaml  |  4 ++++
 4 files changed, 21 insertions(+), 7 deletions(-)
 create mode 100644 releasenotes/notes/inspection-token-b3d9e8e34341d680.yaml

diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py
index c26b10930f..2daa73749f 100644
--- a/ironic/conductor/manager.py
+++ b/ironic/conductor/manager.py
@@ -1333,6 +1333,7 @@ class ConductorManager(base_manager.BaseConductorManager):
                                         'Error: %s') % e
                     node.save()
             node.last_error = _('Inspection was aborted by request.')
+            utils.wipe_token_and_url(task)
             task.process_event('abort')
             LOG.info('Successfully aborted inspection of node %(node)s',
                      {'node': node.uuid})
@@ -3680,9 +3681,9 @@ def _do_inspect_hardware(task):
         log_func("Failed to inspect node %(node)s: %(err)s",
                  {'node': node.uuid, 'err': e})
 
-    # Remove agent_url, while not strictly needed for the inspection path,
-    # lets just remove it out of good practice.
-    utils.remove_agent_url(node)
+    # Inspection cannot start in fast-track mode, wipe token and URL.
+    utils.wipe_token_and_url(task)
+
     try:
         new_state = task.driver.inspect.inspect_hardware(task)
     except exception.IronicException as e:
diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py
index 66c5cf8c75..3b8b54c221 100644
--- a/ironic/conductor/utils.py
+++ b/ironic/conductor/utils.py
@@ -1215,6 +1215,8 @@ def add_secret_token(node, pregenerated=False):
     i_info['agent_secret_token'] = token
     if pregenerated:
         i_info['agent_secret_token_pregenerated'] = True
+    else:
+        i_info.pop('agent_secret_token_pregenerated', None)
     node.driver_internal_info = i_info
 
 
diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py
index a4ce0ecd48..6f520df383 100644
--- a/ironic/tests/unit/conductor/test_manager.py
+++ b/ironic/tests/unit/conductor/test_manager.py
@@ -6057,7 +6057,8 @@ class NodeInspectHardware(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
         node = obj_utils.create_test_node(
             self.context, driver='fake-hardware',
             provision_state=states.INSPECTING,
-            driver_internal_info={'agent_url': 'url'})
+            driver_internal_info={'agent_url': 'url',
+                                  'agent_secret_token': 'token'})
         task = task_manager.TaskManager(self.context, node.uuid)
         mock_inspect.return_value = states.MANAGEABLE
         manager._do_inspect_hardware(task)
@@ -6068,6 +6069,7 @@ class NodeInspectHardware(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
         mock_inspect.assert_called_once_with(task.driver.inspect, task)
         task.node.refresh()
         self.assertNotIn('agent_url', task.node.driver_internal_info)
+        self.assertNotIn('agent_secret_token', task.node.driver_internal_info)
 
     @mock.patch('ironic.drivers.modules.fake.FakeInspect.inspect_hardware',
                 autospec=True)
@@ -7879,9 +7881,12 @@ class DoNodeInspectAbortTestCase(mgr_utils.CommonMixIn,
     @mock.patch('ironic.conductor.task_manager.acquire', autospec=True)
     def test_do_inspect_abort_succeeded(self, mock_acquire, mock_abort):
         self._start_service()
-        node = obj_utils.create_test_node(self.context,
-                                          driver='fake-hardware',
-                                          provision_state=states.INSPECTWAIT)
+        node = obj_utils.create_test_node(
+            self.context,
+            driver='fake-hardware',
+            provision_state=states.INSPECTWAIT,
+            driver_internal_info={'agent_url': 'url',
+                                  'agent_secret_token': 'token'})
         task = task_manager.TaskManager(self.context, node.uuid)
         mock_acquire.side_effect = self._get_acquire_side_effect(task)
         self.service.do_provisioning_action(self.context, task.node.uuid,
@@ -7889,3 +7894,5 @@ class DoNodeInspectAbortTestCase(mgr_utils.CommonMixIn,
         node.refresh()
         self.assertEqual('inspect failed', node.provision_state)
         self.assertIn('Inspection was aborted', node.last_error)
+        self.assertNotIn('agent_url', node.driver_internal_info)
+        self.assertNotIn('agent_secret_token', node.driver_internal_info)
diff --git a/releasenotes/notes/inspection-token-b3d9e8e34341d680.yaml b/releasenotes/notes/inspection-token-b3d9e8e34341d680.yaml
new file mode 100644
index 0000000000..89e4c0d593
--- /dev/null
+++ b/releasenotes/notes/inspection-token-b3d9e8e34341d680.yaml
@@ -0,0 +1,4 @@
+---
+fixes:
+  - |
+    Correctly wipes agent token on inspection start and abort.