Skip to content
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
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
123 changes: 123 additions & 0 deletions dev/codeowners/codeowners.go
Original file line number Diff line number Diff line change
@@ -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
}
105 changes: 105 additions & 0 deletions dev/codeowners/codeowners_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}
1 change: 1 addition & 0 deletions dev/codeowners/noowner/manifest.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
owner: ~
1 change: 1 addition & 0 deletions dev/codeowners/testdata/CODEOWNERS-empty
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# No owners here
4 changes: 4 additions & 0 deletions dev/codeowners/testdata/CODEOWNERS-multiple-owners
Original file line number Diff line number Diff line change
@@ -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
4 changes: 4 additions & 0 deletions dev/codeowners/testdata/CODEOWNERS-no-owner
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# This is invalid, a path has no owner.

/testdata/devexp @elastic/integrations-developer-experience
/testdata/integration
4 changes: 4 additions & 0 deletions dev/codeowners/testdata/CODEOWNERS-precedence
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Here the codeowner is correctly overriden.

/testdata/devexp @jsoriano
/testdata/devexp @elastic/integrations-developer-experience
4 changes: 4 additions & 0 deletions dev/codeowners/testdata/CODEOWNERS-valid
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# This is a valid test file

/testdata/devexp @elastic/integrations-developer-experience
/testdata/integration @elastic/integrations
3 changes: 3 additions & 0 deletions dev/codeowners/testdata/CODEOWNERS-wrong-devexp
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Here the codeowner is not what was expected.

/testdata/devexp @jsoriano
4 changes: 4 additions & 0 deletions dev/codeowners/testdata/CODEOWNERS-wrong-precedence
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Here the codeowner is incorrectly overriden.

/testdata/devexp @elastic/integrations-developer-experience
/testdata/devexp @jsoriano
2 changes: 2 additions & 0 deletions dev/codeowners/testdata/devexp/manifest.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
owner:
github: elastic/integrations-developer-experience
19 changes: 19 additions & 0 deletions magefile.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -27,6 +30,8 @@ func Check() error {
mg.Deps(build)
mg.Deps(format)
mg.Deps(modTidy)
mg.Deps(goTest)
Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't find if go tests were executed in CI. As all the Go code is somehow related to the magefile, I am adding it here, but I guess this could be added somewhere in the Jenkinsfile too.

mg.Deps(codeowners.Check)
return nil
}

Expand Down Expand Up @@ -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 {
Expand Down