confusing "Circular reference found role inference rules ..." error

Bug #1803780 reported by Guang Yee
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Identity (keystone)
Fix Released
Low
Adam Young

Bug Description

When assigning both prior role and implied role in the implied role chain (that is more than two levels deep) to a given user for a given project, you'll see a rather confusing and misleading error in the Keystone log that looks like this.

Nov 16 11:50:03 keystone <email address hidden>[17003]: ERROR keystone.assignment.core [None req-770cd1c8-b5bd-4b37-b2b3-1e7bc57b8093 None None] Circular reference found role inference rules -
c6025062f9704caba0be20ebd3f7b4f0

First off all, this is not a fatal error as the operation will eventually succeed. We merely log it as *ERROR* without reraising it. See

https://github.com/openstack/keystone/blob/master/keystone/assignment/core.py#L673

So it shouldn't be an error from operational standpoint. Perhaps we should consider downgrading it to warning instead?

But the bigger problem is how did we even get into this situation to begin with. Shouldn't this situation be prevented at role assignment? i.e. checking for potential circular inference prior to finalizing the assignment.

Steps to reproduce the problem:

1. provision a devstack
2. source devstack/openrc admin admin
3. openstack role add --user admin --project admin member
4. openstack role assignment list --user admin --project admin --effective
5. sudo journalctl (and you'll see the 'Circular reference found role inference rules' error in the logs)

Another alternative would be to create your own implied role chain.

1. provision a devstack
2. source devstack/openrc admin admin
3. openstack role create foo
4. openstack implied role create --implied-role reader foo
5. openstack role create another_foo
6. openstack implied role create --implied_role foo another_foo
7. openstack role add --user demo --project demo foo
8. openstack role add --user demo --project demo another_foo
9. openstack role assignment list --user demo --project demo --effective
10. sudo journalctl (and you'll see the 'Circular reference found role inference rules' error in the logs)

NOTE: this happens when we an implied role chain is more than two levels deep. i.e.

another_foo -> foo -> reader

Revision history for this message
Morgan Fainberg (mdrnstm) wrote :

This was highlighted as a problem (identified) in an unrelated bug. The error was a red-herring there and probably simply an issue with our validation logic.

I'll poke at the code a little more closely here once I get back home. The core issue is in this for loop[0], and it looks like it's possibly mis-detecting the "next role ref" equivalency in the checked_role_ref list.

[0] https://github.com/openstack/keystone/blob/4b41fa4c8b17b2da510100e9c6cad98d1cd19b0c/keystone/assignment/core.py#L666-L673

Changed in keystone:
importance: Undecided → Medium
assignee: nobody → Morgan Fainberg (mdrnstm)
Revision history for this message
Morgan Fainberg (mdrnstm) wrote :

This is def. an issue with the circular reference detection, it will occur if a role is conveyed by multiple inferred roles. We aren't really checking circular reference issues. This is an erroneous log and should have no impact on functionality. I'll mark this as low priority since it really is an error in logging. The above code linked in comment #1 is where the issue lies.

Changed in keystone:
status: New → Triaged
importance: Medium → Low
tags: added: logging
Revision history for this message
Adam Young (ayoung) wrote :

Ah...yep, I can see that. If the role is added twice, there is no way to tell it is from a cycle. We can just drop the warning, as the implied role construction process does not continue on from a duplicated role.

Revision history for this message
Guang Yee (guang-yee) wrote :

Yes, the current logic won't get into an infinite loop even if we have a circular reference. So dropping the bogus error from the logs should definitely helps in reducing the noise in error detection.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to keystone (master)

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

Changed in keystone:
assignee: Morgan Fainberg (mdrnstm) → Adam Young (ayoung)
status: Triaged → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to keystone (master)

Reviewed: https://review.openstack.org/624553
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=78566e828659b201563e7d07823df30f7b00b0d4
Submitter: Zuul
Branch: master

commit 78566e828659b201563e7d07823df30f7b00b0d4
Author: Adam Young <email address hidden>
Date: Tue Dec 11 21:22:51 2018 -0500

    Remove message about circular role inferences

    While Cycles could be a problem, this code was detercting them even
    when there were none. If a role gets added twice, it was reporting
    an error, but that is possible from the case where two distinct prior
    add the same implied role. Just move on quietly.

    closes-bug 1803780

    Change-Id: I804e5084f74ff4afdd582ece02ff2c833c5f6eb1

Changed in keystone:
status: In Progress → Fix Released
Changed in keystone:
milestone: none → stein-2
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to keystone (stable/rocky)

Fix proposed to branch: stable/rocky
Review: https://review.openstack.org/648033

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to keystone (stable/rocky)

Reviewed: https://review.openstack.org/648033
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=1c013444f91dd88451d5050fafbd1b9fc13ff4a4
Submitter: Zuul
Branch: stable/rocky

commit 1c013444f91dd88451d5050fafbd1b9fc13ff4a4
Author: Adam Young <email address hidden>
Date: Tue Dec 11 21:22:51 2018 -0500

    Remove message about circular role inferences

    While Cycles could be a problem, this code was detercting them even
    when there were none. If a role gets added twice, it was reporting
    an error, but that is possible from the case where two distinct prior
    add the same implied role. Just move on quietly.

    closes-bug 1803780

    Change-Id: I804e5084f74ff4afdd582ece02ff2c833c5f6eb1
    (cherry picked from commit 78566e828659b201563e7d07823df30f7b00b0d4)

tags: added: in-stable-rocky
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.