From 1c4bb7825fed8f04559f2ddf5f121924e3f6c085 Mon Sep 17 00:00:00 2001 From: Pablo Azuaje Date: Wed, 2 Oct 2024 15:28:34 +0200 Subject: [PATCH 1/2] Fix cyrilgdn#321 replaces postgresql_grant all the time. Based on doctolib's fork. --- postgresql/resource_postgresql_grant.go | 93 ++++++++++++-------- postgresql/resource_postgresql_grant_test.go | 2 +- 2 files changed, 58 insertions(+), 37 deletions(-) diff --git a/postgresql/resource_postgresql_grant.go b/postgresql/resource_postgresql_grant.go index 43fddb47..2b59fa44 100644 --- a/postgresql/resource_postgresql_grant.go +++ b/postgresql/resource_postgresql_grant.go @@ -34,12 +34,12 @@ var objectTypes = map[string]string{ "schema": "n", } +type ResourceSchemeGetter func(string) interface{} + func resourcePostgreSQLGrant() *schema.Resource { return &schema.Resource{ Create: PGResourceFunc(resourcePostgreSQLGrantCreate), - // Since all of this resource's arguments force a recreation - // there's no need for an Update function - // Update: + Update: PGResourceFunc(resourcePostgreSQLGrantUpdate), Read: PGResourceFunc(resourcePostgreSQLGrantRead), Delete: PGResourceFunc(resourcePostgreSQLGrantDelete), @@ -86,9 +86,9 @@ func resourcePostgreSQLGrant() *schema.Resource { Description: "The specific columns to grant privileges on for this role", }, "privileges": { - Type: schema.TypeSet, - Required: true, - ForceNew: true, + Type: schema.TypeSet, + Required: true, + //ForceNew: true, Elem: &schema.Schema{Type: schema.TypeString}, Set: schema.HashString, Description: "The list of privileges to grant", @@ -129,6 +129,14 @@ func resourcePostgreSQLGrantRead(db *DBConnection, d *schema.ResourceData) error } func resourcePostgreSQLGrantCreate(db *DBConnection, d *schema.ResourceData) error { + return resourcePostgreSQLGrantCreateOrUpdate(db, d, false) +} + +func resourcePostgreSQLGrantUpdate(db *DBConnection, d *schema.ResourceData) error { + return resourcePostgreSQLGrantCreateOrUpdate(db, d, true) +} + +func resourcePostgreSQLGrantCreateOrUpdate(db *DBConnection, d *schema.ResourceData, usePrevious bool) error { if err := validateFeatureSupport(db, d); err != nil { return fmt.Errorf("feature is not supported: %v", err) } @@ -187,7 +195,7 @@ func resourcePostgreSQLGrantCreate(db *DBConnection, d *schema.ResourceData) err // Revoke all privileges before granting otherwise reducing privileges will not work. // We just have to revoke them in the same transaction so the role will not lost its // privileges between the revoke and grant statements. - if err := revokeRolePrivileges(txn, d); err != nil { + if err := revokeRolePrivileges(txn, d, usePrevious); err != nil { return err } if err := grantRolePrivileges(txn, d); err != nil { @@ -243,7 +251,7 @@ func resourcePostgreSQLGrantDelete(db *DBConnection, d *schema.ResourceData) err } if err := withRolesGranted(txn, owners, func() error { - return revokeRolePrivileges(txn, d) + return revokeRolePrivileges(txn, d, false) }); err != nil { return err } @@ -589,40 +597,40 @@ func createGrantQuery(d *schema.ResourceData, privileges []string) string { return query } -func createRevokeQuery(d *schema.ResourceData) string { +func createRevokeQuery(getter ResourceSchemeGetter) string { var query string - switch strings.ToUpper(d.Get("object_type").(string)) { + switch strings.ToUpper(getter("object_type").(string)) { case "DATABASE": query = fmt.Sprintf( "REVOKE ALL PRIVILEGES ON DATABASE %s FROM %s", - pq.QuoteIdentifier(d.Get("database").(string)), - pq.QuoteIdentifier(d.Get("role").(string)), + pq.QuoteIdentifier(getter("database").(string)), + pq.QuoteIdentifier(getter("role").(string)), ) case "SCHEMA": query = fmt.Sprintf( "REVOKE ALL PRIVILEGES ON SCHEMA %s FROM %s", - pq.QuoteIdentifier(d.Get("schema").(string)), - pq.QuoteIdentifier(d.Get("role").(string)), + pq.QuoteIdentifier(getter("schema").(string)), + pq.QuoteIdentifier(getter("role").(string)), ) case "FOREIGN_DATA_WRAPPER": - fdwName := d.Get("objects").(*schema.Set).List()[0] + fdwName := getter("objects").(*schema.Set).List()[0] query = fmt.Sprintf( "REVOKE ALL PRIVILEGES ON FOREIGN DATA WRAPPER %s FROM %s", pq.QuoteIdentifier(fdwName.(string)), - pq.QuoteIdentifier(d.Get("role").(string)), + pq.QuoteIdentifier(getter("role").(string)), ) case "FOREIGN_SERVER": - srvName := d.Get("objects").(*schema.Set).List()[0] + srvName := getter("objects").(*schema.Set).List()[0] query = fmt.Sprintf( "REVOKE ALL PRIVILEGES ON FOREIGN SERVER %s FROM %s", pq.QuoteIdentifier(srvName.(string)), - pq.QuoteIdentifier(d.Get("role").(string)), + pq.QuoteIdentifier(getter("role").(string)), ) case "COLUMN": - objects := d.Get("objects").(*schema.Set) - columns := d.Get("columns").(*schema.Set) - privileges := d.Get("privileges").(*schema.Set) + objects := getter("objects").(*schema.Set) + columns := getter("columns").(*schema.Set) + privileges := getter("privileges").(*schema.Set) if privileges.Len() == 0 || columns.Len() == 0 { // No privileges to revoke, so don't revoke anything query = "SELECT NULL" @@ -631,13 +639,13 @@ func createRevokeQuery(d *schema.ResourceData) string { "REVOKE %s (%s) ON TABLE %s FROM %s", setToPgIdentSimpleList(privileges), setToPgIdentListWithoutSchema(columns), - setToPgIdentList(d.Get("schema").(string), objects), - pq.QuoteIdentifier(d.Get("role").(string)), + setToPgIdentList(getter("schema").(string), objects), + pq.QuoteIdentifier(getter("role").(string)), ) } case "TABLE", "SEQUENCE", "FUNCTION", "PROCEDURE", "ROUTINE": - objects := d.Get("objects").(*schema.Set) - privileges := d.Get("privileges").(*schema.Set) + objects := getter("objects").(*schema.Set) + privileges := getter("privileges").(*schema.Set) if objects.Len() > 0 { if privileges.Len() > 0 { // Revoking specific privileges instead of all privileges @@ -645,24 +653,24 @@ func createRevokeQuery(d *schema.ResourceData) string { query = fmt.Sprintf( "REVOKE %s ON %s %s FROM %s", setToPgIdentSimpleList(privileges), - strings.ToUpper(d.Get("object_type").(string)), - setToPgIdentList(d.Get("schema").(string), objects), - pq.QuoteIdentifier(d.Get("role").(string)), + strings.ToUpper(getter("object_type").(string)), + setToPgIdentList(getter("schema").(string), objects), + pq.QuoteIdentifier(getter("role").(string)), ) } else { query = fmt.Sprintf( "REVOKE ALL PRIVILEGES ON %s %s FROM %s", - strings.ToUpper(d.Get("object_type").(string)), - setToPgIdentList(d.Get("schema").(string), objects), - pq.QuoteIdentifier(d.Get("role").(string)), + strings.ToUpper(getter("object_type").(string)), + setToPgIdentList(getter("schema").(string), objects), + pq.QuoteIdentifier(getter("role").(string)), ) } } else { query = fmt.Sprintf( "REVOKE ALL PRIVILEGES ON ALL %sS IN SCHEMA %s FROM %s", - strings.ToUpper(d.Get("object_type").(string)), - pq.QuoteIdentifier(d.Get("schema").(string)), - pq.QuoteIdentifier(d.Get("role").(string)), + strings.ToUpper(getter("object_type").(string)), + pq.QuoteIdentifier(getter("schema").(string)), + pq.QuoteIdentifier(getter("role").(string)), ) } } @@ -687,8 +695,21 @@ func grantRolePrivileges(txn *sql.Tx, d *schema.ResourceData) error { return err } -func revokeRolePrivileges(txn *sql.Tx, d *schema.ResourceData) error { - query := createRevokeQuery(d) +func revokeRolePrivileges(txn *sql.Tx, d *schema.ResourceData, usePrevious bool) error { + getter := d.Get + + if usePrevious { + getter = func(name string) interface{} { + if d.HasChange(name) { + old, _ := d.GetChange(name) + return old + } + + return d.Get(name) + } + } + + query := createRevokeQuery(getter) if len(query) == 0 { // Query is empty, don't run anything return nil diff --git a/postgresql/resource_postgresql_grant_test.go b/postgresql/resource_postgresql_grant_test.go index a81c95bc..611986a0 100644 --- a/postgresql/resource_postgresql_grant_test.go +++ b/postgresql/resource_postgresql_grant_test.go @@ -293,7 +293,7 @@ func TestCreateRevokeQuery(t *testing.T) { } for _, c := range cases { - out := createRevokeQuery(c.resource) + out := createRevokeQuery(c.resource.Get) if out != c.expected { t.Fatalf("Error matching output and expected: %#v vs %#v", out, c.expected) } From c4ca8cb6a8442f675b6076b36f7cccb03ad95d02 Mon Sep 17 00:00:00 2001 From: Pablo Azuaje Date: Wed, 2 Oct 2024 15:39:45 +0200 Subject: [PATCH 2/2] Remove ForceNew comment --- postgresql/resource_postgresql_grant.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/postgresql/resource_postgresql_grant.go b/postgresql/resource_postgresql_grant.go index 2b59fa44..608570f6 100644 --- a/postgresql/resource_postgresql_grant.go +++ b/postgresql/resource_postgresql_grant.go @@ -86,9 +86,8 @@ func resourcePostgreSQLGrant() *schema.Resource { Description: "The specific columns to grant privileges on for this role", }, "privileges": { - Type: schema.TypeSet, - Required: true, - //ForceNew: true, + Type: schema.TypeSet, + Required: true, Elem: &schema.Schema{Type: schema.TypeString}, Set: schema.HashString, Description: "The list of privileges to grant",