Allow unowned patterns by default with an option to change it (#113)

README.md

@@ -109,32 +109,33 @@ The experimental checks are disabled by default:
 |----------|---------------------------------------------------------------------------------------------------------------------------------------------|
 | notowned | **[Not Owned File Checker]** <br /><br /> Reports if a given repository contain files that do not have specified owners in CODEOWNERS file. |
 
-To enable experimental check set `EXPERIMENTAL_CHECKS=notowned` environment variable. 
+To enable experimental check set `EXPERIMENTAL_CHECKS=notowned` environment variable.
 
 Check the [Configuration](#configuration) section for more info on how to enable and configure given checks.
 
 ## Configuration
 
 Use the following environment variables to configure the application:
 
-| Name | Default | Description |
-|-----|:--------|:------------|
-| <tt>REPOSITORY_PATH</tt> <b>*</b> | | Path to your repository on your local machine. |
-| <tt>GITHUB_ACCESS_TOKEN</tt>| | GitHub access token. Instruction for creating a token can be found [here](./docs/gh-token.md). If not provided, the owners validating functionality may not work properly. For example, you may reach the API calls quota or, if you are setting GitHub Enterprise base URL, an unauthorized error may occur. |
-| <tt>GITHUB_BASE_URL</tt>| https://api.github.com/ | GitHub base URL for API requests. Defaults to the public GitHub API but can be set to a domain endpoint to use with GitHub Enterprise. |
-| <tt>GITHUB_UPLOAD_URL</tt> | https://uploads.github.com/ | GitHub upload URL for uploading files. <br> <br>It is taken into account only when `GITHUB_BASE_URL` is also set. If only `GITHUB_BASE_URL` is provided, this parameter defaults to the `GITHUB_BASE_URL` value. |
-| <tt>CHECKS</tt>| - |  List of checks to be executed. By default, all checks are executed. Possible values: `files`,`owners`,`duppatterns`,`syntax`. |
-| <tt>EXPERIMENTAL_CHECKS</tt> | - | The comma-separated list of experimental checks that should be executed. By default, all experimental checks are turned off. Possible values: `notowned`.|
-| <tt>CHECK_FAILURE_LEVEL</tt> | `warning` | Defines the level on which the application should treat check issues as failures. Defaults to `warning`, which treats both errors and warnings as failures, and exits with error code 3. Possible values are `error` and `warning`. |
-| <tt>OWNER_CHECKER_REPOSITORY</tt>  <b>*</b>| | The owner and repository name separated by slash. For example, gh-codeowners/codeowners-samples. Used to check if GitHub owner is in the given organization. |
-| <tt>OWNER_CHECKER_IGNORED_OWNERS</tt> | `@ghost`| The comma-separated list of owners that should not be validated. Example: `"@owner1,@owner2,@org/team1,example@email.com"`. |
-| <tt>NOT_OWNED_CHECKER_SKIP_PATTERNS</tt>| - | 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` |
+| Name                                           | Default | Description                                                                                                                                                                                                                                                                                                                                                                                                                                                     |
+|------------------------------------------------|:--------|:----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| <tt>REPOSITORY_PATH</tt> <b>*</b>              | | Path to your repository on your local machine.                                                                                                                                                                                                                                                                                                                                                                                                                  |
+| <tt>GITHUB_ACCESS_TOKEN</tt>                   | | GitHub access token. Instruction for creating a token can be found [here](./docs/gh-token.md). If not provided, the owners validating functionality may not work properly. For example, you may reach the API calls quota or, if you are setting GitHub Enterprise base URL, an unauthorized error may occur.                                                                                                                                                   |
+| <tt>GITHUB_BASE_URL</tt>                       | `https://api.github.com/` | GitHub base URL for API requests. Defaults to the public GitHub API but can be set to a domain endpoint to use with GitHub Enterprise.                                                                                                                                                                                                                                                                                                                          |
+| <tt>GITHUB_UPLOAD_URL</tt>                     | `https://uploads.github.com/` | GitHub upload URL for uploading files. <br> <br>It is taken into account only when `GITHUB_BASE_URL` is also set. If only `GITHUB_BASE_URL` is provided, this parameter defaults to the `GITHUB_BASE_URL` value.                                                                                                                                                                                                                                                |
+| <tt>CHECKS</tt>                                | - | List of checks to be executed. By default, all checks are executed. Possible values: `files`,`owners`,`duppatterns`,`syntax`.                                                                                                                                                                                                                                                                                                                                   |
+| <tt>EXPERIMENTAL_CHECKS</tt>                   | - | The comma-separated list of experimental checks that should be executed. By default, all experimental checks are turned off. Possible values: `notowned`.                                                                                                                                                                                                                                                                                                       |
+| <tt>CHECK_FAILURE_LEVEL</tt>                   | `warning` | Defines the level on which the application should treat check issues as failures. Defaults to `warning`, which treats both errors and warnings as failures, and exits with error code 3. Possible values are `error` and `warning`.                                                                                                                                                                                                                             |
+| <tt>OWNER_CHECKER_REPOSITORY</tt>  <b>*</b>    | | The owner and repository name separated by slash. For example, gh-codeowners/codeowners-samples. Used to check if GitHub owner is in the given organization.                                                                                                                                                                                                                                                                                                    |
+| <tt>OWNER_CHECKER_IGNORED_OWNERS</tt>          | `@ghost`| The comma-separated list of owners that should not be validated. Example: `"@owner1,@owner2,@org/team1,example@email.com"`.                                                                                                                                                                                                                                                                                                                                     |
+| <tt>OWNER_CHECKER_ALLOW_UNOWNED_PATTERNS</tt> | `true` | 	Specifies whether CODEOWNERS may have unowned files. For example: <br> <br>  `/infra/oncall-rotator/                    @sre-team` <br>  `/infra/oncall-rotator/oncall-config.yml` <br> <br>  The `/infra/oncall-rotator/oncall-config.yml` file is not owned by anyone.                                                                                                                                                                                           |
+| <tt>NOT_OWNED_CHECKER_SKIP_PATTERNS</tt>       | - | 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` |
 
  <b>*</b> - Required
 
 #### Exit status codes
 
-Application exits with different status codes which allow you to easily distinguish between error categories.  
+Application exits with different status codes which allow you to easily distinguish between error categories.
 
 | Code | Description |
 |:-----:|:------------|
@@ -144,7 +145,7 @@ Application exits with different status codes which allow you to easily distingu
 
 ## Contributing
 
-Contributions are greatly appreciated! The project follows the typical GitHub pull request model. See [CONTRIBUTING.md](CONTRIBUTING.md) for more details.  
+Contributions are greatly appreciated! The project follows the typical GitHub pull request model. See [CONTRIBUTING.md](CONTRIBUTING.md) for more details.
 
 ## Roadmap
 

action.yml

@@ -47,6 +47,11 @@ inputs:
     description:  "The comma-separated list of owners that should not be validated. Example: @owner1,@owner2,@org/team1,example@email.com."
     required: false
 
+  owner_checker_allow_unowned_patterns:
+    description:  "Specifies whether CODEOWNERS may have unowned files. For example, `/infra/oncall-rotator/oncall-config.yml` doesn't have owner and this is not reported."
+    default: "true"
+    required: false
+
 runs:
   using: 'docker'
   image: 'docker://mszostok/codeowners-validator:v0.6.0'

internal/check/duplicated_pattern.go

@@ -36,7 +36,7 @@ func (d *DuplicatedPattern) Check(ctx context.Context, in Input) (Output, error)
 
 	for name, entries := range patterns {
 		if len(entries) > 1 {
-			msg := fmt.Sprintf("Pattern %q is defined %d times in lines: \n%s", name, len(entries), d.listFormatFunc(entries))
+			msg := fmt.Sprintf("Pattern %q is defined %d times in lines:\n%s", name, len(entries), d.listFormatFunc(entries))
 			bldr.ReportIssue(msg)
 		}
 	}

internal/check/duplicated_pattern_test.go

@@ -18,7 +18,7 @@ func TestDuplicatedPattern(t *testing.T) {
 		"Should report info about duplicated entries": {
 			codeownersInput: `
 					*       @global-owner1 @global-owner2
-					
+
 					/build/logs/ @doctocat
 					/build/logs/ @doctocat
 
@@ -29,21 +29,21 @@ func TestDuplicatedPattern(t *testing.T) {
 				{
 					Severity: check.Error,
 					LineNo:   nil,
-					Message: `Pattern "/build/logs/" is defined 2 times in lines: 
+					Message: `Pattern "/build/logs/" is defined 2 times in lines:
             * 4: with owners: [@doctocat]
             * 5: with owners: [@doctocat]`,
 				},
 				{
 					Severity: check.Error,
 					LineNo:   nil,
-					Message: `Pattern "/script" is defined 2 times in lines: 
+					Message: `Pattern "/script" is defined 2 times in lines:
             * 7: with owners: [@mszostok]
             * 8: with owners: [m.t@g.com]`,
 				},
 			},
 		},
 		"Should not report any issues with correct CODEOWNERS file": {
-			codeownersInput: check.FixtureValidCODEOWNERS,
+			codeownersInput: FixtureValidCODEOWNERS,
 			expectedIssues:  nil,
 		},
 	}
