http: Parameterize normalize_uri::DefaultAuthority #886
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The NormalizeUri module needs is configured with a fallback authority in case we're dealing with poorly-formed HTTP requests (i.e., HTTP/1.0). Currently, the module is parameterized on a `SocketAddr`, because on the outbound side this makes sense as a default value. But on the inbound side, especially in a gateway configuration, it doesn't make sense to support a default value. It's better to just fail the request early. This change updates the NormalizeUri type to require its targets to be parameterized on a DefaultAuthority type. This type provides an _optional_ `http::uri::Authority` so the target may provide a named value (i.e., from a service profile response) when appropriate. It will also enable us to omit a default when appropriate.
olix0r
changed the title
hawkw
approved these changes
Jan 29, 2021
| fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { | ||
| write!( | ||
| f, | ||
| "failed to normalize URI because no authority could be determined" |
There was a problem hiding this comment.
it might be worth including the bad URI in this message? but, that would mean carrying it in the error type
kleimkuhler
approved these changes
Jan 29, 2021
olix0r
added a commit
to linkerd/linkerd2
that referenced
this pull request
Feb 3, 2021
This release changes HTTP protocol detection to prevent timeout errors in two ways: 1. HTTP detection no longer blocks until a newline is read. We've reverted to relying on a single read to make a determination. 2. Detection timeouts are no longer terminal. When a timeout is encountered, we continue forwarding the connection as an opaque TCP connection. These changes may lead to false-negatives--we may fail to detect some HTTP streams--but it should prevent many avoidable detection errors. This release also makes improvements for multicluster gateways, improving caching so that profile lookups are only performed once per target service. Diagnostic `stack_*` metrics have been moved so that they track underlying services, ignoring fail-fast. This should help us get better insights into services that are in failfast. Finally, the opencensus exporter has been improved to ensure that trace events are flushed if the trace buffer is not filled within a timeout. --- * actions: Update actions to use full SHAs (linkerd/linkerd2-proxy#885) * http: Parameterize normalize_ur::DefaultAuthority (linkerd/linkerd2-proxy#886) * http: Parameterize the HTTP server (linkerd/linkerd2-proxy#887) * opencensus: rewrite span exporter using async/await (linkerd/linkerd2-proxy#789) * Update http::Insert to use `Param` (linkerd/linkerd2-proxy#889) * Update crate dependencies (linkerd/linkerd2-proxy#892) * stack: Make the router fallible (linkerd/linkerd2-proxy#888) * Track stack metrics within failfast (linkerd/linkerd2-proxy#891) * outbound: Avoid building balancers when no concrete name (linkerd/linkerd2-proxy#890) * inbound: Cache HTTP gateways per destination (linkerd/linkerd2-proxy#893) * Reorganize the gateway crate (linkerd/linkerd2-proxy#897) * Bias HTTP detection towards availability (linkerd/linkerd2-proxy#894) * inbound: Use ALPN to determine transport header (linkerd/linkerd2-proxy#895) * detect: Return unknown protocol on detection timeout (linkerd/linkerd2-proxy#896) * Extract protocol detection into the gateway crate (linkerd/linkerd2-proxy#898)
olix0r
added a commit
to linkerd/linkerd2
that referenced
this pull request
Feb 3, 2021
This release changes HTTP protocol detection to prevent timeout errors in two ways: 1. HTTP detection no longer blocks until a newline is read. We've reverted to relying on a single read to make a determination. 2. Detection timeouts are no longer terminal. When a timeout is encountered, we continue forwarding the connection as an opaque TCP connection. These changes may lead to false-negatives--we may fail to detect some HTTP streams--but it should prevent many avoidable detection errors. This release also makes improvements for multicluster gateways, improving caching so that profile lookups are only performed once per target service. Diagnostic `stack_*` metrics have been moved so that they track underlying services, ignoring fail-fast. This should help us get better insights into services that are in failfast. Finally, the opencensus exporter has been improved to ensure that trace events are flushed if the trace buffer is not filled within a timeout. --- * actions: Update actions to use full SHAs (linkerd/linkerd2-proxy#885) * http: Parameterize normalize_ur::DefaultAuthority (linkerd/linkerd2-proxy#886) * http: Parameterize the HTTP server (linkerd/linkerd2-proxy#887) * opencensus: rewrite span exporter using async/await (linkerd/linkerd2-proxy#789) * Update http::Insert to use `Param` (linkerd/linkerd2-proxy#889) * Update crate dependencies (linkerd/linkerd2-proxy#892) * stack: Make the router fallible (linkerd/linkerd2-proxy#888) * Track stack metrics within failfast (linkerd/linkerd2-proxy#891) * outbound: Avoid building balancers when no concrete name (linkerd/linkerd2-proxy#890) * inbound: Cache HTTP gateways per destination (linkerd/linkerd2-proxy#893) * Reorganize the gateway crate (linkerd/linkerd2-proxy#897) * Bias HTTP detection towards availability (linkerd/linkerd2-proxy#894) * inbound: Use ALPN to determine transport header (linkerd/linkerd2-proxy#895) * detect: Return unknown protocol on detection timeout (linkerd/linkerd2-proxy#896) * Extract protocol detection into the gateway crate (linkerd/linkerd2-proxy#898)
jijeesh
pushed a commit
to jijeesh/linkerd2
that referenced
this pull request
Mar 23, 2021
This release changes HTTP protocol detection to prevent timeout errors in two ways: 1. HTTP detection no longer blocks until a newline is read. We've reverted to relying on a single read to make a determination. 2. Detection timeouts are no longer terminal. When a timeout is encountered, we continue forwarding the connection as an opaque TCP connection. These changes may lead to false-negatives--we may fail to detect some HTTP streams--but it should prevent many avoidable detection errors. This release also makes improvements for multicluster gateways, improving caching so that profile lookups are only performed once per target service. Diagnostic `stack_*` metrics have been moved so that they track underlying services, ignoring fail-fast. This should help us get better insights into services that are in failfast. Finally, the opencensus exporter has been improved to ensure that trace events are flushed if the trace buffer is not filled within a timeout. --- * actions: Update actions to use full SHAs (linkerd/linkerd2-proxy#885) * http: Parameterize normalize_ur::DefaultAuthority (linkerd/linkerd2-proxy#886) * http: Parameterize the HTTP server (linkerd/linkerd2-proxy#887) * opencensus: rewrite span exporter using async/await (linkerd/linkerd2-proxy#789) * Update http::Insert to use `Param` (linkerd/linkerd2-proxy#889) * Update crate dependencies (linkerd/linkerd2-proxy#892) * stack: Make the router fallible (linkerd/linkerd2-proxy#888) * Track stack metrics within failfast (linkerd/linkerd2-proxy#891) * outbound: Avoid building balancers when no concrete name (linkerd/linkerd2-proxy#890) * inbound: Cache HTTP gateways per destination (linkerd/linkerd2-proxy#893) * Reorganize the gateway crate (linkerd/linkerd2-proxy#897) * Bias HTTP detection towards availability (linkerd/linkerd2-proxy#894) * inbound: Use ALPN to determine transport header (linkerd/linkerd2-proxy#895) * detect: Return unknown protocol on detection timeout (linkerd/linkerd2-proxy#896) * Extract protocol detection into the gateway crate (linkerd/linkerd2-proxy#898) Signed-off-by: Jijeesh <jijeesh.ka@gmail.com>
jijeesh
pushed a commit
to jijeesh/linkerd2
that referenced
this pull request
Apr 21, 2021
This release changes HTTP protocol detection to prevent timeout errors in two ways: 1. HTTP detection no longer blocks until a newline is read. We've reverted to relying on a single read to make a determination. 2. Detection timeouts are no longer terminal. When a timeout is encountered, we continue forwarding the connection as an opaque TCP connection. These changes may lead to false-negatives--we may fail to detect some HTTP streams--but it should prevent many avoidable detection errors. This release also makes improvements for multicluster gateways, improving caching so that profile lookups are only performed once per target service. Diagnostic `stack_*` metrics have been moved so that they track underlying services, ignoring fail-fast. This should help us get better insights into services that are in failfast. Finally, the opencensus exporter has been improved to ensure that trace events are flushed if the trace buffer is not filled within a timeout. --- * actions: Update actions to use full SHAs (linkerd/linkerd2-proxy#885) * http: Parameterize normalize_ur::DefaultAuthority (linkerd/linkerd2-proxy#886) * http: Parameterize the HTTP server (linkerd/linkerd2-proxy#887) * opencensus: rewrite span exporter using async/await (linkerd/linkerd2-proxy#789) * Update http::Insert to use `Param` (linkerd/linkerd2-proxy#889) * Update crate dependencies (linkerd/linkerd2-proxy#892) * stack: Make the router fallible (linkerd/linkerd2-proxy#888) * Track stack metrics within failfast (linkerd/linkerd2-proxy#891) * outbound: Avoid building balancers when no concrete name (linkerd/linkerd2-proxy#890) * inbound: Cache HTTP gateways per destination (linkerd/linkerd2-proxy#893) * Reorganize the gateway crate (linkerd/linkerd2-proxy#897) * Bias HTTP detection towards availability (linkerd/linkerd2-proxy#894) * inbound: Use ALPN to determine transport header (linkerd/linkerd2-proxy#895) * detect: Return unknown protocol on detection timeout (linkerd/linkerd2-proxy#896) * Extract protocol detection into the gateway crate (linkerd/linkerd2-proxy#898) Signed-off-by: Jijeesh <jijeesh.ka@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The NormalizeUri module needs is configured with a fallback authority in
case we're dealing with poorly-formed HTTP requests (i.e., HTTP/1.0).
Currently, the module is parameterized on a
SocketAddr, because on theoutbound side this makes sense as a default value. But on the inbound
side, especially in a gateway configuration, it doesn't make sense to
support a default value. It's better to just fail the request early.
This change updates the NormalizeUri type to require its targets to be
parameterized on a DefaultAuthority type. This type provides an
optional
http::uri::Authorityso the target may provide a namedvalue (i.e., from a service profile response) when appropriate. It will
also enable us to omit a default when appropriate.