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.

Sign up for GitHub

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

Jump to bottom

Add TLS support for Memcached client #3585

Merged
merged 5 commits into from
Apr 19, 2024
Merged

Add TLS support for Memcached client #3585

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
43f40a4
Adding TLS support for Memcached Client
sonisr Apr 17, 2024
ce297a9
Update CHANGELOG.md
sonisr Apr 17, 2024
58cf58d
Resolve linting and build failures
sonisr Apr 17, 2024
4d079e7
newClient not netClient
sonisr Apr 17, 2024
6342810
Include username in CHANGELOG entry
sonisr Apr 17, 2024
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
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## main / unreleased

* [FEATURE] Add TLS support for Memcached Client [#3585](https://github.com/grafana/tempo/pull/3585) (@sonisr)
* [ENHANCEMENT] Add querier metrics for requests executed [#3524](https://github.com/grafana/tempo/pull/3524) (@electron0zero)
* [FEATURE] Added gRPC streaming endpoints for all tag queries. [#3460](https://github.com/grafana/tempo/pull/3460) (@joe-elliott)
* [CHANGE] Align metrics query time ranges to the step parameter [#3490](https://github.com/grafana/tempo/pull/3490) (@mdisibio)
Expand Down
66 changes: 66 additions & 0 deletions docs/sources/tempo/configuration/_index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1667,6 +1667,72 @@ cache:
# (default: 10s)
[circuit_breaker_interval: 10s]

# Enable connecting to Memcached with TLS.
# CLI flag: -<prefix>.memcached.tls-enabled
[tls_enabled: <boolean> | default = false]

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

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

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

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

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

# Override the default cipher suite list (separated by commas). Allowed
# values:
#
# Secure Ciphers:
# - TLS_RSA_WITH_AES_128_CBC_SHA
# - TLS_RSA_WITH_AES_256_CBC_SHA
# - TLS_RSA_WITH_AES_128_GCM_SHA256
# - TLS_RSA_WITH_AES_256_GCM_SHA384
# - TLS_AES_128_GCM_SHA256
# - TLS_AES_256_GCM_SHA384
# - TLS_CHACHA20_POLY1305_SHA256
# - TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA
# - TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA
# - TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA
# - TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA
# - TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
# - TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384
# - TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256
# - TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384
# - TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256
# - TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256
#
# Insecure Ciphers:
# - TLS_RSA_WITH_RC4_128_SHA
# - TLS_RSA_WITH_3DES_EDE_CBC_SHA
# - TLS_RSA_WITH_AES_128_CBC_SHA256
# - TLS_ECDHE_ECDSA_WITH_RC4_128_SHA
# - TLS_ECDHE_RSA_WITH_RC4_128_SHA
# - TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA
# - TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256
# - TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256
# CLI flag: -<prefix>.memcached.tls-cipher-suites
[tls_cipher_suites: <string> | default = ""]

# Override the default minimum TLS version. Allowed values: VersionTLS10,
# VersionTLS11, VersionTLS12, VersionTLS13
# CLI flag: -<prefix>.memcached.tls-min-version
[tls_min_version: <string> | default = ""]

# Redis configuration block
# EXPERIMENTAL
redis:
Expand Down
53 changes: 41 additions & 12 deletions pkg/cache/memcached_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package cache

import (
"context"
"crypto/tls"
"flag"
"fmt"
"net"
Expand All @@ -13,6 +14,7 @@ import (

"github.com/go-kit/log"
"github.com/go-kit/log/level"
dstls "github.com/grafana/dskit/crypto/tls"
"github.com/grafana/dskit/dns"
"github.com/grafana/gomemcache/memcache"
"github.com/prometheus/client_golang/prometheus"
Expand Down Expand Up @@ -61,21 +63,27 @@ type memcachedClient struct {
skipped prometheus.Counter

logger log.Logger

cfg MemcachedClientConfig

DialTimeout func(network, address string, timeout time.Duration) (net.Conn, error)
}

// MemcachedClientConfig defines how a MemcachedClient should be constructed.
type MemcachedClientConfig struct {
Host string `yaml:"host"`
Service string `yaml:"service"`
Addresses string `yaml:"addresses"` // EXPERIMENTAL.
Timeout time.Duration `yaml:"timeout"`
MaxIdleConns int `yaml:"max_idle_conns"`
MaxItemSize int `yaml:"max_item_size"`
UpdateInterval time.Duration `yaml:"update_interval"`
ConsistentHash bool `yaml:"consistent_hash"`
CBFailures uint `yaml:"circuit_breaker_consecutive_failures"`
CBTimeout time.Duration `yaml:"circuit_breaker_timeout"` // reset error count after this long
CBInterval time.Duration `yaml:"circuit_breaker_interval"` // remain closed for this long after CBFailures errors
Host string `yaml:"host"`
Service string `yaml:"service"`
Addresses string `yaml:"addresses"` // EXPERIMENTAL.
Timeout time.Duration `yaml:"timeout"`
MaxIdleConns int `yaml:"max_idle_conns"`
MaxItemSize int `yaml:"max_item_size"`
UpdateInterval time.Duration `yaml:"update_interval"`
ConsistentHash bool `yaml:"consistent_hash"`
CBFailures uint `yaml:"circuit_breaker_consecutive_failures"`
CBTimeout time.Duration `yaml:"circuit_breaker_timeout"` // reset error count after this long
CBInterval time.Duration `yaml:"circuit_breaker_interval"` // remain closed for this long after CBFailures errors
TLSEnabled bool `yaml:"tls_enabled"` // Enable connecting to Memcached with TLS.
TLS dstls.ClientConfig `yaml:",inline"` // TLS to use to connect to the Memcached server.
}

// RegisterFlagsWithPrefix adds the flags required to config this to the given FlagSet
Expand All @@ -91,6 +99,8 @@ func (cfg *MemcachedClientConfig) RegisterFlagsWithPrefix(prefix, description st
f.DurationVar(&cfg.CBTimeout, prefix+"memcached.circuit-breaker-timeout", 10*time.Second, description+"Duration circuit-breaker remains open after tripping (if zero then 60 seconds is used).")
f.DurationVar(&cfg.CBInterval, prefix+"memcached.circuit-breaker-interval", 10*time.Second, description+"Reset circuit-breaker counts after this long (if zero then never reset).")
f.IntVar(&cfg.MaxItemSize, prefix+"memcached.max-item-size", 0, description+"The maximum size of an item stored in memcached. Bigger items are not stored. If set to 0, no maximum size is enforced.")
f.BoolVar(&cfg.TLSEnabled, prefix+"memcached.tls-enabled", false, "Enable connecting to Memcached with TLS.")
cfg.TLS.RegisterFlagsWithPrefix(prefix+"memcached.", f)
}

// NewMemcachedClient creates a new MemcacheClient that gets its server list
Expand All @@ -111,9 +121,26 @@ func NewMemcachedClient(cfg MemcachedClientConfig, name string, r prometheus.Reg
"name": name,
}, r))

dialTimeout := net.DialTimeout
if cfg.TLSEnabled {
cfg, err := cfg.TLS.GetTLSConfig()
if err != nil {
level.Error(logger).Log("msg", "couldn't create TLS configuration", "err", err)
} else {
dialTimeout = func(network string, address string, timeout time.Duration) (net.Conn, error) {
base := new(net.Dialer)
base.Timeout = timeout

return tls.DialWithDialer(base, network, address, cfg)
}
}
}

newClient := &memcachedClient{
DialTimeout: dialTimeout,
name: name,
Client: client,
cfg: cfg,
serverList: selector,
hostname: cfg.Host,
service: cfg.Service,
Expand Down Expand Up @@ -142,6 +169,8 @@ func NewMemcachedClient(cfg MemcachedClientConfig, name string, r prometheus.Reg
}
if cfg.CBFailures > 0 {
newClient.Client.DialTimeout = newClient.dialViaCircuitBreaker
} else {
newClient.Client.DialTimeout = dialTimeout
}

if len(cfg.Addresses) > 0 {
Expand Down Expand Up @@ -181,7 +210,7 @@ func (c *memcachedClient) dialViaCircuitBreaker(network, address string, timeout
c.Unlock()

conn, err := cb.Execute(func() (interface{}, error) {
return net.DialTimeout(network, address, timeout)
return c.DialTimeout(network, address, timeout)
Copy link
Member

Choose a reason for hiding this comment

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

it seems like this DialTimeout method is available here as c.Client.DialTimeout is there a reason to store it directly on the object?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I imagine you could use c.Client.DialTimeout in lieu of storing it directly on the object.

Personally, not so sure if there's any benefits of using one over the other here.

There doesn't seem to be any adverse effects from storing it directly on the object in Loki. Though, it could possibly differ in Tempo.

})
if err != nil {
return nil, err
Expand Down
Loading