Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Remove ErrHTTPRouteMatchEmpty check for Xds IR #1959

Merged
merged 1 commit into from
Oct 13, 2023

Conversation

arkodg
Copy link
Contributor

@arkodg arkodg commented Oct 12, 2023

Empty matches are supported in the gateway api spec e.g. https://gateway-api.sigs.k8s.io/references/spec/#gateway.networking.k8s.io%2fv1alpha2.GRPCRouteRule and we are also tranlating it correctly in the gateway-api layer e.g.

// If no matches are specified, the implementation MUST match every HTTP request.
but the IR layer is rejecting this valid config

Relates to #1958 (comment)

Empty matches are supported in the gateway api spec
e.g. https://gateway-api.sigs.k8s.io/references/spec/#gateway.networking.k8s.io%2fv1alpha2.GRPCRouteRule
and we are also tranlating it correctly in the gateway-api layer
e.g. https://github.com/envoyproxy/gateway/blob/76704d1124eccaf06b81634a96390c95722a0554/internal/gatewayapi/route.go#L212
but the IR layer is rejecting this valid config

Relates to envoyproxy#1958 (comment)

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
@arkodg arkodg requested a review from a team as a code owner October 12, 2023 18:42
@arkodg arkodg requested review from a team, LanceEa, chauhanshubham and Xunzhuo and removed request for a team October 12, 2023 18:46
@arkodg arkodg added the kind/bug Something isn't working label Oct 12, 2023
@arkodg arkodg added this to the 0.6.0-rc1 milestone Oct 12, 2023
@codecov
Copy link

codecov bot commented Oct 12, 2023

Codecov Report

Merging #1959 (5083703) into main (76704d1) will decrease coverage by 0.08%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1959      +/-   ##
==========================================
- Coverage   65.43%   65.36%   -0.08%     
==========================================
  Files          90       90              
  Lines       13367    13364       -3     
==========================================
- Hits         8747     8735      -12     
- Misses       4083     4090       +7     
- Partials      537      539       +2     
Files Coverage Δ
internal/ir/xds.go 74.41% <ø> (-1.10%) ⬇️

... and 2 files with indirect coverage changes

@Xunzhuo Xunzhuo merged commit 3827919 into envoyproxy:main Oct 13, 2023
18 checks passed
@arkodg arkodg deleted the rm-empty-match-ir-check branch October 13, 2023 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants