From 6687a0892a7a2bf94bf2c1f087ba23ccd750b244 Mon Sep 17 00:00:00 2001 From: Aaron Crickenberger Date: Sun, 7 Mar 2021 23:48:24 -0500 Subject: generator: "make test" passes again there's a lot of boilerplate that is now expected in sigs.yaml --- generator/app_test.go | 13 ++++++++----- generator/testdata/sigs.yaml | 20 +++++++++++++++++--- 2 files changed, 25 insertions(+), 8 deletions(-) (limited to 'generator') diff --git a/generator/app_test.go b/generator/app_test.go index f22d16c0..381acc46 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) } diff --git a/generator/testdata/sigs.yaml b/generator/testdata/sigs.yaml index a4fe858c..60474dda 100644 --- a/generator/testdata/sigs.yaml +++ b/generator/testdata/sigs.yaml @@ -1,5 +1,19 @@ sigs: - - name: Foo - - name: Bar + - dir: sig-foo + name: Foo + label: foo + charter_link: foo-charter + mission_statement: covers foo + subprojects: + - name: sub-foo + - dir: sig-bar + name: Bar + label: bar + charter_link: charter-bar + mission_statement: owns areas related to bar + subprojects: + - name: sub-bar workinggroups: - - name: Baz + - dir: wg-baz + name: Baz + label: baz -- cgit v1.2.3 From 320c5f10d6495cbdeb8b87ab9e8acd1b140db226 Mon Sep 17 00:00:00 2001 From: Aaron Crickenberger Date: Mon, 8 Mar 2021 00:19:15 -0500 Subject: generator: add validation of subprojects specifically: - only sigs+committees can have subprojects - subprojects need at least one owners entry - owners should be raw github links I think the last one could/should change, but: - I'm about to auto-generate links assuming this format, so I want to verify I can assume all owners are in this format - I am not quite sure if we should treat sigs.yaml as public api and give notice/deprecation for field changes --- generator/app.go | 16 ++++++++++++++++ generator/testdata/sigs.yaml | 4 ++++ 2 files changed, 20 insertions(+) (limited to 'generator') diff --git a/generator/app.go b/generator/app.go index 3618f4dc..e48a123b 100644 --- a/generator/app.go +++ b/generator/app.go @@ -246,6 +246,7 @@ func (c *Context) Sort() { func (c *Context) Validate() []error { errors := []error{} people := make(map[string]Person) + rawGitHubURL := regexp.MustCompile(regexRawGitHubURL) for prefix, groups := range c.PrefixToGroupMap() { for _, group := range groups { expectedDir := group.DirName(prefix) @@ -294,6 +295,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 !rawGitHubURL.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 diff --git a/generator/testdata/sigs.yaml b/generator/testdata/sigs.yaml index 60474dda..191982fe 100644 --- a/generator/testdata/sigs.yaml +++ b/generator/testdata/sigs.yaml @@ -6,6 +6,8 @@ sigs: mission_statement: covers foo subprojects: - name: sub-foo + owners: + - "https://raw.githubusercontent.com/org/foo/main/OWNERS" - dir: sig-bar name: Bar label: bar @@ -13,6 +15,8 @@ sigs: mission_statement: owns areas related to bar subprojects: - name: sub-bar + owners: + - "https://raw.githubusercontent.com/org/bar/main/test/OWNERS" workinggroups: - dir: wg-baz name: Baz -- cgit v1.2.3 From 7a3a9e4e5705517f272c613b6b5fa22f34dbd43e Mon Sep 17 00:00:00 2001 From: Aaron Crickenberger Date: Sun, 7 Mar 2021 23:49:54 -0500 Subject: generator: add githubURL, orgRepoPath funcs --- generator/app.go | 36 ++++++++++++++++++++++++++ generator/app_test.go | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+) (limited to 'generator') diff --git a/generator/app.go b/generator/app.go index e48a123b..a73dc7d2 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 = "" beginCustomYaml = "## BEGIN CUSTOM CONTENT" endCustomYaml = "## END CUSTOM CONTENT" + + regexRawGitHubURL = "https://raw.githubusercontent.com/(?P[^/]+)/(?P[^/]+)/(?P[^/]+)/(?P.*)" + regexGitHubURL = "https://github.com/(?P[^/]+)/(?P[^/]+)/(blob|tree)/(?P[^/]+)/(?P.*)" ) var ( @@ -369,6 +373,37 @@ func getExistingContent(path string, fileFormat string) (string, error) { var funcMap = template.FuncMap{ "tzUrlEncode": tzURLEncode, "trimSpace": strings.TrimSpace, + "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 @@ -525,6 +560,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 381acc46..184c2b54 100644 --- a/generator/app_test.go +++ b/generator/app_test.go @@ -236,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) + } + } +} -- cgit v1.2.3 From 34684158e963b01c8aca5728d0e33549fc926586 Mon Sep 17 00:00:00 2001 From: Aaron Crickenberger Date: Mon, 8 Mar 2021 00:50:46 -0500 Subject: generator: allow github and raw urls in sigs.yaml --- generator/app.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'generator') diff --git a/generator/app.go b/generator/app.go index a73dc7d2..c9e3f85b 100644 --- a/generator/app.go +++ b/generator/app.go @@ -250,7 +250,8 @@ func (c *Context) Sort() { func (c *Context) Validate() []error { errors := []error{} people := make(map[string]Person) - rawGitHubURL := regexp.MustCompile(regexRawGitHubURL) + reRawGitHubURL := regexp.MustCompile(regexRawGitHubURL) + reGitHubURL := regexp.MustCompile(regexGitHubURL) for prefix, groups := range c.PrefixToGroupMap() { for _, group := range groups { expectedDir := group.DirName(prefix) @@ -309,7 +310,7 @@ func (c *Context) Validate() []error { errors = append(errors, fmt.Errorf("%s/%s: subproject has no owners", group.Dir, subproject.Name)) } for _, ownerURL := range subproject.Owners { - if !rawGitHubURL.MatchString(ownerURL) { + 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)) } } -- cgit v1.2.3 From 6153db5bf650f8f10bc52543433866129d1d4991 Mon Sep 17 00:00:00 2001 From: Aaron Crickenberger Date: Mon, 8 Mar 2021 00:41:42 -0500 Subject: generator: make subproject owners links useful reduce the visual noise, and make the links go to githubs UI so: https://raw.githubusercontent.com/org/repo/branch/path/to/OWNERS becomes: - link text: org/repo/path/to - link target: https://github.com/org/repo/blob/branch/path/to/ORWNERS note: this does assume/imply we don't need to model the complexity of subprojects owning different branches... but, we haven't found a reason to do so in 5 years, seems a safe assumption --- generator/app.go | 1 + generator/committee_readme.tmpl | 2 +- generator/sig_readme.tmpl | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) (limited to 'generator') diff --git a/generator/app.go b/generator/app.go index c9e3f85b..31a1322a 100644 --- a/generator/app.go +++ b/generator/app.go @@ -374,6 +374,7 @@ func getExistingContent(path string, fileFormat string) (string, error) { var funcMap = template.FuncMap{ "tzUrlEncode": tzURLEncode, "trimSpace": strings.TrimSpace, + "trimSuffix": strings.TrimSuffix, "githubURL": githubURL, "orgRepoPath": orgRepoPath, } 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:** -- cgit v1.2.3