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

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,28 +21,46 @@ 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 {
+ var changedFiles []string
  return func() ([]string, error) {
- changed := make(map[string]bool)
- for _, commit := range pe.Commits {
- for _, added := range commit.Added {
- changed[added] = true
+ if changedFiles == nil {
+ // Fallback to use PushEvent
+ changes := make(map[string]bool)
+ for _, commit := range pe.Commits {
+ for _, added := range commit.Added {
+ changes[added] = true
+ }
+ for _, removed := range commit.Removed {
+ changes[removed] = true
+ }
+ for _, modified := range commit.Modified {
+ changes[modified] = true
+ }
  }
- for _, removed := range commit.Removed {
- changed[removed] = true
+ 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
+ }
+ }
  }
- for _, modified := range commit.Modified {
- changed[modified] = true
+
+ if len(changedFiles) == 0 {
+ for file := range changes {
+ changedFiles = append(changedFiles, file)
+ }
  }
  }
- var changedFiles []string
- for file := range changed {
- changedFiles = append(changedFiles, file)
- }
  return changedFiles, nil
  }
 }
@@ -74,7 +92,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 {