Skip to content

Use LDAP to translate upstream users to downstream users#374

Merged
ralphbean merged 3 commits intomainfrom
ldap-lookup
Sep 30, 2025
Merged

Use LDAP to translate upstream users to downstream users#374
ralphbean merged 3 commits intomainfrom
ldap-lookup

Conversation

@webbnh
Copy link
Copy Markdown
Collaborator

@webbnh webbnh commented Sep 29, 2025

Currently, sync2jira relies on a correspondence between the "full name" from the GitHub user profile and the analog in the Jira user profile in order to match upstream users to downstream users (e.g., for issue reporters and assignees). This mechanism is fragile, because users often have different "spellings" of their name in the two profiles or because their name is hidden in the upstream profile. Furthermore, with the impending migration of the downstream from Jira Datacenter to Jira Cloud, it becomes possible that the full name will not be available from the downstream profile, either!

This PR presents an alternative method for performing the mapping. Since Red Hat associates are supposed to include their GitHub account in their Rover profile, we can use LDAP to look up an associate's information based on their GitHub username. This allows us to retrieve their Red Hat email address(es) -- regardless of what email address they have associated with their upstream profile -- and use that as a key to find their Jira user identity. And, this should continue to work with Jira Cloud after the migration.

This change includes the following:

  • A new Python package, Rover_Lookup (which is a peer to the sync-page and sync2jira packages) which currently offers a single function, github_username_to_emails() that provides the LDAP-based translation of GitHub username to Red Hat email(s). This feature is structured as a separate Python package, with its own infrastructure and unit tests, so that it can be built and distributed separately from sync2jira should there be a demand for it; however, the sync2jira code references it locally, and, when Tox is used to run the unit tests for sync2jira, it finds and runs the unit tests for Rover_Lookup, as well...so, at least for now, we've got the best of both worlds.
  • A rework of the downstream match_user() function to accept a list of email addresses instead of the upstream user's fullname.
  • A retooling of the downstream assign_user() to use the Rover_Lookup function instead of the relying on the GitHub fullnames.
  • A tweak to the upstream code to capture an assignee's GitHub login in addition to their fullname, and revisions to a number of unit tests which noticed the addition. 😞
  • Tweaks to a couple of unit tests which were affected by the switch from fullname- to email-based identity lookup and an additional testcase.

All the unit tests pass, and I've done a functional test with Jira mocked out which seems to works as intended; however, I don't know how to do "real" testing other than by deploying this into production...if anyone has thoughts on that, I'd like to hear them. 🙈

Note: the creation of the Rover_Lookup package and the bulk changes to the unit tests were "Assisted by AI" using Cursor running Claude-4-Sonnet.

This is set up as a separable package, because we might want to separate it.

Assisted-by: Cursor/claude-4-sonnet
@ralphbean
Copy link
Copy Markdown
Member

The hardcoding of rhatPrimaryEmail, rhat social, and the ldap server address in the rover lookup module are problematic, but I can't imagine it being useful if we did the work to make that abstract and totally configurable for other sites/companies.

Thinking about performance, there's no caching of usernames to emails. I suppose that since we don't try to access the mapping unless we've already decided to create an issue (which is somewhat rare), then we won't overload rover. If we see any issues there - a cache could be a next step since the username -> email mapping will be rare to change.

License Inconsistency

  • Rover_Lookup/setup.py:29: Claims "MIT License"
  • Rover_Lookup/README.md:122: States "GNU Lesser General Public License, Version 2.1"

Can you align these to the existing license?

I'm happy to get this merged and see how it fares in production, for science.

@webbnh
Copy link
Copy Markdown
Collaborator Author

webbnh commented Sep 30, 2025

The hardcoding of rhatPrimaryEmail, rhat social, and the ldap server address in the rover lookup module are problematic, but I can't imagine it being useful if we did the work to make that abstract and totally configurable for other sites/companies.

Yeah, I pretty much landed on the notion that this solution to the mapping problem is pretty RH-specific and that it wasn't worthwhile to pull the punches. If there is a need in the future, we can iterate and generalize.

Thinking about performance, there's no caching of usernames to emails. I suppose that since we don't try to access the mapping unless we've already decided to create an issue (which is somewhat rare), then we won't overload rover. If we see any issues there - a cache could be a next step since the username -> email mapping will be rare to change.

It's a fair point that the mapping will be rare to change; however, for the reasons you mention and others, I think that accesses to the cache would be "rare" as well (especially as measured per unit time). (Our current average appears to be one request every 20 seconds, of which we discard 85%, so we would be hitting LDAP, at most, every 2.5 minutes, on average...actually, looking just at the updates to the assignees, it looks like it's more like once every half-hour.)

License Inconsistency

  • Rover_Lookup/setup.py:29: Claims "MIT License"
  • Rover_Lookup/README.md:122: States "GNU Lesser General Public License, Version 2.1"

Can you align these to the existing license?

Cursor did that for me, and I failed to catch it (I didn't think to look for that detail...I'm glad you did! -- thanks!). I'll fix that.

I'm happy to get this merged and see how it fares in production, for science.

For SCIENCE!! 😀

Copy link
Copy Markdown
Member

@ralphbean ralphbean left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For science! 🧪 🔬 ⭐

@ralphbean ralphbean merged commit 630a7f0 into main Sep 30, 2025
6 checks passed
@ralphbean ralphbean deleted the ldap-lookup branch September 30, 2025 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants