vault: improve usage of time.Timer (#388)

cmd/kes/gateway.go

@@ -12,7 +12,7 @@ import (
 	"encoding/hex"
 	"errors"
 	"fmt"
-	"io/ioutil"
+	"io"
 	"net/http"
 	"os"
 	"os/signal"
@@ -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 {
@@ -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 {

internal/http/retry.go

@@ -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--
@@ -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(),

internal/https/server.go

@@ -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 {

internal/keystore/cache.go

@@ -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
 	}
 

internal/keystore/vault/client.go

@@ -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
@@ -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.
@@ -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:
 		}
 	}
 }
@@ -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
 	}
 }

internal/keystore/vault/vault.go

@@ -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 {
@@ -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