Fixed failing tests
Removed use of `NewTestAgent`, per review comment Removed CLI flag, per review comment Updated website documentation Added changelog entry
This commit is contained in:
parent
ed719c978b
commit
519f537b8b
|
@ -0,0 +1,3 @@
|
||||||
|
```release-note:feature
|
||||||
|
agent: add config flag `MaxHeaderBytes` to control the maximum size of the http header per client request.
|
||||||
|
```
|
|
@ -250,57 +250,85 @@ func TestAgent_HTTPMaxHeaderBytes(t *testing.T) {
|
||||||
tests := []struct {
|
tests := []struct {
|
||||||
name string
|
name string
|
||||||
maxHeaderBytes int
|
maxHeaderBytes int
|
||||||
expectError bool
|
|
||||||
expectedHTTPResponse int
|
expectedHTTPResponse int
|
||||||
}{
|
}{
|
||||||
{
|
{
|
||||||
"max header bytes 1 returns 431 http response when too large headers are sent",
|
"max header bytes 1 returns 431 http response when too large headers are sent",
|
||||||
1,
|
1,
|
||||||
false,
|
|
||||||
431,
|
431,
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"max header bytes 0 returns 200 http response, as the http.DefaultMaxHeaderBytes size of 1MB is used",
|
"max header bytes 0 returns 200 http response, as the http.DefaultMaxHeaderBytes size of 1MB is used",
|
||||||
0,
|
0,
|
||||||
false,
|
|
||||||
200,
|
200,
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"negative maxHeaderBytes returns 200 http response, as the http.DefaultMaxHeaderBytes size of 1MB is used",
|
"negative maxHeaderBytes returns 200 http response, as the http.DefaultMaxHeaderBytes size of 1MB is used",
|
||||||
-10,
|
-10,
|
||||||
false,
|
|
||||||
200,
|
200,
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
for _, tt := range tests {
|
for _, tt := range tests {
|
||||||
t.Run(tt.name, func(t *testing.T) {
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
a := NewTestAgent(t, fmt.Sprintf(`
|
ports, err := freeport.Take(1)
|
||||||
http_config {
|
require.NoError(t, err)
|
||||||
max_header_bytes = %d
|
t.Cleanup(func() { freeport.Return(ports) })
|
||||||
|
|
||||||
|
caConfig := tlsutil.Config{}
|
||||||
|
tlsConf, err := tlsutil.NewConfigurator(caConfig, hclog.New(nil))
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
bd := BaseDeps{
|
||||||
|
Deps: consul.Deps{
|
||||||
|
Logger: hclog.NewInterceptLogger(nil),
|
||||||
|
Tokens: new(token.Store),
|
||||||
|
TLSConfigurator: tlsConf,
|
||||||
|
},
|
||||||
|
RuntimeConfig: &config.RuntimeConfig{
|
||||||
|
HTTPAddrs: []net.Addr{
|
||||||
|
&net.TCPAddr{IP: net.ParseIP("127.0.0.1"), Port: ports[0]},
|
||||||
|
},
|
||||||
|
HTTPMaxHeaderBytes: tt.maxHeaderBytes,
|
||||||
|
},
|
||||||
|
Cache: cache.New(cache.Options{}),
|
||||||
}
|
}
|
||||||
`, tt.maxHeaderBytes))
|
a, err := New(bd)
|
||||||
defer a.Shutdown()
|
require.NoError(t, err)
|
||||||
|
|
||||||
require.Equal(t, tt.maxHeaderBytes, a.Agent.config.HTTPMaxHeaderBytes)
|
srvs, err := a.listenHTTP()
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
req, err := http.NewRequest(http.MethodGet, "http://"+a.HTTPAddr()+"/v1/health/state/passing", nil)
|
require.Equal(t, tt.maxHeaderBytes, a.config.HTTPMaxHeaderBytes)
|
||||||
require.NoError(t, err, "unexpected error creating new http request")
|
|
||||||
|
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
|
||||||
|
t.Cleanup(cancel)
|
||||||
|
|
||||||
|
g := new(errgroup.Group)
|
||||||
|
for _, s := range srvs {
|
||||||
|
g.Go(s.Run)
|
||||||
|
}
|
||||||
|
|
||||||
|
require.Len(t, srvs, 1)
|
||||||
|
|
||||||
|
client := &http.Client{}
|
||||||
|
for _, s := range srvs {
|
||||||
|
u := url.URL{Scheme: s.Protocol, Host: s.Addr.String()}
|
||||||
|
req, err := http.NewRequest(http.MethodGet, u.String(), nil)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
// This is directly pulled from the testing of request limits in the net/http source
|
// This is directly pulled from the testing of request limits in the net/http source
|
||||||
// https://github.com/golang/go/blob/go1.15.3/src/net/http/serve_test.go#L2897-L2900
|
// https://github.com/golang/go/blob/go1.15.3/src/net/http/serve_test.go#L2897-L2900
|
||||||
var bytesPerHeader = len("header12345: val12345\r\n")
|
var bytesPerHeader = len("header12345: val12345\r\n")
|
||||||
t.Logf("bytesPerHeader: %d", bytesPerHeader)
|
|
||||||
for i := 0; i < ((tt.maxHeaderBytes+4096)/bytesPerHeader)+1; i++ {
|
for i := 0; i < ((tt.maxHeaderBytes+4096)/bytesPerHeader)+1; i++ {
|
||||||
req.Header.Set(fmt.Sprintf("header%05d", i), fmt.Sprintf("val%05d", i))
|
req.Header.Set(fmt.Sprintf("header%05d", i), fmt.Sprintf("val%05d", i))
|
||||||
}
|
}
|
||||||
|
|
||||||
var res *http.Response
|
resp, err := client.Do(req.WithContext(ctx))
|
||||||
res, err = http.DefaultClient.Do(req)
|
require.NoError(t, err)
|
||||||
require.True(t, (tt.expectError && (err != nil)) || !tt.expectError && (err == nil))
|
require.Equal(t, tt.expectedHTTPResponse, resp.StatusCode, "expected a '%d' http response, got '%d'", tt.expectedHTTPResponse, resp.StatusCode)
|
||||||
require.Equal(t, tt.expectedHTTPResponse, res.StatusCode, "expected a '%d' http response, got '%d'", tt.expectedHTTPResponse, res.StatusCode)
|
}
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestAgent_ReconnectConfigWanDisabled(t *testing.T) {
|
func TestAgent_ReconnectConfigWanDisabled(t *testing.T) {
|
||||||
|
|
|
@ -75,7 +75,6 @@ func AddFlags(fs *flag.FlagSet, f *BuilderOpts) {
|
||||||
add(&f.Config.HTTPConfig.AllowWriteHTTPFrom, "allow-write-http-from", "Only allow write endpoint calls from given network. CIDR format, can be specified multiple times.")
|
add(&f.Config.HTTPConfig.AllowWriteHTTPFrom, "allow-write-http-from", "Only allow write endpoint calls from given network. CIDR format, can be specified multiple times.")
|
||||||
add(&f.Config.EncryptKey, "encrypt", "Provides the gossip encryption key.")
|
add(&f.Config.EncryptKey, "encrypt", "Provides the gossip encryption key.")
|
||||||
add(&f.Config.Ports.GRPC, "grpc-port", "Sets the gRPC API port to listen on (currently needed for Envoy xDS only).")
|
add(&f.Config.Ports.GRPC, "grpc-port", "Sets the gRPC API port to listen on (currently needed for Envoy xDS only).")
|
||||||
add(&f.Config.HTTPConfig.MaxHeaderBytes, "http-max-header-bytes", "Sets the HTTP Server's Max Header Bytes.")
|
|
||||||
add(&f.Config.Ports.HTTP, "http-port", "Sets the HTTP API port to listen on.")
|
add(&f.Config.Ports.HTTP, "http-port", "Sets the HTTP API port to listen on.")
|
||||||
add(&f.Config.Ports.HTTPS, "https-port", "Sets the HTTPS API port to listen on.")
|
add(&f.Config.Ports.HTTPS, "https-port", "Sets the HTTPS API port to listen on.")
|
||||||
add(&f.Config.StartJoinAddrsLAN, "join", "Address of an agent to join at start time. Can be specified multiple times.")
|
add(&f.Config.StartJoinAddrsLAN, "join", "Address of an agent to join at start time. Can be specified multiple times.")
|
||||||
|
|
|
@ -7675,6 +7675,7 @@ func TestSanitize(t *testing.T) {
|
||||||
],
|
],
|
||||||
"HTTPBlockEndpoints": [],
|
"HTTPBlockEndpoints": [],
|
||||||
"HTTPMaxConnsPerClient": 0,
|
"HTTPMaxConnsPerClient": 0,
|
||||||
|
"HTTPMaxHeaderBytes": 0,
|
||||||
"HTTPPort": 0,
|
"HTTPPort": 0,
|
||||||
"HTTPResponseHeaders": {},
|
"HTTPResponseHeaders": {},
|
||||||
"HTTPUseCache": false,
|
"HTTPUseCache": false,
|
||||||
|
|
|
@ -1631,6 +1631,8 @@ Valid time units are 'ns', 'us' (or 'µs'), 'ms', 's', 'm', 'h'."
|
||||||
|
|
||||||
- `use_cache` ((#http_config_use_cache)) Defaults to true. If disabled, the agent won't be using [agent caching](/api/features/caching) to answer the request. Even when the url parameter is provided.
|
- `use_cache` ((#http_config_use_cache)) Defaults to true. If disabled, the agent won't be using [agent caching](/api/features/caching) to answer the request. Even when the url parameter is provided.
|
||||||
|
|
||||||
|
- `max_header_bytes` This setting controls the maximum number of bytes the consul http server will read parsing the request header's keys and values, including the request line. It does not limit the size of the request body. If zero, or negative, http.DefaultMaxHeaderBytes is used, which equates to 1 Megabyte.
|
||||||
|
|
||||||
- `leave_on_terminate` If enabled, when the agent receives a TERM signal, it will send a `Leave` message to the rest of the cluster and gracefully leave. The default behavior for this feature varies based on whether or not the agent is running as a client or a server (prior to Consul 0.7 the default value was unconditionally set to `false`). On agents in client-mode, this defaults to `true` and for agents in server-mode, this defaults to `false`.
|
- `leave_on_terminate` If enabled, when the agent receives a TERM signal, it will send a `Leave` message to the rest of the cluster and gracefully leave. The default behavior for this feature varies based on whether or not the agent is running as a client or a server (prior to Consul 0.7 the default value was unconditionally set to `false`). On agents in client-mode, this defaults to `true` and for agents in server-mode, this defaults to `false`.
|
||||||
|
|
||||||
- `limits` Available in Consul 0.9.3 and later, this is a nested
|
- `limits` Available in Consul 0.9.3 and later, this is a nested
|
||||||
|
|
Loading…
Reference in New Issue