add a rate limiter to config auto-reload

agent/agent.go

@@ -361,9 +361,9 @@ type Agent struct {
 	// run by the Agent
 	routineManager *routine.Manager
 
-	// FileWatcher is the watcher responsible to report events when a config file
+	// configFileWatcher is the watcher responsible to report events when a config file
 	// changed
-	FileWatcher config.Watcher
+	configFileWatcher config.Watcher
 
 	// xdsServer serves the XDS protocol for configuring Envoy proxies.
 	xdsServer *xds.Server
@@ -462,6 +462,13 @@ func New(bd BaseDeps) (*Agent, error) {
 			a.baseDeps.WatchedFiles = append(a.baseDeps.WatchedFiles, f.Cfg.CertFile)
 		}
 	}
+	if a.baseDeps.RuntimeConfig.AutoReloadConfig && len(a.baseDeps.WatchedFiles) > 0 {
+		w, err := config.NewRateLimitedFileWatcher(a.baseDeps.WatchedFiles, a.baseDeps.Logger, a.baseDeps.RuntimeConfig.AutoReloadConfigCoalesceInterval)
+		if err != nil {
+			a.baseDeps.Logger.Error("error loading config", "error", err)
+		}
+		a.configFileWatcher = w
+	}
 
 	return &a, nil
 }
@@ -713,25 +720,20 @@ func (a *Agent) Start(ctx context.Context) error {
 	})
 
 	// start a go routine to reload config based on file watcher events
