Removing duplicated items doesn't work in case of federations

[Dmitry Stepanenko]

In commit eed233cac8f34ce74a2f6fa989c484773c491df3 "Concrete role assignments for federated users" there was added handling of federation-related objects. In that implementation objects like roles, projects and domains were aggregated from 2 sources - from appropriate tables directly and from federation-related hooks.
This mechanism can lead to situation when there's duplication of objects, so for such cases code for filtering out duplicates was added.

It was impemented in the following way:

domains = [dict(t) for t in set([tuple(d.items()) for d in domains])]

where domains is a list of dicts, each of which contains information about appropriate domain. This code can work fine in some situations but in general can work in a wrong way because dict "items" method returns key-value pairs in arbitrary order according to https://docs.python.org/2/library/stdtypes.html#dict.items. So, this code may remain unchanged list of 2 similar dicts where items listed out in a different order.

This code was introduced in upstream Thu Feb 25 21:39:15 2016, so it seems that this code remains in newton and ocata and master branch.

[Lance Bragstad (lbragstad)]

The change referenced in the description [0] includes tests for duplicate projects via direct and indirect assignments [1]. This should be testing the code in question.

I ran the tests locally and ensured there were't duplicate projects or domains as a result of the API. Are you able to recreate this with a test of some kind?

[0] https://review.openstack.org/#/c/284943/63
[1] https://github.com/openstack/keystone/blob/bebd7056ad33d294871013067cb7367bc6db1a13/keystone/tests/unit/test_v3_federation.py#L3069-L3132

[Dmitry Stepanenko (dstepanenko)]

Yes, Lance. I found an issue in forked version of keystone with some changes related to federations.
The issue there occured in that line of code. You can look at the example below and check if it works for python 2.7:

domains = [{'id': 1, 'name': 'domain1', 'extra': ''}, {'extra': '', 'id': 1, 'name': 'domain1'}]
domains = [dict(t) for t in set([tuple(d.items()) for d in domains])]

after running these 2 lines of code you can see that there will be 2 items in domains list.

[Lance Bragstad (lbragstad)]

There might be something wrong with how the test is working then. Let's keep this open until we figure out exactly where the mismatch is.

Thanks for following up!

[Lance Bragstad (lbragstad)]

After discussing on IRC privately - it sounded like this was more of an issue with role names being duplicated and not projects or domains. Is that correct, Dmitry?

I'm unable to reproduce this with duplicate domains or projects.

[Lance Bragstad (lbragstad)]

Marking as Incomplete until we get some more feedback.

[Dmitry Stepanenko (dstepanenko)]

Today I was able to reproduce this issue on ocata using federation setup on devstack. I was using federated user which:
  * was in a group according to mapping - http://paste.openstack.org/show/616748/
  * was additionally added to the same group using 'openstack group add user ...'

I ran command "openstack stack create -t auto_scaling.yaml DM" and received message
" ERROR: Remote error: Conflict Conflict occurred attempting to store trust - Duplicate entry. (HTTP 409)"

The duplication occured because heat created the same trust twice. In turn, this happened because keystone returned admin role for current user twice.

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/489647

[Lance Bragstad (lbragstad)]

I was able to recreate this - but it appears to be transient. This is going to still require some investigation to see if we can make it 100% reproducible. See the approach I've taken in the patch from comment #7.

[Lance Bragstad (lbragstad)]

Looks like the code to remove duplicate roles for federated users is susceptible to the case described in the description of the bug report [0]. I was able to recreate this and verify it with the test in comment #7. The reason why this doesn't consistently fail is because python dictionaries are unordered. Whether or not this fails depends on the if the dictionary (role reference) is returned in a state that allows it to match other duplicate role references.

[0] https://github.com/openstack/keystone/blob/2164d0550c39a07b9c0dacc6c2167d0018b0c7bd/keystone/token/providers/common.py#L195-L196

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Change abandoned by Lance Bragstad (<email address hidden>) on branch: master
Review: https://review.openstack.org/489647
Reason: Rolled this into https://review.openstack.org/#/c/494049/

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/pike
Review: https://review.openstack.org/494238

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/494049
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=058a23c0873723d5a4ffa8e99121f7b3b4485db5
Submitter: Jenkins
Branch: master

commit 058a23c0873723d5a4ffa8e99121f7b3b4485db5
Author: Lance Bragstad <email address hidden>
Date: Tue Aug 1 16:03:53 2017 +0000

    Remove duplicate roles from federated auth

    We were using a one-liner to prune duplicate role references from a
    list of roles, but it didn't work in all cases. This reworks the
    logic to pass the existing test case. I also added a comment
    explaining why the logic we used previously doesn't work so we can
    hopefully avoid the pattern in the future.

    Change-Id: Id786d6463364ad8f4f02c22bb83221baac4b83d0
    Closes-Bug: 1701324

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/494238
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=10a9ef511dffb2e7dd19a075039e66fbf307ca85
Submitter: Jenkins
Branch: stable/pike

commit 10a9ef511dffb2e7dd19a075039e66fbf307ca85
Author: Lance Bragstad <email address hidden>
Date: Tue Aug 1 16:03:53 2017 +0000

    Remove duplicate roles from federated auth

    We were using a one-liner to prune duplicate role references from a
    list of roles, but it didn't work in all cases. This reworks the
    logic to pass the existing test case. I also added a comment
    explaining why the logic we used previously doesn't work so we can
    hopefully avoid the pattern in the future.

    Change-Id: Id786d6463364ad8f4f02c22bb83221baac4b83d0
    Closes-Bug: 1701324
    (cherry picked from commit 058a23c0873723d5a4ffa8e99121f7b3b4485db5)

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/keystone 12.0.0.0rc2 release candidate.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/keystone 13.0.0.0b1 development milestone.

[OpenStack Infra (hudson-openstack)]

Change abandoned by Lance Bragstad (<email address hidden>) on branch: master
Review: https://review.openstack.org/481020
Reason: I believe this was already fixed in another patch [0].

[0] https://review.openstack.org/#/c/494049/