Fix grant on database and write an integration test.

Author: cyrilgdn

postgresql/helpers.go

@@ -1,12 +1,11 @@
 package postgresql
 
 import (
+	"database/sql"
 	"fmt"
 	"log"
 	"strings"
 
-	"database/sql"
-
 	"github.com/hashicorp/errwrap"
 	"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
 	"github.com/lib/pq"
@@ -111,7 +110,15 @@ var allowedPrivileges = map[string][]string{
 }
 
 // validatePrivileges checks that privileges to apply are allowed for this object type.
-func validatePrivileges(objectType string, privileges []interface{}) error {
+func validatePrivileges(d *schema.ResourceData) error {
+	objectType := d.Get("object_type").(string)
+	privileges := d.Get("privileges").(*schema.Set).List()
+
+	// Verify fields that are mandatory for specific object types
+	if objectType != "database" && d.Get("schema").(string) == "" {
+		return fmt.Errorf("parameter 'schema' is mandatory for object_type %s", objectType)
+	}
+
 	allowed, ok := allowedPrivileges[objectType]
 	if !ok {
 		return fmt.Errorf("unknown object type %s", objectType)

postgresql/resource_postgresql_default_privileges.go

@@ -93,7 +93,7 @@ func resourcePostgreSQLDefaultPrivilegesRead(d *schema.ResourceData, meta interf
 }
 
 func resourcePostgreSQLDefaultPrivilegesCreate(d *schema.ResourceData, meta interface{}) error {
-	if err := validatePrivileges(d.Get("object_type").(string), d.Get("privileges").(*schema.Set).List()); err != nil {
+	if err := validatePrivileges(d); err != nil {
 		return err
 	}
 
@@ -271,7 +271,6 @@ func revokeRoleDefaultPrivileges(txn *sql.Tx, d *schema.ResourceData) error {
 		return errwrap.Wrapf("could not revoke default privileges: {{err}}", err)
 	}
 	return nil
-
 }
 
 func generateDefaultPrivilegesID(d *schema.ResourceData) string {

postgresql/resource_postgresql_grant.go

@@ -14,20 +14,18 @@ import (
 	"github.com/lib/pq"
 )
 
+var allowedObjectTypes = []string{
+	"database",
+	"table",
+	"sequence",
+}
+
 var objectTypes = map[string]string{
-	"database": "d",
 	"table":    "r",
 	"sequence": "S",
 }
 
 func resourcePostgreSQLGrant() *schema.Resource {
-
-	allowedObjectTypes := make([]string, 0, len(objectTypes))
-
-	for k := range objectTypes {
-		allowedObjectTypes = append(allowedObjectTypes, k)
-	}
-
 	return &schema.Resource{
 		Create: resourcePostgreSQLGrantCreate,
 		// As create revokes and grants we can use it to update too
@@ -50,7 +48,7 @@ func resourcePostgreSQLGrant() *schema.Resource {
 			},
 			"schema": {
 				Type:        schema.TypeString,
-				Required:    true,
+				Optional:    true,
 				ForceNew:    true,
 				Description: "The database schema to grant privileges on for this role",
 			},
@@ -122,7 +120,7 @@ func resourcePostgreSQLGrantCreate(d *schema.ResourceData, meta interface{}) err
 		)
 	}
 
-	if err := validatePrivileges(d.Get("object_type").(string), d.Get("privileges").(*schema.Set).List()); err != nil {
+	if err := validatePrivileges(d); err != nil {
 		return err
 	}
 
@@ -193,7 +191,38 @@ func resourcePostgreSQLGrantDelete(d *schema.ResourceData, meta interface{}) err
 	return nil
 }
 
+func readDatabaseRolePriviges(txn *sql.Tx, d *schema.ResourceData) error {
+	query := `
+SELECT privilege_type
+FROM (
+	SELECT (aclexplode(datacl)).* FROM pg_database WHERE datname=$1
+) as privileges
+JOIN pg_roles ON grantee = pg_roles.oid WHERE rolname = $2
+`
+
+	privileges := []string{}
+	rows, err := txn.Query(query, d.Get("database"), d.Get("role"))
+	if err != nil {
+		return errwrap.Wrapf("could not read database privileges: {{err}}", err)
+	}
+
+	for rows.Next() {
+		var privilegeType string
+		if err := rows.Scan(&privilegeType); err != nil {
+			return errwrap.Wrapf("could not scan database privilege: {{err}}", err)
+		}
+		privileges = append(privileges, privilegeType)
+	}
+
+	d.Set("privileges", privileges)
+	return nil
+}
+
 func readRolePrivileges(txn *sql.Tx, d *schema.ResourceData) error {
+	if d.Get("object_type").(string) == "database" {
+		return readDatabaseRolePriviges(txn, d)
+	}
+
 	// This returns, for the specified role (rolname),
 	// the list of all object of the specified type (relkind) in the specified schema (namespace)
 	// with the list of the currently applied privileges (aggregation of privilege_type)
@@ -312,8 +341,10 @@ func grantRolePrivileges(txn *sql.Tx, d *schema.ResourceData) error {
 
 func revokeRolePrivileges(txn *sql.Tx, d *schema.ResourceData) error {
 	query := createRevokeQuery(d)
-	_, err := txn.Exec(query)
-	return err
+	if _, err := txn.Exec(query); err != nil {
+		return errwrap.Wrapf("could not execute revoke query: {{err}}", err)
+	}
+	return nil
 }
 
 func checkRoleDBSchemaExists(client *Client, d *schema.ResourceData) (bool, error) {
@@ -345,30 +376,37 @@ func checkRoleDBSchemaExists(client *Client, d *schema.ResourceData) (bool, erro
 		return false, nil
 	}
 
-	// Connect on this database to check if schema exists
-	dbTxn, err := startTransaction(client, database)
-	if err != nil {
-		return false, err
-	}
-	defer dbTxn.Rollback()
+	if d.Get("object_type").(string) != "database" {
+		// Connect on this database to check if schema exists
+		dbTxn, err := startTransaction(client, database)
+		if err != nil {
+			return false, err
+		}
+		defer dbTxn.Rollback()
 
-	// Check the schema exists (the SQL connection needs to be on the right database)
-	pgSchema := d.Get("schema").(string)
-	exists, err = schemaExists(dbTxn, pgSchema)
-	if err != nil {
-		return false, err
-	}
-	if !exists {
-		log.Printf("[DEBUG] schema %s does not exists", pgSchema)
-		return false, nil
+		// Check the schema exists (the SQL connection needs to be on the right database)
+		pgSchema := d.Get("schema").(string)
+		exists, err = schemaExists(dbTxn, pgSchema)
+		if err != nil {
+			return false, err
+		}
+		if !exists {
+			log.Printf("[DEBUG] schema %s does not exists", pgSchema)
+			return false, nil
+		}
 	}
 
 	return true, nil
 }
 
 func generateGrantID(d *schema.ResourceData) string {
-	return strings.Join([]string{
-		d.Get("role").(string), d.Get("database").(string),
-		d.Get("schema").(string), d.Get("object_type").(string),
-	}, "_")
+	parts := []string{d.Get("role").(string), d.Get("database").(string)}
+
+	objectType := d.Get("object_type").(string)
+	if objectType != "database" {
+		parts = append(parts, d.Get("schema").(string))
+	}
+	parts = append(parts, objectType)
+
+	return strings.Join(parts, "_")
 }

postgresql/resource_postgresql_grant_test.go

@@ -178,6 +178,9 @@ func TestAccPostgresqlGrant(t *testing.T) {
 			{
 				Config: testGrantSelect,
 				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(
+						"postgresql_grant.test", "id", fmt.Sprintf("%s_%s_test_schema_table", roleName, dbName),
+					),
 					resource.TestCheckResourceAttr("postgresql_grant.test", "privileges.#", "1"),
 					resource.TestCheckResourceAttr("postgresql_grant.test", "privileges.3138006342", "SELECT"),
 					func(*terraform.State) error {
@@ -213,25 +216,27 @@ func TestAccPostgresqlGrant(t *testing.T) {
 }
 
 func TestAccPostgresqlGrantDatabase(t *testing.T) {
-	skipIfNotAcc(t)
+	// create a TF config with placeholder for privileges
+	// it will be filled in each step.
+	config := fmt.Sprintf(`
+resource "postgresql_role" "test" {
+	name     = "test_grant_role"
+	password = "%s"
+	login    = true
+}
 
-	// We have to create the database outside of resource.Test
-	// because we need to create tables to assert that grant are correctly applied
-	// and we don't have this resource yet
-	dbSuffix, teardown := setupTestDatabase(t, true, true)
-	defer teardown()
+resource "postgresql_database" "test_db" {
+	depends_on = [postgresql_role.test]
+	name = "test_grant_db"
+}
 
-	dbName, roleName := getTestDBNames(dbSuffix)
-	var testGrantSelect = fmt.Sprintf(`
-	resource "postgresql_grant" "test" {
-		database           = "%s"
-		role               = "%s"
-		schema             = "test_schema"
-		object_type        = "database"
-		privileges         = ["CONNECT", "CREATE"]
-		with_grant_option  = true
-	}
-	`, dbName, roleName)
+resource "postgresql_grant" "test" {
+	database    = postgresql_database.test_db.name
+	role        = postgresql_role.test.name
+	object_type = "database"
+	privileges  = %%s
+}
+`, testRolePassword)
 
 	resource.Test(t, resource.TestCase{
 		PreCheck: func() {
@@ -240,16 +245,36 @@ func TestAccPostgresqlGrantDatabase(t *testing.T) {
 		},
 		Providers: testAccProviders,
 		Steps: []resource.TestStep{
+			// Not allowed to create
 			{
-				Config: testGrantSelect,
+				Config: fmt.Sprintf(config, "[\"CONNECT\"]"),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("postgresql_grant.test", "id", "test_grant_role_test_grant_db_database"),
+					resource.TestCheckResourceAttr("postgresql_grant.test", "privileges.#", "1"),
+					resource.TestCheckResourceAttr("postgresql_grant.test", "with_grant_option", "false"),
+					testCheckDatabasesPrivileges(t, false),
+				),
+			},
+			// Can create but not grant
+			{
+				Config: fmt.Sprintf(config, "[\"CONNECT\", \"CREATE\"]"),
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr("postgresql_grant.test", "privileges.#", "2"),
-					resource.TestCheckResourceAttr("postgresql_grant.test", "with_grant_option", "true"),
-					func(*terraform.State) error {
-						return testCheckDatabasesPrivileges(t, dbSuffix, []string{"CONNECT"})
-					},
+					testCheckDatabasesPrivileges(t, true),
 				),
 			},
 		},
 	})
 }
+
+func testCheckDatabasesPrivileges(t *testing.T, canCreate bool) func(*terraform.State) error {
+	return func(*terraform.State) error {
+		db := connectAsTestRole(t, "test_grant_role", "test_grant_db")
+		defer db.Close()
+
+		if err := testHasGrantForQuery(db, "CREATE SCHEMA plop", canCreate); err != nil {
+			return err
+		}
+		return nil
+	}
+}

postgresql/utils_test.go

@@ -151,18 +151,45 @@ func createTestTables(t *testing.T, dbSuffix string, tables []string, owner stri
 	}
 }
 
-func testCheckTablesPrivileges(t *testing.T, dbSuffix string, tables []string, allowedPrivileges []string) error {
+// testHasGrantForQuery executes a query and checks that it fails if
+// we were not allowed or succeses if we're allowed.
+func testHasGrantForQuery(db *sql.DB, query string, allowed bool) error {
+	_, err := db.Exec(query)
+	if err != nil {
+		if allowed {
+			return errwrap.Wrapf(
+				fmt.Sprintf(
+					"could not execute %s as expected: {{err}}", query,
+				), err,
+			)
+		}
+		return nil
+	}
+
+	if !allowed {
+		return fmt.Errorf("did not fail as expected when executing query '%s'", query)
+	}
+	return nil
+}
+
+func connectAsTestRole(t *testing.T, role, dbName string) *sql.DB {
 	config := getTestConfig(t)
-	dbName, roleName := getTestDBNames(dbSuffix)
 
 	// Connect as the test role
-	config.Username = roleName
+	config.Username = role
 	config.Password = testRolePassword
 
 	db, err := sql.Open("postgres", config.connStr(dbName))
 	if err != nil {
 		t.Fatalf("could not open connection pool for db %s: %v", dbName, err)
 	}
+	return db
+}
+
+func testCheckTablesPrivileges(t *testing.T, dbSuffix string, tables []string, allowedPrivileges []string) error {
+	dbName, roleName := getTestDBNames(dbSuffix)
+
+	db := connectAsTestRole(t, roleName, dbName)
 	defer db.Close()
 
 	for _, table := range tables {
@@ -174,46 +201,10 @@ func testCheckTablesPrivileges(t *testing.T, dbSuffix string, tables []string, a
 		}
 
 		for queryType, query := range queries {
-			_, err := db.Exec(query)
-
-			if err != nil && sliceContainsStr(allowedPrivileges, queryType) {
-				return errwrap.Wrapf(
-					fmt.Sprintf("could not %s on test table %s: {{err}}", queryType, table),
-					err,
-				)
-
-			} else if err == nil && !sliceContainsStr(allowedPrivileges, queryType) {
-				return errwrap.Wrapf(
-					fmt.Sprintf("%s did not failed as expected for table %s: {{err}}", queryType, table),
-					err,
-				)
+			if err := testHasGrantForQuery(db, query, sliceContainsStr(allowedPrivileges, queryType)); err != nil {
+				return err
 			}
 		}
 	}
 	return nil
 }
-
-func testCheckDatabasesPrivileges(t *testing.T, dbSuffix string, allowedPrivileges []string) error {
-	config := getTestConfig(t)
-	dbName, roleName := getTestDBNames(dbSuffix)
-
-	// Connect as the test role
-	config.Username = roleName
-	config.Password = testRolePassword
-
-	db, err := sql.Open("postgres", config.connStr(dbName))
-	if err != nil {
-		t.Fatalf("could not open connection pool for db %s: %v", dbName, err)
-	}
-	defer db.Close()
-
-	queries := map[string]string{
-		"CREATE": fmt.Sprintf("CREATE DATABASE foo"),
-	}
-
-	for _, query := range queries {
-		db.Exec(query)
-	}
-
-	return nil
-}