Skip to content

Commit 5edbbcd

Browse files
bjee19bjee19
committed
Add fix to parent ref creation logic
1 parent ae9c118 commit 5edbbcd

File tree

4 files changed

+97
-27
lines changed

4 files changed

+97
-27
lines changed

‎internal/controller/state/graph/policies.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,12 +325,18 @@ func checkTargetRoutesForOverlap(
325325
func checkForRouteOverlap(route *L7Route, hostPortPaths map[string]string) *conditions.Condition {
326326
for _, parentRef := range route.ParentRefs {
327327
if parentRef.Attachment != nil {
328+
// fmt.Printf("This is the parentRef attachment: %#v\n", parentRef.Attachment)
328329
port := parentRef.Attachment.ListenerPort
329330
for _, hostname := range parentRef.Attachment.AcceptedHostnames {
331+
// fmt.Println("Here is the keyName: " + keyName)
332+
// fmt.Printf("This is the parentref hostnames: %v\n", hostname)
330333
for _, rule := range route.Spec.Rules {
334+
// fmt.Printf("This is the routeRule: %v\n", rule)
331335
for _, match := range rule.Matches {
336+
// fmt.Printf("This is the match: %v\n", match)
332337
if match.Path != nil && match.Path.Value != nil {
333338
key := fmt.Sprintf("%s:%d%s", hostname, port, *match.Path.Value)
339+
// fmt.Println("This is the key: " + key)
334340
if val, ok := hostPortPaths[key]; !ok {
335341
hostPortPaths[key] = fmt.Sprintf("%s/%s", route.Source.GetNamespace(), route.Source.GetName())
336342
} else {

‎internal/controller/state/graph/policies_test.go

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1126,6 +1126,7 @@ func TestProcessPolicies_RouteOverlap(t *testing.T) {
11261126
policyGVK := schema.GroupVersionKind{Group: "Group", Version: "Version", Kind: "MyPolicy"}
11271127
pol1, pol1Key := createTestPolicyAndKey(policyGVK, "pol1", hrRefCoffee)
11281128
pol2, pol2Key := createTestPolicyAndKey(policyGVK, "pol2", hrRefCoffee, hrRefCoffeeTea)
1129+
pol3, pol3Key := createTestPolicyAndKey(policyGVK, "pol3", hrRefCoffeeTea)
11291130

11301131
tests := []struct {
11311132
validator validation.PolicyValidator
@@ -1153,6 +1154,25 @@ func TestProcessPolicies_RouteOverlap(t *testing.T) {
11531154
},
11541155
valid: true,
11551156
},
1157+
{
1158+
name: "no overlap two policies",
1159+
validator: &policiesfakes.FakeValidator{},
1160+
policies: map[PolicyKey]policies.Policy{
1161+
pol1Key: pol1,
1162+
pol3Key: pol3,
1163+
},
1164+
routes: map[RouteKey]*L7Route{
1165+
{
1166+
RouteType: RouteTypeHTTP,
1167+
NamespacedName: types.NamespacedName{Namespace: testNs, Name: "hr-coffee"},
1168+
}: createTestRouteWithPaths("hr-coffee", "/coffee"),
1169+
{
1170+
RouteType: RouteTypeHTTP,
1171+
NamespacedName: types.NamespacedName{Namespace: testNs, Name: "hr-coffee-tea"},
1172+
}: createTestRouteWithPaths("hr-coffee-tea", "/coffee-tea"),
1173+
},
1174+
valid: true,
1175+
},
11561176
{
11571177
name: "policy references route that overlaps a non-referenced route",
11581178
validator: &policiesfakes.FakeValidator{},
@@ -1256,7 +1276,7 @@ func TestProcessPolicies_RouteOverlap(t *testing.T) {
12561276
g := NewWithT(t)
12571277

12581278
processed := processPolicies(test.policies, test.validator, test.routes, nil, gateways)
1259-
g.Expect(processed).To(HaveLen(1))
1279+
g.Expect(processed).To(HaveLen(len(test.policies)))
12601280

12611281
for _, pol := range processed {
12621282
g.Expect(pol.Valid).To(Equal(test.valid))

‎internal/controller/state/graph/route_common.go

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -285,34 +285,57 @@ func buildSectionNameRefs(
285285
}
286286
uniqueSectionsPerGateway := make(map[key]struct{})
287287

288-
for i, p := range parentRefs {
288+
parentRefIndex := 0
289+
290+
for _, p := range parentRefs {
289291
gw := findGatewayForParentRef(p, routeNamespace, gws)
290292
if gw == nil {
291293
continue
292294
}
293295

294-
var sectionName string
295-
if p.SectionName != nil {
296-
sectionName = string(*p.SectionName)
297-
}
298-
299296
gwNsName := client.ObjectKeyFromObject(gw.Source)
300297
k := key{
301-
gwNsName: gwNsName,
302-
sectionName: sectionName,
298+
gwNsName: gwNsName,
299+
}
300+
301+
// If there is no section name, we create ParentRefs for each listener in the gateway
302+
if p.SectionName == nil {
303+
for _, l := range gw.Listeners {
304+
sectionName := string(l.Source.Name)
305+
k.sectionName = sectionName
306+
307+
if _, exist := uniqueSectionsPerGateway[k]; exist {
308+
return nil, fmt.Errorf("duplicate section name %q for Gateway %s", sectionName, gwNsName.String())
309+
}
310+
uniqueSectionsPerGateway[k] = struct{}{}
311+
312+
sectionNameRefs = append(sectionNameRefs, ParentRef{
313+
Idx: parentRefIndex,
314+
Gateway: CreateParentRefGateway(gw),
315+
SectionName: &l.Source.Name,
316+
Port: p.Port,
317+
})
318+
parentRefIndex++
319+
}
320+
continue
303321
}
304322

323+
sectionName := string(*p.SectionName)
324+
325+
k.sectionName = sectionName
326+
305327
if _, exist := uniqueSectionsPerGateway[k]; exist {
306328
return nil, fmt.Errorf("duplicate section name %q for Gateway %s", sectionName, gwNsName.String())
307329
}
308330
uniqueSectionsPerGateway[k] = struct{}{}
309331

310332
sectionNameRefs = append(sectionNameRefs, ParentRef{
311-
Idx: i,
333+
Idx: parentRefIndex,
312334
Gateway: CreateParentRefGateway(gw),
313335
SectionName: p.SectionName,
314336
Port: p.Port,
315337
})
338+
parentRefIndex++
316339
}
317340

318341
return sectionNameRefs, nil

‎internal/controller/state/graph/route_common_test.go

Lines changed: 38 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ func TestBuildSectionNameRefs(t *testing.T) {
2525

2626
gwNsName1 := types.NamespacedName{Namespace: routeNamespace, Name: "gateway-1"}
2727
gwNsName2 := types.NamespacedName{Namespace: routeNamespace, Name: "gateway-2"}
28+
gwNsName3 := types.NamespacedName{Namespace: routeNamespace, Name: "gateway-3"}
2829

2930
parentRefs := []gatewayv1.ParentReference{
3031
{
@@ -51,6 +52,10 @@ func TestBuildSectionNameRefs(t *testing.T) {
5152
Name: gatewayv1.ObjectName("some-other-gateway"),
5253
SectionName: helpers.GetPointer[gatewayv1.SectionName]("same-name"),
5354
},
55+
{
56+
Name: gatewayv1.ObjectName(gwNsName3.Name),
57+
SectionName: nil,
58+
},
5459
}
5560

5661
gws := map[types.NamespacedName]*Gateway{
@@ -70,6 +75,26 @@ func TestBuildSectionNameRefs(t *testing.T) {
7075
},
7176
},
7277
},
78+
gwNsName3: {
79+
Listeners: []*Listener{
80+
{
81+
Source: gatewayv1.Listener{
82+
Name: "http",
83+
},
84+
},
85+
{
86+
Source: gatewayv1.Listener{
87+
Name: "https",
88+
},
89+
},
90+
},
91+
Source: &gatewayv1.Gateway{
92+
ObjectMeta: metav1.ObjectMeta{
93+
Name: gwNsName3.Name,
94+
Namespace: gwNsName3.Namespace,
95+
},
96+
},
97+
},
7398
}
7499

75100
expected := []ParentRef{
@@ -79,20 +104,30 @@ func TestBuildSectionNameRefs(t *testing.T) {
79104
SectionName: parentRefs[0].SectionName,
80105
},
81106
{
82-
Idx: 2,
107+
Idx: 1,
83108
Gateway: CreateParentRefGateway(gws[gwNsName2]),
84109
SectionName: parentRefs[2].SectionName,
85110
},
86111
{
87-
Idx: 3,
112+
Idx: 2,
88113
Gateway: CreateParentRefGateway(gws[gwNsName1]),
89114
SectionName: parentRefs[3].SectionName,
90115
},
91116
{
92-
Idx: 4,
117+
Idx: 3,
93118
Gateway: CreateParentRefGateway(gws[gwNsName2]),
94119
SectionName: parentRefs[4].SectionName,
95120
},
121+
{
122+
Idx: 4,
123+
Gateway: CreateParentRefGateway(gws[gwNsName3]),
124+
SectionName: helpers.GetPointer[gatewayv1.SectionName]("http"),
125+
},
126+
{
127+
Idx: 5,
128+
Gateway: CreateParentRefGateway(gws[gwNsName3]),
129+
SectionName: helpers.GetPointer[gatewayv1.SectionName]("https"),
130+
},
96131
}
97132

98133
tests := []struct {
@@ -121,20 +156,6 @@ func TestBuildSectionNameRefs(t *testing.T) {
121156
name: "duplicate sectionNames",
122157
expectedError: errors.New("duplicate section name \"http\" for Gateway test/gateway-1"),
123158
},
124-
{
125-
parentRefs: []gatewayv1.ParentReference{
126-
{
127-
Name: gatewayv1.ObjectName(gwNsName1.Name),
128-
SectionName: nil,
129-
},
130-
{
131-
Name: gatewayv1.ObjectName(gwNsName1.Name),
132-
SectionName: nil,
133-
},
134-
},
135-
name: "nil sectionNames",
136-
expectedError: errors.New("duplicate section name \"\" for Gateway test/gateway-1"),
137-
},
138159
}
139160

140161
for _, test := range tests {

0 commit comments

Comments
 (0)