fix(pkg): fallback at using git client when github API files limits is reached.

Github API has an hard limit of 3000 files returned for push/pull request events.
Workaround this when checking pre/post submit jobs to be triggered,
by manually diffing using the git client.

Signed-off-by: Federico Di Pierro <[email protected]>

Author: FedeDP

cmd/status-reconciler/main.go

@@ -127,6 +127,11 @@ func main() {
 		logrus.WithError(err).Fatal("Error getting GitHub client.")
 	}
 
+	gitClient, err := o.github.GitClientFactory("", &o.config.InRepoConfigCacheDirBase, o.dryRun, false)
+	if err != nil {
+		logrus.WithError(err).Fatal("Error getting Git client.")
+	}
+
 	prowJobClient, err := o.kubernetes.ProwJobClient(configAgent.Config().ProwJobNamespace, o.dryRun)
 	if err != nil {
 		logrus.WithError(err).Fatal("Error getting kube client.")
@@ -139,7 +144,7 @@ func main() {
 		logrus.WithError(err).Fatal("Cannot create opener")
 	}
 
-	c := statusreconciler.NewController(o.continueOnError, o.getDenyList(), o.getDenyListAll(), opener, o.config, o.statusURI, prowJobClient, githubClient, pluginAgent)
+	c := statusreconciler.NewController(o.continueOnError, o.getDenyList(), o.getDenyListAll(), opener, o.config, o.statusURI, prowJobClient, gitClient, githubClient, pluginAgent)
 	interrupts.Run(func(ctx context.Context) {
 		c.Run(ctx)
 	})

pkg/config/jobs.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"net/url"
 	"regexp"
+	"sigs.k8s.io/prow/pkg/git/v2"
 	"strings"
 	"time"
 
@@ -525,7 +526,8 @@ type githubClient interface {
 // NewGitHubDeferredChangedFilesProvider uses a closure to lazily retrieve the file changes only if they are needed.
 // We only have to fetch the changes if there is at least one RunIfChanged/SkipIfOnlyChanged job that is not being
 // force run (due to a `/retest` after a failure or because it is explicitly triggered with `/test foo`).
-func NewGitHubDeferredChangedFilesProvider(client githubClient, org, repo string, num int) ChangedFilesProvider {
+func NewGitHubDeferredChangedFilesProvider(gc git.ClientFactory, client githubClient, org, repo string, num int,
+	baseSHA, headSHA string) ChangedFilesProvider {
 	var changedFiles []string
 	return func() ([]string, error) {
 		// Fetch the changed files from github at most once.
@@ -534,8 +536,25 @@ func NewGitHubDeferredChangedFilesProvider(client githubClient, org, repo string
 			if err != nil {
 				return nil, fmt.Errorf("error getting pull request changes: %w", err)
 			}
-			for _, change := range changes {
-				changedFiles = append(changedFiles, change.Filename)
+
+			// Fallback to use gitClient since github API truncated the response
+			if len(changes) == github.ChangesFilesLimit && gc != nil {
+				repoClient, err := gc.ClientFor(org, repo)
+				if err == nil {
+					// Use git client since Github PushEvent is limited to 3000 keys:
+					// https://docs.github.com/en/rest/pulls/pulls?apiVersion=2022-11-28#list-pull-requests-files
+					files, err := repoClient.Diff(headSHA, baseSHA)
+					if err == nil {
+						changedFiles = files
+					}
+				}
+			}
+
+			// in all failure cases, return truncated files
+			if len(changedFiles) == 0 {
+				for _, change := range changes {
+					changedFiles = append(changedFiles, change.Filename)
+				}
 			}
 		}
 		return changedFiles, nil

pkg/github/types.go

@@ -51,6 +51,10 @@ const (
 
 	// DefaultGraphQLEndpoint is the default GitHub GraphQL API endpoint.
 	DefaultGraphQLEndpoint = "https://api.github.com/graphql"
+
+	// ChangesFilesLimit is the limit to the files changed pushed by the github rest API.
+	// See https://docs.github.com/en/rest/pulls/pulls?apiVersion=2022-11-28#list-pull-requests-files.
+	ChangesFilesLimit = 3000
 )
 
 var (

pkg/plugins/skip/skip.go

@@ -114,7 +114,7 @@ func handle(gc githubClient, log *logrus.Entry, e *github.GenericCommentEvent, c
 	}
 	statuses := combinedStatus.Statuses
 
-	filteredPresubmits, err := trigger.FilterPresubmits(honorOkToTest, gc, e.Body, pr, presubmits, log)
+	filteredPresubmits, err := trigger.FilterPresubmits(honorOkToTest, gitClient, gc, e.Body, pr, presubmits, log)
 	if err != nil {
 		resp := fmt.Sprintf("Cannot get combined status for PR #%d in %s/%s: %v", number, org, repo, err)
 		log.Warn(resp)

pkg/plugins/trigger/generic-comment.go

@@ -18,6 +18,7 @@ package trigger
 
 import (
 	"fmt"
+	"sigs.k8s.io/prow/pkg/git/v2"
 
 	"github.com/sirupsen/logrus"
 	"sigs.k8s.io/prow/pkg/kube"
@@ -126,12 +127,12 @@ func handleGenericComment(c Client, trigger plugins.Trigger, gc github.GenericCo
 		return err
 	}
 
-	toTest, err := FilterPresubmits(HonorOkToTest(trigger), c.GitHubClient, gc.Body, pr, presubmits, c.Logger)
+	toTest, err := FilterPresubmits(HonorOkToTest(trigger), c.GitClient, c.GitHubClient, gc.Body, pr, presubmits, c.Logger)
 	if err != nil {
 		return err
 	}
 	if needsHelp, note := pjutil.ShouldRespondWithHelp(gc.Body, len(toTest)); needsHelp {
-		return addHelpComment(c.GitHubClient, gc.Body, org, repo, pr.Base.Ref, pr.Number, presubmits, gc.HTMLURL, commentAuthor, note, c.Logger)
+		return addHelpComment(c.GitClient, c.GitHubClient, gc.Body, org, repo, pr, presubmits, gc.HTMLURL, commentAuthor, note, c.Logger)
 	}
 	// we want to be able to track re-tests separately from the general body of tests
 	additionalLabels := map[string]string{}
@@ -195,7 +196,7 @@ type GitHubClient interface {
 // If a comment that we get matches more than one of the above patterns, we
 // consider the set of matching presubmits the union of the results from the
 // matching cases.
-func FilterPresubmits(honorOkToTest bool, gitHubClient GitHubClient, body string, pr *github.PullRequest, presubmits []config.Presubmit, logger *logrus.Entry) ([]config.Presubmit, error) {
+func FilterPresubmits(honorOkToTest bool, gc git.ClientFactory, gitHubClient GitHubClient, body string, pr *github.PullRequest, presubmits []config.Presubmit, logger *logrus.Entry) ([]config.Presubmit, error) {
 	org, repo, sha := pr.Base.Repo.Owner.Login, pr.Base.Repo.Name, pr.Head.SHA
 
 	contextGetter := func() (sets.Set[string], sets.Set[string], error) {
@@ -212,8 +213,8 @@ func FilterPresubmits(honorOkToTest bool, gitHubClient GitHubClient, body string
 		return nil, err
 	}
 
-	number, branch := pr.Number, pr.Base.Ref
-	changes := config.NewGitHubDeferredChangedFilesProvider(gitHubClient, org, repo, number)
+	number, branch, baseSHA, headSHA := pr.Number, pr.Base.Ref, pr.Base.SHA, pr.Head.SHA
+	changes := config.NewGitHubDeferredChangedFilesProvider(gc, gitHubClient, org, repo, number, baseSHA, headSHA)
 	return pjutil.FilterPresubmits(filter, changes, branch, presubmits, logger)
 }
 
@@ -231,8 +232,9 @@ func getContexts(combinedStatus *github.CombinedStatus) (sets.Set[string], sets.
 	return failedContexts, allContexts
 }
 
-func addHelpComment(githubClient githubClient, body, org, repo, branch string, number int, presubmits []config.Presubmit, HTMLURL, user, note string, logger *logrus.Entry) error {
-	changes := config.NewGitHubDeferredChangedFilesProvider(githubClient, org, repo, number)
+func addHelpComment(gc git.ClientFactory, githubClient githubClient, body, org, repo string, pr *github.PullRequest, presubmits []config.Presubmit, HTMLURL, user, note string, logger *logrus.Entry) error {
+	number, branch, baseSHA, headSHA := pr.Number, pr.Base.Ref, pr.Base.SHA, pr.Head.SHA
+	changes := config.NewGitHubDeferredChangedFilesProvider(gc, githubClient, org, repo, number, baseSHA, headSHA)
 	testAllNames, optionalJobsCommands, requiredJobsCommands, err := pjutil.AvailablePresubmits(changes, branch, presubmits, logger)
 	if err != nil {
 		return err

pkg/plugins/trigger/pull-request.go

@@ -354,8 +354,8 @@ func buildAllButDrafts(c Client, pr *github.PullRequest, eventGUID string, baseS
 
 // buildAll ensures that all builds that should run and will be required are built
 func buildAll(c Client, pr *github.PullRequest, eventGUID string, baseSHA string, presubmits []config.Presubmit) error {
-	org, repo, number, branch := pr.Base.Repo.Owner.Login, pr.Base.Repo.Name, pr.Number, pr.Base.Ref
-	changes := config.NewGitHubDeferredChangedFilesProvider(c.GitHubClient, org, repo, number)
+	org, repo, number, branch, headSHA := pr.Base.Repo.Owner.Login, pr.Base.Repo.Name, pr.Number, pr.Base.Ref, pr.Head.SHA
+	changes := config.NewGitHubDeferredChangedFilesProvider(c.GitClient, c.GitHubClient, org, repo, number, baseSHA, headSHA)
 	toTest, err := pjutil.FilterPresubmits(pjutil.NewTestAllFilter(), changes, branch, presubmits, c.Logger)
 	if err != nil {
 		return err

pkg/plugins/trigger/push.go

@@ -21,27 +21,44 @@ import (
 
 	prowapi "sigs.k8s.io/prow/pkg/apis/prowjobs/v1"
 	"sigs.k8s.io/prow/pkg/config"
+	"sigs.k8s.io/prow/pkg/git/v2"
 	"sigs.k8s.io/prow/pkg/github"
 	"sigs.k8s.io/prow/pkg/pjutil"
 )
 
-func listPushEventChanges(pe github.PushEvent) config.ChangedFilesProvider {
+func listPushEventChanges(gc git.ClientFactory, pe github.PushEvent) config.ChangedFilesProvider {
 	return func() ([]string, error) {
-		changed := make(map[string]bool)
+
+		// Fallback to use PushEvent
+		changes := make(map[string]bool)
 		for _, commit := range pe.Commits {
 			for _, added := range commit.Added {
-				changed[added] = true
+				changes[added] = true
 			}
 			for _, removed := range commit.Removed {
-				changed[removed] = true
+				changes[removed] = true
 			}
 			for _, modified := range commit.Modified {
-				changed[modified] = true
+				changes[modified] = true
 			}
 		}
 		var changedFiles []string
-		for file := range changed {
-			changedFiles = append(changedFiles, file)
+		if len(changes) == github.ChangesFilesLimit && gc != nil {
+			repoClient, err := gc.ClientFor(pe.Repo.Owner.Name, pe.Repo.Name)
+			if err == nil {
+				// Use git client since Github PushEvent is limited to 3000 keys:
+				// https://docs.github.com/en/rest/pulls/pulls?apiVersion=2022-11-28#list-pull-requests-files
+				files, err := repoClient.Diff(pe.After, pe.Before)
+				if err == nil {
+					changedFiles = files
+				}
+			}
+		}
+
+		if len(changedFiles) == 0 {
+			for file := range changes {
+				changedFiles = append(changedFiles, file)
+			}
 		}
 		return changedFiles, nil
 	}
@@ -74,7 +91,7 @@ func handlePE(c Client, pe github.PushEvent) error {
 	postsubmits := getPostsubmits(c.Logger, c.GitClient, c.Config, org+"/"+repo, shaGetter)
 
 	for _, j := range postsubmits {
-		if shouldRun, err := j.ShouldRun(pe.Branch(), listPushEventChanges(pe)); err != nil {
+		if shouldRun, err := j.ShouldRun(pe.Branch(), listPushEventChanges(c.GitClient, pe)); err != nil {
 			return err
 		} else if !shouldRun {
 			continue

pkg/statusreconciler/controller.go

@@ -19,6 +19,7 @@ package statusreconciler
 import (
 	"context"
 	"fmt"
+	"sigs.k8s.io/prow/pkg/git/v2"
 	"strings"
 	"time"
 
@@ -38,7 +39,7 @@ import (
 )
 
 // NewController constructs a new controller to reconcile stauses on config change
-func NewController(continueOnError bool, addedPresubmitDenylist, addedPresubmitDenylistAll sets.Set[string], opener io.Opener, configOpts configflagutil.ConfigOptions, statusURI string, prowJobClient prowv1.ProwJobInterface, githubClient github.Client, pluginAgent *plugins.ConfigAgent) *Controller {
+func NewController(continueOnError bool, addedPresubmitDenylist, addedPresubmitDenylistAll sets.Set[string], opener io.Opener, configOpts configflagutil.ConfigOptions, statusURI string, prowJobClient prowv1.ProwJobInterface, gc git.ClientFactory, githubClient github.Client, pluginAgent *plugins.ConfigAgent) *Controller {
 	sc := &statusController{
 		logger:     logrus.WithField("client", "statusController"),
 		opener:     opener,
@@ -57,6 +58,7 @@ func NewController(continueOnError bool, addedPresubmitDenylist, addedPresubmitD
 			pluginAgent:   pluginAgent,
 		},
 		githubClient: githubClient,
+		gitClient:    gc,
 		statusMigrator: &gitHubMigrator{
 			githubClient:    githubClient,
 			continueOnError: continueOnError,
@@ -151,6 +153,7 @@ type Controller struct {
 	addedPresubmitDenylistAll sets.Set[string]
 	prowJobTriggerer          prowJobTriggerer
 	githubClient              githubClient
+	gitClient                 git.ClientFactory
 	statusMigrator            statusMigrator
 	trustedChecker            trustedChecker
 	statusClient              statusClient
@@ -247,8 +250,8 @@ func (c *Controller) triggerNewPresubmits(addedPresubmits map[string][]config.Pr
 			filter := pjutil.NewArbitraryFilter(func(p config.Presubmit) (shouldRun bool, forcedToRun bool, defaultBehavior bool) {
 				return true, false, true
 			}, "inline-filter")
-			org, repo, number, branch := pr.Base.Repo.Owner.Login, pr.Base.Repo.Name, pr.Number, pr.Base.Ref
-			changes := config.NewGitHubDeferredChangedFilesProvider(c.githubClient, org, repo, number)
+			org, repo, number, branch, baseSHA, headSHA := pr.Base.Repo.Owner.Login, pr.Base.Repo.Name, pr.Number, pr.Base.Ref, pr.Base.SHA, pr.Head.SHA
+			changes := config.NewGitHubDeferredChangedFilesProvider(c.gitClient, c.githubClient, org, repo, number, baseSHA, headSHA)
 			logger := log.WithFields(logrus.Fields{"org": org, "repo": repo, "number": number, "branch": branch})
 			toTrigger, err := pjutil.FilterPresubmits(filter, changes, branch, presubmits, logger)
 			if err != nil {