From 0d8ec101aef79c49825f1f4a1195928adf84a6bf Mon Sep 17 00:00:00 2001 From: Andrew Stucki Date: Thu, 7 Apr 2022 10:49:13 -0400 Subject: [PATCH 1/4] Add annotation for explicitly specifying which service you want registered --- control-plane/connect-inject/annotations.go | 5 + .../connect-inject/endpoints_controller.go | 6 + .../endpoints_controller_test.go | 113 ++++++++++++++++++ 3 files changed, 124 insertions(+) diff --git a/control-plane/connect-inject/annotations.go b/control-plane/connect-inject/annotations.go index 2189a9f31c..5d47e69eb6 100644 --- a/control-plane/connect-inject/annotations.go +++ b/control-plane/connect-inject/annotations.go @@ -20,6 +20,11 @@ const ( // This defaults to the name of the Kubernetes service associated with the pod. annotationService = "consul.hashicorp.com/connect-service" + // annotationKubernetesService is the name of the Kubernetes service to register. + // This allows a pod to specify what Kubernetes service should trigger a Consul + // service registration in the case of multiple services referencing a deployment. + annotationKubernetesService = "consul.hashicorp.com/kubernetes-service" + // annotationPort is the name or value of the port to proxy incoming // connections to. annotationPort = "consul.hashicorp.com/connect-service-port" diff --git a/control-plane/connect-inject/endpoints_controller.go b/control-plane/connect-inject/endpoints_controller.go index 8213fa2278..f4dbfdc893 100644 --- a/control-plane/connect-inject/endpoints_controller.go +++ b/control-plane/connect-inject/endpoints_controller.go @@ -175,6 +175,12 @@ func (r *EndpointsController) Reconcile(ctx context.Context, req ctrl.Request) ( continue } + serviceName, ok := pod.Annotations[annotationKubernetesService] + if ok && serviceEndpoints.Name != serviceName { + r.Log.Info("ignoring endpoint because it doesn't match explicit service annotation", "name", serviceEndpoints.Name, "ns", serviceEndpoints.Namespace) + continue + } + if hasBeenInjected(pod) { endpointPods.Add(address.TargetRef.Name) if err := r.registerServicesAndHealthCheck(pod, serviceEndpoints, healthStatus, endpointAddressMap); err != nil { diff --git a/control-plane/connect-inject/endpoints_controller_test.go b/control-plane/connect-inject/endpoints_controller_test.go index 9b25c149b0..4f941f49f5 100644 --- a/control-plane/connect-inject/endpoints_controller_test.go +++ b/control-plane/connect-inject/endpoints_controller_test.go @@ -3373,6 +3373,119 @@ func TestReconcile_endpointsIgnoredWhenNotInjected(t *testing.T) { require.False(t, resp.Requeue) } +// Test that when an endpoints pod specifies the name for the Kubernetes service it wants to use +// for registration, all other endpoints for that pod are skipped. +func TestReconcile_podSpecifiesExplicitService(t *testing.T) { + nodeName := "test-node" + namespace := "default" + + // Set up the fake Kubernetes client with a few endpoints, pod, consul client, and the default namespace. + badEndpoint := &corev1.Endpoints{ + ObjectMeta: metav1.ObjectMeta{ + Name: "not-in-mesh", + Namespace: namespace, + }, + Subsets: []corev1.EndpointSubset{ + { + Addresses: []corev1.EndpointAddress{ + { + IP: "1.2.3.4", + NodeName: &nodeName, + TargetRef: &corev1.ObjectReference{ + Kind: "Pod", + Name: "pod1", + Namespace: namespace, + }, + }, + }, + }, + }, + } + endpoint := &corev1.Endpoints{ + ObjectMeta: metav1.ObjectMeta{ + Name: "in-mesh", + Namespace: namespace, + }, + Subsets: []corev1.EndpointSubset{ + { + Addresses: []corev1.EndpointAddress{ + { + IP: "1.2.3.4", + NodeName: &nodeName, + TargetRef: &corev1.ObjectReference{ + Kind: "Pod", + Name: "pod1", + Namespace: namespace, + }, + }, + }, + }, + }, + } + pod1 := createPod("pod1", "1.2.3.4", true, true) + pod1.Annotations[annotationKubernetesService] = endpoint.Name + fakeClientPod := createPod("fake-consul-client", "127.0.0.1", false, true) + fakeClientPod.Labels = map[string]string{"component": "client", "app": "consul", "release": "consul"} + ns := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}} + k8sObjects := []runtime.Object{badEndpoint, endpoint, pod1, fakeClientPod, &ns} + fakeClient := fake.NewClientBuilder().WithRuntimeObjects(k8sObjects...).Build() + + // Create test Consul server. + consul, err := testutil.NewTestServerConfigT(t, func(c *testutil.TestServerConfig) { c.NodeName = nodeName }) + require.NoError(t, err) + defer consul.Stop() + consul.WaitForServiceIntentions(t) + cfg := &api.Config{Address: consul.HTTPAddr} + consulClient, err := api.NewClient(cfg) + require.NoError(t, err) + addr := strings.Split(consul.HTTPAddr, ":") + consulPort := addr[1] + + // Create the endpoints controller. + ep := &EndpointsController{ + Client: fakeClient, + Log: logrtest.TestLogger{T: t}, + ConsulClient: consulClient, + ConsulPort: consulPort, + ConsulScheme: "http", + AllowK8sNamespacesSet: mapset.NewSetWith("*"), + DenyK8sNamespacesSet: mapset.NewSetWith(), + ReleaseName: "consul", + ReleaseNamespace: namespace, + ConsulClientCfg: cfg, + } + + // Run the reconcile process to check service registration. + serviceName := badEndpoint.Name + namespacedName := types.NamespacedName{Namespace: badEndpoint.Namespace, Name: serviceName} + resp, err := ep.Reconcile(context.Background(), ctrl.Request{NamespacedName: namespacedName}) + require.NoError(t, err) + require.False(t, resp.Requeue) + + // Check that no services are registered with Consul. + serviceInstances, _, err := consulClient.Catalog().Service(serviceName, "", nil) + require.NoError(t, err) + require.Len(t, serviceInstances, 0) + proxyServiceInstances, _, err := consulClient.Catalog().Service(serviceName+"-sidecar-proxy", "", nil) + require.NoError(t, err) + require.Len(t, proxyServiceInstances, 0) + + // Run the reconcile again with the service we want to register. + serviceName = endpoint.Name + namespacedName = types.NamespacedName{Namespace: badEndpoint.Namespace, Name: serviceName} + resp, err = ep.Reconcile(context.Background(), ctrl.Request{NamespacedName: namespacedName}) + require.NoError(t, err) + require.False(t, resp.Requeue) + + // Check that the correct services are registered with Consul. + serviceInstances, _, err = consulClient.Catalog().Service(serviceName, "", nil) + require.NoError(t, err) + require.Len(t, serviceInstances, 1) + proxyServiceInstances, _, err = consulClient.Catalog().Service(serviceName+"-sidecar-proxy", "", nil) + require.NoError(t, err) + require.Len(t, proxyServiceInstances, 1) +} + func TestFilterAgentPods(t *testing.T) { t.Parallel() cases := map[string]struct { From c913f3f3ac870d42570fae1ae7ae296310bace2a Mon Sep 17 00:00:00 2001 From: Andrew Stucki Date: Thu, 7 Apr 2022 11:09:19 -0400 Subject: [PATCH 2/4] Add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d66cabc8d..71b05306e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ IMPROVEMENTS: * Helm * API Gateway: Allow controller to read Kubernetes namespaces in order to determine if route is allowed for gateway. [[GH-1092](https://github.com/hashicorp/consul-k8s/pull/1092)] * Support a pre-configured bootstrap ACL token. [[GH-1125](https://github.com/hashicorp/consul-k8s/pull/1125)] + * Add a `"consul.hashicorp.com/kubernetes-service"` annotation for pods to specify which Kubernetes service they want to use for registration when multiple services target the same pod. [[GH-1150](https://github.com/hashicorp/consul-k8s/pull/1150)] * Vault * Enable snapshot agent configuration to be retrieved from vault. [[GH-1113](https://github.com/hashicorp/consul-k8s/pull/1113)] * CLI From 5edbd65b1708a190a80c7271b2379c1f7ad864ce Mon Sep 17 00:00:00 2001 From: Andrew Stucki Date: Thu, 7 Apr 2022 11:41:27 -0400 Subject: [PATCH 3/4] Add deregistration test and comment --- .../connect-inject/endpoints_controller.go | 1 + .../endpoints_controller_test.go | 25 ++++++++++++++++--- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/control-plane/connect-inject/endpoints_controller.go b/control-plane/connect-inject/endpoints_controller.go index f4dbfdc893..aa55663b8a 100644 --- a/control-plane/connect-inject/endpoints_controller.go +++ b/control-plane/connect-inject/endpoints_controller.go @@ -178,6 +178,7 @@ func (r *EndpointsController) Reconcile(ctx context.Context, req ctrl.Request) ( serviceName, ok := pod.Annotations[annotationKubernetesService] if ok && serviceEndpoints.Name != serviceName { r.Log.Info("ignoring endpoint because it doesn't match explicit service annotation", "name", serviceEndpoints.Name, "ns", serviceEndpoints.Namespace) + // deregistration happens later because we don't add this pod to the endpointAddressMap continue } diff --git a/control-plane/connect-inject/endpoints_controller_test.go b/control-plane/connect-inject/endpoints_controller_test.go index 4f941f49f5..da198afa31 100644 --- a/control-plane/connect-inject/endpoints_controller_test.go +++ b/control-plane/connect-inject/endpoints_controller_test.go @@ -3455,15 +3455,34 @@ func TestReconcile_podSpecifiesExplicitService(t *testing.T) { ConsulClientCfg: cfg, } - // Run the reconcile process to check service registration. serviceName := badEndpoint.Name + + // Initially register the pod with the bad endpoint + err = consulClient.Agent().ServiceRegister(&api.AgentServiceRegistration{ + ID: "pod1-" + serviceName, + Name: serviceName, + Port: 0, + Address: "1.2.3.4", + Meta: map[string]string{ + "k8s-namespace": namespace, + "k8s-service-name": serviceName, + "managed-by": "consul-k8s-endpoints-controller", + "pod-name": "pod1", + }, + }) + require.NoError(t, err) + serviceInstances, _, err := consulClient.Catalog().Service(serviceName, "", nil) + require.NoError(t, err) + require.Len(t, serviceInstances, 1) + + // Run the reconcile process to check service deregistration. namespacedName := types.NamespacedName{Namespace: badEndpoint.Namespace, Name: serviceName} resp, err := ep.Reconcile(context.Background(), ctrl.Request{NamespacedName: namespacedName}) require.NoError(t, err) require.False(t, resp.Requeue) - // Check that no services are registered with Consul. - serviceInstances, _, err := consulClient.Catalog().Service(serviceName, "", nil) + // Check that the service has been deregistered with Consul. + serviceInstances, _, err = consulClient.Catalog().Service(serviceName, "", nil) require.NoError(t, err) require.Len(t, serviceInstances, 0) proxyServiceInstances, _, err := consulClient.Catalog().Service(serviceName+"-sidecar-proxy", "", nil) From 21a88a8cc5bc9a3e5a3cf651d8c032d48ef56eab Mon Sep 17 00:00:00 2001 From: Andrew Stucki Date: Mon, 11 Apr 2022 10:09:43 -0400 Subject: [PATCH 4/4] Apply suggestions from code review Co-authored-by: Nitya Dhanushkodi --- control-plane/connect-inject/endpoints_controller.go | 2 +- control-plane/connect-inject/endpoints_controller_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/control-plane/connect-inject/endpoints_controller.go b/control-plane/connect-inject/endpoints_controller.go index aa55663b8a..4ad4f48e95 100644 --- a/control-plane/connect-inject/endpoints_controller.go +++ b/control-plane/connect-inject/endpoints_controller.go @@ -178,7 +178,7 @@ func (r *EndpointsController) Reconcile(ctx context.Context, req ctrl.Request) ( serviceName, ok := pod.Annotations[annotationKubernetesService] if ok && serviceEndpoints.Name != serviceName { r.Log.Info("ignoring endpoint because it doesn't match explicit service annotation", "name", serviceEndpoints.Name, "ns", serviceEndpoints.Namespace) - // deregistration happens later because we don't add this pod to the endpointAddressMap + // deregistration for service instances that don't match the annotation happens later because we don't add this pod to the endpointAddressMap. continue } diff --git a/control-plane/connect-inject/endpoints_controller_test.go b/control-plane/connect-inject/endpoints_controller_test.go index da198afa31..fbc3c5f3b6 100644 --- a/control-plane/connect-inject/endpoints_controller_test.go +++ b/control-plane/connect-inject/endpoints_controller_test.go @@ -3491,7 +3491,7 @@ func TestReconcile_podSpecifiesExplicitService(t *testing.T) { // Run the reconcile again with the service we want to register. serviceName = endpoint.Name - namespacedName = types.NamespacedName{Namespace: badEndpoint.Namespace, Name: serviceName} + namespacedName = types.NamespacedName{Namespace: endpoint.Namespace, Name: serviceName} resp, err = ep.Reconcile(context.Background(), ctrl.Request{NamespacedName: namespacedName}) require.NoError(t, err) require.False(t, resp.Requeue)