Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tls config to consul #6374

Merged
merged 1 commit into from
Nov 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
* [ENHANCEMENT] Distributor: Add new `cortex_reduced_resolution_histogram_samples_total` metric to track the number of histogram samples which resolution was reduced. #6182
* [ENHANCEMENT] StoreGateway: Implement metadata API limit in queryable. #6195
* [ENHANCEMENT] Ingester: Add matchers to ingester LabelNames() and LabelNamesStream() RPC. #6209
* [ENHANCEMENT] KV: Add TLS configs to consul. #6374
* [ENHANCEMENT] Ingester/Store Gateway Clients: Introduce an experimental HealthCheck handler to quickly fail requests directed to unhealthy targets. #6225 #6257
* [ENHANCEMENT] Upgrade build image and Go version to 1.23.2. #6261 #6262
* [ENHANCEMENT] Ingester: Introduce a new experimental feature for caching expanded postings on the ingester. #6296
Expand Down
29 changes: 28 additions & 1 deletion docs/configuration/config-file-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -2445,7 +2445,7 @@ The `consul_config` configures the consul client. The supported CLI flags `<pref
# CLI flag: -<prefix>.consul.acl-token
[acl_token: <string> | default = ""]

# HTTP timeout when talking to Consul
# HTTP timeout when talking to Consul.
# CLI flag: -<prefix>.consul.client-timeout
[http_client_timeout: <duration> | default = 20s]

Expand All @@ -2461,6 +2461,33 @@ The `consul_config` configures the consul client. The supported CLI flags `<pref
# Burst size used in rate limit. Values less than 1 are treated as 1.
# CLI flag: -<prefix>.consul.watch-burst-size
[watch_burst_size: <int> | default = 1]

# Enable TLS.
# CLI flag: -<prefix>.consul.tls-enabled
[tls_enabled: <boolean> | default = false]

# Path to the client certificate file, which will be used for authenticating
# with the server. Also requires the key path to be configured.
# CLI flag: -<prefix>.consul.tls-cert-path
[tls_cert_path: <string> | default = ""]

# Path to the key file for the client certificate. Also requires the client
# certificate to be configured.
# CLI flag: -<prefix>.consul.tls-key-path
[tls_key_path: <string> | default = ""]

# Path to the CA certificates file to validate server certificate against. If
# not set, the host's root CA certificates are used.
# CLI flag: -<prefix>.consul.tls-ca-path
[tls_ca_path: <string> | default = ""]

# Override the expected name on the server certificate.
# CLI flag: -<prefix>.consul.tls-server-name
[tls_server_name: <string> | default = ""]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this flag is used. Could we update the implementation so that it is used?

Copy link
Member Author

@SungJin1212 SungJin1212 Nov 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the catch :D


