From 73372733dfe492fac6855a910e40bc0420673bbe Mon Sep 17 00:00:00 2001 From: s-v-a-n <149468449+s-v-a-n@users.noreply.github.com> Date: Fri, 17 Nov 2023 07:33:52 -0800 Subject: [PATCH] [receiver/mysql] expose tls config (#29269) **Description:** Using the mysqlreceiver, we were getting the following error as our MySQL server on AWS RDS requires secure transport for all connections by setting `require_secure_transport=ON` per https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/mysql-ssl-connections.html#mysql-ssl-connections.require-ssl **Example log message** `2023-10-31T10:53:30.239Z error scraperhelper/scrapercontroller.go:200 Error scraping metrics {"kind": "receiver", "name": "mysql", "data_type": "metrics", "error": "Error 3159 (HY000): Connections using insecure transport are prohibited while --require_secure_transport=ON.; ", "scraper": "mysql"}` --- .chloggen/main.yaml | 28 ++++++++++++ receiver/mysqlreceiver/README.md | 4 ++ receiver/mysqlreceiver/client.go | 19 +++++++- receiver/mysqlreceiver/config.go | 19 ++++++++ receiver/mysqlreceiver/config_test.go | 26 +++++++++++ receiver/mysqlreceiver/go.mod | 2 + receiver/mysqlreceiver/go.sum | 4 ++ receiver/mysqlreceiver/integration_test.go | 50 ++++++++++++++++++++- receiver/mysqlreceiver/scraper.go | 7 ++- receiver/mysqlreceiver/testdata/config.yaml | 8 ++++ 10 files changed, 162 insertions(+), 5 deletions(-) create mode 100755 .chloggen/main.yaml diff --git a/.chloggen/main.yaml b/.chloggen/main.yaml new file mode 100755 index 000000000000..360beb3f537b --- /dev/null +++ b/.chloggen/main.yaml @@ -0,0 +1,28 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: enhancement + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: mysqlreceiver + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: "expose tls in mysqlreceiver" + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [29269] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: "If `tls` is not set, the default is to disable TLS connections." + + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [] diff --git a/receiver/mysqlreceiver/README.md b/receiver/mysqlreceiver/README.md index 0a53cffaddac..5a2bc0163970 100644 --- a/receiver/mysqlreceiver/README.md +++ b/receiver/mysqlreceiver/README.md @@ -30,6 +30,10 @@ Collecting most metrics requires the ability to execute `SHOW GLOBAL STATUS`. The following settings are optional: - `endpoint`: (default = `localhost:3306`) +- `tls`: Defines the TLS configuration to use. If `tls` is not set, the default is to disable TLS connections. + - `insecure`: (default = `false`) Set this to `true` to disable TLS connections. + - `insecure_skip_verify`: (default = `false`) Set this to `true` to enable TLS but not verify the certificate. + - `server_name_override`: This sets the ServerName in the TLSConfig. - `username`: (default = `root`) - `password`: The password to the username. - `allow_native_passwords`: (default = `true`) diff --git a/receiver/mysqlreceiver/client.go b/receiver/mysqlreceiver/client.go index 165a8e9f257d..df3de22bb0b8 100644 --- a/receiver/mysqlreceiver/client.go +++ b/receiver/mysqlreceiver/client.go @@ -163,7 +163,20 @@ type ReplicaStatusStats struct { var _ client = (*mySQLClient)(nil) -func newMySQLClient(conf *Config) client { +func newMySQLClient(conf *Config) (client, error) { + tls, err := conf.TLS.LoadTLSConfig() + if err != nil { + return nil, err + } + tlsConfig := "" + if tls != nil { + err := mysql.RegisterTLSConfig("custom", tls) + if err != nil { + return nil, err + } + tlsConfig = "custom" + } + driverConf := mysql.Config{ User: conf.Username, Passwd: string(conf.Password), @@ -171,6 +184,8 @@ func newMySQLClient(conf *Config) client { Addr: conf.Endpoint, DBName: conf.Database, AllowNativePasswords: conf.AllowNativePasswords, + TLS: tls, + TLSConfig: tlsConfig, } connStr := driverConf.FormatDSN() @@ -179,7 +194,7 @@ func newMySQLClient(conf *Config) client { statementEventsDigestTextLimit: conf.StatementEvents.DigestTextLimit, statementEventsLimit: conf.StatementEvents.Limit, statementEventsTimeLimit: conf.StatementEvents.TimeLimit, - } + }, nil } func (c *mySQLClient) Connect() error { diff --git a/receiver/mysqlreceiver/config.go b/receiver/mysqlreceiver/config.go index f4cbf0f0807b..7b832a13a09f 100644 --- a/receiver/mysqlreceiver/config.go +++ b/receiver/mysqlreceiver/config.go @@ -8,6 +8,8 @@ import ( "go.opentelemetry.io/collector/config/confignet" "go.opentelemetry.io/collector/config/configopaque" + "go.opentelemetry.io/collector/config/configtls" + "go.opentelemetry.io/collector/confmap" "go.opentelemetry.io/collector/receiver/scraperhelper" "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/mysqlreceiver/internal/metadata" @@ -26,6 +28,7 @@ type Config struct { Database string `mapstructure:"database,omitempty"` AllowNativePasswords bool `mapstructure:"allow_native_passwords,omitempty"` confignet.NetAddr `mapstructure:",squash"` + TLS configtls.TLSClientSetting `mapstructure:"tls,omitempty"` MetricsBuilderConfig metadata.MetricsBuilderConfig `mapstructure:",squash"` StatementEvents StatementEventsConfig `mapstructure:"statement_events"` } @@ -35,3 +38,19 @@ type StatementEventsConfig struct { Limit int `mapstructure:"limit"` TimeLimit time.Duration `mapstructure:"time_limit"` } + +func (cfg *Config) Unmarshal(componentParser *confmap.Conf) error { + if componentParser == nil { + // Nothing to do if there is no config given. + return nil + } + + // Change the default to Insecure = true as we don't want to break + // existing deployments which does not use TLS by default. + if !componentParser.IsSet("tls") { + cfg.TLS = configtls.TLSClientSetting{} + cfg.TLS.Insecure = true + } + + return componentParser.Unmarshal(cfg, confmap.WithErrorUnused()) +} diff --git a/receiver/mysqlreceiver/config_test.go b/receiver/mysqlreceiver/config_test.go index 4f9ede06b47d..2b63e2b1cf5c 100644 --- a/receiver/mysqlreceiver/config_test.go +++ b/receiver/mysqlreceiver/config_test.go @@ -32,6 +32,32 @@ func TestLoadConfig(t *testing.T) { expected.Password = "${env:MYSQL_PASSWORD}" expected.Database = "otel" expected.CollectionInterval = 10 * time.Second + // This defaults to true when tls is omitted from the configmap. + expected.TLS.Insecure = true + + require.Equal(t, expected, cfg) +} + +func TestLoadConfigDefaultTLS(t *testing.T) { + cm, err := confmaptest.LoadConf(filepath.Join("testdata", "config.yaml")) + require.NoError(t, err) + + factory := NewFactory() + cfg := factory.CreateDefaultConfig() + + sub, err := cm.Sub(component.NewIDWithName(metadata.Type, "").String() + "/default_tls") + require.NoError(t, err) + require.NoError(t, component.UnmarshalConfig(sub, cfg)) + + expected := factory.CreateDefaultConfig().(*Config) + expected.Endpoint = "localhost:3306" + expected.Username = "otel" + expected.Password = "${env:MYSQL_PASSWORD}" + expected.Database = "otel" + expected.CollectionInterval = 10 * time.Second + // This defaults to false when tls is defined in the configmap. + expected.TLS.Insecure = false + expected.TLS.ServerName = "localhost" require.Equal(t, expected, cfg) } diff --git a/receiver/mysqlreceiver/go.mod b/receiver/mysqlreceiver/go.mod index 30986af2a9ca..2ead2c14ed17 100644 --- a/receiver/mysqlreceiver/go.mod +++ b/receiver/mysqlreceiver/go.mod @@ -13,6 +13,7 @@ require ( go.opentelemetry.io/collector/component v0.89.0 go.opentelemetry.io/collector/config/confignet v0.89.0 go.opentelemetry.io/collector/config/configopaque v0.89.0 + go.opentelemetry.io/collector/config/configtls v0.89.0 go.opentelemetry.io/collector/confmap v0.89.0 go.opentelemetry.io/collector/consumer v0.89.0 go.opentelemetry.io/collector/pdata v1.0.0-rcv0018 @@ -35,6 +36,7 @@ require ( github.com/docker/docker v24.0.7+incompatible // indirect github.com/docker/go-connections v0.4.0 // indirect github.com/docker/go-units v0.5.0 // indirect + github.com/fsnotify/fsnotify v1.7.0 // indirect github.com/go-ole/go-ole v1.2.6 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/golang/protobuf v1.5.3 // indirect diff --git a/receiver/mysqlreceiver/go.sum b/receiver/mysqlreceiver/go.sum index 3ccd73e75c38..12cb16d9e1b8 100644 --- a/receiver/mysqlreceiver/go.sum +++ b/receiver/mysqlreceiver/go.sum @@ -48,6 +48,8 @@ github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.m github.com/envoyproxy/go-control-plane v0.9.4/go.mod h1:6rpuAdCZL397s3pYoYcLgu1mIlRU8Am5FuJP05cCM98= github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= github.com/frankban/quicktest v1.11.3/go.mod h1:wRf/ReqHper53s+kmmSZizM8NamnL3IM0I9ntUbOk+k= +github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nosvA= +github.com/fsnotify/fsnotify v1.7.0/go.mod h1:40Bi/Hjc2AVfZrqy+aj+yEI+/bRxZnMJyTJwOpGvigM= github.com/go-kit/log v0.2.1 h1:MRVx0/zhvdseW+Gza6N9rVzU/IVzaeE1SFI4raAhmBU= github.com/go-logfmt/logfmt v0.5.1 h1:otpy5pqBCBZ1ng9RQ0dPu4PN7ba75Y/aA+UpowDyNVA= github.com/go-logr/logr v1.3.0 h1:2y3SDp0ZXuc6/cjLSZ+Q3ir+QB9T/iG5yYRXqsagWSY= @@ -208,6 +210,8 @@ go.opentelemetry.io/collector/config/configopaque v0.89.0 h1:Ad6yGcGBHs+J9SNjked go.opentelemetry.io/collector/config/configopaque v0.89.0/go.mod h1:TPCHaU+QXiEV+JXbgyr6mSErTI9chwQyasDVMdJr3eY= go.opentelemetry.io/collector/config/configtelemetry v0.89.0 h1:NtRknYDfMgP1r8mnByo6qQQK8IBw/lF9Qke5f7VhGZ0= go.opentelemetry.io/collector/config/configtelemetry v0.89.0/go.mod h1:+LAXM5WFMW/UbTlAuSs6L/W72WC+q8TBJt/6z39FPOU= +go.opentelemetry.io/collector/config/configtls v0.89.0 h1:XDeUaTU7LYwnEXz/CSdjbCStJa7n0YR1q0QpK0Vtw9w= +go.opentelemetry.io/collector/config/configtls v0.89.0/go.mod h1:NlE4elqXoyFfzQvYfzgH6uOU1zNVa+5tt6EIq52TJ9Y= go.opentelemetry.io/collector/confmap v0.89.0 h1:N5Vg1+FXEFBHHlGIPg4OSlM9uTHjCI7RlWWrKjtOzWQ= go.opentelemetry.io/collector/confmap v0.89.0/go.mod h1:D8FMPvuihtVxwXaz/qp5q9X2lq9l97QyjfsdZD1spmc= go.opentelemetry.io/collector/consumer v0.89.0 h1:MteKhkudX2L1ylbtdpSazO8SwyHSxl6fUEElc0rRLDQ= diff --git a/receiver/mysqlreceiver/integration_test.go b/receiver/mysqlreceiver/integration_test.go index dc58a2e31fd5..7f3fc4e0dfa2 100644 --- a/receiver/mysqlreceiver/integration_test.go +++ b/receiver/mysqlreceiver/integration_test.go @@ -22,7 +22,7 @@ import ( const mysqlPort = "3306" -func TestIntegration(t *testing.T) { +func TestIntegrationWithoutTLS(t *testing.T) { scraperinttest.NewIntegrationTest( NewFactory(), scraperinttest.WithContainerRequest( @@ -52,6 +52,54 @@ func TestIntegration(t *testing.T) { rCfg.Endpoint = net.JoinHostPort(ci.Host(t), ci.MappedPort(t, mysqlPort)) rCfg.Username = "otel" rCfg.Password = "otel" + // disable TLS connection + rCfg.TLS.Insecure = true + }), + scraperinttest.WithCompareOptions( + pmetrictest.IgnoreResourceAttributeValue("mysql.instance.endpoint"), + pmetrictest.IgnoreMetricValues(), + pmetrictest.IgnoreMetricDataPointsOrder(), + pmetrictest.IgnoreStartTimestamp(), + pmetrictest.IgnoreTimestamp(), + ), + ).Run(t) +} + +func TestIntegrationWithTLS(t *testing.T) { + scraperinttest.NewIntegrationTest( + NewFactory(), + scraperinttest.WithContainerRequest( + testcontainers.ContainerRequest{ + Image: "mysql:8.0.33", + // enable auto TLS certs AND require TLS connections only ! + Cmd: []string{"--auto_generate_certs=ON", "--require_secure_transport=ON"}, + ExposedPorts: []string{mysqlPort}, + WaitingFor: wait.ForListeningPort(mysqlPort). + WithStartupTimeout(2 * time.Minute), + Env: map[string]string{ + "MYSQL_ROOT_PASSWORD": "otel", + "MYSQL_DATABASE": "otel", + "MYSQL_USER": "otel", + "MYSQL_PASSWORD": "otel", + }, + Files: []testcontainers.ContainerFile{ + { + HostFilePath: filepath.Join("testdata", "integration", "init.sh"), + ContainerFilePath: "/docker-entrypoint-initdb.d/init.sh", + FileMode: 700, + }, + }, + }), + scraperinttest.WithCustomConfig( + func(t *testing.T, cfg component.Config, ci *scraperinttest.ContainerInfo) { + rCfg := cfg.(*Config) + rCfg.CollectionInterval = time.Second + rCfg.Endpoint = net.JoinHostPort(ci.Host(t), ci.MappedPort(t, mysqlPort)) + rCfg.Username = "otel" + rCfg.Password = "otel" + // mysql container is using self-signed certs + // InsecureSkipVerify will enable TLS but not verify the certificate. + rCfg.TLS.InsecureSkipVerify = true }), scraperinttest.WithCompareOptions( pmetrictest.IgnoreResourceAttributeValue("mysql.instance.endpoint"), diff --git a/receiver/mysqlreceiver/scraper.go b/receiver/mysqlreceiver/scraper.go index 9d37bd0197f9..0adb055030aa 100644 --- a/receiver/mysqlreceiver/scraper.go +++ b/receiver/mysqlreceiver/scraper.go @@ -46,9 +46,12 @@ func newMySQLScraper( // start starts the scraper by initializing the db client connection. func (m *mySQLScraper) start(_ context.Context, _ component.Host) error { - sqlclient := newMySQLClient(m.config) + sqlclient, err := newMySQLClient(m.config) + if err != nil { + return err + } - err := sqlclient.Connect() + err = sqlclient.Connect() if err != nil { return err } diff --git a/receiver/mysqlreceiver/testdata/config.yaml b/receiver/mysqlreceiver/testdata/config.yaml index eaa814151c56..86f249bcfc8d 100644 --- a/receiver/mysqlreceiver/testdata/config.yaml +++ b/receiver/mysqlreceiver/testdata/config.yaml @@ -4,3 +4,11 @@ mysql: password: ${env:MYSQL_PASSWORD} database: otel collection_interval: 10s +mysql/default_tls: + endpoint: localhost:3306 + username: otel + password: ${env:MYSQL_PASSWORD} + database: otel + collection_interval: 10s + tls: # specified, but use default values + server_name_override: localhost