Skip to content

Commit

Permalink
fix review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@tcolgate
Tristan Colgate authored and tcolgate committed Sep 21, 2018
1 parent 33b4c31 commit 35284f6
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 24 deletions.
4 changes: 2 additions & 2 deletions cmd/thanos/flags.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ func regCommonServerFlags(cmd *kingpin.CmdClause) (
grpcAdvertiseAddr := cmd.Flag("grpc-advertise-address", "Explicit (external) host:port address to advertise for gRPC StoreAPI in gossip cluster. If empty, 'grpc-address' will be used.").
String()

grpcTLSSrvKey = cmd.Flag("grpc-server-tls-key", "TLS Key for the gRPC server, leave blank to disable TLS").Default("").String()
grpcTLSSrvCert = cmd.Flag("grpc-server-tls-cert", "TLS Certificate for gRPC server, leave blank to disable TLS").Default("").String()
grpcTLSSrvClientCA = cmd.Flag("grpc-server-tls-client-ca", "TLS CA to verify clients against").Default("").String()
grpcTLSSrvKey = cmd.Flag("grpc-server-tls-key", "TLS Key for the gRPC server, leave blank to disable TLS").Default("").String()
grpcTLSSrvClientCA = cmd.Flag("grpc-server-tls-client-ca", "TLS CA to verify clients against. If no client CA is specified, there is no client verification on server side. (tls.NoClientCert)").Default("").String()

httpBindAddr = regHTTPAddrFlag(cmd)

Expand Down
19 changes: 13 additions & 6 deletions cmd/thanos/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,12 +232,22 @@ func defaultGRPCServerOpts(logger log.Logger, reg *prometheus.Registry, tracer o
),
}

if key == "" || cert == "" {
if key == "" && cert == "" {
if clientCA != "" {
return nil, errors.New("when a client CA is used a server key and certificate must also be provided")
}

level.Info(logger).Log("msg", "disabled TLS, key and cert must be set to enable")
return opts, nil
}

tlsCfg := &tls.Config{}
if key == "" || cert == "" {
return nil, errors.New("both server key and certificate must be provided")
}

tlsCfg := &tls.Config{
MinVersion: tls.VersionTLS12,
}

tlsCert, err := tls.LoadX509KeyPair(cert, key)
if err != nil {
Expand All @@ -264,10 +274,7 @@ func defaultGRPCServerOpts(logger log.Logger, reg *prometheus.Registry, tracer o
level.Info(logger).Log("msg", "gRPC server TLS client verification enabled")
}

creds := credentials.NewTLS(tlsCfg)
opts = append(opts, grpc.Creds(creds))

return opts, nil
return append(opts, grpc.Creds(credentials.NewTLS(tlsCfg))), nil
}

// metricHTTPListenGroup is a run.Group that servers HTTP endpoint with only Prometheus metrics.
Expand Down
6 changes: 2 additions & 4 deletions cmd/thanos/query.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,7 @@ func storeClientGRPCOpts(logger log.Logger, reg *prometheus.Registry, tracer ope
}

if !secure {
dialOpts = append(dialOpts, grpc.WithInsecure())
return dialOpts, nil
return append(dialOpts, grpc.WithInsecure()), nil
}

level.Info(logger).Log("msg", "Enabling client to server TLS")
Expand Down Expand Up @@ -185,9 +184,8 @@ func storeClientGRPCOpts(logger log.Logger, reg *prometheus.Registry, tracer ope
}

creds := credentials.NewTLS(tlsCfg)
dialOpts = append(dialOpts, grpc.WithTransportCredentials(creds))

return dialOpts, nil
return append(dialOpts, grpc.WithTransportCredentials(creds)), nil
}

// runQuery starts a server that exposes PromQL Query API. It is responsible for querying configured
Expand Down
8 changes: 5 additions & 3 deletions docs/components/query.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,14 @@ Flags:
Explicit (external) host:port address to
advertise for gRPC StoreAPI in gossip cluster.
If empty, 'grpc-address' will be used.
--grpc-server-tls-key="" TLS Key for the gRPC server, leave blank to
disable TLS
--grpc-server-tls-cert="" TLS Certificate for gRPC server, leave blank to
disable TLS
--grpc-server-tls-key="" TLS Key for the gRPC server, leave blank to
disable TLS
--grpc-server-tls-client-ca=""
TLS CA to verify clients against
TLS CA to verify clients against. If no client
CA is specified, there is no client
verification on server side. (tls.NoClientCert)
--http-address="0.0.0.0:10902"
Listen host:port for HTTP endpoints.
--cluster.address="0.0.0.0:10900"
Expand Down
8 changes: 5 additions & 3 deletions docs/components/rule.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,14 @@ Flags:
Explicit (external) host:port address to
advertise for gRPC StoreAPI in gossip cluster.
If empty, 'grpc-address' will be used.
--grpc-server-tls-key="" TLS Key for the gRPC server, leave blank to
disable TLS
--grpc-server-tls-cert="" TLS Certificate for gRPC server, leave blank to
disable TLS
--grpc-server-tls-key="" TLS Key for the gRPC server, leave blank to
disable TLS
--grpc-server-tls-client-ca=""
TLS CA to verify clients against
TLS CA to verify clients against. If no client
CA is specified, there is no client
verification on server side. (tls.NoClientCert)
--http-address="0.0.0.0:10902"
Listen host:port for HTTP endpoints.
--cluster.address="0.0.0.0:10900"
Expand Down
8 changes: 5 additions & 3 deletions docs/components/sidecar.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,14 @@ Flags:
Explicit (external) host:port address to
advertise for gRPC StoreAPI in gossip cluster.
If empty, 'grpc-address' will be used.
--grpc-server-tls-key="" TLS Key for the gRPC server, leave blank to
disable TLS
--grpc-server-tls-cert="" TLS Certificate for gRPC server, leave blank to
disable TLS
--grpc-server-tls-key="" TLS Key for the gRPC server, leave blank to
disable TLS
--grpc-server-tls-client-ca=""
TLS CA to verify clients against
TLS CA to verify clients against. If no client
CA is specified, there is no client
verification on server side. (tls.NoClientCert)
--http-address="0.0.0.0:10902"
Listen host:port for HTTP endpoints.
--cluster.address="0.0.0.0:10900"
Expand Down
8 changes: 5 additions & 3 deletions docs/components/store.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,14 @@ Flags:
Explicit (external) host:port address to
advertise for gRPC StoreAPI in gossip cluster.
If empty, 'grpc-address' will be used.
--grpc-server-tls-key="" TLS Key for the gRPC server, leave blank to
disable TLS
--grpc-server-tls-cert="" TLS Certificate for gRPC server, leave blank to
disable TLS
--grpc-server-tls-key="" TLS Key for the gRPC server, leave blank to
disable TLS
--grpc-server-tls-client-ca=""
TLS CA to verify clients against
TLS CA to verify clients against. If no client
CA is specified, there is no client
verification on server side. (tls.NoClientCert)
--http-address="0.0.0.0:10902"
Listen host:port for HTTP endpoints.
--cluster.address="0.0.0.0:10900"
Expand Down

0 comments on commit 35284f6

Please sign in to comment.