Make the InstanceMapping marker UUID-like
The InstanceMapping class contains the 'instance_uuid' field. When mapping instances, they can be done in batches, and in order to do this, a marker containing the UUID of the last mapped instance is created in the table. However, since the instance_uuid column has a unique constraint, the value is munged by replacing the dashes ('-') with spaces. The oslo_versionedobjects module was recently updated to enforce the validity of all values in UUID fields. As a transition, it only emits warnings, but in the future, it will raise ValueErrors. Tests of the mapping marker behavior show the warning: oslo_versionedobjects/fields.py:348: FutureWarning: 655662b2 6573 42f9 abbf de92533e20a8 is an invalid UUID. Using UUIDFields with invalid UUIDs is no longer supported, and will be removed in a future release. Please update your code to input valid UUIDs or accept ValueErrors for invalid UUIDs. We need to update the InstanceMapping marker code to avoid creating invalid UUIDs. This change accomplishes this be introducing a method that "shifts" the digits in the UUID to store, and unshifts them to recover. The stored value should have no more of a chance of a collision with an existing value than any other UUID added to the table. Closes-Bug: #1742760 Change-Id: I5b3f7f21c0f2593a588a368c977fc71f49b9d222
This commit is contained in:
parent
3bb7c8dd61
commit
a563d1fdd3
|
@ -111,6 +111,20 @@ def _db_error(caught_exception):
|
|||
sys.exit(1)
|
||||
|
||||
|
||||
def _uuid_shift(uuid):
|
||||
"""We need a way to non-destructively modify UUIDs so that we can store a
|
||||
marker without having duplicate InstanceMapping entries. This accomplishes
|
||||
this by applying a ROT8 mapping to the non-dash characters in a UUID.
|
||||
Calling _uuid_shift twice returns the value to the original UUID.
|
||||
"""
|
||||
mapper = {"0": "8", "1": "9", "2": "a", "3": "b", "4": "c", "5": "d",
|
||||
"6": "e", "7": "f", "8": "0", "9": "1", "a": "2", "b": "3",
|
||||
"c": "4", "d": "5", "e": "6", "f": "7", "-": "-"}
|
||||
return "".join([mapper[char] for char in uuid])
|
||||
# Since shifting twice returns you to the original, we can alias.
|
||||
_uuid_unshift = _uuid_shift
|
||||
|
||||
|
||||
class FloatingIpCommands(object):
|
||||
"""Class for managing floating IP."""
|
||||
|
||||
|
@ -1112,7 +1126,7 @@ class CellV2Commands(object):
|
|||
marker = None
|
||||
else:
|
||||
# There should be only one here
|
||||
marker = marker_mapping[0].instance_uuid.replace(' ', '-')
|
||||
marker = _uuid_unshift(marker_mapping[0].instance_uuid)
|
||||
marker_mapping[0].destroy()
|
||||
|
||||
next_marker = True
|
||||
|
@ -1126,7 +1140,7 @@ class CellV2Commands(object):
|
|||
if next_marker:
|
||||
# Don't judge me. There's already an InstanceMapping with this UUID
|
||||
# so the marker needs to be non destructively modified.
|
||||
next_marker = next_marker.replace('-', ' ')
|
||||
next_marker = _uuid_shift(next_marker)
|
||||
objects.InstanceMapping(ctxt, instance_uuid=next_marker,
|
||||
project_id=marker_project_id).create()
|
||||
return 1
|
||||
|
|
|
@ -74,6 +74,17 @@ class UtilitiesTestCase(test.NoDBTestCase):
|
|||
url4_safe,
|
||||
manage.mask_passwd_in_url(url4))
|
||||
|
||||
def test_shift_uuid(self):
|
||||
uuid = uuidutils.generate_uuid()
|
||||
shifted = manage._uuid_shift(uuid)
|
||||
self.assertTrue(uuidutils.is_uuid_like(shifted))
|
||||
unshifted = manage._uuid_unshift(shifted)
|
||||
self.assertEqual(uuid, unshifted)
|
||||
# Verify that shifting twice results in no change to the original
|
||||
uuid = uuidutils.generate_uuid()
|
||||
double_shift = manage._uuid_shift(manage._uuid_shift(uuid))
|
||||
self.assertEqual(uuid, double_shift)
|
||||
|
||||
|
||||
class FloatingIpCommandsTestCase(test.NoDBTestCase):
|
||||
def setUp(self):
|
||||
|
@ -1174,7 +1185,7 @@ class CellV2CommandsTestCase(test.NoDBTestCase):
|
|||
|
||||
# Instances are mapped in the order created so we know the marker is
|
||||
# based off the third instance.
|
||||
marker = instance_uuids[2].replace('-', ' ')
|
||||
marker = manage._uuid_shift(instance_uuids[2])
|
||||
marker_mapping = objects.InstanceMapping.get_by_instance_uuid(ctxt,
|
||||
marker)
|
||||
marker_mapping.destroy()
|
||||
|
|
Loading…
Reference in New Issue