Uploaded image for project: 'Red Hat Internal Developer Platform'
  1. Red Hat Internal Developer Platform
  2. RHIDP-4346

RBAC BackstageRoleManager.hasLink rebuilds the group hierarchy graph unnecessarily

Prepare for Y ReleasePrepare for Z ReleaseRemove QuarterXMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Unresolved
    • Icon: Undefined Undefined
    • None
    • 1.3
    • Performance, RBAC Plugin
    • None
    • False
    • Hide

      None

      Show
      None
    • False

      Description of problem:

      The BackstageRoleManager.hasLink is used to determine if a particular user is associated with a particular role within the RBAC backend plugin. This is done by either querying the catalog api or the catalog database `relations` table to build a group hierarchy graph. This graph is then evaluated to see if the user is associated to a role.

       

      However, after reviewing our BackstageRoleManager some more, I realized that building the graph is happening unnecessarily. There are a few things that are happening that proves my reasoning.

       

      First, we are using filters to reduce the roles that are evaluated done to only the roles that the user is directly and indirectly attached to, see this section of the code base. Which means that when we start the hasLink checks, we are only doing the comparisons on the roles that the user is associated with.

       

      A quick test to prove this is to create a few roles, one that the user is directly attached to `g, user:default/<YOUR_USER>, role:default/direct-role`, one that the user is indirectly attached to `g, group:default<GROUP_USER_IS_A_PART_OF>, role:default/indirect-role`, and two that the user is not a part of `g, user:default/some_other_user, role:default/no-direct-role` `g, group:default/some_other_group, role:default/no-indirect-role`. Then check what roles are evaluated in the BackstageRoleManager.hasLink.

       

      Second, we already build the group hierarchy graph through the use of BackstageRoleManager.getRoles for the user that we are evaluating for. So instead of rebuilding it whenever we make it to the BackstageRoleManager.hasLink check, we could instead cache it through our `allRoles` cache or through the Backstage cache service.

      Prerequisites (if any, like setup, operators/versions):

      Steps to Reproduce

      1. Create a lot of roles, permissions, users, and groups
      2. Attempt to access the catalog

      Actual results:

      Notice that at a certain, large enough, point, we begin to see negative performance impact. This is due to the constant building and checking of the group hierarchy graphs

      Expected results:

      We should be limiting the number of times that we build and check the graph as much as possible.

      Reproducibility (Always/Intermittent/Only Once):

      As we scale with the number of permissions, roles, users, and groups we always will see negative performance impacts.

      Build Details:

      Additional info (Such as Logs, Screenshots, etc):

      We have a couple of ways we can solve this problem but there are some questions that still need answered to make sure that we cover all of the bases. See below for solutions and for questions.

      • We forgo the `hasLink` checks, we already build and filter for the roles that the user is a part of, so therefore the user should always be attached to the roles that are passed to `hasLink`.
        • There are times where roles are not returned and we decide to filter based on the permission, see code here, what would happen in this case?
      • We cache the results from `getRoles` for the particular user either internally or through the use of the Backstage cache service.
        • If we use the Backstage cache service, should we update the graph every time `getRoles` is called for the user or use expirations and trust the cache as long as it is present?
        • If we store it internally, what kind of impact to memory usage will this have since it will be saving graph information for every user?

            Unassigned Unassigned
            rh-ee-pknight Patrick Knight
            RHIDP - Plugins
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

              Created:
              Updated: