fix: avoid nil pointer reference when overriding route with KongIngress

internal/ingress/controller/kong.go

@@ -774,31 +774,8 @@ func (n *NGINXController) syncRoutes(ingressCfg *ingress.Configuration) (bool, e
 			}
 
 			if kongIngress != nil && kongIngress.Route != nil {
-				if len(kongIngress.Route.Methods) > 0 {
-					r.Methods = toStringPtrArray(kongIngress.Route.Methods)
-				}
-
-				if r.PreserveHost == nil {
-					r.PreserveHost = kong.Bool(false)
-				}
-				if kongIngress.Route.PreserveHost != *r.PreserveHost {
-					r.PreserveHost = kong.Bool(kongIngress.Route.PreserveHost)
-				}
-
-				if kongIngress.Route.RegexPriority != 0 &&
-					r.RegexPriority != nil || kongIngress.Route.RegexPriority != *r.RegexPriority {
-					r.RegexPriority = kong.Int(kongIngress.Route.RegexPriority)
-				}
-
-				if r.StripPath == nil {
-					r.StripPath = kong.Bool(false)
-				}
-				if kongIngress.Route.StripPath != *r.StripPath {
-					r.StripPath = kong.Bool(kongIngress.Route.StripPath)
-				}
-				if len(kongIngress.Route.Protocols) != 0 {
-					r.Protocols = toStringPtrArray(kongIngress.Route.Protocols)
-				}
+				// ignore updated on create
+				mergeRouteAndKongIngress(r, kongIngress)
 			}
 
 			if !isRouteInKong(r, kongRoutes) {
@@ -815,62 +792,23 @@ func (n *NGINXController) syncRoutes(ingressCfg *ingress.Configuration) (bool, e
 				if routesToRemove.Has(*route.ID) {
 					routesToRemove.Delete(*route.ID)
 				}
-				outOfSync := false
+				routesOutOfSync, ingressOutOfSync := false, false
 				sort.Strings(protos)
 				p := toStringArray(route.Protocols)
 				sort.Strings(p)
 
 				if !reflect.DeepEqual(protos, p) {
-					outOfSync = true
+					routesOutOfSync = true
 					route.Protocols = toStringPtrArray(protos)
 				}
 
 				if kongIngress != nil && kongIngress.Route != nil {
-					if kongIngress.Route.Methods != nil {
-						sort.Strings(kongIngress.Route.Methods)
-						m := toStringArray(route.Methods)
-						sort.Strings(m)
-						if !reflect.DeepEqual(m, kongIngress.Route.Methods) {
-							outOfSync = true
-							route.Methods = toStringPtrArray(kongIngress.Route.Methods)
-						}
-					}
-
-					if route.PreserveHost == nil {
-						route.PreserveHost = kong.Bool(false)
-					}
-					if kongIngress.Route.PreserveHost != *route.PreserveHost {
-						outOfSync = true
-						route.PreserveHost = kong.Bool(kongIngress.Route.PreserveHost)
-					}
-
-					if route.RegexPriority == nil {
-						route.RegexPriority = kong.Int(0)
-					}
-					if kongIngress.Route.RegexPriority != *route.RegexPriority {
-						outOfSync = true
-						route.RegexPriority = kong.Int(kongIngress.Route.RegexPriority)
-					}
-
-					if route.StripPath == nil {
-						route.StripPath = kong.Bool(false)
-					}
-					if kongIngress.Route.StripPath != *route.StripPath {
-						outOfSync = true
-						route.StripPath = kong.Bool(kongIngress.Route.StripPath)
-					}
-					if len(kongIngress.Route.Protocols) > 0 {
-						sort.Strings(kongIngress.Route.Protocols)
-						if !reflect.DeepEqual(route.Protocols, kongIngress.Route.Protocols) {
-							outOfSync = true
-							glog.Infof("protocols changed form %v to %v", route.Protocols, kongIngress.Route.Protocols)
-							route.Protocols = toStringPtrArray(kongIngress.Route.Protocols)
-						}
-					}
+					ingressOutOfSync = mergeRouteAndKongIngress(route, kongIngress)
 				}
 
-				if outOfSync {
-					glog.Infof("updating Kong Route for host %v, path %v and service %v", server.Hostname, location.Path, svc.ID)
+				if routesOutOfSync || ingressOutOfSync {
+					glog.Infof("updating Kong Route for host %v, path %v and service %v",
+						server.Hostname, location.Path, svc.ID)
 					_, err := client.Routes.Update(nil, route)
 					if err != nil {
 						glog.Errorf("Unexpected error updating Kong Route: %v", err)

internal/ingress/controller/util.go

@@ -17,9 +17,12 @@ limitations under the License.
 package controller
 
 import (
+	"reflect"
+	"sort"
 	"strings"
 
 	"github.com/hbagdi/go-kong/kong"
+	configurationv1 "github.com/kong/kubernetes-ingress-controller/internal/apis/configuration/v1"
 )
 
 func isEmpty(s *string) bool {
@@ -95,3 +98,58 @@ func compareRoute(r1, r2 *kong.Route) bool {
 
 	return true
 }
+
+func mergeRouteAndKongIngress(r *kong.Route, kongIngress *configurationv1.KongIngress) bool {
+	if r == nil {
+		return false
+	}
+	if kongIngress == nil || kongIngress.Route == nil {
+		return false
+	}
+	updated := false
+	if len(kongIngress.Route.Methods) > 0 {
+		sort.Strings(kongIngress.Route.Methods)
+		m := toStringArray(r.Methods)
+		sort.Strings(m)
+		if !reflect.DeepEqual(m, kongIngress.Route.Methods) {
+			updated = true
+			r.Methods = toStringPtrArray(kongIngress.Route.Methods)
+		}
+	}
+
+	if r.PreserveHost == nil {
+		r.PreserveHost = kong.Bool(false)
+	}
+	if kongIngress.Route.PreserveHost != *r.PreserveHost {
+		r.PreserveHost = kong.Bool(kongIngress.Route.PreserveHost)
+		updated = true
+	}
+
+	if r.RegexPriority == nil {
+		r.RegexPriority = kong.Int(0)
+	}
+	if kongIngress.Route.RegexPriority != 0 &&
+		kongIngress.Route.RegexPriority != *r.RegexPriority {
+		r.RegexPriority = kong.Int(kongIngress.Route.RegexPriority)
+		updated = true
+	}
+
+	if r.StripPath == nil {
+		r.StripPath = kong.Bool(false)
+	}
+	if kongIngress.Route.StripPath != *r.StripPath {
+		r.StripPath = kong.Bool(kongIngress.Route.StripPath)
+		updated = true
+	}
+
+	if len(kongIngress.Route.Protocols) > 0 {
+		protocols := toStringArray(r.Protocols)
+		sort.Strings(protocols)
+		sort.Strings(kongIngress.Route.Protocols)
+		if !reflect.DeepEqual(protocols, kongIngress.Route.Protocols) {
+			updated = true
+			r.Protocols = toStringPtrArray(kongIngress.Route.Protocols)
+		}
+	}
+	return updated
+}

internal/ingress/controller/util_test.go

@@ -0,0 +1,64 @@
+package controller
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/hbagdi/go-kong/kong"
+	configurationv1 "github.com/kong/kubernetes-ingress-controller/internal/apis/configuration/v1"
+)
+
+func TestMergeRouteAndKongIngress(t *testing.T) {
+	r := &kong.Route{
+		Hosts: []*string{kong.String("foo.com"), kong.String("bar.com")},
+		Paths: []*string{kong.String("/")},
+	}
+
+	kongIngress := &configurationv1.KongIngress{
+		Route: &configurationv1.Route{},
+	}
+	updated := mergeRouteAndKongIngress(r, kongIngress)
+	if updated {
+		t.Errorf("expected: false, actual: %v", updated)
+	}
+
+	kongIngress.Route.RegexPriority = 10
+	kongIngress.Route.PreserveHost = true
+	updated = mergeRouteAndKongIngress(r, kongIngress)
+	if !updated {
+		t.Errorf("expected: false, actual: %v", updated)
+	}
+	if *r.RegexPriority != 10 {
+		t.Errorf("expected regex priority to be 10")
+	}
+	if !*r.PreserveHost {
+		t.Errorf("expected PreserveHost to be true")
+	}
+
+	kongIngress.Route.Protocols = []string{"https"}
+	updated = mergeRouteAndKongIngress(r, kongIngress)
+	if !updated {
+		t.Errorf("expected: false, actual: %v", updated)
+	}
+	if len(r.Protocols) != 1 {
+		t.Errorf("expected length to be 1")
+	}
+	if *r.Protocols[0] != "https" {
+		t.Errorf("expected protocols to be 'https'")
+	}
+
+	updated = mergeRouteAndKongIngress(r, kongIngress)
+	if updated {
+		t.Errorf("expected updated to be false on no-op")
+	}
+
+	kongIngress.Route.Methods = []string{"GET", "POST"}
+	updated = mergeRouteAndKongIngress(r, kongIngress)
+	if !updated {
+		t.Errorf("expected updated to be true")
+	}
+
+	if !reflect.DeepEqual(toStringArray(r.Methods), kongIngress.Route.Methods) {
+		t.Errorf("expected %v, got %v", kongIngress.Route.Methods, toStringArray(r.Methods))
+	}
+}