Skip to content

docs: elaborate on behavior of scheme for ext_authz#14867

Merged
lizan merged 1 commit intoenvoyproxy:mainfrom
hbagdi:docs/elaborate-scheme
Feb 2, 2021
Merged

docs: elaborate on behavior of scheme for ext_authz#14867
lizan merged 1 commit intoenvoyproxy:mainfrom
hbagdi:docs/elaborate-scheme

Conversation

@hbagdi
Copy link
Copy Markdown
Contributor

@hbagdi hbagdi commented Jan 29, 2021

Scheme is populated for h2 and not for h1.1.
Advise accordingly in documentation.

Signed-off-by: Harry Bagdi harrybagdi@gmail.com

Commit Message: docs: elaborate on behavior of scheme for ext_authz
Additional Description: I spent quite some time to debug and test why scheme was not being populated for my requests. I finally figured out that it only works for h2. This is not obvious from the docs so I think this addition will help.
Risk Level: N/A
Testing: N/A
Docs Changes: Explain when scheme is populated for ext_authz filter and how to get the scheme when the variable is not set.
Release Notes: N/A
Platform Specific Features: N/A

Scheme is populated for h2 and not for h1.1.
Advise accordingly in documentation.

Signed-off-by: Harry Bagdi <harrybagdi@gmail.com>
@repokitteh-read-only
Copy link
Copy Markdown

Hi @hbagdi, 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.

🐱

Caused by: #14867 was opened by hbagdi.

see: more, trace.

@repokitteh-read-only
Copy link
Copy Markdown

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/.
API shepherd assignee is @lizan
CC @envoyproxy/api-watchers: FYI only for changes made to api/envoy/.

🐱

Caused by: #14867 was opened by hbagdi.

see: more, trace.

@repokitteh-read-only repokitteh-read-only bot removed the api label Feb 2, 2021
@lizan lizan merged commit 973a25a into envoyproxy:main Feb 2, 2021
Comment on lines +113 to +115
// The HTTP URL scheme, such as `http` and `https`. This is set for HTTP/2
// requests only. For HTTP/1.1, use "x-forwarded-for" header value to lookup
// the scheme of the request.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @alyssawilk you might want to unwind this as part of #14899?

Copy link
Copy Markdown
Contributor

@alyssawilk alyssawilk Feb 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately this comment is misleading as-is. It's correct for L1s, but for L2 HTTP/2 scheme does not reflect the URL scheme of the client request, and XFP is the correct thing to match on for both HTTP/1 and HTTP/2.

With my PR in flight, :scheme will always be set, but will still not be the correct thing to check at L1 Envoys. With the next PR, :scheme will always be set and will be the source of truth. I expect that to land in ~2 weeks, as I'd like some time between step 1) and 2) just to lower risk.

@hbagdi hbagdi deleted the docs/elaborate-scheme branch February 2, 2021 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants