Handle non-service backends

- skip processing backend when service is nil - return error when a non-service backend is specified on a GCE Ingress
kubernetes · Apr 30, 2021 · 1dbe4ba · 1dbe4ba
1 parent 284f3ac
commit 1dbe4ba
Show file tree

Hide file tree

Showing 7 changed files with 185 additions and 7 deletions.
diff --git a/pkg/controller/translator/testdata/ingress-null-service-backend.json b/pkg/controller/translator/testdata/ingress-null-service-backend.json
@@ -0,0 +1,34 @@
+{
+	"DefaultBackend": {
+		"ID": {
+			"Service": {
+				"Namespace": "kube-system",
+				"Name": "default-http-backend"
+			},
+			"Port": {
+        "Name": "http"
+      }
+		}
+	},
+	"HostRules": [
+		{
+			"HostName": "foo.bar.com",
+			"Paths": [
+				{
+					"Path": "/testpath",
+					"Backend": {
+						"ID": {
+							"Service": {
+								"Namespace": "default",
+								"Name": "first-service"
+							},
+							"Port": {
+                "Number": 80
+              }
+						}
+					}
+				}
+			]
+		}
+	]
+}
diff --git a/pkg/controller/translator/testdata/ingress-null-service-backend.yaml b/pkg/controller/translator/testdata/ingress-null-service-backend.yaml
@@ -0,0 +1,18 @@
+apiVersion: networking.k8s.io/v1
+kind: Ingress
+metadata:
+  name: test-ingress
+  namespace: default
+spec:
+  rules:
+  - host: foo.bar.com
+    http:
+      paths:
+      - path: /empty
+        backend:
+      - path: /testpath
+        backend:
+          service:
+            name: first-service
+            port:
+              number: 80
diff --git a/pkg/controller/translator/translator.go b/pkg/controller/translator/translator.go
@@ -207,7 +207,13 @@ func (t *Translator) TranslateIngress(ing *v1.Ingress, systemDefaultBackend util
 
 		pathRules := []utils.PathRule{}
 		for _, p := range rule.HTTP.Paths {
-			svcPort, err := t.getServicePort(utils.BackendToServicePortID(p.Backend, ing.Namespace), params, namer)
+			svcPortID, err := utils.BackendToServicePortID(p.Backend, ing.Namespace)
+			if err != nil {
+				// Only error possible is Backend is not a Service Backend, so move to next path
+				errs = append(errs, err)
+				continue
+			}
+			svcPort, err := t.getServicePort(svcPortID, params, namer)
 			if err != nil {
 				errs = append(errs, err)
 			}
@@ -238,7 +244,12 @@ func (t *Translator) TranslateIngress(ing *v1.Ingress, systemDefaultBackend util
 	}
 
 	if ing.Spec.DefaultBackend != nil {
-		svcPort, err := t.getServicePort(utils.BackendToServicePortID(*ing.Spec.DefaultBackend, ing.Namespace), params, namer)
+		svcPortID, err := utils.BackendToServicePortID(*ing.Spec.DefaultBackend, ing.Namespace)
+		if err != nil {
+			errs = append(errs, err)
+			return urlMap, errs
+		}
+		svcPort, err := t.getServicePort(svcPortID, params, namer)
 		if err == nil {
 			urlMap.DefaultBackend = svcPort
 			return urlMap, errs

diff --git a/pkg/controller/translator/translator_test.go b/pkg/controller/translator/translator_test.go
@@ -170,6 +170,21 @@ func TestTranslateIngress(t *testing.T) {
 			wantErrCount:  1,
 			wantGCEURLMap: utils.NewGCEURLMap(),
 		},
+		{
+			desc: "null service default backend",
+			ing: test.NewIngress(types.NamespacedName{Name: "my-ingress", Namespace: "default"},
+				v1.IngressSpec{
+					DefaultBackend: &v1.IngressBackend{},
+				}),
+			wantErrCount:  1,
+			wantGCEURLMap: utils.NewGCEURLMap(),
+		},
+		{
+			desc:          "null service backend",
+			ing:           ingressFromFile(t, "ingress-null-service-backend.yaml"),
+			wantErrCount:  1,
+			wantGCEURLMap: gceURLMapFromFile(t, "ingress-null-service-backend.json"),
+		},
 	}
 
 	for _, tc := range cases {

diff --git a/pkg/utils/serviceport.go b/pkg/utils/serviceport.go
@@ -78,14 +78,17 @@ func (sp ServicePort) IGName() string {
 }
 
 // BackendToServicePortID creates a ServicePortID from a given IngressBackend and namespace.
-func BackendToServicePortID(be v1.IngressBackend, namespace string) ServicePortID {
+func BackendToServicePortID(be v1.IngressBackend, namespace string) (ServicePortID, error) {
+	if be.Service == nil {
+		return ServicePortID{}, fmt.Errorf("Ingress Backend is not a service")
+	}
 	return ServicePortID{
 		Service: types.NamespacedName{
 			Name:      be.Service.Name,
 			Namespace: namespace,
 		},
 		Port: be.Service.Port,
-	}
+	}, nil
 }
 
 // NewServicePortWithID returns a ServicePort with only ID.

diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go
@@ -443,7 +443,7 @@ func TraverseIngressBackends(ing *networkingv1.Ingress, process func(id ServiceP
 		return
 	}
 	// Check service of default backend
-	if ing.Spec.DefaultBackend != nil {
+	if ing.Spec.DefaultBackend != nil && ing.Spec.DefaultBackend.Service != nil {
 		if process(ServicePortID{Service: types.NamespacedName{Namespace: ing.Namespace, Name: ing.Spec.DefaultBackend.Service.Name}, Port: ing.Spec.DefaultBackend.Service.Port}) {
 			return
 		}
@@ -455,8 +455,10 @@ func TraverseIngressBackends(ing *networkingv1.Ingress, process func(id ServiceP
 			continue
 		}
 		for _, p := range rule.IngressRuleValue.HTTP.Paths {
-			if process(ServicePortID{Service: types.NamespacedName{Namespace: ing.Namespace, Name: p.Backend.Service.Name}, Port: p.Backend.Service.Port}) {
-				return
+			if p.Backend.Service != nil {
+				if process(ServicePortID{Service: types.NamespacedName{Namespace: ing.Namespace, Name: p.Backend.Service.Name}, Port: p.Backend.Service.Port}) {
+					return
+				}
 			}
 		}
 	}

diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go
@@ -19,6 +19,7 @@ package utils
 import (
 	"errors"
 	"fmt"
+	"reflect"
 	"testing"
 	"time"
 
@@ -432,6 +433,49 @@ func TestTraverseIngressBackends(t *testing.T) {
 				},
 			},
 		},
+		{
+			"non service backend",
+			&networkingv1.Ingress{
+				Spec: networkingv1.IngressSpec{
+					Rules: []networkingv1.IngressRule{
+						{
+							Host: "foo.bar",
+							IngressRuleValue: networkingv1.IngressRuleValue{
+								HTTP: &networkingv1.HTTPIngressRuleValue{
+									Paths: []networkingv1.HTTPIngressPath{
+										{
+											Path: "/foo",
+											Backend: networkingv1.IngressBackend{
+												Service: &networkingv1.IngressServiceBackend{
+													Name: "foo-service",
+													Port: networkingv1.ServiceBackendPort{
+														Number: 80,
+													},
+												},
+											},
+										},
+										{
+											Path:    "/non-service",
+											Backend: networkingv1.IngressBackend{},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			[]networkingv1.IngressBackend{
+				{
+					Service: &networkingv1.IngressServiceBackend{
+						Name: "foo-service",
+						Port: networkingv1.ServiceBackendPort{
+							Number: 80,
+						},
+					},
+				},
+			},
+		},
 	}
 
 	for _, tc := range testCases {
@@ -900,3 +944,54 @@ func TestIsHTTPErrorCode(t *testing.T) {
 		}
 	}
 }
+
+func TestBackendToServicePortID(t *testing.T) {
+	testNS := "test-namespace"
+	for _, tc := range []struct {
+		desc      string
+		backend   networkingv1.IngressBackend
+		expectErr bool
+	}{
+		{
+			desc: "service is populated",
+			backend: networkingv1.IngressBackend{
+				Service: &networkingv1.IngressServiceBackend{
+					Name: "my-svc",
+					Port: networkingv1.ServiceBackendPort{
+						Number: 80,
+					},
+				},
+			},
+		},
+		{
+			desc:      "service is nil",
+			backend:   networkingv1.IngressBackend{},
+			expectErr: true,
+		},
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+
+			svcPortID, err := BackendToServicePortID(tc.backend, testNS)
+			if !tc.expectErr && err != nil {
+				t.Errorf("unexpected error: %q", err)
+			} else if tc.expectErr && err == nil {
+				t.Errorf("expected an error, but got none")
+			}
+
+			expectedID := ServicePortID{}
+			if !tc.expectErr {
+				expectedID = ServicePortID{
+					Service: types.NamespacedName{
+						Name:      tc.backend.Service.Name,
+						Namespace: testNS,
+					},
+					Port: tc.backend.Service.Port,
+				}
+			}
+
+			if !reflect.DeepEqual(expectedID, svcPortID) {
+				t.Errorf("expected svc port id to be %+v, but got %+v", expectedID, svcPortID)
+			}
+		})
+	}
+}