# Skip validating server certificate.
# CLI flag: -<prefix>.consul.tls-insecure-skip-verify
[tls_insecure_skip_verify: <boolean> | default = false]
```

### `distributor_config`
Expand Down
75 changes: 58 additions & 17 deletions pkg/ring/kv/consul/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/cortexproject/cortex/pkg/ring/kv/codec"
"github.com/cortexproject/cortex/pkg/util/backoff"
"github.com/cortexproject/cortex/pkg/util/flagext"
cortextls "github.com/cortexproject/cortex/pkg/util/tls"
)

const (
Expand All @@ -40,12 +41,14 @@ var (

// Config to create a ConsulClient
type Config struct {
Host string `yaml:"host"`
ACLToken flagext.Secret `yaml:"acl_token"`
HTTPClientTimeout time.Duration `yaml:"http_client_timeout"`
ConsistentReads bool `yaml:"consistent_reads"`
WatchKeyRateLimit float64 `yaml:"watch_rate_limit"` // Zero disables rate limit
WatchKeyBurstSize int `yaml:"watch_burst_size"` // Burst when doing rate-limit, defaults to 1
Host string `yaml:"host"`
ACLToken flagext.Secret `yaml:"acl_token"`
HTTPClientTimeout time.Duration `yaml:"http_client_timeout"`
ConsistentReads bool `yaml:"consistent_reads"`
WatchKeyRateLimit float64 `yaml:"watch_rate_limit"` // Zero disables rate limit
WatchKeyBurstSize int `yaml:"watch_burst_size"` // Burst when doing rate-limit, defaults to 1
EnableTLS bool `yaml:"tls_enabled"`
TLS cortextls.ClientConfig `yaml:",inline"`

// Used in tests only.
MaxCasRetries int `yaml:"-"`
Expand Down Expand Up @@ -74,24 +77,62 @@ type Client struct {
func (cfg *Config) RegisterFlags(f *flag.FlagSet, prefix string) {
f.StringVar(&cfg.Host, prefix+"consul.hostname", "localhost:8500", "Hostname and port of Consul.")
f.Var(&cfg.ACLToken, prefix+"consul.acl-token", "ACL Token used to interact with Consul.")
f.DurationVar(&cfg.HTTPClientTimeout, prefix+"consul.client-timeout", 2*longPollDuration, "HTTP timeout when talking to Consul")
f.DurationVar(&cfg.HTTPClientTimeout, prefix+"consul.client-timeout", 2*longPollDuration, "HTTP timeout when talking to Consul.")
f.BoolVar(&cfg.ConsistentReads, prefix+"consul.consistent-reads", false, "Enable consistent reads to Consul.")
f.Float64Var(&cfg.WatchKeyRateLimit, prefix+"consul.watch-rate-limit", 1, "Rate limit when watching key or prefix in Consul, in requests per second. 0 disables the rate limit.")
f.IntVar(&cfg.WatchKeyBurstSize, prefix+"consul.watch-burst-size", 1, "Burst size used in rate limit. Values less than 1 are treated as 1.")
f.BoolVar(&cfg.EnableTLS, prefix+"consul.tls-enabled", false, "Enable TLS.")
cfg.TLS.RegisterFlagsWithPrefix(prefix+"consul", f)
}

// NewClient returns a new Client.
func NewClient(cfg Config, codec codec.Codec, logger log.Logger, registerer prometheus.Registerer) (*Client, error) {
client, err := consul.NewClient(&consul.Config{
func (cfg *Config) GetTLS() *consul.TLSConfig {
return &consul.TLSConfig{
Address: cfg.TLS.ServerName,
CertFile: cfg.TLS.CertPath,
KeyFile: cfg.TLS.KeyPath,
CAFile: cfg.TLS.CAPath,
InsecureSkipVerify: cfg.TLS.InsecureSkipVerify,
}
}

func getConsulConfig(cfg Config) (*consul.Config, error) {
scheme := "http"
transport := cleanhttp.DefaultPooledTransport()

config := &consul.Config{
Address: cfg.Host,
Token: cfg.ACLToken.Value,
Scheme: "http",
HttpClient: &http.Client{
Transport: cleanhttp.DefaultPooledTransport(),
// See https://blog.cloudflare.com/the-complete-guide-to-golang-net-http-timeouts/
Timeout: cfg.HTTPClientTimeout,
},
})
}

if cfg.EnableTLS {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add a test for this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated the pr. Thanks!

tlsConfig := cfg.GetTLS()
tlsClientConfig, err := consul.SetupTLSConfig(tlsConfig)
if err != nil {
return nil, err
}
transport.TLSClientConfig = tlsClientConfig
scheme = "https"
config.TLSConfig = *tlsConfig
}

config.Scheme = scheme
config.HttpClient = &http.Client{
Transport: transport,
// See https://blog.cloudflare.com/the-complete-guide-to-golang-net-http-timeouts/
Timeout: cfg.HTTPClientTimeout,
}

return config, nil
}

// NewClient returns a new Client.
func NewClient(cfg Config, codec codec.Codec, logger log.Logger, registerer prometheus.Registerer) (*Client, error) {
config, err := getConsulConfig(cfg)
if err != nil {
return nil, err
}

client, err := consul.NewClient(config)
if err != nil {
return nil, err
}
Expand Down
78 changes: 78 additions & 0 deletions pkg/ring/kv/consul/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ package consul

import (
"context"
"crypto/x509"
"crypto/x509/pkix"
"fmt"
"path/filepath"
"strconv"
"testing"
"time"
Expand All @@ -12,7 +15,9 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/cortexproject/cortex/integration/ca"
"github.com/cortexproject/cortex/pkg/ring/kv/codec"
"github.com/cortexproject/cortex/pkg/util/tls"
)

func writeValuesToKV(t *testing.T, client *Client, key string, start, end int, sleep time.Duration) <-chan struct{} {
Expand All @@ -30,6 +35,79 @@ func writeValuesToKV(t *testing.T, client *Client, key string, start, end int, s
return ch
}

func TestGetConsulConfig(t *testing.T) {
testCADir := t.TempDir()

serverCA := ca.New("Consul Server CA")
caCertFile := filepath.Join(testCADir, "ca.crt")
require.NoError(t, serverCA.WriteCACertificate(caCertFile))

serverCertFile := filepath.Join(testCADir, "server.crt")
serverKeyFile := filepath.Join(testCADir, "server.key")
require.NoError(t, serverCA.WriteCertificate(
&x509.Certificate{
Subject: pkix.Name{CommonName: "server"},
DNSNames: []string{"localhost"},
ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
},
serverCertFile,
serverKeyFile,
))

clientCA := ca.New("Consul Client CA")
clientCACertFile := filepath.Join(testCADir, "client.crt")
require.NoError(t, clientCA.WriteCACertificate(clientCACertFile))

tests := []struct {
name string
cfg Config
}{
{
name: "tls config validation should return no error (skip verify: true)",
cfg: Config{
Host: "localhost:8501",
EnableTLS: true,
TLS: tls.ClientConfig{
CertPath: serverCertFile,
KeyPath: serverKeyFile,
CAPath: clientCACertFile,
ServerName: "testServer",
InsecureSkipVerify: true,
},
},
},
{
name: "tls config validation should return no error (skip verify: false)",
cfg: Config{
Host: "localhost:8501",
EnableTLS: true,
TLS: tls.ClientConfig{
CertPath: serverCertFile,
KeyPath: serverKeyFile,
CAPath: clientCACertFile,
ServerName: "testServer",
InsecureSkipVerify: false,
},
},
},
{
name: "no tls config should return no error",
cfg: Config{
Host: "localhost:8500",
EnableTLS: false,
},
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
_, err := getConsulConfig(test.cfg)
require.NoError(t, err)
})
}

}

func TestWatchKeyWithRateLimit(t *testing.T) {
c, closer := NewInMemoryClientWithConfig(codec.String{}, Config{
WatchKeyRateLimit: 5.0,
Expand Down
Loading