Make verify-owners limit trusted user checks. #16997
Conversation
k8s-ci-robot
added
cncf-cla: yes
k8s-ci-robot
added
area/prow
| func checkIfTrustedUser(ghc githubClient, log *logrus.Entry, triggerConfig plugins.Trigger, owner, patch, fileName, org, repo string, nonTrustedUsers map[string][]string, repoAliases repoowners.RepoAliases) (map[string][]string, error) { | ||
| func checkIfTrustedUser(ghc githubClient, log *logrus.Entry, triggerConfig plugins.Trigger, owner, patch, fileName, org, repo string, nonTrustedUsers map[string][]string, trustedUsers sets.String, repoAliases repoowners.RepoAliases) (map[string][]string, error) { | ||
| // cap the number of checks to avoid exhausting tokens in case of large OWNERS refactors. | ||
| if len(nonTrustedUsers)+trustedUsers.Len() > 50 { |
There was a problem hiding this comment.
So this effectively means that on larger changes, we might end up merging invalid owners since nonTrustedUsers could be empty, right?
There was a problem hiding this comment.
Also not sure if this correct, because the nonTrusteduser s are a map[string][]string, you re checking the key lenght here (repo I guess?) rather than the unique values
There was a problem hiding this comment.
Correct. We'll still validate that the files parse and do other checks included in the plugin, but after seeing 51 unique users we'll stop checking if they are trusted to avoid using too many tokens.
There was a problem hiding this comment.
The keys are the untrusted owners and the values are the files containing the untrusted owner
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alvaroaleman, cjwagner The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This is an imperfect and dirty quick fix to limit verify-owners token usage.
/assign @stevekuznetsov