authz: reject duplicate rule names in authorization policy

[nvxbug]

parseRules builds its map[string]*Policy keyed on the rule name, but two rules in the same list that share a name collapse to one entry: the second overwrites the first. For deny_rules that is fail-open, a deny the operator wrote is dropped and matching RPCs fall through to the allow policy, while translatePolicy still returns success and the file watcher logs a normal reload, so the lost rule is invisible downstream.

Reject a repeated name in parseRules, which is the last point that still has the rule list before it becomes a map. Both the deny and allow lists go through this function, so one check covers both, and an ambiguous policy is refused instead of silently discarding a rule.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.11%. Comparing base (44332db) to head (f0539d0).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #9312      +/-   ##
==========================================
- Coverage   83.20%   83.11%   -0.10%     
==========================================
  Files         423      423              
  Lines       35321    35323       +2     
==========================================
- Hits        29389    29357      -32     
- Misses       4425     4443      +18     
- Partials     1507     1523      +16     

Files with missing lines	Coverage Δ
authz/rbac_translator.go	98.06% <100.00%> (+0.99%)	⬆️

... and 33 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[easwars]

@mbissa : This is the spec for the authz policy: https://github.com/grpc/proposal/blob/master/A43-grpc-authorization-api.md

We need to be sure what the expected behavior is (and what other language implementations do) before proceeding with this.

[nvxbug]

Looked into both. The A43 schema says the rule name "should be unique within the list of deny (or allow) rules", but it doesn't say what an implementation should do when it isn't.

Neither C-core nor Java enforces it today: C-core's ParseRulesArray (src/core/lib/security/authorization/rbac_translator.cc) assigns into a std::map keyed by the policy name, and Java's AuthorizationPolicyTranslator.parseRules does policies.put(...) on a LinkedHashMap. So all three implementations currently have the same last-rule-wins behavior, including the fail-open case for deny_rules.

Since the spec already calls for unique names, rejecting the policy seemed like the reading most consistent with it, though it does make Go stricter than the others until they add the same check. Happy to file issues against grpc/grpc and grpc-java for parity, or take it to the proposal repo first if you'd rather clarify A43 before changing behavior here.