Repository: jenkins-x/lighthouse (go)

Issue:
Feature request: Share aliases between repositories
As a fall back for aliases missing from the OWNER_ALIASES file I propose support for reading OWNER_ALIASES files from other repositories. 

This is practical when teams have responsibility for several repositories.

Pull request description:
feat: Use OWNERS_ALIASES files from other repos
For organizations with many repositories it would make sense to refer to other repos to find OWNERS_ALIASES files in.

This PR implements the added field foreignAliases.

So an example OWNERS_ALIASES could look like
```
aliases:
  best-approvers:
  - you
  - me
foreignAliases:
- name: a-repo
- org: another-org
  name: the-repo
- org: yaorg
  name: yarepo
  ref: special-branch
```

org defaults to current organization
ref default to HEAD

If an alias is present in several sources the members are merged.

Fixes #1382

Patch:
diff --git a/pkg/repoowners/repoowners.go b/pkg/repoowners/repoowners.go
index a2ac2b08..74428dc4 100644
--- a/pkg/repoowners/repoowners.go
+++ b/pkg/repoowners/repoowners.go
@@ -77,6 +77,7 @@ type FullConfig struct {
 type scmProviderClient interface {
 	ListCollaborators(org, repo string) ([]scm.User, error)
 	GetRef(org, repo, ref string) (string, error)
+	GetFile(org, repo, filepath, ref string) ([]byte, error)
 }
 
 type cacheEntry struct {
@@ -133,6 +134,19 @@ func NewClient(
 // RepoAliases defines groups of people to be used in OWNERS files
 type RepoAliases map[string]sets.String
 
+// ForeignAlias contains a
+type ForeignAlias struct {
+	Org  string `json:"org"`
+	Repo string `json:"name"`
+	Ref  string `json:"ref,omitempty"`
+}
+
+// OwnerAliases contains the parsed OWNER_ALIASES config.
+type OwnerAliases struct {
+	Aliases        map[string][]string `json:"aliases,omitempty"`
+	ForeignAliases []ForeignAlias      `json:"foreignAliases,omitempty"`
+}
+
 // RepoOwner is an interface to work with repoowners
 type RepoOwner interface {
 	FindApproverOwnersForFile(path string) string
@@ -192,7 +206,7 @@ func (c *Client) LoadRepoAliases(org, repo, base string) (RepoAliases, error) {
 			return nil, err
 		}
 
-		entry.aliases = loadAliasesFrom(gitRepo.Dir, log)
+		entry.aliases = c.loadAliasesFrom(gitRepo.Dir, log, org)
 		entry.sha = sha
 		c.cache[fullName] = entry
 	}
@@ -228,7 +242,7 @@ func (c *Client) LoadRepoOwners(org, repo, base string) (RepoOwner, error) {
 
 		if entry.aliases == nil || entry.sha != sha {
 			// aliases must be loaded
-			entry.aliases = loadAliasesFrom(gitRepo.Dir, log)
+			entry.aliases = c.loadAliasesFrom(gitRepo.Dir, log, org)
 		}
 
 		excludeConfig := c.config.OwnersDirExcludes
@@ -294,7 +308,7 @@ func (a RepoAliases) ExpandAliases(logins sets.String) sets.String {
 	return logins
 }
 
-func loadAliasesFrom(baseDir string, log *logrus.Entry) RepoAliases {
+func (c *Client) loadAliasesFrom(baseDir string, log *logrus.Entry, org string) RepoAliases {
 	path := filepath.Join(baseDir, aliasesFileName)
 	b, err := ioutil.ReadFile(path) // #nosec
 	if os.IsNotExist(err) {
@@ -304,22 +318,50 @@ func loadAliasesFrom(baseDir string, log *logrus.Entry) RepoAliases {
 		log.WithError(err).Warnf("Failed to read alias file %q. Using empty alias map.", path)
 		return nil
 	}
-	config := &struct {
-		Data map[string][]string `json:"aliases,omitempty"`
-	}{}
-	if err := yaml.Unmarshal(b, config); err != nil {
-		log.WithError(err).Errorf("Failed to unmarshal aliases from %q. Using empty alias map.", path)
-		return nil
-	}
-
 	result := make(RepoAliases)
-	for alias, expanded := range config.Data {
-		result[scmprovider.NormLogin(alias)] = normLogins(expanded)
+	err = c.parseAliases(result, b, log, org)
+	if err != nil {
+		log.WithError(err).Errorf("Failed to unmarshal aliases from %q.", path)
 	}
 	log.Infof("Loaded %d aliases from %q.", len(result), path)
 	return result
 }
 
+func (c *Client) parseAliases(result RepoAliases, b []byte, log *logrus.Entry, org string) error {
+	config := &OwnerAliases{}
+	err := yaml.Unmarshal(b, config)
+	if err != nil {
+		return err
+	}
+	if config.ForeignAliases != nil {
+		for _, r := range config.ForeignAliases {
+			if r.Ref == "" {
+				r.Ref = "HEAD"
+			}
+			if r.Org == "" {
+				r.Org = org
+			}
+			b, err = c.spc.GetFile(r.Org, r.Repo, aliasesFileName, r.Ref)
+			if err == nil {
+				err = c.parseAliases(result, b, log, org)
+			}
+		}
+	}
+	if result == nil {
+		result = make(RepoAliases)
+	}
+	for alias, expanded := range config.Aliases {
+		normAlias := scmprovider.NormLogin(alias)
+		normExpanded := normLogins(expanded)
+		if result[normAlias] == nil {
+			result[normAlias] = normExpanded
+		} else {
+			result[normAlias].Insert(normExpanded.UnsortedList()...)
+		}
+	}
+	return err
+}
+
 func loadOwnersFrom(baseDir string, mdYaml bool, aliases RepoAliases, dirExcludes sets.String, log *logrus.Entry) (*RepoOwners, error) {
 	o := &RepoOwners{
 		RepoAliases:  aliases,


Test patch:
diff --git a/pkg/repoowners/repoowners_test.go b/pkg/repoowners/repoowners_test.go
index facabf75..96263a4f 100644
--- a/pkg/repoowners/repoowners_test.go
+++ b/pkg/repoowners/repoowners_test.go
@@ -114,7 +114,8 @@ func getTestClient(
 	extraBranchesAndFiles map[string]map[string][]byte,
 ) (*Client, func(), error) {
 	testAliasesFile := map[string][]byte{
-		"OWNERS_ALIASES": []byte("aliases:\n  Best-approvers:\n  - carl\n  - cjwagner\n  best-reviewers:\n  - Carl\n  - BOB"),
+		"OWNERS_ALIASES": []byte("aliases:\n  Best-approvers:\n  - carl\n  - cjwagner\n  best-reviewers:\n  - Carl\n  - BOB\n" +
+			"foreignAliases:\n- name: another-repo\n"),
 	}
 
 	localGit, git, err := localgit.New()
@@ -149,8 +150,15 @@ func getTestClient(
 	}
 
 	return &Client{
-			git:    git,
-			spc:    &fake.SCMClient{Collaborators: []string{"cjwagner", "k8s-ci-robot", "alice", "bob", "carl", "mml", "maggie"}},
+			git: git,
+			spc: &fake.SCMClient{
+				Collaborators: []string{"cjwagner", "k8s-ci-robot", "alice", "bob", "carl", "mml", "maggie"},
+				RemoteFiles: map[string]map[string]string{
+					"OWNERS_ALIASES": {
+						"HEAD": "aliases:\n  best-approvers:\n  - blahonga\n",
+					},
+				},
+			},
 			logger: logrus.WithField("client", "repoowners"),
 			cache:  make(map[string]cacheEntry),
 
@@ -583,7 +591,7 @@ func TestLoadRepoAliases(t *testing.T) {
 			name:            "Normal aliases file",
 			aliasFileExists: true,
 			expectedRepoAliases: RepoAliases{
-				"best-approvers": sets.NewString("carl", "cjwagner"),
+				"best-approvers": sets.NewString("carl", "cjwagner", "blahonga"),
 				"best-reviewers": sets.NewString("carl", "bob"),
 			},
 		},