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

[chore] enforce validation of codeowners membership #24638

Merged
merged 18 commits into from
Aug 23, 2023
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
10 changes: 5 additions & 5 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ updates:
schedule:
interval: "weekly"
day: "wednesday"
- package-ecosystem: "gomod"
directory: "/cmd/githubgen"
schedule:
interval: "weekly"
day: "wednesday"
- package-ecosystem: "gomod"
directory: "/cmd/mdatagen"
schedule:
Expand Down Expand Up @@ -1097,8 +1102,3 @@ updates:
schedule:
interval: "weekly"
day: "wednesday"
- package-ecosystem: "gomod"
directory: "/receiver/splunkenterprisereceiver"
schedule:
interval: "weekly"
day: "wednesday"
13 changes: 10 additions & 3 deletions .github/workflows/build-and-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -208,10 +208,17 @@ jobs:
run: |
make -j2 generate
git diff --exit-code ':!*go.sum' || (echo 'Generated code is out of date, please run "make generate" and commit the changes in this PR.' && exit 1)
- name: Gen codeowners
- uses: dorny/paths-filter@v2
id: codeowner-changes
with:
filters: |
src:
- '**/metadata.yaml'
- name: Check codeowners
if: steps.codeowner-changes.outputs.src == 'true'
run: |
make gengithub
git diff -s --exit-code || (echo 'Generated code is out of date, please run "make gengithub" and commit the changes in this PR.' && exit 1)
GITHUB_TOKEN=${{ secrets.GITHUB_TOKEN }} make gengithub
git diff -s --exit-code || (echo 'Generated code or members.txt are out of date, please run "make gengithubcheck" and commit the changes in this PR.' && exit 1)
- name: Check gendependabot
run: |
make -j2 gendependabot
Expand Down
3 changes: 2 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,8 @@ mdatagen-test:

.PHONY: gengithub
gengithub:
$(GOCMD) run cmd/githubgen/main.go .
cd cmd/githubgen && $(GOCMD) install .
githubgen

.PHONY: update-codeowners
update-codeowners: gengithub generate
Expand Down
1 change: 1 addition & 0 deletions cmd/githubgen/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
include ../../Makefile.Common
31 changes: 31 additions & 0 deletions cmd/githubgen/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# githubgen

This executable is used to generate the `.github/CODEOWNERS` and `.github/ALLOWLIST` files.

It reads status metadata from `metadata.yaml` files located throughout the repository.

It checks that codeowners are known members of the OpenTelemetry organization.

## Usage

```
$> make gengithub
```
The equivalent of:
```
$> cd cmd/githubgen && $(GOCMD) install .
$> GITHUB_TOKEN=<mypattoken> githubgen --folder . [--allowlist cmd/githubgen/allowlist.txt]
```

## Checking codeowners against OpenTelemetry membership via Github API

To authenticate, set the environment variable `GITHUB_TOKEN` to a PAT token.

For each codeowner, the script will check if the user is registered as a member of the OpenTelemetry organization.

If any codeowner is missing, it will stop and print names of missing codeowners.

These can be added to allowlist.txt as a workaround.

If a codeowner is present in allowlist.txt and also a member of the OpenTelemetry organization, the script will error out.

23 changes: 23 additions & 0 deletions cmd/githubgen/allowlist.txt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are these names for?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

allowlist of github usernames who are not currently code owners and yet allowed to be kept in while we resolve #20868

Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
Caleb-Hurshman
Doron-Bargo
MaxKsyunz
MitchellGale
YANG-DB
agoallikmaa
alexvanboxel
architjugran
asaharn
avadhut123pisal
billmeyer
eedorenko
emreyalvac
keep94
kiranmayib
kkujawa-sumo
leonsp-ai
liqiangz
oded-dd
shaochengwang
svrakitin
thepeterstone
yiyang5055
30 changes: 30 additions & 0 deletions cmd/githubgen/go.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
module github.com/open-telemetry/opentelemetry-collector-contrib/cmd/githubgen

go 1.20

require (
github.com/google/go-github/v53 v53.2.0
go.opentelemetry.io/collector/confmap v0.83.0
)

require (
github.com/ProtonMail/go-crypto v0.0.0-20230217124315-7d5c6f04bbb8 // indirect
github.com/cloudflare/circl v1.3.3 // indirect
github.com/golang/protobuf v1.5.2 // indirect
github.com/google/go-querystring v1.1.0 // indirect
github.com/knadh/koanf/maps v0.1.1 // indirect
github.com/knadh/koanf/providers/confmap v0.1.0 // indirect
github.com/knadh/koanf/v2 v2.0.1 // indirect
github.com/mitchellh/copystructure v1.2.0 // indirect
github.com/mitchellh/mapstructure v1.5.0 // indirect
github.com/mitchellh/reflectwalk v1.0.2 // indirect
go.opentelemetry.io/collector/featuregate v1.0.0-rcv0014 // indirect
go.uber.org/multierr v1.11.0 // indirect
golang.org/x/crypto v0.7.0 // indirect
golang.org/x/net v0.10.0 // indirect
golang.org/x/oauth2 v0.8.0 // indirect
golang.org/x/sys v0.8.0 // indirect
google.golang.org/appengine v1.6.7 // indirect
google.golang.org/protobuf v1.28.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)
73 changes: 73 additions & 0 deletions cmd/githubgen/go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

89 changes: 83 additions & 6 deletions cmd/githubgen/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"sort"
"strings"

"github.com/google/go-github/v53/github"
"go.opentelemetry.io/collector/confmap/provider/fileprovider"
)

