Check for syntax errors in CODEOWNERS file

[knl]

Description

A significant class of errors with CODEOWNERS file are syntax errors,
that is, malformed entries.

This change adds checks for validating the codeowners file while it is
being parsed.

It will do a couple of simple checks, like:

• if the format is: pattern owner1 ... ownerN
• if the usernames/team names are following GitHubs restrictions
• if the email looks like an email address (something @ something .
  something)

Related issue(s)

Fixes #33

[mszostok]

Thanks a lot for your PR!

Your expectations are correct and the code should be adjusted but it will be much better to leave the pkg/codeowners/owners.go as dummy as it is.

Reasons:
Right now the output of the not valid owner is not consistent with other checks:

Additionally, it returns the wrong status code

Code	Description
1	The application startup failed due to the wrong configuration or internal error.

source: https://github.com/mszostok/codeowners-validator#exit-status-codes

Moving that logic directly to valid_owner.go solves the above problems as this step is executed and monitored by runner framework :)

==> Executing Valid Owner Checker (1.469938442s)
    [err] line 12: Not valid owner definition "ddf@."
4 check(s) executed, 1 failure(s)
exit status 3

Going deeper
There is already the valid_owner checker which reports problems with those lines:

==> Executing Valid Owner Checker (905.801616ms)
    [err] line 10: Not valid owner definition "#"
    [err] line 10: Not valid owner definition "this"
    [err] line 10: Not valid owner definition "comment"
    [err] line 10: Not valid owner definition "is"
    [err] line 10: Not valid owner definition "a"
    [err] line 10: Not valid owner definition "syntax"
    [err] line 10: Not valid owner definition "error"
    [err] line 12: Not valid owner definition "dupa"

4 check(s) executed, 1 failure(s)
exit status 3

versus implementation from this PR:

FATA[0000] entry '#' on line 10 does not look like an email 
exit status 1

so the best in your implementation is that you fail fast but the message is still quite unclear.

---

I saw your note

Note that I'm not using owners check, as it is not clear what kind of right are needed for the token (but that is a separate issue).

IMO the best solution is to make the valid_owner.go to work in two modes:

• local with static validation (e.g. someone doesn't want to specify the token or do not have an internet connection) but in such mode, this check skips some additional validation
• with GitHub access where you need to specify the token - btw. the token needs to be readonly

Action items:

• make the valid_owner.go to work in two modes: local and GitHub access
• improve the messages and fail fast with a clear statement that EOL comments are not allowed.

If you will need any help on PR, do not hesitate to dm me on k8s slack - nick mszostok :)

[knl]

@mszostok I see your point. What about instead of dividing valid_owner.go to work in two different modes, introduce a new check valid_syntax.go? Because in the code I'm proposing, owner format is in focus at it takes the bulk of the file. However, the real issue I'm trying to deal with is the syntax of the file. As you can see, I'm also checking if there is only one token on the line. In addition, what is missing, in order to have a parity matching with .gitignore, is the handling of line continuation (\ at the end of the line). None of those are related to owners.

[mszostok]

@knl you are right, my bad 🤦 Adding new check makes more sens 👍

are you interested in implementing such check?

[mszostok]

Thanks for your work on this issue, because of inactivity I decided to extract implemented logic as discussed :)

Closing in favor: #46