@@ -54,7 +54,7 @@ func TestDuplicatedPattern(t *testing.T) {
 			sut := check.NewDuplicatedPattern()
 
 			// when
-			out, err := sut.Check(context.TODO(), check.LoadInput(tc.codeownersInput))
+			out, err := sut.Check(context.TODO(), LoadInput(tc.codeownersInput))
 
 			// then
 			require.NoError(t, err)

internal/check/file_exists_test.go

@@ -169,7 +169,7 @@ func TestFileExists(t *testing.T) {
 			defer cancel()
 
 			// when
-			in := check.LoadInput(tc.codeownersInput)
+			in := LoadInput(tc.codeownersInput)
 			in.RepoDir = tmp
 			out, err := fchecker.Check(ctx, in)
 
@@ -191,7 +191,7 @@ func TestFileExistCheckFileSystemFailure(t *testing.T) {
 	err = os.MkdirAll(filepath.Join(tmpdir, "foo"), 0222)
 	require.NoError(t, err)
 
-	in := check.LoadInput("* @pico")
+	in := LoadInput("* @pico")
 	in.RepoDir = tmpdir
 
 	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Millisecond)

internal/check/helpers_test.go

@@ -1,7 +1,12 @@
-package check
+package check_test
 
 import (
 	"strings"
+	"testing"
+
+	"github.com/mszostok/codeowners-validator/internal/check"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/mszostok/codeowners-validator/pkg/codeowners"
 )
@@ -20,10 +25,21 @@ var FixtureValidCODEOWNERS = `
 		/script m.t@g.com
 `
 
-func LoadInput(in string) Input {
+func LoadInput(in string) check.Input {
 	r := strings.NewReader(in)
 
-	return Input{
+	return check.Input{
 		CodeownersEntries: codeowners.ParseCodeowners(r),
 	}
 }
+
+func assertIssue(t *testing.T, expIssue *check.Issue, gotIssues []check.Issue) {
+	t.Helper()
+
+	if expIssue != nil {
+		require.Len(t, gotIssues, 1)
+		assert.EqualValues(t, *expIssue, gotIssues[0])
+	} else {
+		assert.Empty(t, gotIssues)
+	}
+}

internal/check/not_owned_file.go

@@ -73,7 +73,7 @@ func (c *NotOwnedFile) Check(ctx context.Context, in Input) (output Output, err
 	lsOut := strings.TrimSpace(out)
 	if lsOut != "" {
 		lines := strings.Split(lsOut, "\n")
-		msg := fmt.Sprintf("Found %d not owned files (skipped patterns: %q): \n%s", len(lines), c.skipPatternsList(), c.ListFormatFunc(lines))
+		msg := fmt.Sprintf("Found %d not owned files (skipped patterns: %q):\n%s", len(lines), c.skipPatternsList(), c.ListFormatFunc(lines))
 		bldr.ReportIssue(msg)
 	}
 

internal/check/package_test.go

@@ -33,7 +33,7 @@ func TestRespectingCanceledContext(t *testing.T) {
 			cancel()
 
 			// when
-			out, err := sut.Check(ctx, check.LoadInput(check.FixtureValidCODEOWNERS))
+			out, err := sut.Check(ctx, LoadInput(FixtureValidCODEOWNERS))
 
 			// then
 			assert.True(t, errors.Is(err, context.Canceled))

internal/check/valid_owner.go

@@ -19,16 +19,24 @@ type ValidOwnerConfig struct {
 	// More info about the @ghost user: https://docs.github.com/en/free-pro-team@latest/github/setting-up-and-managing-your-github-user-account/deleting-your-user-account
 	// Tip on how @ghost can be used: https://github.saobby.my.eu.orgmunity/t5/How-to-use-Git-and-GitHub/CODEOWNERS-file-with-a-NOT-file-type-condition/m-p/31013/highlight/true#M8523
 	IgnoredOwners []string `envconfig:"default=@ghost"`
+	// AllowUnownedPatterns 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` this file is not owned by anyone.
+	AllowUnownedPatterns bool `envconfig:"default=true"`
 }
 
 // ValidOwner validates each owner
 type ValidOwner struct {
-	ghClient    *github.Client
-	orgMembers  *map[string]struct{}
-	orgName     string
-	orgTeams    []*github.Team
-	orgRepoName string
-	ignOwners   map[string]struct{}
+	ghClient             *github.Client
+	orgMembers           *map[string]struct{}
+	orgName              string
+	orgTeams             []*github.Team
+	orgRepoName          string
+	ignOwners            map[string]struct{}
+	allowUnownedPatterns bool
 }
 
 // NewValidOwner returns new instance of the ValidOwner
@@ -44,10 +52,11 @@ func NewValidOwner(cfg ValidOwnerConfig, ghClient *github.Client) (*ValidOwner,
 	}
 
 	return &ValidOwner{
-		ghClient:    ghClient,
-		orgName:     split[0],
-		orgRepoName: split[1],
-		ignOwners:   ignOwners,
+		ghClient:             ghClient,
+		orgName:              split[0],
+		orgRepoName:          split[1],
+		ignOwners:            ignOwners,
+		allowUnownedPatterns: cfg.AllowUnownedPatterns,
 	}, nil
 }
 
@@ -69,6 +78,11 @@ func (v *ValidOwner) Check(ctx context.Context, in Input) (Output, error) {
 	checkedOwners := map[string]struct{}{}
 
 	for _, entry := range in.CodeownersEntries {
+		if len(entry.Owners) == 0 && !v.allowUnownedPatterns {
+			bldr.ReportIssue("Missing owner, at least one owner is required", WithEntry(entry), WithSeverity(Warning))
+			continue
+		}
+
 		for _, ownerName := range entry.Owners {
 			if ctxutil.ShouldExit(ctx) {
 				return Output{}, ctx.Err()

internal/check/valid_owner_export_test.go

@@ -0,0 +1,5 @@
+package check
+
+func IsValidOwner(owner string) bool {
+	return isEmailAddress(owner) || isGithubUser(owner) || isGithubTeam(owner)
+}