From a16e4b928f1f43eb1fccc567fe0d27b977d8dc9b Mon Sep 17 00:00:00 2001 From: Jeremy Cohen Date: Tue, 12 Apr 2022 09:17:41 -0400 Subject: [PATCH] not-owned-checker: Add git-ls-tree implementation with subdirectory support (#141) --- README.md | 1 + action.yml | 4 ++++ docs/gh-action.md | 3 +++ internal/check/not_owned_file.go | 16 +++++++++++----- tests/integration/integration_test.go | 10 ++++++++++ .../TestCheckFailures/notowned.golden.txt | 3 ++- .../notowned_sub_dirs.golden.txt | 5 +++++ 7 files changed, 36 insertions(+), 6 deletions(-) create mode 100644 tests/integration/testdata/TestCheckFailures/notowned_sub_dirs.golden.txt diff --git a/README.md b/README.md index 0653666..338aea0 100644 --- a/README.md +++ b/README.md @@ -131,6 +131,7 @@ Use the following environment variables to configure the application: | OWNER_CHECKER_ALLOW_UNOWNED_PATTERNS | `true` | Specifies whether CODEOWNERS may have unowned files. For example:

`/infra/oncall-rotator/ @sre-team`
`/infra/oncall-rotator/oncall-config.yml`

The `/infra/oncall-rotator/oncall-config.yml` file is not owned by anyone. | | OWNER_CHEKER_OWNERS_MUST_BE_TEAMS | `false` | Specifies whether only teams are allowed as owners of files. | | NOT_OWNED_CHECKER_SKIP_PATTERNS | - | The comma-separated list of patterns that should be ignored by `not-owned-checker`. For example, you can specify `*` and as a result, the `*` pattern from the **CODEOWNERS** file will be ignored and files owned by this pattern will be reported as unowned unless a later specific pattern will match that path. It's useful because often we have default owners entry at the begging of the CODOEWNERS file, e.g. `* @global-owner1 @global-owner2` | +| NOT_OWNED_CHECKER_SUBDIRECTORIES | - | The comma-separated list of subdirectories to check in `not-owned-checker`. When specified, only files in the listed subdirectories will be checked if they do not have specified owners in CODEOWNERS. | * - Required diff --git a/action.yml b/action.yml index b08de56..6fee558 100644 --- a/action.yml +++ b/action.yml @@ -57,6 +57,10 @@ inputs: default: "false" required: false + not_owned_checker_subdirectories: + description: "Only check listed subdirectories for CODEOWNERS ownership that don't have owners." + required: false + runs: using: 'docker' image: 'docker://ghcr.io/mszostok/codeowners-validator:v0.7.2' diff --git a/docs/gh-action.md b/docs/gh-action.md index 2838f2f..b8a5626 100644 --- a/docs/gh-action.md +++ b/docs/gh-action.md @@ -73,6 +73,9 @@ jobs: # Specifies whether only teams are allowed as owners of files. owner_checker_owners_must_be_teams: "false" + + # Only check listed subdirectories for CODEOWNERS ownership that don't have owners. + not_owned_checker_subdirectories: "" ``` The best is to run this as a cron job and not only if you applying changes to CODEOWNERS file itself, e.g. the CODEOWNERS file can be invalidate when you removing someone from the organization. diff --git a/internal/check/not_owned_file.go b/internal/check/not_owned_file.go index 0f32d5b..101743a 100644 --- a/internal/check/not_owned_file.go +++ b/internal/check/not_owned_file.go @@ -16,11 +16,13 @@ import ( ) type NotOwnedFileConfig struct { - SkipPatterns []string `envconfig:"optional"` + SkipPatterns []string `envconfig:"optional"` + Subdirectories []string `envconfig:"optional"` } type NotOwnedFile struct { - skipPatterns map[string]struct{} + skipPatterns map[string]struct{} + subDirectories []string } func NewNotOwnedFile(cfg NotOwnedFileConfig) *NotOwnedFile { @@ -30,7 +32,8 @@ func NewNotOwnedFile(cfg NotOwnedFileConfig) *NotOwnedFile { } return &NotOwnedFile{ - skipPatterns: skip, + skipPatterns: skip, + subDirectories: cfg.Subdirectories, } } @@ -130,7 +133,7 @@ func (c *NotOwnedFile) GitRemoveIgnoredFiles(repoDir string) error { pipe.ChDir(repoDir), pipe.Line( pipe.Exec("git", "ls-files", "-ci", "--exclude-standard", "-z"), - pipe.Exec("xargs", "-0", "git", "rm", "--cached"), + pipe.Exec("xargs", "-0", "-r", "git", "rm", "--cached"), ), ) @@ -168,9 +171,12 @@ func (c *NotOwnedFile) GitResetCurrentBranch(repoDir string) error { } func (c *NotOwnedFile) GitListFiles(repoDir string) (string, error) { + args := []string{"ls-files"} + args = append(args, c.subDirectories...) + gitls := pipe.Script( pipe.ChDir(repoDir), - pipe.Exec("git", "ls-files"), + pipe.Exec("git", args...), ) stdout, stderr, err := pipe.DividedOutput(gitls) diff --git a/tests/integration/integration_test.go b/tests/integration/integration_test.go index e3e9ae8..3893f7f 100644 --- a/tests/integration/integration_test.go +++ b/tests/integration/integration_test.go @@ -215,6 +215,16 @@ func TestCheckFailures(t *testing.T) { "NOT_OWNED_CHECKER_SKIP_PATTERNS": "*", }, }, + { + name: "notowned_sub_dirs", + envs: Envs{ + "PATH": os.Getenv("PATH"), // need to be set to find the `git` binary + "CHECKS": "disable-all", + "EXPERIMENTAL_CHECKS": "notowned", + "NOT_OWNED_CHECKER_SKIP_PATTERNS": "*", + "NOT_OWNED_CHECKER_SUBDIRECTORIES": "notowned/dir", + }, + }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { diff --git a/tests/integration/testdata/TestCheckFailures/notowned.golden.txt b/tests/integration/testdata/TestCheckFailures/notowned.golden.txt index 87045db..c65b5d1 100644 --- a/tests/integration/testdata/TestCheckFailures/notowned.golden.txt +++ b/tests/integration/testdata/TestCheckFailures/notowned.golden.txt @@ -1,7 +1,8 @@ ==> Executing [Experimental] Not Owned File Checker () - [err] Found 3 not owned files (skipped patterns: "*"): + [err] Found 4 not owned files (skipped patterns: "*"): * .gitignore * CODEOWNERS * action.yml + * notowned/dir/example/sample.txt 1 check(s) executed, 1 failure(s) diff --git a/tests/integration/testdata/TestCheckFailures/notowned_sub_dirs.golden.txt b/tests/integration/testdata/TestCheckFailures/notowned_sub_dirs.golden.txt new file mode 100644 index 0000000..da79988 --- /dev/null +++ b/tests/integration/testdata/TestCheckFailures/notowned_sub_dirs.golden.txt @@ -0,0 +1,5 @@ +==> Executing [Experimental] Not Owned File Checker () + [err] Found 1 not owned files (skipped patterns: "*"): + * notowned/dir/example/sample.txt + +1 check(s) executed, 1 failure(s)