cmd/thanos/receive: remote-write client+server TLS #1668
Conversation
cmd/thanos/main.go
Outdated
| @@ -248,16 +248,28 @@ func registerMetrics(mux *http.ServeMux, g prometheus.Gatherer) { | |||
|
|
|||
| func defaultGRPCServerOpts(logger log.Logger, cert, key, clientCA string) ([]grpc.ServerOption, error) { | |||
There was a problem hiding this comment.
Looks like it is literally defaultGRPCTLSServerOpts right? no other opts (:
cmd/thanos/main.go
Outdated
| } | ||
|
|
||
| func defaultTLSClientOpts(logger log.Logger, secure bool, cert, key, caCert, serverName string) (*tls.Config, error) { | ||
| if !secure { |
There was a problem hiding this comment.
I know this does not make sense from a high level, but I think... let's remove this. With this change if someone use secure false and key and cert set we silently ignore key and cert. I think failing would be better.
There was a problem hiding this comment.
in that case should we remove the -secure flag as well? I also think it is confusing to explicitly have to opt into client TLS in addition to specifying the certs. but it's a breaking change
There was a problem hiding this comment.
I think yes - secure only makes sense if you have certs (?)
There was a problem hiding this comment.
hmm I'm looking at this again and we actually need -secure in order to enable TLS when I don't provide a client cert/key, e.g. I want to use TLS but no client authentication. The issue here is that every gRPC ClientConn created by the querier re-uses the same set of []DialOpt, which must be either secure OR insecure, but not both. For this reason, we need the -secure flag to be able to tell my gRPC client that I want to enable TLS even if not providing any other flags, like a cert and key.
Nevertheless, this is only a requirement for the gRPC client. The HTTP client can be given a TLS config even when connected to non-HTTPS servers and thus doesn't need a -secure flag.
squat
force-pushed
the
clienttls
branch
2 times, most recently
from
cd01b57 to
265afbb
Compare
This commit gives the Thanos receive component the capability to use TLS in both the remote-write client and server. This means that Thanos receive can now authenticate all requests. In order to accomplish this change, this commit abstracts the majority of the logic of `defaultGRPCServerOpts` into a reusable func for gRPC and HTTP servers and creates a similar func for TLS client configurations. Signed-off-by: Lucas Servén Marín <[email protected]>
|
ping @bwplotka (: |
This commit gives the Thanos receive component the capability to use TLS in both the remote-write client and server. This means that Thanos receive can now authenticate all requests. In order to accomplish this change, this commit abstracts the majority of the logic of `defaultGRPCServerOpts` into a reusable func for gRPC and HTTP servers and creates a similar func for TLS client configurations. Signed-off-by: Lucas Servén Marín <[email protected]> Signed-off-by: Giedrius Statkevičius <[email protected]>
This commit gives the Thanos receive component the capability to use TLS
in both the remote-write client and server. This means that Thanos
receive can now authenticate all requests.
In order to accomplish this change, this commit abstracts the majority
of the logic of
defaultGRPCServerOptsinto a reusable func for gRPCand HTTP servers and creates a similar func for TLS client
configurations.
Signed-off-by: Lucas Servén Marín [email protected]
cc @bwplotka @brancz, this is following our conversation yesterday about missing authn and encryption options for Thanos receive.