From 7588e22739bc3a8ad96f259b1342485cfa00bb9d Mon Sep 17 00:00:00 2001
From: Kyle Havlovitz <kylehav@gmail.com>
Date: Thu, 10 Sep 2020 06:12:48 -0700
Subject: [PATCH] Add a stop function to make sure the renewer is shut down on
 leader change

---
 agent/connect/ca/provider.go       |  8 ++++++++
 agent/connect/ca/provider_vault.go | 25 +++++++++++++++++++------
 agent/consul/leader_connect.go     | 11 +++++++++++
 3 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/agent/connect/ca/provider.go b/agent/connect/ca/provider.go
index 75f48302c..f9e637c41 100644
--- a/agent/connect/ca/provider.go
+++ b/agent/connect/ca/provider.go
@@ -164,3 +164,11 @@ type NeedsLogger interface {
 	// SetLogger will pass a configured Logger to the provider.
 	SetLogger(logger hclog.Logger)
 }
+
+// NeedsStop is an optional interface that allows a CA to define a function
+// to be called when the CA instance is no longer in use. This is different
+// from Cleanup(), as only the local provider instance is being shut down
+// such as in the case of a leader change.
+type NeedsStop interface {
+	Stop()
+}
diff --git a/agent/connect/ca/provider_vault.go b/agent/connect/ca/provider_vault.go
index 4f05edd57..f899eb690 100644
--- a/agent/connect/ca/provider_vault.go
+++ b/agent/connect/ca/provider_vault.go
@@ -9,6 +9,7 @@ import (
 	"io/ioutil"
 	"net/http"
 	"strings"
+	"sync"
 
 	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/agent/structs"
@@ -26,7 +27,10 @@ var ErrBackendNotInitialized = fmt.Errorf("backend not initialized")
 type VaultProvider struct {
 	config *structs.VaultCAProviderConfig
 	client *vaultapi.Client
-	doneCh chan struct{}
+
+	shutdown     bool
+	shutdownCh   chan struct{}
+	shutdownLock sync.RWMutex
 
 	isPrimary                    bool
 	clusterID                    string
@@ -71,7 +75,7 @@ func (v *VaultProvider) Configure(cfg ProviderConfig) error {
 	v.isPrimary = cfg.IsPrimary
 	v.clusterID = cfg.ClusterID
 	v.spiffeID = connect.SpiffeIDSigningForCluster(&structs.CAConfiguration{ClusterID: v.clusterID})
-	v.doneCh = make(chan struct{}, 0)
+	v.shutdownCh = make(chan struct{}, 0)
 
 	// Look up the token to see if we can auto-renew its lease.
 	secret, err := client.Auth().Token().Lookup(config.Token)
@@ -116,7 +120,7 @@ func (v *VaultProvider) renewToken(renewer *vaultapi.Renewer) {
 
 	for {
 		select {
-		case <-v.doneCh:
+		case <-v.shutdownCh:
 			renewer.Stop()
 			return
 
@@ -501,13 +505,22 @@ func (c *VaultProvider) SupportsCrossSigning() (bool, error) {
 // this down and recreate it on small config changes because the intermediate
 // certs get bundled with the leaf certs, so there's no cost to the CA changing.
 func (v *VaultProvider) Cleanup() error {
-	if v.doneCh != nil {
-		close(v.doneCh)
-	}
+	v.Stop()
 
 	return v.client.Sys().Unmount(v.config.IntermediatePKIPath)
 }
 
+// Stop shuts down the token renew goroutine.
+func (v *VaultProvider) Stop() {
+	v.shutdownLock.Lock()
+	defer v.shutdownLock.Unlock()
+
+	if !v.shutdown && v.shutdownCh != nil {
+		close(v.shutdownCh)
+		v.shutdown = true
+	}
+}
+
 func ParseVaultCAConfig(raw map[string]interface{}) (*structs.VaultCAProviderConfig, error) {
 	config := structs.VaultCAProviderConfig{
 		CommonCAProviderConfig: defaultCommonConfig(),
diff --git a/agent/consul/leader_connect.go b/agent/consul/leader_connect.go
index fcad53d65..b9049ed88 100644
--- a/agent/consul/leader_connect.go
+++ b/agent/consul/leader_connect.go
@@ -567,6 +567,17 @@ func (s *Server) startConnectLeader() {
 
 // stopConnectLeader stops connect specific leader functions.
 func (s *Server) stopConnectLeader() {
+	s.caProviderReconfigurationLock.Lock()
+	defer s.caProviderReconfigurationLock.Unlock()
+
+	// If the provider implements NeedsStop, we call Stop to perform any shutdown actions.
+	provider, _ := s.getCAProvider()
+	if provider != nil {
+		if needsStop, ok := provider.(ca.NeedsStop); ok {
+			needsStop.Stop()
+		}
+	}
+
 	s.leaderRoutineManager.Stop(secondaryCARootWatchRoutineName)
 	s.leaderRoutineManager.Stop(intentionReplicationRoutineName)
 	s.leaderRoutineManager.Stop(caRootPruningRoutineName)