From 09aeb47bc226db572025c11fa26d0c56363b502e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Peter=20J=C3=BAno=C5=A1?= <petoju@gmail.com>
Date: Thu, 11 Mar 2021 20:13:47 +0100
Subject: [PATCH] Error handling + test for conn_params

---
 mysql/provider.go                | 44 +++++++++++++++++++++++++-------
 mysql/provider_test.go           | 16 ++++++++++--
 website/docs/index.html.markdown |  1 +
 3 files changed, 50 insertions(+), 11 deletions(-)

diff --git a/mysql/provider.go b/mysql/provider.go
index 054b81f4e..159ccbef6 100644
--- a/mysql/provider.go
+++ b/mysql/provider.go
@@ -23,6 +23,7 @@ import (
 const (
 	cleartextPasswords = "cleartext"
 	nativePasswords    = "native"
+	unknownVarErrCode  = 1193
 )
 
 type MySQLConfiguration struct {
@@ -152,13 +153,12 @@ func providerConfigure(d *schema.ResourceData) (interface{}, error) {
 		proto = "unix"
 	}
 
-	for k, v := range d.Get("conn_params").(map[string]interface{}) {
-		temp, ok := v.(string)
+	for k, vint := range d.Get("conn_params").(map[string]interface{}) {
+		v, ok := vint.(string)
 		if !ok {
-			fmt.Errorf("Cannot convert connection parameters to string")
-		} else {
-			conn_params[k] = temp
+			return nil, fmt.Errorf("Cannot convert connection parameters to string")
 		}
+		conn_params[k] = v
 	}
 
 	conf := mysql.Config{
@@ -196,11 +196,18 @@ func providerConfigure(d *schema.ResourceData) (interface{}, error) {
 	}
 
 	mysqlConf.Db = db
+	if err := afterConnect(d, db); err != nil {
+		return nil, fmt.Errorf("Failed running after connect command: %v", err)
+	}
+
+	return mysqlConf, nil
+}
 
+func afterConnect(d *schema.ResourceData, db *sql.DB) error {
 	// Set up env so that we won't create users randomly.
 	currentVersion, err := serverVersion(db)
 	if err != nil {
-		return nil, err
+		return fmt.Errorf("Failed getting server version: %v", err)
 	}
 
 	versionMinInclusive, _ := version.NewVersion("5.7.5")
@@ -210,11 +217,11 @@ func providerConfigure(d *schema.ResourceData) (interface{}, error) {
 		// CONCAT and setting works even if there is no value.
 		_, err := db.Exec(`SET SESSION sql_mode=CONCAT(@@sql_mode, ',NO_AUTO_CREATE_USER')`)
 		if err != nil {
-			return nil, err
+			return fmt.Errorf("Failed setting SQL mode: %v", err)
 		}
 	}
 
-	return mysqlConf, nil
+	return nil
 }
 
 var identQuoteReplacer = strings.NewReplacer("`", "``")
@@ -265,7 +272,7 @@ func serverVersionString(db *sql.DB) (string, error) {
 
 func connectToMySQL(conf *MySQLConfiguration) (*sql.DB, error) {
 	// This is fine - we'll connect serially, but we don't expect more than
-	// 1 or 2 connections at once.
+	// 1 or 2 connections starting at once.
 	connectionCacheMtx.Lock()
 	defer connectionCacheMtx.Unlock()
 
@@ -283,11 +290,18 @@ func connectToMySQL(conf *MySQLConfiguration) (*sql.DB, error) {
 	retryError := resource.Retry(conf.ConnectRetryTimeoutSec, func() *resource.RetryError {
 		db, err = sql.Open("mysql", dsn)
 		if err != nil {
+			if mysqlErrorNumber(err) == unknownVarErrCode {
+				return resource.NonRetryableError(err)
+			}
 			return resource.RetryableError(err)
 		}
 
 		err = db.Ping()
 		if err != nil {
+			if mysqlErrorNumber(err) == unknownVarErrCode {
+				return resource.NonRetryableError(err)
+			}
+
 			return resource.RetryableError(err)
 		}
 
@@ -302,3 +316,15 @@ func connectToMySQL(conf *MySQLConfiguration) (*sql.DB, error) {
 	db.SetMaxOpenConns(conf.MaxOpenConns)
 	return db, nil
 }
+
+// 0 == not mysql error or not error at all.
+func mysqlErrorNumber(err error) uint16 {
+	if err == nil {
+		return 0
+	}
+	me, ok := err.(*mysql.MySQLError)
+	if !ok {
+		return 0
+	}
+	return me.Number
+}
diff --git a/mysql/provider_test.go b/mysql/provider_test.go
index bf5163ac7..6ae7a85cb 100644
--- a/mysql/provider_test.go
+++ b/mysql/provider_test.go
@@ -55,10 +55,22 @@ func testAccPreCheck(t *testing.T) {
 		}
 	}
 
-	//raw := map[string]interface{}{}
-	//err := testAccProvider.Configure(ctx, terraform.NewResourceConfigRaw(raw))
 	err := testAccProvider.Configure(ctx, terraform.NewResourceConfigRaw(nil))
 	if err != nil {
 		t.Fatal(err)
 	}
 }
+
+func TestProviderConfigure(t *testing.T) {
+	ctx := context.Background()
+
+	raw := map[string]interface{}{
+		"conn_params": map[string]interface{}{
+			"sql_log_bin": 0,
+		},
+	}
+	err := testAccProvider.Configure(ctx, terraform.NewResourceConfigRaw(raw))
+	if err != nil {
+		t.Fatal(err)
+	}
+}
diff --git a/website/docs/index.html.markdown b/website/docs/index.html.markdown
index ffd9882c5..f43310b31 100644
--- a/website/docs/index.html.markdown
+++ b/website/docs/index.html.markdown
@@ -83,4 +83,5 @@ The following arguments are supported:
 * `tls` - (Optional) The TLS configuration. One of `false`, `true`, or `skip-verify`. Defaults to `false`. Can also be sourced from the `MYSQL_TLS_CONFIG` environment variable.
 * `max_conn_lifetime_sec` - (Optional) Sets the maximum amount of time a connection may be reused. If d <= 0, connections are reused forever.
 * `max_open_conns` - (Optional) Sets the maximum number of open connections to the database. If n <= 0, then there is no limit on the number of open connections.
+* `conn_params` - (Optional) Sets extra mysql connection parameters (ODBC parameters). Most useful for session variables such as `default_storage_engine`, `foreign_key_checks` or `sql_log_bin`.
 * `authentication_plugin` - (Optional) Sets the authentication plugin, it can be one of the following: `native` or `cleartext`. Defaults to `native`.