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 505 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
Comment thread internal/gatewayapi/route.go Outdated
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
rudrakhp pushed a commit to rudrakhp/gateway that referenced this pull request Apr 15, 2026
…valid rules (envoyproxy#7625)

* set HTTPRoute Accepted condition as true with mixed invalid and valid rules

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>
rudrakhp added a commit that referenced this pull request Apr 16, 2026
* fix: avoid metric increments on no-op delete reconcile paths (#8480)

* fix: avoid metric increments on no-op delete reconcile paths

Signed-off-by: Felipe Sabadini Facina <fsabadini@hotmail.com>

* Update internal/infrastructure/kubernetes/infra_resource_test.go

Signed-off-by: Isaac Wilson <isaac.wilson514@gmail.com>

* Update internal/infrastructure/kubernetes/infra_resource_test.go

Signed-off-by: Isaac Wilson <isaac.wilson514@gmail.com>

---------

Signed-off-by: Felipe Sabadini Facina <fsabadini@hotmail.com>
Signed-off-by: Isaac Wilson <isaac.wilson514@gmail.com>
Co-authored-by: Isaac Wilson <isaac.wilson514@gmail.com>
Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>

* fix: restore failure-path metric recording for delete and HPA reconcile (#8656)

Fixes #8651

Signed-off-by: Felipe Sabadini Facina <fsabadini@hotmail.com>
Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>

* fix: helm secrets rbac for gateway namespace with watch list of namespaces (#8706)

* fix: helm secrets rbac for gateway namespace with watch list of namespaces

Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com>

* add release notes

Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com>

* review update

Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com>

---------

Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com>
Co-authored-by: Rudrakh Panigrahi <rudrakh97@gmail.com>
Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>

* fix: handle network errors in rate limit e2e tests (#8446)

Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>

* fix: propagate the HTTPFilter translation errors to the outer layer (#7556)

* progate the HTTPFilter validation errors to the outer layer

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>

* fix: return 500 error for invalid filters (#7605)

return 500 error for invalid filters

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>

* fix: prevent configuring requestMirror filter and directResponse/RequestRedirect filter together (#7474)

* fix: prevent configuring RequestMirror and DirectResponse filters together

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

check redirect respose filter

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* address comments

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* fix lint

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

---------

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>

* fix bug with grpcroute mirror filter (#8541)

* fix bug with grpcroute mirror filter

Signed-off-by: Adam Buran <aburan@roblox.com>

* add indexers test

Signed-off-by: Adam Buran <aburan@roblox.com>

* add release note

Signed-off-by: Adam Buran <aburan@roblox.com>

---------

Signed-off-by: Adam Buran <aburan@roblox.com>
Signed-off-by: Arko Dasgupta <arkodg@users.noreply.github.com>
Co-authored-by: Arko Dasgupta <arkodg@users.noreply.github.com>
Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>

* fix: normalize CRLF line endings in htpasswd basic auth secrets (#8557)

Fixes #8554

Signed-off-by: stekole <stefan@sandnetworks.com>
Signed-off-by: stekole <30674956+stekole@users.noreply.github.com>
Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>

* fix: status for mirror backend (#8675)

Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>
Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>

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

* set HTTPRoute Accepted condition as true with mixed invalid and valid rules

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>

* fix: basic auth validation (#8053)

* fix basic auth validation

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>

* [release/v1.6] fix gen check

Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>

---------

Signed-off-by: Felipe Sabadini Facina <fsabadini@hotmail.com>
Signed-off-by: Isaac Wilson <isaac.wilson514@gmail.com>
Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>
Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com>
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: Adam Buran <aburan@roblox.com>
Signed-off-by: Arko Dasgupta <arkodg@users.noreply.github.com>
Signed-off-by: stekole <stefan@sandnetworks.com>
Signed-off-by: stekole <30674956+stekole@users.noreply.github.com>
Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
Co-authored-by: Felipe Sabadini <fsabadini@hotmail.com>
Co-authored-by: Isaac Wilson <isaac.wilson514@gmail.com>
Co-authored-by: Karol Szwaj <karol.szwaj@gmail.com>
Co-authored-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
Co-authored-by: aburanrbx <aburan@roblox.com>
Co-authored-by: Arko Dasgupta <arkodg@users.noreply.github.com>
Co-authored-by: stekole <30674956+stekole@users.noreply.github.com>
Co-authored-by: Kota Kimura <86363983+kkk777-7@users.noreply.github.com>
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.

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

5 participants