Skip to content

Commit

Permalink
add a rate limiter to config auto-reload (#12490)
Browse files Browse the repository at this point in the history 
* add config watcher to the config package

* add logging to watcher

* add test and refactor to add WatcherEvent.

* add all API calls and fix a bug with recreated files

* add tests for watcher

* remove the unnecessary use of context

* Add debug log and a test for file rename

* use inode to detect if the file is recreated/replaced and only listen to create events.

* tidy ups (#1535)

* tidy ups

* Add tests for inode reconcile

* fix linux vs windows syscall

* fix linux vs windows syscall

* fix windows compile error

* increase timeout

* use ctime ID

* remove remove/creation test as it's a use case that fail in linux

* fix linux/windows to use Ino/CreationTime

* fix the watcher to only overwrite current file id

* fix linter error

* fix remove/create test

* set reconcile loop to 200 Milliseconds

* fix watcher to not trigger event on remove, add more tests

* on a remove event try to add the file back to the watcher and trigger the handler if success

* fix race condition

* fix flaky test

* fix race conditions

* set level to info

* fix when file is removed and get an event for it after

* fix to trigger handler when we get a remove but re-add fail

* fix error message

* add tests for directory watch and fixes

* detect if a file is a symlink and return an error on Add

* rename Watcher to FileWatcher and remove symlink deref

* add fsnotify@v1.5.1

* fix go mod

* do not reset timer on errors, rename OS specific files

* rename New func

* events trigger on write and rename

* add missing test

* fix flaking tests

* fix flaky test

* check reconcile when removed

* delete invalid file

* fix test to create files with different mod time.

* back date file instead of sleeping

* add watching file in agent command.

* fix watcher call to use new API

* add configuration and stop watcher when server stop

* add certs as watched files

* move FileWatcher to the agent start instead of the command code

* stop watcher before replacing it

* save watched files in agent

* add add and remove interfaces to the file watcher

* fix remove to not return an error

* use `Add` and `Remove` to update certs files

* fix tests

* close events channel on the file watcher even when the context is done

* extract `NotAutoReloadableRuntimeConfig` is a separate struct

* fix linter errors

* add Ca configs and outgoing verify to the not auto reloadable config

* add some logs and fix to use background context

* add tests to auto-config reload

* remove stale test

* add tests to changes to config files

* add check to see if old cert files still trigger updates

* rename `NotAutoReloadableRuntimeConfig` to `StaticRuntimeConfig`

* fix to re add both key and cert file. Add test to cover this case.

* review suggestion

Co-authored-by: R.B. Boyer <4903+rboyer@users.noreply.github.com>

* add check to static runtime config changes

* fix test

* add changelog file

* fix review comments

* Apply suggestions from code review

Co-authored-by: R.B. Boyer <4903+rboyer@users.noreply.github.com>

* update flag description

Co-authored-by: FFMMM <FFMMM@users.noreply.github.com>

* fix compilation error

* add static runtime config support

* fix test

* fix review comments

* fix log test

* Update .changelog/12329.txt

Co-authored-by: Dan Upton <daniel@floppy.co>

* transfer tests to runtime_test.go

* fix filewatcher Replace to not deadlock.

* avoid having lingering locks

Co-authored-by: R.B. Boyer <4903+rboyer@users.noreply.github.com>

* split ReloadConfig func

* fix warning message

Co-authored-by: R.B. Boyer <4903+rboyer@users.noreply.github.com>

* convert `FileWatcher` into an interface

* fix compilation errors

* fix tests

* extract func for adding and removing files

* add a coalesceTimer with a very small timer

* extract coaelsce Timer and add a shim for testing

* add tests to coalesceTimer fix to send remaining events

* set `coalesceTimer` to 1 Second

* support symlink, fix a nil deref.

* fix compile error

* fix compile error

* refactor file watcher rate limiting to be a Watcher implementation

* fix linter issue

* fix runtime config

* fix runtime test

* fix flaky tests

* fix compile error

* Apply suggestions from code review

Co-authored-by: R.B. Boyer <4903+rboyer@users.noreply.github.com>

* fix agent New to return an error if File watcher New return an error

* quit timer loop if ctx is canceled

* Apply suggestions from code review

Co-authored-by: Chris S. Kim <ckim@hashicorp.com>

Co-authored-by: Ashwin Venkatesh <ashwin@hashicorp.com>
Co-authored-by: R.B. Boyer <4903+rboyer@users.noreply.github.com>
Co-authored-by: FFMMM <FFMMM@users.noreply.github.com>
Co-authored-by: Daniel Upton <daniel@floppy.co>
Co-authored-by: Chris S. Kim <ckim@hashicorp.com>
  • Loading branch information
@dhiaayachi @thisisnotashwin
@rboyer @FFMMM @boxofrad
6 people authored Apr 4, 2022
1 parent 18f55be commit 83720e5
Show file tree
Hide file tree
Showing 11 changed files with 437 additions and 103 deletions.
48 changes: 25 additions & 23 deletions agent/agent.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
return nil, err
}
a.configFileWatcher = w
}

return &a, nil
}
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
}
Expand Down
155 changes: 140 additions & 15 deletions agent/agent_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
})
}

Expand Down Expand Up @@ -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,
Expand All @@ -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))
Expand All @@ -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,
Expand All @@ -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)
})

}
Loading

0 comments on commit 83720e5

Please sign in to comment.