summaryrefslogtreecommitdiff
path: root/ironic
diff options
context:
space:
mode:
authorJulia Kreger <juliaashleykreger@gmail.com>2019-11-01 13:29:36 -0700
committerJulia Kreger <juliaashleykreger@gmail.com>2020-02-20 14:33:32 -0800
commitbb3b2349f9c044a6b51e7b425c2290c887bb4cb2 (patch)
treecc34fb8cb75e608ac656ee709b7d1363dfc6c0ca /ironic
parentd86b0f61d7b18a57e6eee6995000b59280b2fbb9 (diff)
downloadironic-bb3b2349f9c044a6b51e7b425c2290c887bb4cb2.tar.gz
Pre-shared agent token
In order to improve security of the lookup/heartbeat endpoints, we need to generate and provide temporary tokens to the initial callers, if supported, to facilitate the verification of commands. This is the first patch in an entire series which utimately enables the endpoint communication to be better secured. The idea behind this started in private story 2006634 which is locked as a security related filing covering multiple aspects of ironic/ironic-python-agent interaction centered around miss-use and generally exposed endpoints. That story will remain marked as a private bug because it has several different items covered, some of which did not prove to be actually exploitable, but spawned stories 2006777, 2006773, 2007025, and is ultimately similar to Story 1526748. Operationally this is a minimally invasive security enhancement to lay the foundation to harden interactions with the agent. This will take place over a series of patches to both Ironic and the Ironic-Python-Agent. Also see "Security of /heartbeat and /lookup endpoints" in http://lists.openstack.org/pipermail/openstack-discuss/2019-November/010789.html Story: 2007025 Task: 37818 Change-Id: I0118007cac3d6548e9d41c5e615a819150b6ef1a
Diffstat (limited to 'ironic')
-rw-r--r--ironic/api/controllers/v1/node.py3
-rw-r--r--ironic/api/controllers/v1/ramdisk.py36
-rw-r--r--ironic/api/controllers/v1/utils.py5
-rw-r--r--ironic/api/controllers/v1/versions.py4
-rw-r--r--ironic/cmd/conductor.py12
-rw-r--r--ironic/common/release_mappings.py4
-rw-r--r--ironic/conductor/cleaning.py5
-rw-r--r--ironic/conductor/deployments.py4
-rw-r--r--ironic/conductor/manager.py76
-rw-r--r--ironic/conductor/rpcapi.py22
-rw-r--r--ironic/conductor/utils.py94
-rw-r--r--ironic/conf/default.py9
-rw-r--r--ironic/drivers/modules/agent_base.py4
-rw-r--r--ironic/drivers/modules/deploy_utils.py22
-rw-r--r--ironic/objects/node.py2
-rw-r--r--ironic/tests/unit/api/controllers/v1/test_node.py9
-rw-r--r--ironic/tests/unit/api/controllers/v1/test_ramdisk.py44
-rw-r--r--ironic/tests/unit/api/controllers/v1/test_utils.py6
-rw-r--r--ironic/tests/unit/conductor/test_deployments.py3
-rw-r--r--ironic/tests/unit/conductor/test_manager.py196
-rw-r--r--ironic/tests/unit/conductor/test_rpcapi.py17
-rw-r--r--ironic/tests/unit/conductor/test_utils.py34
-rw-r--r--ironic/tests/unit/drivers/modules/test_agent_base.py24
-rw-r--r--ironic/tests/unit/drivers/modules/test_deploy_utils.py21
-rw-r--r--ironic/tests/unit/objects/test_node.py6
25 files changed, 630 insertions, 32 deletions
diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py
index 6f4814788..bcbd23873 100644
--- a/ironic/api/controllers/v1/node.py
+++ b/ironic/api/controllers/v1/node.py
@@ -1245,6 +1245,9 @@ class Node(base.APIBase):
if self.instance_info.get('image_url'):
self.instance_info['image_url'] = "******"
+ if self.driver_internal_info.get('agent_secret_token'):
+ self.driver_internal_info['agent_secret_token'] = "******"
+
update_state_in_older_versions(self)
hide_fields_in_newer_versions(self)
diff --git a/ironic/api/controllers/v1/ramdisk.py b/ironic/api/controllers/v1/ramdisk.py
index 6fa778a33..625b98ccc 100644
--- a/ironic/api/controllers/v1/ramdisk.py
+++ b/ironic/api/controllers/v1/ramdisk.py
@@ -39,7 +39,7 @@ _LOOKUP_RETURN_FIELDS = ('uuid', 'properties', 'instance_info',
'driver_internal_info')
-def config():
+def config(token):
return {
'metrics': {
'backend': CONF.metrics.agent_backend,
@@ -52,7 +52,8 @@ def config():
'statsd_host': CONF.metrics_statsd.agent_statsd_host,
'statsd_port': CONF.metrics_statsd.agent_statsd_port
},
- 'heartbeat_timeout': CONF.api.ramdisk_heartbeat_timeout
+ 'heartbeat_timeout': CONF.api.ramdisk_heartbeat_timeout,
+ 'agent_token': token
}
@@ -72,8 +73,9 @@ class LookupResult(base.APIBase):
@classmethod
def convert_with_links(cls, node):
+ token = node.driver_internal_info.get('agent_secret_token')
node = node_ctl.Node.convert_with_links(node, _LOOKUP_RETURN_FIELDS)
- return cls(node=node, config=config())
+ return cls(node=node, config=config(token))
class LookupController(rest.RestController):
@@ -149,15 +151,27 @@ class LookupController(rest.RestController):
and node.provision_state not in self.lookup_allowed_states):
raise exception.NotFound()
- return LookupResult.convert_with_links(node)
+ if api_utils.allow_agent_token() or CONF.require_agent_token:
+ try:
+ topic = api.request.rpcapi.get_topic_for(node)
+ except exception.NoValidHost as e:
+ e.code = http_client.BAD_REQUEST
+ raise
+
+ found_node = api.request.rpcapi.get_node_with_token(
+ api.request.context, node.uuid, topic=topic)
+ else:
+ found_node = node
+ return LookupResult.convert_with_links(found_node)
class HeartbeatController(rest.RestController):
"""Controller handling heartbeats from deploy ramdisk."""
@expose.expose(None, types.uuid_or_name, str,
- str, status_code=http_client.ACCEPTED)
- def post(self, node_ident, callback_url, agent_version=None):
+ str, str, status_code=http_client.ACCEPTED)
+ def post(self, node_ident, callback_url, agent_version=None,
+ agent_token=None):
"""Process a heartbeat from the deploy ramdisk.
:param node_ident: the UUID or logical name of a node.
@@ -197,6 +211,14 @@ class HeartbeatController(rest.RestController):
raise exception.Invalid(
_('Detected change in ramdisk provided '
'"callback_url"'))
+ # NOTE(TheJulia): If tokens are required, lets go ahead and fail the
+ # heartbeat very early on.
+ token_required = CONF.require_agent_token
+ if token_required and agent_token is None:
+ LOG.error('Agent heartbeat received for node %(node)s '
+ 'without an agent token.', {'node': node_ident})
+ raise exception.InvalidParameterValue(
+ _('Agent token is required for heartbeat processing.'))
try:
topic = api.request.rpcapi.get_topic_for(rpc_node)
@@ -206,4 +228,4 @@ class HeartbeatController(rest.RestController):
api.request.rpcapi.heartbeat(
api.request.context, rpc_node.uuid, callback_url,
- agent_version, topic=topic)
+ agent_version, agent_token, topic=topic)
diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py
index 28b6c9174..7a77378e7 100644
--- a/ironic/api/controllers/v1/utils.py
+++ b/ironic/api/controllers/v1/utils.py
@@ -1316,3 +1316,8 @@ def allow_allocation_owner():
Version 1.60 of the API added the owner field to the allocation object.
"""
return api.request.version.minor >= versions.MINOR_60_ALLOCATION_OWNER
+
+
+def allow_agent_token():
+ """Check if agent token is available."""
+ return api.request.version.minor >= versions.MINOR_62_AGENT_TOKEN
diff --git a/ironic/api/controllers/v1/versions.py b/ironic/api/controllers/v1/versions.py
index 4b56a8dc4..f39c077b2 100644
--- a/ironic/api/controllers/v1/versions.py
+++ b/ironic/api/controllers/v1/versions.py
@@ -99,6 +99,7 @@ BASE_VERSION = 1
# v1.59: Add support vendor data in configdrives.
# v1.60: Add owner to the allocation object.
# v1.61: Add retired and retired_reason to the node object.
+# v1.62: Add agent_token support for agent communication.
MINOR_0_JUNO = 0
MINOR_1_INITIAL_VERSION = 1
@@ -162,6 +163,7 @@ MINOR_58_ALLOCATION_BACKFILL = 58
MINOR_59_CONFIGDRIVE_VENDOR_DATA = 59
MINOR_60_ALLOCATION_OWNER = 60
MINOR_61_NODE_RETIRED = 61
+MINOR_62_AGENT_TOKEN = 62
# When adding another version, update:
# - MINOR_MAX_VERSION
@@ -169,7 +171,7 @@ MINOR_61_NODE_RETIRED = 61
# explanation of what changed in the new version
# - common/release_mappings.py, RELEASE_MAPPING['master']['api']
-MINOR_MAX_VERSION = MINOR_61_NODE_RETIRED
+MINOR_MAX_VERSION = MINOR_62_AGENT_TOKEN
# String representations of the minor and maximum versions
_MIN_VERSION_STRING = '{}.{}'.format(BASE_VERSION, MINOR_1_INITIAL_VERSION)
diff --git a/ironic/cmd/conductor.py b/ironic/cmd/conductor.py
index 7d65d2514..cce081839 100644
--- a/ironic/cmd/conductor.py
+++ b/ironic/cmd/conductor.py
@@ -54,9 +54,21 @@ def warn_about_missing_default_boot_option(conf):
'an explicit value for it during the transition period')
+def warn_about_agent_token_deprecation(conf):
+ if not conf.require_agent_token:
+ LOG.warning('The ``[DEFAULT]require_agent_token`` option is not '
+ 'set and support for ironic-python-agents that do not '
+ 'utilize agent tokens, along with the configuration '
+ 'option will be removed in the W development cycle. '
+ 'Please upgrade your ironic-python-agent version, and '
+ 'consider adopting the require_agent_token setting '
+ 'during the Victoria development cycle.')
+
+
def issue_startup_warnings(conf):
warn_about_unsafe_shred_parameters(conf)
warn_about_missing_default_boot_option(conf)
+ warn_about_agent_token_deprecation(conf)
def main():
diff --git a/ironic/common/release_mappings.py b/ironic/common/release_mappings.py
index 17f6c6892..4dfc23387 100644
--- a/ironic/common/release_mappings.py
+++ b/ironic/common/release_mappings.py
@@ -214,8 +214,8 @@ RELEASE_MAPPING = {
}
},
'master': {
- 'api': '1.61',
- 'rpc': '1.48',
+ 'api': '1.62',
+ 'rpc': '1.49',
'objects': {
'Allocation': ['1.1'],
'Node': ['1.33', '1.32'],
diff --git a/ironic/conductor/cleaning.py b/ironic/conductor/cleaning.py
index 9f306e574..351c38121 100644
--- a/ironic/conductor/cleaning.py
+++ b/ironic/conductor/cleaning.py
@@ -213,6 +213,9 @@ def do_next_clean_step(task, step_index):
driver_internal_info.pop('clean_step_index', None)
driver_internal_info.pop('cleaning_reboot', None)
driver_internal_info.pop('cleaning_polling', None)
+ driver_internal_info.pop('agent_secret_token', None)
+ driver_internal_info.pop('agent_secret_token_pregenerated', None)
+
# Remove agent_url
if not utils.fast_track_able(task):
driver_internal_info.pop('agent_url', None)
@@ -271,6 +274,8 @@ def do_node_clean_abort(task, step_name=None):
info.pop('cleaning_polling', None)
info.pop('skip_current_clean_step', None)
info.pop('agent_url', None)
+ info.pop('agent_secret_token', None)
+ info.pop('agent_secret_token_pregenerated', None)
node.driver_internal_info = info
node.save()
LOG.info(info_message)
diff --git a/ironic/conductor/deployments.py b/ironic/conductor/deployments.py
index f94afd8ea..20c3c15b6 100644
--- a/ironic/conductor/deployments.py
+++ b/ironic/conductor/deployments.py
@@ -133,7 +133,7 @@ def start_deploy(task, manager, configdrive=None, event='deploy'):
def do_node_deploy(task, conductor_id=None, configdrive=None):
"""Prepare the environment and deploy a node."""
node = task.node
-
+ utils.del_secret_token(node)
try:
if configdrive:
if isinstance(configdrive, dict):
@@ -392,6 +392,8 @@ def do_next_deploy_step(task, step_index, conductor_id):
# Finished executing the steps. Clear deploy_step.
node.deploy_step = None
driver_internal_info = node.driver_internal_info
+ driver_internal_info.pop('agent_secret_token', None)
+ driver_internal_info.pop('agent_secret_token_pregenerated', None)
driver_internal_info['deploy_steps'] = None
driver_internal_info.pop('deploy_step_index', None)
driver_internal_info.pop('deployment_reboot', None)
diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py
index 83aad588b..6d0f8381f 100644
--- a/ironic/conductor/manager.py
+++ b/ironic/conductor/manager.py
@@ -90,7 +90,7 @@ class ConductorManager(base_manager.BaseConductorManager):
# NOTE(rloo): This must be in sync with rpcapi.ConductorAPI's.
# NOTE(pas-ha): This also must be in sync with
# ironic.common.release_mappings.RELEASE_MAPPING['master']
- RPC_API_VERSION = '1.48'
+ RPC_API_VERSION = '1.49'
target = messaging.Target(version=RPC_API_VERSION)
@@ -1008,6 +1008,8 @@ class ConductorManager(base_manager.BaseConductorManager):
node.instance_info = {}
node.instance_uuid = None
driver_internal_info = node.driver_internal_info
+ driver_internal_info.pop('agent_secret_token', None)
+ driver_internal_info.pop('agent_secret_token_pregenerated', None)
driver_internal_info.pop('instance', None)
driver_internal_info.pop('clean_steps', None)
driver_internal_info.pop('root_uuid_or_disk_id', None)
@@ -2924,7 +2926,8 @@ class ConductorManager(base_manager.BaseConductorManager):
@METRICS.timer('ConductorManager.heartbeat')
@messaging.expected_exceptions(exception.NoFreeConductorWorker)
- def heartbeat(self, context, node_id, callback_url, agent_version=None):
+ def heartbeat(self, context, node_id, callback_url, agent_version=None,
+ agent_token=None):
"""Process a heartbeat from the ramdisk.
:param context: request context.
@@ -2945,10 +2948,43 @@ class ConductorManager(base_manager.BaseConductorManager):
if agent_version is None:
agent_version = '3.0.0'
+ token_required = CONF.require_agent_token
+
# NOTE(dtantsur): we acquire a shared lock to begin with, drivers are
# free to promote it to an exclusive one.
with task_manager.acquire(context, node_id, shared=True,
purpose='heartbeat') as task:
+
+ # NOTE(TheJulia): The "token" line of defense.
+ # either tokens are required and they are present,
+ # or a token is present in general and needs to be
+ # validated.
+ if token_required or utils.is_agent_token_present(task.node):
+ if not utils.is_agent_token_valid(task.node, agent_token):
+ LOG.error('Invalid agent_token receieved for node '
+ '%(node)s', {'node': node_id})
+ raise exception.InvalidParameterValue(
+ 'Invalid or missing agent token received.')
+ elif utils.is_agent_token_supported(agent_version):
+ LOG.error('Suspicious activity detected for node %(node)s '
+ 'when attempting to heartbeat. Heartbeat '
+ 'request has been rejected as the version of '
+ 'ironic-python-agent indicated in the heartbeat '
+ 'operation should support agent token '
+ 'functionality.',
+ {'node': task.node.uuid})
+ raise exception.InvalidParameterValue(
+ 'Invalid or missing agent token received.')
+ else:
+ LOG.warning('Out of date agent detected for node '
+ '%(node)s. Agent version %(version) '
+ 'reported. Support for this version is '
+ 'deprecated.',
+ {'node': task.node.uuid,
+ 'version': agent_version})
+ # TODO(TheJulia): raise an exception as of the
+ # ?Victoria? development cycle.
+
task.spawn_after(
self._spawn_worker, task.driver.deploy.heartbeat,
task, callback_url, agent_version)
@@ -3301,6 +3337,42 @@ class ConductorManager(base_manager.BaseConductorManager):
LOG.exception('Unexpected exception when taking over '
'allocation %s', allocation.uuid)
+ @METRICS.timer('ConductorManager.get_node_with_token')
+ @messaging.expected_exceptions(exception.NodeLocked,
+ exception.Invalid)
+ def get_node_with_token(self, context, node_id):
+ """Add secret agent token to node.
+
+ :param context: request context.
+ :param node_id: node ID or UUID.
+ :returns: Secret token set for the node.
+ :raises: NodeLocked, if node has an exclusive lock held on it
+ :raises: Invalid, if the node already has a token set.
+ """
+ LOG.debug("RPC get_node_with_token called for the node %(node_id)s",
+ {'node_id': node_id})
+ with task_manager.acquire(context, node_id,
+ purpose='generate_token',
+ shared=True) as task:
+ node = task.node
+ if utils.is_agent_token_present(task.node):
+ LOG.warning('An agent token generation request is being '
+ 'refused as one is already present for '
+ 'node %(node)s',
+ {'node': node_id})
+ # Allow lookup to work by returning a value, it is just an
+ # unusable value that can't be verified against.
+ # This is important if the agent lookup has occured with
+ # pre-generation of tokens with virtual media usage.
+ node.driver_internal_info['agent_secret_token'] = "******"
+ return node
+ task.upgrade_lock()
+ LOG.debug('Generating agent token for node %(node)s',
+ {'node': task.node.uuid})
+ utils.add_secret_token(task.node)
+ task.node.save()
+ return task.node
+
@METRICS.timer('get_vendor_passthru_metadata')
def get_vendor_passthru_metadata(route_dict):
diff --git a/ironic/conductor/rpcapi.py b/ironic/conductor/rpcapi.py
index 0dd36ed2e..84054c36a 100644
--- a/ironic/conductor/rpcapi.py
+++ b/ironic/conductor/rpcapi.py
@@ -99,13 +99,15 @@ class ConductorAPI(object):
| 1.46 - Added reset_interfaces to update_node
| 1.47 - Added support for conductor groups
| 1.48 - Added allocation API
+ | 1.49 - Added get_node_with_token and agent_token argument to
+ heartbeat
"""
# NOTE(rloo): This must be in sync with manager.ConductorManager's.
# NOTE(pas-ha): This also must be in sync with
# ironic.common.release_mappings.RELEASE_MAPPING['master']
- RPC_API_VERSION = '1.48'
+ RPC_API_VERSION = '1.49'
def __init__(self, topic=None):
super(ConductorAPI, self).__init__()
@@ -811,7 +813,7 @@ class ConductorAPI(object):
node_id=node_id, clean_steps=clean_steps)
def heartbeat(self, context, node_id, callback_url, agent_version,
- topic=None):
+ agent_token=None, topic=None):
"""Process a node heartbeat.
:param context: request context.
@@ -825,6 +827,9 @@ class ConductorAPI(object):
if self.client.can_send_version('1.42'):
version = '1.42'
new_kws['agent_version'] = agent_version
+ if self.client.can_send_version('1.49'):
+ version = '1.49'
+ new_kws['agent_token'] = agent_token
cctxt = self.client.prepare(topic=topic or self.topic, version=version)
return cctxt.call(context, 'heartbeat', node_id=node_id,
callback_url=callback_url, **new_kws)
@@ -1133,3 +1138,16 @@ class ConductorAPI(object):
"""
cctxt = self.client.prepare(topic=topic or self.topic, version='1.48')
return cctxt.call(context, 'destroy_allocation', allocation=allocation)
+
+ def get_node_with_token(self, context, node_id, topic=None):
+ """Request the node from the conductor with an agent token
+
+ :param context: request context.
+ :param node_id: node ID or UUID.
+ :param topic: RPC topic. Defaults to self.topic.
+ :raises: NodeLocked if node is locked by another conductor.
+
+ :returns: A Node object with agent token.
+ """
+ cctxt = self.client.prepare(topic=topic or self.topic, version='1.49')
+ return cctxt.call(context, 'get_node_with_token', node_id=node_id)
diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py
index 9a64151d3..7f9cea147 100644
--- a/ironic/conductor/utils.py
+++ b/ironic/conductor/utils.py
@@ -14,6 +14,9 @@
import contextlib
import datetime
+from distutils.version import StrictVersion
+import random
+import string
import time
from openstack.baremetal import configdrive as os_configdrive
@@ -1005,3 +1008,94 @@ def get_node_next_clean_steps(task, skip_current_step=True):
def get_node_next_deploy_steps(task, skip_current_step=True):
return _get_node_next_steps(task, 'deploy',
skip_current_step=skip_current_step)
+
+
+def add_secret_token(node):
+ """Adds a secret token to driver_internal_info for IPA verification."""
+ characters = string.ascii_letters + string.digits
+ token = ''.join(
+ random.SystemRandom().choice(characters) for i in range(128))
+ i_info = node.driver_internal_info
+ i_info['agent_secret_token'] = token
+ node.driver_internal_info = i_info
+
+
+def del_secret_token(node):
+ """Deletes the IPA agent secret token.
+
+ Removes the agent token secret from the driver_internal_info field
+ from the Node object.
+
+ :param node: Node object
+ """
+ i_info = node.driver_internal_info
+ i_info.pop('agent_secret_token', None)
+ node.driver_internal_info = i_info
+
+
+def is_agent_token_present(node):
+ """Determines if an agent token is present upon a node.
+
+ :param node: Node object
+ :returns: True if an agent_secret_token value is present in a node
+ driver_internal_info field.
+ """
+ # TODO(TheJulia): we should likely record the time when we add the token
+ # and then compare if it was in the last ?hour? to act as an additional
+ # guard rail, but if we do that we will want to check the last heartbeat
+ # because the heartbeat overrides the age of the token.
+ # We may want to do this elsewhere or nowhere, just a thought for the
+ # future.
+ return node.driver_internal_info.get(
+ 'agent_secret_token', None) is not None
+
+
+def is_agent_token_valid(node, token):
+ """Validates if a supplied token is valid for the node.
+
+ :param node: Node object
+ :token: A token value to validate against the driver_internal_info field
+ agent_sercret_token.
+ :returns: True if the supplied token matches the token recorded in the
+ supplied node object.
+ """
+ if token is None:
+ # No token is never valid.
+ return False
+ known_token = node.driver_internal_info.get('agent_secret_token', None)
+ return known_token == token
+
+
+def is_agent_token_supported(agent_version):
+ # NOTE(TheJulia): This is hoped that 6.x supports
+ # agent token capabilities and realistically needs to be updated
+ # once that version of IPA is out there in some shape or form.
+ # This allows us to gracefully allow older agent's that were
+ # launched via pre-generated agent_tokens, to still work
+ # and could likely be removed at some point down the road.
+ version = str(agent_version).replace('.dev', 'b', 1)
+ return StrictVersion(version) > StrictVersion('6.1.0')
+
+
+def is_agent_token_pregenerated(node):
+ """Determines if the token was generated for out of band configuration.
+
+ Ironic supports the ability to provide configuration data to the agent
+ through the a virtual floppy or as part of the virtual media image
+ which is attached to the BMC.
+
+ This method helps us identify WHEN we did so as we don't need to remove
+ records of the token prior to rebooting the token. This is important as
+ tokens provided through out of band means presist in the virtual media
+ image, are loaded as part of the agent ramdisk, and do not require
+ regeneration of the token upon the initial lookup, ultimately making
+ the overall usage of virtual media and pregenerated tokens far more
+ secure.
+
+ :param node: Node Object
+ :returns: True if the token was pregenerated as indicated by the node's
+ driver_internal_info field.
+ False in all other cases.
+ """
+ return node.driver_internal_info.get(
+ 'agent_secret_token_pregenerated', False)
diff --git a/ironic/conf/default.py b/ironic/conf/default.py
index 4379982d0..857acb029 100644
--- a/ironic/conf/default.py
+++ b/ironic/conf/default.py
@@ -327,6 +327,15 @@ service_opts = [
('json-rpc', _('use JSON RPC transport'))],
help=_('Which RPC transport implementation to use between '
'conductor and API services')),
+ cfg.BoolOpt('require_agent_token',
+ default=False,
+ help=_('Used to require the use of agent tokens. These '
+ 'tokens are used to guard the api lookup endpoint and '
+ 'conductor heartbeat processing logic to authenticate '
+ 'transactions with the ironic-python-agent. Tokens '
+ 'are provided only upon the first lookup of a node '
+ 'and may be provided via out of band means through '
+ 'the use of virtual media.')),
]
utils_opts = [
diff --git a/ironic/drivers/modules/agent_base.py b/ironic/drivers/modules/agent_base.py
index 6ff4c8ff2..fd0246fd8 100644
--- a/ironic/drivers/modules/agent_base.py
+++ b/ironic/drivers/modules/agent_base.py
@@ -166,6 +166,10 @@ def _cleaning_reboot(task):
# Signify that we've rebooted
driver_internal_info = task.node.driver_internal_info
driver_internal_info['cleaning_reboot'] = True
+ if not driver_internal_info.get('agent_secret_token_pregenerated', False):
+ # Wipes out the existing recorded token because the machine will
+ # need to re-establish the token.
+ driver_internal_info.pop('agent_secret_token', None)
task.node.driver_internal_info = driver_internal_info
task.node.save()
diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py
index 9e26e0c4b..1d6e79247 100644
--- a/ironic/drivers/modules/deploy_utils.py
+++ b/ironic/drivers/modules/deploy_utils.py
@@ -1486,6 +1486,24 @@ def get_async_step_return_state(node):
return states.CLEANWAIT if node.clean_step else states.DEPLOYWAIT
+def _check_agent_token_prior_to_agent_reboot(driver_internal_info):
+ """Removes the agent token if it was not pregenerated.
+
+ Removal of the agent token in cases where it is not pregenerated
+ is a vital action prior to rebooting the agent, as without doing
+ so the agent will be unable to establish communication with
+ the ironic API after the reboot. Effectively locking itself out
+ as in cases where the value is not pregenerated, it is not
+ already included in the payload and must be generated again
+ upon lookup.
+
+ :param driver_internal_info: The driver_interal_info dict object
+ from a Node object.
+ """
+ if not driver_internal_info.get('agent_secret_token_pregenerated', False):
+ driver_internal_info.pop('agent_secret_token', None)
+
+
def set_async_step_flags(node, reboot=None, skip_current_step=None,
polling=None):
"""Sets appropriate reboot flags in driver_internal_info based on operation
@@ -1515,6 +1533,10 @@ def set_async_step_flags(node, reboot=None, skip_current_step=None,
fields = cleaning if node.clean_step else deployment
if reboot is not None:
info[fields['reboot']] = reboot
+ if reboot:
+ # If rebooting, we must ensure that we check and remove
+ # an agent token if necessary.
+ _check_agent_token_prior_to_agent_reboot(info)
if skip_current_step is not None:
info[fields['skip']] = skip_current_step
if polling is not None:
diff --git a/ironic/objects/node.py b/ironic/objects/node.py
index 73a257586..484e8d22d 100644
--- a/ironic/objects/node.py
+++ b/ironic/objects/node.py
@@ -171,6 +171,8 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat):
d.get('driver_info', {}), "******")
d['instance_info'] = strutils.mask_dict_password(
d.get('instance_info', {}), "******")
+ d['driver_internal_info'] = strutils.mask_dict_password(
+ d.get('driver_internal_info', {}), "******")
return d
def _validate_property_values(self, properties):
diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py
index 7bf9e89d6..db571ffa5 100644
--- a/ironic/tests/unit/api/controllers/v1/test_node.py
+++ b/ironic/tests/unit/api/controllers/v1/test_node.py
@@ -605,6 +605,15 @@ class TestListNodes(test_api_base.BaseApiTest):
headers={api_base.Version.string: '1.61'})
self.assertIn('retired', response)
+ def test_get_one_with_no_agent_secret(self):
+ node = obj_utils.create_test_node(
+ self.context,
+ driver_internal_info={'agent_secret_token': 'abcdefg'})
+ response = self.get_json('/nodes/%s' % (node.uuid),
+ headers={api_base.Version.string: '1.52'})
+ token_value = response['driver_internal_info']['agent_secret_token']
+ self.assertEqual('******', token_value)
+
def test_detail(self):
node = obj_utils.create_test_node(self.context,
chassis_id=self.chassis.id)
diff --git a/ironic/tests/unit/api/controllers/v1/test_ramdisk.py b/ironic/tests/unit/api/controllers/v1/test_ramdisk.py
index 086c3edf4..7ff108443 100644
--- a/ironic/tests/unit/api/controllers/v1/test_ramdisk.py
+++ b/ironic/tests/unit/api/controllers/v1/test_ramdisk.py
@@ -50,6 +50,16 @@ class TestLookup(test_api_base.BaseApiTest):
fixtures.MockPatchObject(rpcapi.ConductorAPI, 'get_conductor_for',
autospec=True)).mock
self.mock_get_conductor_for.return_value = 'fake.conductor'
+ self.mock_get_node_with_token = self.useFixture(
+ fixtures.MockPatchObject(rpcapi.ConductorAPI,
+ 'get_node_with_token',
+ autospec=True)).mock
+
+ def _set_secret_mock(self, node, token_value):
+ driver_internal = node.driver_internal_info
+ driver_internal['agent_secret_token'] = token_value
+ node.driver_internal_info = driver_internal
+ self.mock_get_node_with_token.return_value = node
def _check_config(self, data):
expected_metrics = {
@@ -65,9 +75,12 @@ class TestLookup(test_api_base.BaseApiTest):
'statsd_host': CONF.metrics_statsd.agent_statsd_host,
'statsd_port': CONF.metrics_statsd.agent_statsd_port
},
- 'heartbeat_timeout': CONF.api.ramdisk_heartbeat_timeout
+ 'heartbeat_timeout': CONF.api.ramdisk_heartbeat_timeout,
+ 'agent_token': mock.ANY
}
self.assertEqual(expected_metrics, data['config'])
+ self.assertIsNotNone(data['config']['agent_token'])
+ self.assertNotEqual('******', data['config']['agent_token'])
def test_nothing_provided(self):
response = self.get_json(
@@ -95,6 +108,7 @@ class TestLookup(test_api_base.BaseApiTest):
self.assertEqual(http_client.NOT_FOUND, response.status_int)
def test_found_by_addresses(self):
+ self._set_secret_mock(self.node, 'some-value')
obj_utils.create_test_port(self.context,
node_id=self.node.id,
address=self.addresses[1])
@@ -109,6 +123,7 @@ class TestLookup(test_api_base.BaseApiTest):
@mock.patch.object(ramdisk.LOG, 'warning', autospec=True)
def test_ignore_malformed_address(self, mock_log):
+ self._set_secret_mock(self.node, '123456')
obj_utils.create_test_port(self.context,
node_id=self.node.id,
address=self.addresses[1])
@@ -125,6 +140,7 @@ class TestLookup(test_api_base.BaseApiTest):
self.assertTrue(mock_log.called)
def test_found_by_uuid(self):
+ self._set_secret_mock(self.node, 'this_thing_on?')
data = self.get_json(
'/lookup?addresses=%s&node_uuid=%s' %
(','.join(self.addresses), self.node.uuid),
@@ -135,6 +151,7 @@ class TestLookup(test_api_base.BaseApiTest):
self._check_config(data)
def test_found_by_only_uuid(self):
+ self._set_secret_mock(self.node, 'xyzabc')
data = self.get_json(
'/lookup?node_uuid=%s' % self.node.uuid,
headers={api_base.Version.string: str(api_v1.max_version())})
@@ -153,6 +170,7 @@ class TestLookup(test_api_base.BaseApiTest):
def test_no_restrict_lookup(self):
CONF.set_override('restrict_lookup', False, 'api')
+ self._set_secret_mock(self.node2, '234567890')
data = self.get_json(
'/lookup?addresses=%s&node_uuid=%s' %
(','.join(self.addresses), self.node2.uuid),
@@ -163,6 +181,7 @@ class TestLookup(test_api_base.BaseApiTest):
self._check_config(data)
def test_fast_deploy_lookup(self):
+ self._set_secret_mock(self.node, 'abcxyz')
CONF.set_override('fast_track', True, 'deploy')
for provision_state in [states.ENROLL, states.MANAGEABLE,
states.AVAILABLE]:
@@ -203,7 +222,7 @@ class TestHeartbeat(test_api_base.BaseApiTest):
self.assertEqual(http_client.ACCEPTED, response.status_int)
self.assertEqual(b'', response.body)
mock_heartbeat.assert_called_once_with(mock.ANY, mock.ANY,
- node.uuid, 'url', None,
+ node.uuid, 'url', None, None,
topic='test-topic')
@mock.patch.object(rpcapi.ConductorAPI, 'heartbeat', autospec=True)
@@ -216,7 +235,7 @@ class TestHeartbeat(test_api_base.BaseApiTest):
self.assertEqual(http_client.ACCEPTED, response.status_int)
self.assertEqual(b'', response.body)
mock_heartbeat.assert_called_once_with(mock.ANY, mock.ANY,
- node.uuid, 'url', None,
+ node.uuid, 'url', None, None,
topic='test-topic')
@mock.patch.object(rpcapi.ConductorAPI, 'heartbeat', autospec=True)
@@ -229,7 +248,7 @@ class TestHeartbeat(test_api_base.BaseApiTest):
self.assertEqual(http_client.ACCEPTED, response.status_int)
self.assertEqual(b'', response.body)
mock_heartbeat.assert_called_once_with(mock.ANY, mock.ANY,
- node.uuid, 'url', None,
+ node.uuid, 'url', None, None,
topic='test-topic')
@mock.patch.object(rpcapi.ConductorAPI, 'heartbeat', autospec=True)
@@ -243,7 +262,7 @@ class TestHeartbeat(test_api_base.BaseApiTest):
self.assertEqual(http_client.ACCEPTED, response.status_int)
self.assertEqual(b'', response.body)
mock_heartbeat.assert_called_once_with(mock.ANY, mock.ANY,
- node.uuid, 'url', '1.4.1',
+ node.uuid, 'url', '1.4.1', None,
topic='test-topic')
@mock.patch.object(rpcapi.ConductorAPI, 'heartbeat', autospec=True)
@@ -268,3 +287,18 @@ class TestHeartbeat(test_api_base.BaseApiTest):
headers={api_base.Version.string: str(api_v1.max_version())},
expect_errors=True)
self.assertEqual(http_client.BAD_REQUEST, response.status_int)
+
+ @mock.patch.object(rpcapi.ConductorAPI, 'heartbeat', autospec=True)
+ def test_ok_agent_token(self, mock_heartbeat):
+ node = obj_utils.create_test_node(self.context)
+ response = self.post_json(
+ '/heartbeat/%s' % node.uuid,
+ {'callback_url': 'url',
+ 'agent_token': 'abcdef1'},
+ headers={api_base.Version.string: str(api_v1.max_version())})
+ self.assertEqual(http_client.ACCEPTED, response.status_int)
+ self.assertEqual(b'', response.body)
+ mock_heartbeat.assert_called_once_with(mock.ANY, mock.ANY,
+ node.uuid, 'url', None,
+ 'abcdef1',
+ topic='test-topic')
diff --git a/ironic/tests/unit/api/controllers/v1/test_utils.py b/ironic/tests/unit/api/controllers/v1/test_utils.py
index 68e8a7f47..53ee1207f 100644
--- a/ironic/tests/unit/api/controllers/v1/test_utils.py
+++ b/ironic/tests/unit/api/controllers/v1/test_utils.py
@@ -565,6 +565,12 @@ class TestCheckAllowFields(base.TestCase):
mock_request.version.minor = 54
self.assertFalse(utils.allow_deploy_templates())
+ def test_allow_agent_token(self, mock_request):
+ mock_request.version.minor = 62
+ self.assertTrue(utils.allow_agent_token())
+ mock_request.version.minor = 61
+ self.assertFalse(utils.allow_agent_token())
+
@mock.patch.object(api, 'request')
class TestNodeIdent(base.TestCase):
diff --git a/ironic/tests/unit/conductor/test_deployments.py b/ironic/tests/unit/conductor/test_deployments.py
index ba8b327fe..6d71adfe6 100644
--- a/ironic/tests/unit/conductor/test_deployments.py
+++ b/ironic/tests/unit/conductor/test_deployments.py
@@ -358,7 +358,8 @@ class DoNodeDeployTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
expected_calls = [
mock.call(node.uuid,
{'version': mock.ANY,
- 'instance_info': expected_instance_info}),
+ 'instance_info': expected_instance_info,
+ 'driver_internal_info': mock.ANY}),
mock.call(node.uuid,
{'version': mock.ANY,
'last_error': mock.ANY}),
diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py
index 106c267f6..b6eecf3b8 100644
--- a/ironic/tests/unit/conductor/test_manager.py
+++ b/ironic/tests/unit/conductor/test_manager.py
@@ -6885,6 +6885,10 @@ class DoNodeTakeOverTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
@mgr_utils.mock_record_keepalive
class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
+ def _fake_spawn(self, conductor_obj, func, *args, **kwargs):
+ func(*args, **kwargs)
+ return mock.MagicMock()
+
@mock.patch('ironic.drivers.modules.fake.FakePower.validate')
@mock.patch('ironic.drivers.modules.fake.FakeBoot.validate')
@mock.patch('ironic.drivers.modules.fake.FakeConsole.start_console')
@@ -7031,6 +7035,10 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
self.assertEqual(states.NOSTATE, node.target_provision_state)
self.assertIsNone(node.last_error)
+ # TODO(TheJulia): We should double check if these heartbeat tests need
+ # to move. I have this strange feeling we were lacking rpc testing of
+ # heartbeat until we did adoption testing....
+
@mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat',
autospec=True)
@mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker',
@@ -7046,10 +7054,7 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
mock_spawn.reset_mock()
- def fake_spawn(conductor_obj, func, *args, **kwargs):
- func(*args, **kwargs)
- return mock.MagicMock()
- mock_spawn.side_effect = fake_spawn
+ mock_spawn.side_effect = self._fake_spawn
self.service.heartbeat(self.context, node.uuid, 'http://callback')
mock_heartbeat.assert_called_with(mock.ANY, mock.ANY,
@@ -7070,16 +7075,191 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
mock_spawn.reset_mock()
- def fake_spawn(conductor_obj, func, *args, **kwargs):
- func(*args, **kwargs)
- return mock.MagicMock()
- mock_spawn.side_effect = fake_spawn
+ mock_spawn.side_effect = self._fake_spawn
self.service.heartbeat(
self.context, node.uuid, 'http://callback', '1.4.1')
mock_heartbeat.assert_called_with(mock.ANY, mock.ANY,
'http://callback', '1.4.1')
+ @mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat',
+ autospec=True)
+ @mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker',
+ autospec=True)
+ def test_heartbeat_with_no_required_agent_token(self, mock_spawn,
+ mock_heartbeat):
+ """Tests that we kill the heartbeat attempt very early on."""
+ self.config(require_agent_token=True)
+ node = obj_utils.create_test_node(
+ self.context, driver='fake-hardware',
+ provision_state=states.DEPLOYING,
+ target_provision_state=states.ACTIVE)
+
+ self._start_service()
+
+ mock_spawn.reset_mock()
+
+ mock_spawn.side_effect = self._fake_spawn
+
+ self.assertRaises(
+ exception.InvalidParameterValue, self.service.heartbeat,
+ self.context, node.uuid, 'http://callback', agent_token=None)
+ self.assertFalse(mock_heartbeat.called)
+
+ @mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat',
+ autospec=True)
+ @mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker',
+ autospec=True)
+ def test_heartbeat_with_required_agent_token(self, mock_spawn,
+ mock_heartbeat):
+ """Test heartbeat works when token matches."""
+ self.config(require_agent_token=True)
+ node = obj_utils.create_test_node(
+ self.context, driver='fake-hardware',
+ provision_state=states.DEPLOYING,
+ target_provision_state=states.ACTIVE,
+ driver_internal_info={'agent_secret_token': 'a secret'})
+
+ self._start_service()
+
+ mock_spawn.reset_mock()
+
+ mock_spawn.side_effect = self._fake_spawn
+
+ self.service.heartbeat(self.context, node.uuid, 'http://callback',
+ agent_token='a secret')
+ mock_heartbeat.assert_called_with(mock.ANY, mock.ANY,
+ 'http://callback', '3.0.0')
+
+ @mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat',
+ autospec=True)
+ @mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker',
+ autospec=True)
+ def test_heartbeat_with_agent_token(self, mock_spawn,
+ mock_heartbeat):
+ """Test heartbeat works when token matches."""
+ self.config(require_agent_token=False)
+ node = obj_utils.create_test_node(
+ self.context, driver='fake-hardware',
+ provision_state=states.DEPLOYING,
+ target_provision_state=states.ACTIVE,
+ driver_internal_info={'agent_secret_token': 'a secret'})
+
+ self._start_service()
+
+ mock_spawn.reset_mock()
+
+ mock_spawn.side_effect = self._fake_spawn
+
+ self.service.heartbeat(self.context, node.uuid, 'http://callback',
+ agent_token='a secret')
+ mock_heartbeat.assert_called_with(mock.ANY, mock.ANY,
+ 'http://callback', '3.0.0')
+
+ @mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat',
+ autospec=True)
+ @mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker',
+ autospec=True)
+ def test_heartbeat_invalid_agent_token(self, mock_spawn,
+ mock_heartbeat):
+ """Heartbeat fails when it does not match."""
+ self.config(require_agent_token=False)
+ node = obj_utils.create_test_node(
+ self.context, driver='fake-hardware',
+ provision_state=states.DEPLOYING,
+ target_provision_state=states.ACTIVE,
+ driver_internal_info={'agent_secret_token': 'a secret'})
+
+ self._start_service()
+
+ mock_spawn.reset_mock()
+
+ mock_spawn.side_effect = self._fake_spawn
+
+ self.assertRaises(exception.InvalidParameterValue,
+ self.service.heartbeat, self.context,
+ node.uuid, 'http://callback',
+ agent_token='evil', agent_version='5.0.0b23')
+ self.assertFalse(mock_heartbeat.called)
+
+ @mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat',
+ autospec=True)
+ @mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker',
+ autospec=True)
+ def test_heartbeat_invalid_agent_token_older_version(
+ self, mock_spawn, mock_heartbeat):
+ """Heartbeat is rejected if token is received that is invalid."""
+ self.config(require_agent_token=False)
+ node = obj_utils.create_test_node(
+ self.context, driver='fake-hardware',
+ provision_state=states.DEPLOYING,
+ target_provision_state=states.ACTIVE,
+ driver_internal_info={'agent_secret_token': 'a secret'})
+
+ self._start_service()
+
+ mock_spawn.reset_mock()
+
+ mock_spawn.side_effect = self._fake_spawn
+ # Intentionally sending an older client in case something fishy
+ # occurs.
+ self.assertRaises(exception.InvalidParameterValue,
+ self.service.heartbeat, self.context,
+ node.uuid, 'http://callback',
+ agent_token='evil', agent_version='4.0.0')
+ self.assertFalse(mock_heartbeat.called)
+
+ @mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat',
+ autospec=True)
+ @mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker',
+ autospec=True)
+ def test_heartbeat_invalid_when_token_on_file_older_agent(
+ self, mock_spawn, mock_heartbeat):
+ """Heartbeat rejected if a token is on file."""
+ self.config(require_agent_token=False)
+ node = obj_utils.create_test_node(
+ self.context, driver='fake-hardware',
+ provision_state=states.DEPLOYING,
+ target_provision_state=states.ACTIVE,
+ driver_internal_info={'agent_secret_token': 'a secret'})
+
+ self._start_service()
+
+ mock_spawn.reset_mock()
+
+ mock_spawn.side_effect = self._fake_spawn
+
+ self.assertRaises(exception.InvalidParameterValue,
+ self.service.heartbeat, self.context,
+ node.uuid, 'http://callback',
+ agent_token=None, agent_version='4.0.0')
+ self.assertFalse(mock_heartbeat.called)
+
+ @mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat',
+ autospec=True)
+ @mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker',
+ autospec=True)
+ def test_heartbeat_invalid_newer_version(
+ self, mock_spawn, mock_heartbeat):
+ """Heartbeat rejected if client should be sending a token."""
+ self.config(require_agent_token=False)
+ node = obj_utils.create_test_node(
+ self.context, driver='fake-hardware',
+ provision_state=states.DEPLOYING,
+ target_provision_state=states.ACTIVE)
+
+ self._start_service()
+
+ mock_spawn.reset_mock()
+
+ mock_spawn.side_effect = self._fake_spawn
+
+ self.assertRaises(exception.InvalidParameterValue,
+ self.service.heartbeat, self.context,
+ node.uuid, 'http://callback',
+ agent_token=None, agent_version='6.1.5')
+ self.assertFalse(mock_heartbeat.called)
+
@mgr_utils.mock_record_keepalive
class DestroyVolumeConnectorTestCase(mgr_utils.ServiceSetUpMixin,
diff --git a/ironic/tests/unit/conductor/test_rpcapi.py b/ironic/tests/unit/conductor/test_rpcapi.py
index 57fd0d06f..ddbc8bd1b 100644
--- a/ironic/tests/unit/conductor/test_rpcapi.py
+++ b/ironic/tests/unit/conductor/test_rpcapi.py
@@ -486,7 +486,16 @@ class RPCAPITestCase(db_base.DbTestCase):
node_id='fake-node',
callback_url='http://ramdisk.url:port',
agent_version=None,
- version='1.42')
+ version='1.49')
+
+ def test_heartbeat_agent_token(self):
+ self._test_rpcapi('heartbeat',
+ 'call',
+ node_id='fake-node',
+ callback_url='http://ramdisk.url:port',
+ agent_version=None,
+ agent_token='xyz1',
+ version='1.49')
def test_destroy_volume_connector(self):
fake_volume_connector = db_utils.get_test_volume_connector()
@@ -555,6 +564,12 @@ class RPCAPITestCase(db_base.DbTestCase):
version='1.43',
node_id=self.fake_node['uuid'])
+ def test_get_node_with_token(self):
+ self._test_rpcapi('get_node_with_token',
+ 'call',
+ version='1.49',
+ node_id=self.fake_node['uuid'])
+
def _test_can_send_rescue(self, can_send):
rpcapi = conductor_rpcapi.ConductorAPI(topic='fake-topic')
with mock.patch.object(rpcapi.client,
diff --git a/ironic/tests/unit/conductor/test_utils.py b/ironic/tests/unit/conductor/test_utils.py
index 482387ddf..6a437debe 100644
--- a/ironic/tests/unit/conductor/test_utils.py
+++ b/ironic/tests/unit/conductor/test_utils.py
@@ -2018,3 +2018,37 @@ class GetNodeNextStepsTestCase(db_base.DbTestCase):
with task_manager.acquire(self.context, node.uuid) as task:
step_index = conductor_utils.get_node_next_clean_steps(task)
self.assertEqual(0, step_index)
+
+
+class AgentTokenUtilsTestCase(tests_base.TestCase):
+
+ def setUp(self):
+ super(AgentTokenUtilsTestCase, self).setUp()
+ self.node = obj_utils.get_test_node(self.context,
+ driver='fake-hardware')
+
+ def test_add_secret_token(self):
+ self.assertNotIn('agent_secret_token', self.node.driver_internal_info)
+ conductor_utils.add_secret_token(self.node)
+ self.assertEqual(
+ 128, len(self.node.driver_internal_info['agent_secret_token']))
+
+ def test_del_secret_token(self):
+ conductor_utils.add_secret_token(self.node)
+ self.assertIn('agent_secret_token', self.node.driver_internal_info)
+ conductor_utils.del_secret_token(self.node)
+ self.assertNotIn('agent_secret_token', self.node.driver_internal_info)
+
+ def test_is_agent_token_present(self):
+ # This should always be False as the token has not been added yet.
+ self.assertFalse(conductor_utils.is_agent_token_present(self.node))
+ conductor_utils.add_secret_token(self.node)
+ self.assertTrue(conductor_utils.is_agent_token_present(self.node))
+
+ def test_is_agent_token_supported(self):
+ self.assertTrue(
+ conductor_utils.is_agent_token_supported('6.1.1.dev39'))
+ self.assertTrue(
+ conductor_utils.is_agent_token_supported('6.2.1'))
+ self.assertFalse(
+ conductor_utils.is_agent_token_supported('6.0.0'))
diff --git a/ironic/tests/unit/drivers/modules/test_agent_base.py b/ironic/tests/unit/drivers/modules/test_agent_base.py
index 5f51938b4..457452892 100644
--- a/ironic/tests/unit/drivers/modules/test_agent_base.py
+++ b/ironic/tests/unit/drivers/modules/test_agent_base.py
@@ -1559,11 +1559,35 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest):
def test__cleaning_reboot(self, mock_reboot, mock_prepare, mock_build_opt):
with task_manager.acquire(self.context, self.node['uuid'],
shared=False) as task:
+ i_info = task.node.driver_internal_info
+ i_info['agent_secret_token'] = 'magicvalue01'
+ task.node.driver_internal_info = i_info
agent_base._cleaning_reboot(task)
self.assertTrue(mock_build_opt.called)
self.assertTrue(mock_prepare.called)
mock_reboot.assert_called_once_with(task, states.REBOOT)
self.assertTrue(task.node.driver_internal_info['cleaning_reboot'])
+ self.assertNotIn('agent_secret_token',
+ task.node.driver_internal_info)
+
+ @mock.patch.object(deploy_utils, 'build_agent_options', autospec=True)
+ @mock.patch.object(pxe.PXEBoot, 'prepare_ramdisk', spec_set=True,
+ autospec=True)
+ @mock.patch.object(manager_utils, 'node_power_action', autospec=True)
+ def test__cleaning_reboot_pregenerated_token(
+ self, mock_reboot, mock_prepare, mock_build_opt):
+ with task_manager.acquire(self.context, self.node['uuid'],
+ shared=False) as task:
+ i_info = task.node.driver_internal_info
+ i_info['agent_secret_token'] = 'magicvalue01'
+ i_info['agent_secret_token_pregenerated'] = True
+ task.node.driver_internal_info = i_info
+ agent_base._cleaning_reboot(task)
+ self.assertTrue(mock_build_opt.called)
+ self.assertTrue(mock_prepare.called)
+ mock_reboot.assert_called_once_with(task, states.REBOOT)
+ self.assertIn('agent_secret_token',
+ task.node.driver_internal_info)
@mock.patch.object(deploy_utils, 'build_agent_options', autospec=True)
@mock.patch.object(pxe.PXEBoot, 'prepare_ramdisk', spec_set=True,
diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py
index 9624286b5..fe1a8a41f 100644
--- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py
+++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py
@@ -2939,10 +2939,14 @@ class AsyncStepTestCase(db_base.DbTestCase):
def test_set_async_step_flags_deploying_set_all(self):
self.node.deploy_step = {'step': 'create_configuration',
'interface': 'raid'}
- self.node.driver_internal_info = {}
+ self.node.driver_internal_info = {
+ 'agent_secret_token': 'test',
+ 'agent_secret_token_pregenerated': True}
expected = {'deployment_reboot': True,
'deployment_polling': True,
- 'skip_current_deploy_step': True}
+ 'skip_current_deploy_step': True,
+ 'agent_secret_token': 'test',
+ 'agent_secret_token_pregenerated': True}
self.node.save()
utils.set_async_step_flags(self.node, reboot=True,
skip_current_step=True,
@@ -2957,3 +2961,16 @@ class AsyncStepTestCase(db_base.DbTestCase):
utils.set_async_step_flags(self.node, reboot=True)
self.assertEqual({'deployment_reboot': True},
self.node.driver_internal_info)
+
+ def test_set_async_step_flags_clears_non_pregenerated_token(self):
+ self.node.clean_step = {'step': 'create_configuration',
+ 'interface': 'raid'}
+ self.node.driver_internal_info = {'agent_secret_token': 'test'}
+ expected = {'cleaning_reboot': True,
+ 'cleaning_polling': True,
+ 'skip_current_clean_step': True}
+ self.node.save()
+ utils.set_async_step_flags(self.node, reboot=True,
+ skip_current_step=True,
+ polling=True)
+ self.assertEqual(expected, self.node.driver_internal_info)
diff --git a/ironic/tests/unit/objects/test_node.py b/ironic/tests/unit/objects/test_node.py
index 07d4cdae7..18693047e 100644
--- a/ironic/tests/unit/objects/test_node.py
+++ b/ironic/tests/unit/objects/test_node.py
@@ -40,18 +40,24 @@ class TestNodeObject(db_base.DbTestCase, obj_utils.SchemasTestMixIn):
def test_as_dict_insecure(self):
self.node.driver_info['ipmi_password'] = 'fake'
self.node.instance_info['configdrive'] = 'data'
+ self.node.driver_internal_info['agent_secret_token'] = 'abc'
d = self.node.as_dict()
self.assertEqual('fake', d['driver_info']['ipmi_password'])
self.assertEqual('data', d['instance_info']['configdrive'])
+ self.assertEqual('abc',
+ d['driver_internal_info']['agent_secret_token'])
# Ensure the node can be serialised.
jsonutils.dumps(d)
def test_as_dict_secure(self):
self.node.driver_info['ipmi_password'] = 'fake'
self.node.instance_info['configdrive'] = 'data'
+ self.node.driver_internal_info['agent_secret_token'] = 'abc'
d = self.node.as_dict(secure=True)
self.assertEqual('******', d['driver_info']['ipmi_password'])
self.assertEqual('******', d['instance_info']['configdrive'])
+ self.assertEqual('******',
+ d['driver_internal_info']['agent_secret_token'])
# Ensure the node can be serialised.
jsonutils.dumps(d)