diff --git a/postgresql/helpers.go b/postgresql/helpers.go index 9a83de4f..566fd0f9 100644 --- a/postgresql/helpers.go +++ b/postgresql/helpers.go @@ -266,6 +266,20 @@ func validatePrivileges(d *schema.ResourceData) error { return nil } +// getDataForRevoke returns a value for a REVOKE statement from the given resource data key. +// +// If the key has changes, the "old" value, which incorporates both previous Terraform state and any drift detected +// during the planning phase, will be returned. +// +// If the key does not have changes, the current value is returned. +func getDataForRevoke(d *schema.ResourceData, key string) interface{} { + if d.HasChange(key) { + oldValue, _ := d.GetChange(key) + return oldValue + } + return d.Get(key) +} + func pgArrayToSet(arr pq.ByteaArray) *schema.Set { s := make([]interface{}, len(arr)) for i, v := range arr { diff --git a/postgresql/resource_postgresql_grant.go b/postgresql/resource_postgresql_grant.go index 34fa205f..767034d1 100644 --- a/postgresql/resource_postgresql_grant.go +++ b/postgresql/resource_postgresql_grant.go @@ -1,6 +1,7 @@ package postgresql import ( + "context" "database/sql" "fmt" "log" @@ -36,13 +37,23 @@ var objectTypes = map[string]string{ 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: + Create: PGResourceFunc(resourcePostgreSQLGrantCreateOrUpdate), + Update: PGResourceFunc(resourcePostgreSQLGrantCreateOrUpdate), Read: PGResourceFunc(resourcePostgreSQLGrantRead), Delete: PGResourceFunc(resourcePostgreSQLGrantDelete), + CustomizeDiff: func(ctx context.Context, diff *schema.ResourceDiff, i interface{}) error { + // Object types that only support a single object always force recreation + objectType := strings.ToUpper(diff.Get("object_type").(string)) + if (diff.HasChange("permissions") || diff.HasChange("objects")) && + (objectType == "FOREIGN_DATA_WRAPPER" || objectType == "FOREIGN_SERVER") { + if err := diff.ForceNew("objects"); err != nil { + return err + } + } + return nil + }, + Schema: map[string]*schema.Schema{ "role": { Type: schema.TypeString, @@ -72,7 +83,7 @@ func resourcePostgreSQLGrant() *schema.Resource { "objects": { Type: schema.TypeSet, Optional: true, - ForceNew: true, + ForceNew: false, Elem: &schema.Schema{Type: schema.TypeString}, Set: schema.HashString, Description: "The specific objects to grant privileges on for this role (empty means all objects of the requested type)", @@ -80,7 +91,7 @@ func resourcePostgreSQLGrant() *schema.Resource { "columns": { Type: schema.TypeSet, Optional: true, - ForceNew: true, + ForceNew: false, Elem: &schema.Schema{Type: schema.TypeString}, Set: schema.HashString, Description: "The specific columns to grant privileges on for this role", @@ -88,7 +99,7 @@ func resourcePostgreSQLGrant() *schema.Resource { "privileges": { Type: schema.TypeSet, Required: true, - ForceNew: true, + ForceNew: false, Elem: &schema.Schema{Type: schema.TypeString}, Set: schema.HashString, Description: "The list of privileges to grant", @@ -128,7 +139,7 @@ func resourcePostgreSQLGrantRead(db *DBConnection, d *schema.ResourceData) error return readRolePrivileges(txn, d) } -func resourcePostgreSQLGrantCreate(db *DBConnection, d *schema.ResourceData) error { +func resourcePostgreSQLGrantCreateOrUpdate(db *DBConnection, d *schema.ResourceData) error { if err := validateFeatureSupport(db, d); err != nil { return fmt.Errorf("feature is not supported: %v", err) } @@ -606,9 +617,9 @@ func createRevokeQuery(d *schema.ResourceData) string { pq.QuoteIdentifier(d.Get("role").(string)), ) case "COLUMN": - objects := d.Get("objects").(*schema.Set) - columns := d.Get("columns").(*schema.Set) - privileges := d.Get("privileges").(*schema.Set) + objects := getDataForRevoke(d, "objects").(*schema.Set) + columns := getDataForRevoke(d, "columns").(*schema.Set) + privileges := getDataForRevoke(d, "privileges").(*schema.Set) if privileges.Len() == 0 || columns.Len() == 0 { // No privileges to revoke, so don't revoke anything query = "SELECT NULL" @@ -622,8 +633,8 @@ func createRevokeQuery(d *schema.ResourceData) string { ) } case "TABLE", "SEQUENCE", "FUNCTION", "PROCEDURE", "ROUTINE": - objects := d.Get("objects").(*schema.Set) - privileges := d.Get("privileges").(*schema.Set) + objects := getDataForRevoke(d, "objects").(*schema.Set) + privileges := getDataForRevoke(d, "privileges").(*schema.Set) if objects.Len() > 0 { if privileges.Len() > 0 { // Revoking specific privileges instead of all privileges @@ -668,6 +679,7 @@ func grantRolePrivileges(txn *sql.Tx, d *schema.ResourceData) error { } query := createGrantQuery(d, privileges) + log.Printf("[DEBUG] grantRolePrivileges: %s", query) _, err := txn.Exec(query) return err @@ -675,6 +687,7 @@ func grantRolePrivileges(txn *sql.Tx, d *schema.ResourceData) error { func revokeRolePrivileges(txn *sql.Tx, d *schema.ResourceData) error { query := createRevokeQuery(d) + log.Printf("[DEBUG] revokeRolePrivileges: %s", query) 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 e89ec312..7b6287c1 100644 --- a/postgresql/resource_postgresql_grant_test.go +++ b/postgresql/resource_postgresql_grant_test.go @@ -184,6 +184,7 @@ func TestCreateRevokeQuery(t *testing.T) { cases := []struct { resource *schema.ResourceData + diff map[string]interface{} expected string }{ { @@ -261,6 +262,20 @@ func TestCreateRevokeQuery(t *testing.T) { }), expected: fmt.Sprintf(`REVOKE UPDATE,INSERT ON TABLE %[1]s."o2",%[1]s."o1" FROM %s`, pq.QuoteIdentifier(databaseName), pq.QuoteIdentifier(roleName)), }, + // Haven't been able to figure out how to mock old/new state in a Terraform ResourceData instance + //{ + // resource: schema.TestResourceDataRaw(t, resourcePostgreSQLGrant().Schema, map[string]interface{}{ + // "object_type": "table", + // "objects": tableObjects, + // "schema": databaseName, + // "role": roleName, + // "privileges": []interface{}{"INSERT", "UPDATE"}, + // }), + // diff: map[string]interface{}{ + // "objects": []interface{}{"o1", "o3"}, + // }, + // expected: fmt.Sprintf(`REVOKE UPDATE,INSERT ON TABLE %[1]s."o2",%[1]s."o1" FROM %s`, pq.QuoteIdentifier(databaseName), pq.QuoteIdentifier(roleName)), + //}, { resource: schema.TestResourceDataRaw(t, resourcePostgreSQLGrant().Schema, map[string]interface{}{ "object_type": "column", @@ -291,7 +306,18 @@ func TestCreateRevokeQuery(t *testing.T) { } for _, c := range cases { - out := createRevokeQuery(c.resource) + d := c.resource + d.SetId("fakeID") + d = resourcePostgreSQLGrant().Data(d.State()) + // Attempt (failed) at mocking new/old instance state + //if len(c.diff) > 0 { + // for k, v := range c.diff { + // if err := d.Set(k, v); err != nil { + // t.Fatalf(err.Error()) + // } + // } + //} + out := createRevokeQuery(d) if out != c.expected { t.Fatalf("Error matching output and expected: %#v vs %#v", out, c.expected) }