Merge pull request #21693 from chaodaiG/revert-revert-ownersdir-denylist

Revert revert ownersdir denylist

prow/ANNOUNCEMENTS.md

@@ -135,6 +135,7 @@ Note: versions specified in these announcements may not include bug fixes made
 in more recent versions so it is recommended that the most recent versions are
 used when updating deployments.
 
+ - *April 1st, 2021* The `owners_dir_blacklist` field in prow config has been deprecated in favor of `owners_dir_denylist`. The support of `owners_dir_blacklist` will be stopped in October 2021.
  - *April 1st, 2021* The `labels_blacklist` field in verify-owners plugin config
    is deprecated in favor of `labels_denylist`. The support for `labels_blacklist` shall be stopped in
    *October 2021*.

prow/cmd/hook/main.go

@@ -204,13 +204,23 @@ func main() {
 	skipCollaborators := func(org, repo string) bool {
 		return pluginAgent.Config().SkipCollaborators(org, repo)
 	}
-	ownersDirBlacklist := func() config.OwnersDirBlacklist {
-		return configAgent.Config().OwnersDirBlacklist
+	ownersDirDenylist := func() *config.OwnersDirDenylist {
+		deprecated := configAgent.Config().OwnersDirBlacklist
+		l := configAgent.Config().OwnersDirDenylist
+		if deprecated != nil {
+			logrus.Warn("owners_dir_blacklist will be deprecated after October 2021, use owners_dir_denylist instead")
+			if l != nil {
+				logrus.Warn("Both owners_dir_blacklist and owners_dir_denylist are provided, owners_dir_blacklist is discarded")
+			} else {
+				l = deprecated
+			}
+		}
+		return l
 	}
 	resolver := func(org, repo string) ownersconfig.Filenames {
 		return pluginAgent.Config().OwnersFilenames(org, repo)
 	}
-	ownersClient := repoowners.NewClient(git.ClientFactoryFrom(gitClient), githubClient, mdYAMLEnabled, skipCollaborators, ownersDirBlacklist, resolver)
+	ownersClient := repoowners.NewClient(git.ClientFactoryFrom(gitClient), githubClient, mdYAMLEnabled, skipCollaborators, ownersDirDenylist, resolver)
 
 	clientAgent := &plugins.ClientAgent{
 		GitHubClient:              githubClient,

prow/config/config.go

@@ -152,9 +152,13 @@ type ProwConfig struct {
 	// PushGateway is a prometheus push gateway.
 	PushGateway PushGateway `json:"push_gateway,omitempty"`
 
-	// OwnersDirBlacklist is used to configure regular expressions matching directories
+	// OwnersDirDenylist is used to configure regular expressions matching directories
 	// to ignore when searching for OWNERS{,_ALIAS} files in a repo.
-	OwnersDirBlacklist OwnersDirBlacklist `json:"owners_dir_blacklist,omitempty"`
+	OwnersDirDenylist *OwnersDirDenylist `json:"owners_dir_denylist,omitempty"`
+
+	// OwnersDirBlacklist is deprecated, use OwnersDirDenylist instead
+	// TODO(chaodaiG, November 2021): Removed after October 2021
+	OwnersDirBlacklist *OwnersDirDenylist `json:"owners_dir_blacklist,omitempty"`
 
 	// Pub/Sub Subscriptions that we want to listen to
 	PubSubSubscriptions PubsubSubscriptions `json:"pubsub_subscriptions,omitempty"`
@@ -377,9 +381,9 @@ func (c *Config) GetPostsubmits(gc git.ClientFactory, identifier string, baseSHA
 	return append(c.PostsubmitsStatic[identifier], prowYAML.Postsubmits...), nil
 }
 
-// OwnersDirBlacklist is used to configure regular expressions matching directories
+// OwnersDirDenylist is used to configure regular expressions matching directories
 // to ignore when searching for OWNERS{,_ALIAS} files in a repo.
-type OwnersDirBlacklist struct {
+type OwnersDirDenylist struct {
 	// Repos configures a directory blacklist per repo (or org)
 	Repos map[string][]string `json:"repos,omitempty"`
 	// Default configures a default blacklist for all repos (or orgs).
@@ -395,17 +399,17 @@ type OwnersDirBlacklist struct {
 
 // ListIgnoredDirs returns regular expressions matching directories to ignore when
 // searching for OWNERS{,_ALIAS} files in a repo.
-func (ownersDirBlacklist OwnersDirBlacklist) ListIgnoredDirs(org, repo string) (ignorelist []string) {
-	ignorelist = append(ignorelist, ownersDirBlacklist.Default...)
-	if bl, ok := ownersDirBlacklist.Repos[org]; ok {
+func (o OwnersDirDenylist) ListIgnoredDirs(org, repo string) (ignorelist []string) {
+	ignorelist = append(ignorelist, o.Default...)
+	if bl, ok := o.Repos[org]; ok {
 		ignorelist = append(ignorelist, bl...)
 	}
-	if bl, ok := ownersDirBlacklist.Repos[org+"/"+repo]; ok {
+	if bl, ok := o.Repos[org+"/"+repo]; ok {
 		ignorelist = append(ignorelist, bl...)
 	}
 
 	preconfiguredDefaults := []string{"\\.git$", "_output$", "vendor/.*/.*"}
-	if !ownersDirBlacklist.IgnorePreconfiguredDefaults {
+	if !o.IgnorePreconfiguredDefaults {
 		ignorelist = append(ignorelist, preconfiguredDefaults...)
 	}
 	return

prow/config/config_test.go

@@ -5974,7 +5974,6 @@ log_level: info
 managed_webhooks:
   auto_accept_invitation: false
   respect_legacy_global_token: false
-owners_dir_blacklist: {}
 plank:
   max_goroutines: 20
   pod_pending_timeout: 10m0s

prow/config/prow-config-documented.yaml

@@ -481,8 +481,7 @@ managed_webhooks:
     respect_legacy_global_token: false
 
 
-# OwnersDirBlacklist is used to configure regular expressions matching directories
-# to ignore when searching for OWNERS{,_ALIAS} files in a repo.
+# OwnersDirBlacklist is deprecated, use OwnersDirDenylist instead
 owners_dir_blacklist:
     # Default configures a default blacklist for all repos (or orgs).
     # Some directories like ".git", "_output" and "vendor/.*/OWNERS"
@@ -493,6 +492,20 @@ owners_dir_blacklist:
     # Repos configures a directory blacklist per repo (or org)
     repos:
         "": null
+
+
+# OwnersDirDenylist is used to configure regular expressions matching directories
+# to ignore when searching for OWNERS{,_ALIAS} files in a repo.
+owners_dir_denylist:
+    # Default configures a default blacklist for all repos (or orgs).
+    # Some directories like ".git", "_output" and "vendor/.*/OWNERS"
+    # are already preconfigured to be blacklisted, and need not be included here.
+    default:
+      - ""
+
+    # Repos configures a directory blacklist per repo (or org)
+    repos:
+        "": null
 plank:
     # DefaultDecorationConfigEntries holds the default decoration config for specific values.
     # Each entry in the slice specifies Repo and Cluster regexp filter fields to

prow/hook/hook_test.go

@@ -108,7 +108,7 @@ func TestHook(t *testing.T) {
 	ca := &config.Agent{}
 	clientAgent := &plugins.ClientAgent{
 		GitHubClient:   github.NewFakeClient(),
-		OwnersClient:   repoowners.NewClient(nil, nil, func(org, repo string) bool { return false }, func(org, repo string) bool { return false }, func() config.OwnersDirBlacklist { return config.OwnersDirBlacklist{} }, ownersconfig.FakeResolver),
+		OwnersClient:   repoowners.NewClient(nil, nil, func(org, repo string) bool { return false }, func(org, repo string) bool { return false }, func() *config.OwnersDirDenylist { return &config.OwnersDirDenylist{} }, ownersconfig.FakeResolver),
 		BugzillaClient: &bugzilla.Fake{},
 	}
 	metrics := githubeventserver.NewMetrics()

prow/repoowners/repoowners.go

@@ -163,10 +163,10 @@ type Client struct {
 type delegate struct {
 	git git.ClientFactory
 
-	mdYAMLEnabled      func(org, repo string) bool
-	skipCollaborators  func(org, repo string) bool
-	ownersDirBlacklist func() prowConf.OwnersDirBlacklist
-	filenames          ownersconfig.Resolver
+	mdYAMLEnabled     func(org, repo string) bool
+	skipCollaborators func(org, repo string) bool
+	ownersDirDenylist func() *prowConf.OwnersDirDenylist
+	filenames         ownersconfig.Resolver
 
 	cache *cache
 }
@@ -196,7 +196,7 @@ func NewClient(
 	ghc github.Client,
 	mdYAMLEnabled func(org, repo string) bool,
 	skipCollaborators func(org, repo string) bool,
-	ownersDirBlacklist func() prowConf.OwnersDirBlacklist,
+	ownersDirDenylist func() *prowConf.OwnersDirDenylist,
 	filenames ownersconfig.Resolver,
 ) *Client {
 	return &Client{
@@ -206,10 +206,10 @@ func NewClient(
 			git:   gc,
 			cache: newCache(),
 
-			mdYAMLEnabled:      mdYAMLEnabled,
-			skipCollaborators:  skipCollaborators,
-			ownersDirBlacklist: ownersDirBlacklist,
-			filenames:          filenames,
+			mdYAMLEnabled:     mdYAMLEnabled,
+			skipCollaborators: skipCollaborators,
+			ownersDirDenylist: ownersDirDenylist,
+			filenames:         filenames,
 		},
 	}
 }
@@ -394,7 +394,10 @@ func (c *Client) cacheEntryFor(org, repo, base, cloneRef, fullName, sha string,
 			log.WithField("duration", time.Since(start).String()).Debugf("Completed loadAliasesFrom(%s, log)", gitRepo.Directory())
 
 			start = time.Now()
-			ignoreDirPatterns := c.ownersDirBlacklist().ListIgnoredDirs(org, repo)
+			var ignoreDirPatterns []string
+			if ownersDirDenylist := c.ownersDirDenylist(); ownersDirDenylist != nil {
+				ignoreDirPatterns = ownersDirDenylist.ListIgnoredDirs(org, repo)
+			}
 			var dirIgnorelist []*regexp.Regexp
 			for _, pattern := range ignoreDirPatterns {
 				re, err := regexp.Compile(pattern)

prow/repoowners/repoowners_test.go

@@ -149,8 +149,8 @@ func getTestClient(
 	skipCollab,
 	includeAliases bool,
 	ignorePreconfiguredDefaults bool,
-	ownersDirBlacklistDefault []string,
-	ownersDirBlacklistByRepo map[string][]string,
+	ownersDirDenylistDefault []string,
+	ownersDirDenylistByRepo map[string][]string,
 	extraBranchesAndFiles map[string]map[string][]byte,
 	cacheOptions *cacheOptions,
 	clients localgit.Clients,
@@ -276,10 +276,10 @@ labels:
 				skipCollaborators: func(org, repo string) bool {
 					return skipCollab
 				},
-				ownersDirBlacklist: func() prowConf.OwnersDirBlacklist {
-					return prowConf.OwnersDirBlacklist{
-						Repos:                       ownersDirBlacklistByRepo,
-						Default:                     ownersDirBlacklistDefault,
+				ownersDirDenylist: func() *prowConf.OwnersDirDenylist {
+					return &prowConf.OwnersDirDenylist{
+						Repos:                       ownersDirDenylistByRepo,
+						Default:                     ownersDirDenylistDefault,
 						IgnorePreconfiguredDefaults: ignorePreconfiguredDefaults,
 					}
 				},
@@ -294,16 +294,16 @@ labels:
 		nil
 }
 
-func TestOwnersDirBlacklist(t *testing.T) {
-	testOwnersDirBlacklist(localgit.New, t)
+func TestOwnersDirDenylist(t *testing.T) {
+	testOwnersDirDenylist(localgit.New, t)
 }
 
-func TestOwnersDirBlacklistV2(t *testing.T) {
-	testOwnersDirBlacklist(localgit.NewV2, t)
+func TestOwnersDirDenylistV2(t *testing.T) {
+	testOwnersDirDenylist(localgit.NewV2, t)
 }
 
-func testOwnersDirBlacklist(clients localgit.Clients, t *testing.T) {
-	getRepoOwnersWithBlacklist := func(t *testing.T, defaults []string, byRepo map[string][]string, ignorePreconfiguredDefaults bool) *RepoOwners {
+func testOwnersDirDenylist(clients localgit.Clients, t *testing.T) {
+	getRepoOwnersWithDenylist := func(t *testing.T, defaults []string, byRepo map[string][]string, ignorePreconfiguredDefaults bool) *RepoOwners {
 		client, cleanup, err := getTestClient(testFiles, true, false, true, ignorePreconfiguredDefaults, defaults, byRepo, nil, nil, clients)
 		if err != nil {
 			t.Fatalf("Error creating test client: %v.", err)
@@ -392,7 +392,7 @@ func testOwnersDirBlacklist(clients localgit.Clients, t *testing.T) {
 
 	for name, conf := range tests {
 		t.Run(name, func(t *testing.T) {
-			ro := getRepoOwnersWithBlacklist(t, conf.blacklistDefault, conf.blacklistByRepo, conf.ignorePreconfiguredDefaults)
+			ro := getRepoOwnersWithDenylist(t, conf.blacklistDefault, conf.blacklistByRepo, conf.ignorePreconfiguredDefaults)
 
 			includeDirs := sets.NewString(conf.includeDirs...)
 			excludeDirs := sets.NewString(conf.excludeDirs...)