From bf616909f7ac754407f08408569ce6d172eb91d6 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Mon, 19 Jun 2017 11:10:09 -0400 Subject: [PATCH 01/25] Add API helper for renewing a token as another token --- api/auth_token.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/api/auth_token.go b/api/auth_token.go index aff10f4109cb..d1ffcff0c910 100644 --- a/api/auth_token.go +++ b/api/auth_token.go @@ -135,6 +135,26 @@ func (c *TokenAuth) RenewSelf(increment int) (*Secret, error) { return ParseSecret(resp.Body) } +// RenewSelfAsToken behaves like renew-self, but authenticates using a provided +// token instead of the token attached to the client. +func (c *TokenAuth) RenewSelfAsToken(token string, increment int) (*Secret, error) { + r := c.c.NewRequest("PUT", "/v1/auth/token/renew-self") + r.ClientToken = token + + body := map[string]interface{}{"increment": increment} + if err := r.SetJSONBody(body); err != nil { + return nil, err + } + + resp, err := c.c.RawRequest(r) + if err != nil { + return nil, err + } + defer resp.Body.Close() + + return ParseSecret(resp.Body) +} + // RevokeAccessor revokes a token associated with the given accessor // along with all the child tokens. func (c *TokenAuth) RevokeAccessor(accessor string) error { From 46fa7be9114f672c2500eae3e61e8883deed5100 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Mon, 19 Jun 2017 21:33:50 -0700 Subject: [PATCH 02/25] Add test stubs for starting a vault server and pg database --- api/api_test.go | 99 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/api/api_test.go b/api/api_test.go index d9059eab1585..ca4d219b70b7 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -1,10 +1,18 @@ package api import ( + "database/sql" "fmt" "net" "net/http" + "os" + "os/exec" + "sync/atomic" "testing" + "time" + + _ "github.com/lib/pq" + dockertest "gopkg.in/ory-am/dockertest.v3" "golang.org/x/net/http2" ) @@ -29,3 +37,94 @@ func testHTTPServer( return config, ln } + +// nextPort is the next port to use for the API server. +var nextPort int32 = 28200 + +// restVaultServer runs an instance of the Vault server in development mode. +// This requires that the vault binary is installed and in the $PATH. +func testVaultServer(t *testing.T) (*Client, func()) { + bin, err := exec.LookPath("vault") + if err != nil || bin == "" { + t.Fatal("vault binary not found") + } + + // Get the port number + port := atomic.AddInt32(&nextPort, 1) + + // Construct the address + addr := fmt.Sprintf("127.0.0.1:%d", port) + + // Start the server + cmd := exec.Command( + bin, "server", "-dev", + "-dev-listen-address", addr, + "-dev-root-token-id", "root", + ) + if err := cmd.Start(); err != nil { + t.Fatalf("err: %s", err) + } + + for i := 0; i < 10; i++ { + conn, err := net.DialTimeout("tcp", addr, time.Second) + if err != nil { + time.Sleep(100 * time.Millisecond) + continue + } + conn.Close() + + config := DefaultConfig() + config.Address = fmt.Sprintf("http://%s", addr) + client, err := NewClient(config) + if err != nil { + t.Fatal(err) + } + client.SetToken("root") + + return client, func() { + cmd.Process.Signal(os.Interrupt) + cmd.Process.Wait() + } + } + + t.Fatalf("timeout waiting for vault server") + return nil, nil +} + +func testPostgresDatabase(t *testing.T) (string, func()) { + if os.Getenv("PG_URL") != "" { + return os.Getenv("PG_URL"), func() {} + } + + pool, err := dockertest.NewPool("") + if err != nil { + t.Fatalf("Failed to connect to docker: %s", err) + } + + resource, err := pool.Run("postgres", "latest", []string{"POSTGRES_PASSWORD=secret", "POSTGRES_DB=database"}) + if err != nil { + t.Fatalf("Could not start local PostgreSQL docker container: %s", err) + } + + cleanup := func() { + err := pool.Purge(resource) + if err != nil { + t.Fatalf("Failed to cleanup local container: %s", err) + } + } + + pgURL := fmt.Sprintf("postgres://postgres:secret@localhost:%s/database?sslmode=disable", resource.GetPort("5432/tcp")) + + // exponential backoff-retry + if err := pool.Retry(func() error { + db, err := sql.Open("postgres", pgURL) + if err != nil { + return err + } + return db.Ping() + }); err != nil { + t.Fatalf("Could not connect to PostgreSQL docker container: %s", err) + } + + return pgURL, cleanup +} From dcbd729afa5593c2be3be54156e3af03f5bf4e04 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Mon, 19 Jun 2017 21:34:11 -0700 Subject: [PATCH 03/25] Add secret renewer --- api/renewer.go | 243 ++++++++++++++++++++++++++++++++++++++++++ api/renewer_test.go | 251 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 494 insertions(+) create mode 100644 api/renewer.go create mode 100644 api/renewer_test.go diff --git a/api/renewer.go b/api/renewer.go new file mode 100644 index 000000000000..921c6dd47f0a --- /dev/null +++ b/api/renewer.go @@ -0,0 +1,243 @@ +package api + +import ( + "errors" + "sync" + "time" +) + +// RenewerInput is used as input to the renew function. +type RenewerInput struct { + // Secret is the secret to renew + Secret *Secret + + // Grace is a minimum renewal (in seconds) before returring so the upstream + // client can do a re-read. This can be used to prevent clients from waiting + // too long to read a new credential and incur downtime. + Grace int +} + +// Renewer is a process for renewing a secret. +// +// renewer, err := client.NewRenewer(&RenewerInput{ +// Secret: mySecret, +// }) +// go renewer.Renew() +// defer renewer.Stop() +// +// for { +// select { +// case err := <-DoneCh(): +// if err != nil { +// log.Fatal(err) +// } +// +// // Renewal is now over +// case <-TickCh(): +// log.Println("Successfully renewed") +// default: +// } +// } +// +// +// The `DoneCh` will return if renewal fails or if the remaining lease duration +// after a renewal is less than or equal to the grace (in number of seconds). In +// both cases, the caller should attempt a re-read of the secret. Clients should +// check the return value of the channel to see if renewal was successful. +type Renewer struct { + sync.Mutex + + client *Client + secret *Secret + grace int + doneCh chan error + tickCh chan struct{} + + stopped bool + stopCh chan struct{} +} + +var ( + ErrRenewerMissingInput = errors.New("missing input to renewer") + ErrRenewerMissingSecret = errors.New("missing secret to renew") + ErrRenewerNotRenewable = errors.New("secret is not renewable") + ErrRenewerNoSecretData = errors.New("returned empty secret data") + + // DefaultRenewerGrace is the default grace period + DefaultRenewerGrace = 15 +) + +// NewRenewer creates a new renewer from the given input. +func (c *Client) NewRenewer(i *RenewerInput) (*Renewer, error) { + if i == nil { + return nil, ErrRenewerMissingInput + } + + secret := i.Secret + if secret == nil { + return nil, ErrRenewerMissingSecret + } + + grace := i.Grace + if grace == 0 { + grace = DefaultRenewerGrace + } + + return &Renewer{ + client: c, + secret: secret, + grace: grace, + doneCh: make(chan error, 1), + tickCh: make(chan struct{}, 5), + + stopped: false, + stopCh: make(chan struct{}, 1), + }, nil +} + +// DoneCh returns the channel where the renewer will publish when renewal stops. +// If there is an error, this will be an error. +func (r *Renewer) DoneCh() <-chan error { + return r.doneCh +} + +// TickCh is a channel that receives a message when a successful renewal takes +// place. +func (r *Renewer) TickCh() <-chan struct{} { + return r.tickCh +} + +// Stop stops the renewer. +func (r *Renewer) Stop() { + r.Lock() + if !r.stopped { + close(r.stopCh) + r.stopped = true + } + r.Unlock() +} + +// Renew starts a background process for renewing this secret. When the secret +// is has auth data, this attempts to renew the auth (token). When the secret +// has a lease, this attempts to renew the lease. +// +// This function will not return if nothing is reading from doneCh (it blocks) +// on a write to the channel. +func (r *Renewer) Renew() { + if r.secret.Auth != nil { + r.doneCh <- r.renewAuth() + } else { + r.doneCh <- r.renewLease() + } +} + +// renewAuth is a helper for renewing authentication. +func (r *Renewer) renewAuth() error { + if !r.secret.Auth.Renewable || r.secret.Auth.ClientToken == "" { + return ErrRenewerNotRenewable + } + + client, token := r.client, r.secret.Auth.ClientToken + + for { + // Check if we are stopped. + select { + case <-r.stopCh: + return nil + default: + } + + // Renew the auth. + renewal, err := client.Auth().Token().RenewSelfAsToken(token, 0) + if err != nil { + return err + } + + // Push a message that a renewal took place. + select { + case r.tickCh <- struct{}{}: + default: + } + + // Somehow, sometimes, this happens. + if renewal == nil || renewal.Auth == nil { + return ErrRenewerNoSecretData + } + + // Do nothing if we are not renewable + if !renewal.Auth.Renewable { + return ErrRenewerNotRenewable + } + + // Grab the lease duration - note that we grab the auth lease duration, not + // the secret lease duration. + leaseDuration := renewal.Auth.LeaseDuration + + // If we are within grace, return now. + if leaseDuration <= r.grace { + return nil + } + + select { + case <-r.stopCh: + return nil + case <-time.After(time.Duration(leaseDuration/2.0) * time.Second): + continue + } + } +} + +// renewLease is a helper for renewing a lease. +func (r *Renewer) renewLease() error { + if !r.secret.Renewable || r.secret.LeaseID == "" { + return ErrRenewerNotRenewable + } + + client, leaseID := r.client, r.secret.LeaseID + + for { + // Check if we are stopped. + select { + case <-r.stopCh: + return nil + default: + } + + // Renew the lease. + renewal, err := client.Sys().Renew(leaseID, 0) + if err != nil { + return err + } + + // Push a message that a renewal took place. + select { + case r.tickCh <- struct{}{}: + default: + } + + // Somehow, sometimes, this happens. + if renewal == nil { + return ErrRenewerNoSecretData + } + + // Do nothing if we are not renewable + if !renewal.Renewable { + return ErrRenewerNotRenewable + } + + // Grab the lease duration + leaseDuration := renewal.LeaseDuration + + // If we are within grace, return now. + if leaseDuration <= r.grace { + return nil + } + + select { + case <-r.stopCh: + return nil + case <-time.After(time.Duration(leaseDuration/2.0) * time.Second): + continue + } + } +} diff --git a/api/renewer_test.go b/api/renewer_test.go new file mode 100644 index 000000000000..43812e75f274 --- /dev/null +++ b/api/renewer_test.go @@ -0,0 +1,251 @@ +package api + +import ( + "fmt" + "reflect" + "testing" + "time" +) + +func TestRenewer_NewRenewer(t *testing.T) { + t.Parallel() + + client, err := NewClient(DefaultConfig()) + if err != nil { + t.Fatal(err) + } + + cases := []struct { + name string + i *RenewerInput + e *Renewer + err bool + }{ + { + "nil", + nil, + nil, + true, + }, + { + "missing_secret", + &RenewerInput{ + Secret: nil, + }, + nil, + true, + }, + { + "default_grace", + &RenewerInput{ + Secret: &Secret{}, + }, + &Renewer{ + secret: &Secret{}, + grace: DefaultRenewerGrace, + }, + false, + }, + { + "custom_grace", + &RenewerInput{ + Secret: &Secret{}, + Grace: 30, + }, + &Renewer{ + secret: &Secret{}, + grace: 30, + }, + false, + }, + } + + for i, tc := range cases { + t.Run(fmt.Sprintf("%d_%s", i, tc.name), func(t *testing.T) { + v, err := client.NewRenewer(tc.i) + if (err != nil) != tc.err { + t.Fatal(err) + } + + if v == nil { + return + } + + // Zero-out channels because reflect + v.client = nil + v.doneCh = nil + v.tickCh = nil + v.stopCh = nil + + if !reflect.DeepEqual(tc.e, v) { + t.Errorf("not equal\nexp: %#v\nact: %#v", tc.e, v) + } + }) + } +} + +func TestRenewer_Renew(t *testing.T) { + client, vaultDone := testVaultServer(t) + defer vaultDone() + + pgURL, pgDone := testPostgresDatabase(t) + defer pgDone() + + // Generic + if _, err := client.Logical().Write("secret/value", map[string]interface{}{ + "foo": "bar", + }); err != nil { + t.Fatal(err) + } + + // Transit + if err := client.Sys().Mount("transit", &MountInput{ + Type: "transit", + }); err != nil { + t.Fatal(err) + } + + // PostgreSQL + if err := client.Sys().Mount("database", &MountInput{ + Type: "database", + }); err != nil { + t.Fatal(err) + } + if _, err := client.Logical().Write("database/config/postgresql", map[string]interface{}{ + "plugin_name": "postgresql-database-plugin", + "connection_url": pgURL, + "allowed_roles": "readonly", + }); err != nil { + t.Fatal(err) + } + if _, err := client.Logical().Write("database/roles/readonly", map[string]interface{}{ + "db_name": "postgresql", + "creation_statements": `` + + `CREATE ROLE "{{name}}" WITH LOGIN PASSWORD '{{password}}' VALID UNTIL '{{expiration}}';` + + `GRANT SELECT ON ALL TABLES IN SCHEMA public TO "{{name}}";`, + "default_ttl": "2s", + "max_ttl": "5s", + }); err != nil { + t.Fatal(err) + } + + t.Run("generic", func(t *testing.T) { + secret, err := client.Logical().Read("secret/value") + if err != nil { + t.Fatal(err) + } + + v, err := client.NewRenewer(&RenewerInput{ + Secret: secret, + }) + if err != nil { + t.Fatal(err) + } + go v.Renew() + defer v.Stop() + + select { + case err := <-v.DoneCh(): + if err != ErrRenewerNotRenewable { + t.Fatal(err) + } + } + }) + + t.Run("transit", func(t *testing.T) { + secret, err := client.Logical().Write("transit/encrypt/my-app", map[string]interface{}{ + "plaintext": "Zm9vCg==", + }) + if err != nil { + t.Fatal(err) + } + + v, err := client.NewRenewer(&RenewerInput{ + Secret: secret, + }) + if err != nil { + t.Fatal(err) + } + go v.Renew() + defer v.Stop() + + select { + case err := <-v.DoneCh(): + if err != ErrRenewerNotRenewable { + t.Fatal(err) + } + } + }) + + t.Run("dynamic", func(t *testing.T) { + secret, err := client.Logical().Read("database/creds/readonly") + if err != nil { + t.Fatal(err) + } + + v, err := client.NewRenewer(&RenewerInput{ + Secret: secret, + }) + if err != nil { + t.Fatal(err) + } + go v.Renew() + defer v.Stop() + + select { + case err := <-v.DoneCh(): + t.Errorf("should have renewed once before returning: %s", err) + case <-v.TickCh(): + // Received a renewal + case <-time.After(5 * time.Second): + t.Errorf("no data in 5s") + } + + select { + case err := <-v.DoneCh(): + if err != nil { + t.Fatal(err) + } + case <-time.After(5 * time.Second): + t.Errorf("no data in 5s") + } + }) + + t.Run("auth", func(t *testing.T) { + secret, err := client.Auth().Token().Create(&TokenCreateRequest{ + Policies: []string{"default"}, + TTL: "2s", + ExplicitMaxTTL: "5s", + }) + if err != nil { + t.Fatal(err) + } + + v, err := client.NewRenewer(&RenewerInput{ + Secret: secret, + }) + if err != nil { + t.Fatal(err) + } + go v.Renew() + defer v.Stop() + + select { + case err := <-v.DoneCh(): + t.Errorf("should have renewed once before returning: %s", err) + case <-v.TickCh(): + // Received a renewal + case <-time.After(5 * time.Second): + t.Errorf("no data in 5s") + } + + select { + case err := <-v.DoneCh(): + if err != nil { + t.Fatal(err) + } + case <-time.After(5 * time.Second): + t.Errorf("no data in 5s") + } + }) +} From 42354aed99e096db7e404ea2f5eb8cda47680a29 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Tue, 20 Jun 2017 15:52:40 -0700 Subject: [PATCH 04/25] Use RenewTokenAsSelf instead --- api/auth_token.go | 4 ++-- api/renewer.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/api/auth_token.go b/api/auth_token.go index d1ffcff0c910..4f74f61fe5f2 100644 --- a/api/auth_token.go +++ b/api/auth_token.go @@ -135,9 +135,9 @@ func (c *TokenAuth) RenewSelf(increment int) (*Secret, error) { return ParseSecret(resp.Body) } -// RenewSelfAsToken behaves like renew-self, but authenticates using a provided +// RenewTokenAsSelf behaves like renew-self, but authenticates using a provided // token instead of the token attached to the client. -func (c *TokenAuth) RenewSelfAsToken(token string, increment int) (*Secret, error) { +func (c *TokenAuth) RenewTokenAsSelf(token string, increment int) (*Secret, error) { r := c.c.NewRequest("PUT", "/v1/auth/token/renew-self") r.ClientToken = token diff --git a/api/renewer.go b/api/renewer.go index 921c6dd47f0a..a1c20eea458b 100644 --- a/api/renewer.go +++ b/api/renewer.go @@ -148,7 +148,7 @@ func (r *Renewer) renewAuth() error { } // Renew the auth. - renewal, err := client.Auth().Token().RenewSelfAsToken(token, 0) + renewal, err := client.Auth().Token().RenewTokenAsSelf(token, 0) if err != nil { return err } From 7e08052e14fc8c342fa06e2fc3be491a956bcd9f Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Tue, 20 Jun 2017 15:55:50 -0700 Subject: [PATCH 05/25] Use a time.Duration instead of an int for grace --- api/renewer.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/api/renewer.go b/api/renewer.go index a1c20eea458b..ed8c3a6b25b0 100644 --- a/api/renewer.go +++ b/api/renewer.go @@ -14,7 +14,7 @@ type RenewerInput struct { // Grace is a minimum renewal (in seconds) before returring so the upstream // client can do a re-read. This can be used to prevent clients from waiting // too long to read a new credential and incur downtime. - Grace int + Grace time.Duration } // Renewer is a process for renewing a secret. @@ -49,7 +49,7 @@ type Renewer struct { client *Client secret *Secret - grace int + grace time.Duration doneCh chan error tickCh chan struct{} @@ -64,7 +64,7 @@ var ( ErrRenewerNoSecretData = errors.New("returned empty secret data") // DefaultRenewerGrace is the default grace period - DefaultRenewerGrace = 15 + DefaultRenewerGrace = 15 * time.Second ) // NewRenewer creates a new renewer from the given input. @@ -171,7 +171,7 @@ func (r *Renewer) renewAuth() error { // Grab the lease duration - note that we grab the auth lease duration, not // the secret lease duration. - leaseDuration := renewal.Auth.LeaseDuration + leaseDuration := time.Duration(renewal.Auth.LeaseDuration) * time.Second // If we are within grace, return now. if leaseDuration <= r.grace { @@ -226,7 +226,7 @@ func (r *Renewer) renewLease() error { } // Grab the lease duration - leaseDuration := renewal.LeaseDuration + leaseDuration := time.Duration(renewal.LeaseDuration) * time.Second // If we are within grace, return now. if leaseDuration <= r.grace { From 320d76422a27569a23d643644529be78ded20156 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Tue, 20 Jun 2017 15:57:05 -0700 Subject: [PATCH 06/25] Use unbuffered channels --- api/renewer.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/renewer.go b/api/renewer.go index ed8c3a6b25b0..e0b0ae7fa142 100644 --- a/api/renewer.go +++ b/api/renewer.go @@ -87,11 +87,11 @@ func (c *Client) NewRenewer(i *RenewerInput) (*Renewer, error) { client: c, secret: secret, grace: grace, - doneCh: make(chan error, 1), + doneCh: make(chan error), tickCh: make(chan struct{}, 5), stopped: false, - stopCh: make(chan struct{}, 1), + stopCh: make(chan struct{}), }, nil } From f9465a8a5be31e4410dcfcf493efb9f8c0b3aac0 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Tue, 20 Jun 2017 15:57:34 -0700 Subject: [PATCH 07/25] Reorg --- api/renewer.go | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/api/renewer.go b/api/renewer.go index e0b0ae7fa142..419cd2a89219 100644 --- a/api/renewer.go +++ b/api/renewer.go @@ -6,16 +6,15 @@ import ( "time" ) -// RenewerInput is used as input to the renew function. -type RenewerInput struct { - // Secret is the secret to renew - Secret *Secret +var ( + ErrRenewerMissingInput = errors.New("missing input to renewer") + ErrRenewerMissingSecret = errors.New("missing secret to renew") + ErrRenewerNotRenewable = errors.New("secret is not renewable") + ErrRenewerNoSecretData = errors.New("returned empty secret data") - // Grace is a minimum renewal (in seconds) before returring so the upstream - // client can do a re-read. This can be used to prevent clients from waiting - // too long to read a new credential and incur downtime. - Grace time.Duration -} + // DefaultRenewerGrace is the default grace period + DefaultRenewerGrace = 15 * time.Second +) // Renewer is a process for renewing a secret. // @@ -57,15 +56,16 @@ type Renewer struct { stopCh chan struct{} } -var ( - ErrRenewerMissingInput = errors.New("missing input to renewer") - ErrRenewerMissingSecret = errors.New("missing secret to renew") - ErrRenewerNotRenewable = errors.New("secret is not renewable") - ErrRenewerNoSecretData = errors.New("returned empty secret data") +// RenewerInput is used as input to the renew function. +type RenewerInput struct { + // Secret is the secret to renew + Secret *Secret - // DefaultRenewerGrace is the default grace period - DefaultRenewerGrace = 15 * time.Second -) + // Grace is a minimum renewal (in seconds) before returring so the upstream + // client can do a re-read. This can be used to prevent clients from waiting + // too long to read a new credential and incur downtime. + Grace time.Duration +} // NewRenewer creates a new renewer from the given input. func (c *Client) NewRenewer(i *RenewerInput) (*Renewer, error) { From de0250a66fa9e848e0187426f38ff898ed8035ad Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Tue, 20 Jun 2017 16:06:08 -0700 Subject: [PATCH 08/25] Send a more useful struct for renewal --- api/renewer.go | 48 ++++++++++++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/api/renewer.go b/api/renewer.go index 419cd2a89219..9596b6e5c9bc 100644 --- a/api/renewer.go +++ b/api/renewer.go @@ -32,8 +32,8 @@ var ( // } // // // Renewal is now over -// case <-TickCh(): -// log.Println("Successfully renewed") +// case renewal := <-RenewCh(): +// log.Printf("Successfully renewed: %#v", renewal) // default: // } // } @@ -46,11 +46,11 @@ var ( type Renewer struct { sync.Mutex - client *Client - secret *Secret - grace time.Duration - doneCh chan error - tickCh chan struct{} + client *Client + secret *Secret + grace time.Duration + doneCh chan error + renewCh chan *RenewOutput stopped bool stopCh chan struct{} @@ -67,6 +67,18 @@ type RenewerInput struct { Grace time.Duration } +// RenewOutput is the metadata returned to the client (if it's listening) to +// renew messages. +type RenewOutput struct { + // RenewedAt is the timestamp when the renewal took place (UTC). + RenewedAt time.Time + + // Secret is the underlying renewal data. It's the same struct as all data + // that is returned from Vault, but since this is renewal data, it will not + // usually include the secret itself. + Secret *Secret +} + // NewRenewer creates a new renewer from the given input. func (c *Client) NewRenewer(i *RenewerInput) (*Renewer, error) { if i == nil { @@ -84,11 +96,11 @@ func (c *Client) NewRenewer(i *RenewerInput) (*Renewer, error) { } return &Renewer{ - client: c, - secret: secret, - grace: grace, - doneCh: make(chan error), - tickCh: make(chan struct{}, 5), + client: c, + secret: secret, + grace: grace, + doneCh: make(chan error), + renewCh: make(chan *RenewOutput, 5), stopped: false, stopCh: make(chan struct{}), @@ -101,10 +113,10 @@ func (r *Renewer) DoneCh() <-chan error { return r.doneCh } -// TickCh is a channel that receives a message when a successful renewal takes -// place. -func (r *Renewer) TickCh() <-chan struct{} { - return r.tickCh +// RenewCh is a channel that receives a message when a successful renewal takes +// place and includes metadata about the renewal. +func (r *Renewer) RenewCh() <-chan *RenewOutput { + return r.renewCh } // Stop stops the renewer. @@ -155,7 +167,7 @@ func (r *Renewer) renewAuth() error { // Push a message that a renewal took place. select { - case r.tickCh <- struct{}{}: + case r.renewCh <- &RenewOutput{time.Now().UTC(), renewal}: default: } @@ -211,7 +223,7 @@ func (r *Renewer) renewLease() error { // Push a message that a renewal took place. select { - case r.tickCh <- struct{}{}: + case r.renewCh <- &RenewOutput{time.Now().UTC(), renewal}: default: } From 91a255bd2f19150a2da604e012576ca70d6e0e66 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Mon, 26 Jun 2017 16:21:14 -0500 Subject: [PATCH 09/25] Use a separate package for API integration tests This removes the cyclic dependency --- api/api_integration_test.go | 117 ++++++++++++++++++++++++++++++++++++ api/api_test.go | 99 ------------------------------ 2 files changed, 117 insertions(+), 99 deletions(-) create mode 100644 api/api_integration_test.go diff --git a/api/api_integration_test.go b/api/api_integration_test.go new file mode 100644 index 000000000000..a2a463099a20 --- /dev/null +++ b/api/api_integration_test.go @@ -0,0 +1,117 @@ +package api_test + +import ( + "database/sql" + "fmt" + "net/http" + "testing" + + "github.com/hashicorp/vault/api" + "github.com/hashicorp/vault/builtin/logical/pki" + "github.com/hashicorp/vault/builtin/logical/transit" + "github.com/hashicorp/vault/logical" + "github.com/hashicorp/vault/vault" + + cleanhttp "github.com/hashicorp/go-cleanhttp" + vaulthttp "github.com/hashicorp/vault/http" + logxi "github.com/mgutz/logxi/v1" + dockertest "gopkg.in/ory-am/dockertest.v3" +) + +var testVaultServerDefaultBackends = map[string]logical.Factory{ + "transit": transit.Factory, + "pki": pki.Factory, +} + +func testVaultServer(t testing.TB) (*api.Client, func()) { + return testVaultServerBackends(t, testVaultServerDefaultBackends) +} + +func testVaultServerBackends(t testing.TB, backends map[string]logical.Factory) (*api.Client, func()) { + handlers := []http.Handler{ + http.NewServeMux(), + http.NewServeMux(), + http.NewServeMux(), + } + + coreConfig := &vault.CoreConfig{ + DisableMlock: true, + DisableCache: true, + Logger: logxi.NullLog, + LogicalBackends: backends, + } + + // Chicken-and-egg: Handler needs a core. So we create handlers first, then + // add routes chained to a Handler-created handler. + cores := vault.TestCluster(t, handlers, coreConfig, true) + for i, core := range cores { + handlers[i].(*http.ServeMux).Handle("/", vaulthttp.Handler(core.Core)) + } + + // make it easy to get access to the active + core := cores[0].Core + vault.TestWaitActive(t, core) + + rootToken := cores[0].Root + address := fmt.Sprintf("https://127.0.0.1:%d", cores[1].Listeners[0].Address.Port) + + config := api.DefaultConfig() + config.Address = address + config.HttpClient = cleanhttp.DefaultClient() + config.HttpClient.Transport.(*http.Transport).TLSClientConfig = cores[0].TLSConfig + client, err := api.NewClient(config) + if err != nil { + t.Fatalf("error creating vault cluster: %s", err) + } + client.SetToken(rootToken) + + // Sanity check + secret, err := client.Auth().Token().LookupSelf() + if err != nil { + t.Fatal(err) + } + if secret == nil || secret.Data["id"].(string) != rootToken { + t.Fatal("token mismatch: %q vs %q", secret, rootToken) + } + + return client, func() { + for _, core := range cores { + defer core.CloseListeners() + } + } +} + +// testPostgresDB creates a testing postgres database in a Docker container, +// returning the connection URL and the associated closer function. +func testPostgresDB(t testing.TB) (string, func()) { + pool, err := dockertest.NewPool("") + if err != nil { + t.Fatalf("postgresdb: failed to connect to docker: %s", err) + } + + resource, err := pool.Run("postgres", "latest", []string{ + "POSTGRES_PASSWORD=secret", + "POSTGRES_DB=database", + }) + if err != nil { + t.Fatalf("postgresdb: could not start container: %s", err) + } + + addr := fmt.Sprintf("postgres://postgres:secret@localhost:%s/database?sslmode=disable", resource.GetPort("5432/tcp")) + + if err := pool.Retry(func() error { + db, err := sql.Open("postgres", addr) + if err != nil { + return err + } + return db.Ping() + }); err != nil { + t.Fatalf("postgresdb: could not connect: %s", err) + } + + return addr, func() { + if err := pool.Purge(resource); err != nil { + t.Fatalf("postgresdb: failed to cleanup container: %s", err) + } + } +} diff --git a/api/api_test.go b/api/api_test.go index ca4d219b70b7..d9059eab1585 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -1,18 +1,10 @@ package api import ( - "database/sql" "fmt" "net" "net/http" - "os" - "os/exec" - "sync/atomic" "testing" - "time" - - _ "github.com/lib/pq" - dockertest "gopkg.in/ory-am/dockertest.v3" "golang.org/x/net/http2" ) @@ -37,94 +29,3 @@ func testHTTPServer( return config, ln } - -// nextPort is the next port to use for the API server. -var nextPort int32 = 28200 - -// restVaultServer runs an instance of the Vault server in development mode. -// This requires that the vault binary is installed and in the $PATH. -func testVaultServer(t *testing.T) (*Client, func()) { - bin, err := exec.LookPath("vault") - if err != nil || bin == "" { - t.Fatal("vault binary not found") - } - - // Get the port number - port := atomic.AddInt32(&nextPort, 1) - - // Construct the address - addr := fmt.Sprintf("127.0.0.1:%d", port) - - // Start the server - cmd := exec.Command( - bin, "server", "-dev", - "-dev-listen-address", addr, - "-dev-root-token-id", "root", - ) - if err := cmd.Start(); err != nil { - t.Fatalf("err: %s", err) - } - - for i := 0; i < 10; i++ { - conn, err := net.DialTimeout("tcp", addr, time.Second) - if err != nil { - time.Sleep(100 * time.Millisecond) - continue - } - conn.Close() - - config := DefaultConfig() - config.Address = fmt.Sprintf("http://%s", addr) - client, err := NewClient(config) - if err != nil { - t.Fatal(err) - } - client.SetToken("root") - - return client, func() { - cmd.Process.Signal(os.Interrupt) - cmd.Process.Wait() - } - } - - t.Fatalf("timeout waiting for vault server") - return nil, nil -} - -func testPostgresDatabase(t *testing.T) (string, func()) { - if os.Getenv("PG_URL") != "" { - return os.Getenv("PG_URL"), func() {} - } - - pool, err := dockertest.NewPool("") - if err != nil { - t.Fatalf("Failed to connect to docker: %s", err) - } - - resource, err := pool.Run("postgres", "latest", []string{"POSTGRES_PASSWORD=secret", "POSTGRES_DB=database"}) - if err != nil { - t.Fatalf("Could not start local PostgreSQL docker container: %s", err) - } - - cleanup := func() { - err := pool.Purge(resource) - if err != nil { - t.Fatalf("Failed to cleanup local container: %s", err) - } - } - - pgURL := fmt.Sprintf("postgres://postgres:secret@localhost:%s/database?sslmode=disable", resource.GetPort("5432/tcp")) - - // exponential backoff-retry - if err := pool.Retry(func() error { - db, err := sql.Open("postgres", pgURL) - if err != nil { - return err - } - return db.Ping() - }); err != nil { - t.Fatalf("Could not connect to PostgreSQL docker container: %s", err) - } - - return pgURL, cleanup -} From 47260ed02404b94c3fa2a1dd3b1b72434de8d6cb Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Mon, 26 Jun 2017 16:21:37 -0500 Subject: [PATCH 10/25] Move renewer integration tests into separate package --- api/renewer_integration_test.go | 228 ++++++++++++++++++++++++++++++++ api/renewer_test.go | 177 +------------------------ 2 files changed, 233 insertions(+), 172 deletions(-) create mode 100644 api/renewer_integration_test.go diff --git a/api/renewer_integration_test.go b/api/renewer_integration_test.go new file mode 100644 index 000000000000..2b965ca03d44 --- /dev/null +++ b/api/renewer_integration_test.go @@ -0,0 +1,228 @@ +package api_test + +import ( + "testing" + "time" + + "github.com/hashicorp/vault/api" + "github.com/hashicorp/vault/builtin/logical/database" + "github.com/hashicorp/vault/builtin/logical/pki" + "github.com/hashicorp/vault/builtin/logical/transit" + "github.com/hashicorp/vault/logical" +) + +func TestRenewer_Renew(t *testing.T) { + t.Parallel() + + client, vaultDone := testVaultServerBackends(t, map[string]logical.Factory{ + "database": database.Factory, + "pki": pki.Factory, + "transit": transit.Factory, + }) + defer vaultDone() + + pgURL, pgDone := testPostgresDB(t) + defer pgDone() + + t.Run("group", func(t *testing.T) { + t.Run("generic", func(t *testing.T) { + t.Parallel() + + if _, err := client.Logical().Write("secret/value", map[string]interface{}{ + "foo": "bar", + }); err != nil { + t.Fatal(err) + } + + secret, err := client.Logical().Read("secret/value") + if err != nil { + t.Fatal(err) + } + + v, err := client.NewRenewer(&api.RenewerInput{ + Secret: secret, + }) + if err != nil { + t.Fatal(err) + } + go v.Renew() + defer v.Stop() + + select { + case err := <-v.DoneCh(): + if err != api.ErrRenewerNotRenewable { + t.Fatal(err) + } + case renew := <-v.RenewCh(): + t.Error("received renew, but should have been nil: %#v", renew) + case <-time.After(500 * time.Millisecond): + t.Error("should have been non-renewable") + } + }) + + t.Run("transit", func(t *testing.T) { + t.Parallel() + + if err := client.Sys().Mount("transit", &api.MountInput{ + Type: "transit", + }); err != nil { + t.Fatal(err) + } + + secret, err := client.Logical().Write("transit/encrypt/my-app", map[string]interface{}{ + "plaintext": "Zm9vCg==", + }) + if err != nil { + t.Fatal(err) + } + + v, err := client.NewRenewer(&api.RenewerInput{ + Secret: secret, + }) + if err != nil { + t.Fatal(err) + } + go v.Renew() + defer v.Stop() + + select { + case err := <-v.DoneCh(): + if err != api.ErrRenewerNotRenewable { + t.Fatal(err) + } + case renew := <-v.RenewCh(): + t.Error("received renew, but should have been nil: %#v", renew) + case <-time.After(500 * time.Millisecond): + t.Error("should have been non-renewable") + } + }) + + t.Run("database", func(t *testing.T) { + t.Parallel() + + if err := client.Sys().Mount("database", &api.MountInput{ + Type: "database", + }); err != nil { + t.Fatal(err) + } + if _, err := client.Logical().Write("database/config/postgresql", map[string]interface{}{ + "plugin_name": "postgresql-database-plugin", + "connection_url": pgURL, + "allowed_roles": "readonly", + }); err != nil { + t.Fatal(err) + } + if _, err := client.Logical().Write("database/roles/readonly", map[string]interface{}{ + "db_name": "postgresql", + "creation_statements": `` + + `CREATE ROLE "{{name}}" WITH LOGIN PASSWORD '{{password}}' VALID UNTIL '{{expiration}}';` + + `GRANT SELECT ON ALL TABLES IN SCHEMA public TO "{{name}}";`, + "default_ttl": "1s", + "max_ttl": "3s", + }); err != nil { + t.Fatal(err) + } + + secret, err := client.Logical().Read("database/creds/readonly") + if err != nil { + t.Fatal(err) + } + + v, err := client.NewRenewer(&api.RenewerInput{ + Secret: secret, + }) + if err != nil { + t.Fatal(err) + } + go v.Renew() + defer v.Stop() + + select { + case err := <-v.DoneCh(): + t.Errorf("should have renewed once before returning: %s", err) + case renew := <-v.RenewCh(): + if renew == nil { + t.Fatal("renew is nil") + } + if !renew.Secret.Renewable { + t.Errorf("expected lease to be renewable: %#v", renew) + } + if renew.Secret.LeaseDuration > 2 { + t.Errorf("expected lease to < 2s: %#v", renew) + } + case <-time.After(3 * time.Second): + t.Errorf("no renewal") + } + + select { + case err := <-v.DoneCh(): + if err != nil { + t.Fatal(err) + } + case renew := <-v.RenewCh(): + t.Fatalf("should not have renewed (lease should be up): %#v", renew) + case <-time.After(3 * time.Second): + t.Errorf("no data") + } + }) + + t.Run("auth", func(t *testing.T) { + t.Parallel() + + secret, err := client.Auth().Token().Create(&api.TokenCreateRequest{ + Policies: []string{"default"}, + TTL: "1s", + ExplicitMaxTTL: "3s", + }) + if err != nil { + t.Fatal(err) + } + + v, err := client.NewRenewer(&api.RenewerInput{ + Secret: secret, + }) + if err != nil { + t.Fatal(err) + } + go v.Renew() + defer v.Stop() + + select { + case err := <-v.DoneCh(): + t.Errorf("should have renewed once before returning: %s", err) + case renew := <-v.RenewCh(): + if renew == nil { + t.Fatal("renew is nil") + } + if renew.Secret.Auth == nil { + t.Fatal("renew auth is nil") + } + if !renew.Secret.Auth.Renewable { + t.Errorf("expected lease to be renewable: %#v", renew) + } + if renew.Secret.Auth.LeaseDuration > 2 { + t.Errorf("expected lease to < 2s: %#v", renew) + } + if renew.Secret.Auth.ClientToken == "" { + t.Error("expected a client token") + } + if renew.Secret.Auth.Accessor == "" { + t.Error("expected an accessor") + } + case <-time.After(3 * time.Second): + t.Errorf("no renewal") + } + + select { + case err := <-v.DoneCh(): + if err != nil { + t.Fatal(err) + } + case renew := <-v.RenewCh(): + t.Fatalf("should not have renewed (lease should be up): %#v", renew) + case <-time.After(3 * time.Second): + t.Errorf("no data") + } + }) + }) +} diff --git a/api/renewer_test.go b/api/renewer_test.go index 43812e75f274..c48bcdba41df 100644 --- a/api/renewer_test.go +++ b/api/renewer_test.go @@ -1,7 +1,6 @@ package api import ( - "fmt" "reflect" "testing" "time" @@ -50,18 +49,18 @@ func TestRenewer_NewRenewer(t *testing.T) { "custom_grace", &RenewerInput{ Secret: &Secret{}, - Grace: 30, + Grace: 30 * time.Second, }, &Renewer{ secret: &Secret{}, - grace: 30, + grace: 30 * time.Second, }, false, }, } - for i, tc := range cases { - t.Run(fmt.Sprintf("%d_%s", i, tc.name), func(t *testing.T) { + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { v, err := client.NewRenewer(tc.i) if (err != nil) != tc.err { t.Fatal(err) @@ -74,7 +73,7 @@ func TestRenewer_NewRenewer(t *testing.T) { // Zero-out channels because reflect v.client = nil v.doneCh = nil - v.tickCh = nil + v.renewCh = nil v.stopCh = nil if !reflect.DeepEqual(tc.e, v) { @@ -83,169 +82,3 @@ func TestRenewer_NewRenewer(t *testing.T) { }) } } - -func TestRenewer_Renew(t *testing.T) { - client, vaultDone := testVaultServer(t) - defer vaultDone() - - pgURL, pgDone := testPostgresDatabase(t) - defer pgDone() - - // Generic - if _, err := client.Logical().Write("secret/value", map[string]interface{}{ - "foo": "bar", - }); err != nil { - t.Fatal(err) - } - - // Transit - if err := client.Sys().Mount("transit", &MountInput{ - Type: "transit", - }); err != nil { - t.Fatal(err) - } - - // PostgreSQL - if err := client.Sys().Mount("database", &MountInput{ - Type: "database", - }); err != nil { - t.Fatal(err) - } - if _, err := client.Logical().Write("database/config/postgresql", map[string]interface{}{ - "plugin_name": "postgresql-database-plugin", - "connection_url": pgURL, - "allowed_roles": "readonly", - }); err != nil { - t.Fatal(err) - } - if _, err := client.Logical().Write("database/roles/readonly", map[string]interface{}{ - "db_name": "postgresql", - "creation_statements": `` + - `CREATE ROLE "{{name}}" WITH LOGIN PASSWORD '{{password}}' VALID UNTIL '{{expiration}}';` + - `GRANT SELECT ON ALL TABLES IN SCHEMA public TO "{{name}}";`, - "default_ttl": "2s", - "max_ttl": "5s", - }); err != nil { - t.Fatal(err) - } - - t.Run("generic", func(t *testing.T) { - secret, err := client.Logical().Read("secret/value") - if err != nil { - t.Fatal(err) - } - - v, err := client.NewRenewer(&RenewerInput{ - Secret: secret, - }) - if err != nil { - t.Fatal(err) - } - go v.Renew() - defer v.Stop() - - select { - case err := <-v.DoneCh(): - if err != ErrRenewerNotRenewable { - t.Fatal(err) - } - } - }) - - t.Run("transit", func(t *testing.T) { - secret, err := client.Logical().Write("transit/encrypt/my-app", map[string]interface{}{ - "plaintext": "Zm9vCg==", - }) - if err != nil { - t.Fatal(err) - } - - v, err := client.NewRenewer(&RenewerInput{ - Secret: secret, - }) - if err != nil { - t.Fatal(err) - } - go v.Renew() - defer v.Stop() - - select { - case err := <-v.DoneCh(): - if err != ErrRenewerNotRenewable { - t.Fatal(err) - } - } - }) - - t.Run("dynamic", func(t *testing.T) { - secret, err := client.Logical().Read("database/creds/readonly") - if err != nil { - t.Fatal(err) - } - - v, err := client.NewRenewer(&RenewerInput{ - Secret: secret, - }) - if err != nil { - t.Fatal(err) - } - go v.Renew() - defer v.Stop() - - select { - case err := <-v.DoneCh(): - t.Errorf("should have renewed once before returning: %s", err) - case <-v.TickCh(): - // Received a renewal - case <-time.After(5 * time.Second): - t.Errorf("no data in 5s") - } - - select { - case err := <-v.DoneCh(): - if err != nil { - t.Fatal(err) - } - case <-time.After(5 * time.Second): - t.Errorf("no data in 5s") - } - }) - - t.Run("auth", func(t *testing.T) { - secret, err := client.Auth().Token().Create(&TokenCreateRequest{ - Policies: []string{"default"}, - TTL: "2s", - ExplicitMaxTTL: "5s", - }) - if err != nil { - t.Fatal(err) - } - - v, err := client.NewRenewer(&RenewerInput{ - Secret: secret, - }) - if err != nil { - t.Fatal(err) - } - go v.Renew() - defer v.Stop() - - select { - case err := <-v.DoneCh(): - t.Errorf("should have renewed once before returning: %s", err) - case <-v.TickCh(): - // Received a renewal - case <-time.After(5 * time.Second): - t.Errorf("no data in 5s") - } - - select { - case err := <-v.DoneCh(): - if err != nil { - t.Fatal(err) - } - case <-time.After(5 * time.Second): - t.Errorf("no data in 5s") - } - }) -} From a8fe1646944ef5ba44b33aeea952d0975fb6d3a5 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Mon, 26 Jun 2017 16:21:58 -0500 Subject: [PATCH 11/25] Seed the random generator --- api/renewer.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/api/renewer.go b/api/renewer.go index 9596b6e5c9bc..2b220336dfea 100644 --- a/api/renewer.go +++ b/api/renewer.go @@ -2,10 +2,16 @@ package api import ( "errors" + "math/rand" "sync" "time" ) +func init() { + // Seed the random generator + rand.Seed(time.Now().Unix()) +} + var ( ErrRenewerMissingInput = errors.New("missing input to renewer") ErrRenewerMissingSecret = errors.New("missing secret to renew") From 3d74752524f6cbbfe413d711b716eef8f8896ab9 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Mon, 26 Jun 2017 16:22:18 -0500 Subject: [PATCH 12/25] Use a more heurstic function for calculating sleep backoff --- api/renewer.go | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/api/renewer.go b/api/renewer.go index 2b220336dfea..e9f7aa204349 100644 --- a/api/renewer.go +++ b/api/renewer.go @@ -187,19 +187,20 @@ func (r *Renewer) renewAuth() error { return ErrRenewerNotRenewable } - // Grab the lease duration - note that we grab the auth lease duration, not - // the secret lease duration. + // Grab the lease duration and sleep duration - note that we grab the auth + // lease duration, not the secret lease duration. leaseDuration := time.Duration(renewal.Auth.LeaseDuration) * time.Second + sleepDuration := sleepDuration(leaseDuration) // If we are within grace, return now. - if leaseDuration <= r.grace { + if leaseDuration <= r.grace || sleepDuration <= r.grace { return nil } select { case <-r.stopCh: return nil - case <-time.After(time.Duration(leaseDuration/2.0) * time.Second): + case <-time.After(sleepDuration): continue } } @@ -243,19 +244,37 @@ func (r *Renewer) renewLease() error { return ErrRenewerNotRenewable } - // Grab the lease duration + // Grab the lease duration and sleep duration leaseDuration := time.Duration(renewal.LeaseDuration) * time.Second + sleepDuration := sleepDuration(leaseDuration) // If we are within grace, return now. - if leaseDuration <= r.grace { + if leaseDuration <= r.grace || sleepDuration <= r.grace { return nil } select { case <-r.stopCh: return nil - case <-time.After(time.Duration(leaseDuration/2.0) * time.Second): + case <-time.After(sleepDuration): continue } } } + +// sleepDuration calculates the time to sleep given the base lease duration. The +// base is the resulting lease duration. It will be reduced to 1/3 and +// multiplied by a random float between 0.0 and 1.0. This extra randomness +// prevents multiple clients from all trying to renew simultaneously. +func sleepDuration(base time.Duration) time.Duration { + sleep := float64(base) + + // Renew at 1/3 the remaining lease. This will give us an opportunity to retry + // at least one more time should the first renewal fail. + sleep = sleep / 3.0 + + // Use a randomness so many clients do not hit Vault simultaneously. + sleep = sleep * rand.Float64() + + return time.Duration(sleep) * time.Second +} From 437b8e71ab0767f5a16ffc7dbb71eef8216d6355 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Mon, 26 Jun 2017 16:26:59 -0500 Subject: [PATCH 13/25] Use Fatalf --- api/api_integration_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/api_integration_test.go b/api/api_integration_test.go index a2a463099a20..09e8901ba8c7 100644 --- a/api/api_integration_test.go +++ b/api/api_integration_test.go @@ -71,7 +71,7 @@ func testVaultServerBackends(t testing.TB, backends map[string]logical.Factory) t.Fatal(err) } if secret == nil || secret.Data["id"].(string) != rootToken { - t.Fatal("token mismatch: %q vs %q", secret, rootToken) + t.Fatalf("token mismatch: %q vs %q", secret, rootToken) } return client, func() { From c0b2d41d8f3918b9306c63edba6bbb7312a7c90e Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Mon, 26 Jun 2017 19:55:54 -0500 Subject: [PATCH 14/25] Allow a custom randomizer --- api/renewer.go | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/api/renewer.go b/api/renewer.go index e9f7aa204349..c05c2136aef0 100644 --- a/api/renewer.go +++ b/api/renewer.go @@ -55,6 +55,7 @@ type Renewer struct { client *Client secret *Secret grace time.Duration + random *rand.Rand doneCh chan error renewCh chan *RenewOutput @@ -71,6 +72,11 @@ type RenewerInput struct { // client can do a re-read. This can be used to prevent clients from waiting // too long to read a new credential and incur downtime. Grace time.Duration + + // Rand is the randomizer to use for underlying randomization. If not + // provided, one will be generated and seeded automatically. If provided, it + // is assumed to have already been seeded. + Rand *rand.Rand } // RenewOutput is the metadata returned to the client (if it's listening) to @@ -101,10 +107,16 @@ func (c *Client) NewRenewer(i *RenewerInput) (*Renewer, error) { grace = DefaultRenewerGrace } + random := i.Rand + if random == nil { + random = rand.New(rand.NewSource(int64(time.Now().Nanosecond()))) + } + return &Renewer{ client: c, secret: secret, grace: grace, + random: random, doneCh: make(chan error), renewCh: make(chan *RenewOutput, 5), @@ -190,7 +202,7 @@ func (r *Renewer) renewAuth() error { // Grab the lease duration and sleep duration - note that we grab the auth // lease duration, not the secret lease duration. leaseDuration := time.Duration(renewal.Auth.LeaseDuration) * time.Second - sleepDuration := sleepDuration(leaseDuration) + sleepDuration := r.sleepDuration(leaseDuration) // If we are within grace, return now. if leaseDuration <= r.grace || sleepDuration <= r.grace { @@ -246,7 +258,7 @@ func (r *Renewer) renewLease() error { // Grab the lease duration and sleep duration leaseDuration := time.Duration(renewal.LeaseDuration) * time.Second - sleepDuration := sleepDuration(leaseDuration) + sleepDuration := r.sleepDuration(leaseDuration) // If we are within grace, return now. if leaseDuration <= r.grace || sleepDuration <= r.grace { @@ -266,7 +278,7 @@ func (r *Renewer) renewLease() error { // base is the resulting lease duration. It will be reduced to 1/3 and // multiplied by a random float between 0.0 and 1.0. This extra randomness // prevents multiple clients from all trying to renew simultaneously. -func sleepDuration(base time.Duration) time.Duration { +func (r *Renewer) sleepDuration(base time.Duration) time.Duration { sleep := float64(base) // Renew at 1/3 the remaining lease. This will give us an opportunity to retry @@ -274,7 +286,7 @@ func sleepDuration(base time.Duration) time.Duration { sleep = sleep / 3.0 // Use a randomness so many clients do not hit Vault simultaneously. - sleep = sleep * rand.Float64() + sleep = sleep * r.random.Float64() return time.Duration(sleep) * time.Second } From 8cdc0372b76b218e9a7aea51e6e8d12823cf7db4 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Mon, 26 Jun 2017 19:56:00 -0500 Subject: [PATCH 15/25] Fix vet errors --- api/api_integration_test.go | 2 +- api/renewer_integration_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/api/api_integration_test.go b/api/api_integration_test.go index 09e8901ba8c7..24db00186067 100644 --- a/api/api_integration_test.go +++ b/api/api_integration_test.go @@ -71,7 +71,7 @@ func testVaultServerBackends(t testing.TB, backends map[string]logical.Factory) t.Fatal(err) } if secret == nil || secret.Data["id"].(string) != rootToken { - t.Fatalf("token mismatch: %q vs %q", secret, rootToken) + t.Fatalf("token mismatch: %#v vs %q", secret, rootToken) } return client, func() { diff --git a/api/renewer_integration_test.go b/api/renewer_integration_test.go index 2b965ca03d44..82ffe508d176 100644 --- a/api/renewer_integration_test.go +++ b/api/renewer_integration_test.go @@ -54,7 +54,7 @@ func TestRenewer_Renew(t *testing.T) { t.Fatal(err) } case renew := <-v.RenewCh(): - t.Error("received renew, but should have been nil: %#v", renew) + t.Errorf("received renew, but should have been nil: %#v", renew) case <-time.After(500 * time.Millisecond): t.Error("should have been non-renewable") } @@ -91,7 +91,7 @@ func TestRenewer_Renew(t *testing.T) { t.Fatal(err) } case renew := <-v.RenewCh(): - t.Error("received renew, but should have been nil: %#v", renew) + t.Errorf("received renew, but should have been nil: %#v", renew) case <-time.After(500 * time.Millisecond): t.Error("should have been non-renewable") } From b2b9cdfdeb7c6c124a731985238cbd34c499f0f4 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Mon, 26 Jun 2017 19:57:51 -0500 Subject: [PATCH 16/25] Remove init() seed --- api/renewer.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/api/renewer.go b/api/renewer.go index c05c2136aef0..9ff74a5113c7 100644 --- a/api/renewer.go +++ b/api/renewer.go @@ -7,11 +7,6 @@ import ( "time" ) -func init() { - // Seed the random generator - rand.Seed(time.Now().Unix()) -} - var ( ErrRenewerMissingInput = errors.New("missing input to renewer") ErrRenewerMissingSecret = errors.New("missing secret to renew") From ce43621894240b11761086ed129b7020f59a8950 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Thu, 29 Jun 2017 08:13:24 +0800 Subject: [PATCH 17/25] Make lock private --- api/renewer.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/api/renewer.go b/api/renewer.go index 9ff74a5113c7..d11a73fe4107 100644 --- a/api/renewer.go +++ b/api/renewer.go @@ -45,7 +45,7 @@ var ( // both cases, the caller should attempt a re-read of the secret. Clients should // check the return value of the channel to see if renewal was successful. type Renewer struct { - sync.Mutex + l sync.Mutex client *Client secret *Secret @@ -134,12 +134,12 @@ func (r *Renewer) RenewCh() <-chan *RenewOutput { // Stop stops the renewer. func (r *Renewer) Stop() { - r.Lock() + r.l.Lock() if !r.stopped { close(r.stopCh) r.stopped = true } - r.Unlock() + r.l.Unlock() } // Renew starts a background process for renewing this secret. When the secret From 54224b06dcfe89f540a3254c050b440b8a3a1ae8 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Thu, 29 Jun 2017 08:16:28 +0800 Subject: [PATCH 18/25] Do not block writing to doneCh if stopped --- api/renewer.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/api/renewer.go b/api/renewer.go index d11a73fe4107..2bd6d6672cf5 100644 --- a/api/renewer.go +++ b/api/renewer.go @@ -149,10 +149,16 @@ func (r *Renewer) Stop() { // This function will not return if nothing is reading from doneCh (it blocks) // on a write to the channel. func (r *Renewer) Renew() { + var result error if r.secret.Auth != nil { - r.doneCh <- r.renewAuth() + result = r.renewAuth() } else { - r.doneCh <- r.renewLease() + result = r.renewLease() + } + + select { + case r.doneCh <- result: + case <-r.stopCh: } } From dfb6166cd8de97a98243743f37ef2b28579c9fe1 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Thu, 29 Jun 2017 08:19:00 +0800 Subject: [PATCH 19/25] Add configurable buffer size --- api/renewer.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/api/renewer.go b/api/renewer.go index 2bd6d6672cf5..73be99de1ef0 100644 --- a/api/renewer.go +++ b/api/renewer.go @@ -15,6 +15,10 @@ var ( // DefaultRenewerGrace is the default grace period DefaultRenewerGrace = 15 * time.Second + + // DefaultRenewerRenewBuffer is the default size of the buffer for renew + // messages on the channel. + DefaultRenewerRenewBuffer = 5 ) // Renewer is a process for renewing a secret. @@ -72,6 +76,10 @@ type RenewerInput struct { // provided, one will be generated and seeded automatically. If provided, it // is assumed to have already been seeded. Rand *rand.Rand + + // RenewBuffer is the size of the buffered channel where renew messages are + // dispatched. + RenewBuffer int } // RenewOutput is the metadata returned to the client (if it's listening) to @@ -107,13 +115,18 @@ func (c *Client) NewRenewer(i *RenewerInput) (*Renewer, error) { random = rand.New(rand.NewSource(int64(time.Now().Nanosecond()))) } + renewBuffer := i.RenewBuffer + if renewBuffer == 0 { + renewBuffer = DefaultRenewerRenewBuffer + } + return &Renewer{ client: c, secret: secret, grace: grace, random: random, doneCh: make(chan error), - renewCh: make(chan *RenewOutput, 5), + renewCh: make(chan *RenewOutput, renewBuffer), stopped: false, stopCh: make(chan struct{}), From da9d57f5f1e87422c7b28aa0244b7bfb607f7a26 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Thu, 29 Jun 2017 08:29:21 +0800 Subject: [PATCH 20/25] Buffer doneCh --- api/renewer.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/api/renewer.go b/api/renewer.go index 73be99de1ef0..3137507030fa 100644 --- a/api/renewer.go +++ b/api/renewer.go @@ -125,7 +125,7 @@ func (c *Client) NewRenewer(i *RenewerInput) (*Renewer, error) { secret: secret, grace: grace, random: random, - doneCh: make(chan error), + doneCh: make(chan error, 1), renewCh: make(chan *RenewOutput, renewBuffer), stopped: false, @@ -158,9 +158,6 @@ func (r *Renewer) Stop() { // Renew starts a background process for renewing this secret. When the secret // is has auth data, this attempts to renew the auth (token). When the secret // has a lease, this attempts to renew the lease. -// -// This function will not return if nothing is reading from doneCh (it blocks) -// on a write to the channel. func (r *Renewer) Renew() { var result error if r.secret.Auth != nil { From 8f97e9b04d7aa2a1154fac0bc741a0c3d90e13db Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Thu, 29 Jun 2017 08:38:03 +0800 Subject: [PATCH 21/25] Fix failing test --- api/renewer_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/api/renewer_test.go b/api/renewer_test.go index c48bcdba41df..262484e0fa01 100644 --- a/api/renewer_test.go +++ b/api/renewer_test.go @@ -72,6 +72,7 @@ func TestRenewer_NewRenewer(t *testing.T) { // Zero-out channels because reflect v.client = nil + v.random = nil v.doneCh = nil v.renewCh = nil v.stopCh = nil From 21a17b69c3bddcc9f78f425cdb91e9986d9cc2c3 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Sun, 2 Jul 2017 23:24:58 -0400 Subject: [PATCH 22/25] Use the core client --- api/api_integration_test.go | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/api/api_integration_test.go b/api/api_integration_test.go index 24db00186067..1567214708a9 100644 --- a/api/api_integration_test.go +++ b/api/api_integration_test.go @@ -12,7 +12,6 @@ import ( "github.com/hashicorp/vault/logical" "github.com/hashicorp/vault/vault" - cleanhttp "github.com/hashicorp/go-cleanhttp" vaulthttp "github.com/hashicorp/vault/http" logxi "github.com/mgutz/logxi/v1" dockertest "gopkg.in/ory-am/dockertest.v3" @@ -52,17 +51,10 @@ func testVaultServerBackends(t testing.TB, backends map[string]logical.Factory) core := cores[0].Core vault.TestWaitActive(t, core) + // Grab the root token rootToken := cores[0].Root - address := fmt.Sprintf("https://127.0.0.1:%d", cores[1].Listeners[0].Address.Port) - config := api.DefaultConfig() - config.Address = address - config.HttpClient = cleanhttp.DefaultClient() - config.HttpClient.Transport.(*http.Transport).TLSClientConfig = cores[0].TLSConfig - client, err := api.NewClient(config) - if err != nil { - t.Fatalf("error creating vault cluster: %s", err) - } + client := cores[0].Client client.SetToken(rootToken) // Sanity check From c29e85125db3b5b2c84df616a0cc78c3a9eb280c Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Fri, 7 Jul 2017 17:15:43 -0400 Subject: [PATCH 23/25] Fix doc --- api/renewer.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/api/renewer.go b/api/renewer.go index 3137507030fa..1777647df595 100644 --- a/api/renewer.go +++ b/api/renewer.go @@ -31,15 +31,14 @@ var ( // // for { // select { -// case err := <-DoneCh(): +// case err := <-renewer.DoneCh(): // if err != nil { // log.Fatal(err) // } // // // Renewal is now over -// case renewal := <-RenewCh(): +// case renewal := <-renewer.RenewCh(): // log.Printf("Successfully renewed: %#v", renewal) -// default: // } // } // From 6034c9adc83aeb548a7f926c4cfd0e513ae2558d Mon Sep 17 00:00:00 2001 From: Chris Hoffman Date: Mon, 10 Jul 2017 20:47:03 -0700 Subject: [PATCH 24/25] updating for TestCluster changes --- api/api_integration_test.go | 29 ++++++++--------------------- 1 file changed, 8 insertions(+), 21 deletions(-) diff --git a/api/api_integration_test.go b/api/api_integration_test.go index 1567214708a9..90a90f68c178 100644 --- a/api/api_integration_test.go +++ b/api/api_integration_test.go @@ -3,7 +3,6 @@ package api_test import ( "database/sql" "fmt" - "net/http" "testing" "github.com/hashicorp/vault/api" @@ -27,12 +26,6 @@ func testVaultServer(t testing.TB) (*api.Client, func()) { } func testVaultServerBackends(t testing.TB, backends map[string]logical.Factory) (*api.Client, func()) { - handlers := []http.Handler{ - http.NewServeMux(), - http.NewServeMux(), - http.NewServeMux(), - } - coreConfig := &vault.CoreConfig{ DisableMlock: true, DisableCache: true, @@ -40,21 +33,20 @@ func testVaultServerBackends(t testing.TB, backends map[string]logical.Factory) LogicalBackends: backends, } - // Chicken-and-egg: Handler needs a core. So we create handlers first, then - // add routes chained to a Handler-created handler. - cores := vault.TestCluster(t, handlers, coreConfig, true) - for i, core := range cores { - handlers[i].(*http.ServeMux).Handle("/", vaulthttp.Handler(core.Core)) + cluster := vault.NewTestCluster(t, coreConfig, true) + cluster.StartListeners() + for _, core := range cluster.Cores { + core.Handler.Handle("/", vaulthttp.Handler(core.Core)) } // make it easy to get access to the active - core := cores[0].Core + core := cluster.Cores[0].Core vault.TestWaitActive(t, core) // Grab the root token - rootToken := cores[0].Root + rootToken := cluster.Cores[0].Root - client := cores[0].Client + client := cluster.Cores[0].Client client.SetToken(rootToken) // Sanity check @@ -65,12 +57,7 @@ func testVaultServerBackends(t testing.TB, backends map[string]logical.Factory) if secret == nil || secret.Data["id"].(string) != rootToken { t.Fatalf("token mismatch: %#v vs %q", secret, rootToken) } - - return client, func() { - for _, core := range cores { - defer core.CloseListeners() - } - } + return client, func() { defer cluster.CloseListeners() } } // testPostgresDB creates a testing postgres database in a Docker container, From 2fbb19285b692c32833dc88e0a71f3df14419da9 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Mon, 10 Jul 2017 22:26:42 -0700 Subject: [PATCH 25/25] Fix typo --- api/renewer.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/renewer.go b/api/renewer.go index 1777647df595..3f9f23bc3e61 100644 --- a/api/renewer.go +++ b/api/renewer.go @@ -66,8 +66,8 @@ type RenewerInput struct { // Secret is the secret to renew Secret *Secret - // Grace is a minimum renewal (in seconds) before returring so the upstream - // client can do a re-read. This can be used to prevent clients from waiting + // Grace is a minimum renewal before returning so the upstream client + // can do a re-read. This can be used to prevent clients from waiting // too long to read a new credential and incur downtime. Grace time.Duration