Expand Down Expand Up @@ -79,9 +80,10 @@ const unmaintainedStatus = "unmaintained"
// .github/CODEOWNERS
// .github/ALLOWLIST
func main() {
folder := flag.String("folder", ".", "folder investigated for codeowners")
allowlistFilePath := flag.String("allowlist", "cmd/githubgen/allowlist.txt", "path to a file containing an allowlist of members outside the OpenTelemetry organization")
flag.Parse()
folder := flag.Arg(0)
if err := run(folder); err != nil {
if err := run(*folder, *allowlistFilePath); err != nil {
log.Fatal(err)
}
}
Expand Down Expand Up @@ -127,11 +129,27 @@ func loadMetadata(filePath string) (metadata, error) {
return md, nil
}

func run(folder string) error {
func run(folder string, allowlistFilePath string) error {
members, err := getGithubMembers()
if err != nil {
return err
}
allowlistData, err := os.ReadFile(allowlistFilePath)
if err != nil {
return err
}
allowlistLines := strings.Split(string(allowlistData), "\n")

allowlist := make(map[string]struct{}, len(allowlistLines))
for _, line := range allowlistLines {
allowlist[line] = struct{}{}
}

components := map[string]metadata{}
foldersList := []string{}
var foldersList []string
maxLength := 0
err := filepath.Walk(folder, func(path string, info fs.FileInfo, err error) error {
allCodeowners := map[string]struct{}{}
err = filepath.Walk(folder, func(path string, info fs.FileInfo, err error) error {
if info.Name() == "metadata.yaml" {
m, err := loadMetadata(path)
if err != nil {
Expand All @@ -149,16 +167,43 @@ func run(folder string) error {
return nil
}
}
for _, id := range m.Status.Codeowners.Active {
allCodeowners[id] = struct{}{}
}
if len(key) > maxLength {
maxLength = len(key)
}
}
return nil
})
sort.Strings(foldersList)
if err != nil {
return err
}
sort.Strings(foldersList)
var missingCodeowners []string
var duplicateCodeowners []string
for codeowner := range allCodeowners {
_, present := members[codeowner]

if !present {
_, allowed := allowlist[codeowner]
allowed = allowed || strings.HasPrefix(codeowner, "open-telemetry/")
if !allowed {
missingCodeowners = append(missingCodeowners, codeowner)
}
} else if _, ok := allowlist[codeowner]; ok {
duplicateCodeowners = append(duplicateCodeowners, codeowner)
}
}
if len(missingCodeowners) > 0 {
sort.Strings(missingCodeowners)
return fmt.Errorf("codeowners are not members: %s", strings.Join(missingCodeowners, ", "))
}
if len(duplicateCodeowners) > 0 {
sort.Strings(duplicateCodeowners)
return fmt.Errorf("codeowners members duplicate in allowlist: %s", strings.Join(duplicateCodeowners, ", "))
}

codeowners := codeownersHeader
deprecatedList := "## DEPRECATED components\n"
unmaintainedList := "\n## UNMAINTAINED components\n"
Expand Down Expand Up @@ -206,3 +251,35 @@ LOOP:

return nil
}

func getGithubMembers() (map[string]struct{}, error) {
client := github.NewTokenClient(context.Background(), os.Getenv("GITHUB_TOKEN"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth passing these values in?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, otherwise it fails because the github token is not set.

var allUsers []*github.User
pageIndex := 0
for {
users, resp, err := client.Organizations.ListMembers(context.Background(), "open-telemetry",
&github.ListMembersOptions{
PublicOnly: false,
ListOptions: github.ListOptions{
PerPage: 50,
Page: pageIndex,
},
},
)
if err != nil {
return nil, err
}
defer resp.Body.Close()
if len(users) == 0 {
break
}
allUsers = append(allUsers, users...)
pageIndex++
}

usernames := make(map[string]struct{}, len(allUsers))
for _, u := range allUsers {
usernames[*u.Login] = struct{}{}
}
return usernames, nil
}
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@ require (
github.com/open-telemetry/opentelemetry-collector-contrib/receiver/zipkinreceiver v0.83.0
github.com/open-telemetry/opentelemetry-collector-contrib/receiver/zookeeperreceiver v0.83.0
go.opentelemetry.io/collector v0.83.0
go.opentelemetry.io/collector/confmap v0.83.0
atoulme marked this conversation as resolved.
Show resolved Hide resolved
go.opentelemetry.io/collector/exporter v0.83.0
go.opentelemetry.io/collector/exporter/loggingexporter v0.83.0
go.opentelemetry.io/collector/exporter/otlpexporter v0.83.0
Expand Down Expand Up @@ -627,6 +626,7 @@ require (
go.opentelemetry.io/collector/config/configtelemetry v0.83.0 // indirect
go.opentelemetry.io/collector/config/configtls v0.83.0 // indirect
go.opentelemetry.io/collector/config/internal v0.83.0 // indirect
go.opentelemetry.io/collector/confmap v0.83.0 // indirect
go.opentelemetry.io/collector/connector v0.83.0 // indirect
go.opentelemetry.io/collector/consumer v0.83.0 // indirect
go.opentelemetry.io/collector/extension/auth v0.83.0 // indirect
Expand Down
1 change: 1 addition & 0 deletions versions.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ module-sets:
modules:
- github.com/open-telemetry/opentelemetry-collector-contrib
- github.com/open-telemetry/opentelemetry-collector-contrib/cmd/configschema
- github.com/open-telemetry/opentelemetry-collector-contrib/cmd/githubgen
- github.com/open-telemetry/opentelemetry-collector-contrib/cmd/mdatagen
- github.com/open-telemetry/opentelemetry-collector-contrib/cmd/opampsupervisor
- github.com/open-telemetry/opentelemetry-collector-contrib/cmd/telemetrygen
Expand Down