Fix mirroring all services when remote selector is empty

multicluster/service-mirror/cluster_watcher.go

@@ -739,7 +739,7 @@ func (rcsw *RemoteClusterServiceWatcher) getMirrorServices() (*corev1.ServiceLis
 }
 
 func (rcsw *RemoteClusterServiceWatcher) handleOnDelete(service *corev1.Service) {
-	if rcsw.isExported(service.Labels) {
+	if rcsw.isExported(service.Labels) || rcsw.isRemoteDiscovery(service.Labels) {
 		rcsw.eventsQueue.Add(&RemoteServiceDeleted{
 			Name:      service.Name,
 			Namespace: service.Namespace,
@@ -1242,12 +1242,7 @@ func (rcsw *RemoteClusterServiceWatcher) isExported(l map[string]string) bool {
 		rcsw.log.Errorf("Invalid selector: %s", err)
 		return false
 	}
-	remoteDiscoverySelector, err := metav1.LabelSelectorAsSelector(&rcsw.link.RemoteDiscoverySelector)
-	if err != nil {
-		rcsw.log.Errorf("Invalid selector: %s", err)
-		return false
-	}
-	return selector.Matches(labels.Set(l)) || remoteDiscoverySelector.Matches(labels.Set(l))
+	return selector.Matches(labels.Set(l))
 }
 
 func (rcsw *RemoteClusterServiceWatcher) isRemoteDiscovery(l map[string]string) bool {

multicluster/service-mirror/cluster_watcher_mirroring_test.go

@@ -641,6 +641,64 @@ func TestRemoteServiceUpdatedMirroring(t *testing.T) {
 	}
 }
 
+// TestEmptyRemoteSelectors asserts that empty selectors do not introduce side
+// effects, such as mirroring unexported services. An empty label selector
+// functions as a catch-all (i.e. matches everything), the cluster watcher must
+// uphold an invariant whereby empty selectors do not export everything by
+// default.
+func TestEmptyRemoteSelectors(t *testing.T) {
+	for _, tt := range []mirroringTestCase{
+		{
+			description: "empty remote discovery selector does not result in exports",
+			environment: createEnvWithSelector(defaultSelector, &metav1.LabelSelector{}),
+			expectedEventsInQueue: []interface{}{&RemoteServiceCreated{
+				service: remoteService("service-one", "ns1", "111", map[string]string{
+					consts.DefaultExportedServiceSelector: "true",
+				}, []corev1.ServicePort{
+					{
+						Name:     "port1",
+						Protocol: "TCP",
+						Port:     555,
+					},
+					{
+						Name:     "port2",
+						Protocol: "TCP",
+						Port:     666,
+					},
+				}),
+			},
+			},
+		},
+		{
+			description: "empty default selector does not result in exports",
+			environment: createEnvWithSelector(&metav1.LabelSelector{}, defaultRemoteDiscoverySelector),
+			expectedEventsInQueue: []interface{}{&RemoteServiceCreated{
+				service: remoteService("service-two", "ns1", "111", map[string]string{
+					consts.DefaultExportedServiceSelector: "remote-discovery",
+				}, []corev1.ServicePort{
+					{
+						Name:     "port1",
+						Protocol: "TCP",
+						Port:     555,
+					},
+					{
+						Name:     "port2",
+						Protocol: "TCP",
+						Port:     666,
+					},
+				}),
+			}},
+		},
+		{
+			description: "no selector in link does not result in exports",
+			environment: createEnvWithSelector(&metav1.LabelSelector{}, &metav1.LabelSelector{}),
+		},
+	} {
+		tc := tt
+		tc.run(t)
+	}
+}
+
 func TestRemoteEndpointsUpdatedMirroring(t *testing.T) {
 	for _, tt := range []mirroringTestCase{
 		{

multicluster/service-mirror/cluster_watcher_test_util.go

@@ -1222,3 +1222,67 @@ func endpointMirrorEndpointsAsYaml(name, namespace, hostname, gatewayIP, gateway
 
 	return string(bytes)
 }
+
+// createEmptySelectorEnv will create a test environment with two services. It
+// accepts a default and a remote discovery selector which it will use for the
+// link creation. This function is used to create environments that differ only
+// in the selector used in the link.
+func createEnvWithSelector(defaultSelector, remoteSelector *metav1.LabelSelector) *testEnvironment {
+	return &testEnvironment{
+		events: []interface{}{
+			&OnAddCalled{
+				svc: remoteService("service-one", "ns1", "111", map[string]string{
+					consts.DefaultExportedServiceSelector: "true",
+				}, []corev1.ServicePort{
+					{
+						Name:     "port1",
+						Protocol: "TCP",
+						Port:     555,
+					},
+					{
+						Name:     "port2",
+						Protocol: "TCP",
+						Port:     666,
+					},
+				}),
+			},
+			&OnAddCalled{
+				svc: remoteService("service-two", "ns1", "111", map[string]string{
+					consts.DefaultExportedServiceSelector: "remote-discovery",
+				}, []corev1.ServicePort{
+					{
+						Name:     "port1",
+						Protocol: "TCP",
+						Port:     555,
+					},
+					{
+						Name:     "port2",
+						Protocol: "TCP",
+						Port:     666,
+					},
+				}),
+			},
+		},
+		localResources: []string{
+			namespaceAsYaml("ns1"),
+		},
+		remoteResources: []string{
+			endpointsAsYaml("service-one", "ns1", "192.0.2.127", "gateway-identity", []corev1.EndpointPort{}),
+			endpointsAsYaml("service-two", "ns1", "192.0.3.127", "gateway-identity", []corev1.EndpointPort{}),
+		},
+		link: multicluster.Link{
+			TargetClusterName:   clusterName,
+			TargetClusterDomain: clusterDomain,
+			GatewayIdentity:     "",
+			GatewayAddress:      "",
+			GatewayPort:         0,
+			ProbeSpec: multicluster.ProbeSpec{
+				Path:   "",
+				Port:   0,
+				Period: time.Duration(0) * time.Second,
+			},
+			Selector:                *defaultSelector,
+			RemoteDiscoverySelector: *remoteSelector,
+		},
+	}
+}