Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Nov 28, 2023
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/3210.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
control-plane: Only delete ACL tokens matched Pod UID in Service Registration metadata
```
3 changes: 3 additions & 0 deletions control-plane/connect-inject/constants/constants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
13 changes: 9 additions & 4 deletions control-plane/connect-inject/controllers/endpoints/endpoints_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,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 {
Expand Down Expand Up @@ -645,6 +646,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{
Expand Down Expand Up @@ -917,7 +919,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)
Expand All @@ -932,8 +934,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
Expand Down Expand Up @@ -969,8 +971,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)
Expand Down
20 changes: 13 additions & 7 deletions control-plane/connect-inject/controllers/endpoints/endpoints_controller_ent_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,15 +122,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,
},
Expand All @@ -145,7 +145,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,
},
Expand All @@ -158,7 +158,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,
},
Expand Down Expand Up @@ -393,7 +393,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{
Expand All @@ -412,7 +412,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,
Expand All @@ -421,7 +421,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{
Expand Down Expand Up @@ -1312,6 +1312,7 @@ func TestReconcileUpdateEndpointWithNamespaces(t *testing.T) {
constants.MetaKeyPodName: "pod1",
constants.MetaKeyKubeNS: ts.SourceKubeNS,
metaKeySyntheticNode: "true",
constants.MetaKeyPodUID: "",
},
Namespace: ts.ExpConsulNS,
},
Expand All @@ -1338,6 +1339,7 @@ func TestReconcileUpdateEndpointWithNamespaces(t *testing.T) {
constants.MetaKeyPodName: "pod1",
constants.MetaKeyKubeNS: ts.SourceKubeNS,
metaKeySyntheticNode: "true",
constants.MetaKeyPodUID: "",
},
Namespace: ts.ExpConsulNS,
},
Expand Down Expand Up @@ -1404,6 +1406,7 @@ func TestReconcileUpdateEndpointWithNamespaces(t *testing.T) {
metaKeyManagedBy: constants.ManagedByValue,
constants.MetaKeyPodName: "pod1",
metaKeySyntheticNode: "true",
constants.MetaKeyPodUID: "",
},
Namespace: ts.ExpConsulNS,
},
Expand All @@ -1430,6 +1433,7 @@ func TestReconcileUpdateEndpointWithNamespaces(t *testing.T) {
metaKeyManagedBy: constants.ManagedByValue,
constants.MetaKeyPodName: "pod1",
metaKeySyntheticNode: "true",
constants.MetaKeyPodUID: "",
},
Namespace: ts.ExpConsulNS,
},
Expand All @@ -1451,6 +1455,7 @@ func TestReconcileUpdateEndpointWithNamespaces(t *testing.T) {
metaKeyManagedBy: constants.ManagedByValue,
constants.MetaKeyPodName: "pod2",
metaKeySyntheticNode: "true",
constants.MetaKeyPodUID: "",
},
Namespace: ts.ExpConsulNS,
},
Expand All @@ -1477,6 +1482,7 @@ func TestReconcileUpdateEndpointWithNamespaces(t *testing.T) {
metaKeyManagedBy: constants.ManagedByValue,
constants.MetaKeyPodName: "pod2",
metaKeySyntheticNode: "true",
constants.MetaKeyPodUID: "",
},
Namespace: ts.ExpConsulNS,
},
Expand Down
Loading
Loading