Add support for configuring gRPC max-connection-age for the kiam server #440
Conversation
|
@pingles curios on your thoughts here, I added support for all of the |
8cfb9f7 to
3385c10
Compare
|
Thanks. Not sure how to better handle defaults, although perhaps the infinity defaults would be better to be limited anyway? The failure mode occurs when a server fails to resolve addresses (or server addresses are removed) but the load-balance won't re-fill the pool unless a connection change occurs. So the gRPC team's advice was to set those settings to ensure it triggers a reconnection. I wanted to get some tests to try and prove the failing behaviour today, and managed to get something rolling with #441. |
I was not sure of a good default here, but you are right we can do something more reasonable for "infinity" I am sure 1 month is even enough here? or we could pick a reasonable default that we know works whether that be 1 minute or something else we decide on. |
|
I would've thought something on the order of 15 minutes is a good starting point. https://github.com/pomerium/pomerium set it to 5 minutes. |
Done! if merged will add support to the chart over in #439 I also added support for the time/timeout config options too while I was there so that there is full control over all grpc options. |
| @@ -59,6 +59,11 @@ func (o *serverOptions) bind(parser parser) { | |||
| parser.Flag("session-refresh", "How soon STS Tokens should be refreshed before their expiration.").Default("5m").DurationVar(&o.SessionRefresh) | |||
| parser.Flag("assume-role-arn", "IAM Role to assume before processing requests").Default("").StringVar(&o.AssumeRoleArn) | |||
| parser.Flag("region", "AWS Region to use for regional STS calls (e.g. us-west-2). Defaults to the global endpoint.").Default("").StringVar(&o.Region) | |||
| parser.Flag("grpc-keepalive-time-duration", "gRPC keepalive time").Default("10s").DurationVar(&o.KeepaliveParams.Time) | |||
There was a problem hiding this comment.
I took the defaults for the time/timeout from the existing agent options.
| @@ -59,6 +59,11 @@ func (o *serverOptions) bind(parser parser) { | |||
| parser.Flag("session-refresh", "How soon STS Tokens should be refreshed before their expiration.").Default("5m").DurationVar(&o.SessionRefresh) | |||
| parser.Flag("assume-role-arn", "IAM Role to assume before processing requests").Default("").StringVar(&o.AssumeRoleArn) | |||
| parser.Flag("region", "AWS Region to use for regional STS calls (e.g. us-west-2). Defaults to the global endpoint.").Default("").StringVar(&o.Region) | |||
| parser.Flag("grpc-keepalive-time-duration", "gRPC keepalive time").Default("10s").DurationVar(&o.KeepaliveParams.Time) | |||
| parser.Flag("grpc-keepalive-timeout-duration", "gRPC keepalive timeout").Default("2s").DurationVar(&o.KeepaliveParams.Timeout) | |||
| parser.Flag("grpc-max-connection-idle-duration", "gRPC max connection idle").Default("15m").DurationVar(&o.KeepaliveParams.MaxConnectionIdle) | |||
There was a problem hiding this comment.
Defaulted max-connection options to 15m as discussed.
|
Thanks @stefansedich. I think some of those defaults may need to increase but let me pull the code and check. |
|
Actually, looks ok to me. I think it'd be good to get some real-world usage and see how it behaves but seems sensible to me to start with these. Thanks again. |
This PR adds support for configuring the
max-connectionrelated options for the kiam server.Closes #433