Skip to content

Commit 9c180d5

Browse files
committed
Address code review
1 parent 451f884 commit 9c180d5

File tree

5 files changed

+58
-50
lines changed

5 files changed

+58
-50
lines changed

internal/mode/static/state/graph/backend_refs_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1009,7 +1009,6 @@ func TestCreateBackend(t *testing.T) {
10091009
policies := map[types.NamespacedName]*BackendTLSPolicy{
10101010
client.ObjectKeyFromObject(btp.Source): &btp,
10111011
client.ObjectKeyFromObject(btp2.Source): &btp2,
1012-
// client.ObjectKeyFromObject(btpDupe.Source): &btpDupe,
10131012
}
10141013

10151014
refPath := field.NewPath("test")

internal/mode/static/state/graph/gateway.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import (
1212
staticConds "github.com/nginx/nginx-gateway-fabric/internal/mode/static/state/conditions"
1313
)
1414

15-
// Gateway represents the a Gateway resource.
15+
// Gateway represents a Gateway resource.
1616
type Gateway struct {
1717
// LatestReloadResult is the result of the last nginx reload attempt.
1818
LatestReloadResult NginxReloadResult
@@ -36,7 +36,7 @@ type Gateway struct {
3636
Valid bool
3737
}
3838

39-
// processGateways determines which Gateway resource belong to NGF (determined by the Gateway GatewayClassName field).
39+
// processGateways determines which Gateway resources belong to NGF (determined by the Gateway GatewayClassName field).
4040
func processGateways(
4141
gws map[types.NamespacedName]*v1.Gateway,
4242
gcName string,

internal/mode/static/state/graph/gateway_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ func TestBuildGateway(t *testing.T) {
221221
foo8081Listener := createHTTPListener("foo-8081", "foo.example.com", 8081)
222222
foo443HTTPListener := createHTTPListener("foo-443-http", "foo.example.com", 443)
223223

224-
// // foo https listeners
224+
// foo https listeners
225225
foo80HTTPSListener := createHTTPSListener("foo-80-https", "foo.example.com", 80, gatewayTLSConfigSameNs)
226226
foo443HTTPSListener1 := createHTTPSListener("foo-443-https-1", "foo.example.com", 443, gatewayTLSConfigSameNs)
227227
foo8443HTTPSListener := createHTTPSListener("foo-8443-https", "foo.example.com", 8443, gatewayTLSConfigSameNs)

internal/mode/static/state/graph/graph.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ type ClusterState struct {
4545
type Graph struct {
4646
// GatewayClass holds the GatewayClass resource.
4747
GatewayClass *GatewayClass
48-
// Gateway holds the all Gateway resource.
48+
// Gateways holds the all Gateway resource.
4949
Gateways map[types.NamespacedName]*Gateway
5050
// IgnoredGatewayClasses holds the ignored GatewayClass resources, which reference NGINX Gateway Fabric in the
5151
// controllerName, but are not configured via the NGINX Gateway Fabric CLI argument. It doesn't hold the GatewayClass

internal/mode/static/state/graph/multiple_gateways_test.go

Lines changed: 54 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -67,24 +67,29 @@ var (
6767
}
6868
)
6969

70-
func createGateway(name, nginxProxyName string, listeners []gatewayv1.Listener) *gatewayv1.Gateway {
71-
return &gatewayv1.Gateway{
70+
func createGateway(name, namespace, nginxProxyName string, listeners []gatewayv1.Listener) *gatewayv1.Gateway {
71+
gateway := &gatewayv1.Gateway{
7272
ObjectMeta: metav1.ObjectMeta{
73-
Namespace: testNs,
73+
Namespace: namespace,
7474
Name: name,
7575
},
7676
Spec: gatewayv1.GatewaySpec{
7777
GatewayClassName: gcName,
78-
Infrastructure: &gatewayv1.GatewayInfrastructure{
79-
ParametersRef: &gatewayv1.LocalParametersReference{
80-
Group: ngfAPIv1alpha2.GroupName,
81-
Kind: kinds.NginxProxy,
82-
Name: nginxProxyName,
83-
},
84-
},
85-
Listeners: listeners,
78+
Listeners: listeners,
8679
},
8780
}
81+
82+
if nginxProxyName != "" {
83+
gateway.Spec.Infrastructure = &gatewayv1.GatewayInfrastructure{
84+
ParametersRef: &gatewayv1.LocalParametersReference{
85+
Group: ngfAPIv1alpha2.GroupName,
86+
Kind: kinds.NginxProxy,
87+
Name: nginxProxyName,
88+
},
89+
}
90+
}
91+
92+
return gateway
8893
}
8994

9095
func createGatewayClass(name, controllerName, npName, npNamespace string) *gatewayv1.GatewayClass {
@@ -145,7 +150,7 @@ func convertedGateway(
145150
nginxProxy *NginxProxy,
146151
effectiveNp *EffectiveNginxProxy,
147152
listeners []*Listener,
148-
conds ...conditions.Condition,
153+
conds []conditions.Condition,
149154
) *Gateway {
150155
return &Gateway{
151156
Source: gw,
@@ -222,7 +227,7 @@ func Test_MultipleGateways_WithNginxProxy(t *testing.T) {
222227
},
223228
})
224229

225-
nginxProxyGateway3 := createNginxProxy("nginx-proxy-gateway-3", testNs, ngfAPIv1alpha2.NginxProxySpec{
230+
nginxProxyGateway3 := createNginxProxy("nginx-proxy-gateway-3", "test2", ngfAPIv1alpha2.NginxProxySpec{
226231
Kubernetes: &ngfAPIv1alpha2.KubernetesSpec{
227232
Deployment: &ngfAPIv1alpha2.DeploymentSpec{
228233
Replicas: helpers.GetPointer(int32(3)),
@@ -232,12 +237,12 @@ func Test_MultipleGateways_WithNginxProxy(t *testing.T) {
232237
})
233238

234239
gatewayClass := createGatewayClass(gcName, controllerName, "nginx-proxy", testNs)
235-
gateway1 := createGateway("gateway-1", "nginx-proxy", []gatewayv1.Listener{})
236-
gateway2 := createGateway("gateway-2", "nginx-proxy", []gatewayv1.Listener{})
237-
gateway3 := createGateway("gateway-3", "nginx-proxy", []gatewayv1.Listener{})
240+
gateway1 := createGateway("gateway-1", testNs, "", []gatewayv1.Listener{})
241+
gateway2 := createGateway("gateway-2", testNs, "", []gatewayv1.Listener{})
242+
gateway3 := createGateway("gateway-3", "test2", "", []gatewayv1.Listener{})
238243

239-
gateway1withNP := createGateway("gateway-1", "nginx-proxy-gateway-1", []gatewayv1.Listener{})
240-
gateway3withNP := createGateway("gateway-3", "nginx-proxy-gateway-3", []gatewayv1.Listener{})
244+
gateway1withNP := createGateway("gateway-1", testNs, "nginx-proxy-gateway-1", []gatewayv1.Listener{})
245+
gateway3withNP := createGateway("gateway-3", "test2", "nginx-proxy-gateway-3", []gatewayv1.Listener{})
241246

242247
gcConditions := []conditions.Condition{staticConds.NewGatewayClassResolvedRefs()}
243248

@@ -269,24 +274,24 @@ func Test_MultipleGateways_WithNginxProxy(t *testing.T) {
269274
Gateways: map[types.NamespacedName]*Gateway{
270275
client.ObjectKeyFromObject(gateway1): convertedGateway(
271276
gateway1,
272-
&NginxProxy{Source: nginxProxyGlobal, Valid: true},
277+
nil,
273278
&EffectiveNginxProxy{DisableHTTP2: helpers.GetPointer(true)},
274279
[]*Listener{},
275-
gcConditions...,
280+
nil,
276281
),
277282
client.ObjectKeyFromObject(gateway2): convertedGateway(
278283
gateway2,
279-
&NginxProxy{Source: nginxProxyGlobal, Valid: true},
284+
nil,
280285
&EffectiveNginxProxy{DisableHTTP2: helpers.GetPointer(true)},
281286
[]*Listener{},
282-
gcConditions...,
287+
nil,
283288
),
284289
client.ObjectKeyFromObject(gateway3): convertedGateway(
285290
gateway3,
286-
&NginxProxy{Source: nginxProxyGlobal, Valid: true},
291+
nil,
287292
&EffectiveNginxProxy{DisableHTTP2: helpers.GetPointer(true)},
288293
[]*Listener{},
289-
gcConditions...,
294+
nil,
290295
),
291296
},
292297
ReferencedNginxProxies: map[types.NamespacedName]*NginxProxy{
@@ -307,9 +312,9 @@ func Test_MultipleGateways_WithNginxProxy(t *testing.T) {
307312
client.ObjectKeyFromObject(gatewayClass): gatewayClass,
308313
},
309314
Gateways: map[types.NamespacedName]*gatewayv1.Gateway{
310-
client.ObjectKeyFromObject(gateway1): gateway1withNP,
311-
client.ObjectKeyFromObject(gateway2): gateway2,
312-
client.ObjectKeyFromObject(gateway3): gateway3withNP,
315+
client.ObjectKeyFromObject(gateway1withNP): gateway1withNP,
316+
client.ObjectKeyFromObject(gateway2): gateway2,
317+
client.ObjectKeyFromObject(gateway3withNP): gateway3withNP,
313318
},
314319
NginxProxies: map[types.NamespacedName]*ngfAPIv1alpha2.NginxProxy{
315320
client.ObjectKeyFromObject(nginxProxyGlobal): nginxProxyGlobal,
@@ -334,14 +339,14 @@ func Test_MultipleGateways_WithNginxProxy(t *testing.T) {
334339
DisableHTTP2: helpers.GetPointer(true),
335340
},
336341
[]*Listener{},
337-
gcConditions...,
342+
gcConditions,
338343
),
339344
client.ObjectKeyFromObject(gateway2): convertedGateway(
340345
gateway2,
341-
&NginxProxy{Source: nginxProxyGlobal, Valid: true},
346+
nil,
342347
&EffectiveNginxProxy{DisableHTTP2: helpers.GetPointer(true)},
343348
[]*Listener{},
344-
gcConditions...,
349+
nil,
345350
),
346351
client.ObjectKeyFromObject(gateway3withNP): convertedGateway(
347352
gateway3withNP,
@@ -355,7 +360,7 @@ func Test_MultipleGateways_WithNginxProxy(t *testing.T) {
355360
DisableHTTP2: helpers.GetPointer(false),
356361
},
357362
[]*Listener{},
358-
gcConditions...,
363+
gcConditions,
359364
),
360365
},
361366
ReferencedNginxProxies: map[types.NamespacedName]*NginxProxy{
@@ -457,7 +462,7 @@ func Test_MultipleGateways_WithListeners(t *testing.T) {
457462
},
458463
}
459464

460-
gateway1 := createGateway("gateway-1", "nginx-proxy", []gatewayv1.Listener{
465+
gateway1 := createGateway("gateway-1", testNs, "nginx-proxy", []gatewayv1.Listener{
461466
createListener(
462467
"listener-tls-mode-terminate",
463468
"*.example.com",
@@ -467,7 +472,7 @@ func Test_MultipleGateways_WithListeners(t *testing.T) {
467472
allowedRoutesHTTPGRPC,
468473
),
469474
})
470-
gateway2 := createGateway("gateway-2", "nginx-proxy", []gatewayv1.Listener{
475+
gateway2 := createGateway("gateway-2", testNs, "nginx-proxy", []gatewayv1.Listener{
471476
createListener(
472477
"listener-tls-mode-terminate",
473478
"*.example.com",
@@ -532,12 +537,14 @@ func Test_MultipleGateways_WithListeners(t *testing.T) {
532537
allowedRoutesTLS,
533538
),
534539
}
535-
gatewayMultipleListeners1 := createGateway("gateway-multiple-listeners-1", "nginx-proxy", listeners)
536-
gatewayMultipleListeners2 := createGateway("gateway-multiple-listeners-2", "nginx-proxy", listeners)
537-
gatewayMultipleListeners3 := createGateway("gateway-multiple-listeners-3", "nginx-proxy", listeners)
540+
gatewayMultipleListeners1 := createGateway("gateway-multiple-listeners-1", testNs, "nginx-proxy", listeners)
541+
gatewayMultipleListeners2 := createGateway("gateway-multiple-listeners-2", testNs, "nginx-proxy", listeners)
542+
gatewayMultipleListeners3 := createGateway("gateway-multiple-listeners-3", testNs, "nginx-proxy", listeners)
538543

539544
// valid TLS and https listener same port and hostname
540-
gatewayTLSSamePortHostname := createGateway("gateway-tls-foo",
545+
gatewayTLSSamePortHostname := createGateway(
546+
"gateway-tls-foo",
547+
testNs,
541548
"nginx-proxy",
542549
[]gatewayv1.Listener{
543550
createListener(
@@ -551,7 +558,9 @@ func Test_MultipleGateways_WithListeners(t *testing.T) {
551558
},
552559
)
553560

554-
gatewayHTTPSSamePortHostname := createGateway("gateway-http-foo",
561+
gatewayHTTPSSamePortHostname := createGateway(
562+
"gateway-http-foo",
563+
testNs,
555564
"nginx-proxy",
556565
[]gatewayv1.Listener{
557566
createListener(
@@ -608,7 +617,7 @@ func Test_MultipleGateways_WithListeners(t *testing.T) {
608617
map[L4RouteKey]*L4Route{},
609618
),
610619
},
611-
staticConds.NewGatewayClassResolvedRefs(),
620+
[]conditions.Condition{staticConds.NewGatewayClassResolvedRefs()},
612621
),
613622
client.ObjectKeyFromObject(gateway2): convertedGateway(
614623
gateway2,
@@ -624,7 +633,7 @@ func Test_MultipleGateways_WithListeners(t *testing.T) {
624633
map[L4RouteKey]*L4Route{},
625634
),
626635
},
627-
staticConds.NewGatewayClassResolvedRefs(),
636+
[]conditions.Condition{staticConds.NewGatewayClassResolvedRefs()},
628637
),
629638
},
630639
Routes: map[RouteKey]*L7Route{},
@@ -697,7 +706,7 @@ func Test_MultipleGateways_WithListeners(t *testing.T) {
697706
map[L4RouteKey]*L4Route{},
698707
),
699708
},
700-
staticConds.NewGatewayClassResolvedRefs(),
709+
[]conditions.Condition{staticConds.NewGatewayClassResolvedRefs()},
701710
),
702711
client.ObjectKeyFromObject(gatewayMultipleListeners2): convertedGateway(
703712
gatewayMultipleListeners2,
@@ -729,7 +738,7 @@ func Test_MultipleGateways_WithListeners(t *testing.T) {
729738
map[L4RouteKey]*L4Route{},
730739
),
731740
},
732-
staticConds.NewGatewayClassResolvedRefs(),
741+
[]conditions.Condition{staticConds.NewGatewayClassResolvedRefs()},
733742
),
734743
client.ObjectKeyFromObject(gatewayMultipleListeners3): convertedGateway(
735744
gatewayMultipleListeners3,
@@ -761,7 +770,7 @@ func Test_MultipleGateways_WithListeners(t *testing.T) {
761770
map[L4RouteKey]*L4Route{},
762771
),
763772
},
764-
staticConds.NewGatewayClassResolvedRefs(),
773+
[]conditions.Condition{staticConds.NewGatewayClassResolvedRefs()},
765774
),
766775
},
767776
Routes: map[RouteKey]*L7Route{},
@@ -816,7 +825,7 @@ func Test_MultipleGateways_WithListeners(t *testing.T) {
816825
map[L4RouteKey]*L4Route{},
817826
),
818827
},
819-
staticConds.NewGatewayClassResolvedRefs(),
828+
[]conditions.Condition{staticConds.NewGatewayClassResolvedRefs()},
820829
),
821830
client.ObjectKeyFromObject(gatewayHTTPSSamePortHostname): convertedGateway(
822831
gatewayHTTPSSamePortHostname,
@@ -832,7 +841,7 @@ func Test_MultipleGateways_WithListeners(t *testing.T) {
832841
map[L4RouteKey]*L4Route{},
833842
),
834843
},
835-
staticConds.NewGatewayClassResolvedRefs(),
844+
[]conditions.Condition{staticConds.NewGatewayClassResolvedRefs()},
836845
),
837846
},
838847
Routes: map[RouteKey]*L7Route{},

0 commit comments

Comments
 (0)