Merge pull request #4726 from linuxfoundation/unicron-4701-allow-bots-to-skip-cla-post-deploy-updates

Add more visibility and fix one dminor debugging message in another function

Author: lukaszgryglicki

WHITELISTING_BOTS.md

@@ -109,3 +109,4 @@ Or using AWS CLI:
 aws --profile "lfproduct-prod" dynamodb scan --table-name "cla-prod-github-orgs" --filter-expression "contains(organization_name,:v)" --expression-attribute-values "{\":v\":{\"S\":\"linuxfoundation\"}}" --max-items 100 | jq -r '.Items'
 ```
 
+To check for log entries related to skipping CLA check, you can use the following command: `` STAGE=dev DTFROM='1 hour ago' DTTO='1 second ago' ./utils/search_aws_log_group.sh 'cla-backend-dev-githubactivity' 'skip_cla' ``.

cla-backend-go/github/bots.go

@@ -85,6 +85,27 @@ func isActorSkipped(actor *UserCommitSummary, config string) bool {
 	return propertyMatches(usernamePattern, username) && propertyMatches(emailPattern, email)
 }
 
+func actorToString(actor *UserCommitSummary) string {
+	const nullStr = "(null)"
+	if actor == nil {
+		return nullStr
+	}
+	id, login, username, email := nullStr, nullStr, nullStr, nullStr
+	if actor.CommitAuthor != nil && actor.CommitAuthor.ID != nil {
+		id = fmt.Sprintf("%v", *actor.CommitAuthor.ID)
+	}
+	if actor.CommitAuthor != nil && actor.CommitAuthor.Login != nil {
+		login = *actor.CommitAuthor.Login
+	}
+	if actor.CommitAuthor != nil && actor.CommitAuthor.Name != nil {
+		username = *actor.CommitAuthor.Name
+	}
+	if actor.CommitAuthor != nil && actor.CommitAuthor.Email != nil {
+		email = *actor.CommitAuthor.Email
+	}
+	return fmt.Sprintf("id='%v',login='%v',username='%v',email='%v'", id, login, username, email)
+}
+
 // SkipWhitelistedBots- check if the actors are whitelisted based on the skip_cla configuration.
 // Returns two lists:
 // - actors still missing cla: actors who still need to sign the CLA after checking skip_cla
@@ -127,7 +148,6 @@ func SkipWhitelistedBots(ev events.Service, orgModel *models.GithubOrganization,
 	}
 
 	var config string
-
 	// 1. Exact match
 	if val, ok := skipCLA[repo]; ok {
 		config = val
@@ -168,27 +188,21 @@ func SkipWhitelistedBots(ev events.Service, orgModel *models.GithubOrganization,
 		log.WithFields(f).Debug("No skip_cla config found for repo, skipping whitelisted bots check")
 		return actorsMissingCLA, []*UserCommitSummary{}
 	}
-	const nullStr = "(null)"
+
+	// Log full configuration
+	actorDebugData := make([]string, 0, len(actorsMissingCLA))
+	for _, a := range actorsMissingCLA {
+		actorDebugData = append(actorDebugData, actorToString(a))
+	}
+	log.WithFields(f).Debugf("final skip_cla config for repo %s is %s; actorsMissingCLA: [%s]", orgRepo, config, strings.Join(actorDebugData, ", "))
 
 	for _, actor := range actorsMissingCLA {
+		if actor == nil {
+			continue
+		}
+		actorData := actorToString(actor)
+		log.WithFields(f).Debugf("Checking actor: %s for skip_cla config: %s", actorData, config)
 		if isActorSkipped(actor, config) {
-			if actor == nil {
-				continue
-			}
-			id, login, username, email := nullStr, nullStr, nullStr, nullStr
-			if actor.CommitAuthor != nil && actor.CommitAuthor.ID != nil {
-				id = fmt.Sprintf("%v", *actor.CommitAuthor.ID)
-			}
-			if actor.CommitAuthor != nil && actor.CommitAuthor.Login != nil {
-				login = *actor.CommitAuthor.Login
-			}
-			if actor.CommitAuthor != nil && actor.CommitAuthor.Name != nil {
-				username = *actor.CommitAuthor.Name
-			}
-			if actor.CommitAuthor != nil && actor.CommitAuthor.Email != nil {
-				email = *actor.CommitAuthor.Email
-			}
-			actorData := fmt.Sprintf("id='%v',login='%v',username='%v',email='%v'", id, login, username, email)
 			msg := fmt.Sprintf(
 				"Skipping CLA check for repo='%s', actor: %s due to skip_cla config: '%s'",
 				orgRepo, actorData, config,
@@ -202,8 +216,6 @@ func SkipWhitelistedBots(ev events.Service, orgModel *models.GithubOrganization,
 			ev.LogEvent(&events.LogEventArgs{
 				EventType: events.BypassCLA,
 				EventData: &eventData,
-				UserID:    id,
-				UserName:  login,
 				ProjectID: projectID,
 			})
 			log.WithFields(f).Debugf("event logged")

cla-backend/cla/models/github_models.py

@@ -597,7 +597,7 @@ def update_merge_group_status(
                 f"signing url: {sign_url}"
             )
             cla.log.warning(
-                "{fn} - This is an error condition - "
+                f"{fn} - This is an error condition - "
                 f"should have at least one committer in one of these lists: "
                 f"{len(signed)} passed, {missing}"
             )
@@ -1037,19 +1037,28 @@ def skip_whitelisted_bots(self, org_model, org_repo, actors_missing_cla) -> Tupl
                 cla.log.debug("No skip_cla config found for repo %s, skipping whitelisted bots check", org_repo)
                 return actors_missing_cla, []
 
+            actor_debug_data = [
+                f"id='{getattr(a, 'author_id', '(null)')}',"
+                f"login='{getattr(a, 'author_login', '(null)')}',"
+                f"username='{getattr(a, 'author_username', '(null)')}',"
+                f"email='{getattr(a, 'author_email', '(null)')}'"
+                for a in actors_missing_cla
+            ]
+            cla.log.debug("final skip_cla config for repo %s is %s; actors_missing_cla: [%s]", org_repo, config, ", ".join(actor_debug_data))
             out_actors_missing_cla = []
             whitelisted_actors = []
             for actor in actors_missing_cla:
                 if actor is None:
                     continue
                 try:
+                    actor_data = "id='{}',login='{}',username='{}',email='{}'".format(
+                        getattr(actor, "author_id", "(null)"),
+                        getattr(actor, "author_login", "(null)"),
+                        getattr(actor, "author_username", "(null)"),
+                        getattr(actor, "author_email", "(null)"),
+                    )
+                    cla.log.debug("Checking actor: %s for skip_cla config: %s", actor_data, config)
                     if self.is_actor_skipped(actor, config):
-                        actor_data = "id='{}',login='{}',username='{}',email='{}'".format(
-                            getattr(actor, "author_id", "(null)"),
-                            getattr(actor, "author_login", "(null)"),
-                            getattr(actor, "author_username", "(null)"),
-                            getattr(actor, "author_email", "(null)"),
-                        )
                         msg = "Skipping CLA check for repo='{}', actor: {} due to skip_cla config: '{}'".format(
                             org_repo,
                             actor_data,
@@ -1912,7 +1921,7 @@ def update_pull_request(
                 f"signing url: {sign_url}"
             )
             cla.log.warning(
-                "{fn} - This is an error condition - "
+                f"{fn} - This is an error condition - "
                 f"should have at least one committer in one of these lists: "
                 f"{len(signed)} passed, {missing}"
             )

utils/skip_cla_entry.sh

@@ -6,7 +6,9 @@
 # delete-item Deletes the entire `skip_cla` entry.
 #
 # MODE=add-key ./utils/skip_cla_entry.sh sun-test-org 'repo1' 're:vee?rendra' '*'
+# MODE=add-key ./utils/skip_cla_entry.sh 'sun-test-org' 'repo1' 'lukaszgryglicki' 're:gryglicki'
 # ./utils/scan.sh github-orgs organization_name sun-test-org
+# STAGE=dev DTFROM='1 hour ago' DTTO='1 second ago' ./utils/search_aws_log_group.sh 'cla-backend-dev-githubactivity' 'skip_cla'
 
 if [ -z "$MODE" ]
 then