Skip to content

Commit

Permalink
move service name creation out of function
Browse files Browse the repository at this point in the history
  • Loading branch information
salonichf5 committed Jul 17, 2024
1 parent b52b90d commit 9ef4868
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 37 deletions.
29 changes: 15 additions & 14 deletions internal/mode/static/state/graph/backend_refs.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,8 @@ import (
"slices"

v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/validation/field"
"sigs.k8s.io/controller-runtime/pkg/client"
gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
"sigs.k8s.io/gateway-api/apis/v1alpha3"

Expand Down Expand Up @@ -151,20 +149,25 @@ func createBackendRef(
return backendRef, &cond
}

svc, svcPort, err := getServiceAndPortFromRef(ref.BackendRef, sourceNamespace, services, refPath)
ns := sourceNamespace
if ref.BackendRef.Namespace != nil {
ns = string(*ref.Namespace)
}
svcNsName := types.NamespacedName{Name: string(ref.BackendRef.Name), Namespace: ns}
svcIPFamily, svcPort, err := getServiceAndPortFromRef(ref.BackendRef, sourceNamespace, services, refPath)
if err != nil {
backendRef = BackendRef{
Weight: weight,
Valid: false,
Weight: weight,
Valid: false,
SvcNsName: svcNsName,
ServicePort: v1.ServicePort{},
}

cond := staticConds.NewRouteBackendRefRefBackendNotFound(err.Error())
return backendRef, &cond
}

svcNsName := client.ObjectKeyFromObject(svc)

if err := verifyIPFamily(npCfg, svc.Spec.IPFamilies); err != nil {
if err := verifyIPFamily(npCfg, svcIPFamily); err != nil {
backendRef = BackendRef{
SvcNsName: svcNsName,
ServicePort: svcPort,
Expand Down Expand Up @@ -299,7 +302,7 @@ func getServiceAndPortFromRef(
routeNamespace string,
services map[types.NamespacedName]*v1.Service,
refPath *field.Path,
) (*v1.Service, v1.ServicePort, error) {
) ([]v1.IPFamily, v1.ServicePort, error) {
ns := routeNamespace
if ref.Namespace != nil {
ns = string(*ref.Namespace)
Expand All @@ -308,18 +311,16 @@ func getServiceAndPortFromRef(
svcNsName := types.NamespacedName{Name: string(ref.Name), Namespace: ns}
svc, ok := services[svcNsName]
if !ok {
return &v1.Service{ObjectMeta: metav1.ObjectMeta{Namespace: ns, Name: string(ref.Name)}},
v1.ServicePort{},
field.NotFound(refPath.Child("name"), ref.Name)
return []v1.IPFamily{}, v1.ServicePort{}, field.NotFound(refPath.Child("name"), ref.Name)
}

// safe to dereference port here because we already validated that the port is not nil in validateBackendRef.
svcPort, err := getServicePort(svc, int32(*ref.Port))
if err != nil {
return &v1.Service{ObjectMeta: metav1.ObjectMeta{Namespace: ns, Name: string(ref.Name)}}, v1.ServicePort{}, err
return []v1.IPFamily{}, v1.ServicePort{}, err
}

return svc, svcPort, nil
return svc.Spec.IPFamilies, svcPort, nil
}

func verifyIPFamily(npCfg *NginxProxy, svcIPFamily []v1.IPFamily) error {
Expand Down
45 changes: 22 additions & 23 deletions internal/mode/static/state/graph/backend_refs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,10 +271,6 @@ func TestGetServiceAndPortFromRef(t *testing.T) {
IPFamilies: []v1.IPFamily{v1.IPv4Protocol},
},
}
svc1NsName := types.NamespacedName{
Namespace: "test",
Name: "service1",
}

svc2 := &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -284,49 +280,48 @@ func TestGetServiceAndPortFromRef(t *testing.T) {
}

tests := []struct {
expSvc *v1.Service
ref gatewayv1.BackendRef
expServiceNsName types.NamespacedName
name string
expServicePort v1.ServicePort
expErr bool
expSvc *v1.Service
ref gatewayv1.BackendRef
expSvcIPFamily []v1.IPFamily
name string
expServicePort v1.ServicePort
expErr bool
}{
{
name: "normal case",
ref: getNormalRef(),
expServiceNsName: svc1NsName,
expServicePort: v1.ServicePort{Port: 80},
expSvc: svc1,
name: "normal case",
ref: getNormalRef(),
expServicePort: v1.ServicePort{Port: 80},
expSvcIPFamily: []v1.IPFamily{v1.IPv4Protocol},
},
{
name: "service does not exist",
ref: getModifiedRef(func(backend gatewayv1.BackendRef) gatewayv1.BackendRef {
backend.Name = "does-not-exist"
return backend
}),
expErr: true,
expServiceNsName: types.NamespacedName{Name: "does-not-exist", Namespace: "test"},
expServicePort: v1.ServicePort{},
expErr: true,
expServicePort: v1.ServicePort{},
expSvc: &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "does-not-exist", Namespace: "test",
},
},
expSvcIPFamily: []v1.IPFamily{},
},
{
name: "no matching port for service and port",
ref: getModifiedRef(func(backend gatewayv1.BackendRef) gatewayv1.BackendRef {
backend.Port = helpers.GetPointer[gatewayv1.PortNumber](504)
return backend
}),
expErr: true,
expServiceNsName: svc1NsName,
expServicePort: v1.ServicePort{},
expErr: true,
expServicePort: v1.ServicePort{},
expSvc: &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "service1", Namespace: "test",
},
},
expSvcIPFamily: []v1.IPFamily{},
},
}

Expand All @@ -341,11 +336,11 @@ func TestGetServiceAndPortFromRef(t *testing.T) {
t.Run(test.name, func(t *testing.T) {
g := NewWithT(t)

svc, servicePort, err := getServiceAndPortFromRef(test.ref, "test", services, refPath)
svcIPFamily, servicePort, err := getServiceAndPortFromRef(test.ref, "test", services, refPath)

g.Expect(err != nil).To(Equal(test.expErr))
g.Expect(servicePort).To(Equal(test.expServicePort))
g.Expect(svc).To(Equal(test.expSvc))
g.Expect(svcIPFamily).To(Equal(test.expSvcIPFamily))
})
}
}
Expand Down Expand Up @@ -953,6 +948,10 @@ func TestCreateBackend(t *testing.T) {
expectedBackend: BackendRef{
Weight: 5,
Valid: false,
SvcNsName: types.NamespacedName{
Namespace: "test",
Name: "not-exist",
},
},
expectedServicePortReference: "",
expectedCondition: helpers.GetPointer(
Expand Down

0 comments on commit 9ef4868

Please sign in to comment.