From 94fde4b3b461ccd0283ed844e3785fb44d5562ae Mon Sep 17 00:00:00 2001
From: Julia Kreger <juliaashleykreger@gmail.com>
Date: Thu, 13 Feb 2025 07:23:00 -0800
Subject: [PATCH] Remove agent_token_required upgrade knob
To help ease upgrades to Victoria, IPA had a knob added
to enable operators to express if agent tokens were required
in their deployment. Since then, the feature is required, however
we left the logic enabling the fun upgrade case handling.
At this point, this knob serves no further use, and can be removed.
Change-Id: I202f06e1b6598a802c9853fb99201c55e7a40cb1
---
ironic_python_agent/agent.py | 19 ++++++++++---------
ironic_python_agent/config.py | 6 ------
.../tests/unit/extensions/test_poll.py | 6 ++----
ironic_python_agent/tests/unit/test_agent.py | 4 +---
...agent-token-required-808e99b83b4456a1.yaml | 9 +++++++++
5 files changed, 22 insertions(+), 22 deletions(-)
create mode 100644 releasenotes/notes/remove-agent-token-required-808e99b83b4456a1.yaml
diff --git a/ironic_python_agent/agent.py b/ironic_python_agent/agent.py
index ef74bd2b4..fbc616c61 100644
--- a/ironic_python_agent/agent.py
+++ b/ironic_python_agent/agent.py
@@ -253,7 +253,6 @@ class IronicPythonAgent(base.ExecuteCommandMixin):
# Allows this to be turned on by the conductor while running,
# in the event of long running ramdisks where the conductor
# got upgraded somewhere along the way.
- self.agent_token_required = cfg.CONF.agent_token_required
self.generated_cert = None
def get_status(self):
@@ -470,8 +469,6 @@ class IronicPythonAgent(base.ExecuteCommandMixin):
md5_allowed = config.get('agent_md5_checksum_enable')
if md5_allowed is not None:
cfg.CONF.set_override('md5_enabled', md5_allowed)
- if config.get('agent_token_required'):
- self.agent_token_required = True
token = config.get('agent_token')
if token:
if len(token) >= 32:
@@ -487,11 +484,15 @@ class IronicPythonAgent(base.ExecuteCommandMixin):
'intended and the deployment may fail '
'depending on settings in the ironic '
'deployment.')
- if not self.agent_token and self.agent_token_required:
- LOG.error('Ironic is signaling that agent tokens '
- 'are required, however we do not have '
- 'a token on file. '
- 'This is likely **FATAL**.')
+ if not self.agent_token:
+ LOG.error('We do not have a token on file '
+ 'from the Ironic deployment, and '
+ 'one should be on file. '
+ 'Possible external agent restart '
+ 'outside of Ironic\'s process. '
+ 'This is **FATAL**.')
+ self.serve_api = False
+ self.lockdown = True
else:
LOG.info('An invalid token was received.')
if self.agent_token and not self.standalone:
@@ -558,7 +559,7 @@ class IronicPythonAgent(base.ExecuteCommandMixin):
'found, please check your pxe append parameters.')
in_rescued_mode = os.path.exists('/etc/.rescued')
- if not in_rescued_mode:
+ if not in_rescued_mode and self.serve_api:
self.serve_ipa_api()
else:
# NOTE(cid): In rescued state, we don't call _lockdown_system() as
diff --git a/ironic_python_agent/config.py b/ironic_python_agent/config.py
index 1ce795bb7..395821601 100644
--- a/ironic_python_agent/config.py
+++ b/ironic_python_agent/config.py
@@ -292,12 +292,6 @@ cli_opts = [
'This variable can be also configured via image_info.'
'Value coming from image_info takes precedence over'
'value coming from command line or configuration file.'),
- cfg.BoolOpt('agent_token_required',
- default=APARAMS.get('ipa-agent-token-required', False),
- help='Control to enforce if API command requests should '
- 'enforce token validation. The configuration provided '
- 'by the conductor MAY override this and force this '
- 'setting to be changed to True in memory.'),
cfg.IntOpt('image_download_connection_timeout', min=1,
default=APARAMS.get(
'ipa-image-download-connection-timeout', 60),
diff --git a/ironic_python_agent/tests/unit/extensions/test_poll.py b/ironic_python_agent/tests/unit/extensions/test_poll.py
index b144d1047..757b8699a 100644
--- a/ironic_python_agent/tests/unit/extensions/test_poll.py
+++ b/ironic_python_agent/tests/unit/extensions/test_poll.py
@@ -42,8 +42,7 @@ class TestPollExtension(base.IronicAgentTest):
def test_set_node_info_success(self):
self.mock_agent.standalone = True
node_info = {'node': {'uuid': 'fake-node', 'properties': {}},
- 'config': {'agent_token_required': True,
- 'agent_token': 'blah' * 8}}
+ 'config': {'agent_token': 'blah' * 8}}
result = self.agent_extension.set_node_info(node_info=node_info)
self.mock_agent.process_lookup_data.assert_called_once_with(node_info)
self.assertEqual('SUCCEEDED', result.command_status)
@@ -51,8 +50,7 @@ class TestPollExtension(base.IronicAgentTest):
def test_set_node_info_not_standalone(self):
self.mock_agent.standalone = False
node_info = {'node': {'uuid': 'fake-node', 'properties': {}},
- 'config': {'agent_token_required': True,
- 'agent_token': 'blah' * 8}}
+ 'config': {'agent_token': 'blah' * 8}}
self.assertRaises(errors.InvalidCommandError,
self.agent_extension.set_node_info,
node_info=node_info)
diff --git a/ironic_python_agent/tests/unit/test_agent.py b/ironic_python_agent/tests/unit/test_agent.py
index 113208a84..4e840ead5 100644
--- a/ironic_python_agent/tests/unit/test_agent.py
+++ b/ironic_python_agent/tests/unit/test_agent.py
@@ -465,7 +465,6 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest):
'config': {
'heartbeat_timeout': 300,
'agent_token': '1' * 128,
- 'agent_token_required': True
}
}
@@ -1323,7 +1322,6 @@ class TestBaseAgentVMediaToken(ironic_agent_base.IronicAgentTest):
def test_run_agent_token_vmedia(self, mock_get_managers, mock_wsgi,
mock_wait, mock_dispatch):
CONF.set_override('inspection_callback_url', '')
-
wsgi_server = mock_wsgi.return_value
def set_serve_api():
@@ -1339,11 +1337,11 @@ class TestBaseAgentVMediaToken(ironic_agent_base.IronicAgentTest):
'config': {
'heartbeat_timeout': 300,
'agent_token': '********',
- 'agent_token_required': True
}
}
self.agent.run()
+ self.assertFalse(self.agent.lockdown)
mock_wsgi.assert_called_once_with(CONF, 'ironic-python-agent',
app=self.agent.api,
diff --git a/releasenotes/notes/remove-agent-token-required-808e99b83b4456a1.yaml b/releasenotes/notes/remove-agent-token-required-808e99b83b4456a1.yaml
new file mode 100644
index 000000000..e727c831d
--- /dev/null
+++ b/releasenotes/notes/remove-agent-token-required-808e99b83b4456a1.yaml
@@ -0,0 +1,9 @@
+---
+security:
+ - |
+ The ``agent_token_required`` configuration option has been removed.
+ This was a knob to help ease the upgrade process by enabling operators
+ to express if their environment was requiring an agent token. This was
+ key to help navigate possible upgrade cases, however that was for the
+ migration *to* the Victoria release, and such an upgrade is outside
+ of the version support matrix of Ironic.