pycadf ID validation fails for multi-domain IDs

[Steve Martinelli]

With the latest pycadf release (2.0.0), there is a more strict validation on the ID fields of various CADF resources, in this case, the initiator is failing to validate some keystone user IDs.

This only happens when multi-domains are configured. An ID for a user in a multi-domain setup is in fact two IDs concatenated together.

The code to check for a valid ID / UUID is:
https://github.com/openstack/pycadf/blob/master/pycadf/identifier.py#L50-L60

def is_valid(value):
    """Validation to ensure Identifier is correct.
    """
    if value in ['target', 'initiator', 'observer']:
        return True
    try:
        uuid.UUID(value)
    except ValueError:
        return False
    else:
        return True

A typical userID in a multi domain setup is: c79a927caef36ade4ed36679cd084fa45df4563f94af6a956fafa936889b4faf

When this is validated in pycadf, it fails:

>>> import uuid
>>> uuid.UUID("c79a927caef36ade4ed36679cd084fa45df4563f94af6a956fafa936889b4faf")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python2.7/uuid.py", line 134, in __init__
    raise ValueError('badly formed hexadecimal UUID string')
ValueError: badly formed hexadecimal UUID string

Options: we can revert the change to pycadf and loosen the validation of IDs, or make keystone use a different value.

This is the part of keystone that fails:
https://github.com/openstack/keystone/blob/master/keystone/notifications.py#L504-L505

[Steve Martinelli (stevemar)]

I've proposed a revert of the pycadf change: https://review.openstack.org/#/c/252175/

Not saying we have to revert it, it is an option though.

[Steve Martinelli (stevemar)]

Run this test to see it fail: tox -e py27 test_create_user_none_password

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/252182

[gordon chung (chungg)]

i changed it so we allow anything that is a non-empty string (as before) but we have a warning if it's not UUID.

[Steve Martinelli (stevemar)]

link to gordons patch: https://review.openstack.org/#/c/252183/

[OpenStack Infra (hudson-openstack)]

Change abandoned by Steve Martinelli (<email address hidden>) on branch: master
Review: https://review.openstack.org/252175

[Steve Martinelli (stevemar)]

Downgrading keystone's importance since with a new pycadf release we no longer failing the gate. We still need a plan to use UUIDs since users will see a bunch of warnings otherwise.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/252183
Committed: https://git.openstack.org/cgit/openstack/pycadf/commit/?id=6f1de9ca0471e3a6e079f89ad03adf39e8ea97d0
Submitter: Jenkins
Branch: master

commit 6f1de9ca0471e3a6e079f89ad03adf39e8ea97d0
Author: gordon chung <email address hidden>
Date: Tue Dec 1 23:44:47 2015 -0500

    relax id validation

    we want uuid, unfortunately most of openstack is a mess so we can't
    have nice things. this patch makes it so we continue to allow basically
    anything that is a string but we warn that they should stop this.

    Change-Id: I2a89aed143c22b9a3e2261d5473af93c1cad67cd
    Closes-Bug: #1521844

[Doug Hellmann (doug-hellmann)]

This issue was fixed in the openstack/pycadf 2.0.1 release.

[Lance Bragstad (lbragstad)]

I imagine this is no longer an issue in keystone since it was fixed upstream in pycadf.

[Lance Bragstad (lbragstad)]

Just saw Steve's comment on not using UUIDs. Disregard my previous comment.

[Steve Martinelli (stevemar)]

the immediate issue is fixed in pycadf

[Steve Martinelli (stevemar)]

I removed myself since I haven't worked on this in a while, an initial fix is at: https://review.openstack.org/#/c/252182/

[Matt Rutkowski (mrutkows)]

My 2 cents...

since these UUIDs seem to be single UUIDs of char. length=32 <or> double-length (concatenated) char. length=64,.

then my recommendation (within pyCADF) is to test for length (32 or 64) and if 32 use the previous code to test for valid UUID; if 64 then assume (since this is an OpenStack impl.) and split the 64 char string into 2, 32-char strings and validate both parts are valid UUIDs (concatenated). If neither 32 or 64 length, then consider throwing a format exception...

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/252182
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=944bd0a2dca60a5b144378587a13220d33298051
Submitter: Jenkins
Branch: master

commit 944bd0a2dca60a5b144378587a13220d33298051
Author: Ajaya Agrawal <email address hidden>
Date: Wed Jan 20 17:01:52 2016 +0000

    Ensure pycadf initiator IDs are UUID

    pycadf now has more strict validation for IDs, specifically, it tests to
    make sure they are UUIDs. In a multi-domain configuration this fails
    since the public ID that is generated by keystone is not an actual UUID.

    Change-Id: I1fd13bd7a7fe037bd0e1b7d6fb0214460ff7c963
    Closes-Bug: 1521844
    Co-Authored-By: Steve Martinelli <email address hidden>

[Thierry Carrez (ttx)]

This issue was fixed in the openstack/keystone 9.0.0.0b3 development milestone.