Fix crash on ingress without default backend service. (#1870)

Signed-off-by: Alexander Wels <awels@redhat.com>
kubevirt · Jul 23, 2021 · e3eb246 · e3eb246
1 parent c1aad16
commit e3eb246
Show file tree

Hide file tree

Showing 3 changed files with 85 additions and 3 deletions.
diff --git a/pkg/controller/config-controller.go b/pkg/controller/config-controller.go
@@ -596,7 +596,7 @@ func watchClusterProxy(mgr manager.Manager, configController controller.Controll
 }
 
 func getURLFromIngress(ing *networkingv1.Ingress, uploadProxyServiceName string) string {
-	if ing.Spec.DefaultBackend != nil {
+	if ing.Spec.DefaultBackend != nil && ing.Spec.DefaultBackend.Service != nil {
 		if ing.Spec.DefaultBackend.Service.Name != uploadProxyServiceName {
 			return ""
 		}
@@ -607,15 +607,14 @@ func getURLFromIngress(ing *networkingv1.Ingress, uploadProxyServiceName string)
 			continue
 		}
 		for _, path := range rule.HTTP.Paths {
-			if path.Backend.Service.Name == uploadProxyServiceName {
+			if path.Backend.Service != nil && path.Backend.Service.Name == uploadProxyServiceName {
 				if rule.Host != "" {
 					return rule.Host
 				}
 			}
 		}
 	}
 	return ""
-
 }
 
 func getURLFromRoute(route *routev1.Route, uploadProxyServiceName string) string {

diff --git a/pkg/controller/config-controller_test.go b/pkg/controller/config-controller_test.go
@@ -172,6 +172,22 @@ var _ = Describe("Controller ingress reconcile loop", func() {
 		Expect(err).ToNot(HaveOccurred())
 		Expect(*cdiConfig.Status.UploadProxyURL).To(Equal(testURL))
 	})
+
+	DescribeTable("Should not set proxyURL if invalid ingress exists", func(createIngress func(name, ns, service, url string) *networkingv1.Ingress) {
+		reconciler, cdiConfig := createConfigReconciler(createIngressList(
+			*createIngress("test-ingress", "test-ns", "service", testURL),
+		))
+		reconciler.uploadProxyServiceName = testServiceName
+		err := reconciler.reconcileIngress(cdiConfig)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(cdiConfig).ToNot(BeNil())
+		Expect(cdiConfig.Status).ToNot(BeNil())
+		Expect(cdiConfig.Status.UploadProxyURL).To(BeNil())
+	},
+		Entry("No default backend", createNoDefaultBackendIngress),
+		Entry("No service", createNoServiceIngress),
+		Entry("0 rules", createNoRulesIngress),
+	)
 })
 
 var _ = Describe("Controller route reconcile loop", func() {
@@ -839,6 +855,24 @@ func createIngress(name, ns, service, url string) *networkingv1.Ingress {
 	}
 }
 
+func createNoDefaultBackendIngress(name, ns, service, url string) *networkingv1.Ingress {
+	res := createIngress(name, ns, service, url)
+	res.Spec.DefaultBackend = nil
+	return res
+}
+
+func createNoServiceIngress(name, ns, service, url string) *networkingv1.Ingress {
+	res := createIngress(name, ns, service, url)
+	res.Spec.DefaultBackend.Service = nil
+	return res
+}
+
+func createNoRulesIngress(name, ns, service, url string) *networkingv1.Ingress {
+	res := createIngress(name, ns, service, url)
+	res.Spec.Rules = []networkingv1.IngressRule{}
+	return res
+}
+
 func createConfigMap(name, namespace string) *corev1.ConfigMap {
 	isController := true
 	cm := &corev1.ConfigMap{

diff --git a/tests/cdiconfig_test.go b/tests/cdiconfig_test.go
@@ -13,6 +13,7 @@ import (
 
 	"github.com/coreos/go-semver/semver"
 	route1client "github.com/openshift/client-go/route/clientset/versioned"
+	v1 "k8s.io/api/core/v1"
 	networkingv1 "k8s.io/api/networking/v1"
 	storagev1 "k8s.io/api/storage/v1"
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
@@ -414,6 +415,19 @@ var _ = Describe("CDI ingress config tests", func() {
 			return *config.Status.UploadProxyURL
 		}, time.Second*30, time.Second).Should(Equal(override))
 	})
+
+	It("Should not crash and uploadProxy empty, if ingress defined without service", func() {
+		ingress = createIngress("test-ingress", f.CdiInstallNs, "cdi-uploadproxy", ingressUrl)
+		// The cdi controller will not process this ingress because it doesn't have the url we are looking for.
+		_, err := f.K8sClient.NetworkingV1().Ingresses(f.CdiInstallNs).Create(context.TODO(), ingress, metav1.CreateOptions{})
+		Expect(err).ToNot(HaveOccurred())
+		By("Expecting uploadproxy url to be blank")
+		Eventually(func() bool {
+			config, err := f.CdiClient.CdiV1beta1().CDIConfigs().Get(context.TODO(), common.ConfigName, metav1.GetOptions{})
+			Expect(err).ToNot(HaveOccurred())
+			return config.Status.UploadProxyURL == nil
+		}, time.Second*30, time.Second).Should(BeTrue())
+	})
 })
 
 var _ = Describe("CDI route config tests", func() {
@@ -606,6 +620,41 @@ func createIngress(name, ns, service, hostUrl string) *networkingv1.Ingress {
 	}
 }
 
+func createNoServiceIngress(name, ns string) *networkingv1.Ingress {
+	apiGroup := "test.test"
+	return &networkingv1.Ingress{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: "networking/v1",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: ns,
+			Labels: map[string]string{
+				"nginx.org/ssl-services":                      "cdi-uploadproxy",
+				"ingress.kubernetes.io/ssl-passthrough":       "true",
+				"nginx.ingress.kubernetes.io/secure-backends": "true",
+				"nginx.ingress.kubernetes.io/proxy-body-size": "0",
+			},
+		},
+		Spec: networkingv1.IngressSpec{
+			DefaultBackend: &networkingv1.IngressBackend{
+				Resource: &v1.TypedLocalObjectReference{
+					APIGroup: &apiGroup,
+					Kind:     "test",
+					Name:     "test",
+				},
+			},
+			TLS: []networkingv1.IngressTLS{
+				{
+					Hosts: []string{
+						"cdi-uploadproxy.example.com",
+					},
+				},
+			},
+		},
+	}
+}
+
 func SetStorageClassDefault(f *framework.Framework, scName string, isDefault bool) error {
 	sc, err := f.K8sClient.StorageV1().StorageClasses().Get(context.TODO(), scName, metav1.GetOptions{})
 	if err != nil {