fix: tag Services backed by KongServiceFacade correctly (#5281)

internal/dataplane/translator/ingressrules.go

@@ -96,31 +96,7 @@ func (ir *ingressRules) populateServices(logger logr.Logger, s store.Storer, fai
 				}
 			}
 		}
-		if len(k8sServices) > 1 {
-			if parent, ok := ir.ServiceNameToParent[*service.Name]; ok {
-				service.Tags = util.GenerateTagsForObject(parent)
-			} else {
-				logger.Error(nil, "Multi-service backend lacks parent info, cannot generate tags",
-					"service", *service.Name)
-			}
-		} else if len(k8sServices) > 0 {
-			service.Tags = util.GenerateTagsForObject(k8sServices[0])
-		} else {
-			// this tag generation code runs _before_ we would discard routes that are invalid because their backend
-			// Service doesn't actually exist. attempting to generate tags for that Service would trigger a panic.
-			// the translator should discard this invalid route later, but this adds a placeholder value in case it doesn't.
-			// if you encounter an actual config where a service has these tags, something strange has happened.
-			logger.V(util.DebugLevel).Info("Service has zero k8sServices backends, cannot generate tags for it properly",
-				"service", *service.Name)
-			service.Tags = kong.StringSlice(
-				util.K8sNameTagPrefix+"UNKNOWN",
-				util.K8sNamespaceTagPrefix+"UNKNOWN",
-				util.K8sKindTagPrefix+"Service",
-				util.K8sUIDTagPrefix+"00000000-0000-0000-0000-000000000000",
-				util.K8sGroupTagPrefix+"core",
-				util.K8sVersionTagPrefix+"v1",
-			)
-		}
+		service.Tags = ir.generateKongServiceTags(k8sServices, service, logger)
 
 		// Kubernetes Services have been populated for this Kong Service, so it can
 		// now be cached.
@@ -129,6 +105,47 @@ func (ir *ingressRules) populateServices(logger logr.Logger, s store.Storer, fai
 	return serviceNamesToSkip
 }
 
+func (ir *ingressRules) generateKongServiceTags(
+	k8sServices []*corev1.Service,
+	service kongstate.Service,
+	logger logr.Logger,
+) []*string {
+	// For multi-backend Services we expect ServiceNameToParent to be populated.
+	if len(k8sServices) > 1 {
+		if parent, ok := ir.ServiceNameToParent[*service.Name]; ok {
+			return util.GenerateTagsForObject(parent)
+		}
+		logger.Error(nil, "Multi-service backend lacks parent info, cannot generate tags",
+			"service", *service.Name)
+		return nil
+	}
+
+	// For single-backend Services we ...
+	if len(k8sServices) == 1 {
+		// ... either use the parent object of the Service when its backend is a KongServiceFacade ...
+		if len(service.Backends) == 1 && service.Backends[0].Type == kongstate.ServiceBackendTypeKongServiceFacade {
+			return util.GenerateTagsForObject(service.Parent)
+		}
+		// ... or use the backing Kubernetes Service.
+		return util.GenerateTagsForObject(k8sServices[0])
+	}
+
+	// This tag generation code runs _before_ we would discard routes that are invalid because their backend
+	// Service doesn't actually exist. attempting to generate tags for that Service would trigger a panic.
+	// The translator should discard this invalid route later, but this adds a placeholder value in case it doesn't.
+	// If you encounter an actual config where a service has these tags, something strange has happened.
+	logger.V(util.DebugLevel).Info("Service has zero k8sServices backends, cannot generate tags for it properly",
+		"service", *service.Name)
+	return kong.StringSlice(
+		util.K8sNameTagPrefix+"UNKNOWN",
+		util.K8sNamespaceTagPrefix+"UNKNOWN",
+		util.K8sKindTagPrefix+"Service",
+		util.K8sUIDTagPrefix+"00000000-0000-0000-0000-000000000000",
+		util.K8sGroupTagPrefix+"core",
+		util.K8sVersionTagPrefix+"v1",
+	)
+}
+
 type SecretNameToSNIs struct {
 	// secretToSNIs maps secrets (by 'namespace/name' key) to SNIs they are related to.
 	secretToSNIs map[string]*SNIs

internal/dataplane/translator/subtranslator/ingress.go

@@ -11,6 +11,7 @@ import (
 	"github.com/samber/lo"
 	corev1 "k8s.io/api/core/v1"
 	netv1 "k8s.io/api/networking/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	k8stypes "k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -307,7 +308,16 @@ func (m *ingressTranslationMeta) translateIntoKongStateService(kongServiceName s
 				Namespace: m.parentIngress.GetNamespace(),
 				PortDef:   portDef,
 			}},
-			Parent: m.parentIngress,
+			Parent: &incubatorv1alpha1.KongServiceFacade{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: m.parentIngress.GetNamespace(),
+					Name:      m.backend.name,
+				},
+				TypeMeta: metav1.TypeMeta{
+					Kind:       incubatorv1alpha1.KongServiceFacadeKind,
+					APIVersion: incubatorv1alpha1.GroupVersion.String(),
+				},
+			},
 		}
 	}
 

