Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vault: improve usage of time.Timer #388

Merged
merged 1 commit into from
Aug 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions cmd/kes/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
"encoding/hex"
"errors"
"fmt"
"io/ioutil"
"io"
"net/http"
"os"
"os/signal"
Expand Down Expand Up @@ -142,6 +142,7 @@ func startGateway(cliConfig gatewayConfig) {
for {
select {
case <-ctx.Done():
return
case <-ticker.C:
tlsConfig, err := newTLSConfig(config, cliConfig.TLSAuth)
if err != nil {
Expand Down Expand Up @@ -526,12 +527,12 @@ func newGatewayConfig(ctx context.Context, config *edge.ServerConfig, tlsConfig
if config.Log.Error {
rConfig.ErrorLog = log.New(os.Stderr, "Error: ", log.Ldate|log.Ltime|log.Lmsgprefix)
} else {
rConfig.ErrorLog = log.New(ioutil.Discard, "Error: ", log.Ldate|log.Ltime|log.Lmsgprefix)
rConfig.ErrorLog = log.New(io.Discard, "Error: ", log.Ldate|log.Ltime|log.Lmsgprefix)
}
if config.Log.Audit {
rConfig.AuditLog = log.New(os.Stdout, "", 0)
} else {
rConfig.AuditLog = log.New(ioutil.Discard, "", 0)
rConfig.AuditLog = log.New(io.Discard, "", 0)
}

if len(config.TLS.Proxies) != 0 {
Expand Down
6 changes: 2 additions & 4 deletions internal/http/retry.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,9 +186,6 @@ func (r *Retry) Do(req *http.Request) (*http.Response, error) {
}
}

timer := time.NewTimer(0)
defer timer.Stop()

resp, err := r.Client.Do(req)
for N > 0 && (isTemporary(err) || (resp != nil && resp.StatusCode >= http.StatusInternalServerError)) {
N--
Expand All @@ -202,9 +199,10 @@ func (r *Retry) Do(req *http.Request) (*http.Response, error) {
delay = Delay + time.Duration(rand.Int63n(Jitter.Milliseconds()))*time.Millisecond
}

timer.Reset(delay)
timer := time.NewTimer(delay)
select {
case <-req.Context().Done():
timer.Stop()
return nil, &url.Error{
Op: req.Method,
URL: req.URL.String(),
Expand Down
2 changes: 1 addition & 1 deletion internal/https/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func (s *Server) Start(ctx context.Context) error {
GetConfigForClient: func(*tls.ClientHelloInfo) (*tls.Config, error) {
s.lock.RLock()
defer s.lock.RUnlock()
return s.tlsConfig, nil
return s.tlsConfig.Clone(), nil
},
})
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion internal/keystore/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ func (c *Cache) Get(ctx context.Context, name string) (key.Key, error) {

// gc executes f periodically until the ctx.Done() channel returns.
func (c *Cache) gc(ctx context.Context, interval time.Duration, f func()) {
if interval == 0 {
if interval <= 0 {
return
}

Expand Down
96 changes: 45 additions & 51 deletions internal/keystore/vault/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
type client struct {
*vaultapi.Client

sealed uint32 // Atomic bool: sealed == 0 is false, sealed == 1 is true
sealed atomic.Bool
}

// Sealed returns true if the most recently fetched vault
Expand All @@ -30,9 +30,9 @@ type client struct {
// reflect the current status of the vault server - because it
// may have changed in the meantime.
//
// If the vault health status hasn't been queried every then
// If the vault health status hasn't been queried ever then
// Sealed returns false.
func (c *client) Sealed() bool { return atomic.LoadUint32(&c.sealed) == 1 }
func (c *client) Sealed() bool { return c.sealed.Load() }

// CheckStatus keeps fetching the vault health status every delay
// unit of time until <-ctx.Done() returns.
Expand All @@ -48,26 +48,19 @@ func (c *client) CheckStatus(ctx context.Context, delay time.Duration) {
delay = 10 * time.Second
}

timer := time.NewTimer(0)
defer timer.Stop()
ticker := time.NewTicker(delay)
defer ticker.Stop()

for {
status, err := c.Sys().Health()
if err == nil {
if status.Sealed {
atomic.StoreUint32(&c.sealed, 1)
} else {
atomic.StoreUint32(&c.sealed, 0)
}
c.sealed.Store(status.Sealed)
}

// Add the delay to wait before next health check.
timer.Reset(delay)

select {
case <-ctx.Done():
return
case <-timer.C:
case <-ticker.C:
}
}
}
Expand Down Expand Up @@ -171,75 +164,76 @@ func (c *client) RenewToken(ctx context.Context, authenticate authFunc, ttl, ret
retry = 5 * time.Second
}

timer := time.NewTimer(0)
defer timer.Stop()

for {
// If Vault is sealed we have to wait
// until it is unsealed again.
//
// Users should start client.CheckStatus() in
// another go routine to unblock this for-loop
// once vault becomes unsealed again.
for c.Sealed() {
timer.Reset(time.Second)

if c.Sealed() {
timer := time.NewTimer(1 * time.Second)
select {
case <-ctx.Done():
if !timer.Stop() {
<-timer.C
}
return
case <-timer.C:
}
continue
}

// If the TTL is 0 we cannot renew the token.
// Therefore, we try to re-authenticate and
// get a new token. We repeat that until we
// successfully authenticate and got a token.
if ttl == 0 {
var (
token string
err error
)
token, ttl, err = authenticate()
if err != nil {
ttl = 0 // On error, set the TTL again to 0 to re-auth. again.
timer.Reset(retry)
token, newTTL, err := authenticate()
if err != nil || newTTL == 0 {
timer := time.NewTimer(retry)
select {
case <-ctx.Done():
if !timer.Stop() {
<-timer.C
}
return
case <-timer.C:
}
continue
} else {
ttl = newTTL
c.SetToken(token) // SetToken is safe to call from different go routines
}
c.SetToken(token) // SetToken is safe to call from different go routines
continue
}

// Now the client has a token with a non-zero TTL
// such tht we can renew it. We repeat that until
// the renewable process fails once. In this case
// we try to re-authenticate again.
for {
timer.Reset(ttl / 2)

select {
case <-ctx.Done():
return
case <-timer.C:
}
secret, err := c.Auth().Token().RenewSelf(int(ttl.Seconds()))
if err != nil || secret == nil {
break
}
if ok, err := secret.TokenIsRenewable(); !ok || err != nil {
break
}
ttl, err := secret.TokenTTL()
if err != nil || ttl == 0 {
break
timer := time.NewTimer(ttl / 2)
select {
case <-ctx.Done():
if !timer.Stop() {
<-timer.C
}
return
case <-timer.C:
}

secret, err := c.Auth().Token().RenewSelfWithContext(ctx, int(ttl.Seconds()))
if err != nil || secret == nil {
ttl = 0
continue
}
if ok, err := secret.TokenIsRenewable(); !ok || err != nil {
ttl = 0
continue
}
ttl, err = secret.TokenTTL()
if err != nil || ttl == 0 {
ttl = 0
continue
}
// If we exit the for-loop above set the TTL to 0 to trigger
// a re-authentication.
ttl = 0
}
}
14 changes: 1 addition & 13 deletions internal/keystore/vault/vault.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,6 @@ func (s *Store) Status(ctx context.Context) (kv.State, error) {
}
client.ClearNamespace()

// First, we try to fetch the Vault health information.
// Only if this fails we try to dial Vault directly over
// a TCP connection. See: https://github.com/minio/kes-go/issues/230

start := time.Now()
health, err := client.Sys().HealthWithContext(ctx)
if err == nil {
Expand All @@ -168,15 +164,7 @@ func (s *Store) Status(ctx context.Context) (kv.State, error) {
if errors.Is(err, context.Canceled) && errors.Is(err, context.DeadlineExceeded) {
return kv.State{}, &kv.Unreachable{Err: err}
}

start = time.Now()
req := s.client.Client.NewRequest(http.MethodGet, "")
if _, err = s.client.Client.RawRequestWithContext(ctx, req); err != nil {
return kv.State{}, &kv.Unreachable{Err: err}
}
return kv.State{
Latency: time.Since(start),
}, nil
return kv.State{}, err
}

// Create creates the given key-value pair at Vault if and only
Expand Down
Loading