Skip to content

Commit

Permalink
feat: better support for resource conflicts (#114)
Browse files Browse the repository at this point in the history
  • Loading branch information
DerekTBrown authored Mar 7, 2024
1 parent 0c455c4 commit 69227ff
Show file tree
Hide file tree
Showing 3 changed files with 178 additions and 14 deletions.
23 changes: 18 additions & 5 deletions mysql/resource_grant.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,16 @@ import (
"context"
"database/sql"
"fmt"
"github.com/hashicorp/go-version"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"log"
"reflect"
"regexp"
"sort"
"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"
)
Expand All @@ -25,6 +26,8 @@ var (
kTable ObjectT = "TABLE"
)

var grantCreateMutex = NewKeyedMutex()

type MySQLGrant interface {
GetId() string
SQLGrantStatement() string
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)
Expand Down
131 changes: 123 additions & 8 deletions mysql/resource_grant_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -199,7 +200,7 @@ func TestAccGrantComplex(t *testing.T) {
// Create table first
Config: testAccGrantConfigNoGrant(dbName),
Check: resource.ComposeTestCheckFunc(
prepareTable(dbName),
prepareTable(dbName, "tbl"),
),
},
{
Expand Down Expand Up @@ -289,7 +290,7 @@ func TestAccGrantComplexMySQL8(t *testing.T) {
// Create table first
Config: testAccGrantConfigNoGrant(dbName),
Check: resource.ComposeTestCheckFunc(
prepareTable(dbName),
prepareTable(dbName, "tbl"),
),
},
{
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -878,7 +879,7 @@ func TestAccGrantOnProcedure(t *testing.T) {
// Create table first
Config: testAccGrantConfigNoGrant(dbName),
Check: resource.ComposeTestCheckFunc(
prepareTable(dbName),
prepareTable(dbName, "tbl"),
),
},
{
Expand Down Expand Up @@ -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"),
},
},
})
}
38 changes: 37 additions & 1 deletion mysql/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))))
}
Expand Down

0 comments on commit 69227ff

Please sign in to comment.