update the ext-authz extension provider and promote to alpha

[yangminzhu]

This PR makes necessary change to promote the ext-authz feature to alpha:

1. Added with_request_body to support including body in the check request: This addresses user feedback: HTTP body missing from gRPC ExternalAuthorization provider CheckRequest istio#31182 (comment) and Better External Authorization support istio#27790 (comment)
2. Added prefix and suffix matching when including headers in the check request: This matches the syntax in the authorization policy, this is needed to simplify the configuration when there are large number of headers with same prefix (e.g. x-forwarded-access-token, x-forwarded-user, x-forwarded-email, ...) need to be included
3. Added add_headers_in_check to support including extra headers in the check request: This is just another way to include more headers.
4. Added timeout to support configuring the timeout to the external service: This is needed as we have found that the timeout in DestinationRule does not cover the ext-authz filter usage because the ext-authz filer has its own timeout constraints. The current default value 600s is chosen to match the current workaround, we could change it to a lower value (e.g. 200ms) if needed.
5. Updated the authorization_policy.proto documentation

[istio-policy-bot]

🤔 🐛 You appear to be fixing a bug in Go code, yet your PR doesn't include updates to any test files. Did you forget to add a test?

Courtesy of your friendly test nag.

[yangminzhu]

@louiscryan @smawson @nrjpoddar @linsun please take a look at these changes for promoting the ext-authz feature to alpha in 1.10, let me know if you have any questions, thank you.

[nrjpoddar]

@yangminzhu we need to look at the string match API here as we are creating a divergence from other Networking APIs.

[yangminzhu]

@yangminzhu we need to look at the string match API here as we are creating a divergence from other Networking APIs.

@nrjpoddar I was trying to align with the AuthZ API but I do not have a strong preference here. I think either one does the job and it is probably more convenient to be consistent with the AuthZ as this provider is only used with AuthZ API.

[nrjpoddar]

@yangminzhu we need to look at the string match API here as we are creating a divergence from other Networking APIs.

@nrjpoddar I was trying to align with the AuthZ API but I do not have a strong preference here. I think either one does the job and it is probably more convenient to be consistent with the AuthZ as this provider is only used with AuthZ API.

Yeah, I see it now. Can we link to https://istio.io/latest/docs/reference/config/security/authorization-policy/#Rule so that users know that API semantics are same between this and AuthZ policies?

[yangminzhu]

@nrjpoddar @howardjohn @smawson I have updated the PR to address comments so far. PTAL and let me know if you still have more comments, thanks.

[smawson]

My concerns were met so deferring to @howardjohn and @nrjpoddar

[nrjpoddar]

@yangminzhu I missed submitting my review yesterday. Updates look good. I have 2 minor comments:

1. For one field you have a comment saying: “Note that this setting will have precedence over the fail_open field.” but don’t explain the relationship? They don’t seem related at first glance.
2. Secondly, yesterday when I had looked your comment said that there was a bug for duplicated header keys. I think we need to document the expected behavior instead of expecting users to go through Envoy docs and PRs. If indeed, there’s a discrepancy for duplicated headers it can lead to CVEs so we should be careful about merging this without understanding what’s happening.

[nrjpoddar]

I’m adding do-not-merge until I understand the duplicate headers behavior.

[istio-testing]

@yangminzhu: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
release-notes_api	d5fdc05	link	/test release-notes_api

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[nrjpoddar]

Talked offline with @yangminzhu. Removing the don’t merge label. Thanks for incorporating the suggestion!

[yangminzhu]

/test gencheck_api