Merge pull request #5611 from spiffxp/link-to-owners-in-readme

generator: make subproject owners links useful

5 files changed, 155 insertions, 10 deletions

diff --git a/generator/app.go b/generator/app.go
index 3618f4dc..31a1322a 100644
--- a/generator/app.go
+++ b/generator/app.go
@@ -23,6 +23,7 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
+	"regexp"
 	"sort"
 	"strings"
 	"text/template"
@@ -47,6 +48,9 @@ const (
 	endCustomMarkdown   = "<!-- END CUSTOM CONTENT -->"
 	beginCustomYaml     = "## BEGIN CUSTOM CONTENT"
 	endCustomYaml       = "## END CUSTOM CONTENT"
+
+	regexRawGitHubURL = "https://raw.githubusercontent.com/(?P<org>[^/]+)/(?P<repo>[^/]+)/(?P<branch>[^/]+)/(?P<path>.*)"
+	regexGitHubURL    = "https://github.com/(?P<org>[^/]+)/(?P<repo>[^/]+)/(blob|tree)/(?P<branch>[^/]+)/(?P<path>.*)"
 )
 
 var (
@@ -246,6 +250,8 @@ func (c *Context) Sort() {
 func (c *Context) Validate() []error {
 	errors := []error{}
 	people := make(map[string]Person)
+	reRawGitHubURL := regexp.MustCompile(regexRawGitHubURL)
+	reGitHubURL := regexp.MustCompile(regexGitHubURL)
 	for prefix, groups := range c.PrefixToGroupMap() {
 		for _, group := range groups {
 			expectedDir := group.DirName(prefix)
@@ -294,6 +300,21 @@ func (c *Context) Validate() []error {
 					errors = append(errors, fmt.Errorf("%s: has no subprojects", group.Dir))
 				}
 			}
+			if prefix != "committee" && prefix != "sig" {
+				if len(group.Subprojects) > 0 {
+					errors = append(errors, fmt.Errorf("%s: only sigs and committees can own code / have subprojects, found: %v", group.Dir, group.Subprojects))
+				}
+			}
+			for _, subproject := range group.Subprojects {
+				if len(subproject.Owners) == 0 {
+					errors = append(errors, fmt.Errorf("%s/%s: subproject has no owners", group.Dir, subproject.Name))
+				}
+				for _, ownerURL := range subproject.Owners {
+					if !reRawGitHubURL.MatchString(ownerURL) && !reGitHubURL.MatchString(ownerURL) {
+						errors = append(errors, fmt.Errorf("%s/%s: subproject owners should match regexp %s, found: %s", group.Dir, subproject.Name, regexRawGitHubURL, ownerURL))
+					}
+				}
+			}
 		}
 	}
 	return errors
@@ -353,6 +374,38 @@ func getExistingContent(path string, fileFormat string) (string, error) {
 var funcMap = template.FuncMap{
 	"tzUrlEncode": tzURLEncode,
 	"trimSpace":   strings.TrimSpace,
+	"trimSuffix":  strings.TrimSuffix,
+	"githubURL":   githubURL,
+	"orgRepoPath": orgRepoPath,
+}
+
+// githubURL converts a raw GitHub url (links directly to file contents) into a
+// regular GitHub url (links to Code view for file), otherwise returns url untouched
+func githubURL(url string) string {
+	re := regexp.MustCompile(regexRawGitHubURL)
+	mat := re.FindStringSubmatchIndex(url)
+	if mat == nil {
+		return url
+	}
+	result := re.ExpandString([]byte{}, "https://github.com/${org}/${repo}/blob/${branch}/${path}", url, mat)
+	return string(result)
+}
+
+// orgRepoPath converts either
+//  - a regular GitHub url of form https://github.com/org/repo/blob/branch/path/to/file
+//  - a raw GitHub url of form https://raw.githubusercontent.com/org/repo/branch/path/to/file
+// to a string of form 'org/repo/path/to/file'
+func orgRepoPath(url string) string {
+	for _, regex := range []string{regexRawGitHubURL, regexGitHubURL} {
+		re := regexp.MustCompile(regex)
+		mat := re.FindStringSubmatchIndex(url)
+		if mat == nil {
+			continue
+		}
+		result := re.ExpandString([]byte{}, "${org}/${repo}/${path}", url, mat)
+		return string(result)
+	}
+	return url
 }
 
 // tzUrlEncode returns a url encoded string without the + shortcut. This is
@@ -509,6 +562,7 @@ func main() {
 		log.Fatal(err)
 	}
 
+	fmt.Println("Generating group READMEs")
 	for prefix, groups := range ctx.PrefixToGroupMap() {
 		err = createGroupReadme(groups, prefix)
 		if err != nil {
diff --git a/generator/app_test.go b/generator/app_test.go
index f22d16c0..184c2b54 100644
--- a/generator/app_test.go
+++ b/generator/app_test.go
@@ -155,19 +155,22 @@ func TestGroupDirName(t *testing.T) {
 func TestCreateGroupReadmes(t *testing.T) {
 	baseGeneratorDir = "generated"
 	templateDir = "../../generator"
+	const groupType = "sig"
 
-	groups := []Group{
-		{Name: "Foo"},
-		{Name: "Bar"},
+	groups := []Group{}
+	for _, n := range []string{"Foo", "Bar"} {
+		g := Group{Name: n}
+		g.Dir = g.DirName(groupType)
+		groups = append(groups, g)
 	}
 
-	err := createGroupReadme(groups, "sig")
+	err := createGroupReadme(groups, groupType)
 	if err != nil {
 		t.Fatal(err)
 	}
 
 	for _, group := range groups {
-		path := filepath.Join(baseGeneratorDir, group.DirName("sig"), "README.md")
+		path := filepath.Join(baseGeneratorDir, group.DirName(groupType), "README.md")
 		if !pathExists(path) {
 			t.Fatalf("%s should exist", path)
 		}
@@ -233,3 +236,73 @@ func TestFullGeneration(t *testing.T) {
 		}
 	}
 }
+
+func TestGitHubURL(t *testing.T) {
+	cases := []struct {
+		name     string
+		url      string
+		expected string
+	}{
+		{
+			name:     "kubernetes-sigs root raw github url",
+			url:      "https://raw.githubusercontent.com/kubernetes-sigs/boskos/main/OWNERS",
+			expected: "https://github.com/kubernetes-sigs/boskos/blob/main/OWNERS",
+		},
+		{
+			name:     "kubernetes non-root raw github url",
+			url:      "https://raw.githubusercontent.com/kubernetes/kubernetes/main/test/OWNERS",
+			expected: "https://github.com/kubernetes/kubernetes/blob/main/test/OWNERS",
+		},
+		{
+			name:     "kubernetes github url should be unchanged",
+			url:      "https://github.com/kubernetes/kubernetes/blob/main/test/OWNERS",
+			expected: "https://github.com/kubernetes/kubernetes/blob/main/test/OWNERS",
+		},
+		{
+			name:     "non-github url should be unchanged",
+			url:      "https://viewsource.com/github/kubernetes/community/generator/app.go",
+			expected: "https://viewsource.com/github/kubernetes/community/generator/app.go",
+		},
+	}
+	for _, c := range cases {
+		actual := githubURL(c.url)
+		if actual != c.expected {
+			t.Errorf("FAIL %s: got: '%s' but expected: '%s'", c.name, actual, c.expected)
+		}
+	}
+}
+
+func TestOrgRepoPath(t *testing.T) {
+	cases := []struct {
+		name     string
+		url      string
+		expected string
+	}{
+		{
+			name:     "kubernetes-sigs root raw github url",
+			url:      "https://raw.githubusercontent.com/kubernetes-sigs/boskos/main/OWNERS",
+			expected: "kubernetes-sigs/boskos/OWNERS",
+		},
+		{
+			name:     "kubernetes non-root raw github url",
+			url:      "https://raw.githubusercontent.com/kubernetes/kubernetes/main/test/OWNERS",
+			expected: "kubernetes/kubernetes/test/OWNERS",
+		},
+		{
+			name:     "kubernetes github url",
+			url:      "https://github.com/kubernetes/kubernetes/blob/main/test/OWNERS",
+			expected: "kubernetes/kubernetes/test/OWNERS",
+		},
+		{
+			name:     "non-github url should be unchanged",
+			url:      "https://viewsource.com/github/kubernetes/community/generator/app.go",
+			expected: "https://viewsource.com/github/kubernetes/community/generator/app.go",
+		},
+	}
+	for _, c := range cases {
+		actual := orgRepoPath(c.url)
+		if actual != c.expected {
+			t.Errorf("FAIL %s: got: '%s' but expected: '%s'", c.name, actual, c.expected)
+		}
+	}
+}
diff --git a/generator/committee_readme.tmpl b/generator/committee_readme.tmpl
index 23eb0137..e72836b4 100644
--- a/generator/committee_readme.tmpl
+++ b/generator/committee_readme.tmpl
@@ -66,7 +66,7 @@ The following [subprojects][subproject-definition] are owned by the {{.Name}} Co
 {{- end }}
 - **Owners:**
 {{- range .Owners }}
-  - {{.}}
+  - [{{trimSuffix (orgRepoPath .) "/OWNERS"}}]({{githubURL .}})
 {{- end }}
 {{- if .Contact }}
 - **Contact:**
diff --git a/generator/sig_readme.tmpl b/generator/sig_readme.tmpl
index 58c1b7b3..62dcc9f1 100644
--- a/generator/sig_readme.tmpl
+++ b/generator/sig_readme.tmpl
@@ -76,7 +76,7 @@ The following [subprojects][subproject-definition] are owned by sig-{{.Label}}:
 {{- end }}
 - **Owners:**
 {{- range .Owners }}
-  - {{.}}
+  - [{{trimSuffix (orgRepoPath .) "/OWNERS"}}]({{githubURL .}})
 {{- end }}
 {{- if .Contact }}
 - **Contact:**
diff --git a/generator/testdata/sigs.yaml b/generator/testdata/sigs.yaml
index a4fe858c..191982fe 100644
--- a/generator/testdata/sigs.yaml
+++ b/generator/testdata/sigs.yaml
@@ -1,5 +1,23 @@
 sigs:
-  - name: Foo
-  - name: Bar
+  - dir: sig-foo
+    name: Foo
+    label: foo
+    charter_link: foo-charter
+    mission_statement: covers foo
+    subprojects:
+    - name: sub-foo
+      owners:
+      - "https://raw.githubusercontent.com/org/foo/main/OWNERS"
+  - dir: sig-bar
+    name: Bar
+    label: bar
+    charter_link: charter-bar
+    mission_statement: owns areas related to bar
+    subprojects:
+    - name: sub-bar
+      owners:
+      - "https://raw.githubusercontent.com/org/bar/main/test/OWNERS"
 workinggroups:
-  - name: Baz
+  - dir: wg-baz
+    name: Baz
+    label: baz