diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 404b35c0192..7a78dd764fc 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -47,6 +47,7 @@ /packages/fleet_server @elastic/elastic-agent-control-plane /packages/fortinet @elastic/security-external-integrations /packages/gcp @elastic/security-external-integrations @elastic/obs-cloud-monitoring +/packages/gcp_pubsub @elastic/security-external-integrations /packages/github @elastic/security-external-integrations /packages/google_workspace @elastic/security-external-integrations /packages/haproxy @elastic/integrations diff --git a/dev/codeowners/codeowners.go b/dev/codeowners/codeowners.go new file mode 100644 index 00000000000..f438e642ef5 --- /dev/null +++ b/dev/codeowners/codeowners.go @@ -0,0 +1,123 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package codeowners + +import ( + "bufio" + "io/fs" + "io/ioutil" + "os" + "path/filepath" + "strings" + + "github.com/pkg/errors" + "gopkg.in/yaml.v2" +) + +const ( + codeownersPath = ".github/CODEOWNERS" +) + +func Check() error { + codeowners, err := readGithubOwners(codeownersPath) + if err != nil { + return err + } + + const packagesDir = "packages" + return filepath.WalkDir(packagesDir, func(path string, d fs.DirEntry, err error) error { + if d.IsDir() { + if path != packagesDir && filepath.Dir(path) != packagesDir { + return fs.SkipDir + } + return nil + } + if d.Name() != "manifest.yml" { + return nil + } + + return codeowners.checkManifest(path) + }) +} + +type githubOwners struct { + owners map[string][]string + path string +} + +func readGithubOwners(codeownersPath string) (*githubOwners, error) { + f, err := os.Open(codeownersPath) + if err != nil { + return nil, errors.Wrapf(err, "failed to open %q", codeownersPath) + } + defer f.Close() + + codeowners := githubOwners{ + owners: make(map[string][]string), + path: codeownersPath, + } + + scanner := bufio.NewScanner(f) + lineNumber := 0 + for scanner.Scan() { + lineNumber++ + line := strings.TrimSpace(scanner.Text()) + if len(line) == 0 || strings.HasPrefix(line, "#") { + continue + } + fields := strings.Fields(line) + if len(fields) < 2 { + return nil, errors.Errorf("invalid line %d in %q: %q", lineNumber, codeownersPath, line) + } + path, owners := fields[0], fields[1:] + + // It is ok to overwrite because latter lines have precedence in these files. + codeowners.owners[path] = owners + } + if err := scanner.Err(); err != nil { + return nil, errors.Wrapf(err, "scanner error") + } + + return &codeowners, nil +} + +func (codeowners *githubOwners) checkManifest(path string) error { + pkgDir := filepath.Dir(path) + owners, found := codeowners.owners["/"+pkgDir] + if !found { + return errors.Errorf("there is no owner for %q in %q", pkgDir, codeowners.path) + } + + content, err := ioutil.ReadFile(path) + if err != nil { + return err + } + + var manifest struct { + Owner struct { + Github string `yaml:"github"` + } `yaml:"owner"` + } + err = yaml.Unmarshal(content, &manifest) + if err != nil { + return err + } + + if manifest.Owner.Github == "" { + return errors.Errorf("no owner specified in %q", path) + } + + found = false + for _, owner := range owners { + if owner == "@"+manifest.Owner.Github { + found = true + break + } + } + if !found { + return errors.Errorf("owner %q defined in %q is not in %q", manifest.Owner.Github, path, codeowners.path) + } + return nil +} diff --git a/dev/codeowners/codeowners_test.go b/dev/codeowners/codeowners_test.go new file mode 100644 index 00000000000..9d4cbcff99c --- /dev/null +++ b/dev/codeowners/codeowners_test.go @@ -0,0 +1,105 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package codeowners + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCheckManifest(t *testing.T) { + cases := []struct { + codeownersPath string + manifestPath string + valid bool + }{ + { + codeownersPath: "testdata/CODEOWNERS-valid", + manifestPath: "testdata/devexp/manifest.yml", + valid: true, + }, + { + codeownersPath: "testdata/CODEOWNERS-valid", + manifestPath: "testdata/noowner/manifest.yml", + valid: false, + }, + { + codeownersPath: "testdata/CODEOWNERS-multiple-owners", + manifestPath: "testdata/devexp/manifest.yml", + valid: true, + }, + { + codeownersPath: "testdata/CODEOWNERS-empty", + manifestPath: "testdata/devexp/manifest.yml", + valid: false, + }, + { + codeownersPath: "testdata/CODEOWNERS-wrong-devexp", + manifestPath: "testdata/devexp/manifest.yml", + valid: false, + }, + { + codeownersPath: "testdata/CODEOWNERS-precedence", + manifestPath: "testdata/devexp/manifest.yml", + valid: true, + }, + { + codeownersPath: "testdata/CODEOWNERS-wrong-precedence", + manifestPath: "testdata/devexp/manifest.yml", + valid: false, + }, + } + + for _, c := range cases { + t.Run(c.codeownersPath+"_"+c.manifestPath, func(t *testing.T) { + owners, err := readGithubOwners(c.codeownersPath) + require.NoError(t, err) + + err = owners.checkManifest(c.manifestPath) + if c.valid { + assert.NoError(t, err) + } else { + assert.Error(t, err) + } + }) + } +} + +func TestReadGithubOwners(t *testing.T) { + cases := []struct { + codeownersPath string + valid bool + }{ + { + codeownersPath: "testdata/CODEOWNERS-valid", + valid: true, + }, + { + codeownersPath: "notexsists", + valid: false, + }, + { + codeownersPath: "testdata/CODEOWNERS-no-owner", + valid: false, + }, + { + codeownersPath: "testdata/CODEOWNERS-multiple-owners", + valid: true, + }, + } + + for _, c := range cases { + t.Run(c.codeownersPath, func(t *testing.T) { + _, err := readGithubOwners(c.codeownersPath) + if c.valid { + assert.NoError(t, err) + } else { + assert.Error(t, err) + } + }) + } +} diff --git a/dev/codeowners/noowner/manifest.yml b/dev/codeowners/noowner/manifest.yml new file mode 100644 index 00000000000..68e7e18c4cc --- /dev/null +++ b/dev/codeowners/noowner/manifest.yml @@ -0,0 +1 @@ +owner: ~ diff --git a/dev/codeowners/testdata/CODEOWNERS-empty b/dev/codeowners/testdata/CODEOWNERS-empty new file mode 100644 index 00000000000..fc7cc61296b --- /dev/null +++ b/dev/codeowners/testdata/CODEOWNERS-empty @@ -0,0 +1 @@ +# No owners here diff --git a/dev/codeowners/testdata/CODEOWNERS-multiple-owners b/dev/codeowners/testdata/CODEOWNERS-multiple-owners new file mode 100644 index 00000000000..3df2bd60d50 --- /dev/null +++ b/dev/codeowners/testdata/CODEOWNERS-multiple-owners @@ -0,0 +1,4 @@ +# This is a valid test file with multiple owners for a path + +/testdata/devexp @elastic/integrations @elastic/integrations-developer-experience +/testdata/integration @elastic/integrations diff --git a/dev/codeowners/testdata/CODEOWNERS-no-owner b/dev/codeowners/testdata/CODEOWNERS-no-owner new file mode 100644 index 00000000000..b5b5dfe7c44 --- /dev/null +++ b/dev/codeowners/testdata/CODEOWNERS-no-owner @@ -0,0 +1,4 @@ +# This is invalid, a path has no owner. + +/testdata/devexp @elastic/integrations-developer-experience +/testdata/integration diff --git a/dev/codeowners/testdata/CODEOWNERS-precedence b/dev/codeowners/testdata/CODEOWNERS-precedence new file mode 100644 index 00000000000..9bb5c85fd91 --- /dev/null +++ b/dev/codeowners/testdata/CODEOWNERS-precedence @@ -0,0 +1,4 @@ +# Here the codeowner is correctly overriden. + +/testdata/devexp @jsoriano +/testdata/devexp @elastic/integrations-developer-experience diff --git a/dev/codeowners/testdata/CODEOWNERS-valid b/dev/codeowners/testdata/CODEOWNERS-valid new file mode 100644 index 00000000000..3b89c0edafe --- /dev/null +++ b/dev/codeowners/testdata/CODEOWNERS-valid @@ -0,0 +1,4 @@ +# This is a valid test file + +/testdata/devexp @elastic/integrations-developer-experience +/testdata/integration @elastic/integrations diff --git a/dev/codeowners/testdata/CODEOWNERS-wrong-devexp b/dev/codeowners/testdata/CODEOWNERS-wrong-devexp new file mode 100644 index 00000000000..17385c32e7e --- /dev/null +++ b/dev/codeowners/testdata/CODEOWNERS-wrong-devexp @@ -0,0 +1,3 @@ +# Here the codeowner is not what was expected. + +/testdata/devexp @jsoriano diff --git a/dev/codeowners/testdata/CODEOWNERS-wrong-precedence b/dev/codeowners/testdata/CODEOWNERS-wrong-precedence new file mode 100644 index 00000000000..9d56bca9713 --- /dev/null +++ b/dev/codeowners/testdata/CODEOWNERS-wrong-precedence @@ -0,0 +1,4 @@ +# Here the codeowner is incorrectly overriden. + +/testdata/devexp @elastic/integrations-developer-experience +/testdata/devexp @jsoriano diff --git a/dev/codeowners/testdata/devexp/manifest.yml b/dev/codeowners/testdata/devexp/manifest.yml new file mode 100644 index 00000000000..6e10c80be6e --- /dev/null +++ b/dev/codeowners/testdata/devexp/manifest.yml @@ -0,0 +1,2 @@ +owner: + github: elastic/integrations-developer-experience diff --git a/magefile.go b/magefile.go index 58f2cbbca86..719f5ecf030 100644 --- a/magefile.go +++ b/magefile.go @@ -7,12 +7,15 @@ package main import ( + "io" "os" "path/filepath" "github.com/magefile/mage/mg" "github.com/magefile/mage/sh" "github.com/pkg/errors" + + "github.com/elastic/integrations/dev/codeowners" ) var ( @@ -27,6 +30,8 @@ func Check() error { mg.Deps(build) mg.Deps(format) mg.Deps(modTidy) + mg.Deps(goTest) + mg.Deps(codeowners.Check) return nil } @@ -86,6 +91,20 @@ func goImports() error { return sh.RunV("goimports", args...) } +func goTest() error { + args := []string{"test"} + stdout := io.Discard + stderr := io.Discard + if mg.Verbose() { + args = append(args, "-v") + stdout = os.Stdout + stderr = os.Stderr + } + args = append(args, "./dev/...") + _, err := sh.Exec(nil, stdout, stderr, "go", args...) + return err +} + func findFilesRecursive(match func(path string, info os.FileInfo) bool) ([]string, error) { var matches []string err := filepath.Walk(".", func(path string, info os.FileInfo, err error) error {