From 09a9f303a29ff13b462e926d8b4c6ff7313e4d1d Mon Sep 17 00:00:00 2001 From: talbx Date: Sat, 26 Aug 2023 11:14:56 +0200 Subject: [PATCH 01/11] feat: improved privilege matching in case of ALL --- postgresql/resource_postgresql_grant.go | 37 +++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/postgresql/resource_postgresql_grant.go b/postgresql/resource_postgresql_grant.go index e6cc5315..f119aae1 100644 --- a/postgresql/resource_postgresql_grant.go +++ b/postgresql/resource_postgresql_grant.go @@ -108,7 +108,9 @@ func resourcePostgreSQLGrantRead(db *DBConnection, d *schema.ResourceData) error if err := validateFeatureSupport(db, d); err != nil { return fmt.Errorf("feature is not supported: %v", err) } - + if true { + //return fmt.Errorf("hello world! %v", "you") + } exists, err := checkRoleDBSchemaExists(db.client, d) if err != nil { return err @@ -415,6 +417,9 @@ func readRolePrivileges(txn *sql.Tx, d *schema.ResourceData) error { role := d.Get("role").(string) objectType := d.Get("object_type").(string) objects := d.Get("objects").(*schema.Set) + for _, entry := range objects.List() { + log.Printf("this is a entry: %v", entry) + } roleOID, err := getRoleOID(txn, role) if err != nil { @@ -502,8 +507,8 @@ GROUP BY pg_class.relname } privilegesSet := pgArrayToSet(privileges) - - if !privilegesSet.Equal(d.Get("privileges").(*schema.Set)) { + privilegesEqual := strictOrImplicitEqual(privilegesSet, d.Get("privileges").(*schema.Set), objectType) + if !privilegesEqual { // If any object doesn't have the same privileges as saved in the state, // we return its privileges to force an update. log.Printf( @@ -518,6 +523,32 @@ GROUP BY pg_class.relname return nil } +func strictOrImplicitEqual(granted *schema.Set, wanted *schema.Set, objectType string) bool { + if granted.Equal(wanted) { + return true + } + + if !wanted.Contains("ALL") { + return false + } + + log.Printf("The wanted privilege is 'ALL'. therefore, we will check if the current privileges are ALL, implicitely") + implicits := make([]string, 0) + for _, p := range allowedPrivileges[objectType] { + if p != "ALL" { + implicits = append(implicits, p) + } + } + + s := make([]interface{}, len(implicits)) + for i, privilege := range implicits { + s[i] = privilege + } + wantedSet := schema.NewSet(schema.HashString, s) + log.Printf("wanted: %v, present: %v", wantedSet.List(), granted.List()) + return granted.Equal(wantedSet) +} + func createGrantQuery(d *schema.ResourceData, privileges []string) string { var query string From edacad1ec7e628dd764addfdaa49c5e505adb510 Mon Sep 17 00:00:00 2001 From: talbx Date: Sun, 27 Aug 2023 15:53:23 +0200 Subject: [PATCH 02/11] feat: added implicit privilege check for all object types except column aswell as for default privilege resource --- postgresql/helpers.go | 28 +++++++++ .../resource_postgresql_default_privileges.go | 8 ++- postgresql/resource_postgresql_grant.go | 57 ++++++++----------- 3 files changed, 59 insertions(+), 34 deletions(-) diff --git a/postgresql/helpers.go b/postgresql/helpers.go index a11103d2..89f319c1 100644 --- a/postgresql/helpers.go +++ b/postgresql/helpers.go @@ -267,6 +267,34 @@ func validatePrivileges(d *schema.ResourceData) error { return nil } +func arePrivilegesEqual(granted *schema.Set, wanted *schema.Set, d *schema.ResourceData) bool { + objectType := d.Get("object_type").(string) + + if granted.Equal(wanted) { + return true + } + + if !wanted.Contains("ALL") { + return false + } + + // implicit check: e.g. for object_type schema -> ALL == ["CREATE", "USAGE"] + log.Printf("The wanted privilege is 'ALL'. therefore, we will check if the current privileges are ALL implicitely") + implicits := make([]string, 0) + for _, p := range allowedPrivileges[objectType] { + if p != "ALL" { + implicits = append(implicits, p) + } + } + + s := make([]interface{}, len(implicits)) + for i, privilege := range implicits { + s[i] = privilege + } + wantedSet := schema.NewSet(schema.HashString, s) + return granted.Equal(wantedSet) +} + func pgArrayToSet(arr pq.ByteaArray) *schema.Set { s := make([]interface{}, len(arr)) for i, v := range arr { diff --git a/postgresql/resource_postgresql_default_privileges.go b/postgresql/resource_postgresql_default_privileges.go index 351ec803..149ccfd3 100644 --- a/postgresql/resource_postgresql_default_privileges.go +++ b/postgresql/resource_postgresql_default_privileges.go @@ -268,8 +268,12 @@ func readRoleDefaultPrivileges(txn *sql.Tx, d *schema.ResourceData) error { } privilegesSet := pgArrayToSet(privileges) - d.Set("privileges", privilegesSet) - d.SetId(generateDefaultPrivilegesID(d)) + privilegesEqual := arePrivilegesEqual(privilegesSet, d.Get("privileges").(*schema.Set), d) + + if !privilegesEqual { + d.Set("privileges", privilegesSet) + d.SetId(generateDefaultPrivilegesID(d)) + } return nil } diff --git a/postgresql/resource_postgresql_grant.go b/postgresql/resource_postgresql_grant.go index f119aae1..f48e8de8 100644 --- a/postgresql/resource_postgresql_grant.go +++ b/postgresql/resource_postgresql_grant.go @@ -271,8 +271,12 @@ WHERE grantee = $2 if err := txn.QueryRow(query, dbName, roleOID).Scan(&privileges); err != nil { return fmt.Errorf("could not read privileges for database %s: %w", dbName, err) } - - d.Set("privileges", pgArrayToSet(privileges)) + granted := pgArrayToSet(privileges) + wanted := d.Get("privileges").(*schema.Set) + equal := arePrivilegesEqual(granted, wanted, d) + if !equal { + return d.Set("privileges", wanted) + } return nil } @@ -291,7 +295,12 @@ WHERE grantee = $2 return fmt.Errorf("could not read privileges for schema %s: %w", dbName, err) } - d.Set("privileges", pgArrayToSet(privileges)) + granted := pgArrayToSet(privileges) + wanted := d.Get("privileges").(*schema.Set) + equal := arePrivilegesEqual(granted, wanted, d) + if !equal { + return d.Set("privileges", wanted) + } return nil } @@ -311,7 +320,12 @@ WHERE grantee = $2 return fmt.Errorf("could not read privileges for foreign data wrapper %s: %w", fdwName, err) } - d.Set("privileges", pgArrayToSet(privileges)) + granted := pgArrayToSet(privileges) + wanted := d.Get("privileges").(*schema.Set) + equal := arePrivilegesEqual(granted, wanted, d) + if !equal { + return d.Set("privileges", wanted) + } return nil } @@ -331,7 +345,12 @@ WHERE grantee = $2 return fmt.Errorf("could not read privileges for foreign server %s: %w", srvName, err) } - d.Set("privileges", pgArrayToSet(privileges)) + granted := pgArrayToSet(privileges) + wanted := d.Get("privileges").(*schema.Set) + equal := arePrivilegesEqual(granted, wanted, d) + if !equal { + return d.Set("privileges", wanted) + } return nil } @@ -507,7 +526,7 @@ GROUP BY pg_class.relname } privilegesSet := pgArrayToSet(privileges) - privilegesEqual := strictOrImplicitEqual(privilegesSet, d.Get("privileges").(*schema.Set), objectType) + privilegesEqual := arePrivilegesEqual(privilegesSet, d.Get("privileges").(*schema.Set), d) if !privilegesEqual { // If any object doesn't have the same privileges as saved in the state, // we return its privileges to force an update. @@ -523,32 +542,6 @@ GROUP BY pg_class.relname return nil } -func strictOrImplicitEqual(granted *schema.Set, wanted *schema.Set, objectType string) bool { - if granted.Equal(wanted) { - return true - } - - if !wanted.Contains("ALL") { - return false - } - - log.Printf("The wanted privilege is 'ALL'. therefore, we will check if the current privileges are ALL, implicitely") - implicits := make([]string, 0) - for _, p := range allowedPrivileges[objectType] { - if p != "ALL" { - implicits = append(implicits, p) - } - } - - s := make([]interface{}, len(implicits)) - for i, privilege := range implicits { - s[i] = privilege - } - wantedSet := schema.NewSet(schema.HashString, s) - log.Printf("wanted: %v, present: %v", wantedSet.List(), granted.List()) - return granted.Equal(wantedSet) -} - func createGrantQuery(d *schema.ResourceData, privileges []string) string { var query string From e3b92a0fffc8ddf47761e4754cf9fe8012b20a0d Mon Sep 17 00:00:00 2001 From: talbx Date: Sun, 27 Aug 2023 16:29:19 +0200 Subject: [PATCH 03/11] test: provided unit test for arePriviligesEqual function --- postgresql/helpers_test.go | 72 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/postgresql/helpers_test.go b/postgresql/helpers_test.go index 95632d50..643bf584 100644 --- a/postgresql/helpers_test.go +++ b/postgresql/helpers_test.go @@ -1,6 +1,7 @@ package postgresql import ( + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "testing" "github.com/stretchr/testify/assert" @@ -17,3 +18,74 @@ func TestFindStringSubmatchMap(t *testing.T) { }, ) } + +func TestArePrivilegesEqual(t *testing.T) { + + type PrivilegesTestObject struct { + d *schema.ResourceData + granted *schema.Set + wanted *schema.Set + assertion bool + } + + tt := []PrivilegesTestObject{ + { + buildResourceData("database", t), + buildPrivilegesSet("CONNECT", "CREATE", "TEMPORARY"), + all(), + true, + }, + { + buildResourceData("database", t), + buildPrivilegesSet("CREATE", "USAGE"), + buildPrivilegesSet("USAGE"), + false, + }, + { + buildResourceData("table", t), + buildPrivilegesSet("SELECT", "INSERT", "UPDATE", "DELETE", "TRUNCATE", "REFERENCES", "TRIGGER"), + all(), + true, + }, + { + buildResourceData("table", t), + buildPrivilegesSet("SELECT"), + buildPrivilegesSet("SELECT, INSERT"), + false, + }, + { + buildResourceData("schema", t), + buildPrivilegesSet("CREATE", "USAGE"), + all(), + true, + }, + { + buildResourceData("schema", t), + buildPrivilegesSet("CREATE"), + all(), + false, + }, + } + + for _, configuration := range tt { + equal := arePrivilegesEqual(configuration.granted, configuration.wanted, configuration.d) + assert.Equal(t, configuration.assertion, equal) + } +} + +func buildPrivilegesSet(grants ...interface{}) *schema.Set { + return schema.NewSet(schema.HashString, grants) +} + +func all() *schema.Set { + return buildPrivilegesSet("ALL") +} + +func buildResourceData(objectType string, t *testing.T) *schema.ResourceData { + var testSchema = map[string]*schema.Schema{ + "object_type": {Type: schema.TypeString}, + } + m := make(map[string]interface{}) + m["object_type"] = objectType + return schema.TestResourceDataRaw(t, testSchema, m) +} From 8dfcf905d1bfb5746001a6e98fca82492058bb1b Mon Sep 17 00:00:00 2001 From: talbx Date: Mon, 26 Feb 2024 10:28:09 +0100 Subject: [PATCH 04/11] fix: apply review findings - removed redundant wanted param of privilegesEqual fn, removed former logging bits, adapted tests --- postgresql/helpers.go | 3 ++- postgresql/helpers_test.go | 25 +++++++++++-------- .../resource_postgresql_default_privileges.go | 2 +- postgresql/resource_postgresql_grant.go | 17 ++++--------- 4 files changed, 22 insertions(+), 25 deletions(-) diff --git a/postgresql/helpers.go b/postgresql/helpers.go index e911f191..952394ee 100644 --- a/postgresql/helpers.go +++ b/postgresql/helpers.go @@ -267,8 +267,9 @@ func validatePrivileges(d *schema.ResourceData) error { return nil } -func arePrivilegesEqual(granted *schema.Set, wanted *schema.Set, d *schema.ResourceData) bool { +func resourcePrivilegesEqual(granted *schema.Set, d *schema.ResourceData) bool { objectType := d.Get("object_type").(string) + wanted := d.Get("privileges").(*schema.Set) if granted.Equal(wanted) { return true diff --git a/postgresql/helpers_test.go b/postgresql/helpers_test.go index 1ce7d10a..11538618 100644 --- a/postgresql/helpers_test.go +++ b/postgresql/helpers_test.go @@ -60,7 +60,7 @@ func TestArePrivilegesEqual(t *testing.T) { { buildResourceData("database", t), buildPrivilegesSet("CONNECT", "CREATE", "TEMPORARY"), - all(), + buildPrivilegesSet("ALL"), true, }, { @@ -72,7 +72,7 @@ func TestArePrivilegesEqual(t *testing.T) { { buildResourceData("table", t), buildPrivilegesSet("SELECT", "INSERT", "UPDATE", "DELETE", "TRUNCATE", "REFERENCES", "TRIGGER"), - all(), + buildPrivilegesSet("ALL"), true, }, { @@ -84,19 +84,21 @@ func TestArePrivilegesEqual(t *testing.T) { { buildResourceData("schema", t), buildPrivilegesSet("CREATE", "USAGE"), - all(), + buildPrivilegesSet("ALL"), true, }, { buildResourceData("schema", t), buildPrivilegesSet("CREATE"), - all(), + buildPrivilegesSet("ALL"), false, }, } for _, configuration := range tt { - equal := arePrivilegesEqual(configuration.granted, configuration.wanted, configuration.d) + err := configuration.d.Set("privileges", configuration.wanted) + assert.NoError(t, err) + equal := resourcePrivilegesEqual(configuration.granted, configuration.d) assert.Equal(t, configuration.assertion, equal) } } @@ -105,16 +107,17 @@ func buildPrivilegesSet(grants ...interface{}) *schema.Set { return schema.NewSet(schema.HashString, grants) } -func all() *schema.Set { - return buildPrivilegesSet("ALL") -} - func buildResourceData(objectType string, t *testing.T) *schema.ResourceData { var testSchema = map[string]*schema.Schema{ "object_type": {Type: schema.TypeString}, + "privileges": { + Type: schema.TypeSet, + Elem: &schema.Schema{Type: schema.TypeString}, + Set: schema.HashString, + }, } - m := make(map[string]interface{}) + + m := make(map[string]any) m["object_type"] = objectType return schema.TestResourceDataRaw(t, testSchema, m) } - diff --git a/postgresql/resource_postgresql_default_privileges.go b/postgresql/resource_postgresql_default_privileges.go index 149ccfd3..c091bdf0 100644 --- a/postgresql/resource_postgresql_default_privileges.go +++ b/postgresql/resource_postgresql_default_privileges.go @@ -268,7 +268,7 @@ func readRoleDefaultPrivileges(txn *sql.Tx, d *schema.ResourceData) error { } privilegesSet := pgArrayToSet(privileges) - privilegesEqual := arePrivilegesEqual(privilegesSet, d.Get("privileges").(*schema.Set), d) + privilegesEqual := resourcePrivilegesEqual(privilegesSet, d) if !privilegesEqual { d.Set("privileges", privilegesSet) diff --git a/postgresql/resource_postgresql_grant.go b/postgresql/resource_postgresql_grant.go index 1e1d6844..529d197d 100644 --- a/postgresql/resource_postgresql_grant.go +++ b/postgresql/resource_postgresql_grant.go @@ -108,9 +108,6 @@ func resourcePostgreSQLGrantRead(db *DBConnection, d *schema.ResourceData) error if err := validateFeatureSupport(db, d); err != nil { return fmt.Errorf("feature is not supported: %v", err) } - if true { - //return fmt.Errorf("hello world! %v", "you") - } exists, err := checkRoleDBSchemaExists(db.client, d) if err != nil { return err @@ -273,7 +270,7 @@ WHERE grantee = $2 } granted := pgArrayToSet(privileges) wanted := d.Get("privileges").(*schema.Set) - equal := arePrivilegesEqual(granted, wanted, d) + equal := resourcePrivilegesEqual(granted, d) if !equal { return d.Set("privileges", wanted) } @@ -297,7 +294,7 @@ WHERE grantee = $2 granted := pgArrayToSet(privileges) wanted := d.Get("privileges").(*schema.Set) - equal := arePrivilegesEqual(granted, wanted, d) + equal := resourcePrivilegesEqual(granted, d) if !equal { return d.Set("privileges", wanted) } @@ -322,7 +319,7 @@ WHERE grantee = $2 granted := pgArrayToSet(privileges) wanted := d.Get("privileges").(*schema.Set) - equal := arePrivilegesEqual(granted, wanted, d) + equal := resourcePrivilegesEqual(granted, d) if !equal { return d.Set("privileges", wanted) } @@ -347,7 +344,7 @@ WHERE grantee = $2 granted := pgArrayToSet(privileges) wanted := d.Get("privileges").(*schema.Set) - equal := arePrivilegesEqual(granted, wanted, d) + equal := resourcePrivilegesEqual(granted, d) if !equal { return d.Set("privileges", wanted) } @@ -436,9 +433,6 @@ func readRolePrivileges(txn *sql.Tx, d *schema.ResourceData) error { role := d.Get("role").(string) objectType := d.Get("object_type").(string) objects := d.Get("objects").(*schema.Set) - for _, entry := range objects.List() { - log.Printf("this is a entry: %v", entry) - } roleOID, err := getRoleOID(txn, role) if err != nil { @@ -526,8 +520,7 @@ GROUP BY pg_class.relname } privilegesSet := pgArrayToSet(privileges) - privilegesEqual := arePrivilegesEqual(privilegesSet, d.Get("privileges").(*schema.Set), d) - if !privilegesEqual { + if !resourcePrivilegesEqual(privilegesSet, d) { // If any object doesn't have the same privileges as saved in the state, // we return its privileges to force an update. log.Printf( From db54bfcdf27549a299b2bf23a7e2f58e747d75b8 Mon Sep 17 00:00:00 2001 From: talbx Date: Mon, 26 Feb 2024 10:33:38 +0100 Subject: [PATCH 05/11] refactor: inlined all equal checks --- postgresql/resource_postgresql_grant.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/postgresql/resource_postgresql_grant.go b/postgresql/resource_postgresql_grant.go index 529d197d..082df75d 100644 --- a/postgresql/resource_postgresql_grant.go +++ b/postgresql/resource_postgresql_grant.go @@ -270,8 +270,7 @@ WHERE grantee = $2 } granted := pgArrayToSet(privileges) wanted := d.Get("privileges").(*schema.Set) - equal := resourcePrivilegesEqual(granted, d) - if !equal { + if !resourcePrivilegesEqual(granted, d) { return d.Set("privileges", wanted) } return nil @@ -294,8 +293,7 @@ WHERE grantee = $2 granted := pgArrayToSet(privileges) wanted := d.Get("privileges").(*schema.Set) - equal := resourcePrivilegesEqual(granted, d) - if !equal { + if !resourcePrivilegesEqual(granted, d) { return d.Set("privileges", wanted) } return nil @@ -319,8 +317,7 @@ WHERE grantee = $2 granted := pgArrayToSet(privileges) wanted := d.Get("privileges").(*schema.Set) - equal := resourcePrivilegesEqual(granted, d) - if !equal { + if !resourcePrivilegesEqual(granted, d) { return d.Set("privileges", wanted) } return nil @@ -344,8 +341,7 @@ WHERE grantee = $2 granted := pgArrayToSet(privileges) wanted := d.Get("privileges").(*schema.Set) - equal := resourcePrivilegesEqual(granted, d) - if !equal { + if !resourcePrivilegesEqual(granted, d) { return d.Set("privileges", wanted) } return nil From 54fc593f610136ed3d41b95dbe9f826de0a49bf6 Mon Sep 17 00:00:00 2001 From: talbx Date: Tue, 5 Mar 2024 21:42:40 +0100 Subject: [PATCH 06/11] fix: use the granted priviliges set from tf instead of db, added acceptance test --- postgresql/resource_postgresql_grant.go | 12 +-- postgresql/resource_postgresql_grant_test.go | 93 ++++++++++++++++++++ 2 files changed, 97 insertions(+), 8 deletions(-) diff --git a/postgresql/resource_postgresql_grant.go b/postgresql/resource_postgresql_grant.go index 082df75d..ba22dbb1 100644 --- a/postgresql/resource_postgresql_grant.go +++ b/postgresql/resource_postgresql_grant.go @@ -269,9 +269,8 @@ WHERE grantee = $2 return fmt.Errorf("could not read privileges for database %s: %w", dbName, err) } granted := pgArrayToSet(privileges) - wanted := d.Get("privileges").(*schema.Set) if !resourcePrivilegesEqual(granted, d) { - return d.Set("privileges", wanted) + return d.Set("privileges", granted) } return nil } @@ -292,9 +291,8 @@ WHERE grantee = $2 } granted := pgArrayToSet(privileges) - wanted := d.Get("privileges").(*schema.Set) if !resourcePrivilegesEqual(granted, d) { - return d.Set("privileges", wanted) + return d.Set("privileges", granted) } return nil } @@ -316,9 +314,8 @@ WHERE grantee = $2 } granted := pgArrayToSet(privileges) - wanted := d.Get("privileges").(*schema.Set) if !resourcePrivilegesEqual(granted, d) { - return d.Set("privileges", wanted) + return d.Set("privileges", granted) } return nil } @@ -340,9 +337,8 @@ WHERE grantee = $2 } granted := pgArrayToSet(privileges) - wanted := d.Get("privileges").(*schema.Set) if !resourcePrivilegesEqual(granted, d) { - return d.Set("privileges", wanted) + return d.Set("privileges", granted) } return nil } diff --git a/postgresql/resource_postgresql_grant_test.go b/postgresql/resource_postgresql_grant_test.go index a81c95bc..09c97860 100644 --- a/postgresql/resource_postgresql_grant_test.go +++ b/postgresql/resource_postgresql_grant_test.go @@ -1139,6 +1139,83 @@ resource "postgresql_grant" "test" { }) } +func TestAccPostgresqlImplicitGrants(t *testing.T) { + skipIfNotAcc(t) + + dbSuffix, teardown := setupTestDatabase(t, true, true) + defer teardown() + + testTables := []string{"test_schema.test_table"} + createTestTables(t, dbSuffix, testTables, "") + + dbName, roleName := getTestDBNames(dbSuffix) + + // create a TF config with placeholder for privileges + // it will be filled in each step. + var testGrant = fmt.Sprintf(` + resource "postgresql_grant" "test" { + database = "%s" + role = "%s" + schema = "test_schema" + object_type = "table" + objects = ["test_table"] + privileges = %%s + } + `, dbName, roleName) + + var testCheckFunc = func(grants ...string) resource.TestCheckFunc { + return func(*terraform.State) error { + return testCheckTablesPrivileges(t, dbName, roleName, []string{testTables[0]}, grants) + } + } + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testCheckCompatibleVersion(t, featurePrivileges) + }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: fmt.Sprintf(testGrant, `["ALL"]`), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "postgresql_grant.test", "id", fmt.Sprintf("%s_%s_test_schema_table_test_table", roleName, dbName), + ), + resource.TestCheckResourceAttr("postgresql_grant.test", "objects.#", "1"), + resource.TestCheckResourceAttr("postgresql_grant.test", "objects.0", "test_table"), + testCheckFunc("SELECT", "INSERT", "UPDATE", "DELETE"), + ), + }, + { + Config: fmt.Sprintf(testGrant, `["SELECT"]`), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("postgresql_grant.test", "objects.#", "1"), + resource.TestCheckResourceAttr("postgresql_grant.test", "objects.0", "test_table"), + testCheckFunc("SELECT"), + ), + }, + { + // Empty list means that privileges will be applied on all tables. + Config: fmt.Sprintf(testGrant, `["SELECT", "INSERT", "UPDATE", "DELETE"]`), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("postgresql_grant.test", "objects.#", "1"), + resource.TestCheckResourceAttr("postgresql_grant.test", "objects.0", "test_table"), + testCheckFunc("SELECT", "INSERT", "UPDATE", "DELETE"), + ), + }, + { + Config: fmt.Sprintf(testGrant, `[]`), + Destroy: true, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("postgresql_grant.test", "objects.#", "1"), + resource.TestCheckResourceAttr("postgresql_grant.test", "objects.0", "test_table"), + testCheckFunc(""), + ), + }, + }, + }) +} + func TestAccPostgresqlGrantSchema(t *testing.T) { // create a TF config with placeholder for privileges // it will be filled in each step. @@ -1416,6 +1493,22 @@ func testCheckDatabasesPrivileges(t *testing.T, canCreate bool) func(*terraform. } } +func testCheckTablePrivileges(t *testing.T, canCreate bool, tableName string, grantee string) func(*terraform.State) error { + return func(*terraform.State) error { + db := connectAsTestRole(t, "test_grant_role", "test_grant_db") + defer db.Close() + + result, err := db.Exec(fmt.Sprintf("SELECT grantee, privilege_type FROM information_schema.role_table_grants WHERE table_name=%s AND grantee = %s", tableName, grantee)) + t.Log("res", result) + if err != nil { + panic(err) + } + t.Log("res", result) + + return nil + } +} + func testCheckFunctionExecutable(t *testing.T, role, function string) func(*terraform.State) error { return func(*terraform.State) error { db := connectAsTestRole(t, role, "postgres") From c64e95693257b459677886a60370d1a20555852d Mon Sep 17 00:00:00 2001 From: talbx Date: Tue, 5 Mar 2024 21:55:35 +0100 Subject: [PATCH 07/11] cleanup: fixed function name typos & removed useless testing helper fn --- postgresql/resource_postgresql_grant.go | 8 ++++---- postgresql/resource_postgresql_grant_test.go | 16 ---------------- 2 files changed, 4 insertions(+), 20 deletions(-) diff --git a/postgresql/resource_postgresql_grant.go b/postgresql/resource_postgresql_grant.go index ba22dbb1..f3699e80 100644 --- a/postgresql/resource_postgresql_grant.go +++ b/postgresql/resource_postgresql_grant.go @@ -254,7 +254,7 @@ func resourcePostgreSQLGrantDelete(db *DBConnection, d *schema.ResourceData) err return nil } -func readDatabaseRolePriviges(txn *sql.Tx, d *schema.ResourceData, roleOID uint32) error { +func readDatabaseRolePrivileges(txn *sql.Tx, d *schema.ResourceData, roleOID uint32) error { dbName := d.Get("database").(string) query := ` SELECT array_agg(privilege_type) @@ -275,7 +275,7 @@ WHERE grantee = $2 return nil } -func readSchemaRolePriviges(txn *sql.Tx, d *schema.ResourceData, roleOID uint32) error { +func readSchemaRolePrivileges(txn *sql.Tx, d *schema.ResourceData, roleOID uint32) error { dbName := d.Get("schema").(string) query := ` SELECT array_agg(privilege_type) @@ -436,10 +436,10 @@ func readRolePrivileges(txn *sql.Tx, d *schema.ResourceData) error { switch objectType { case "database": - return readDatabaseRolePriviges(txn, d, roleOID) + return readDatabaseRolePrivileges(txn, d, roleOID) case "schema": - return readSchemaRolePriviges(txn, d, roleOID) + return readSchemaRolePrivileges(txn, d, roleOID) case "foreign_data_wrapper": return readForeignDataWrapperRolePrivileges(txn, d, roleOID) diff --git a/postgresql/resource_postgresql_grant_test.go b/postgresql/resource_postgresql_grant_test.go index 09c97860..95fa8ec8 100644 --- a/postgresql/resource_postgresql_grant_test.go +++ b/postgresql/resource_postgresql_grant_test.go @@ -1493,22 +1493,6 @@ func testCheckDatabasesPrivileges(t *testing.T, canCreate bool) func(*terraform. } } -func testCheckTablePrivileges(t *testing.T, canCreate bool, tableName string, grantee string) func(*terraform.State) error { - return func(*terraform.State) error { - db := connectAsTestRole(t, "test_grant_role", "test_grant_db") - defer db.Close() - - result, err := db.Exec(fmt.Sprintf("SELECT grantee, privilege_type FROM information_schema.role_table_grants WHERE table_name=%s AND grantee = %s", tableName, grantee)) - t.Log("res", result) - if err != nil { - panic(err) - } - t.Log("res", result) - - return nil - } -} - func testCheckFunctionExecutable(t *testing.T, role, function string) func(*terraform.State) error { return func(*terraform.State) error { db := connectAsTestRole(t, role, "postgres") From a091370dafa34780d8eb009df959f4bf35712e8a Mon Sep 17 00:00:00 2001 From: talbx Date: Tue, 5 Mar 2024 21:59:52 +0100 Subject: [PATCH 08/11] provided a more reasonable name for testCheck function in implicit grants acceptance test --- postgresql/resource_postgresql_grant_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/postgresql/resource_postgresql_grant_test.go b/postgresql/resource_postgresql_grant_test.go index 95fa8ec8..1bc2b5aa 100644 --- a/postgresql/resource_postgresql_grant_test.go +++ b/postgresql/resource_postgresql_grant_test.go @@ -1163,7 +1163,7 @@ func TestAccPostgresqlImplicitGrants(t *testing.T) { } `, dbName, roleName) - var testCheckFunc = func(grants ...string) resource.TestCheckFunc { + var testCheckTableGrants = func(grants ...string) resource.TestCheckFunc { return func(*terraform.State) error { return testCheckTablesPrivileges(t, dbName, roleName, []string{testTables[0]}, grants) } @@ -1183,7 +1183,7 @@ func TestAccPostgresqlImplicitGrants(t *testing.T) { ), resource.TestCheckResourceAttr("postgresql_grant.test", "objects.#", "1"), resource.TestCheckResourceAttr("postgresql_grant.test", "objects.0", "test_table"), - testCheckFunc("SELECT", "INSERT", "UPDATE", "DELETE"), + testCheckTableGrants("SELECT", "INSERT", "UPDATE", "DELETE"), ), }, { @@ -1191,7 +1191,7 @@ func TestAccPostgresqlImplicitGrants(t *testing.T) { Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("postgresql_grant.test", "objects.#", "1"), resource.TestCheckResourceAttr("postgresql_grant.test", "objects.0", "test_table"), - testCheckFunc("SELECT"), + testCheckTableGrants("SELECT"), ), }, { @@ -1200,7 +1200,7 @@ func TestAccPostgresqlImplicitGrants(t *testing.T) { Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("postgresql_grant.test", "objects.#", "1"), resource.TestCheckResourceAttr("postgresql_grant.test", "objects.0", "test_table"), - testCheckFunc("SELECT", "INSERT", "UPDATE", "DELETE"), + testCheckTableGrants("SELECT", "INSERT", "UPDATE", "DELETE"), ), }, { @@ -1209,7 +1209,7 @@ func TestAccPostgresqlImplicitGrants(t *testing.T) { Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("postgresql_grant.test", "objects.#", "1"), resource.TestCheckResourceAttr("postgresql_grant.test", "objects.0", "test_table"), - testCheckFunc(""), + testCheckTableGrants(""), ), }, }, From 0393157115fd8e37e3eea65dfbe28e598a211104 Mon Sep 17 00:00:00 2001 From: Tom Albrecht Date: Wed, 23 Oct 2024 21:04:14 +0200 Subject: [PATCH 09/11] simplified privilege set creation Co-authored-by: Cyril Gaudin --- postgresql/helpers.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/postgresql/helpers.go b/postgresql/helpers.go index 3e9e8d2a..98464dfd 100644 --- a/postgresql/helpers.go +++ b/postgresql/helpers.go @@ -295,18 +295,13 @@ func resourcePrivilegesEqual(granted *schema.Set, d *schema.ResourceData) bool { // implicit check: e.g. for object_type schema -> ALL == ["CREATE", "USAGE"] log.Printf("The wanted privilege is 'ALL'. therefore, we will check if the current privileges are ALL implicitely") - implicits := make([]string, 0) + implicits :=[]interface{}{} for _, p := range allowedPrivileges[objectType] { if p != "ALL" { implicits = append(implicits, p) } } - - s := make([]interface{}, len(implicits)) - for i, privilege := range implicits { - s[i] = privilege - } - wantedSet := schema.NewSet(schema.HashString, s) + wantedSet := schema.NewSet(schema.HashString, implicits) return granted.Equal(wantedSet) } From c35441d539a2dc3b7e08bfa9ae5c0573cd53ec3f Mon Sep 17 00:00:00 2001 From: talbx Date: Wed, 23 Oct 2024 21:08:11 +0200 Subject: [PATCH 10/11] fix: always set id, no matter if privileges equal or not --- postgresql/resource_postgresql_default_privileges.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/postgresql/resource_postgresql_default_privileges.go b/postgresql/resource_postgresql_default_privileges.go index 1f68f856..124b3511 100644 --- a/postgresql/resource_postgresql_default_privileges.go +++ b/postgresql/resource_postgresql_default_privileges.go @@ -272,8 +272,8 @@ func readRoleDefaultPrivileges(txn *sql.Tx, d *schema.ResourceData) error { if !privilegesEqual { d.Set("privileges", privilegesSet) - d.SetId(generateDefaultPrivilegesID(d)) } + d.SetId(generateDefaultPrivilegesID(d)) return nil } From 4f651398e353e4f310e627c6131f9ed1966d0096 Mon Sep 17 00:00:00 2001 From: Cyril Gaudin Date: Thu, 24 Oct 2024 09:54:26 +0200 Subject: [PATCH 11/11] fmt --- postgresql/helpers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/postgresql/helpers.go b/postgresql/helpers.go index 98464dfd..2eb93e88 100644 --- a/postgresql/helpers.go +++ b/postgresql/helpers.go @@ -295,7 +295,7 @@ func resourcePrivilegesEqual(granted *schema.Set, d *schema.ResourceData) bool { // implicit check: e.g. for object_type schema -> ALL == ["CREATE", "USAGE"] log.Printf("The wanted privilege is 'ALL'. therefore, we will check if the current privileges are ALL implicitely") - implicits :=[]interface{}{} + implicits := []interface{}{} for _, p := range allowedPrivileges[objectType] { if p != "ALL" { implicits = append(implicits, p)