fix(k8s): fix VIPs configmap entries with invalid keys for ExternalName services (backport of #8168)

pkg/dns/vips/interfaces.go

@@ -2,6 +2,8 @@ package vips
 
 import (
 	"fmt"
+
+	"github.com/pkg/errors"
 )
 
 type EntryType int
@@ -41,8 +43,11 @@ func (e HostnameEntry) MarshalText() ([]byte, error) {
 }
 
 func (e *HostnameEntry) UnmarshalText(text []byte) error {
-	_, err := fmt.Sscanf(string(text), "%v:%s", &e.Type, &e.Name)
-	return err
+	if _, err := fmt.Sscanf(string(text), "%v:%s", &e.Type, &e.Name); err != nil {
+		return errors.Wrapf(err, "could not parse hostname entry: %+q", text)
+	}
+
+	return nil
 }
 
 func (e *HostnameEntry) Less(o *HostnameEntry) bool {

pkg/plugins/runtime/k8s/controllers/inbound_converter.go

@@ -125,10 +125,17 @@ func inboundForServiceless(zone string, pod *kube_core.Pod, name string) *mesh_p
 }
 
 func (i *InboundConverter) InboundInterfacesFor(ctx context.Context, zone string, pod *kube_core.Pod, services []*kube_core.Service) ([]*mesh_proto.Dataplane_Networking_Inbound, error) {
-	ifaces := []*mesh_proto.Dataplane_Networking_Inbound{}
+	var ifaces []*mesh_proto.Dataplane_Networking_Inbound
 	for _, svc := range services {
-		svcIfaces := inboundForService(zone, pod, svc)
-		ifaces = append(ifaces, svcIfaces...)
+		// Services of ExternalName type should not have any selectors.
+		// Kubernetes does not validate this, so in rare cases, a service of
+		// ExternalName type could point to a workload inside the mesh. If this
+		// happens, we would incorrectly generate inbounds including
+		// ExternalName service. We do not currently support ExternalName
+		// services, so we can safely skip them from processing.
+		if svc.Spec.Type != kube_core.ServiceTypeExternalName {
+			ifaces = append(ifaces, inboundForService(zone, pod, svc)...)
+		}
 	}
 
 	if len(ifaces) == 0 {

pkg/plugins/runtime/k8s/controllers/kube_hosts_converter.go

@@ -33,11 +33,21 @@ func KubeHosts(
 			continue // one invalid Dataplane definition should not break the entire mesh
 		}
 
-		// Do not generate outbounds for service-less
+		// Do not generate hostnames for service-less
 		if isServiceLess(port) {
 			continue
 		}
 
+		// Do not generate hostnames for ExternalName Service
+		if isExternalNameService(service) {
+			converterLog.V(1).Info(
+				"ignoring hostnames generation for unsupported ExternalName Service",
+				"name", service.GetName(),
+				"namespace", service.GetNamespace(),
+			)
+			continue
+		}
+
 		if isHeadlessService(service) {
 			// Generate outbound listeners for every endpoint of services.
 			for _, endpoint := range endpoints[serviceTag] {

pkg/plugins/runtime/k8s/controllers/outbound_converter.go

@@ -27,7 +27,7 @@ func (p *PodConverter) OutboundInterfacesFor(
 		reachableServicesMap[service] = true
 	}
 
-	dataplanes := []*core_mesh.DataplaneResource{}
+	var dataplanes []*core_mesh.DataplaneResource
 	for _, other := range others {
 		dp := core_mesh.NewDataplaneResource()
 		if err := p.ResourceConverter.ToCoreResource(other, dp); err != nil {
@@ -53,6 +53,16 @@ func (p *PodConverter) OutboundInterfacesFor(
 			continue
 		}
 
+		// Do not generate hostnames for ExternalName Service
+		if isExternalNameService(service) {
+			converterLog.V(1).Info(
+				"ignoring outbound generation for unsupported ExternalName Service",
+				"name", service.GetName(),
+				"namespace", service.GetNamespace(),
+			)
+			continue
+		}
+
 		if isHeadlessService(service) {
 			// Generate outbound listeners for every endpoint of services.
 			for _, endpoint := range endpoints[serviceTag] {
@@ -83,7 +93,18 @@ func (p *PodConverter) OutboundInterfacesFor(
 }
 
 func isHeadlessService(svc *kube_core.Service) bool {
-	return svc.Spec.ClusterIP == "None"
+	return svc.Spec.ClusterIP == kube_core.ClusterIPNone
+}
+
+// Services of ExternalName type should not have any selectors.
+// Kubernetes does not validate this, so in rare cases, a service of
+// ExternalName type could point to a workload inside the mesh. If this
+// happens, we will add the service to the VIPs config map, but we will
+// not be able to obtain its IP address. As a result, the key in the map
+// will be incorrect (e.g., "1:"). We do not currently support
+// ExternalName services, so we can safely skip them from processing.
+func isExternalNameService(svc *kube_core.Service) bool {
+	return svc != nil && svc.Spec.Type == kube_core.ServiceTypeExternalName
 }
 
 func isServiceLess(port uint32) bool {

pkg/plugins/runtime/k8s/controllers/service_controller.go

@@ -58,6 +58,15 @@ func (r *ServiceReconciler) Reconcile(ctx context.Context, req kube_ctrl.Request
 		return kube_ctrl.Result{}, nil
 	}
 
+	if svc.Spec.Type == kube_core.ServiceTypeExternalName {
+		log.V(1).Info(
+			"ignoring Service because it is of an unsupported type",
+			"name", req.NamespacedName.String(),
+			"type", kube_core.ServiceTypeExternalName,
+		)
+		return kube_ctrl.Result{}, nil
+	}
+
 	log.Info("annotating service which is part of the mesh", "annotation", fmt.Sprintf("%s=%s", metadata.IngressServiceUpstream, metadata.AnnotationTrue))
 	annotations := metadata.Annotations(svc.Annotations)
 	if annotations == nil {

pkg/plugins/runtime/k8s/controllers/service_controller_test.go

@@ -90,6 +90,18 @@ var _ = Describe("ServiceReconciler", func() {
 				},
 				Spec: kube_core.ServiceSpec{},
 			},
+			&kube_core.Service{
+				ObjectMeta: kube_meta.ObjectMeta{
+					Namespace: "non-system-ns-with-sidecar-injection",
+					Name:      "external-name",
+					Annotations: map[string]string{
+						metadata.KumaSidecarInjectionAnnotation: metadata.AnnotationEnabled,
+					},
+				},
+				Spec: kube_core.ServiceSpec{
+					Type: kube_core.ServiceTypeExternalName,
+				},
+			},
 			&kube_core.Service{
 				ObjectMeta: kube_meta.ObjectMeta{
 					Namespace:   "non-system-ns-with-sidecar-injection",
@@ -145,6 +157,26 @@ var _ = Describe("ServiceReconciler", func() {
 		Expect(svc.GetAnnotations()).ToNot(HaveKey(metadata.IngressServiceUpstream))
 	})
 
+	It("should ignore service of ExternalName type", func() {
+		// given
+		req := kube_ctrl.Request{
+			NamespacedName: kube_types.NamespacedName{Namespace: "non-system-ns-with-sidecar-injection", Name: "external-name"},
+		}
+
+		// when
+		result, err := reconciler.Reconcile(context.Background(), req)
+
+		// then
+		Expect(err).ToNot(HaveOccurred())
+		Expect(result).To(BeZero())
+
+		// and service is not annotated
+		svc := &kube_core.Service{}
+		err = kubeClient.Get(context.Background(), req.NamespacedName, svc)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(svc.GetAnnotations()).ToNot(HaveKey(metadata.IngressServiceUpstream))
+	})
+
 	It("should update service in an annotated namespace", func() {
 		// given
 		req := kube_ctrl.Request{

test/e2e_env/kubernetes/externalname-services/externalname-services.go

@@ -0,0 +1,62 @@
+package externalname_services
+
+import (
+	"fmt"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+
+	. "github.com/kumahq/kuma/test/framework"
+	"github.com/kumahq/kuma/test/framework/deployments/testserver"
+	"github.com/kumahq/kuma/test/framework/envs/kubernetes"
+)
+
+func ExternalNameServices() {
+	meshName := "externalname-services"
+	namespace := "externalname-services"
+
+	externalNameService := fmt.Sprintf(`
+apiVersion: v1
+kind: Service
+metadata:
+  name: externalname-service
+  namespace: %s
+spec:
+  type: ExternalName
+  externalName: foo.bar
+  ports: 
+  - appProtocol: tcp
+    port: 3000
+    protocol: TCP
+    targetPort: 3000
+  selector:
+    app: test-server
+`, namespace)
+
+	BeforeAll(func() {
+		Expect(NewClusterSetup().
+			Install(MeshKubernetes(meshName)).
+			Install(NamespaceWithSidecarInjection(namespace)).
+			Install(testserver.Install(
+				testserver.WithName("test-server"),
+				testserver.WithNamespace(namespace),
+				testserver.WithMesh(meshName),
+			)).
+			Setup(kubernetes.Cluster),
+		).To(Succeed())
+	})
+
+	E2EAfterAll(func() {
+		Expect(kubernetes.Cluster.TriggerDeleteNamespace(namespace)).To(Succeed())
+		Expect(kubernetes.Cluster.DeleteMesh(meshName)).To(Succeed())
+	})
+
+	It("should ignore ExternalName Service", func() {
+		// when
+		Expect(kubernetes.Cluster.Install(YamlK8s(externalNameService))).To(Succeed())
+
+		// then
+		Consistently(kubernetes.Cluster.GetKumaCPLogs, "10s", "1s").
+			ShouldNot(ContainSubstring("could not parse hostname entry"))
+	})
+}

test/e2e_env/kubernetes/kubernetes_suite_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/kumahq/kuma/test/e2e_env/kubernetes/connectivity"
 	"github.com/kumahq/kuma/test/e2e_env/kubernetes/container_patch"
 	"github.com/kumahq/kuma/test/e2e_env/kubernetes/defaults"
+	"github.com/kumahq/kuma/test/e2e_env/kubernetes/externalname-services"
 	"github.com/kumahq/kuma/test/e2e_env/kubernetes/externalservices"
 	"github.com/kumahq/kuma/test/e2e_env/kubernetes/gateway"
 	"github.com/kumahq/kuma/test/e2e_env/kubernetes/graceful"
@@ -68,6 +69,7 @@ var (
 	_ = Describe("Defaults", defaults.Defaults, Ordered)
 	_ = Describe("External Services", externalservices.ExternalServices, Ordered)
 	_ = Describe("External Services Permissive MTLS", externalservices.PermissiveMTLS, Ordered)
+	_ = Describe("ExternalName Services", externalname_services.ExternalNameServices, Ordered)
 	_ = Describe("Virtual Outbound", virtualoutbound.VirtualOutbound, Ordered)
 	_ = Describe("Kong Ingress Controller", Label("arm-not-supported"), kic.KICKubernetes, Ordered)
 	_ = Describe("MeshTrafficPermission API", meshtrafficpermission.API, Ordered)