Skip to content

Commit 4071e97

Browse files
committed
Feedback
Signed-off-by: jukie <10012479+jukie@users.noreply.github.com>
1 parent 5acc84b commit 4071e97

27 files changed

Lines changed: 521 additions & 471 deletions

internal/gatewayapi/listener_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1379,7 +1379,7 @@ func TestProcessBackendRefsBackendTLSPolicy(t *testing.T) {
13791379
},
13801380
}
13811381
backendEndpoints := []*ir.DestinationEndpoint{{Host: "otel.example.com", Port: 443}}
1382-
backendMetadata := &ir.ResourceMetadata{Name: backendName, Namespace: ns}
1382+
backendMetadata := &ir.ResourceMetadata{Kind: resource.KindBackend, Name: backendName, Namespace: ns}
13831383
backendPolicyTLS := &ir.TLSUpstreamConfig{
13841384
SNI: ptr.To("otel.example.com"), UseSystemTrustStore: true,
13851385
CACertificate: &ir.TLSCACertificate{Name: "otel-tls/test-ns-ca"}, SubjectAltNames: []ir.SubjectAltName{},
@@ -1417,7 +1417,7 @@ func TestProcessBackendRefsBackendTLSPolicy(t *testing.T) {
14171417
},
14181418
}
14191419
serviceEndpoints := []*ir.DestinationEndpoint{{Host: "7.7.7.7", Port: 4317}}
1420-
serviceMetadata := &ir.ResourceMetadata{Name: serviceName, Namespace: ns, SectionName: "4317"}
1420+
serviceMetadata := &ir.ResourceMetadata{Kind: resource.KindService, Name: serviceName, Namespace: ns, SectionName: "4317"}
14211421
servicePolicyTLS := &ir.TLSUpstreamConfig{
14221422
SNI: ptr.To("otel-svc.example.com"), UseSystemTrustStore: true,
14231423
CACertificate: &ir.TLSCACertificate{Name: "otel-svc-tls/test-ns-ca"}, SubjectAltNames: []ir.SubjectAltName{},

internal/gatewayapi/overlap_test.go

Lines changed: 78 additions & 114 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,13 @@ import (
1414
"github.com/envoyproxy/gateway/internal/ir"
1515
)
1616

17-
func TestRouteMatchesOverlap(t *testing.T) {
17+
// routesOverlap is a test helper that returns true if two routes produce the
18+
// same canonical overlap key (i.e., they match the same set of requests).
19+
func routesOverlap(a, b *ir.HTTPRoute) bool {
20+
return buildOverlapKey(a) == buildOverlapKey(b)
21+
}
22+
23+
func TestBuildOverlapKey(t *testing.T) {
1824
tests := []struct {
1925
name string
2026
a *ir.HTTPRoute
@@ -143,156 +149,114 @@ func TestRouteMatchesOverlap(t *testing.T) {
143149
overlap: false,
144150
},
145151
{
146-
name: "identical exact path with identical query param matches",
152+
name: "header names are compared case-insensitively",
147153
a: &ir.HTTPRoute{
148154
Hostname: "example.com",
149155
PathMatch: &ir.StringMatch{Exact: ptr.To("/foo")},
150-
QueryParamMatches: []*ir.StringMatch{
151-
{Name: "key", Exact: ptr.To("value")},
156+
HeaderMatches: []*ir.StringMatch{
157+
{Name: "X-Custom", Exact: ptr.To("val1")},
152158
},
153159
},
154160
b: &ir.HTTPRoute{
155161
Hostname: "example.com",
156162
PathMatch: &ir.StringMatch{Exact: ptr.To("/foo")},
157-
QueryParamMatches: []*ir.StringMatch{
158-
{Name: "key", Exact: ptr.To("value")},
163+
HeaderMatches: []*ir.StringMatch{
164+
{Name: "x-custom", Exact: ptr.To("val1")},
159165
},
160166
},
161167
overlap: true,
162168
},
163169
{
164-
name: "identical regex path matches are detected as overlap",
170+
name: "header matches in different order still overlap",
165171
a: &ir.HTTPRoute{
166172
Hostname: "example.com",
167-
PathMatch: &ir.StringMatch{SafeRegex: ptr.To("/foo.*")},
173+
PathMatch: &ir.StringMatch{Exact: ptr.To("/foo")},
174+
HeaderMatches: []*ir.StringMatch{
175+
{Name: "X-A", Exact: ptr.To("1")},
176+
{Name: "X-B", Exact: ptr.To("2")},
177+
},
168178
},
169179
b: &ir.HTTPRoute{
170180
Hostname: "example.com",
171-
PathMatch: &ir.StringMatch{SafeRegex: ptr.To("/foo.*")},
181+
PathMatch: &ir.StringMatch{Exact: ptr.To("/foo")},
182+
HeaderMatches: []*ir.StringMatch{
183+
{Name: "X-B", Exact: ptr.To("2")},
184+
{Name: "X-A", Exact: ptr.To("1")},
185+
},
172186
},
173187
overlap: true,
174188
},
175-
}
176-
177-
for _, tt := range tests {
178-
t.Run(tt.name, func(t *testing.T) {
179-
assert.Equal(t, tt.overlap, routeMatchesOverlap(tt.a, tt.b))
180-
})
181-
}
182-
}
183-
184-
func TestStringMatchEqual(t *testing.T) {
185-
tests := []struct {
186-
name string
187-
a *ir.StringMatch
188-
b *ir.StringMatch
189-
equal bool
190-
}{
191-
{
192-
name: "both nil",
193-
a: nil,
194-
b: nil,
195-
equal: true,
196-
},
197-
{
198-
name: "one nil",
199-
a: &ir.StringMatch{Exact: ptr.To("foo")},
200-
b: nil,
201-
equal: false,
202-
},
203-
{
204-
name: "identical exact",
205-
a: &ir.StringMatch{Name: "h", Exact: ptr.To("v")},
206-
b: &ir.StringMatch{Name: "h", Exact: ptr.To("v")},
207-
equal: true,
208-
},
209-
{
210-
name: "different name",
211-
a: &ir.StringMatch{Name: "a", Exact: ptr.To("v")},
212-
b: &ir.StringMatch{Name: "b", Exact: ptr.To("v")},
213-
equal: false,
214-
},
215189
{
216-
name: "different value",
217-
a: &ir.StringMatch{Name: "h", Exact: ptr.To("v1")},
218-
b: &ir.StringMatch{Name: "h", Exact: ptr.To("v2")},
219-
equal: false,
220-
},
221-
{
222-
name: "different distinct",
223-
a: &ir.StringMatch{Name: "h", Distinct: true},
224-
b: &ir.StringMatch{Name: "h", Distinct: false},
225-
equal: false,
226-
},
227-
{
228-
name: "different invert",
229-
a: &ir.StringMatch{Name: "h", Exact: ptr.To("v"), Invert: ptr.To(true)},
230-
b: &ir.StringMatch{Name: "h", Exact: ptr.To("v")},
231-
equal: false,
232-
},
233-
{
234-
name: "same invert",
235-
a: &ir.StringMatch{Name: "h", Exact: ptr.To("v"), Invert: ptr.To(true)},
236-
b: &ir.StringMatch{Name: "h", Exact: ptr.To("v"), Invert: ptr.To(true)},
237-
equal: true,
238-
},
239-
}
240-
241-
for _, tt := range tests {
242-
t.Run(tt.name, func(t *testing.T) {
243-
assert.Equal(t, tt.equal, stringMatchEqual(tt.a, tt.b))
244-
})
245-
}
246-
}
247-
248-
func TestStringMatchSliceEqual(t *testing.T) {
249-
tests := []struct {
250-
name string
251-
a []*ir.StringMatch
252-
b []*ir.StringMatch
253-
equal bool
254-
}{
255-
{
256-
name: "both empty",
257-
a: nil,
258-
b: nil,
259-
equal: true,
190+
name: "identical exact path with identical query param matches",
191+
a: &ir.HTTPRoute{
192+
Hostname: "example.com",
193+
PathMatch: &ir.StringMatch{Exact: ptr.To("/foo")},
194+
QueryParamMatches: []*ir.StringMatch{
195+
{Name: "key", Exact: ptr.To("value")},
196+
},
197+
},
198+
b: &ir.HTTPRoute{
199+
Hostname: "example.com",
200+
PathMatch: &ir.StringMatch{Exact: ptr.To("/foo")},
201+
QueryParamMatches: []*ir.StringMatch{
202+
{Name: "key", Exact: ptr.To("value")},
203+
},
204+
},
205+
overlap: true,
260206
},
261207
{
262-
name: "different lengths",
263-
a: []*ir.StringMatch{{Name: "a", Exact: ptr.To("1")}},
264-
b: nil,
265-
equal: false,
208+
name: "query param matches in different order still overlap",
209+
a: &ir.HTTPRoute{
210+
Hostname: "example.com",
211+
QueryParamMatches: []*ir.StringMatch{
212+
{Name: "a", Exact: ptr.To("1")},
213+
{Name: "b", Exact: ptr.To("2")},
214+
},
215+
},
216+
b: &ir.HTTPRoute{
217+
Hostname: "example.com",
218+
QueryParamMatches: []*ir.StringMatch{
219+
{Name: "b", Exact: ptr.To("2")},
220+
{Name: "a", Exact: ptr.To("1")},
221+
},
222+
},
223+
overlap: true,
266224
},
267225
{
268-
name: "identical",
269-
a: []*ir.StringMatch{
270-
{Name: "a", Exact: ptr.To("1")},
271-
{Name: "b", Exact: ptr.To("2")},
226+
name: "cookie matches in different order still overlap",
227+
a: &ir.HTTPRoute{
228+
Hostname: "example.com",
229+
CookieMatches: []*ir.StringMatch{
230+
{Name: "a", Exact: ptr.To("1")},
231+
{Name: "b", Exact: ptr.To("2")},
232+
},
272233
},
273-
b: []*ir.StringMatch{
274-
{Name: "a", Exact: ptr.To("1")},
275-
{Name: "b", Exact: ptr.To("2")},
234+
b: &ir.HTTPRoute{
235+
Hostname: "example.com",
236+
CookieMatches: []*ir.StringMatch{
237+
{Name: "b", Exact: ptr.To("2")},
238+
{Name: "a", Exact: ptr.To("1")},
239+
},
276240
},
277-
equal: true,
241+
overlap: true,
278242
},
279243
{
280-
name: "same elements different order",
281-
a: []*ir.StringMatch{
282-
{Name: "a", Exact: ptr.To("1")},
283-
{Name: "b", Exact: ptr.To("2")},
244+
name: "identical regex path matches are detected as overlap",
245+
a: &ir.HTTPRoute{
246+
Hostname: "example.com",
247+
PathMatch: &ir.StringMatch{SafeRegex: ptr.To("/foo.*")},
284248
},
285-
b: []*ir.StringMatch{
286-
{Name: "b", Exact: ptr.To("2")},
287-
{Name: "a", Exact: ptr.To("1")},
249+
b: &ir.HTTPRoute{
250+
Hostname: "example.com",
251+
PathMatch: &ir.StringMatch{SafeRegex: ptr.To("/foo.*")},
288252
},
289-
equal: false,
253+
overlap: true,
290254
},
291255
}
292256

293257
for _, tt := range tests {
294258
t.Run(tt.name, func(t *testing.T) {
295-
assert.Equal(t, tt.equal, stringMatchSliceEqual(tt.a, tt.b))
259+
assert.Equal(t, tt.overlap, routesOverlap(tt.a, tt.b))
296260
})
297261
}
298262
}

0 commit comments

Comments
 (0)