Skip to content
This repository has been archived by the owner on Nov 14, 2020. It is now read-only.

Commit

Permalink
Review remark: handle error correctly when looking for schema.
Browse files Browse the repository at this point in the history
  • Loading branch information
cyrilgdn committed Jul 3, 2020
1 parent 76f94ee commit 2803762
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 2 deletions.
10 changes: 8 additions & 2 deletions postgresql/resource_postgresql_schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,8 @@ func createSchema(d *schema.ResourceData, c *Client, txn *sql.Tx) error {
err := txn.QueryRow(`SELECT TRUE FROM pg_catalog.pg_namespace WHERE nspname = $1`, schemaName).Scan(&foundSchema)

queries := []string{}
if err == sql.ErrNoRows {
switch {
case err == sql.ErrNoRows:
b := bytes.NewBufferString("CREATE SCHEMA ")
if c.featureSupported(featureSchemaCreateIfNotExist) {
if v := d.Get(schemaIfNotExists); v.(bool) {
Expand All @@ -187,7 +188,12 @@ func createSchema(d *schema.ResourceData, c *Client, txn *sql.Tx) error {
fmt.Fprint(b, " AUTHORIZATION ", pq.QuoteIdentifier(v.(string)))
}
queries = append(queries, b.String())
} else {

case err != nil:
return fmt.Errorf("Error looking for schema: %w", err)

default:
// The schema already exists, we just set the owner.
if err := setSchemaOwner(txn, d); err != nil {
return err
}
Expand Down
59 changes: 59 additions & 0 deletions postgresql/resource_postgresql_schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,39 @@ resource "postgresql_schema" "test_cascade" {
})
}

func TestAccPostgresqlSchema_AlreadyExists(t *testing.T) {
skipIfNotAcc(t)

dbSuffix, teardown := setupTestDatabase(t, true, true)
defer teardown()

dbName, roleName := getTestDBNames(dbSuffix)

// Test to create the schema 'public' that already exists
// to assert it does not fail.
var testAccPostgresqlSchemaConfig = fmt.Sprintf(`
resource "postgresql_schema" "public" {
name = "public"
database = "%s"
owner = "%s"
}
`, dbName, roleName)
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckPostgresqlSchemaDestroy,
Steps: []resource.TestStep{
{
Config: testAccPostgresqlSchemaConfig,
Check: resource.ComposeTestCheckFunc(
testAccCheckPostgresqlSchemaExists("postgresql_schema.public", "public"),
testAccCheckSchemaOwner(dbName, "public", roleName),
),
},
},
})
}

func testAccCheckPostgresqlSchemaDestroy(s *terraform.State) error {
client := testAccProvider.Meta().(*Client)

Expand Down Expand Up @@ -339,6 +372,32 @@ func testAccCreateSchemaTable(database, schemaName string) resource.TestCheckFun
}
}

func testAccCheckSchemaOwner(database, schemaName, expectedOwner string) resource.TestCheckFunc {
return func(s *terraform.State) error {
client, err := testAccProvider.Meta().(*Client).config.NewClient(database)
if err != nil {
return fmt.Errorf("could not create client on database %s: %w", schemaName, err)
}
db := client.DB()

var owner string

query := "SELECT pg_catalog.pg_get_userbyid(n.nspowner) FROM pg_catalog.pg_namespace n WHERE n.nspname=$1"
switch err := db.QueryRow(query, schemaName).Scan(&owner); {
case err == sql.ErrNoRows:
return fmt.Errorf("could not find schema %s while checking owner", schemaName)
case err != nil:
return fmt.Errorf("error reading owner of schema %s: %w", schemaName, err)
}

if owner != expectedOwner {
return fmt.Errorf("expected owner of schema %s to be %s; got %s", schemaName, expectedOwner, owner)
}

return nil
}
}

const testAccPostgresqlSchemaConfig = `
resource "postgresql_role" "role_all_without_grant" {
name = "role_all_without_grant"
Expand Down

0 comments on commit 2803762

Please sign in to comment.