scottk/vault
1
0
Fork 0
mirror of https://github.com/optim-enterprises-bv/vault.git synced 2025-10-29 01:32:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

Add maximum request duration (timeouts) for all requests except actual monitor and events requests (#28230)

Browse Source 
* fix paths for sys/monitor and sys/events

* add changelog

* add changelog

* Update http/handler.go

Co-authored-by: Violet Hynes <violet.hynes@hashicorp.com>

* added some modifications to check for timeout

---------

Co-authored-by: Violet Hynes <violet.hynes@hashicorp.com>
This commit is contained in:
akshya96
2024-08-30 11:00:12 -07:00
committed by GitHub
parent 8e1db67f6f
commit 06fac16a1e
4 changed files with 18 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
changelog/28230.txt Normal file
View File

@@ -0,0 +1,3 @@
```release-note:bug
core: Fixed an issue where maximum request duration timeout was not being added to all requests containing strings sys/monitor and sys/events. With this change, timeout is now added to all requests except monitor and events endpoint.
```

9
command/agentproxyshared/cache/static_secret_cache_updater_test.go vendored
View File

@@ -6,6 +6,7 @@ package cache
import (
"context"
"fmt"
"os"
"sync"
syncatomic "sync/atomic"
"testing"
@@ -281,6 +282,8 @@ func TestOpenWebSocketConnection_AutoAuthSelfHeal(t *testing.T) {
// works as expected with the default KVV1 mount, and then the connection can be used to receive an event.
// This acts as more of an event system sanity check than a test of the updater
// logic. It's still important coverage, though.
// It also adds a client timeout of 1 second and checks that the connection does not timeout as this is a
// streaming request.
func TestOpenWebSocketConnectionReceivesEventsDefaultMount(t *testing.T) {
if !constants.IsEnterprise {
t.Skip("test can only run on enterprise due to requiring the event notification system")
@@ -290,6 +293,11 @@ func TestOpenWebSocketConnectionReceivesEventsDefaultMount(t *testing.T) {
cluster := vault.NewTestCluster(t, nil, &vault.TestClusterOptions{
HandlerFunc: vaulthttp.Handler,
})
oldClientTimeout := os.Getenv("VAULT_CLIENT_TIMEOUT")
os.Setenv("VAULT_CLIENT_TIMEOUT", "1")
defer os.Setenv("VAULT_CLIENT_TIMEOUT", oldClientTimeout)
client := cluster.Cores[0].Client
updater := testNewStaticSecretCacheUpdater(t, client)
@@ -318,6 +326,7 @@ func TestOpenWebSocketConnectionReceivesEventsDefaultMount(t *testing.T) {
// This method blocks until it gets a secret, so this test
// will only pass if we're receiving events correctly.
// It will fail here if the connection times out.
_, _, err = conn.Read(context.Background())
require.NoError(t, err)
}

4
http/handler.go
View File

@@ -386,7 +386,9 @@ func wrapGenericHandler(core *vault.Core, h http.Handler, props *vault.HandlerPr
ctx := r.Context()
var cancelFunc context.CancelFunc
// Add our timeout, but not for the monitor or events endpoints, as they are streaming
if strings.HasSuffix(r.URL.Path, "sys/monitor") || strings.Contains(r.URL.Path, "sys/events") {
// Request URL path for sys/monitor looks like /v1/sys/monitor
// Request URL paths for event subscriptions look like /v1/sys/events/subscribe/{eventType}. Example: /v1/sys/events/subscribe/kv*
if r.URL.Path == "/v1/sys/monitor" || strings.HasPrefix(r.URL.Path, "/v1/sys/events/subscribe") {
ctx, cancelFunc = context.WithCancel(ctx)
} else {
ctx, cancelFunc = context.WithTimeout(ctx, maxRequestDuration)

5
http/sys_monitor_test.go
View File

@@ -97,7 +97,8 @@ func TestSysMonitorStreamingLogs(t *testing.T) {
}
jsonLog := &jsonlog{}
timeCh := time.After(5 * time.Second)
// default timeout is 90 seconds
timeCh := time.After(120 * time.Second)
for {
select {
@@ -119,7 +120,7 @@ func TestSysMonitorStreamingLogs(t *testing.T) {
return
}
case <-timeCh:
t.Fatal("Failed to get a DEBUG message after 5 seconds")
t.Fatal("Failed to get a DEBUG message after 120 seconds")
}
}
})