internal/dataplane/translator/subtranslator/ingress_atc_test.go

@@ -364,7 +364,16 @@ func TestTranslateIngressATC(t *testing.T) {
 						Namespace: corev1.NamespaceDefault,
 						PortDef:   PortDefFromPortNumber(8080),
 					}},
-					Parent: expectedParentIngress(),
+					Parent: &incubatorv1alpha1.KongServiceFacade{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "svc-facade",
+							Namespace: corev1.NamespaceDefault,
+						},
+						TypeMeta: metav1.TypeMeta{
+							Kind:       incubatorv1alpha1.KongServiceFacadeKind,
+							APIVersion: incubatorv1alpha1.GroupVersion.String(),
+						},
+					},
 				},
 			},
 		},

internal/dataplane/translator/subtranslator/ingress_test.go

@@ -2,6 +2,7 @@ package subtranslator
 
 import (
 	"errors"
+	"sort"
 	"testing"
 
 	"github.com/go-logr/logr"
@@ -1361,7 +1362,16 @@ func TestTranslateIngress(t *testing.T) {
 						Namespace: corev1.NamespaceDefault,
 						PortDef:   PortDefFromPortNumber(8080),
 					}},
-					Parent: expectedParentIngress(),
+					Parent: &incubatorv1alpha1.KongServiceFacade{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "svc-facade",
+							Namespace: "default",
+						},
+						TypeMeta: metav1.TypeMeta{
+							Kind:       incubatorv1alpha1.KongServiceFacadeKind,
+							APIVersion: incubatorv1alpha1.GroupVersion.String(),
+						},
+					},
 				},
 			},
 		},
@@ -1520,7 +1530,16 @@ func TestTranslateIngress(t *testing.T) {
 						Namespace: corev1.NamespaceDefault,
 						PortDef:   PortDefFromPortNumber(8080),
 					}},
-					Parent: expectedParentIngress(),
+					Parent: &incubatorv1alpha1.KongServiceFacade{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "svc-facade",
+							Namespace: corev1.NamespaceDefault,
+						},
+						TypeMeta: metav1.TypeMeta{
+							Kind:       incubatorv1alpha1.KongServiceFacadeKind,
+							APIVersion: incubatorv1alpha1.GroupVersion.String(),
+						},
+					},
 				},
 			},
 		},
@@ -1541,7 +1560,7 @@ func TestTranslateIngress(t *testing.T) {
 			storer := lo.Must(store.NewFakeStore(store.FakeObjects{
 				KongServiceFacades: tt.kongServiceFacades,
 			}))
-			diff := cmp.Diff(tt.expected, TranslateIngresses(
+			translatedServices := TranslateIngresses(
 				tt.ingresses,
 				kongv1alpha1.IngressClassParametersSpec{},
 				TranslateIngressFeatureFlags{
@@ -1551,7 +1570,17 @@ func TestTranslateIngress(t *testing.T) {
 				noopObjectsCollector{},
 				failuresCollector,
 				storer,
-			), checkOnlyObjectMeta)
+			)
+
+			// Sort routes to make the test deterministic. Not doing this in the code itself as the deterministic
+			// order is not required on this level of translation and that would be an unnecessary performance hit.
+			for _, service := range translatedServices {
+				sort.Slice(service.Routes, func(i, j int) bool {
+					return *service.Routes[i].Route.Name > *service.Routes[j].Route.Name
+				})
+			}
+
+			diff := cmp.Diff(tt.expected, translatedServices, checkOnlyObjectMeta)
 			require.Empty(t, diff, "expected no difference between expected and translated ingress")
 		})
 	}