fixed acl deletetion in endpoints controller (#3210) (#3278)

* fixed acl deletetion in endpoints controller * fixed tests * fixed other tests * fixed ent tests * added changelog * updated TestReconcileDeleteEndpoint to support deleting token by pod uid * passed pod-uid to dataplane * fixed tests * fixed more tests * fixed dataplane env * fixed test * fixed passing env to dataplane * fixed unit test
hashicorp · Nov 28, 2023 · aef3271 · aef3271
1 parent 848cfd1
commit aef3271
Show file tree

Hide file tree

Showing 7 changed files with 104 additions and 36 deletions.
diff --git a/.changelog/3210.txt b/.changelog/3210.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+control-plane: Only delete ACL tokens matched Pod UID in Service Registration metadata 
+```
diff --git a/control-plane/connect-inject/constants/constants.go b/control-plane/connect-inject/constants/constants.go
@@ -16,6 +16,9 @@ const (
 	// MetaKeyPodName is the meta key name for Kubernetes pod name used for the Consul services.
 	MetaKeyPodName = "pod-name"
 
+	// MetaKeyPodUID is the meta key name for Kubernetes pod uid used for the Consul services.
+	MetaKeyPodUID = "pod-uid"
+
 	// DefaultGracefulPort is the default port that consul-dataplane uses for graceful shutdown.
 	DefaultGracefulPort = 20600
 

diff --git a/control-plane/connect-inject/controllers/endpoints/endpoints_controller.go b/control-plane/connect-inject/controllers/endpoints/endpoints_controller.go
@@ -407,6 +407,7 @@ func (r *Controller) createServiceRegistrations(pod corev1.Pod, serviceEndpoints
 		metaKeyKubeServiceName:   serviceEndpoints.Name,
 		constants.MetaKeyKubeNS:  serviceEndpoints.Namespace,
 		metaKeyManagedBy:         constants.ManagedByValue,
+		constants.MetaKeyPodUID:  string(pod.UID),
 		metaKeySyntheticNode:     "true",
 	}
 	for k, v := range pod.Annotations {
@@ -654,6 +655,7 @@ func (r *Controller) createGatewayRegistrations(pod corev1.Pod, serviceEndpoints
 		constants.MetaKeyKubeNS:  serviceEndpoints.Namespace,
 		metaKeyManagedBy:         constants.ManagedByValue,
 		metaKeySyntheticNode:     "true",
+		constants.MetaKeyPodUID:  string(pod.UID),
 	}
 
 	service := &api.AgentService{
@@ -933,7 +935,7 @@ func (r *Controller) deregisterService(apiClient *api.Client, k8sSvcName, k8sSvc
 
 			if r.AuthMethod != "" && serviceDeregistered {
 				r.Log.Info("reconciling ACL tokens for service", "svc", svc.Service)
-				err := r.deleteACLTokensForServiceInstance(apiClient, svc, k8sSvcNamespace, svc.Meta[constants.MetaKeyPodName])
+				err := r.deleteACLTokensForServiceInstance(apiClient, svc, k8sSvcNamespace, svc.Meta[constants.MetaKeyPodName], svc.Meta[constants.MetaKeyPodUID])
 				if err != nil {
 					r.Log.Error(err, "failed to reconcile ACL tokens for service", "svc", svc.Service)
 					errs = multierror.Append(errs, err)
@@ -948,8 +950,8 @@ func (r *Controller) deregisterService(apiClient *api.Client, k8sSvcName, k8sSvc
 
 // deleteACLTokensForServiceInstance finds the ACL tokens that belongs to the service instance and deletes it from Consul.
 // It will only check for ACL tokens that have been created with the auth method this controller
-// has been configured with and will only delete tokens for the provided podName.
-func (r *Controller) deleteACLTokensForServiceInstance(apiClient *api.Client, svc *api.AgentService, k8sNS, podName string) error {
+// has been configured with and will only delete tokens for the provided podName and podUID.
+func (r *Controller) deleteACLTokensForServiceInstance(apiClient *api.Client, svc *api.AgentService, k8sNS, podName, podUID string) error {
 	// Skip if podName is empty.
 	if podName == "" {
 		return nil
@@ -985,8 +987,11 @@ func (r *Controller) deleteACLTokensForServiceInstance(apiClient *api.Client, sv
 
 			tokenPodName := strings.TrimPrefix(tokenMeta[tokenMetaPodNameKey], k8sNS+"/")
 
+			// backward compability logic on token with no podUID in metadata
+			podUIDMatched := tokenMeta[constants.MetaKeyPodUID] == podUID || tokenMeta[constants.MetaKeyPodUID] == ""
+
 			// If we can't find token's pod, delete it.
-			if tokenPodName == podName {
+			if tokenPodName == podName && podUIDMatched {
 				r.Log.Info("deleting ACL token for pod", "name", podName)
 				if _, err := apiClient.ACL().TokenDelete(token.AccessorID, &api.WriteOptions{Namespace: svc.Namespace}); err != nil {
 					return fmt.Errorf("failed to delete token from Consul: %s", err)

diff --git a/control-plane/connect-inject/controllers/endpoints/endpoints_controller_ent_test.go b/control-plane/connect-inject/controllers/endpoints/endpoints_controller_ent_test.go
@@ -125,15 +125,15 @@ func TestReconcileCreateEndpointWithNamespaces(t *testing.T) {
 					ServiceID:      "pod1-service-created",
 					ServiceName:    "service-created",
 					ServiceAddress: "1.2.3.4",
-					ServiceMeta:    map[string]string{constants.MetaKeyPodName: "pod1", metaKeyKubeServiceName: "service-created", constants.MetaKeyKubeNS: testCase.SourceKubeNS, metaKeyManagedBy: constants.ManagedByValue, metaKeySyntheticNode: "true"},
+					ServiceMeta:    map[string]string{constants.MetaKeyPodName: "pod1", metaKeyKubeServiceName: "service-created", constants.MetaKeyKubeNS: testCase.SourceKubeNS, metaKeyManagedBy: constants.ManagedByValue, metaKeySyntheticNode: "true", constants.MetaKeyPodUID: ""},
 					ServiceTags:    []string{},
 					Namespace:      testCase.ExpConsulNS,
 				},
 				{
 					ServiceID:      "pod2-service-created",
 					ServiceName:    "service-created",
 					ServiceAddress: "2.2.3.4",
-					ServiceMeta:    map[string]string{constants.MetaKeyPodName: "pod2", metaKeyKubeServiceName: "service-created", constants.MetaKeyKubeNS: testCase.SourceKubeNS, metaKeyManagedBy: constants.ManagedByValue, metaKeySyntheticNode: "true"},
+					ServiceMeta:    map[string]string{constants.MetaKeyPodName: "pod2", metaKeyKubeServiceName: "service-created", constants.MetaKeyKubeNS: testCase.SourceKubeNS, metaKeyManagedBy: constants.ManagedByValue, metaKeySyntheticNode: "true", constants.MetaKeyPodUID: ""},
 					ServiceTags:    []string{},
 					Namespace:      testCase.ExpConsulNS,
 				},
@@ -148,7 +148,7 @@ func TestReconcileCreateEndpointWithNamespaces(t *testing.T) {
 						DestinationServiceName: "service-created",
 						DestinationServiceID:   "pod1-service-created",
 					},
-					ServiceMeta: map[string]string{constants.MetaKeyPodName: "pod1", metaKeyKubeServiceName: "service-created", constants.MetaKeyKubeNS: testCase.SourceKubeNS, metaKeyManagedBy: constants.ManagedByValue, metaKeySyntheticNode: "true"},
+					ServiceMeta: map[string]string{constants.MetaKeyPodName: "pod1", metaKeyKubeServiceName: "service-created", constants.MetaKeyKubeNS: testCase.SourceKubeNS, metaKeyManagedBy: constants.ManagedByValue, metaKeySyntheticNode: "true", constants.MetaKeyPodUID: ""},
 					ServiceTags: []string{},
 					Namespace:   testCase.ExpConsulNS,
 				},
@@ -161,7 +161,7 @@ func TestReconcileCreateEndpointWithNamespaces(t *testing.T) {
 						DestinationServiceName: "service-created",
 						DestinationServiceID:   "pod2-service-created",
 					},
-					ServiceMeta: map[string]string{constants.MetaKeyPodName: "pod2", metaKeyKubeServiceName: "service-created", constants.MetaKeyKubeNS: testCase.SourceKubeNS, metaKeyManagedBy: constants.ManagedByValue, metaKeySyntheticNode: "true"},
+					ServiceMeta: map[string]string{constants.MetaKeyPodName: "pod2", metaKeyKubeServiceName: "service-created", constants.MetaKeyKubeNS: testCase.SourceKubeNS, metaKeyManagedBy: constants.ManagedByValue, metaKeySyntheticNode: "true", constants.MetaKeyPodUID: ""},
 					ServiceTags: []string{},
 					Namespace:   testCase.ExpConsulNS,
 				},
@@ -396,7 +396,7 @@ func TestReconcileCreateGatewayWithNamespaces(t *testing.T) {
 					ServiceID:      "mesh-gateway",
 					ServiceName:    "mesh-gateway",
 					ServiceAddress: "3.3.3.3",
-					ServiceMeta:    map[string]string{constants.MetaKeyPodName: "mesh-gateway", metaKeyKubeServiceName: "gateway", constants.MetaKeyKubeNS: "default", metaKeyManagedBy: constants.ManagedByValue, metaKeySyntheticNode: "true"},
+					ServiceMeta:    map[string]string{constants.MetaKeyPodName: "mesh-gateway", metaKeyKubeServiceName: "gateway", constants.MetaKeyKubeNS: "default", metaKeyManagedBy: constants.ManagedByValue, metaKeySyntheticNode: "true", constants.MetaKeyPodUID: ""},
 					ServiceTags:    []string{},
 					ServicePort:    8443,
 					ServiceTaggedAddresses: map[string]api.ServiceAddress{
@@ -415,7 +415,7 @@ func TestReconcileCreateGatewayWithNamespaces(t *testing.T) {
 					ServiceID:      "terminating-gateway",
 					ServiceName:    "terminating-gateway",
 					ServiceAddress: "4.4.4.4",
-					ServiceMeta:    map[string]string{constants.MetaKeyPodName: "terminating-gateway", metaKeyKubeServiceName: "gateway", constants.MetaKeyKubeNS: "default", metaKeyManagedBy: constants.ManagedByValue, metaKeySyntheticNode: "true"},
+					ServiceMeta:    map[string]string{constants.MetaKeyPodName: "terminating-gateway", metaKeyKubeServiceName: "gateway", constants.MetaKeyKubeNS: "default", metaKeyManagedBy: constants.ManagedByValue, metaKeySyntheticNode: "true", constants.MetaKeyPodUID: ""},
 					ServiceTags:    []string{},
 					ServicePort:    8443,
 					Namespace:      testCase.ConsulNS,
@@ -424,7 +424,7 @@ func TestReconcileCreateGatewayWithNamespaces(t *testing.T) {
 					ServiceID:      "ingress-gateway",
 					ServiceName:    "ingress-gateway",
 					ServiceAddress: "5.5.5.5",
-					ServiceMeta:    map[string]string{constants.MetaKeyPodName: "ingress-gateway", metaKeyKubeServiceName: "gateway", constants.MetaKeyKubeNS: "default", metaKeyManagedBy: constants.ManagedByValue, metaKeySyntheticNode: "true"},
+					ServiceMeta:    map[string]string{constants.MetaKeyPodName: "ingress-gateway", metaKeyKubeServiceName: "gateway", constants.MetaKeyKubeNS: "default", metaKeyManagedBy: constants.ManagedByValue, metaKeySyntheticNode: "true", constants.MetaKeyPodUID: ""},
 					ServiceTags:    []string{},
 					ServicePort:    21000,
 					ServiceTaggedAddresses: map[string]api.ServiceAddress{
@@ -1315,6 +1315,7 @@ func TestReconcileUpdateEndpointWithNamespaces(t *testing.T) {
 								constants.MetaKeyPodName: "pod1",
 								constants.MetaKeyKubeNS:  ts.SourceKubeNS,
 								metaKeySyntheticNode:     "true",
+								constants.MetaKeyPodUID:  "",
 							},
 							Namespace: ts.ExpConsulNS,
 						},
@@ -1341,6 +1342,7 @@ func TestReconcileUpdateEndpointWithNamespaces(t *testing.T) {
 								constants.MetaKeyPodName: "pod1",
 								constants.MetaKeyKubeNS:  ts.SourceKubeNS,
 								metaKeySyntheticNode:     "true",
+								constants.MetaKeyPodUID:  "",
 							},
 							Namespace: ts.ExpConsulNS,
 						},
@@ -1407,6 +1409,7 @@ func TestReconcileUpdateEndpointWithNamespaces(t *testing.T) {
 								metaKeyManagedBy:         constants.ManagedByValue,
 								constants.MetaKeyPodName: "pod1",
 								metaKeySyntheticNode:     "true",
+								constants.MetaKeyPodUID:  "",
 							},
 							Namespace: ts.ExpConsulNS,
 						},
@@ -1433,6 +1436,7 @@ func TestReconcileUpdateEndpointWithNamespaces(t *testing.T) {
 								metaKeyManagedBy:         constants.ManagedByValue,
 								constants.MetaKeyPodName: "pod1",
 								metaKeySyntheticNode:     "true",
+								constants.MetaKeyPodUID:  "",
 							},
 							Namespace: ts.ExpConsulNS,
 						},
@@ -1454,6 +1458,7 @@ func TestReconcileUpdateEndpointWithNamespaces(t *testing.T) {
 								metaKeyManagedBy:         constants.ManagedByValue,
 								constants.MetaKeyPodName: "pod2",
 								metaKeySyntheticNode:     "true",
+								constants.MetaKeyPodUID:  "",
 							},
 							Namespace: ts.ExpConsulNS,
 						},
@@ -1480,6 +1485,7 @@ func TestReconcileUpdateEndpointWithNamespaces(t *testing.T) {
 								metaKeyManagedBy:         constants.ManagedByValue,
 								constants.MetaKeyPodName: "pod2",
 								metaKeySyntheticNode:     "true",
+								constants.MetaKeyPodUID:  "",
 							},
 							Namespace: ts.ExpConsulNS,
 						},