From d92a3caedf75bc0acc91a96561145b603e4854cc Mon Sep 17 00:00:00 2001 From: Sean Chittenden Date: Sun, 25 Dec 2016 04:56:39 -0800 Subject: [PATCH] Before revoking a privilege from a schema, check to ensure role exists. --- .../postgresql/resource_postgresql_schema.go | 13 +++++++- .../resource_postgresql_schema_test.go | 30 +++++++++---------- 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/builtin/providers/postgresql/resource_postgresql_schema.go b/builtin/providers/postgresql/resource_postgresql_schema.go index 22de321ab5..dbfba4c599 100644 --- a/builtin/providers/postgresql/resource_postgresql_schema.go +++ b/builtin/providers/postgresql/resource_postgresql_schema.go @@ -350,7 +350,18 @@ func setSchemaPolicy(txn *sql.Tx, d *schema.ResourceData) error { for _, p := range dropped { pMap := p.(map[string]interface{}) rolePolicy := schemaPolicyToACL(pMap) - queries = append(queries, rolePolicy.Revokes(schemaName)...) + + var foundUser bool + err := txn.QueryRow(`SELECT TRUE FROM pg_catalog.pg_user WHERE usename = $1`, rolePolicy.Role).Scan(&foundUser) + switch { + case err == sql.ErrNoRows: + // Don't execute this role's REVOKEs because the role + // was dropped first and therefore doesn't exist. + case err != nil: + return errwrap.Wrapf("Error reading schema: {{err}}", err) + default: + queries = append(queries, rolePolicy.Revokes(schemaName)...) + } } for _, p := range added { diff --git a/builtin/providers/postgresql/resource_postgresql_schema_test.go b/builtin/providers/postgresql/resource_postgresql_schema_test.go index cffd9c850f..abf5c81306 100644 --- a/builtin/providers/postgresql/resource_postgresql_schema_test.go +++ b/builtin/providers/postgresql/resource_postgresql_schema_test.go @@ -69,11 +69,11 @@ func TestAccPostgresqlSchema_AddPolicy(t *testing.T) { resource.TestCheckResourceAttr("postgresql_role.policy_move", "name", "policy_move"), resource.TestCheckResourceAttr("postgresql_role.all_with_grantstay", "name", "all_with_grantstay"), - // resource.TestCheckResourceAttr("postgresql_role.all_with_grantdrop", "name", "all_with_grantdrop"), + resource.TestCheckResourceAttr("postgresql_role.all_with_grantdrop", "name", "all_with_grantdrop"), resource.TestCheckResourceAttr("postgresql_schema.test4", "name", "test4"), resource.TestCheckResourceAttr("postgresql_schema.test4", "owner", "all_without_grant_stay"), - resource.TestCheckResourceAttr("postgresql_schema.test4", "policy.#", "6"), + resource.TestCheckResourceAttr("postgresql_schema.test4", "policy.#", "7"), resource.TestCheckResourceAttr("postgresql_schema.test4", "policy.108605972.create", "false"), resource.TestCheckResourceAttr("postgresql_schema.test4", "policy.108605972.create_with_grant", "true"), resource.TestCheckResourceAttr("postgresql_schema.test4", "policy.108605972.role", "all_with_grantstay"), @@ -99,11 +99,11 @@ func TestAccPostgresqlSchema_AddPolicy(t *testing.T) { resource.TestCheckResourceAttr("postgresql_schema.test4", "policy.3959936977.role", "policy_compose"), resource.TestCheckResourceAttr("postgresql_schema.test4", "policy.3959936977.usage", "false"), resource.TestCheckResourceAttr("postgresql_schema.test4", "policy.3959936977.usage_with_grant", "true"), - // resource.TestCheckResourceAttr("postgresql_schema.test4", "policy.4178211897.create", "false"), - // resource.TestCheckResourceAttr("postgresql_schema.test4", "policy.4178211897.create_with_grant", "true"), - // resource.TestCheckResourceAttr("postgresql_schema.test4", "policy.4178211897.role", "all_with_grantdrop"), - // resource.TestCheckResourceAttr("postgresql_schema.test4", "policy.4178211897.usage", "false"), - // resource.TestCheckResourceAttr("postgresql_schema.test4", "policy.4178211897.usage_with_grant", "true"), + resource.TestCheckResourceAttr("postgresql_schema.test4", "policy.4178211897.create", "false"), + resource.TestCheckResourceAttr("postgresql_schema.test4", "policy.4178211897.create_with_grant", "true"), + resource.TestCheckResourceAttr("postgresql_schema.test4", "policy.4178211897.role", "all_with_grantdrop"), + resource.TestCheckResourceAttr("postgresql_schema.test4", "policy.4178211897.usage", "false"), + resource.TestCheckResourceAttr("postgresql_schema.test4", "policy.4178211897.usage_with_grant", "true"), resource.TestCheckResourceAttr("postgresql_schema.test4", "policy.815478369.create", "true"), resource.TestCheckResourceAttr("postgresql_schema.test4", "policy.815478369.create_with_grant", "false"), resource.TestCheckResourceAttr("postgresql_schema.test4", "policy.815478369.role", "policy_compose"), @@ -298,9 +298,9 @@ resource "postgresql_role" "all_with_grantstay" { name = "all_with_grantstay" } -// resource "postgresql_role" "all_with_grantdrop" { -// name = "all_with_grantdrop" -// } +resource "postgresql_role" "all_with_grantdrop" { + name = "all_with_grantdrop" +} resource "postgresql_schema" "test4" { name = "test4" @@ -336,11 +336,11 @@ resource "postgresql_schema" "test4" { role = "${postgresql_role.all_with_grantstay.name}" } - // policy { - // create_with_grant = true - // usage_with_grant = true - // role = "${postgresql_role.all_with_grantdrop.name}" - // } + policy { + create_with_grant = true + usage_with_grant = true + role = "${postgresql_role.all_with_grantdrop.name}" + } policy { create_with_grant = true