Skip to content

Commit

Permalink
connect: endpoints controller deletes ACL token when service is dereg…
Browse files Browse the repository at this point in the history
…istered (hashicorp#571)

Fixes hashicorp#540

* Modify endpoints controller to delete ACL tokens for each service instance that it deregisters
* Remove TLS+ACLs table tests from endpoints controller tests. These tests were testing that endpoints controller works with a client configured to have TLS and ACLs. I thought this test was not necessary because there isn't any code in the controller that behaves differently if the consul client is configured with any of those and as a result there's no way these tests could fail. The tests testing to the new ACL logic are there but they are only testing the logic that was added and configure test agent to accommodate for that.
* Create test package under helper and move GenerateServerCerts function from subcommand/common there because it was used outside of subcommand.
* Create a helper test function to set up auth methods and refactor existing connect-init command tests to use that function.
* Minor editing fixes of comments etc.
  • Loading branch information
ishustava authored Jul 29, 2021
1 parent 8946d1a commit 81c05aa
Show file tree
Hide file tree
Showing 15 changed files with 1,244 additions and 559 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ BUG FIXES:
* Connect: Use `AdmissionregistrationV1` instead of `AdmissionregistrationV1beta1` API as it was deprecated in k8s 1.16. [[GH-558](https://github.com/hashicorp/consul-k8s/pull/558)]
* Connect: Fix bug where environment variables `<NAME>_CONNECT_SERVICE_HOST` and
`<NAME>_CONNECT_SERVICE_PORT` weren't being set when the upstream annotation was used. [[GH-549](https://github.com/hashicorp/consul-k8s/issues/549)]
* Connect: Fix a bug with leaving around ACL tokens after a service has been deregistered. [[GH-571](https://github.com/hashicorp/consul-k8s/issues/540)]
* CRDs: Fix ProxyDefaults and ServiceDefaults resources not syncing with Consul < 1.10.0 [[GH-1023](https://github.com/hashicorp/consul-helm/issues/1023)]

## 0.26.0 (June 22, 2021)
Expand Down
75 changes: 74 additions & 1 deletion connect-inject/endpoints_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/json"
"fmt"
"net"
"regexp"
"strings"

"github.com/deckarep/golang-set"
Expand Down Expand Up @@ -34,13 +35,13 @@ const (
MetaKeyKubeServiceName = "k8s-service-name"
MetaKeyKubeNS = "k8s-namespace"
MetaKeyManagedBy = "managed-by"
TokenMetaPodNameKey = "pod"
kubernetesSuccessReasonMsg = "Kubernetes health checks passing"
envoyPrometheusBindAddr = "envoy_prometheus_bind_addr"
envoySidecarContainer = "envoy-sidecar"

// clusterIPTaggedAddressName is the key for the tagged address to store the service's cluster IP and service port
// in Consul. Note: This value should not be changed without a corresponding change in Consul.
// TODO: change this to a constant shared with Consul to avoid accidentally changing this.
clusterIPTaggedAddressName = "virtual"

// exposedPathsLivenessPortsRangeStart is the start of the port range that we will use as
Expand Down Expand Up @@ -100,6 +101,11 @@ type EndpointsController struct {
// TProxyOverwriteProbes controls whether the endpoints controller should expose pod's HTTP probes
// via Envoy proxy.
TProxyOverwriteProbes bool
// AuthMethod is the name of the Kubernetes Auth Method that
// was used to login with Consul. The Endpoints controller
// will delete any tokens associated with this auth method
// whenever service instances are deregistered.
AuthMethod string

MetricsConfig MetricsConfig
Log logr.Logger
Expand Down Expand Up @@ -718,11 +724,78 @@ func (r *EndpointsController) deregisterServiceOnAllAgents(ctx context.Context,
return err
}
}

if r.AuthMethod != "" {
r.Log.Info("reconciling ACL tokens for service", "svc", serviceRegistration.Service)
err = r.reconcileACLTokensForService(client, serviceRegistration.Service, k8sSvcNamespace)
if err != nil {
r.Log.Error(err, "failed to reconcile ACL tokens for service", "svc", serviceRegistration.Service)
return err
}
}
}
}
return nil
}

// reconcileACLTokensForService finds the ACL tokens that belongs to the service 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.
func (r *EndpointsController) reconcileACLTokensForService(client *api.Client, serviceName, k8sNS string) error {
tokens, _, err := client.ACL().TokenList(nil)
if err != nil {
return fmt.Errorf("failed to get a list of tokens from Consul: %s", err)
}

for _, token := range tokens {
// Only delete tokens that:
// * have been created with the auth method configured for this endpoints controller
// * have a single service identity whose service name is the same as 'serviceName'
if token.AuthMethod == r.AuthMethod &&
len(token.ServiceIdentities) == 1 &&
token.ServiceIdentities[0].ServiceName == serviceName {
tokenMeta, err := getTokenMetaFromDescription(token.Description)
if err != nil {
return fmt.Errorf("failed to parse token metadata: %s", err)
}

podName := strings.TrimPrefix(tokenMeta[TokenMetaPodNameKey], k8sNS+"/")
err = r.Client.Get(r.Context, types.NamespacedName{Name: podName, Namespace: k8sNS}, &corev1.Pod{})
// If we can't find token's pod, delete it.
if err != nil && k8serrors.IsNotFound(err) {
r.Log.Info("deleting ACL token for pod", "name", podName)
_, err = client.ACL().TokenDelete(token.AccessorID, nil)
if err != nil {
return fmt.Errorf("failed to delete token from Consul: %s", err)
}
} else if err != nil {
return err
}
}
}

return nil
}

// getTokenMetaFromDescription parses JSON metadata from token's description.
func getTokenMetaFromDescription(description string) (map[string]string, error) {
re := regexp.MustCompile(`.*({.+})`)

matches := re.FindStringSubmatch(description)
if len(matches) != 2 {
return nil, fmt.Errorf("failed to extract token metadata from description: %s", description)
}
tokenMetaJSON := matches[1]

var tokenMeta map[string]string
err := json.Unmarshal([]byte(tokenMetaJSON), &tokenMeta)
if err != nil {
return nil, fmt.Errorf("failed to unmarshal token metadata '%s': %s", tokenMetaJSON, err)
}

return tokenMeta, nil
}

// serviceInstancesForK8SServiceNameAndNamespace calls Consul's ServicesWithFilter to get the list
// of services instances that have the provided k8sServiceName and k8sServiceNamespace in their metadata.
func serviceInstancesForK8SServiceNameAndNamespace(k8sServiceName, k8sServiceNamespace string, client *api.Client) (map[string]*api.AgentService, error) {
Expand Down
Loading

0 comments on commit 81c05aa

Please sign in to comment.