-	if a.baseDeps.RuntimeConfig.AutoReloadConfig && len(a.baseDeps.WatchedFiles) > 0 {
-		w, err := config.NewFileWatcher(a.baseDeps.WatchedFiles, a.baseDeps.Logger)
-		if err != nil {
-			a.baseDeps.Logger.Error("error loading config", "error", err)
-		} else {
-			a.FileWatcher = w
-			a.baseDeps.Logger.Debug("starting file watcher")
-			a.FileWatcher.Start(context.Background())
-			go func() {
-				for event := range a.FileWatcher.EventsCh() {
-					a.baseDeps.Logger.Debug("auto-reload config triggered", "event-file", event.Filename)
-					err := a.AutoReloadConfig()
-					if err != nil {
-						a.baseDeps.Logger.Error("error loading config", "error", err)
-					}
+	if a.configFileWatcher != nil {
+		a.baseDeps.Logger.Debug("starting file watcher")
+		a.configFileWatcher.Start(context.Background())
+		go func() {
+			for event := range a.configFileWatcher.EventsCh() {
+				a.baseDeps.Logger.Debug("auto-reload config triggered", "num-events", len(event.Filenames))
+				err := a.AutoReloadConfig()
+				if err != nil {
+					a.baseDeps.Logger.Error("error loading config", "error", err)
 				}
-			}()
-		}
+			}
+		}()
 	}
+
 	return nil
 }
 
@@ -1413,8 +1415,8 @@ func (a *Agent) ShutdownAgent() error {
 	a.stopAllWatches()
 
 	// Stop config file watcher
-	if a.FileWatcher != nil {
-		a.FileWatcher.Stop()
+	if a.configFileWatcher != nil {
+		a.configFileWatcher.Stop()
 	}
 
 	a.stopLicenseManager()
@@ -3772,13 +3774,13 @@ func (a *Agent) reloadConfig(autoReload bool) error {
 			{a.config.TLS.HTTPS, newCfg.TLS.HTTPS},
 		} {
 			if f.oldCfg.KeyFile != f.newCfg.KeyFile {
-				a.FileWatcher.Replace(f.oldCfg.KeyFile, f.newCfg.KeyFile)
+				a.configFileWatcher.Replace(f.oldCfg.KeyFile, f.newCfg.KeyFile)
 				if err != nil {
 					return err
 				}
 			}
 			if f.oldCfg.CertFile != f.newCfg.CertFile {
-				a.FileWatcher.Replace(f.oldCfg.CertFile, f.newCfg.CertFile)
+				a.configFileWatcher.Replace(f.oldCfg.CertFile, f.newCfg.CertFile)
 				if err != nil {
 					return err
 				}

agent/agent_test.go

@@ -5545,7 +5545,8 @@ func TestAgent_AutoReloadDoReload_WhenCertThenKeyUpdated(t *testing.T) {
 
 	testrpc.WaitForTestAgent(t, srv.RPC, "dc1", testrpc.WithToken(TestDefaultInitialManagementToken))
 
-	cert1 := srv.tlsConfigurator.Cert()
+	cert1Pub := srv.tlsConfigurator.Cert().Certificate
+	cert1Key := srv.tlsConfigurator.Cert().PrivateKey
 
 	certNew, privateKeyNew, err := tlsutil.GenerateCert(tlsutil.CertOpts{
 		Signer:      signer,
@@ -5575,17 +5576,19 @@ func TestAgent_AutoReloadDoReload_WhenCertThenKeyUpdated(t *testing.T) {
 	// cert should not change as we did not update the associated key
 	time.Sleep(1 * time.Second)
 	retry.Run(t, func(r *retry.R) {
-		require.Equal(r, cert1.Certificate, srv.tlsConfigurator.Cert().Certificate)
-		require.Equal(r, cert1.PrivateKey, srv.tlsConfigurator.Cert().PrivateKey)
+		cert := srv.tlsConfigurator.Cert()
+		require.NotNil(r, cert)
+		require.Equal(r, cert1Pub, cert.Certificate)
+		require.Equal(r, cert1Key, cert.PrivateKey)
 	})
 
 	require.NoError(t, ioutil.WriteFile(keyFile, []byte(privateKeyNew), 0600))
 
 	// cert should change as we did not update the associated key
 	time.Sleep(1 * time.Second)
 	retry.Run(t, func(r *retry.R) {
-		require.NotEqual(r, cert1.Certificate, srv.tlsConfigurator.Cert().Certificate)
-		require.NotEqual(r, cert1.PrivateKey, srv.tlsConfigurator.Cert().PrivateKey)
+		require.NotEqual(r, cert1Pub, srv.tlsConfigurator.Cert().Certificate)
+		require.NotEqual(r, cert1Key, srv.tlsConfigurator.Cert().PrivateKey)
 	})
 }
 
@@ -5647,11 +5650,13 @@ func TestAgent_AutoReloadDoReload_WhenKeyThenCertUpdated(t *testing.T) {
 	`), 0600))
 
 	srv := StartTestAgent(t, TestAgent{Name: "TestAgent-Server", HCL: hclConfig, configFiles: []string{configFile}})
+
 	defer srv.Shutdown()
 
 	testrpc.WaitForTestAgent(t, srv.RPC, "dc1", testrpc.WithToken(TestDefaultInitialManagementToken))
 
-	cert1 := srv.tlsConfigurator.Cert()
+	cert1Pub := srv.tlsConfigurator.Cert().Certificate
+	cert1Key := srv.tlsConfigurator.Cert().PrivateKey
 
 	certNew, privateKeyNew, err := tlsutil.GenerateCert(tlsutil.CertOpts{
 		Signer:      signer,
@@ -5667,8 +5672,10 @@ func TestAgent_AutoReloadDoReload_WhenKeyThenCertUpdated(t *testing.T) {
 	// cert should not change as we did not update the associated key
 	time.Sleep(1 * time.Second)
 	retry.Run(t, func(r *retry.R) {
-		require.Equal(r, cert1.Certificate, srv.tlsConfigurator.Cert().Certificate)
-		require.Equal(r, cert1.PrivateKey, srv.tlsConfigurator.Cert().PrivateKey)
+		cert := srv.tlsConfigurator.Cert()
+		require.NotNil(r, cert)
+		require.Equal(r, cert1Pub, cert.Certificate)
+		require.Equal(r, cert1Key, cert.PrivateKey)
 	})
 
 	require.NoError(t, ioutil.WriteFile(certFileNew, []byte(certNew), 0600))
@@ -5689,10 +5696,13 @@ func TestAgent_AutoReloadDoReload_WhenKeyThenCertUpdated(t *testing.T) {
 	// cert should change as we did not update the associated key
 	time.Sleep(1 * time.Second)
 	retry.Run(t, func(r *retry.R) {
-		require.NotEqual(r, cert1.Certificate, srv.tlsConfigurator.Cert().Certificate)
-		require.NotEqual(r, cert1.PrivateKey, srv.tlsConfigurator.Cert().PrivateKey)
+		cert := srv.tlsConfigurator.Cert()
+		require.NotNil(r, cert)
+		require.NotEqual(r, cert1Key, cert.Certificate)
+		require.NotEqual(r, cert1Key, cert.PrivateKey)
 	})
-	cert2 := srv.tlsConfigurator.Cert()
+	cert2Pub := srv.tlsConfigurator.Cert().Certificate
+	cert2Key := srv.tlsConfigurator.Cert().PrivateKey
 
 	certNew2, privateKeyNew2, err := tlsutil.GenerateCert(tlsutil.CertOpts{
 		Signer:      signer,
@@ -5707,16 +5717,131 @@ func TestAgent_AutoReloadDoReload_WhenKeyThenCertUpdated(t *testing.T) {
 	// cert should not change as we did not update the associated cert
 	time.Sleep(1 * time.Second)
 	retry.Run(t, func(r *retry.R) {
-		require.Equal(r, cert2.Certificate, srv.tlsConfigurator.Cert().Certificate)
-		require.Equal(r, cert2.PrivateKey, srv.tlsConfigurator.Cert().PrivateKey)
+		cert := srv.tlsConfigurator.Cert()
+		require.NotNil(r, cert)
+		require.Equal(r, cert2Pub, cert.Certificate)
+		require.Equal(r, cert2Key, cert.PrivateKey)
 	})
 
 	require.NoError(t, ioutil.WriteFile(certFileNew, []byte(certNew2), 0600))
 
 	// cert should change as we did  update the associated key
 	time.Sleep(1 * time.Second)
 	retry.Run(t, func(r *retry.R) {
-		require.NotEqual(r, cert2.Certificate, srv.tlsConfigurator.Cert().Certificate)
-		require.NotEqual(r, cert2.PrivateKey, srv.tlsConfigurator.Cert().PrivateKey)
+		cert := srv.tlsConfigurator.Cert()
+		require.NotNil(r, cert)
+		require.NotEqual(r, cert2Pub, cert.Certificate)
+		require.NotEqual(r, cert2Key, cert.PrivateKey)
+	})
+}
+
+func Test_coalesceTimerTwoPeriods(t *testing.T) {
+
+	certsDir := testutil.TempDir(t, "auto-config")
+
+	// write some test TLS certificates out to the cfg dir
+	serverName := "server.dc1.consul"
+	signer, _, err := tlsutil.GeneratePrivateKey()
+	require.NoError(t, err)
+
+	ca, _, err := tlsutil.GenerateCA(tlsutil.CAOpts{Signer: signer})
+	require.NoError(t, err)
+
+	cert, privateKey, err := tlsutil.GenerateCert(tlsutil.CertOpts{
+		Signer:      signer,
+		CA:          ca,
+		Name:        "Test Cert Name",
+		Days:        365,
+		DNSNames:    []string{serverName},
+		ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth},
+	})
+	require.NoError(t, err)
+
+	certFile := filepath.Join(certsDir, "cert.pem")
+	caFile := filepath.Join(certsDir, "cacert.pem")
+	keyFile := filepath.Join(certsDir, "key.pem")
+
+	require.NoError(t, ioutil.WriteFile(certFile, []byte(cert), 0600))
+	require.NoError(t, ioutil.WriteFile(caFile, []byte(ca), 0600))
+	require.NoError(t, ioutil.WriteFile(keyFile, []byte(privateKey), 0600))
+
+	// generate a gossip key
+	gossipKey := make([]byte, 32)
+	n, err := rand.Read(gossipKey)
+	require.NoError(t, err)
+	require.Equal(t, 32, n)
+	gossipKeyEncoded := base64.StdEncoding.EncodeToString(gossipKey)
+
+	hclConfig := TestACLConfigWithParams(nil)
+
+	configFile := testutil.TempDir(t, "config") + "/config.hcl"
+	require.NoError(t, ioutil.WriteFile(configFile, []byte(`
+		encrypt = "`+gossipKeyEncoded+`"
+		encrypt_verify_incoming = true
+		encrypt_verify_outgoing = true
+		verify_incoming = true
+		verify_outgoing = true
+		verify_server_hostname = true
+		ca_file = "`+caFile+`"
+		cert_file = "`+certFile+`"
+		key_file = "`+keyFile+`"
+		connect { enabled = true }
+		auto_reload_config = true
+	`), 0600))
+
+	coalesceInterval := 100 * time.Millisecond
+	testAgent := TestAgent{Name: "TestAgent-Server", HCL: hclConfig, configFiles: []string{configFile}, Config: &config.RuntimeConfig{
+		AutoReloadConfigCoalesceInterval: coalesceInterval,
+	}}
+	srv := StartTestAgent(t, testAgent)
+	defer srv.Shutdown()
+
+	testrpc.WaitForTestAgent(t, srv.RPC, "dc1", testrpc.WithToken(TestDefaultInitialManagementToken))
+
+	cert1Pub := srv.tlsConfigurator.Cert().Certificate
+	cert1Key := srv.tlsConfigurator.Cert().PrivateKey
+
+	certNew, privateKeyNew, err := tlsutil.GenerateCert(tlsutil.CertOpts{
+		Signer:      signer,
+		CA:          ca,
+		Name:        "Test Cert Name",
+		Days:        365,
+		DNSNames:    []string{serverName},
+		ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth},
+	})
+	require.NoError(t, err)
+	certFileNew := filepath.Join(certsDir, "cert_new.pem")
+	require.NoError(t, ioutil.WriteFile(certFileNew, []byte(certNew), 0600))
+	require.NoError(t, ioutil.WriteFile(configFile, []byte(`
+		encrypt = "`+gossipKeyEncoded+`"
+		encrypt_verify_incoming = true
+		encrypt_verify_outgoing = true
+		verify_incoming = true
+		verify_outgoing = true
+		verify_server_hostname = true
+		ca_file = "`+caFile+`"
+		cert_file = "`+certFileNew+`"
+		key_file = "`+keyFile+`"
+		connect { enabled = true }
+		auto_reload_config = true
+	`), 0600))
+
+	// cert should not change as we did not update the associated key
+	time.Sleep(coalesceInterval * 2)
+	retry.Run(t, func(r *retry.R) {
+		cert := srv.tlsConfigurator.Cert()
+		require.NotNil(r, cert)
+		require.Equal(r, cert1Pub, cert.Certificate)
+		require.Equal(r, cert1Key, cert.PrivateKey)
+	})
+
+	require.NoError(t, ioutil.WriteFile(keyFile, []byte(privateKeyNew), 0600))
+
+	// cert should change as we did not update the associated key
+	time.Sleep(coalesceInterval * 2)
+	retry.Run(t, func(r *retry.R) {
+		require.NotEqual(r, cert1Pub, srv.tlsConfigurator.Cert().Certificate)
+		require.NotEqual(r, cert1Key, srv.tlsConfigurator.Cert().PrivateKey)
 	})
+
 }