From 55c001130e9b1f5cdcdc0bbe52990f3aee89a25b Mon Sep 17 00:00:00 2001 From: Jaime Soriano Pastor Date: Wed, 20 Apr 2022 12:36:35 +0200 Subject: [PATCH] Exclude ecosystem as owner of subdirectories of packages /packages/* only include the first level of files under /packages. Removing the wildcard matches everything under /packages. --- .github/CODEOWNERS | 2 +- dev/codeowners/codeowners.go | 7 +++++++ dev/codeowners/codeowners_test.go | 4 ++++ dev/codeowners/testdata/CODEOWNERS-invalid-override | 2 +- .../testdata/CODEOWNERS-invalid-override-wildcard | 5 +++++ dev/codeowners/testdata/CODEOWNERS-multiple-owners | 1 + dev/codeowners/testdata/CODEOWNERS-valid | 2 +- 7 files changed, 20 insertions(+), 3 deletions(-) create mode 100644 dev/codeowners/testdata/CODEOWNERS-invalid-override-wildcard diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 3556140f8dc..4abe7d8e9fa 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1,6 +1,6 @@ # Everything outside of packages is maintained by the ecosystem team. * @elastic/ecosystem -/packages/* +/packages/ # CODEOWNERS file is checked by CI. /.github/CODEOWNERS diff --git a/dev/codeowners/codeowners.go b/dev/codeowners/codeowners.go index 8c0d6e11b91..08a95f53672 100644 --- a/dev/codeowners/codeowners.go +++ b/dev/codeowners/codeowners.go @@ -103,6 +103,13 @@ func (codeowners *githubOwners) checkSingleField(field string) error { if matches || strings.HasPrefix(field, path) { return fmt.Errorf("%q would remove owners for %q", field, path) } + + if strings.HasPrefix(path, field) { + _, err := filepath.Rel(field, path) + if err == nil { + return fmt.Errorf("%q would remove owners for %q", field, path) + } + } } // Excluding other files is fine. diff --git a/dev/codeowners/codeowners_test.go b/dev/codeowners/codeowners_test.go index 6a1ec32fb1a..61b48615033 100644 --- a/dev/codeowners/codeowners_test.go +++ b/dev/codeowners/codeowners_test.go @@ -99,6 +99,10 @@ func TestReadGithubOwners(t *testing.T) { codeownersPath: "testdata/CODEOWNERS-invalid-override", valid: false, }, + { + codeownersPath: "testdata/CODEOWNERS-invalid-override-wildcard", + valid: false, + }, } for _, c := range cases { diff --git a/dev/codeowners/testdata/CODEOWNERS-invalid-override b/dev/codeowners/testdata/CODEOWNERS-invalid-override index fce387f1adb..38ebc75e541 100644 --- a/dev/codeowners/testdata/CODEOWNERS-invalid-override +++ b/dev/codeowners/testdata/CODEOWNERS-invalid-override @@ -1,5 +1,5 @@ # This is not valid because there is an override that would remove owners of a directory. /testdata/devexp @elastic/integrations @elastic/integrations-developer-experience -/testdata/* +/testdata/ diff --git a/dev/codeowners/testdata/CODEOWNERS-invalid-override-wildcard b/dev/codeowners/testdata/CODEOWNERS-invalid-override-wildcard new file mode 100644 index 00000000000..fce387f1adb --- /dev/null +++ b/dev/codeowners/testdata/CODEOWNERS-invalid-override-wildcard @@ -0,0 +1,5 @@ +# This is not valid because there is an override that would remove owners of a directory. + +/testdata/devexp @elastic/integrations @elastic/integrations-developer-experience +/testdata/* + diff --git a/dev/codeowners/testdata/CODEOWNERS-multiple-owners b/dev/codeowners/testdata/CODEOWNERS-multiple-owners index 3df2bd60d50..b13c5e1ecae 100644 --- a/dev/codeowners/testdata/CODEOWNERS-multiple-owners +++ b/dev/codeowners/testdata/CODEOWNERS-multiple-owners @@ -1,4 +1,5 @@ # This is a valid test file with multiple owners for a path /testdata/devexp @elastic/integrations @elastic/integrations-developer-experience +/testdata/devexp/manifest.yml @elastic/integrations /testdata/integration @elastic/integrations diff --git a/dev/codeowners/testdata/CODEOWNERS-valid b/dev/codeowners/testdata/CODEOWNERS-valid index ea4fd56d973..c755a2c130d 100644 --- a/dev/codeowners/testdata/CODEOWNERS-valid +++ b/dev/codeowners/testdata/CODEOWNERS-valid @@ -1,7 +1,7 @@ # This is a valid test file * @elastic/ecosystem -/testdata/* +/testdata/ /testdata/devexp @elastic/integrations-developer-experience /testdata/integration @elastic/integrations