Fix H2 URI path matching in ALB health check layer#4288
Fix H2 URI path matching in ALB health check layer#4288
Conversation
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
| use tower::{service_fn, ServiceExt}; | ||
|
|
||
| #[tokio::test] | ||
| async fn health_check_matches_path_with_scheme_and_authority() { |
There was a problem hiding this comment.
For my own edification where is the "with scheme and authority" part of this test coming from?
There was a problem hiding this comment.
The scheme and authority are intentionally hard-coded in this test. I've renamed the test to health_check_matches_path_when_uri_contains_scheme_authority_and_path to better reflect its purpose.
This test specifically verifies that the health check correctly matches requests based solely on their path component, regardless of whether the URI also includes scheme and authority components. This ensures our health check behavior remains consistent across different URI.
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
drganjoo
force-pushed
the
fahadzub/health
branch
from
049edb6 to
fc73951
Compare
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
This PR resolves the CI failure [occurring in PR #4288](https://github.com/smithy-lang/smithy-rs/actions/runs/17608605180/job/50025264439?pr=4288#step:4:871) by correcting the path reference.
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
| [package] | ||
| name = "aws-smithy-http-server" | ||
| version = "0.65.5" | ||
| version = "0.66.0" |
There was a problem hiding this comment.
why did you make a new breaking version? isn't this going to cause problems? This change seems backwards compatible?
There was a problem hiding this comment.
You're absolutely right - this is a backwards compatible change. I bumped to 0.66 as a precautionary measure because I wanted to avoid any potential versioning issues during integration testing. At the time, this was planned to be included in the next code generator release, so the version bump seemed appropriate. However, since this now needs to be released independently of the code generator, you're correct that 0.65.6 would be more suitable.
There was a problem hiding this comment.
well and it seems like making a new breaking change release of the libraries when we don't need it only causes lots of pain?
There was a problem hiding this comment.
We typically never release intermediate crate versions of aws-smithy-http-server independently of the code generator, so bumping the major version, combined with a code-generator release, is usually the safer approach - it eliminates any risk of breaking users due to potential CI tooling issues. When releases are tied to the code generator, a major version bump has minimal downstream impact compared to accidentally introducing breaking changes.
However, since this particular release now needs to happen independently of the code generator, I've changed it to only bump the minor version.
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
3 participants
Summary
Fixed URI path matching for H2 protocol in the ALB health check layer to ensure explicit path comparison.
Changes
alb_health_check.rsto explicitly match URI paths for H2 requestsTesting