chore: remove KongIngress support for Gateway API Route objects

CHANGELOG.md

@@ -136,6 +136,11 @@
   `networking.knative.dev/ingress-class` and the deprecated `networking.knative.dev/ingress.class` one
   to adapt to [what has already been done in knative](https://github.com/knative/networking/pull/522).
   [#2485](https://github.com/Kong/kubernetes-ingress-controller/issues/2485)
+- Remove KongIngress support for Gateway API Route objects and Services referenced
+  by those Routes. This disables an undocumented ability of customizing Gateway API
+  `*Route` objects and `Service`s that are set as backendRefs for those `*Route`s
+  via `konghq.com/override` annotations.
+  [#2554](https://github.com/Kong/kubernetes-ingress-controller/issues/2554)
 
 ## [2.3.1]
 

internal/dataplane/kongstate/kongstate.go

@@ -137,17 +137,20 @@ func (ks *KongState) FillOverrides(log logrus.FieldLogger, s store.Storer) {
 		// Services
 		kongIngress, err := getKongIngressForServices(s, ks.Services[i].K8sServices)
 		if err != nil {
-			log.WithError(err).Errorf("failed to fetch KongIngress resource for Services %s", PrettyPrintServiceList(ks.Services[i].K8sServices))
+			log.WithError(err).
+				Errorf("failed to fetch KongIngress resource for Services %s",
+					PrettyPrintServiceList(ks.Services[i].K8sServices),
+				)
 			continue
 		}
 
 		for _, svc := range ks.Services[i].K8sServices {
-			ks.Services[i].override(kongIngress, svc.Annotations)
+			ks.Services[i].override(log, kongIngress, svc)
 		}
 
 		// Routes
 		for j := 0; j < len(ks.Services[i].Routes); j++ {
-			kongIngress, err := getKongIngressFromObjectMeta(s, &ks.Services[i].Routes[j].Ingress)
+			kongIngress, err := getKongIngressFromObjectMeta(s, ks.Services[i].Routes[j].Ingress)
 			if err != nil {
 				log.WithFields(logrus.Fields{
 					"resource_name":      ks.Services[i].Routes[j].Ingress.Name,
@@ -163,12 +166,15 @@ func (ks *KongState) FillOverrides(log logrus.FieldLogger, s store.Storer) {
 	for i := 0; i < len(ks.Upstreams); i++ {
 		kongIngress, err := getKongIngressForServices(s, ks.Upstreams[i].Service.K8sServices)
 		if err != nil {
-			log.WithError(err).Errorf("failed to fetch KongIngress resource for Services %s", PrettyPrintServiceList(ks.Upstreams[i].Service.K8sServices))
+			log.WithError(err).
+				Errorf("failed to fetch KongIngress resource for Services %s",
+					PrettyPrintServiceList(ks.Upstreams[i].Service.K8sServices),
+				)
 			continue
 		}
 
 		for _, svc := range ks.Upstreams[i].Service.K8sServices {
-			ks.Upstreams[i].override(kongIngress, svc.Annotations)
+			ks.Upstreams[i].override(log, kongIngress, svc)
 		}
 	}
 }

internal/dataplane/kongstate/route.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/kong/go-kong/kong"
 	"github.com/sirupsen/logrus"
+	gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
 
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/annotations"
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/util"
@@ -117,7 +118,6 @@ func (r *Route) overrideProtocols(anns map[string]string) {
 }
 
 func (r *Route) overrideHTTPSRedirectCode(anns map[string]string) {
-
 	if annotations.HasForceSSLRedirectAnnotation(anns) {
 		r.HTTPSRedirectStatusCode = kong.Int(302)
 		r.useSSLProtocol()
@@ -235,6 +235,21 @@ func (r *Route) override(log logrus.FieldLogger, kongIngress *configurationv1.Ko
 	if r == nil {
 		return
 	}
+
+	// Check if we're trying to get KongIngress configuration based on an annotation
+	// on Gateway API object (this would most likely happen for *Route objects but
+	// log a warning for all other Gateway API objects as well since that also should
+	// not happen) and if that's the case then skip it since those should not be
+	// affected by said annotation.
+	if gvk := r.Ingress.GroupVersionKind; gvk.Group == gatewayv1alpha2.GroupName && kongIngress != nil {
+		log.WithFields(logrus.Fields{
+			"resource_name":      r.Ingress.Name,
+			"resource_namespace": r.Ingress.Namespace,
+			"resource_kind":      gvk.Kind,
+		}).Warn("KongIngress annotation is not allowed on Gateway API objects.")
+		return
+	}
+
 	r.overrideByKongIngress(log, kongIngress)
 	r.overrideByAnnotation(log)
 	r.normalizeProtocols()

internal/dataplane/kongstate/route_test.go

@@ -1,6 +1,7 @@
 package kongstate
 
 import (
+	"io/ioutil"
 	"reflect"
 	"testing"
 
@@ -356,8 +357,11 @@ func TestNormalizeProtocols(t *testing.T) {
 	}
 
 	assert.NotPanics(func() {
+		log := logrus.New()
+		log.SetOutput(ioutil.Discard)
+
 		var nilUpstream *Upstream
-		nilUpstream.override(nil, make(map[string]string))
+		nilUpstream.override(log, nil, nil)
 	})
 }
 

internal/dataplane/kongstate/service.go

@@ -4,7 +4,10 @@ import (
 	"strings"
 
 	"github.com/kong/go-kong/kong"
+	"github.com/sirupsen/logrus"
 	corev1 "k8s.io/api/core/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
 
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/annotations"
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/util"
@@ -48,6 +51,7 @@ type Service struct {
 
 	Backends    []ServiceBackend
 	K8sServices map[string]*corev1.Service
+	Parent      client.Object
 }
 
 // overrideByKongIngress sets Service fields by KongIngress
@@ -112,15 +116,48 @@ func (s *Service) overrideByAnnotation(anns map[string]string) {
 	s.overridePath(anns)
 }
 
-// override sets Service fields by KongIngress first, then by annotation
-func (s *Service) override(kongIngress *configurationv1.KongIngress,
-	anns map[string]string) {
+// override sets Service fields by KongIngress first, then by k8s Service's annotations
+func (s *Service) override(
+	log logrus.FieldLogger,
+	kongIngress *configurationv1.KongIngress,
+	svc *corev1.Service,
+) {
 	if s == nil {
 		return
 	}
 
+	if s.Parent != nil && kongIngress != nil {
+		kongIngressFromSvcAnnotation := annotations.ExtractConfigurationName(svc.Annotations)
+		if kongIngressFromSvcAnnotation != "" {
+			// If the parent object behind Kong Service is a Gateway API object
+			// (probably *Route but log a warning for all other objects as well)
+			// then check if we're trying to override said Service configuration with
+			// a KongIngress object and if that's the case then skip it since those
+			// should not be affected.
+			gvk := s.Parent.GetObjectKind().GroupVersionKind()
+			if gvk.Group == gatewayv1alpha2.GroupName {
+				obj := s.Parent
+				fields := logrus.Fields{
+					"resource_name":      obj.GetName(),
+					"resource_namespace": obj.GetNamespace(),
+					"resource_kind":      gvk.Kind,
+				}
+				if svc != nil {
+					fields["service_name"] = svc.Name
+					fields["service_namespace"] = svc.Namespace
+				}
+				log.WithFields(fields).
+					Warn("KongIngress annotation is not allowed on Services " +
+						"referenced by Gateway API *Route objects.")
+				return
+			}
+		}
+	}
+
 	s.overrideByKongIngress(kongIngress)
-	s.overrideByAnnotation(anns)
+	if svc != nil {
+		s.overrideByAnnotation(svc.Annotations)
+	}
 
 	if *s.Protocol == "grpc" || *s.Protocol == "grpcs" {
 		// grpc(s) doesn't accept a path

internal/dataplane/kongstate/service_test.go

@@ -1,10 +1,12 @@
 package kongstate
 
 import (
+	"io/ioutil"
 	"reflect"
 	"testing"
 
 	"github.com/kong/go-kong/kong"
+	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 
 	configurationv1 "github.com/kong/kubernetes-ingress-controller/v2/pkg/apis/configuration/v1"
@@ -361,13 +363,22 @@ func TestOverrideService(t *testing.T) {
 	}
 
 	for _, testcase := range testTable {
-		testcase.inService.override(&testcase.inKongIngresss, testcase.inAnnotation)
-		assert.Equal(testcase.inService, testcase.outService)
+		log := logrus.New()
+		log.SetOutput(ioutil.Discard)
+
+		k8sServices := testcase.inService.K8sServices
+		for _, svc := range k8sServices {
+			testcase.inService.override(log, &testcase.inKongIngresss, svc)
+			assert.Equal(testcase.inService, testcase.outService)
+		}
 	}
 
 	assert.NotPanics(func() {
+		log := logrus.New()
+		log.SetOutput(ioutil.Discard)
+
 		var nilService *Service
-		nilService.override(nil, nil)
+		nilService.override(log, nil, nil)
 	})
 }
 

internal/dataplane/kongstate/upstream.go

@@ -2,6 +2,9 @@ package kongstate
 
 import (
 	"github.com/kong/go-kong/kong"
+	"github.com/sirupsen/logrus"
+	corev1 "k8s.io/api/core/v1"
+	gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
 
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/annotations"
 	configurationv1 "github.com/kong/kubernetes-ingress-controller/v2/pkg/apis/configuration/v1"
@@ -79,12 +82,44 @@ func (u *Upstream) overrideByKongIngress(kongIngress *configurationv1.KongIngres
 	// TODO https://github.com/Kong/kubernetes-ingress-controller/issues/2075
 }
 
-// override sets Upstream fields by KongIngress first, then by annotation
-func (u *Upstream) override(kongIngress *configurationv1.KongIngress, anns map[string]string) {
+// override sets Upstream fields by KongIngress first, then by k8s Service's annotations
+func (u *Upstream) override(
+	log logrus.FieldLogger,
+	kongIngress *configurationv1.KongIngress,
+	svc *corev1.Service,
+) {
 	if u == nil {
 		return
 	}
 
+	if u.Service.Parent != nil && kongIngress != nil {
+		// If the parent object behind Kong Upstream's is a Gateway API object
+		// (probably *Route but log a warning for all other objects as well)
+		// then check if we're trying to override said Service configuration with
+		// a KongIngress object and if that's the case then skip it since those
+		// should not be affected.
+		gvk := u.Service.Parent.GetObjectKind().GroupVersionKind()
+		if gvk.Group == gatewayv1alpha2.GroupName {
+			obj := u.Service.Parent
+			fields := logrus.Fields{
+				"resource_name":      obj.GetName(),
+				"resource_namespace": obj.GetNamespace(),
+				"resource_kind":      gvk.Kind,
+			}
+			if svc != nil {
+				fields["service_name"] = svc.Name
+				fields["service_namespace"] = svc.Namespace
+			}
+			// No log needed here as there will be one issued already from Kong's
+			// Service override. The reason for this is that there is no other
+			// object in Kubernetes that creates a Kong's Upstream and Kubernetes
+			// Service will already trigger Kong's Service creation and log issuance.
+			return
+		}
+	}
+
 	u.overrideByKongIngress(kongIngress)
-	u.overrideByAnnotation(anns)
+	if svc != nil {
+		u.overrideByAnnotation(svc.Annotations)
+	}
 }

internal/dataplane/kongstate/upstream_test.go

@@ -1,10 +1,14 @@
 package kongstate
 
 import (
+	"io/ioutil"
 	"testing"
 
 	"github.com/kong/go-kong/kong"
+	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	configurationv1 "github.com/kong/kubernetes-ingress-controller/v2/pkg/apis/configuration/v1"
 )
@@ -16,7 +20,7 @@ func TestOverrideUpstream(t *testing.T) {
 		inUpstream     Upstream
 		inKongIngresss *configurationv1.KongIngress
 		outUpstream    Upstream
-		annotations    map[string]string
+		svc            *corev1.Service
 	}{
 		{
 			inUpstream: Upstream{
@@ -61,19 +65,29 @@ func TestOverrideUpstream(t *testing.T) {
 					Slots:              kong.Int(42),
 				},
 			},
-			annotations: map[string]string{
-				"konghq.com/host-header": "foo.com",
+			svc: &corev1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						"konghq.com/host-header": "foo.com",
+					},
+				},
 			},
 		},
 	}
 
 	for _, testcase := range testTable {
-		testcase.inUpstream.override(testcase.inKongIngresss, testcase.annotations)
+		log := logrus.New()
+		log.SetOutput(ioutil.Discard)
+
+		testcase.inUpstream.override(log, testcase.inKongIngresss, testcase.svc)
 		assert.Equal(testcase.inUpstream, testcase.outUpstream)
 	}
 
 	assert.NotPanics(func() {
+		log := logrus.New()
+		log.SetOutput(ioutil.Discard)
+
 		var nilUpstream *Upstream
-		nilUpstream.override(nil, make(map[string]string))
+		nilUpstream.override(log, nil, nil)
 	})
 }

internal/dataplane/kongstate/util.go

@@ -48,23 +48,30 @@ func getKongIngressForServices(
 	return nil, nil
 }
 
-func getKongIngressFromObjectMeta(s store.Storer, obj *util.K8sObjectInfo) (
-	*configurationv1.KongIngress, error) {
-	return getKongIngressFromIngressAnnotations(s, obj.Namespace, obj.Name, obj.Annotations)
+func getKongIngressFromObjectMeta(
+	s store.Storer,
+	obj util.K8sObjectInfo,
+) (
+	*configurationv1.KongIngress, error,
+) {
+	return getKongIngressFromObjAnnotations(s, obj)
 }
 
-func getKongIngressFromIngressAnnotations(s store.Storer, namespace, name string,
-	anns map[string]string) (
-	*configurationv1.KongIngress, error) {
-	confName := annotations.ExtractConfigurationName(anns)
+func getKongIngressFromObjAnnotations(
+	s store.Storer,
+	obj util.K8sObjectInfo,
+) (
+	*configurationv1.KongIngress, error,
+) {
+	confName := annotations.ExtractConfigurationName(obj.Annotations)
 	if confName != "" {
-		ki, err := s.GetKongIngress(namespace, confName)
+		ki, err := s.GetKongIngress(obj.Namespace, confName)
 		if err == nil {
 			return ki, nil
 		}
 	}
 
-	ki, err := s.GetKongIngress(namespace, name)
+	ki, err := s.GetKongIngress(obj.Namespace, obj.Name)
 	if err == nil {
 		return ki, nil
 	}