Prow accepted invalid OWNERS_ALIASES change and broke approvals

[tuminoid]

Issue

We use OWNERS file that points to aliases listed in OWNERS_ALIASES.

We promoted all of our reviewers to approvers in this PR, which then caused Prow to stop accepting approves, such as here and here.

Expected outcome

When merging PR116, Prow should've labeled the PR as "do-not-merge/invalid-owners-file" and blocked it.

Actual outcome

Prow/tide did not complain, we merged it, and noticed that approving was broken. Had to manually merge a fix PR, which then solved the situation right away.

Looking at the verify-owners_test.go:

1. It seems it should fail to empty groups. I'm suspecting there is loophole due usage of OWNERS_ALIASES, which is much less tested here.
2. Another alternative is the use of {} to signal empty group, which is valid YAML, but maybe not expected by Prow?

Logs

There was no error logs in Prow, just regular comment/approval handling logs, that did not indicate OWNERS was broken. It just claimed "comment did not result in approval".

[tuminoid]

/kind bug

[nikhilkumar13199]

Can I assign this to myself? Prima facie, seems like a missed edge case,

[tuminoid]

Can I assign this to myself? Prima facie, seems like a missed edge case,

/assign @nikhilkumar13199

Any help would be appreciated, Prow is needing contributors!

[nikhilnishad-goog]

I see possible problems. One thing is with {} and without {} seemed to be parsed different by go. Also, we are simply iterating over strings(usernames) in each alias in ParseAliasesConfig and not handling the case where an alias group may be empty (have no users in it). Ideally, we should not accept such empty cases and throw.