openstack/ironic
Code Issues Proposed changes

Change [agent]require_tls to True by default

Browse Source 
While looking at the overall heartbeat/agent workflow, it seemed
like the [agent]require_tls setting should likely be True by
default, as we are well past the initial phase where operators
might not have the TLS capability when upgrading.

Change-Id: Id526e948e6c5ed032d7542232b1c1a31cb285b26
This commit is contained in:
Julia Kreger 2024-06-04 11:00:37 -07:00
parent 88c45151fa
commit 6095b4017d
3 changed files with 22 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
ironic/conf/agent.py
View File

@ -153,10 +153,12 @@ opts = [
help=_('Wait time in seconds between attempts for validating ' help=_('Wait time in seconds between attempts for validating '
'Neutron agent status.')), 'Neutron agent status.')),
cfg.BoolOpt('require_tls', cfg.BoolOpt('require_tls',
default=False, default=True,
mutable=True, mutable=True,
help=_('If set to True, callback URLs without https:// will ' help=_('If set to False, callback URLs without https:// will '
'be rejected by the conductor.')), 'be permitted by the conductor, which may be needed '
'for backwards compatibility outside of the supported '
'version window.')),
cfg.StrOpt('certificates_path', cfg.StrOpt('certificates_path',
default='/var/lib/ironic/certificates', default='/var/lib/ironic/certificates',
help=_('Path to store auto-generated TLS certificates used to ' help=_('Path to store auto-generated TLS certificates used to '

24
ironic/tests/unit/conductor/test_manager.py
View File

@ -7669,11 +7669,11 @@ class HeartbeatTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
mock_spawn.side_effect = self._fake_spawn mock_spawn.side_effect = self._fake_spawn
self.service.heartbeat(self.context, node.uuid, 'http://callback', self.service.heartbeat(self.context, node.uuid, 'https://callback',
agent_version=None, agent_token='magic', agent_version=None, agent_token='magic',
agent_status='start') agent_status='start')
mock_heartbeat.assert_called_with(mock.ANY, mock.ANY, mock_heartbeat.assert_called_with(mock.ANY, mock.ANY,
'http://callback', None, 'https://callback', None,
None, 'start', None) None, 'start', None)
@mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat', @mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat',
@ -7682,6 +7682,7 @@ class HeartbeatTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
autospec=True) autospec=True)
def test_heartbeat_with_agent_version(self, mock_spawn, mock_heartbeat): def test_heartbeat_with_agent_version(self, mock_spawn, mock_heartbeat):
"""Test heartbeating.""" """Test heartbeating."""
self.config(require_tls=False, group='agent')
node = obj_utils.create_test_node( node = obj_utils.create_test_node(
self.context, driver='fake-hardware', self.context, driver='fake-hardware',
provision_state=states.DEPLOYING, provision_state=states.DEPLOYING,
@ -7743,10 +7744,10 @@ class HeartbeatTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
mock_spawn.side_effect = self._fake_spawn mock_spawn.side_effect = self._fake_spawn
self.service.heartbeat(self.context, node.uuid, 'http://callback', self.service.heartbeat(self.context, node.uuid, 'https://callback',
'6.1.0', agent_token='a secret') '6.1.0', agent_token='a secret')
mock_heartbeat.assert_called_with(mock.ANY, mock.ANY, mock_heartbeat.assert_called_with(mock.ANY, mock.ANY,
'http://callback', '6.1.0', None, 'https://callback', '6.1.0', None,
None, None) None, None)
@mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat', @mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat',
@ -7768,10 +7769,10 @@ class HeartbeatTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
mock_spawn.side_effect = self._fake_spawn mock_spawn.side_effect = self._fake_spawn
self.service.heartbeat(self.context, node.uuid, 'http://callback', self.service.heartbeat(self.context, node.uuid, 'https://callback',
'6.1.0', agent_token='a secret') '6.1.0', agent_token='a secret')
mock_heartbeat.assert_called_with(mock.ANY, mock.ANY, mock_heartbeat.assert_called_with(mock.ANY, mock.ANY,
'http://callback', '6.1.0', None, 'https://callback', '6.1.0', None,
None, None) None, None)
@mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat', @mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat',
@ -7795,7 +7796,7 @@ class HeartbeatTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
exc = self.assertRaises(messaging.rpc.ExpectedException, exc = self.assertRaises(messaging.rpc.ExpectedException,
self.service.heartbeat, self.context, self.service.heartbeat, self.context,
node.uuid, 'http://callback', node.uuid, 'https://callback',
agent_token='evil', agent_version='5.0.0b23') agent_token='evil', agent_version='5.0.0b23')
self.assertEqual(exception.InvalidParameterValue, exc.exc_info[0]) self.assertEqual(exception.InvalidParameterValue, exc.exc_info[0])
self.assertFalse(mock_heartbeat.called) self.assertFalse(mock_heartbeat.called)
@ -7822,7 +7823,7 @@ class HeartbeatTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
# occurs. # occurs.
exc = self.assertRaises(messaging.rpc.ExpectedException, exc = self.assertRaises(messaging.rpc.ExpectedException,
self.service.heartbeat, self.context, self.service.heartbeat, self.context,
node.uuid, 'http://callback', node.uuid, 'https://callback',
agent_token='evil', agent_version='4.0.0') agent_token='evil', agent_version='4.0.0')
self.assertEqual(exception.InvalidParameterValue, exc.exc_info[0]) self.assertEqual(exception.InvalidParameterValue, exc.exc_info[0])
self.assertFalse(mock_heartbeat.called) self.assertFalse(mock_heartbeat.called)
@ -7847,7 +7848,7 @@ class HeartbeatTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
exc = self.assertRaises(messaging.rpc.ExpectedException, exc = self.assertRaises(messaging.rpc.ExpectedException,
self.service.heartbeat, self.context, self.service.heartbeat, self.context,
node.uuid, 'http://callback', node.uuid, 'https://callback',
agent_token=None, agent_version='6.1.5') agent_token=None, agent_version='6.1.5')
self.assertEqual(exception.InvalidParameterValue, exc.exc_info[0]) self.assertEqual(exception.InvalidParameterValue, exc.exc_info[0])
self.assertFalse(mock_heartbeat.called) self.assertFalse(mock_heartbeat.called)
@ -7858,7 +7859,6 @@ class HeartbeatTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
autospec=True) autospec=True)
def test_heartbeat_tls_required(self, mock_spawn, mock_heartbeat): def test_heartbeat_tls_required(self, mock_spawn, mock_heartbeat):
"""Heartbeat fails when it does not match.""" """Heartbeat fails when it does not match."""
self.config(require_tls=True, group='agent')
node = obj_utils.create_test_node( node = obj_utils.create_test_node(
self.context, driver='fake-hardware', self.context, driver='fake-hardware',
provision_state=states.DEPLOYING, provision_state=states.DEPLOYING,
@ -7900,11 +7900,11 @@ class HeartbeatTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
mock_spawn.reset_mock() mock_spawn.reset_mock()
mock_spawn.side_effect = self._fake_spawn mock_spawn.side_effect = self._fake_spawn
self.service.heartbeat(self.context, node.uuid, 'http://callback', self.service.heartbeat(self.context, node.uuid, 'https://callback',
agent_version='6.1.0', agent_token='a secret', agent_version='6.1.0', agent_token='a secret',
agent_verify_ca='abcd') agent_verify_ca='abcd')
mock_heartbeat.assert_called_with( mock_heartbeat.assert_called_with(
mock.ANY, mock.ANY, 'http://callback', '6.1.0', '/path/to/crt', mock.ANY, mock.ANY, 'https://callback', '6.1.0', '/path/to/crt',
None, None) None, None)

5
releasenotes/notes/set-agent-to-require-tls-08a9571793e75943.yaml Normal file
View File

@ -0,0 +1,5 @@
---
security:
- |
Agent communication now requires an HTTPS url by default. This can be
changed using the ``[agent]require_tls`` setting.