Skip to content

Commit

Permalink
Revert #19676, we decided this was unnecessary. (#20826)
Browse files Browse the repository at this point in the history
  • Loading branch information
ncabatoff committed May 29, 2023
1 parent 439c904 commit 8d66586
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 42 deletions.
3 changes: 3 additions & 0 deletions changelog/20826.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:change
core: Revert #19676 (VAULT_GRPC_MIN_CONNECT_TIMEOUT env var) as we decided it was unnecessary.
```
3 changes: 1 addition & 2 deletions vault/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -327,8 +327,7 @@ func (c *Core) startClusterListener(ctx context.Context) error {
c.clusterListener.Store(cluster.NewListener(networkLayer,
c.clusterCipherSuites,
listenerLogger,
5*c.clusterHeartbeatInterval,
c.grpcMinConnectTimeout))
5*c.clusterHeartbeatInterval))

c.AddLogger(listenerLogger)

Expand Down
17 changes: 2 additions & 15 deletions vault/cluster/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,9 @@ type Listener struct {
logger log.Logger
l sync.RWMutex
tlsConnectionLoggingLevel log.Level
grpcMinConnectTimeout time.Duration
}

func NewListener(networkLayer NetworkLayer, cipherSuites []uint16, logger log.Logger, idleTimeout, grpcMinConnectTimeout time.Duration) *Listener {
func NewListener(networkLayer NetworkLayer, cipherSuites []uint16, logger log.Logger, idleTimeout time.Duration) *Listener {
var maxStreams uint32 = math.MaxUint32
if override := os.Getenv("VAULT_GRPC_MAX_STREAMS"); override != "" {
i, err := strconv.ParseUint(override, 10, 32)
Expand Down Expand Up @@ -112,7 +111,6 @@ func NewListener(networkLayer NetworkLayer, cipherSuites []uint16, logger log.Lo
cipherSuites: cipherSuites,
logger: logger,
tlsConnectionLoggingLevel: log.LevelFromString(os.Getenv("VAULT_CLUSTER_TLS_SESSION_LOG_LEVEL")),
grpcMinConnectTimeout: grpcMinConnectTimeout,
}
}

Expand Down Expand Up @@ -463,21 +461,10 @@ func (cl *Listener) GetDialerFunc(ctx context.Context, alpn string) func(string,
}

tlsConfig.NextProtos = []string{alpn}
args := []interface{}{
"address", addr,
"alpn", alpn,
"host", tlsConfig.ServerName,
"timeout", fmt.Sprintf("%s", timeout),
}
if cl.grpcMinConnectTimeout != 0 {
args = append(args, "timeout_env_override", fmt.Sprintf("%s", cl.grpcMinConnectTimeout))
}
cl.logger.Debug("creating rpc dialer", args...)
cl.logger.Debug("creating rpc dialer", "address", addr, "alpn", alpn, "host", tlsConfig.ServerName)

start := time.Now()
conn, err := cl.networkLayer.Dial(addr, timeout, tlsConfig)
if err != nil {
cl.logger.Debug("dial failure", "address", addr, "alpn", alpn, "host", tlsConfig.ServerName, "duration", fmt.Sprintf("%s", time.Since(start)), "error", err)
return nil, err
}
cl.logTLSSessionStart(conn.RemoteAddr().String(), conn.ConnectionState())
Expand Down
13 changes: 0 additions & 13 deletions vault/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -645,9 +645,6 @@ type Core struct {

pendingRemovalMountsAllowed bool
expirationRevokeRetryBase time.Duration

// if populated, override the default gRPC min connect timeout (currently 20s in grpc 1.51)
grpcMinConnectTimeout time.Duration
}

func (c *Core) HAState() consts.HAState {
Expand Down Expand Up @@ -1189,16 +1186,6 @@ func NewCore(conf *CoreConfig) (*Core, error) {
c.versionHistory = make(map[string]VaultVersion)
}

minConnectTimeoutRaw := os.Getenv("VAULT_GRPC_MIN_CONNECT_TIMEOUT")
if minConnectTimeoutRaw != "" {
dur, err := time.ParseDuration(minConnectTimeoutRaw)
if err != nil {
c.logger.Warn("VAULT_GRPC_MIN_CONNECT_TIMEOUT contains non-duration value, ignoring")
} else if dur != 0 {
c.grpcMinConnectTimeout = dur
}
}

return c, nil
}

Expand Down
14 changes: 2 additions & 12 deletions vault/request_forwarding.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"github.com/hashicorp/vault/vault/replication"
"golang.org/x/net/http2"
"google.golang.org/grpc"
"google.golang.org/grpc/backoff"
"google.golang.org/grpc/keepalive"
)

Expand Down Expand Up @@ -276,8 +275,7 @@ func (c *Core) refreshRequestForwardingConnection(ctx context.Context, clusterAd
// ALPN header right. It's just "insecure" because GRPC isn't managing
// the TLS state.
dctx, cancelFunc := context.WithCancel(ctx)

opts := []grpc.DialOption{
c.rpcClientConn, err = grpc.DialContext(dctx, clusterURL.Host,
grpc.WithDialer(clusterListener.GetDialerFunc(ctx, consts.RequestForwardingALPN)),
grpc.WithInsecure(), // it's not, we handle it in the dialer
grpc.WithKeepaliveParams(keepalive.ClientParameters{
Expand All @@ -286,15 +284,7 @@ func (c *Core) refreshRequestForwardingConnection(ctx context.Context, clusterAd
grpc.WithDefaultCallOptions(
grpc.MaxCallRecvMsgSize(math.MaxInt32),
grpc.MaxCallSendMsgSize(math.MaxInt32),
),
}
if c.grpcMinConnectTimeout != 0 {
opts = append(opts, grpc.WithConnectParams(grpc.ConnectParams{
MinConnectTimeout: c.grpcMinConnectTimeout,
Backoff: backoff.DefaultConfig,
}))
}
c.rpcClientConn, err = grpc.DialContext(dctx, clusterURL.Host, opts...)
))
if err != nil {
cancelFunc()
c.logger.Error("err setting up forwarding rpc client", "error", err)
Expand Down

0 comments on commit 8d66586

Please sign in to comment.