diff --git a/multicluster/service-mirror/cluster_watcher.go b/multicluster/service-mirror/cluster_watcher.go index 67fadc4ead6b8..9304ac1133f0a 100644 --- a/multicluster/service-mirror/cluster_watcher.go +++ b/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 { diff --git a/multicluster/service-mirror/cluster_watcher_mirroring_test.go b/multicluster/service-mirror/cluster_watcher_mirroring_test.go index 7966101293145..728e9f6f93af5 100644 --- a/multicluster/service-mirror/cluster_watcher_mirroring_test.go +++ b/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: "default1", + Protocol: "TCP", + Port: 555, + }, + { + Name: "default2", + 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: "remote1", + Protocol: "TCP", + Port: 777, + }, + { + Name: "remote2", + Protocol: "TCP", + Port: 888, + }, + }), + }}, + }, + { + 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{ { diff --git a/multicluster/service-mirror/cluster_watcher_test_util.go b/multicluster/service-mirror/cluster_watcher_test_util.go index ce055f6629054..abf1e29ad6880 100644 --- a/multicluster/service-mirror/cluster_watcher_test_util.go +++ b/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: "default1", + Protocol: "TCP", + Port: 555, + }, + { + Name: "default2", + Protocol: "TCP", + Port: 666, + }, + }), + }, + &OnAddCalled{ + svc: remoteService("service-two", "ns1", "111", map[string]string{ + consts.DefaultExportedServiceSelector: "remote-discovery", + }, []corev1.ServicePort{ + { + Name: "remote1", + Protocol: "TCP", + Port: 777, + }, + { + Name: "remote2", + Protocol: "TCP", + Port: 888, + }, + }), + }, + }, + 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, + }, + } +}