Make `is_valid` more flexible with uuid validation

In keystone, multiple domain ids are handled by concatenating
two valid uuids together. This causes issues with pycadf validation
and causes the following warning to be thrown:

warnings.warn('Invalid uuid. To ensure interoperability, identifiers '
'should be a valid uuid.')

This appears throughout the testing logs while running keystone tests
and there is a current attempt to remove most/all invalid uuids to
eliminate this warning [0]. However due to the multiple domain id
issue, this cannot be solved in keystone alone.

The idea to allow multiple uuids that were joined together was
mentioned before in a previous attempt to solve this issue [1].

This change:
- Allows 2 or more concatenated uuids to be considered valid without
emitting a warning
- Cleaned up the list of words that are exceptions for validation
- Added 'default' to list of valid words in exception list
- Added offending value to printed warning
- Broke up test_identifiers for better clarity about which tests
were valid. This also solves the issue of `warning_mock.called` always
being `True` once an invalid uuid of type string was checked within
test_identifier
- Added test for valid exception words and extra characters that
can be present in a valid uuid according to [2]

[0] https://bugs.launchpad.net/keystone/+bug/1659053
[1] https://bugs.launchpad.net/keystone/+bug/1521844
[2] https://docs.python.org/2/library/uuid.html

Co-Authored-By: Tin Lam <tinlam@gmail.com>

Partial-Bug: #1659053
Change-Id: I58bba04c21c2d24fd37850c9ecc6fac99deb3fc4
This commit is contained in:
Gage Hugo 2017-02-02 20:01:53 -06:00 committed by Tin Lam
parent 6e7ecfae0f
commit a4a8f46248
2 changed files with 83 additions and 8 deletions

View File

@ -12,6 +12,7 @@
# License for the specific language governing permissions and limitations under
# the License.
import hashlib
import re
import uuid
import warnings
@ -33,6 +34,8 @@ if CONF.audit.namespace:
md5_hash = hashlib.md5(CONF.audit.namespace.encode('utf-8'))
AUDIT_NS = uuid.UUID(md5_hash.hexdigest())
VALID_EXCEPTIONS = ['default', 'initiator', 'observer', 'target']
def generate_uuid():
"""Generate a CADF identifier."""
@ -48,15 +51,31 @@ def norm_ns(str_id):
return prefix + str_id
def _check_valid_uuid(value):
"""Checks a value for one or multiple valid uuids joined together."""
if not value:
raise ValueError
value = re.sub('[{}-]|urn:uuid:', '', value)
for val in [value[i:i + 32] for i in range(0, len(value), 32)]:
uuid.UUID(val)
def is_valid(value):
"""Validation to ensure Identifier is correct."""
if value in ['target', 'initiator', 'observer']:
"""Validation to ensure Identifier is correct.
If the Identifier value is a string type but not a valid UUID string,
warn against interoperability issues and return True. This relaxes
the requirement of having strict UUID checking.
"""
if value in VALID_EXCEPTIONS:
return True
try:
uuid.UUID(value)
_check_valid_uuid(value)
except (ValueError, TypeError):
if not isinstance(value, six.string_types) or not value:
return False
warnings.warn('Invalid uuid. To ensure interoperability, identifiers '
'should be a valid uuid.')
warnings.warn(('Invalid uuid: %s. To ensure interoperability, '
'identifiers should be a valid uuid.' % (value)))
return True

View File

@ -38,16 +38,72 @@ from pycadf import timestamp
class TestCADFSpec(base.TestCase):
@mock.patch('pycadf.identifier.warnings.warn')
def test_identifier(self, warning_mock):
# empty string
self.assertFalse(identifier.is_valid(''))
def test_identifier_generated_uuid(self, warning_mock):
# generated uuid
self.assertTrue(identifier.is_valid(identifier.generate_uuid()))
self.assertFalse(warning_mock.called)
@mock.patch('pycadf.identifier.warnings.warn')
def test_identifier_empty_string_is_invalid(self, warning_mock):
# empty string
self.assertFalse(identifier.is_valid(''))
self.assertFalse(warning_mock.called)
@mock.patch('pycadf.identifier.warnings.warn')
def test_identifier_any_string_is_invalid(self, warning_mock):
# any string
self.assertTrue(identifier.is_valid('blah'))
self.assertTrue(warning_mock.called)
@mock.patch('pycadf.identifier.warnings.warn')
def test_identifier_joined_uuids_are_valid(self, warning_mock):
# multiple uuids joined together
long_128_uuids = [
('3adce28e67e44544a5a9d5f1ab54f578a86d310aac3a465e9d'
'd2693a78b45c0e42dce28e67e44544a5a9d5f1ab54f578a86d'
'310aac3a465e9dd2693a78b45c0e'),
('{3adce28e67e44544a5a9d5f1ab54f578a86d310aac3a465e9d'
'd2693a78b45c0e42dce28e67e44544a5a9d5f1ab54f578a86d'
'310aac3a465e9dd2693a78b45c0e}'),
('{12345678-1234-5678-1234-567812345678'
'12345678-1234-5678-1234-567812345678'
'12345678-1234-5678-1234-567812345678'
'12345678-1234-5678-1234-567812345678}'),
('urn:uuid:3adce28e67e44544a5a9d5f1ab54f578a86d310aac3a465e9d'
'd2693a78b45c0e42dce28e67e44544a5a9d5f1ab54f578a86d'
'310aac3a465e9dd2693a78b45c0e')]
for value in long_128_uuids:
self.assertTrue(identifier.is_valid(value))
self.assertFalse(warning_mock.called)
@mock.patch('pycadf.identifier.warnings.warn')
def test_identifier_long_nonjoined_uuid_is_invalid(self, warning_mock):
# long uuid not of size % 32
char_42_id = '3adce28e67e44544a5a9d5f1ab54f578a86d310aac'
self.assertTrue(identifier.is_valid(char_42_id))
self.assertTrue(warning_mock.called)
@mock.patch('pycadf.identifier.warnings.warn')
def test_identifier_specific_exceptions_are_valid(self, warning_mock):
# uuid exceptions
for value in identifier.VALID_EXCEPTIONS:
self.assertTrue(identifier.is_valid(value))
self.assertFalse(warning_mock.called)
@mock.patch('pycadf.identifier.warnings.warn')
def test_identifier_valid_id_extra_chars_is_valid(self, warning_mock):
# valid uuid with additional characters according to:
# https://docs.python.org/2/library/uuid.html
valid_ids = [
'{1234567890abcdef1234567890abcdef}',
'{12345678-1234-5678-1234-567812345678}',
'urn:uuid:12345678-1234-5678-1234-567812345678']
for value in valid_ids:
self.assertTrue(identifier.is_valid(value))
self.assertFalse(warning_mock.called)
def test_endpoint(self):
endp = endpoint.Endpoint(url='http://192.168.0.1',
name='endpoint name',