diff --git a/mysql/resource_grant.go b/mysql/resource_grant.go index 03826ec93..5a7e500d4 100644 --- a/mysql/resource_grant.go +++ b/mysql/resource_grant.go @@ -4,8 +4,6 @@ import ( "context" "database/sql" "fmt" - "github.com/hashicorp/go-version" - "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "log" "reflect" "regexp" @@ -13,6 +11,9 @@ import ( "strings" "unicode" + "github.com/hashicorp/go-version" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/go-sql-driver/mysql" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) @@ -25,6 +26,8 @@ var ( kTable ObjectT = "TABLE" ) +var grantCreateMutex = NewKeyedMutex() + type MySQLGrant interface { GetId() string SQLGrantStatement() string @@ -127,7 +130,7 @@ type TablePrivilegeGrant struct { } func (t *TablePrivilegeGrant) GetId() string { - return fmt.Sprintf("%s:%s", t.UserOrRole.IDString(), t.GetDatabase()) + return fmt.Sprintf("%s:%s:%s", t.UserOrRole.IDString(), t.GetDatabase(), t.GetTable()) } func (t *TablePrivilegeGrant) GetUserOrRole() UserOrRole { @@ -200,7 +203,7 @@ type ProcedurePrivilegeGrant struct { } func (t *ProcedurePrivilegeGrant) GetId() string { - return fmt.Sprintf("%s:%s", t.UserOrRole.IDString(), t.GetDatabase()) + return fmt.Sprintf("%s:%s:%s", t.UserOrRole.IDString(), t.GetDatabase(), t.GetCallableName()) } func (t *ProcedurePrivilegeGrant) GetUserOrRole() UserOrRole { @@ -493,13 +496,18 @@ func CreateGrant(ctx context.Context, d *schema.ResourceData, meta interface{}) return diag.Errorf("role grants are not supported by this version of MySQL") } + // Acquire a lock for the user + // This is necessary so that the conflicting grant check is correct with respect to other grants being created + grantCreateMutex.Lock(grant.GetUserOrRole().IDString()) + defer grantCreateMutex.Unlock(grant.GetUserOrRole().IDString()) + // Check to see if there are existing roles that might be clobbered by this grant conflictingGrant, err := getMatchingGrant(ctx, db, grant) if err != nil { return diag.Errorf("failed showing grants: %v", err) } if conflictingGrant != nil { - return diag.Errorf("user/role %s already has unmanaged grant %v - import it first", grant.GetUserOrRole(), conflictingGrant) + return diag.Errorf("user/role %s already has grant %v - ", grant.GetUserOrRole(), conflictingGrant) } stmtSQL := grant.SQLGrantStatement() @@ -612,11 +620,16 @@ func DeleteGrant(ctx context.Context, d *schema.ResourceData, meta interface{}) return diag.FromErr(err) } + // Parse the grant from ResourceData grant, diagErr := parseResourceFromData(d) if err != nil { return diagErr } + // Acquire a lock for the user + grantCreateMutex.Lock(grant.GetUserOrRole().IDString()) + defer grantCreateMutex.Unlock(grant.GetUserOrRole().IDString()) + sqlStatement := grant.SQLRevokeStatement() log.Printf("[DEBUG] SQL: %s", sqlStatement) _, err = db.ExecContext(ctx, sqlStatement) diff --git a/mysql/resource_grant_test.go b/mysql/resource_grant_test.go index 87dc27cbd..470172608 100644 --- a/mysql/resource_grant_test.go +++ b/mysql/resource_grant_test.go @@ -3,14 +3,15 @@ package mysql import ( "context" "fmt" - _ "github.com/go-sql-driver/mysql" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" - "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" "log" "math/rand" "regexp" "strings" "testing" + + _ "github.com/go-sql-driver/mysql" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" ) func TestAccGrant(t *testing.T) { @@ -199,7 +200,7 @@ func TestAccGrantComplex(t *testing.T) { // Create table first Config: testAccGrantConfigNoGrant(dbName), Check: resource.ComposeTestCheckFunc( - prepareTable(dbName), + prepareTable(dbName, "tbl"), ), }, { @@ -289,7 +290,7 @@ func TestAccGrantComplexMySQL8(t *testing.T) { // Create table first Config: testAccGrantConfigNoGrant(dbName), Check: resource.ComposeTestCheckFunc( - prepareTable(dbName), + prepareTable(dbName, "tbl"), ), }, { @@ -381,14 +382,14 @@ func TestAccGrant_complexRoleGrants(t *testing.T) { }) } -func prepareTable(dbname string) resource.TestCheckFunc { +func prepareTable(dbname string, tableName string) resource.TestCheckFunc { return func(s *terraform.State) error { ctx := context.Background() db, err := connectToMySQL(ctx, testAccProvider.Meta().(*MySQLConfiguration)) if err != nil { return err } - if _, err := db.Exec(fmt.Sprintf("CREATE TABLE `%s`.`tbl`(c1 INT, c2 INT, c3 INT,c4 INT,c5 INT);", dbname)); err != nil { + if _, err := db.Exec(fmt.Sprintf("CREATE TABLE `%s`.`%s`(c1 INT, c2 INT, c3 INT,c4 INT,c5 INT);", dbname, tableName)); err != nil { return fmt.Errorf("error reading grant: %s", err) } return nil @@ -878,7 +879,7 @@ func TestAccGrantOnProcedure(t *testing.T) { // Create table first Config: testAccGrantConfigNoGrant(dbName), Check: resource.ComposeTestCheckFunc( - prepareTable(dbName), + prepareTable(dbName, "tbl"), ), }, { @@ -1038,3 +1039,117 @@ func revokeUserPrivs(dbname string, privs string) resource.TestCheckFunc { return nil } } + +func TestAllowDuplicateUsersDifferentTables(t *testing.T) { + dbName := fmt.Sprintf("tf-test-%d", rand.Intn(100)) + + duplicateUserConfig := fmt.Sprintf(` + resource "mysql_database" "test" { + name = "%s" + } + + resource "mysql_user" "test" { + user = "jdoe-%s" + host = "example.com" + } + + resource "mysql_grant" "grant1" { + user = "${mysql_user.test.user}" + host = "${mysql_user.test.host}" + database = "${mysql_database.test.name}" + table = "table1" + privileges = ["UPDATE", "SELECT"] + } + + resource "mysql_grant" "grant2" { + user = "${mysql_user.test.user}" + host = "${mysql_user.test.host}" + database = "${mysql_database.test.name}" + table = "table2" + privileges = ["UPDATE", "SELECT"] + } + `, dbName, dbName) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t); testAccPreCheckSkipRds(t) }, + Providers: testAccProviders, + CheckDestroy: testAccGrantCheckDestroy, + Steps: []resource.TestStep{ + { + // Create table first + Config: testAccGrantConfigNoGrant(dbName), + Check: resource.ComposeTestCheckFunc( + prepareTable(dbName, "table1"), + prepareTable(dbName, "table2"), + ), + }, + { + Config: duplicateUserConfig, + Check: resource.ComposeTestCheckFunc( + testAccPrivilege("mysql_grant.grant1", "SELECT", true, false), + resource.TestCheckResourceAttr("mysql_grant.grant1", "table", "table1"), + testAccPrivilege("mysql_grant.grant2", "SELECT", true, false), + resource.TestCheckResourceAttr("mysql_grant.grant2", "table", "table2"), + ), + }, + { + RefreshState: true, + Check: resource.ComposeTestCheckFunc( + testAccPrivilege("mysql_grant.grant1", "SELECT", true, false), + resource.TestCheckResourceAttr("mysql_grant.grant1", "table", "table1"), + testAccPrivilege("mysql_grant.grant2", "SELECT", true, false), + resource.TestCheckResourceAttr("mysql_grant.grant2", "table", "table2"), + ), + }, + }, + }) +} + +func TestDisallowDuplicateUsersSameTable(t *testing.T) { + dbName := fmt.Sprintf("tf-test-%d", rand.Intn(100)) + + duplicateUserConfig := fmt.Sprintf(` + resource "mysql_database" "test" { + name = "%s" + } + + resource "mysql_user" "test" { + user = "jdoe-%s" + host = "example.com" + } + + resource "mysql_grant" "grant1" { + user = "${mysql_user.test.user}" + host = "${mysql_user.test.host}" + database = "${mysql_database.test.name}" + table = "table1" + privileges = ["UPDATE", "SELECT"] + } + + resource "mysql_grant" "grant2" { + user = "${mysql_user.test.user}" + host = "${mysql_user.test.host}" + database = "${mysql_database.test.name}" + table = "table1" + privileges = ["UPDATE", "SELECT"] + } + `, dbName, dbName) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t); testAccPreCheckSkipRds(t) }, + Providers: testAccProviders, + CheckDestroy: testAccGrantCheckDestroy, + Steps: []resource.TestStep{ + { + Config: testAccGrantConfigNoGrant(dbName), + Check: resource.ComposeTestCheckFunc( + prepareTable(dbName, "table1"), + ), + }, + { + Config: duplicateUserConfig, + ExpectError: regexp.MustCompile("already has"), + }, + }, + }) +} diff --git a/mysql/utils.go b/mysql/utils.go index a15f0a5f9..f8eef0f31 100644 --- a/mysql/utils.go +++ b/mysql/utils.go @@ -5,10 +5,46 @@ import ( "crypto/sha256" "database/sql" "fmt" - "github.com/hashicorp/go-version" "log" + "sync" + + "github.com/hashicorp/go-version" ) +type KeyedMutex struct { + mu sync.Mutex // Protects access to the internal map + locks map[string]*sync.Mutex +} + +func NewKeyedMutex() *KeyedMutex { + return &KeyedMutex{ + locks: make(map[string]*sync.Mutex), + } +} + +func (km *KeyedMutex) Lock(key string) { + km.mu.Lock() + lock, exists := km.locks[key] + if !exists { + lock = &sync.Mutex{} + km.locks[key] = lock + } + km.mu.Unlock() + + lock.Lock() +} + +func (km *KeyedMutex) Unlock(key string) { + km.mu.Lock() + lock, exists := km.locks[key] + if !exists { + panic("unlock of unlocked mutex") + } + km.mu.Unlock() + + lock.Unlock() +} + func hashSum(contents interface{}) string { return fmt.Sprintf("%x", sha256.Sum256([]byte(contents.(string)))) }