Repository: jenkins-x/lighthouse
Pull request #879 (2020-08-03 19:01:01)

fix: Get rid of whitelist/blacklist terminology
the `owners_dir_blacklist` field in the `config` `ConfigMap`, and the `labels_blacklist` field in the `plugins` `ConfigMap` should now change to `owners_dir_excludes` and `labels_excludes` respectively, but the existing fields will still function.

Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>


Patch:
diff --git a/pkg/config/config.go b/pkg/config/config.go
index 6ac0d716..43574457 100644
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@@ -106,9 +106,12 @@ type ProwConfig struct {
 	// PushGateway is a prometheus push gateway.
 	PushGateway PushGateway `json:"push_gateway,omitempty"`
 
-	// OwnersDirBlacklist is used to configure which directories to ignore when
+	// OwnersDirExcludes is used to configure which directories to ignore when
 	// searching for OWNERS{,_ALIAS} files in a repo.
-	OwnersDirBlacklist OwnersDirBlacklist `json:"owners_dir_blacklist,omitempty"`
+	OwnersDirExcludes *OwnersDirExcludes `json:"owners_dir_excludes,omitempty"`
+
+	// OwnersDirExcludes is DEPRECATED in favor of OwnersDirExcludes
+	OwnersDirBlacklist *OwnersDirExcludes `json:"owners_dir_blacklist,omitempty"`
 
 	// Pub/Sub Subscriptions that we want to listen to
 	PubSubSubscriptions PubsubSubscriptions `json:"pubsub_subscriptions,omitempty"`
@@ -131,9 +134,9 @@ type ProviderConfig struct {
 	BotUser string `json:"botUser,omitempty"`
 }
 
-// OwnersDirBlacklist is used to configure which directories to ignore when
+// OwnersDirExcludes is used to configure which directories to ignore when
 // searching for OWNERS{,_ALIAS} files in a repo.
-type OwnersDirBlacklist struct {
+type OwnersDirExcludes struct {
 	// Repos configures a directory blacklist per repo (or org)
 	Repos map[string][]string `json:"repos"`
 	// Default configures a default blacklist for repos (or orgs) not
@@ -639,6 +642,10 @@ func (c *Config) finalizeJobConfig() error {
 		}
 	}
 
+	if c.OwnersDirExcludes == nil {
+		c.OwnersDirExcludes = c.OwnersDirBlacklist
+	}
+
 	return nil
 }
 
diff --git a/pkg/config/jobs.go b/pkg/config/jobs.go
index 4a725007..ff4550fe 100644
--- a/pkg/config/jobs.go
+++ b/pkg/config/jobs.go
@@ -231,7 +231,7 @@ func (br Brancher) GetRE() *regexp.Regexp {
 	return br.re
 }
 
-// ShouldRun returns true if the input branch matches, given the whitelist/blacklist.
+// ShouldRun returns true if the input branch matches, given the includes/excludes.
 func (br Brancher) ShouldRun(branch string) bool {
 	if br.RunsAgainstAllBranch() {
 		return true
diff --git a/pkg/keeper/status.go b/pkg/keeper/status.go
index 75305e9d..5c499f23 100644
--- a/pkg/keeper/status.go
+++ b/pkg/keeper/status.go
@@ -126,22 +126,22 @@ func requirementDiff(pr *PullRequest, q *config.KeeperQuery, cc contextChecker)
 
 	// Weight incorrect branches with very high diff so that we select the query
 	// for the correct branch.
-	targetBranchBlacklisted := false
+	targetBranchExcluded := false
 	for _, excludedBranch := range q.ExcludedBranches {
 		if string(pr.BaseRef.Name) == excludedBranch {
-			targetBranchBlacklisted = true
+			targetBranchExcluded = true
 			break
 		}
 	}
-	// if no whitelist is configured, the target is OK by default
-	targetBranchWhitelisted := len(q.IncludedBranches) == 0
+	// if no inclusion list is configured, the target is OK by default
+	targetBranchIncluded := len(q.IncludedBranches) == 0
 	for _, includedBranch := range q.IncludedBranches {
 		if string(pr.BaseRef.Name) == includedBranch {
-			targetBranchWhitelisted = true
+			targetBranchIncluded = true
 			break
 		}
 	}
-	if targetBranchBlacklisted || !targetBranchWhitelisted {
+	if targetBranchExcluded || !targetBranchIncluded {
 		diff += 1000
 		if desc == "" {
 			desc = fmt.Sprintf(" Merging to branch %s is forbidden.", pr.BaseRef.Name)
diff --git a/pkg/plugins/config.go b/pkg/plugins/config.go
index 6e7438ee..4fbafad9 100644
--- a/pkg/plugins/config.go
+++ b/pkg/plugins/config.go
@@ -143,9 +143,12 @@ type Owners struct {
 	// control in the provided repos.
 	SkipCollaborators []string `json:"skip_collaborators,omitempty"`
 
-	// LabelsBlackList holds a list of labels that should not be present in any
+	// LabelsExcludeList holds a list of labels that should not be present in any
 	// OWNERS file, preventing their automatic addition by the owners-label plugin.
 	// This check is performed by the verify-owners plugin.
+	LabelsExcludeList []string `json:"labels_excludes,omitempty"`
+
+	// LabelsBlackList is DEPRECATED in favor of LabelsExcludeList
 	LabelsBlackList []string `json:"labels_blacklist,omitempty"`
 }
 
@@ -438,16 +441,20 @@ type ConfigUpdater struct {
 
 // MergeWarning is a config for the slackevents plugin's manual merge warnings.
 // If a PR is pushed to any of the repos listed in the config then send messages
-// to the all the slack channels listed if pusher is NOT in the whitelist.
+// to the all the slack channels listed if pusher is NOT in the includes.
 type MergeWarning struct {
 	// Repos is either of the form org/repos or just org.
 	Repos []string `json:"repos,omitempty"`
 	// List of channels on which a event is published.
 	Channels []string `json:"channels,omitempty"`
-	// A slack event is published if the user is not part of the WhiteList.
-	WhiteList []string `json:"whitelist,omitempty"`
-	// A slack event is published if the user is not on the branch whitelist
-	BranchWhiteList map[string][]string `json:"branch_whitelist,omitempty"`
+	// A slack event is published if the user is not part of the includes.
+	Includes []string `json:"includes,omitempty"`
+	// Deprecated in favor of Includes
+	Whitelist []string `json:"whitelist,omitempty"`
+	// A slack event is published if the user is not in the branch includes
+	BranchIncludes map[string][]string `json:"branch_includes,omitempty"`
+	// Deprecated in favor of BranchIncludes
+	BranchWhitelist map[string][]string `json:"branch_whitelist,omitempty"`
 }
 
 // Welcome is config for the welcome plugin.
@@ -689,8 +696,11 @@ func (c *Configuration) setDefaults() {
 	if c.SigMention.Regexp == "" {
 		c.SigMention.Regexp = `(?m)@kubernetes/sig-([\w-]*)-(misc|test-failures|bugs|feature-requests|proposals|pr-reviews|api-reviews)`
 	}
-	if c.Owners.LabelsBlackList == nil {
-		c.Owners.LabelsBlackList = []string{labels.Approved, labels.LGTM}
+	if c.Owners.LabelsExcludeList == nil {
+		c.Owners.LabelsExcludeList = c.Owners.LabelsBlackList
+	}
+	if c.Owners.LabelsExcludeList == nil {
+		c.Owners.LabelsExcludeList = []string{labels.Approved, labels.LGTM}
 	}
 	for _, milestone := range c.RepoMilestone {
 		if milestone.MaintainersFriendlyName == "" {
diff --git a/pkg/repoowners/repoowners.go b/pkg/repoowners/repoowners.go
index 3e34de58..99f2234b 100644
--- a/pkg/repoowners/repoowners.go
+++ b/pkg/repoowners/repoowners.go
@@ -43,7 +43,7 @@ const (
 	baseDirConvention = ""
 )
 
-var defaultDirBlacklist = sets.NewString(".git", "_output")
+var defaultDirExcludes = sets.NewString(".git", "_output")
 
 type dirOptions struct {
 	NoParentOwners bool `json:"no_parent_owners,omitempty"`
@@ -160,7 +160,7 @@ type RepoOwners struct {
 
 	baseDir      string
 	enableMDYAML bool
-	dirBlacklist sets.String
+	dirExcludes  sets.String
 
 	log *logrus.Entry
 }
@@ -231,16 +231,16 @@ func (c *Client) LoadRepoOwners(org, repo, base string) (RepoOwner, error) {
 			entry.aliases = loadAliasesFrom(gitRepo.Dir, log)
 		}
 
-		blacklistConfig := c.config.OwnersDirBlacklist
+		excludeConfig := c.config.OwnersDirExcludes
 
-		dirBlacklist := defaultDirBlacklist.Union(sets.NewString(blacklistConfig.Default...))
-		if bl, ok := blacklistConfig.Repos[org]; ok {
-			dirBlacklist.Insert(bl...)
+		dirExcludes := defaultDirExcludes.Union(sets.NewString(excludeConfig.Default...))
+		if bl, ok := excludeConfig.Repos[org]; ok {
+			dirExcludes.Insert(bl...)
 		}
-		if bl, ok := blacklistConfig.Repos[org+"/"+repo]; ok {
-			dirBlacklist.Insert(bl...)
+		if bl, ok := excludeConfig.Repos[org+"/"+repo]; ok {
+			dirExcludes.Insert(bl...)
 		}
-		entry.owners, err = loadOwnersFrom(gitRepo.Dir, mdYaml, entry.aliases, dirBlacklist, log)
+		entry.owners, err = loadOwnersFrom(gitRepo.Dir, mdYaml, entry.aliases, dirExcludes, log)
 		if err != nil {
 			return nil, fmt.Errorf("failed to load RepoOwners for %s: %v", fullName, err)
 		}
@@ -316,7 +316,7 @@ func loadAliasesFrom(baseDir string, log *logrus.Entry) RepoAliases {
 	return result
 }
 
-func loadOwnersFrom(baseDir string, mdYaml bool, aliases RepoAliases, dirBlacklist sets.String, log *logrus.Entry) (*RepoOwners, error) {
+func loadOwnersFrom(baseDir string, mdYaml bool, aliases RepoAliases, dirExcludes sets.String, log *logrus.Entry) (*RepoOwners, error) {
 	o := &RepoOwners{
 		RepoAliases:  aliases,
 		baseDir:      baseDir,
@@ -329,7 +329,7 @@ func loadOwnersFrom(baseDir string, mdYaml bool, aliases RepoAliases, dirBlackli
 		labels:            make(map[string]map[*regexp.Regexp]sets.String),
 		options:           make(map[string]dirOptions),
 
-		dirBlacklist: dirBlacklist,
+		dirExcludes: dirExcludes,
 	}
 
 	return o, filepath.Walk(o.baseDir, o.walkFunc)
@@ -353,7 +353,7 @@ func (o *RepoOwners) walkFunc(path string, info os.FileInfo, err error) error {
 	}
 	filename := filepath.Base(path)
 
-	if info.Mode().IsDir() && o.dirBlacklist.Has(filename) {
+	if info.Mode().IsDir() && o.dirExcludes.Has(filename) {
 		return filepath.SkipDir
 	}
 	if !info.Mode().IsRegular() {


Test patch:
diff --git a/pkg/keeper/status_test.go b/pkg/keeper/status_test.go
index 211b62de..11ca60ca 100644
--- a/pkg/keeper/status_test.go
+++ b/pkg/keeper/status_test.go
@@ -36,15 +36,15 @@ func TestExpectedStatus(t *testing.T) {
 	testcases := []struct {
 		name string
 
-		baseref         string
-		branchWhitelist []string
-		branchBlacklist []string
-		sameBranchReqs  bool
-		labels          []string
-		milestone       string
-		contexts        []Context
-		inPool          bool
-		blocks          []int
+		baseref           string
+		branchIncludeList []string
+		branchExcludeList []string
+		sameBranchReqs    bool
+		labels            []string
+		milestone         string
+		contexts          []Context
+		inPool            bool
+		blocks            []int
 
 		state string
 		desc  string
@@ -101,34 +101,34 @@ func TestExpectedStatus(t *testing.T) {
 			desc:  fmt.Sprintf(statusNotInPool, " Needs need-1 label."),
 		},
 		{
-			name:            "against excluded branch",
-			baseref:         "bad",
-			branchBlacklist: []string{"bad"},
-			sameBranchReqs:  true,
-			labels:          neededLabels,
-			inPool:          false,
+			name:              "against excluded branch",
+			baseref:           "bad",
+			branchExcludeList: []string{"bad"},
+			sameBranchReqs:    true,
+			labels:            neededLabels,
+			inPool:            false,
 
 			state: scmprovider.StatusPending,
 			desc:  fmt.Sprintf(statusNotInPool, " Merging to branch bad is forbidden."),
 		},
 		{
-			name:            "not against included branch",
-			baseref:         "bad",
-			branchWhitelist: []string{"good"},
-			sameBranchReqs:  true,
-			labels:          neededLabels,
-			inPool:          false,
+			name:              "not against included branch",
+			baseref:           "bad",
+			branchIncludeList: []string{"good"},
+			sameBranchReqs:    true,
+			labels:            neededLabels,
+			inPool:            false,
 
 			state: scmprovider.StatusPending,
 			desc:  fmt.Sprintf(statusNotInPool, " Merging to branch bad is forbidden."),
 		},
 		{
-			name:            "choose query for correct branch",
-			baseref:         "bad",
-			branchWhitelist: []string{"good"},
-			milestone:       "v1.0",
-			labels:          neededLabels,
-			inPool:          false,
+			name:              "choose query for correct branch",
+			baseref:           "bad",
+			branchIncludeList: []string{"good"},
+			milestone:         "v1.0",
+			labels:            neededLabels,
+			inPool:            false,
 
 			state: scmprovider.StatusPending,
 			desc:  fmt.Sprintf(statusNotInPool, " Needs 1, 2, 3, 4, 5, 6, 7 labels."),
@@ -215,14 +215,14 @@ func TestExpectedStatus(t *testing.T) {
 			Milestone: "v1.0",
 		}
 		if tc.sameBranchReqs {
-			secondQuery.ExcludedBranches = tc.branchBlacklist
-			secondQuery.IncludedBranches = tc.branchWhitelist
+			secondQuery.ExcludedBranches = tc.branchExcludeList
+			secondQuery.IncludedBranches = tc.branchIncludeList
 		}
 		queriesByRepo := config.KeeperQueries{
 			config.KeeperQuery{
 				Orgs:             []string{""},
-				ExcludedBranches: tc.branchBlacklist,
-				IncludedBranches: tc.branchWhitelist,
+				ExcludedBranches: tc.branchExcludeList,
+				IncludedBranches: tc.branchIncludeList,
 				Labels:           neededLabels,
 				MissingLabels:    forbiddenLabels,
 				Milestone:        "v1.0",
diff --git a/pkg/repoowners/repoowners_test.go b/pkg/repoowners/repoowners_test.go
index 99007147..b7f940aa 100644
--- a/pkg/repoowners/repoowners_test.go
+++ b/pkg/repoowners/repoowners_test.go
@@ -105,8 +105,8 @@ func getTestClient(
 	enableMdYaml,
 	skipCollab,
 	includeAliases bool,
-	ownersDirBlacklistDefault []string,
-	ownersDirBlacklistByRepo map[string][]string,
+	ownersDirExcludesDefault []string,
+	ownersDirExcludesByRepo map[string][]string,
 	extraBranchesAndFiles map[string]map[string][]byte,
 ) (*Client, func(), error) {
 	testAliasesFile := map[string][]byte{
@@ -158,9 +158,9 @@ func getTestClient(
 			},
 			config: &prowConf.Config{
 				ProwConfig: prowConf.ProwConfig{
-					OwnersDirBlacklist: prowConf.OwnersDirBlacklist{
-						Repos:   ownersDirBlacklistByRepo,
-						Default: ownersDirBlacklistDefault,
+					OwnersDirExcludes: &prowConf.OwnersDirExcludes{
+						Repos:   ownersDirExcludesByRepo,
+						Default: ownersDirExcludesDefault,
 					},
 				},
 			},
@@ -173,7 +173,7 @@ func getTestClient(
 		nil
 }
 
-func TestOwnersDirBlacklist(t *testing.T) {
+func TestOwnersDirExcludes(t *testing.T) {
 	validatorExcluded := func(t *testing.T, ro *RepoOwners) {
 		for dir := range ro.approvers {
 			if strings.Contains(dir, "src") {
@@ -211,7 +211,7 @@ func TestOwnersDirBlacklist(t *testing.T) {
 		}
 	}
 
-	getRepoOwnersWithBlacklist := func(t *testing.T, defaults []string, byRepo map[string][]string) *RepoOwners {
+	getRepoOwnersWithExcludes := func(t *testing.T, defaults []string, byRepo map[string][]string) *RepoOwners {
 		client, cleanup, err := getTestClient(testFiles, true, false, true, defaults, byRepo, nil)
 		if err != nil {
 			t.Fatalf("Error creating test client: %v.", err)
@@ -227,34 +227,34 @@ func TestOwnersDirBlacklist(t *testing.T) {
 	}
 
 	type testConf struct {
-		blackistDefault []string
-		blacklistByRepo map[string][]string
+		excludesDefault []string
+		excludesByRepo  map[string][]string
 		validator       func(t *testing.T, ro *RepoOwners)
 	}
 
 	tests := map[string]testConf{}
 
-	tests["blacklist by org"] = testConf{
-		blacklistByRepo: map[string][]string{
+	tests["excludes by org"] = testConf{
+		excludesByRepo: map[string][]string{
 			"org": {"src"},
 		},
 		validator: validatorExcluded,
 	}
-	tests["blacklist by org/repo"] = testConf{
-		blacklistByRepo: map[string][]string{
+	tests["excludes by org/repo"] = testConf{
+		excludesByRepo: map[string][]string{
 			"org/repo": {"src"},
 		},
 		validator: validatorExcluded,
 	}
-	tests["blacklist by default"] = testConf{
-		blackistDefault: []string{"src"},
+	tests["excludes by default"] = testConf{
+		excludesDefault: []string{"src"},
 		validator:       validatorExcluded,
 	}
-	tests["no blacklist setup"] = testConf{
+	tests["no excludes setup"] = testConf{
 		validator: validatorIncluded,
 	}
-	tests["blacklist setup but not matching this repo"] = testConf{
-		blacklistByRepo: map[string][]string{
+	tests["excludes setup but not matching this repo"] = testConf{
+		excludesByRepo: map[string][]string{
 			"not_org/not_repo": {"src"},
 			"not_org":          {"src"},
 		},
@@ -263,7 +263,7 @@ func TestOwnersDirBlacklist(t *testing.T) {
 
 	for name, conf := range tests {
 		t.Run(name, func(t *testing.T) {
-			ro := getRepoOwnersWithBlacklist(t, conf.blackistDefault, conf.blacklistByRepo)
+			ro := getRepoOwnersWithExcludes(t, conf.excludesDefault, conf.excludesByRepo)
 			conf.validator(t, ro)
 		})
 	}
diff --git a/pkg/webhook/test_data/test_config.yaml b/pkg/webhook/test_data/test_config.yaml
index 6e85be20..ff36a23d 100644
--- a/pkg/webhook/test_data/test_config.yaml
+++ b/pkg/webhook/test_data/test_config.yaml
@@ -68,7 +68,7 @@ branch-protection:
 deck:
   spyglass: {}
 gerrit: {}
-owners_dir_blacklist:
+owners_dir_excludes:
   default: null
   repos: null
 plank: {}
diff --git a/test/e2e/test_data/example-config.tmpl.yml b/test/e2e/test_data/example-config.tmpl.yml
index 0dcb34c6..c89eb2b0 100644
--- a/test/e2e/test_data/example-config.tmpl.yml
+++ b/test/e2e/test_data/example-config.tmpl.yml
@@ -12,7 +12,7 @@ deck:
 gerrit: {}
 github:
   LinkURL: null
-owners_dir_blacklist:
+owners_dir_excludes:
   default: null
   repos: null
 plank: {}