From 90c462342569aebeb8df0a821874697c4fa9edab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20Fern=C3=A1ndez?= Date: Wed, 3 Feb 2021 18:45:18 +0100 Subject: [PATCH 1/7] Preparatory refactoring I: Remove unused InstanceKey methods --- pkg/mysql/instance_key.go | 15 +-------------- pkg/mysql/instance_key_test.go | 9 --------- 2 files changed, 1 insertion(+), 23 deletions(-) diff --git a/pkg/mysql/instance_key.go b/pkg/mysql/instance_key.go index e7c40389..3b32013b 100644 --- a/pkg/mysql/instance_key.go +++ b/pkg/mysql/instance_key.go @@ -15,7 +15,7 @@ const ( DefaultMySQLPort = 3306 ) -// InstanceKey is an instance indicator, identifued by hostname and port +// InstanceKey is an instance indicator, identified by hostname and port type InstanceKey struct { Hostname string Port int @@ -46,14 +46,6 @@ func ParseInstanceKey(hostPort string, defaultPort int) (*InstanceKey, error) { return newRawInstanceKey(hostPort) } -// Equals tests equality between this key and another key -func (this *InstanceKey) Equals(other *InstanceKey) bool { - if other == nil { - return false - } - return this.Hostname == other.Hostname && this.Port == other.Port -} - // SmallerThan returns true if this key is dictionary-smaller than another. // This is used for consistent sorting/ordering; there's nothing magical about it. func (this *InstanceKey) SmallerThan(other *InstanceKey) bool { @@ -79,11 +71,6 @@ func (this *InstanceKey) StringCode() string { return fmt.Sprintf("%s:%d", this.Hostname, this.Port) } -// DisplayString returns a user-friendly string representation of this key -func (this *InstanceKey) DisplayString() string { - return this.StringCode() -} - // String returns a user-friendly string representation of this key func (this InstanceKey) String() string { return this.StringCode() diff --git a/pkg/mysql/instance_key_test.go b/pkg/mysql/instance_key_test.go index b689491a..539948c0 100644 --- a/pkg/mysql/instance_key_test.go +++ b/pkg/mysql/instance_key_test.go @@ -52,15 +52,6 @@ func TestParseInstanceKey(t *testing.T) { } } -func TestEquals(t *testing.T) { - { - expect := &InstanceKey{Hostname: "127.0.0.1", Port: 3306} - key, err := ParseInstanceKey("127.0.0.1", 3306) - test.S(t).ExpectNil(err) - test.S(t).ExpectTrue(key.Equals(expect)) - } -} - func TestStringCode(t *testing.T) { { key := &InstanceKey{Hostname: "127.0.0.1", Port: 3306} From 899acf307891eef64fa0cbae80697eefece17a52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20Fern=C3=A1ndez?= Date: Wed, 3 Feb 2021 18:55:31 +0100 Subject: [PATCH 2/7] Preparatory refactoring II: Remove unused Probe methods --- pkg/mysql/probe.go | 18 --------------- pkg/mysql/probe_test.go | 51 ----------------------------------------- 2 files changed, 69 deletions(-) delete mode 100644 pkg/mysql/probe_test.go diff --git a/pkg/mysql/probe.go b/pkg/mysql/probe.go index 46853700..0dd8dabc 100644 --- a/pkg/mysql/probe.go +++ b/pkg/mysql/probe.go @@ -47,28 +47,10 @@ func NewProbe() *Probe { return config } -// DuplicateCredentials creates a new connection config with given key and with same credentials as this config -func (this *Probe) DuplicateCredentials(key InstanceKey) *Probe { - config := &Probe{ - Key: key, - User: this.User, - Password: this.Password, - } - return config -} - -func (this *Probe) Duplicate() *Probe { - return this.DuplicateCredentials(this.Key) -} - func (this *Probe) String() string { return fmt.Sprintf("%s, user=%s", this.Key.DisplayString(), this.User) } -func (this *Probe) Equals(other *Probe) bool { - return this.Key.Equals(&other.Key) -} - func (this *Probe) GetDBUri(databaseName string) string { hostname := this.Key.Hostname var ip = net.ParseIP(hostname) diff --git a/pkg/mysql/probe_test.go b/pkg/mysql/probe_test.go deleted file mode 100644 index d4145b0c..00000000 --- a/pkg/mysql/probe_test.go +++ /dev/null @@ -1,51 +0,0 @@ -/* - Copyright 2017 GitHub Inc. - See https://github.com/github/freno/blob/master/LICENSE -*/ - -package mysql - -import ( - "testing" - - "github.com/outbrain/golib/log" - test "github.com/outbrain/golib/tests" -) - -func init() { - log.SetLevel(log.ERROR) -} - -func TestNewProbe(t *testing.T) { - c := NewProbe() - test.S(t).ExpectEquals(c.Key.Hostname, "") - test.S(t).ExpectEquals(c.Key.Port, 0) - test.S(t).ExpectEquals(c.User, "") - test.S(t).ExpectEquals(c.Password, "") -} - -func TestDuplicateCredentials(t *testing.T) { - c := NewProbe() - c.Key = InstanceKey{Hostname: "myhost", Port: 3306} - c.User = "gromit" - c.Password = "penguin" - - dup := c.DuplicateCredentials(InstanceKey{Hostname: "otherhost", Port: 3310}) - test.S(t).ExpectEquals(dup.Key.Hostname, "otherhost") - test.S(t).ExpectEquals(dup.Key.Port, 3310) - test.S(t).ExpectEquals(dup.User, "gromit") - test.S(t).ExpectEquals(dup.Password, "penguin") -} - -func TestDuplicate(t *testing.T) { - c := NewProbe() - c.Key = InstanceKey{Hostname: "myhost", Port: 3306} - c.User = "gromit" - c.Password = "penguin" - - dup := c.Duplicate() - test.S(t).ExpectEquals(dup.Key.Hostname, "myhost") - test.S(t).ExpectEquals(dup.Key.Port, 3306) - test.S(t).ExpectEquals(dup.User, "gromit") - test.S(t).ExpectEquals(dup.Password, "penguin") -} From 0868263abd4ed051445405ec283a87693e901d73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20Fern=C3=A1ndez?= Date: Wed, 3 Feb 2021 18:58:47 +0100 Subject: [PATCH 3/7] Preparatory refactoring III: Extract mysql URI creation simplifying probe and mysqlInventory --- pkg/group/mysql.go | 43 +++++++++++++++------- pkg/mysql/mysql_throttle_metric.go | 4 +- pkg/mysql/probe.go | 46 ++++++++++++----------- pkg/mysql/uri.go | 24 ++++++++++++ pkg/throttle/throttler.go | 59 +++++++++++++++++------------- 5 files changed, 113 insertions(+), 63 deletions(-) create mode 100644 pkg/mysql/uri.go diff --git a/pkg/group/mysql.go b/pkg/group/mysql.go index 526afdcf..1f129d07 100644 --- a/pkg/group/mysql.go +++ b/pkg/group/mysql.go @@ -24,7 +24,7 @@ service_id varchar(128) NOT NULL, CREATE TABLE throttled_apps ( app_name varchar(128) NOT NULL, throttled_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, - expires_at TIMESTAMP NOT NULL, + expires_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, ratio DOUBLE, PRIMARY KEY (app_name) ); @@ -40,6 +40,8 @@ import ( "sync/atomic" "time" + "github.com/github/freno/pkg/mysql" + "github.com/github/freno/pkg/base" "github.com/github/freno/pkg/config" "github.com/github/freno/pkg/throttle" @@ -59,27 +61,42 @@ type MySQLBackend struct { throttler *throttle.Throttler } -const maxConnections = 3 -const electionExpireSeconds = 5 +const ( + maxConnections = 3 + electionExpireSeconds = 5 + + electionInterval = time.Second + healthInterval = 2 * electionInterval + stateInterval = 10 * time.Second -const electionInterval = time.Second -const healthInterval = 2 * electionInterval -const stateInterval = 10 * time.Second + connectionTimeout = 500 * time.Millisecond +) func NewMySQLBackend(throttler *throttle.Throttler) (*MySQLBackend, error) { - if config.Settings().BackendMySQLHost == "" { + settings := config.Settings() + if settings.BackendMySQLHost == "" { return nil, nil } - dbUri := fmt.Sprintf("%s:%s@tcp(%s:%d)/%s?interpolateParams=true&charset=utf8mb4,utf8,latin1&timeout=500ms", - config.Settings().BackendMySQLUser, config.Settings().BackendMySQLPassword, config.Settings().BackendMySQLHost, config.Settings().BackendMySQLPort, config.Settings().BackendMySQLSchema, - ) - db, _, err := sqlutils.GetDB(dbUri) + uri, err := mysql.MakeUri( + settings.BackendMySQLHost, + settings.BackendMySQLPort, + settings.BackendMySQLSchema, + settings.BackendMySQLUser, + settings.BackendMySQLPassword, + connectionTimeout) + if err != nil { return nil, err } + + db, _, err := sqlutils.GetDB(uri) + if err != nil { + return nil, err + } + db.SetMaxOpenConns(maxConnections) db.SetMaxIdleConns(maxConnections) - log.Debugf("created db at: %s", dbUri) + log.Debugf("created db at: %s", uri) hostname, err := os.Hostname() if err != nil { return nil, err @@ -368,7 +385,7 @@ func (backend *MySQLBackend) ThrottleApp(appName string, ttlMinutes int64, expir on duplicate key update ratio=values(ratio) ` - args = sqlutils.Args(appName, throttle.DefaultThrottleTTLMinutes, ratio) + args = sqlutils.Args(appName, throttle.DefaultThrottleTTL.Minutes(), ratio) } _, err := sqlutils.ExecNoPrepare(backend.db, query, args...) backend.throttler.ThrottleApp(appName, expireAt, ratio) diff --git a/pkg/mysql/mysql_throttle_metric.go b/pkg/mysql/mysql_throttle_metric.go index a67031ed..d47dc448 100644 --- a/pkg/mysql/mysql_throttle_metric.go +++ b/pkg/mysql/mysql_throttle_metric.go @@ -88,9 +88,7 @@ func ReadThrottleMetric(probe *Probe, clusterName string) (mySQLThrottleMetric * }() }(mySQLThrottleMetric, started) - dbUri := probe.GetDBUri("information_schema") - db, fromCache, err := sqlutils.GetDB(dbUri) - + db, fromCache, err := sqlutils.GetDB(probe.Uri) if err != nil { mySQLThrottleMetric.Err = err return mySQLThrottleMetric diff --git a/pkg/mysql/probe.go b/pkg/mysql/probe.go index 0dd8dabc..80aaacf3 100644 --- a/pkg/mysql/probe.go +++ b/pkg/mysql/probe.go @@ -7,18 +7,19 @@ package mysql import ( "fmt" - "net" + "time" ) -const maxPoolConnections = 3 -const maxIdleConnections = 3 -const timeoutMillis = 1000 +const ( + maxPoolConnections = 3 + maxIdleConnections = 3 + probeTimeout = 10 * time.Millisecond +) // Probe is the minimal configuration required to connect to a MySQL server type Probe struct { Key InstanceKey - User string - Password string + Uri string MetricQuery string CacheMillis int QueryInProgress int64 @@ -40,23 +41,26 @@ func NewProbes() *Probes { return &Probes{} } -func NewProbe() *Probe { - config := &Probe{ - Key: InstanceKey{}, +// NewProbe allocates memory for a new Probe value and returns its address, or an error in case tlsConfiguration parameters were +// provided, but TLS configuration couldn't be registered. If that's the case, the address of the probe will be nil. +func NewProbe(key *InstanceKey, user, password, databaseName, metricQuery string, cacheMillis int, httpCheckPath string, httpCheckPort int) (*Probe, error) { + uri, err := MakeUri(key.Hostname, key.Port, user, password, databaseName, probeTimeout) + if err != nil { + return nil, fmt.Errorf("cannot create probe. Cause: %w", err) } - return config -} -func (this *Probe) String() string { - return fmt.Sprintf("%s, user=%s", this.Key.DisplayString(), this.User) + p := Probe{ + Key: *key, + Uri: uri, + MetricQuery: metricQuery, + CacheMillis: cacheMillis, + HttpCheckPath: httpCheckPath, + HttpCheckPort: httpCheckPort, + } + + return &p, nil } -func (this *Probe) GetDBUri(databaseName string) string { - hostname := this.Key.Hostname - var ip = net.ParseIP(hostname) - if (ip != nil) && (ip.To4() == nil) { - // Wrap IPv6 literals in square brackets - hostname = fmt.Sprintf("[%s]", hostname) - } - return fmt.Sprintf("%s:%s@tcp(%s:%d)/%s?interpolateParams=true&charset=utf8mb4,utf8,latin1&timeout=%dms", this.User, this.Password, hostname, this.Key.Port, databaseName, timeoutMillis) +func (p *Probe) String() string { + return p.Key.String() } diff --git a/pkg/mysql/uri.go b/pkg/mysql/uri.go new file mode 100644 index 00000000..3b60e695 --- /dev/null +++ b/pkg/mysql/uri.go @@ -0,0 +1,24 @@ +package mysql + +import ( + "fmt" + "net" + "time" +) + +const timeout = 10 * time.Millisecond + +// MakeUri creates a new string representing the URI for the mysql driver to connect to, including timeout, charset and tls settings. +// In case the URI cannot be created due to a wrong TLS configuration, an error is returned. +func MakeUri(hostname string, port int, databaseName, user, password string, timeout time.Duration) (uri string, err error) { + tlsKey := "false" + + ip := net.ParseIP(hostname) + if (ip != nil) && (ip.To4() == nil) { + // Wrap IPv6 literals in square brackets + hostname = fmt.Sprintf("[%s]", hostname) + } + + uri = fmt.Sprintf("%s:%s@tcp(%s:%d)/%s?interpolateParams=true&charset=utf8mb4,utf8,latin1&tls=%s&timeout=%dms", user, password, hostname, port, databaseName, tlsKey, timeout.Milliseconds()) + return uri, err +} diff --git a/pkg/throttle/throttler.go b/pkg/throttle/throttler.go index 8210304d..b5bb4d4c 100644 --- a/pkg/throttle/throttler.go +++ b/pkg/throttle/throttler.go @@ -25,23 +25,27 @@ import ( metrics "github.com/rcrowley/go-metrics" ) -const leaderCheckInterval = 1 * time.Second -const mysqlCollectInterval = 50 * time.Millisecond -const mysqlRefreshInterval = 10 * time.Second -const mysqlAggreateInterval = 25 * time.Millisecond -const mysqlHttpCheckInterval = 5 * time.Second -const sharedDomainCollectInterval = 1 * time.Second - -const aggregatedMetricsExpiration = 5 * time.Second -const aggregatedMetricsCleanup = 1 * time.Second -const throttledAppsSnapshotInterval = 5 * time.Second -const recentAppsExpiration = time.Hour * 24 - -const nonDeprioritizedAppMapExpiration = time.Second -const nonDeprioritizedAppMapInterval = 100 * time.Millisecond - -const DefaultThrottleTTLMinutes = 60 -const DefaultThrottleRatio = 1.0 +const ( + leaderCheckInterval = 1 * time.Second + mysqlCollectInterval = 50 * time.Millisecond + mysqlRefreshInterval = 10 * time.Second + mysqlAggreateInterval = 25 * time.Millisecond + mysqlHttpCheckInterval = 5 * time.Second + + sharedDomainCollectInterval = 1 * time.Second + aggregatedMetricsExpiration = 5 * time.Second + aggregatedMetricsCleanup = 1 * time.Second + throttledAppsSnapshotInterval = 5 * time.Second + recentAppsExpiration = time.Hour * 24 + + nonDeprioritizedAppMapExpiration = time.Second + nonDeprioritizedAppMapInterval = 100 * time.Millisecond + + defaultDatabaseName = "information-schema" + + DefaultThrottleTTL = 60 * time.Minute + DefaultThrottleRatio = 1.0 +) func init() { rand.Seed(time.Now().UnixNano()) @@ -280,14 +284,17 @@ func (throttler *Throttler) refreshMySQLInventory() error { } log.Debugf("read instance key: %+v", key) - probe := &mysql.Probe{ - Key: *key, - User: clusterSettings.User, - Password: clusterSettings.Password, - MetricQuery: clusterSettings.MetricQuery, - CacheMillis: clusterSettings.CacheMillis, - HttpCheckPath: clusterSettings.HttpCheckPath, - HttpCheckPort: clusterSettings.HttpCheckPort, + probe, err := mysql.NewProbe(key, + clusterSettings.User, + clusterSettings.Password, + defaultDatabaseName, + clusterSettings.MetricQuery, + clusterSettings.CacheMillis, + clusterSettings.HttpCheckPath, + clusterSettings.HttpCheckPort, + ) + if err != nil { + log.Fatale(err) } (*probes)[*key] = probe } @@ -509,7 +516,7 @@ func (throttler *Throttler) ThrottleApp(appName string, expireAt time.Time, rati } } else { if expireAt.IsZero() { - expireAt = now.Add(DefaultThrottleTTLMinutes * time.Minute) + expireAt = now.Add(DefaultThrottleTTL) } if ratio < 0 { ratio = DefaultThrottleRatio From fb9728f7c92304b1351f1e86c187a4e2a9f95112 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20Fern=C3=A1ndez?= Date: Wed, 3 Feb 2021 19:00:27 +0100 Subject: [PATCH 4/7] Implement TLS configuration --- doc/mysql-backend.md | 10 +++++++- doc/mysql.md | 5 ++++ pkg/config/config.go | 50 ++++++++++++++++++++++++-------------- pkg/config/mysql_config.go | 30 +++++++++++++++++++++-- 4 files changed, 74 insertions(+), 21 deletions(-) diff --git a/doc/mysql-backend.md b/doc/mysql-backend.md index 7561a981..24d78599 100644 --- a/doc/mysql-backend.md +++ b/doc/mysql-backend.md @@ -31,6 +31,9 @@ Let's dissect the general section of the [sample config file](../resources/freno "BackendMySQLSchema": "freno_backend", "BackendMySQLUser": "freno_daemon", "BackendMySQLPassword": "123456", + "BackendMySQLTlsCaCertPath": "/usr/local/share/certs/ca.crt", + "BackendMySQLTlsClientCertPath": "/usr/local/share/certs/client.crt", + "BackendMySQLTlsClientKeyPath": "/usr/local/share/certs/client.key", "Domain": "us-east-1/production", "ShareDomain": "production", } @@ -40,12 +43,14 @@ Let's dissect the general section of the [sample config file](../resources/freno - `BackendMySQLPort`: MySQL master port - `BackendMySQLSchema`: schema where `freno` will read/write state (see below) - `BackendMySQLUser`: user with read+write privileges on backend schema +- `BackendMySQLTlsCaCertPath´: optional file system path for the PEM-encoded CA certificate to be used when connecting to mysql using TLS +- `BackendMySQLTlsClientCertPath`: optional file system path for the PEM-encoded client certificate +- `BackendMySQLTlsClientKeyPath`: optional file system path for the PEM-encoded client key - `BackendMySQLPassword`: password - `Domain`: the same MySQL backend can serve multiple, unrelated `freno` clusters. Nodes within the same cluster should have the same `Domain` value and will compete for leadership. - `ShareDomain`: it is possible for clusters to collaborate. Clusters with same `ShareDomain` will consul with each other's metric health reports. A cluster may reject a `check` request if another cluster considers the `check` metrics unhealthy. You may exchange the above for environment variables: - ```json { "BackendMySQLHost": "${MYSQL_BACKEND_HOST}", @@ -53,6 +58,9 @@ You may exchange the above for environment variables: "BackendMySQLSchema": "${MYSQL_BACKEND_SCHEMA}", "BackendMySQLUser": "${MYSQL_BACKEND_RW_USER}", "BackendMySQLPassword": "${MYSQL_BACKEND_RW_PASSWORD}", + "BackendMySQLTlsCaCertPath": "${MYSQL_BACKEND_CA_CERT}", + "BackendMySQLTlsClientCertPath": "${MYSQL_BACKEND_CLIENT_CERT}", + "BackendMySQLTlsClientKeyPath": "${MYSQL_BACKEND_CLIENT_KEY}", "Domain": "us-east-1/production", "ShareDomain": "production", } diff --git a/doc/mysql.md b/doc/mysql.md index 0b651ceb..c5458197 100644 --- a/doc/mysql.md +++ b/doc/mysql.md @@ -34,6 +34,9 @@ You will find the top-level configuration: "MySQL": { "User": "some_user", "Password": "${mysql_password_env_variable}", + "TlsCaCertPath": "/usr/local/share/certs/ca_freno.pem", + "TlsClientCertPath": "${client_cert_path_env_variable}", + "TlsClientKeyPath": "${client_key_path_env_variable}", "MetricQuery": "select unix_timestamp(now(6)) - unix_timestamp(ts) as lag_check from meta.heartbeat order by ts desc limit 1", "CacheMillis": 0, "ThrottleThreshold": 1.0, @@ -53,6 +56,8 @@ You will find the top-level configuration: These params apply in general to all MySQL clusters, unless specified differently (overridden) on a per-cluster basis. - `User`, `Password`: these can be specified as plaintext, or in a `${some_env_variable}` format, in which case `freno` will look up its environment for specified variable. (e.g. to match the above config, a `shell` script invoking `freno` can `export mysql_password_env_variable=flyingcircus`) +- `TlsCaCertPath`, `TlsClientCertPath`, `TlsClientKeyCertPath`. Are the file system paths of the PEM-encoded, TLS certificates needed to connect to MySQL using TLS. + They can also be specified as text or in `${some_env_variable}` format. - `MetricQuery`: - Note: returned value is expected to be `[0..)` (`0` or more), where lower values are "better" and higher values are "worse". - if not provided, `freno` will assume you're interested in replication lag, and will issue a `SHOW SLAVE STATUS` to extract `Seconds_behind_master` diff --git a/pkg/config/config.go b/pkg/config/config.go index 10e90360..76d94805 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -92,24 +92,29 @@ func (config *Configuration) Reload() error { // Some of the settinges have reasonable default values, and some other // (like database credentials) are strictly expected from user. type ConfigurationSettings struct { - ListenPort int - DataCenter string - Environment string - Domain string - ShareDomain string - RaftBind string - RaftDataDir string - DefaultRaftPort int // if a RaftNodes entry does not specify port, use this one - RaftNodes []string // Raft nodes to make initial connection with - BackendMySQLHost string - BackendMySQLPort int - BackendMySQLSchema string - BackendMySQLUser string - BackendMySQLPassword string - MemcacheServers []string // if given, freno will report to aggregated values to given memcache - MemcachePath string // use as prefix to metric path in memcache key, e.g. if `MemcachePath` is "myprefix" the key would be "myprefix/mysql/maincluster". Default: "freno" - EnableProfiling bool // enable pprof profiling http api - Stores StoresSettings + ListenPort int + DataCenter string + Environment string + Domain string + ShareDomain string + RaftBind string + RaftDataDir string + DefaultRaftPort int // if a RaftNodes entry does not specify port, use this one + RaftNodes []string // Raft nodes to make initial connection with + BackendMySQLHost string + BackendMySQLPort int + BackendMySQLSchema string + BackendMySQLUser string + BackendMySQLPassword string + BackendMySQLTlsCaCertPath string // optional, if not provided it won't setup any TLS connection + BackendMySQLTlsClientCertPath string + BackendMySQLTlsClientKeyPath string + BackendMySQLTlsSkipVerify bool // optional, set it to true to skip certificate verification. Not recommended in production + + MemcacheServers []string // if given, freno will report to aggregated values to given memcache + MemcachePath string // use as prefix to metric path in memcache key, e.g. if `MemcachePath` is "myprefix" the key would be "myprefix/mysql/maincluster". Default: "freno" + EnableProfiling bool // enable pprof profiling http api + Stores StoresSettings } func newConfigurationSettings() *ConfigurationSettings { @@ -145,6 +150,15 @@ func (settings *ConfigurationSettings) postReadAdjustments() error { if submatch := envVariableRegexp.FindStringSubmatch(settings.BackendMySQLPassword); len(submatch) > 1 { settings.BackendMySQLPassword = os.Getenv(submatch[1]) } + if submatch := envVariableRegexp.FindStringSubmatch(settings.BackendMySQLTlsCaCertPath); len(submatch) > 1 { + settings.BackendMySQLTlsCaCertPath = os.Getenv(submatch[1]) + } + if submatch := envVariableRegexp.FindStringSubmatch(settings.BackendMySQLTlsClientCertPath); len(submatch) > 1 { + settings.BackendMySQLTlsClientCertPath = os.Getenv(submatch[1]) + } + if submatch := envVariableRegexp.FindStringSubmatch(settings.BackendMySQLTlsClientKeyPath); len(submatch) > 1 { + settings.BackendMySQLTlsClientKeyPath = os.Getenv(submatch[1]) + } if settings.RaftDataDir == "" && settings.BackendMySQLHost == "" { return fmt.Errorf("Either RaftDataDir or BackendMySQLHost must be set") } diff --git a/pkg/config/mysql_config.go b/pkg/config/mysql_config.go index 9ccd79e0..40efd104 100644 --- a/pkg/config/mysql_config.go +++ b/pkg/config/mysql_config.go @@ -11,8 +11,12 @@ import ( const DefaultMySQLPort = 3306 type MySQLClusterConfigurationSettings struct { - User string // override MySQLConfigurationSettings's, or leave empty to inherit those settings - Password string // override MySQLConfigurationSettings's, or leave empty to inherit those settings + User string // override MySQLConfigurationSettings's, or leave empty to inherit those settings + Password string // override MySQLConfigurationSettings's, or leave empty to inherit those settings + TlsCaCertPath string // optional, if not provided it won't setup any TLS connection + TlsClientCertPath string + TlsClientKeyPath string + TlsSkipVerify bool // optional, set it to true to skip certificate verification. Not recommended in production. MetricQuery string // override MySQLConfigurationSettings's, or leave empty to inherit those settings CacheMillis int // override MySQLConfigurationSettings's, or leave empty to inherit those settings ThrottleThreshold float64 // override MySQLConfigurationSettings's, or leave empty to inherit those settings @@ -49,6 +53,10 @@ func (settings *MySQLClusterConfigurationSettings) postReadAdjustments() error { type MySQLConfigurationSettings struct { User string Password string + TlsCaCertPath string // optional, if not provided it won't setup any TLS connection + TlsClientCertPath string + TlsClientKeyPath string + TlsSkipVerify bool // optional, set it to true to skip certificate verification. Not recommended in production. MetricQuery string CacheMillis int // optional, if defined then probe result will be cached, and future probes may use cached value ThrottleThreshold float64 @@ -81,6 +89,15 @@ func (settings *MySQLConfigurationSettings) postReadAdjustments() error { if submatch := envVariableRegexp.FindStringSubmatch(settings.Password); len(submatch) > 1 { settings.Password = os.Getenv(submatch[1]) } + if submatch := envVariableRegexp.FindStringSubmatch(settings.TlsCaCertPath); len(submatch) > 1 { + settings.TlsCaCertPath = os.Getenv(submatch[1]) + } + if submatch := envVariableRegexp.FindStringSubmatch(settings.TlsClientCertPath); len(submatch) > 1 { + settings.TlsClientCertPath = os.Getenv(submatch[1]) + } + if submatch := envVariableRegexp.FindStringSubmatch(settings.TlsClientKeyPath); len(submatch) > 1 { + settings.TlsClientKeyPath = os.Getenv(submatch[1]) + } for _, clusterSettings := range settings.Clusters { if err := clusterSettings.postReadAdjustments(); err != nil { @@ -92,6 +109,15 @@ func (settings *MySQLConfigurationSettings) postReadAdjustments() error { if clusterSettings.Password == "" { clusterSettings.Password = settings.Password } + if clusterSettings.TlsCaCertPath == "" { + clusterSettings.TlsCaCertPath = settings.TlsCaCertPath + } + if clusterSettings.TlsClientCertPath == "" { + clusterSettings.TlsClientCertPath = settings.TlsClientCertPath + } + if clusterSettings.TlsClientKeyPath == "" { + clusterSettings.TlsClientKeyPath = settings.TlsClientKeyPath + } if clusterSettings.MetricQuery == "" { clusterSettings.MetricQuery = settings.MetricQuery } From ddf5d12e0eb451dec2baabdf578e04b82157da13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20Fern=C3=A1ndez?= Date: Wed, 3 Feb 2021 19:01:11 +0100 Subject: [PATCH 5/7] Implement TLS connection support --- pkg/group/mysql.go | 4 +++ pkg/mysql/probe.go | 4 +-- pkg/mysql/uri.go | 56 ++++++++++++++++++++++++++++++++++++++- pkg/throttle/throttler.go | 4 +++ 4 files changed, 65 insertions(+), 3 deletions(-) diff --git a/pkg/group/mysql.go b/pkg/group/mysql.go index 1f129d07..c372b436 100644 --- a/pkg/group/mysql.go +++ b/pkg/group/mysql.go @@ -83,6 +83,10 @@ func NewMySQLBackend(throttler *throttle.Throttler) (*MySQLBackend, error) { settings.BackendMySQLSchema, settings.BackendMySQLUser, settings.BackendMySQLPassword, + settings.BackendMySQLTlsCaCertPath, + settings.BackendMySQLTlsClientCertPath, + settings.BackendMySQLTlsClientKeyPath, + settings.BackendMySQLTlsSkipVerify, connectionTimeout) if err != nil { diff --git a/pkg/mysql/probe.go b/pkg/mysql/probe.go index 80aaacf3..823744cf 100644 --- a/pkg/mysql/probe.go +++ b/pkg/mysql/probe.go @@ -43,8 +43,8 @@ func NewProbes() *Probes { // NewProbe allocates memory for a new Probe value and returns its address, or an error in case tlsConfiguration parameters were // provided, but TLS configuration couldn't be registered. If that's the case, the address of the probe will be nil. -func NewProbe(key *InstanceKey, user, password, databaseName, metricQuery string, cacheMillis int, httpCheckPath string, httpCheckPort int) (*Probe, error) { - uri, err := MakeUri(key.Hostname, key.Port, user, password, databaseName, probeTimeout) +func NewProbe(key *InstanceKey, user, password, databaseName, tlsCaCertPath, tlsClientCertPath, tlsClientKeyPath string, tlsSkipVerify bool, metricQuery string, cacheMillis int, httpCheckPath string, httpCheckPort int) (*Probe, error) { + uri, err := MakeUri(key.Hostname, key.Port, user, password, databaseName, tlsCaCertPath, tlsClientCertPath, tlsClientKeyPath, tlsSkipVerify, probeTimeout) if err != nil { return nil, fmt.Errorf("cannot create probe. Cause: %w", err) } diff --git a/pkg/mysql/uri.go b/pkg/mysql/uri.go index 3b60e695..07ec55e9 100644 --- a/pkg/mysql/uri.go +++ b/pkg/mysql/uri.go @@ -1,18 +1,32 @@ package mysql import ( + "crypto/tls" + "crypto/x509" + "errors" "fmt" + "io/ioutil" "net" "time" + + "github.com/go-sql-driver/mysql" ) const timeout = 10 * time.Millisecond // MakeUri creates a new string representing the URI for the mysql driver to connect to, including timeout, charset and tls settings. // In case the URI cannot be created due to a wrong TLS configuration, an error is returned. -func MakeUri(hostname string, port int, databaseName, user, password string, timeout time.Duration) (uri string, err error) { +func MakeUri(hostname string, port int, databaseName, user, password, tlsCaCerPath, tlsClientCertPath, tlsClientKeyPath string, tlsSkipVerify bool, timeout time.Duration) (uri string, err error) { tlsKey := "false" + if tlsCaCerPath != "" || tlsClientCertPath != "" || tlsClientKeyPath != "" { + tlsKey = fmt.Sprintf("%s:%d", hostname, port) + err = registerTlsConfig(tlsKey, tlsCaCerPath, tlsClientCertPath, tlsClientKeyPath, tlsSkipVerify) + if err != nil { + return "", err + } + } + ip := net.ParseIP(hostname) if (ip != nil) && (ip.To4() == nil) { // Wrap IPv6 literals in square brackets @@ -22,3 +36,43 @@ func MakeUri(hostname string, port int, databaseName, user, password string, tim uri = fmt.Sprintf("%s:%s@tcp(%s:%d)/%s?interpolateParams=true&charset=utf8mb4,utf8,latin1&tls=%s&timeout=%dms", user, password, hostname, port, databaseName, tlsKey, timeout.Milliseconds()) return uri, err } + +// registerTlsConfig registers the certificates under a given key which is calculated based on the +// paths of the certificates, and returns that key, or an error if the certificates couldn't be registered. +func registerTlsConfig(key, tlsCaCertificatePath, tlsClientCertPath, tlsClientKeyPath string, tlsSkipVerify bool) (err error) { + var cert tls.Certificate + var rootCertPool *x509.CertPool + var pem []byte + + if tlsCaCertificatePath == "" { + rootCertPool, err = x509.SystemCertPool() + if err != nil { + return + } + } else { + rootCertPool = x509.NewCertPool() + pem, err = ioutil.ReadFile(tlsCaCertificatePath) + if err != nil { + return + } + if ok := rootCertPool.AppendCertsFromPEM(pem); !ok { + err = errors.New("cannot add ca certificate to cert pool") + return + } + } + if tlsClientCertPath != "" || tlsClientKeyPath != "" { + cert, err = tls.LoadX509KeyPair(tlsClientCertPath, tlsClientKeyPath) + if err != nil { + return + } + } + + config := tls.Config{ + Certificates: []tls.Certificate{cert}, + RootCAs: rootCertPool, + InsecureSkipVerify: tlsSkipVerify, + } + + err = mysql.RegisterTLSConfig(key, &config) + return +} diff --git a/pkg/throttle/throttler.go b/pkg/throttle/throttler.go index b5bb4d4c..347b1348 100644 --- a/pkg/throttle/throttler.go +++ b/pkg/throttle/throttler.go @@ -288,6 +288,10 @@ func (throttler *Throttler) refreshMySQLInventory() error { clusterSettings.User, clusterSettings.Password, defaultDatabaseName, + clusterSettings.TlsCaCertPath, + clusterSettings.TlsClientCertPath, + clusterSettings.TlsClientKeyPath, + clusterSettings.TlsSkipVerify, clusterSettings.MetricQuery, clusterSettings.CacheMillis, clusterSettings.HttpCheckPath, From 9ea132281074d78a17d5db77e0a21b72e0c8f426 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20Fern=C3=A1ndez?= Date: Wed, 3 Feb 2021 20:28:09 +0100 Subject: [PATCH 6/7] Update main workflow to go 1.15 --- .github/workflows/main.yml | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index ec8978d5..d93a730b 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -10,11 +10,10 @@ jobs: steps: - uses: actions/checkout@master - - name: Set up Go 1.12 - uses: actions/setup-go@v1 + - name: Set up Go + uses: actions/setup-go@v2 with: - version: 1.12 - id: go + go-version: 1.15 - name: Build run: script/cibuild From 041f938f9a0dbcdb9c5f3ce059c5ded936ee5df4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20Fern=C3=A1ndez?= Date: Mon, 8 Feb 2021 14:06:13 +0100 Subject: [PATCH 7/7] Promote mysql uri, to net/URL and redact log output --- pkg/group/mysql.go | 6 +++--- pkg/mysql/mysql_throttle_metric.go | 2 +- pkg/mysql/probe.go | 7 ++++--- pkg/mysql/{uri.go => url.go} | 16 ++++++++-------- 4 files changed, 16 insertions(+), 15 deletions(-) rename pkg/mysql/{uri.go => url.go} (72%) diff --git a/pkg/group/mysql.go b/pkg/group/mysql.go index c372b436..b7dc0313 100644 --- a/pkg/group/mysql.go +++ b/pkg/group/mysql.go @@ -77,7 +77,7 @@ func NewMySQLBackend(throttler *throttle.Throttler) (*MySQLBackend, error) { if settings.BackendMySQLHost == "" { return nil, nil } - uri, err := mysql.MakeUri( + url, err := mysql.NewURL( settings.BackendMySQLHost, settings.BackendMySQLPort, settings.BackendMySQLSchema, @@ -93,14 +93,14 @@ func NewMySQLBackend(throttler *throttle.Throttler) (*MySQLBackend, error) { return nil, err } - db, _, err := sqlutils.GetDB(uri) + db, _, err := sqlutils.GetDB(url.String()) if err != nil { return nil, err } db.SetMaxOpenConns(maxConnections) db.SetMaxIdleConns(maxConnections) - log.Debugf("created db at: %s", uri) + log.Debugf("created db at: %s", url.Redacted()) hostname, err := os.Hostname() if err != nil { return nil, err diff --git a/pkg/mysql/mysql_throttle_metric.go b/pkg/mysql/mysql_throttle_metric.go index d47dc448..47309f4e 100644 --- a/pkg/mysql/mysql_throttle_metric.go +++ b/pkg/mysql/mysql_throttle_metric.go @@ -88,7 +88,7 @@ func ReadThrottleMetric(probe *Probe, clusterName string) (mySQLThrottleMetric * }() }(mySQLThrottleMetric, started) - db, fromCache, err := sqlutils.GetDB(probe.Uri) + db, fromCache, err := sqlutils.GetDB(probe.Url.String()) if err != nil { mySQLThrottleMetric.Err = err return mySQLThrottleMetric diff --git a/pkg/mysql/probe.go b/pkg/mysql/probe.go index 823744cf..67966da6 100644 --- a/pkg/mysql/probe.go +++ b/pkg/mysql/probe.go @@ -7,6 +7,7 @@ package mysql import ( "fmt" + "net/url" "time" ) @@ -19,7 +20,7 @@ const ( // Probe is the minimal configuration required to connect to a MySQL server type Probe struct { Key InstanceKey - Uri string + Url *url.URL MetricQuery string CacheMillis int QueryInProgress int64 @@ -44,14 +45,14 @@ func NewProbes() *Probes { // NewProbe allocates memory for a new Probe value and returns its address, or an error in case tlsConfiguration parameters were // provided, but TLS configuration couldn't be registered. If that's the case, the address of the probe will be nil. func NewProbe(key *InstanceKey, user, password, databaseName, tlsCaCertPath, tlsClientCertPath, tlsClientKeyPath string, tlsSkipVerify bool, metricQuery string, cacheMillis int, httpCheckPath string, httpCheckPort int) (*Probe, error) { - uri, err := MakeUri(key.Hostname, key.Port, user, password, databaseName, tlsCaCertPath, tlsClientCertPath, tlsClientKeyPath, tlsSkipVerify, probeTimeout) + url, err := NewURL(key.Hostname, key.Port, user, password, databaseName, tlsCaCertPath, tlsClientCertPath, tlsClientKeyPath, tlsSkipVerify, probeTimeout) if err != nil { return nil, fmt.Errorf("cannot create probe. Cause: %w", err) } p := Probe{ Key: *key, - Uri: uri, + Url: url, MetricQuery: metricQuery, CacheMillis: cacheMillis, HttpCheckPath: httpCheckPath, diff --git a/pkg/mysql/uri.go b/pkg/mysql/url.go similarity index 72% rename from pkg/mysql/uri.go rename to pkg/mysql/url.go index 07ec55e9..d80b5414 100644 --- a/pkg/mysql/uri.go +++ b/pkg/mysql/url.go @@ -7,23 +7,23 @@ import ( "fmt" "io/ioutil" "net" + "net/url" "time" "github.com/go-sql-driver/mysql" ) -const timeout = 10 * time.Millisecond - -// MakeUri creates a new string representing the URI for the mysql driver to connect to, including timeout, charset and tls settings. -// In case the URI cannot be created due to a wrong TLS configuration, an error is returned. -func MakeUri(hostname string, port int, databaseName, user, password, tlsCaCerPath, tlsClientCertPath, tlsClientKeyPath string, tlsSkipVerify bool, timeout time.Duration) (uri string, err error) { +// NewURL creates a new string representing the URI for the mysql driver to connect to, including timeout, charset and tls settings. +// In case the URL cannot be created due to a wrong TLS configuration, an error is returned. +func NewURL(hostname string, port int, databaseName, user, password, tlsCaCerPath, tlsClientCertPath, tlsClientKeyPath string, tlsSkipVerify bool, timeout time.Duration) (*url.URL, error) { + var err error tlsKey := "false" if tlsCaCerPath != "" || tlsClientCertPath != "" || tlsClientKeyPath != "" { tlsKey = fmt.Sprintf("%s:%d", hostname, port) err = registerTlsConfig(tlsKey, tlsCaCerPath, tlsClientCertPath, tlsClientKeyPath, tlsSkipVerify) if err != nil { - return "", err + return nil, err } } @@ -33,8 +33,8 @@ func MakeUri(hostname string, port int, databaseName, user, password, tlsCaCerPa hostname = fmt.Sprintf("[%s]", hostname) } - uri = fmt.Sprintf("%s:%s@tcp(%s:%d)/%s?interpolateParams=true&charset=utf8mb4,utf8,latin1&tls=%s&timeout=%dms", user, password, hostname, port, databaseName, tlsKey, timeout.Milliseconds()) - return uri, err + s := fmt.Sprintf("%s:%s@tcp(%s:%d)/%s?interpolateParams=true&charset=utf8mb4,utf8,latin1&tls=%s&timeout=%dms", user, password, hostname, port, databaseName, tlsKey, timeout.Milliseconds()) + return url.Parse(s) } // registerTlsConfig registers the certificates under a given key which is calculated based on the