Merge pull request #4728 from linuxfoundation/unicron-4701-allow-bots-to-skip-cla-support-arrays-and-names

Refactor to support array of configs, additional name pattern and make email & name patterns optional + update util scripts and docs

Author: lukaszgryglicki

WHITELISTING_BOTS.md

@@ -4,14 +4,26 @@ You can allow specific bot users to automatically pass the CLA check.
 
 This can be done on the GitHub organization level by setting the `skip_cla` property on `cla-{stage}-github-orgs` DynamoDB table.
 
-This property is a Map attribute that contains mapping from repository pattern to bot username and email pattern.
+Replace `{stage}` with either `dev` or `prod`.
 
-Each pattern is a string and can be one of three possible types:
-- `"name"` - exact match for repository name, GitHub username, or email address.
+This property is a Map attribute that contains mapping from repository pattern to bot username (GitHub login), email and name pattern.
+
+Example username/login is lukaszgryglicki (like any username/login that can be accessed via `https://github.com/username`).
+
+Example name is "Lukasz Gryglicki".
+
+Email pattern and name pattern are optional and `*` is assumed for them if not specified.
+
+Each pattern is a string and can be one of three possible types (and are checked tin this order):
+- `"name"` - exact match for repository name, GitHub login/username, email address, GitHub name.
 - `"re:regexp"` - regular expression match for repository name, GitHub username, or email address.
 - `"*"` - matches all.
 
-So the format is like `"repository_pattern": "github_username_pattern;email_pattern"`.
+So the format is like `"repository_pattern": "github_username_pattern;email_pattern;name_pattern"`. `;` is used as a separator.
+
+You can also specify multiple patterns so different set is used for multiple users - in such case configuration must start with `[`, end with `]` and be `||` separated.
+
+For example: `"[copilot-swe-agent[bot];*;*||re:(?i)^l(ukasz)?gryglicki$;*;re:Gryglicki]"`.
 
 There can be multiple entries under one Github Organization DynamoDB entry.
 
