Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@salonichf5
salonichf5 committed Jul 17, 2024
1 parent 9ef4868 commit e802e82
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 150 deletions.
212 changes: 90 additions & 122 deletions internal/mode/static/state/dataplane/configuration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,6 @@ func getNormalBackendRef() graph.BackendRef {
}
}

func getModifiedBackendRef(mod func(ref graph.BackendRef) graph.BackendRef) graph.BackendRef {
return mod(getNormalBackendRef())
}

func getExpectedConfiguration() Configuration {
return Configuration{
BaseHTTPConfig: BaseHTTPConfig{HTTP2: true, IPFamily: Dual},
Expand Down Expand Up @@ -183,9 +179,7 @@ func TestBuildConfiguration(t *testing.T) {
fakeResolver := &resolverfakes.FakeServiceResolver{}
fakeResolver.ResolveReturns(fooEndpoints, nil)

validBackendRef := getModifiedBackendRef(func(backend graph.BackendRef) graph.BackendRef {
return backend
})
validBackendRef := getNormalBackendRef()

expValidBackend := Backend{
UpstreamName: fooUpstreamName,
Expand Down Expand Up @@ -837,14 +831,12 @@ func TestBuildConfiguration(t *testing.T) {
},
{
graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph {
g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{
{
Name: "invalid-listener",
Source: invalidListener,
Valid: false,
ResolvedSecret: &secret1NsName,
},
}...)
g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{
Name: "invalid-listener",
Source: invalidListener,
Valid: false,
ResolvedSecret: &secret1NsName,
})
g.Routes = map[graph.RouteKey]*graph.L7Route{
graph.CreateRouteKey(httpsHR1): httpsRouteHR1,
graph.CreateRouteKey(httpsHR2): httpsRouteHR2,
Expand All @@ -862,17 +854,15 @@ func TestBuildConfiguration(t *testing.T) {
},
{
graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph {
g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{
{
Name: "listener-80-1",
Source: listener80,
Valid: true,
Routes: map[graph.RouteKey]*graph.L7Route{
graph.CreateRouteKey(hr1): routeHR1,
graph.CreateRouteKey(hr2): routeHR2,
},
g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{
Name: "listener-80-1",
Source: listener80,
Valid: true,
Routes: map[graph.RouteKey]*graph.L7Route{
graph.CreateRouteKey(hr1): routeHR1,
graph.CreateRouteKey(hr2): routeHR2,
},
}...)
})
g.Routes = map[graph.RouteKey]*graph.L7Route{
graph.CreateRouteKey(hr1): routeHR1,
graph.CreateRouteKey(hr2): routeHR2,
Expand Down Expand Up @@ -925,16 +915,14 @@ func TestBuildConfiguration(t *testing.T) {
},
{
graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph {
g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{
{
Name: "listener-80-1",
Source: listener80,
Valid: true,
Routes: map[graph.RouteKey]*graph.L7Route{
graph.CreateRouteKey(gr): routeGR,
},
g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{
Name: "listener-80-1",
Source: listener80,
Valid: true,
Routes: map[graph.RouteKey]*graph.L7Route{
graph.CreateRouteKey(gr): routeGR,
},
}...)
})
g.Routes[graph.CreateRouteKey(gr)] = routeGR
return g
}),
Expand Down Expand Up @@ -1411,16 +1399,14 @@ func TestBuildConfiguration(t *testing.T) {
{
graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph {
g.GatewayClass.Valid = false
g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{
{
Name: "listener-80-1",
Source: listener80,
Valid: true,
Routes: map[graph.RouteKey]*graph.L7Route{
graph.CreateRouteKey(hr1): routeHR1,
},
g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{
Name: "listener-80-1",
Source: listener80,
Valid: true,
Routes: map[graph.RouteKey]*graph.L7Route{
graph.CreateRouteKey(hr1): routeHR1,
},
}...)
})
g.Routes = map[graph.RouteKey]*graph.L7Route{
graph.CreateRouteKey(hr1): routeHR1,
}
Expand All @@ -1432,16 +1418,14 @@ func TestBuildConfiguration(t *testing.T) {
{
graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph {
g.GatewayClass.Valid = false
g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{
{
Name: "listener-80-1",
Source: listener80,
Valid: true,
Routes: map[graph.RouteKey]*graph.L7Route{
graph.CreateRouteKey(hr1): routeHR1,
},
g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{
Name: "listener-80-1",
Source: listener80,
Valid: true,
Routes: map[graph.RouteKey]*graph.L7Route{
graph.CreateRouteKey(hr1): routeHR1,
},
}...)
})
g.Routes = map[graph.RouteKey]*graph.L7Route{
graph.CreateRouteKey(hr1): routeHR1,
}
Expand All @@ -1460,16 +1444,14 @@ func TestBuildConfiguration(t *testing.T) {
},
{
graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph {
g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{
{
Name: "listener-80-1",
Source: listener80,
Valid: true,
Routes: map[graph.RouteKey]*graph.L7Route{
graph.CreateRouteKey(hr5): routeHR5,
},
g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{
Name: "listener-80-1",
Source: listener80,
Valid: true,
Routes: map[graph.RouteKey]*graph.L7Route{
graph.CreateRouteKey(hr5): routeHR5,
},
}...)
})
g.Routes = map[graph.RouteKey]*graph.L7Route{
graph.CreateRouteKey(hr5): routeHR5,
}
Expand Down Expand Up @@ -1599,16 +1581,14 @@ func TestBuildConfiguration(t *testing.T) {
},
{
graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph {
g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{
{
Name: "listener-80-1",
Source: listener80,
Valid: true,
Routes: map[graph.RouteKey]*graph.L7Route{
graph.CreateRouteKey(hr7): routeHR7,
},
g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{
Name: "listener-80-1",
Source: listener80,
Valid: true,
Routes: map[graph.RouteKey]*graph.L7Route{
graph.CreateRouteKey(hr7): routeHR7,
},
}...)
})
g.Routes = map[graph.RouteKey]*graph.L7Route{
graph.CreateRouteKey(hr7): routeHR7,
}
Expand Down Expand Up @@ -1731,17 +1711,15 @@ func TestBuildConfiguration(t *testing.T) {
},
{
graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph {
g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{
{
Name: "listener-443",
Source: listener443,
Valid: true,
Routes: map[graph.RouteKey]*graph.L7Route{
graph.CreateRouteKey(httpsHR8): httpsRouteHR8,
},
ResolvedSecret: &secret1NsName,
g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{
Name: "listener-443",
Source: listener443,
Valid: true,
Routes: map[graph.RouteKey]*graph.L7Route{
graph.CreateRouteKey(httpsHR8): httpsRouteHR8,
},
}...)
ResolvedSecret: &secret1NsName,
})
g.Routes = map[graph.RouteKey]*graph.L7Route{
graph.CreateRouteKey(httpsHR8): httpsRouteHR8,
}
Expand Down Expand Up @@ -1792,17 +1770,15 @@ func TestBuildConfiguration(t *testing.T) {
},
{
graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph {
g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{
{
Name: "listener-443",
Source: listener443,
Valid: true,
Routes: map[graph.RouteKey]*graph.L7Route{
graph.CreateRouteKey(httpsHR9): httpsRouteHR9,
},
ResolvedSecret: &secret1NsName,
g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{
Name: "listener-443",
Source: listener443,
Valid: true,
Routes: map[graph.RouteKey]*graph.L7Route{
graph.CreateRouteKey(httpsHR9): httpsRouteHR9,
},
}...)
ResolvedSecret: &secret1NsName,
})
g.Routes = map[graph.RouteKey]*graph.L7Route{
graph.CreateRouteKey(httpsHR9): httpsRouteHR9,
}
Expand Down Expand Up @@ -1857,14 +1833,12 @@ func TestBuildConfiguration(t *testing.T) {
Name: "gw",
Namespace: "ns",
}
g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{
{
Name: "listener-80-1",
Source: listener80,
Valid: true,
Routes: map[graph.RouteKey]*graph.L7Route{},
},
}...)
g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{
Name: "listener-80-1",
Source: listener80,
Valid: true,
Routes: map[graph.RouteKey]*graph.L7Route{},
})
g.NginxProxy = nginxProxy
return g
}),
Expand All @@ -1890,14 +1864,12 @@ func TestBuildConfiguration(t *testing.T) {
Name: "gw",
Namespace: "ns",
}
g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{
{
Name: "listener-80-1",
Source: listener80,
Valid: true,
Routes: map[graph.RouteKey]*graph.L7Route{},
},
}...)
g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{
Name: "listener-80-1",
Source: listener80,
Valid: true,
Routes: map[graph.RouteKey]*graph.L7Route{},
})
g.NginxProxy = &graph.NginxProxy{
Valid: false,
Source: &ngfAPI.NginxProxy{
Expand Down Expand Up @@ -2077,14 +2049,12 @@ func TestBuildConfiguration(t *testing.T) {
Name: "gw",
Namespace: "ns",
}
g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{
{
Name: "listener-80-1",
Source: listener80,
Valid: true,
Routes: map[graph.RouteKey]*graph.L7Route{},
},
}...)
g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{
Name: "listener-80-1",
Source: listener80,
Valid: true,
Routes: map[graph.RouteKey]*graph.L7Route{},
})
g.NginxProxy = nginxProxyIPv4
return g
}),
Expand All @@ -2102,14 +2072,12 @@ func TestBuildConfiguration(t *testing.T) {
Name: "gw",
Namespace: "ns",
}
g.Gateway.Listeners = append(g.Gateway.Listeners, []*graph.Listener{
{
Name: "listener-80-1",
Source: listener80,
Valid: true,
Routes: map[graph.RouteKey]*graph.L7Route{},
},
}...)
g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{
Name: "listener-80-1",
Source: listener80,
Valid: true,
Routes: map[graph.RouteKey]*graph.L7Route{},
})
g.NginxProxy = nginxProxyIPv6
return g
}),
Expand Down
20 changes: 8 additions & 12 deletions internal/mode/static/state/graph/backend_refs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ func createBackendRef(
ns = string(*ref.Namespace)
}
svcNsName := types.NamespacedName{Name: string(ref.BackendRef.Name), Namespace: ns}
svcIPFamily, svcPort, err := getServiceAndPortFromRef(ref.BackendRef, sourceNamespace, services, refPath)
svcIPFamily, svcPort, err := getIPFamilyAndPortFromRef(ref.BackendRef, svcNsName, services, refPath)
if err != nil {
backendRef = BackendRef{
Weight: weight,
Expand Down Expand Up @@ -293,22 +293,16 @@ func findBackendTLSPolicyForService(
return beTLSPolicy, err
}

// getServiceAndPortFromRef extracts the NamespacedName of the Service and the port from a BackendRef.
// getIPFamilyAndPortFromRef extracts the IPFamily of the Service and the port from a BackendRef.
// It can return an error and an empty v1.ServicePort in two cases:
// 1. The Service referenced from the BackendRef does not exist in the cluster/state.
// 2. The Port on the BackendRef does not match any of the ServicePorts on the Service.
func getServiceAndPortFromRef(
func getIPFamilyAndPortFromRef(
ref gatewayv1.BackendRef,
routeNamespace string,
svcNsName types.NamespacedName,
services map[types.NamespacedName]*v1.Service,
refPath *field.Path,
) ([]v1.IPFamily, v1.ServicePort, error) {
ns := routeNamespace
if ref.Namespace != nil {
ns = string(*ref.Namespace)
}

svcNsName := types.NamespacedName{Name: string(ref.Name), Namespace: ns}
svc, ok := services[svcNsName]
if !ok {
return []v1.IPFamily{}, v1.ServicePort{}, field.NotFound(refPath.Child("name"), ref.Name)
Expand All @@ -334,14 +328,16 @@ func verifyIPFamily(npCfg *NginxProxy, svcIPFamily []v1.IPFamily) error {
if slices.Contains(svcIPFamily, v1.IPv6Protocol) {
// capitalizing error message to match the rest of the error messages associated with a condition
return errors.New( //nolint: stylecheck
"Service configured with IPv6 family but NginxProxy is configured with IPv4")
"Service configured with IPv6 family but NginxProxy is configured with IPv4",
)
}
}
if *npIPFamily == ngfAPI.IPv6 {
if slices.Contains(svcIPFamily, v1.IPv4Protocol) {
// capitalizing error message to match the rest of the error messages associated with a condition
return errors.New( //nolint: stylecheck
"Service configured with IPv4 family but NginxProxy is configured with IPv6")
"Service configured with IPv4 family but NginxProxy is configured with IPv6",
)
}
}

Expand Down
Loading

0 comments on commit e802e82

Please sign in to comment.