OSSM-6615 Make sure IOR ignores services without selectors (#1022) (#1029)

* OSSM-6615 Make sure IOR ignores services without selectors

IOR finds a matching service for a pod by checking each service's label selector. However, this also matches services with an empty selector, which is wrong. These services should be ignored by IOR since an empty label selector indicates that the service endpoints are managed externally and not that the service should match all pods.

* Add test

* Increase timeout

The previous timeout was only 3s, which caused the retry.UntilSuccessOrFail to fail on the first attempt instead of allowing the retry to be performed.

Author: luksa

pilot/pkg/config/kube/ior/controller.go

@@ -289,6 +289,11 @@ func (r *routeController) findService(gateway *networking.Gateway) (*types.Names
 		podLabels := labels.Set(pod.ObjectMeta.Labels)
 		// Look for a service whose selector matches the pod labels
 		for _, service := range services {
+			if len(service.Spec.Selector) == 0 {
+				// an empty label selector does not mean that the service should match all pods, but that the
+				// service endpoints are managed externally; IOR should therefore ignore this service
+				continue
+			}
 			svcSelector := labels.SelectorFromSet(service.Spec.Selector)
 
 			iorLog.Debugf("matching service selector %s against %s", svcSelector.String(), podLabels)

pilot/pkg/config/kube/ior/ior_test.go

@@ -255,6 +255,9 @@ func TestCreate(t *testing.T) {
 	defer func() { close(stop) }()
 	store, k8sClient, routerClient := initClients(t, stop)
 
+	// create unrelated service with an empty selector to confirm that IOR is not picking it up
+	createService(t, k8sClient.GetActualClient(), controlPlaneNs, "service-with-empty-selector", nil)
+
 	createIngressGateway(t, k8sClient.GetActualClient(), controlPlaneNs, map[string]string{"istio": "ingressgateway"})
 
 	for i, c := range cases {
@@ -324,6 +327,11 @@ func validateRoutes(t *testing.T, hosts []string, list *[]*routeapiv1.Route, gat
 			t.Fatal("Route's host wrongly not set to wildcard.")
 		}
 
+		// check service
+		if route.Spec.To.Name != "gw-service" {
+			t.Fatalf("Route points to wrong service: %s", route.Spec.To.Name)
+		}
+
 		// TLS
 		if tls {
 			if route.Spec.TLS.InsecureEdgeTerminationPolicy != routeapiv1.InsecureEdgeTerminationPolicyRedirect {
@@ -530,29 +538,35 @@ func findRouteByHost(list *[]*routeapiv1.Route, host string) *routeapiv1.Route {
 
 func createIngressGateway(t *testing.T, client kube.Client, ns string, labels map[string]string) {
 	t.Helper()
-	createPod(t, client, ns, labels)
-	createService(t, client, ns, labels)
+	createPod(t, client, ns, "gw-pod", labels)
+	createService(t, client, ns, "gw-service", labels)
 }
 
-func createPod(t *testing.T, client kube.Client, ns string, labels map[string]string) {
+func createPod(t *testing.T, client kube.Client, ns, name string, labels map[string]string) {
 	t.Helper()
 
 	_, err := client.Kube().CoreV1().Pods(ns).Create(context.TODO(), &k8sioapicorev1.Pod{
 		ObjectMeta: metav1.ObjectMeta{
-			Labels: labels,
+			Name:      name,
+			Namespace: ns,
+			Labels:    labels,
 		},
 	}, metav1.CreateOptions{})
 	if err != nil {
 		t.Fatal(err)
 	}
 }
 
-func createService(t *testing.T, client kube.Client, ns string, labels map[string]string) {
+func createService(t *testing.T, client kube.Client, ns, name string, selector map[string]string) {
 	t.Helper()
 
 	_, err := client.Kube().CoreV1().Services(ns).Create(context.TODO(), &k8sioapicorev1.Service{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: ns,
+		},
 		Spec: k8sioapicorev1.ServiceSpec{
-			Selector: labels,
+			Selector: selector,
 		},
 	}, metav1.CreateOptions{})
 	if err != nil {

tests/integration/servicemesh/managingroutes/main_test.go

@@ -248,14 +248,14 @@ func applyGatewayOrFail(ctx framework.TestContext, ns, name string, labels map[s
 	// retry because of flaky validation webhook
 	retry.UntilSuccessOrFail(ctx, func() error {
 		return ctx.ConfigIstio().EvalFile(ns, map[string]interface{}{"hosts": hosts, "name": name, "labels": labels}, gatewayTmpl).Apply()
-	}, retry.Timeout(3*time.Second))
+	}, retry.Timeout(15*time.Second))
 }
 
 func deleteGatewayOrFail(ctx framework.TestContext, ns, name string, labels map[string]string, hosts ...string) {
 	// retry because of flaky validation webhook
 	retry.UntilSuccessOrFail(ctx, func() error {
 		return ctx.ConfigIstio().EvalFile(ns, map[string]interface{}{"hosts": hosts, "name": name, "labels": labels}, gatewayTmpl).Delete()
-	}, retry.Timeout(3*time.Second))
+	}, retry.Timeout(15*time.Second))
 }
 
 func applyVirtualServiceOrFail(ctx framework.TestContext, ns, gatewayNs, gatewayName, virtualServiceName string) {
@@ -266,5 +266,5 @@ func applyVirtualServiceOrFail(ctx framework.TestContext, ns, gatewayNs, gateway
 	}
 	retry.UntilSuccessOrFail(ctx, func() error {
 		return ctx.ConfigIstio().EvalFile(ns, values, virtualSvcTmpl).Apply()
-	}, retry.Timeout(3*time.Second))
+	}, retry.Timeout(15*time.Second))
 }