Skip to content

fix: set HTTPRoute Accepted condition as true with mixed invalid and valid rules#7625

Merged
zhaohuabing merged 5 commits intoenvoyproxy:mainfrom
zhaohuabing:fix-mixed-valid-invalid-httprules
Dec 2, 2025
Merged

fix: set HTTPRoute Accepted condition as true with mixed invalid and valid rules#7625
zhaohuabing merged 5 commits intoenvoyproxy:mainfrom
zhaohuabing:fix-mixed-valid-invalid-httprules

Conversation

@zhaohuabing
Copy link
Copy Markdown
Member

@zhaohuabing zhaohuabing commented Nov 28, 2025

This PR is the follow-up of #7556.

Currently, the Gateway API translator sets HTTPRoute Accepted condition as false when any of the its rules is invalid. This PR changes the behavior to set Accepted as false only if all rules are invalid, if both valid and invalid rules exist, the Accepted is set to true, and a PartiallyInvalid condition is added to the status.

The updated behavior aligns with the Gateway API spec.

https://gateway-api.sigs.k8s.io/geps/gep-1364/

A HTTPRoute with two rules, one valid and one which specifies a HTTPRequestRedirect filter _and a HTTPURLRewrite filter. Accepted is true, because the valid rule can produce some config in the data plane. We'll need to raise the more specific error condition for an incompatible filter combination as well to make the partial validity clear.

RouteConditionPartiallyInvalid:

// This condition indicates that the Route contains a combination of both
// valid and invalid rules.
//
// When this happens, implementations MUST take one of the following
// approaches:
//
// 1) Drop Rule(s): With this approach, implementations will drop the
// invalid Route Rule(s) until they are fully valid again. The message
// for this condition MUST start with the prefix "Dropped Rule" and
// include information about which Rules have been dropped. In this
// state, the "Accepted" condition MUST be set to "True" with the latest
// generation of the resource.
// 2) Fall Back: With this approach, implementations will fall back to the
// last known good state of the entire Route. The message for this
// condition MUST start with the prefix "Fall Back" and include
// information about why the current Rule(s) are invalid. To represent
// this, the "Accepted" condition MUST be set to "True" with the
// generation of the last known good state of the resource.

Fix: #7545

@zhaohuabing zhaohuabing requested a review from a team as a code owner November 28, 2025 11:45
@codecov
Copy link
Copy Markdown

codecov bot commented Nov 28, 2025

Codecov Report

❌ Patch coverage is 87.77778% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.37%. Comparing base (7864465) to head (6cf22c4).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
internal/gatewayapi/route.go 87.77% 9 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7625      +/-   ##
==========================================
+ Coverage   72.33%   72.37%   +0.03%     
==========================================
  Files         232      232              
  Lines       34143    34209      +66     
==========================================
+ Hits        24699    24760      +61     
- Misses       7669     7675       +6     
+ Partials     1775     1774       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@zhaohuabing zhaohuabing force-pushed the fix-mixed-valid-invalid-httprules branch 3 times, most recently from e609358 to 6beebb8 Compare November 28, 2025 12:27
@zhaohuabing zhaohuabing marked this pull request as draft November 28, 2025 14:26
@zhaohuabing zhaohuabing force-pushed the fix-mixed-valid-invalid-httprules branch 2 times, most recently from 291ae33 to c3d5f14 Compare December 1, 2025 02:46
@zhaohuabing zhaohuabing marked this pull request as ready for review December 1, 2025 02:47
… rules

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
@zhaohuabing zhaohuabing force-pushed the fix-mixed-valid-invalid-httprules branch from c3d5f14 to 046ecdf Compare December 1, 2025 02:47
if len(unacceptedRules) == 0 {
return fmt.Sprintf("Dropped Rule(s): %s", status.Error2ConditionMsg(err))
}
return fmt.Sprintf("Dropped Rule(s) %v: %s", unacceptedRules, status.Error2ConditionMsg(err))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

thoughts on skipped vs dropped

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is required in the Gateway API spec:

  1. Drop Rule(s): With this approach, implementations will drop the
    invalid Route Rule(s) until they are fully valid again. The message
    for this condition MUST start with the prefix "Dropped Rule" and
    include information about which Rules have been dropped. In this
    state, the "Accepted" condition MUST be set to "True" with the latest
    generation of the resource.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ah thanks !

@zhaohuabing zhaohuabing requested a review from arkodg December 2, 2025 01:11
arkodg
arkodg previously approved these changes Dec 2, 2025
Copy link
Copy Markdown
Contributor

@arkodg arkodg left a comment

Choose a reason for hiding this comment

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

LGTM thanks

@arkodg arkodg requested review from a team December 2, 2025 01:30
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
@kkk777-7
Copy link
Copy Markdown
Member

kkk777-7 commented Dec 2, 2025

LGTM, thanks!
This is a good change that aligns with the Gateway API Spec :)

@kkk777-7
Copy link
Copy Markdown
Member

kkk777-7 commented Dec 2, 2025

/retest

@zhaohuabing zhaohuabing merged commit 4620106 into envoyproxy:main Dec 2, 2025
53 of 55 checks passed
@zhaohuabing zhaohuabing deleted the fix-mixed-valid-invalid-httprules branch December 2, 2025 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

All HTTPRoute rules return 404 when referenced HTTPRouteFilter in one rule does not exist

4 participants