external authz: Interpret error grpc codes of the external authz server as failure to fix failure_mode_allowed feature

[konstantin-baidin-y42]

trafficstars

Commit Message: Interpret error grpc codes of the external authz server as failure to fix failure_mode_allowed feature Additional Description: Fixes issue https://github.com/envoyproxy/envoy/issues/34705. Mark authz response as denied only for grpc codes PermissionDenied and Unauthenticated. Other codes are considered as failure and authz response status is set to error. This new behaviour is aligned with the HTTP external authz checks implementation, where error HTTP codes are considered as failure too. It shouldn't change anything when failure_mode_allowed=false as any non-OK grpc codes will lead to 403 for a client. But it fixes behaviour of failure_mode_allowed=true. Risk Level: Low Testing: unit test Docs Changes: --- Release Notes: With failure_mode_allowed enabled and external grpc authz server returning error codes like internal error, the request will be allowed (fail-open), see https://github.com/envoyproxy/envoy/issues/34705 for details. Platform Specific Features: --- Optional Runtime guard: I'm not sure if it should be guarded. It affects end users, but it's a bug fix. Fixes #34705

[repokitteh-read-only[bot]]

Hi @konstantin-baidin-y42, welcome and thank you for your contribution.

We will try to review your Pull Request as quickly as possible.

In the meantime, please take a look at the contribution guidelines if you have not done so already.

:cat:

Caused by: https://github.com/envoyproxy/envoy/pull/34951 was opened by konstantin-baidin-y42.

see: more, trace.

[repokitteh-read-only[bot]]

As a reminder, PRs marked as draft will not be automatically assigned reviewers, or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

:cat:

Caused by: https://github.com/envoyproxy/envoy/pull/34951 was opened by konstantin-baidin-y42.

see: more, trace.

[htuch]

Probably needs at least runtime guarding given potential for differences in dataplane behavior. @tyxia should the semantics here be harmonized with ext_proc, in particular if we start using it for auth purposes (CC @rshriram)?

[repokitteh-read-only[bot]]

CC @envoyproxy/runtime-guard-changes: FYI only for changes made to (source/common/runtime/runtime_features.cc).

:cat:

Caused by: https://github.com/envoyproxy/envoy/pull/34951 was synchronize by konstantin-baidin-y42.

see: more, trace.

[repokitteh-read-only[bot]]

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy[\w/]*/(v1alpha\d?|v1|v2alpha\d?|v2))|(api/envoy/type/(matcher/)?\w+.proto). CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/). envoyproxy/api-shepherds assignee is @htuch CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

:cat:

Caused by: https://github.com/envoyproxy/envoy/pull/34951 was synchronize by konstantin-baidin-y42.

see: more, trace.

[konstantin-baidin-y42]

Hello @ggreenway, thank you for the review.

• I clarified in the docs for failure_mode_allow (in ext_authz.proto) which grpc response codes are still treated as denied when this is true.
• The runtime guard is now true by default.
• I replaced the old test AuthorizationDeniedGrpcUnknownStatus with the new test AuthorizationReturnsDeniedOnUnknownGrpcCodeWhenFeatureFlagDisabled
• I tried to add a link for failure_mode_allow docs, but it looks like the https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/http/ext_authz/v3/ext_authz.proto page doesn't have anchor links, so I added just a link to the page where this param can be found.

Can you please review again?

[konstantin-baidin-y42]

Resolved conflict