@@ -25,10 +37,10 @@ Example:
     "skip_cla": {
       "M": {
         "*": {
-          "S": "copilot-swe-agent[bot];*"
+          "S": "copilot-swe-agent[bot];*;*"
         },
-        "repo1": {
-          "S": "re:vee?rendra;*"
+        "re:(?i)^repo[0-9]+$": {
+          "S": "re:vee?rendra;*;*"
         }
       }
     },
@@ -41,22 +53,24 @@ Algorithm to match pattern is as follows:
 - If no exact match is found, we check for regular expression match. Only keys starting with `re:` are considered. If we find a match, we use that entry and stop searching.
 - If no match is found, we check for `*` entry. If it exists, we use that entry and stop searching.
 - If no match is found, we don't skip CLA check.
-- Now when we have the entry, it is in the following format: `github_username_pattern;email_pattern`.
-- We check both GitHub username and email address against the patterns. Algorith is the same - username and email patterns can be either direct match or `re:regexp` or `*`.
-- If both username and email match the patterns, we skip CLA check. If username or email is not set but the pattern is `*` it means hit.
-- So setting pattern to `username_pattern;*` means that we only check for username match and assume all emails are valid.
-- If we set `repo_pattern` to `*` it means that this configuration applies to all repositories in the organization. If there are also specific repository patterns, they will be checked first.
+- Now when we have the entry, it is in the following format: `github_username_pattern;email_pattern;name_pattern` or `"[github_username_pattern;email_pattern;name_pattern||...]" (array)`.
+- We check GitHub username/login, email address and name against the patterns. Algorithm is the same - username, email and name patterns can be either direct match or `re:regexp` or `*`.
+- If username, email and name match the patterns, we skip CLA check. If username or email or name is not set but the pattern is `*` it means hit.
+- So setting pattern to `username_pattern;*;*` or `username_pattern` (which is equivalent) means that we only check for username match and assume all emails and names are valid.
+- Any actor that matches any of the entries in the array will be skipped (logical OR).
+- If we set `repo_pattern` to `*` it means that this configuration applies to all repositories in the organization. If there are also specific repository patterns, they will be used instead of `*` (fallback for all).
 
 
 There is a script that allows you to update the `skip_cla` property in the DynamoDB table. It is located in `utils/skip_cla_entry.sh`. You can run it like this:
-- `` MODE=mode ./utils/skip_cla_entry.sh 'org-name' 'repo-pattern' 'github-username-pattern' 'email-pattern' ``.
-- `` MODE=add-key ./utils/skip_cla_entry.sh 'sun-test-org' '*' 'copilot-swe-agent[bot]' '*' ``.
+- `` MODE=mode ./utils/skip_cla_entry.sh 'org-name' 'repo-pattern' 'github-username-pattern;email-pattern;name_pattern' ``.
+- `` MODE=add-key ./utils/skip_cla_entry.sh 'sun-test-org' '*' 'copilot-swe-agent[bot];*;*' ``.
+- Complex example: `` MODE=add-key ./utils/skip_cla_entry.sh 'sun-test-org' 're:(?i)^repo[0-9]+$' '[re:(?i)^l(ukasz)?gryglicki$;re:(?i)^l(ukasz)?gryglicki@;*||copilot-swe-agent[bot]]' ``.
 
 `MODE` can be one of:
-- `put-item`: Overwrites/adds the entire `skip_cla` property. Needs all 4 arguments org, repo, username and email.
-- `add-key`: Adds or updates a key/value inside the `skip_cla` map (preserves other keys). Needs all 4 args.
+- `put-item`: Overwrites/adds the entire `skip_cla` property. Needs all 3 arguments org, repo, and pattern.
+- `add-key`: Adds or updates a key/value inside the `skip_cla` map (preserves other keys). Needs all 3 args.
 - `delete-key`: Removes a key from the `skip_cla` map. Needs 2 arguments: org and repo.
-- `delete-item`: Deletes the entire `skip_cla` item. Needs 1 argument: org.
+- `delete-item`: Deletes the entire `skip_cla` from the item. Needs 1 argument: org.
 
 
 You can also use AWS CLI to update the `skip_cla` property. Here is an example command:
@@ -68,7 +82,7 @@ aws --profile "lfproduct-prod" --region "us-east-1" dynamodb update-item \
   --table-name "cla-prod-github-orgs" \
   --key '{"organization_name": {"S": "linuxfoundation"}}' \
   --update-expression 'SET skip_cla = :val' \
-  --expression-attribute-values '{":val": {"M": {"re:^easycla":{"S":"copilot-swe-agent[bot];*"}}}}'
+  --expression-attribute-values '{":val": {"M": {"re:^easycla":{"S":"copilot-swe-agent[bot];*;*"}}}}'
 ```
 
 To add a new key to an existing `skip_cla` entry (or replace the existing key):
@@ -79,7 +93,7 @@ aws --profile "lfproduct-prod" --region "us-east-1" dynamodb update-item \
   --key '{"organization_name": {"S": "linuxfoundation"}}' \
   --update-expression "SET skip_cla.#repo = :val" \
   --expression-attribute-names '{"#repo": "re:^easycla"}' \
-  --expression-attribute-values '{":val": {"S": "copilot-swe-agent[bot];*"}}'
+  --expression-attribute-values '{":val": {"S": "copilot-swe-agent[bot]"}}'
 ```
 
 To delete a key from an existing `skip_cla` entry:
@@ -110,3 +124,4 @@ aws --profile "lfproduct-prod" dynamodb scan --table-name "cla-prod-github-orgs"
 ```
 
 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

@@ -52,39 +52,40 @@ func stripOrg(repoFull string) string {
 	return repoFull
 }
 
-// isActorSkipped returns true if the given actor should be skipped according to the skip_cla config pattern.
-// config format: "<username_pattern>;<email_pattern>"
-// Actor.CommitAuthor.Login and Actor.CommitAuthor.Email should be *string, can be nil.
-func isActorSkipped(actor *UserCommitSummary, config string) bool {
-	f := logrus.Fields{
-		"functionName": "github.isActorSkipped",
-		"config":       config,
-	}
-	// Defensive: must have exactly one ';'
-	if !strings.Contains(config, ";") {
-		log.WithFields(f).Debugf("Invalid skip_cla config format: %s, expected '<username_pattern>;<email_pattern>'", config)
-		return false
-	}
-	parts := strings.SplitN(config, ";", 2)
-	if len(parts) != 2 {
-		return false
-	}
-	usernamePattern := parts[0]
-	emailPattern := parts[1]
-	var (
-		username string
-		email    string
-	)
-	if actor != nil && actor.CommitAuthor != nil && actor.CommitAuthor.Login != nil {
-		username = *actor.CommitAuthor.Login
-	}
-	if actor != nil && actor.CommitAuthor != nil && actor.CommitAuthor.Email != nil {
-		email = *actor.CommitAuthor.Email
-	}
+// isActorSkipped returns true if the actor should be skipped according to ANY pattern in config.
+// Each config entry is "<login_pattern>;<email_pattern>;<name_pattern>"
+// Any missing pattern defaults to "*"
+func isActorSkipped(actor *UserCommitSummary, config []string) bool {
+	for _, pattern := range config {
+		parts := strings.Split(pattern, ";")
+		for len(parts) < 3 {
+			parts = append(parts, "*")
+		}
+		loginPattern, emailPattern, namePattern := parts[0], parts[1], parts[2]
+
+		var login, email, name string
+		if actor != nil && actor.CommitAuthor != nil {
+			if actor.CommitAuthor.Login != nil {
+				login = *actor.CommitAuthor.Login
+			}
+			if actor.CommitAuthor.Email != nil {
+				email = *actor.CommitAuthor.Email
+			}
+			if actor.CommitAuthor.Name != nil {
+				name = *actor.CommitAuthor.Name
+			}
+		}
 
-	return propertyMatches(usernamePattern, username) && propertyMatches(emailPattern, email)
+		if propertyMatches(loginPattern, login) &&
+			propertyMatches(emailPattern, email) &&
+			propertyMatches(namePattern, name) {
+			return true
+		}
+	}
+	return false
 }
 
+// actorToString converts a UserCommitSummary actor to a string representation.
 func actorToString(actor *UserCommitSummary) string {
 	const nullStr = "(null)"
 	if actor == nil {
@@ -106,6 +107,22 @@ func actorToString(actor *UserCommitSummary) string {
 	return fmt.Sprintf("id='%v',login='%v',username='%v',email='%v'", id, login, username, email)
 }
 
+// parseConfigPatterns takes a config string and returns a slice of pattern strings.
+// If the config starts with '[' and ends with ']', splits by '||' inside; else returns []string{config}.
+// Trims whitespace from each pattern.
+func parseConfigPatterns(config string) []string {
+	config = strings.TrimSpace(config)
+	if len(config) >= 2 && strings.HasPrefix(config, "[") && strings.HasSuffix(config, "]") {
+		inner := config[1 : len(config)-1]
+		parts := strings.Split(inner, "||")
+		for i, p := range parts {
+			parts[i] = strings.TrimSpace(p)
+		}
+		return parts
+	}
+	return []string{config}
+}
+
 // 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
@@ -117,18 +134,20 @@ func actorToString(actor *UserCommitSummary) string {
 // : in cla-{stage}-github-orgs table there can be a skip_cla field which is a dict with the following structure:
 //
 //	{
-//	    "repo-name": "<username_pattern>;<email_pattern>",
-//	    "re:repo-regexp": "<username_pattern>;<email_pattern>",
-//	    "*": "<username_pattern>;<email_pattern>"
+//	    "repo-name": "<username_pattern>;<email_pattern>;<name_pattern>",
+//	    "re:repo-regexp": "[<username_pattern>;<email_pattern>;<name_pattern>||...]",
+//	    "*": "<login_pattern>"
 //	}
 //
 // where:
 //   - repo-name is the exact repository name under given org (e.g., "my-repo" not "my-org/my-repo")
 //   - re:repo-regexp is a regex pattern to match repository names
 //   - * is a wildcard that applies to all repositories
 //   - <username_pattern> is a GitHub username pattern (exact match or regex prefixed by re: or match all '*')
-//   - <email_pattern> is a GitHub email pattern (exact match or regex prefixed by re: or match all '*')
-//     The username and email patterns are separated by a semicolon (;).
+//   - <email_pattern> is a GitHub email pattern (exact match or regex prefixed by re: or match all '*') if not specified defaults to '*'
+//   - <name_pattern> is a GitHub name pattern (exact match or regex prefixed by re: or match all '*') if not specified defaults to '*'
+//     The username/login, email and name patterns are separated by a semicolon (;). Email and name parts are optional.
+//     There can be an array of patterns for a single repository, separated by ||. It must start with a '[' and end with a ']': "[...||...||...]"
 //     If the skip_cla is not set, it will skip the whitelisted bots check.
 func SkipWhitelistedBots(ev events.Service, orgModel *models.GithubOrganization, orgRepo, projectID string, actorsMissingCLA []*UserCommitSummary) ([]*UserCommitSummary, []*UserCommitSummary) {
 	repo := stripOrg(orgRepo)
@@ -189,23 +208,25 @@ func SkipWhitelistedBots(ev events.Service, orgModel *models.GithubOrganization,
 		return actorsMissingCLA, []*UserCommitSummary{}
 	}
 
+	configArray := parseConfigPatterns(config)
+
 	// 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, ", "))
+	log.WithFields(f).Debugf("final skip_cla config for repo %s is %+v; actorsMissingCLA: [%s]", orgRepo, configArray, 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) {
+		log.WithFields(f).Debugf("Checking actor: %s for skip_cla config: %+v", actorData, configArray)
+		if isActorSkipped(actor, configArray) {
 			msg := fmt.Sprintf(
-				"Skipping CLA check for repo='%s', actor: %s due to skip_cla config: '%s'",
-				orgRepo, actorData, config,
+				"Skipping CLA check for repo='%s', actor: %s due to skip_cla config: %+v",
+				orgRepo, actorData, configArray,
 			)
 			log.WithFields(f).Info(msg)
 			eventData := events.BypassCLAEventData{

cla-backend-go/swagger/common/github-organization.yaml

@@ -74,11 +74,27 @@ properties:
     additionalProperties:
       type: string
     description: |
-      Map of repository name or pattern (e.g. 'repo1', '*', 're:pattern') to a string in the form '<username_pattern>;<email_pattern>' for skipping CLA checks for certain bots. Patterns can be exact, wildcard '*', or regexp prefixed with 're:'.
-    example:
-      "*": "copilot-swe-agent[bot];*"
-      "repo1": "re:vee?rendra;*"
+      Map of repository name or pattern (e.g. 'repo1', '*', 're:pattern') to a string or array-string of pattern entries for skipping CLA checks for certain bots.
+
+      Each value can be either:
+      - A string in the form '<login_pattern>;<email_pattern>;<name_pattern>' (email and name patterns are optional, default to '*').
+      - Or an OR-array in the form '[<entry1>||<entry2>||...]', where each entry uses the same pattern format above.
 
+      Patterns can be:
+      - An exact match (e.g. 'repo1', 'username', 'email@domain').
+      - A wildcard '*' to match all.
+      - A regular expression prefixed with 're:' (e.g. 're:(?i)^bot.*').
+
+      Example formats:
+      - "copilot-swe-agent[bot];*;*"
+      - "re:vee?rendra;*;*"
+      - "[re:(?i)^l(ukasz)?gryglicki$;re:(?i)^l(ukasz)?gryglicki@;*||copilot-swe-agent[bot]]"
+      - "username;*"
+      - "username;[email protected];Real Name"
+    example:
+      "*": "copilot-swe-agent[bot];*;*"
+      "repo1": "re:vee?rendra;*;*"
+      "re:(?i)^repo[0-9]+$": "[re:(?i)^l(ukasz)?gryglicki$;re:(?i)^l(ukasz)?gryglicki@;*||copilot-swe-agent[bot]]"
   repositories:
     type: object
     properties: