diff --git a/Rover_Lookup/lookup.py b/Rover_Lookup/lookup.py index d8db17cc..00631a16 100644 --- a/Rover_Lookup/lookup.py +++ b/Rover_Lookup/lookup.py @@ -63,7 +63,13 @@ def github_username_to_emails( try: # Create LDAP server connection; if credentials were not provided, the connection # will use an anonymous binding. - conn = Connection(ldap_server, ldap_bind_dn, ldap_password, auto_bind=True) + conn = Connection( + ldap_server, + ldap_bind_dn, + ldap_password, + auto_bind=True, + raise_exceptions=True, + ) except LDAPException as e: msg = f"Error connecting to LDAP server {ldap_server!r}: {str(e)}" if "redhat.com" in ldap_server and "invalid server address" in str(e): @@ -76,8 +82,8 @@ def github_username_to_emails( ) return None - logger.debug(f"Searching for GitHub username: {github_username}") - logger.debug(f"LDAP filter: {ldap_filter}") + logger.debug("Searching for GitHub username: %r", github_username) + logger.debug("LDAP filter: %r", ldap_filter) try: # Perform the LDAP search @@ -89,21 +95,23 @@ def github_username_to_emails( ) if not success: - logger.warning(f"LDAP search failed for GitHub username: {github_username}") + logger.info("LDAP search failed for GitHub username: %r", github_username) return None entries = conn.entries - logger.debug(f"Found {len(entries)} LDAP entries") + logger.debug("Found %d LDAP entries", len(entries)) if not entries: - logger.info(f"No LDAP entries found for GitHub username: {github_username}") + logger.info( + "No LDAP entries found for GitHub username: %r", github_username + ) return [] # Extract email addresses from all entries email_addresses = set() # Use set to automatically handle uniqueness for entry in entries: - logger.debug(f"Processing LDAP entry: {entry.entry_dn}") + logger.debug("Processing LDAP entry: %r", entry.entry_dn) # Check each email field for attr_name in attributes: @@ -124,9 +132,11 @@ def github_username_to_emails( result_emails = sorted(list(email_addresses)) logger.debug( - f"Found {len(result_emails)} unique email addresses for GitHub username: {github_username}" + "Found %d unique email addresses for GitHub username, %r: %s", + len(result_emails), + github_username, + result_emails, ) - logger.debug(f"Email addresses: {result_emails}") return result_emails diff --git a/Rover_Lookup/tests/test_lookup.py b/Rover_Lookup/tests/test_lookup.py index cf2bb85f..43efd2d3 100644 --- a/Rover_Lookup/tests/test_lookup.py +++ b/Rover_Lookup/tests/test_lookup.py @@ -170,7 +170,7 @@ def test_ldap_search_failure(self, mock_connection_class, caplog): mock_connection_class.return_value = mock_conn mock_conn.search.return_value = False # Search failed - with caplog.at_level(logging.WARNING): + with caplog.at_level(logging.INFO): result = github_username_to_emails("test-user") assert result is None @@ -269,6 +269,7 @@ def test_custom_ldap_parameters(self, mock_connection_class): "cn=bind,dc=custom,dc=com", "secret", auto_bind=True, + raise_exceptions=True, ) # Verify custom base DN was used in search diff --git a/sync2jira/downstream_issue.py b/sync2jira/downstream_issue.py index c6a0582f..0e87c205 100644 --- a/sync2jira/downstream_issue.py +++ b/sync2jira/downstream_issue.py @@ -385,9 +385,29 @@ def match_user( if not users: continue - if len(users) > 1: - log.warning("Found multiple Jira users for %r", email) - return users[0].name # TODO: We should probably return the ID here, instead. + + if len(users) == 1: + # TODO: We should probably return the ID here, instead. + return users[0].name + + limit = 5 + log.warning( + "Found %d Jira users for %r: %s%s", + len(users), + email, + ", ".join(u.name for u in users[0:limit]), + "..." if len(users) > limit else "", + ) + for user in users: + # This condition _should_ be true for *all* entries returned, in + # which case we'll just return the first entry; however it appears + # that sometimes Jira returns _all_ assignable users, so do our own + # filtering. + if user.emailAddress == email: + log.info("Found matching user: %r", user.name) + return user.name + else: + log.warning("Found no Jira user which matches %r", email) return None @@ -409,6 +429,7 @@ def assign_user( if remove_all: # Update the issue to have no assignees downstream.update(assignee={"name": ""}) + log.info("Cleared assignment of %s.", downstream.key) return # JIRA only supports one assignee; if we have more than one (i.e., from @@ -426,6 +447,7 @@ def assign_user( if match_name: # Assign the downstream issue to the matched user downstream.update({"assignee": {"name": match_name}}) + log.info("Assigned %s to %r", downstream.key, match_name) return if issue.assignee: @@ -501,7 +523,7 @@ def _get_preferred_issue_types(config, issue): In all cases, a list of one item is returned, except when the upstream issue has multiple tags which match multiple entries in the configured mapping, in which case multiple entries are returned, sorted in ascending - lexographical order. + lexicographical order. :param Dict config: Config dict :param sync2jira.intermediary.Issue issue: Issue object @@ -907,7 +929,6 @@ def _update_assignee(client, existing, issue, overwrite): if update: assign_user(client, issue, existing, remove_all=clear) - log.info("%s assignee", "Cleared" if clear else "Updated") def _update_jira_labels(issue, labels): diff --git a/tests/test_downstream_issue.py b/tests/test_downstream_issue.py index 7751d7dd..3b35d1eb 100644 --- a/tests/test_downstream_issue.py +++ b/tests/test_downstream_issue.py @@ -292,16 +292,19 @@ def test_assign_user_multiple(self, mock_client, mock_rover_lookup): # Set up return values mock_user = MagicMock() mock_user.displayName = "mock_assignee" + mock_user.name = "mock_assignee_name" + mock_user.emailAddress = "mock_user@redhat.com" mock_user.key = "mock_user_key" mock_user2 = MagicMock() mock_user2.displayName = "mock_assignee2" - mock_user2.key = "mock_user_key2" + mock_user2.name = "mock_assignee2_name" + mock_user2.emailAddress = "wrong_mock_user@redhat.com" + mock_user2.key = "mock_user2_key" mock_client.search_assignable_users_for_issues.return_value = [ mock_user, mock_user2, ] mock_client.assign_issue.return_value = True - mock_rover_lookup.return_value = ["mock_user@redhat.com"] self.mock_issue.assignee = [ {"fullname": None, "login": "login1"}, {"fullname": "", "login": "login2"}, @@ -310,6 +313,16 @@ def test_assign_user_multiple(self, mock_client, mock_rover_lookup): # Should not match this next -- should match the previous. {"fullname": "mock_assignee2", "login": "login5"}, ] + rlu = { + "login1": [], + "login2": [], + "login3": ["not_a_match@redhat.com"], + "login4": [mock_user.emailAddress], + "login5": [mock_user2.emailAddress], + } + mock_rover_lookup.side_effect = lambda un: rlu.get( + un, AssertionError("Test bug! Missing assignee login") + ) # Call the assign user function d.assign_user(