Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace owners_dir_blacklist with owners_dir_denylist #21577

Merged
merged 1 commit into from
Apr 6, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions prow/ANNOUNCEMENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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*.
Expand Down
16 changes: 13 additions & 3 deletions prow/cmd/hook/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
22 changes: 13 additions & 9 deletions prow/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
chaodaiG marked this conversation as resolved.
Show resolved Hide resolved
// 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"`
Expand Down Expand Up @@ -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).
Expand All @@ -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
Expand Down
1 change: 0 additions & 1 deletion prow/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5973,7 +5973,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
Expand Down
17 changes: 15 additions & 2 deletions prow/config/prow-config-documented.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion prow/hook/hook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
20 changes: 10 additions & 10 deletions prow/repoowners/repoowners.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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{
Expand All @@ -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,
},
}
}
Expand Down Expand Up @@ -394,7 +394,7 @@ 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)
ignoreDirPatterns := c.ownersDirDenylist().ListIgnoredDirs(org, repo)
var dirIgnorelist []*regexp.Regexp
for _, pattern := range ignoreDirPatterns {
re, err := regexp.Compile(pattern)
Expand Down
26 changes: 13 additions & 13 deletions prow/repoowners/repoowners_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
}
},
Expand All @@ -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)
Expand Down Expand Up @@ -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...)
Expand Down