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

pycadf/identifier.py

@@ -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

pycadf/tests/test_cadf_spec.py

@@ -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',