Skip to content

Commit 8f6e773

Browse files
author
github-actions
committed
chore: regenerated
1 parent df3b203 commit 8f6e773

2 files changed

Lines changed: 228 additions & 6 deletions

File tree

‎pkg/repoowners/repoowners.go‎

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ type Config struct {
5555
Reviewers []string `json:"reviewers,omitempty"`
5656
RequiredReviewers []string `json:"required_reviewers,omitempty"`
5757
Labels []string `json:"labels,omitempty"`
58+
MinimumReviewers *int `json:"minimum_reviewers,omitempty"`
5859
}
5960

6061
// SimpleConfig holds options and Config applied to everything under the containing directory
@@ -65,7 +66,7 @@ type SimpleConfig struct {
6566

6667
// Empty checks if a SimpleConfig could be considered empty
6768
func (s *SimpleConfig) Empty() bool {
68-
return len(s.Approvers) == 0 && len(s.Reviewers) == 0 && len(s.RequiredReviewers) == 0 && len(s.Labels) == 0
69+
return len(s.Approvers) == 0 && len(s.Reviewers) == 0 && len(s.RequiredReviewers) == 0 && len(s.Labels) == 0 && s.MinimumReviewers == nil
6970
}
7071

7172
// FullConfig contains Filters which apply specific Config to files matching its regexp
@@ -158,6 +159,7 @@ type RepoOwner interface {
158159
LeafReviewers(path string) sets.String
159160
Reviewers(path string) sets.String
160161
RequiredReviewers(path string) sets.String
162+
MinimumReviewersForFile(path string) int
161163
}
162164

163165
var _ RepoOwner = &RepoOwners{}
@@ -170,6 +172,7 @@ type RepoOwners struct {
170172
reviewers map[string]map[*regexp.Regexp]sets.String
171173
requiredReviewers map[string]map[*regexp.Regexp]sets.String
172174
labels map[string]map[*regexp.Regexp]sets.String
175+
minimumReviewers map[string]map[*regexp.Regexp]int
173176
options map[string]dirOptions
174177

175178
baseDir string
@@ -387,6 +390,7 @@ func loadOwnersFrom(baseDir string, mdYaml bool, aliases RepoAliases, dirExclude
387390
reviewers: make(map[string]map[*regexp.Regexp]sets.String),
388391
requiredReviewers: make(map[string]map[*regexp.Regexp]sets.String),
389392
labels: make(map[string]map[*regexp.Regexp]sets.String),
393+
minimumReviewers: make(map[string]map[*regexp.Regexp]int),
390394
options: make(map[string]dirOptions),
391395

392396
dirExcludes: dirExcludes,
@@ -554,6 +558,18 @@ func (o *RepoOwners) applyConfigToPath(path string, re *regexp.Regexp, config *C
554558
}
555559
o.labels[path][re] = sets.NewString(config.Labels...)
556560
}
561+
562+
if config.MinimumReviewers != nil {
563+
minReviewers := *config.MinimumReviewers
564+
if minReviewers < 1 {
565+
o.log.WithField("path", path).Warnf("minimum_reviewers value %d is invalid, must be >= 1; defaulting to 1", minReviewers)
566+
minReviewers = 1
567+
}
568+
if o.minimumReviewers[path] == nil {
569+
o.minimumReviewers[path] = make(map[*regexp.Regexp]int)
570+
}
571+
o.minimumReviewers[path][re] = minReviewers
572+
}
557573
}
558574

559575
func (o *RepoOwners) applyOptionsToPath(path string, opts dirOptions) {
@@ -671,6 +687,58 @@ func (o *RepoOwners) entriesForFile(path string, people map[string]map[*regexp.R
671687
return out
672688
}
673689

690+
// MinimumReviewersForFile returns the minimum number of reviewers required to approve the requested file.
691+
// This is determined by the OWNERS file closest to the requested file.
692+
// If pkg/OWNERS has 2 minimum reviewers and pkg/util/OWNERS has 3 minimum reviewers this will return 3 for the path pkg/util/sets/file.go
693+
// If no minimum reviewers can be found then 1 is returned (the default behavior).
694+
func (o *RepoOwners) MinimumReviewersForFile(path string) int {
695+
d := path
696+
if !o.enableMDYAML || !strings.HasSuffix(path, ".md") {
697+
d = filepath.Dir(d)
698+
d = canonicalize(d)
699+
}
700+
701+
var foundMinReviewer *int
702+
for {
703+
relative, err := filepath.Rel(d, path)
704+
if err != nil {
705+
o.log.WithError(err).WithField("path", path).Errorf("Unable to find relative path between %q and path.", d)
706+
foundMinReviewer = nil
707+
break
708+
}
709+
var defaultValue *int
710+
for re, s := range o.minimumReviewers[d] {
711+
if re == nil {
712+
// Store default value but check regex patterns first
713+
defaultValue = &s
714+
continue
715+
}
716+
if re.MatchString(relative) {
717+
// Take the max of all matching regex patterns for deterministic behavior
718+
if foundMinReviewer == nil || s > *foundMinReviewer {
719+
foundMinReviewer = &s
720+
}
721+
}
722+
}
723+
// If no regex matched but we have a default, use it
724+
if foundMinReviewer == nil && defaultValue != nil {
725+
foundMinReviewer = defaultValue
726+
}
727+
if foundMinReviewer != nil || d == baseDirConvention {
728+
break
729+
}
730+
if o.options[d].NoParentOwners {
731+
break
732+
}
733+
d = filepath.Dir(d)
734+
d = canonicalize(d)
735+
}
736+
if foundMinReviewer == nil {
737+
return 1
738+
}
739+
return *foundMinReviewer
740+
}
741+
674742
// LeafApprovers returns a set of users who are the closest approvers to the
675743
// requested file. If pkg/OWNERS has user1 and pkg/util/OWNERS has user2 this
676744
// will only return user2 for the path pkg/util/sets/file.go

‎pkg/repoowners/repoowners_test.go‎

Lines changed: 159 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package repoowners
1818

1919
import (
2020
"fmt"
21+
"github.com/stretchr/testify/assert"
2122
"path/filepath"
2223
"reflect"
2324
"regexp"
@@ -58,6 +59,7 @@ reviewers:
5859
- jakub
5960
required_reviewers:
6061
- ben
62+
minimum_reviewers: 2
6163
labels:
6264
- src-code`),
6365
"src/dir/conformance/OWNERS": []byte(`options:
@@ -331,6 +333,7 @@ func TestLoadRepoOwners(t *testing.T) {
331333
extraBranchesAndFiles map[string]map[string][]byte
332334

333335
expectedApprovers, expectedReviewers, expectedRequiredReviewers, expectedLabels map[string]map[string]sets.String
336+
expectedMinRequiredReviewers map[string]map[string]int
334337

335338
expectedOptions map[string]dirOptions
336339
}{
@@ -354,6 +357,9 @@ func TestLoadRepoOwners(t *testing.T) {
354357
"": patternAll("EVERYTHING"),
355358
"src/dir": patternAll("src-code"),
356359
},
360+
expectedMinRequiredReviewers: map[string]map[string]int{
361+
"src/dir": {"": 2},
362+
},
357363
expectedOptions: map[string]dirOptions{
358364
"src/dir/conformance": {
359365
NoParentOwners: true,
@@ -381,6 +387,9 @@ func TestLoadRepoOwners(t *testing.T) {
381387
"": patternAll("EVERYTHING"),
382388
"src/dir": patternAll("src-code"),
383389
},
390+
expectedMinRequiredReviewers: map[string]map[string]int{
391+
"src/dir": {"": 2},
392+
},
384393
expectedOptions: map[string]dirOptions{
385394
"src/dir/conformance": {
386395
NoParentOwners: true,
@@ -411,6 +420,9 @@ func TestLoadRepoOwners(t *testing.T) {
411420
"src/dir": patternAll("src-code"),
412421
"docs/file.md": patternAll("docs"),
413422
},
423+
expectedMinRequiredReviewers: map[string]map[string]int{
424+
"src/dir": {"": 2},
425+
},
414426
expectedOptions: map[string]dirOptions{
415427
"src/dir/conformance": {
416428
NoParentOwners: true,
@@ -444,6 +456,9 @@ func TestLoadRepoOwners(t *testing.T) {
444456
"": patternAll("EVERYTHING"),
445457
"src/dir": patternAll("src-code"),
446458
},
459+
expectedMinRequiredReviewers: map[string]map[string]int{
460+
"src/dir": {"": 2},
461+
},
447462
expectedOptions: map[string]dirOptions{
448463
"src/dir/conformance": {
449464
NoParentOwners: true,
@@ -476,6 +491,9 @@ func TestLoadRepoOwners(t *testing.T) {
476491
"": patternAll("EVERYTHING"),
477492
"src/dir": patternAll("src-code"),
478493
},
494+
expectedMinRequiredReviewers: map[string]map[string]int{
495+
"src/dir": {"": 2},
496+
},
479497
expectedOptions: map[string]dirOptions{
480498
"src/dir/conformance": {
481499
NoParentOwners: true,
@@ -503,6 +521,9 @@ func TestLoadRepoOwners(t *testing.T) {
503521
"": patternAll("EVERYTHING"),
504522
"src/dir": patternAll("src-code"),
505523
},
524+
expectedMinRequiredReviewers: map[string]map[string]int{
525+
"src/dir": {"": 2},
526+
},
506527
expectedOptions: map[string]dirOptions{
507528
"src/dir/conformance": {
508529
NoParentOwners: true,
@@ -544,7 +565,7 @@ func TestLoadRepoOwners(t *testing.T) {
544565
continue
545566
}
546567

547-
check := func(field string, expected map[string]map[string]sets.String, got map[string]map[*regexp.Regexp]sets.String) {
568+
checkStringSet := func(field string, expected map[string]map[string]sets.String, got map[string]map[*regexp.Regexp]sets.String) {
548569
converted := map[string]map[string]sets.String{}
549570
for path, m := range got {
550571
converted[path] = map[string]sets.String{}
@@ -560,10 +581,29 @@ func TestLoadRepoOwners(t *testing.T) {
560581
t.Errorf("Expected %s to be:\n%+v\ngot:\n%+v.", field, expected, converted)
561582
}
562583
}
563-
check("approvers", test.expectedApprovers, ro.approvers)
564-
check("reviewers", test.expectedReviewers, ro.reviewers)
565-
check("required_reviewers", test.expectedRequiredReviewers, ro.requiredReviewers)
566-
check("labels", test.expectedLabels, ro.labels)
584+
585+
checkInt := func(field string, expected map[string]map[string]int, got map[string]map[*regexp.Regexp]int) {
586+
converted := map[string]map[string]int{}
587+
for path, m := range got {
588+
converted[path] = map[string]int{}
589+
for re, s := range m {
590+
var pattern string
591+
if re != nil {
592+
pattern = re.String()
593+
}
594+
converted[path][pattern] = s
595+
}
596+
}
597+
if !reflect.DeepEqual(expected, converted) {
598+
t.Errorf("Expected %s to be:\n%+v\ngot:\n%+v.", field, expected, converted)
599+
}
600+
}
601+
602+
checkStringSet("approvers", test.expectedApprovers, ro.approvers)
603+
checkStringSet("reviewers", test.expectedReviewers, ro.reviewers)
604+
checkStringSet("required_reviewers", test.expectedRequiredReviewers, ro.requiredReviewers)
605+
checkStringSet("labels", test.expectedLabels, ro.labels)
606+
checkInt("min_required_reviewers", test.expectedMinRequiredReviewers, ro.minimumReviewers)
567607
if !reflect.DeepEqual(test.expectedOptions, ro.options) {
568608
t.Errorf("Expected options to be:\n%#v\ngot:\n%#v.", test.expectedOptions, ro.options)
569609
}
@@ -858,3 +898,117 @@ func TestExpandAliases(t *testing.T) {
858898
}
859899
}
860900
}
901+
902+
func TestMinimumReviewersForFile(t *testing.T) {
903+
const (
904+
// No min reviewers
905+
baseDir = ""
906+
// Min reviewers set to 1
907+
secondDir = "a"
908+
// Min reviewers set to 2
909+
thirdDir = "a/b"
910+
// Min reviewers set to 3
911+
fourthDir = "a/b/c"
912+
// Dir with NoParentOwners - no min reviewers set, should not inherit from parent
913+
noParentDir = "a/b/noparent"
914+
// Dir with regex filters
915+
filterDir = "filtered"
916+
)
917+
918+
goFileRegex := regexp.MustCompile(`\.go$`)
919+
mdFileRegex := regexp.MustCompile(`\.md$`)
920+
// Overlapping regexes - both match main.go
921+
mainFileRegex := regexp.MustCompile(`^main`)
922+
allFilesRegex := regexp.MustCompile(`.*`)
923+
924+
ro := &RepoOwners{
925+
minimumReviewers: map[string]map[*regexp.Regexp]int{
926+
secondDir: {nil: 1},
927+
thirdDir: {nil: 2},
928+
fourthDir: {nil: 3},
929+
filterDir: {
930+
goFileRegex: 3,
931+
mdFileRegex: 1,
932+
nil: 2, // default
933+
},
934+
// Dir with overlapping regex patterns - should take max
935+
"overlap": {
936+
mainFileRegex: 2, // matches main.go
937+
allFilesRegex: 4, // also matches main.go - should take this (max)
938+
nil: 1, // default
939+
},
940+
},
941+
options: map[string]dirOptions{
942+
noParentDir: {
943+
NoParentOwners: true,
944+
},
945+
},
946+
}
947+
testCases := []struct {
948+
name string
949+
filePath string
950+
expectedRequiredApprovers int
951+
}{
952+
{
953+
name: "Modified Base Dir",
954+
filePath: filepath.Join(baseDir, "main.go"),
955+
expectedRequiredApprovers: 1,
956+
},
957+
{
958+
name: "Modified Second Dir",
959+
filePath: filepath.Join(secondDir, "main.go"),
960+
expectedRequiredApprovers: ro.minimumReviewers[secondDir][nil],
961+
},
962+
{
963+
name: "Modified Third Dir",
964+
filePath: filepath.Join(thirdDir, "main.go"),
965+
expectedRequiredApprovers: ro.minimumReviewers[thirdDir][nil],
966+
},
967+
{
968+
name: "Modified Nested Dir Without OWNERS (default to fourth dir)",
969+
filePath: filepath.Join(fourthDir, "d", "main.go"),
970+
expectedRequiredApprovers: ro.minimumReviewers[fourthDir][nil],
971+
},
972+
{
973+
name: "Modified Nonexistent Dir (default to Base Dir)",
974+
filePath: filepath.Join("nonexistent", "main.go"),
975+
expectedRequiredApprovers: 1,
976+
},
977+
{
978+
name: "NoParentOwners stops inheritance - should return default 1",
979+
filePath: filepath.Join(noParentDir, "main.go"),
980+
expectedRequiredApprovers: 1,
981+
},
982+
{
983+
name: "Regex filter matches .go file",
984+
filePath: filepath.Join(filterDir, "main.go"),
985+
expectedRequiredApprovers: 3,
986+
},
987+
{
988+
name: "Regex filter matches .md file",
989+
filePath: filepath.Join(filterDir, "README.md"),
990+
expectedRequiredApprovers: 1,
991+
},
992+
{
993+
name: "Regex filter no match uses default",
994+
filePath: filepath.Join(filterDir, "data.txt"),
995+
expectedRequiredApprovers: 2,
996+
},
997+
{
998+
name: "Overlapping regex patterns takes max value",
999+
filePath: filepath.Join("overlap", "main.go"),
1000+
expectedRequiredApprovers: 4, // both mainFileRegex(2) and allFilesRegex(4) match, should take max
1001+
},
1002+
{
1003+
name: "Overlapping regex - only one matches",
1004+
filePath: filepath.Join("overlap", "other.go"),
1005+
expectedRequiredApprovers: 4, // only allFilesRegex matches
1006+
},
1007+
}
1008+
for _, tc := range testCases {
1009+
t.Run(tc.name, func(t *testing.T) {
1010+
actual := ro.MinimumReviewersForFile(tc.filePath)
1011+
assert.Equal(t, tc.expectedRequiredApprovers, actual)
1012+
})
1013+
}
1014+
}

0 commit comments

Comments
 (0)