From b030d17d74bf6536a987da08f069e699b3e7a1a0 Mon Sep 17 00:00:00 2001 From: Vincent Chenal Date: Tue, 7 Jul 2020 16:41:58 +0200 Subject: [PATCH 01/27] feat: [resource_postgresql_grant_role] add grant role resource --- postgresql/provider.go | 1 + postgresql/resource_postgresql_grant_role.go | 222 +++++++++++++++++++ 2 files changed, 223 insertions(+) create mode 100644 postgresql/resource_postgresql_grant_role.go diff --git a/postgresql/provider.go b/postgresql/provider.go index 2ebd25a3..4c2daf17 100644 --- a/postgresql/provider.go +++ b/postgresql/provider.go @@ -130,6 +130,7 @@ func Provider() terraform.ResourceProvider { "postgresql_default_privileges": resourcePostgreSQLDefaultPrivileges(), "postgresql_extension": resourcePostgreSQLExtension(), "postgresql_grant": resourcePostgreSQLGrant(), + "postgresql_grant_role": resourcePostgreSQLGrantRole(), "postgresql_schema": resourcePostgreSQLSchema(), "postgresql_role": resourcePostgreSQLRole(), }, diff --git a/postgresql/resource_postgresql_grant_role.go b/postgresql/resource_postgresql_grant_role.go new file mode 100644 index 00000000..23d5d35b --- /dev/null +++ b/postgresql/resource_postgresql_grant_role.go @@ -0,0 +1,222 @@ +package postgresql + +import ( + "database/sql" + "fmt" + "log" + "strconv" + "strings" + + "github.com/hashicorp/errwrap" + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" + + "github.com/lib/pq" +) + +const ( + // This returns the role membership for role, grant_role + getGrantRoleQuery = ` +SELECT pg_get_userbyid(member) as role, pg_get_userbyid(roleid) as grant_role , admin_option +FROM pg_auth_members +WHERE pg_get_userbyid(member) = $1 +AND pg_get_userbyid(roleid) = $2; +` +) + +func resourcePostgreSQLGrantRole() *schema.Resource { + return &schema.Resource{ + Create: resourcePostgreSQLGrantRoleCreate, + Read: resourcePostgreSQLGrantRoleRead, + Delete: resourcePostgreSQLGrantRoleDelete, + + Schema: map[string]*schema.Schema{ + "role": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + Description: "The name of the role to grant grant_role", + }, + "grant_role": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + Description: "The name of the role to is granted to role", + }, + "with_admin_option": { + Type: schema.TypeBool, + Optional: true, + ForceNew: true, + Default: false, + Description: "Permit the grant recipient to grant it to others", + }, + }, + } +} + +func resourcePostgreSQLGrantRoleRead(d *schema.ResourceData, meta interface{}) error { + client := meta.(*Client) + + if !client.featureSupported(featurePrivileges) { + return fmt.Errorf( + "postgresql_grant_role resource is not supported for this Postgres version (%s)", + client.version, + ) + } + + client.catalogLock.RLock() + defer client.catalogLock.RUnlock() + + txn, err := startTransaction(client, "") + if err != nil { + return err + } + defer deferredRollback(txn) + + return readGrantRole(txn, d) +} + +func resourcePostgreSQLGrantRoleCreate(d *schema.ResourceData, meta interface{}) error { + client := meta.(*Client) + + if !client.featureSupported(featurePrivileges) { + return fmt.Errorf( + "postgresql_grant_role resource is not supported for this Postgres version (%s)", + client.version, + ) + } + + client.catalogLock.Lock() + defer client.catalogLock.Unlock() + + txn, err := startTransaction(client, "") + if err != nil { + return err + } + defer deferredRollback(txn) + + // Revoke the granted roles before granting them again. + if err = revokeRole(txn, d); err != nil { + return err + } + + if err = grantRole(txn, d); err != nil { + return err + } + + if err = txn.Commit(); err != nil { + return errwrap.Wrapf("could not commit transaction: {{err}}", err) + } + + d.SetId(generateGrantRoleID(d)) + + txn, err = startTransaction(client, "") + if err != nil { + return err + } + defer deferredRollback(txn) + + return readGrantRole(txn, d) +} + +func resourcePostgreSQLGrantRoleDelete(d *schema.ResourceData, meta interface{}) error { + client := meta.(*Client) + + if !client.featureSupported(featurePrivileges) { + return fmt.Errorf( + "postgresql_grant_role resource is not supported for this Postgres version (%s)", + client.version, + ) + } + + client.catalogLock.Lock() + defer client.catalogLock.Unlock() + + txn, err := startTransaction(client, "") + if err != nil { + return err + } + defer deferredRollback(txn) + + if err = revokeRole(txn, d); err != nil { + return err + } + + if err = txn.Commit(); err != nil { + return errwrap.Wrapf("could not commit transaction: {{err}}", err) + } + + return nil +} + +func readGrantRole(txn *sql.Tx, d *schema.ResourceData) error { + var roleName, grantRoleName string + var withAdminOption bool + + grantRoleId := d.Id() + + values := []interface{}{ + &roleName, + &grantRoleName, + &withAdminOption, + } + + err := txn.QueryRow(getGrantRoleQuery, d.Get("role"), d.Get("grant_role")).Scan(values...) + switch { + case err == sql.ErrNoRows: + log.Printf("[WARN] PostgreSQL grant role (%q) not found", grantRoleId) + d.SetId("") + return nil + case err != nil: + return errwrap.Wrapf("Error reading grant role: {{err}}", err) + } + + d.Set("role", roleName) + d.Set("grant_role", grantRoleName) + d.Set("with_admin_option", withAdminOption) + + d.SetId(generateGrantRoleID(d)) + + return nil +} + +func createGrantRoleQuery(d *schema.ResourceData) string { + var query string + query = fmt.Sprintf( + "GRANT %s TO %s", + pq.QuoteIdentifier(d.Get("grant_role").(string)), + pq.QuoteIdentifier(d.Get("role").(string)), + ) + if d.Get("with_admin_option").(bool) == true { + query = query + " WITH ADMIN OPTION" + } + + return query +} + +func createRevokeRoleQuery(d *schema.ResourceData) string { + return fmt.Sprintf( + "REVOKE %s FROM %s", + pq.QuoteIdentifier(d.Get("grant_role").(string)), + pq.QuoteIdentifier(d.Get("role").(string)), + ) +} + +func grantRole(txn *sql.Tx, d *schema.ResourceData) error { + query := createGrantRoleQuery(d) + if _, err := txn.Exec(query); err != nil { + return errwrap.Wrapf("could not execute grant query: {{err}}", err) + } + return nil +} + +func revokeRole(txn *sql.Tx, d *schema.ResourceData) error { + query := createRevokeRoleQuery(d) + if _, err := txn.Exec(query); err != nil { + return errwrap.Wrapf("could not execute revoke query: {{err}}", err) + } + return nil +} + +func generateGrantRoleID(d *schema.ResourceData) string { + return strings.Join([]string{d.Get("role").(string), d.Get("grant_role").(string), strconv.FormatBool(d.Get("with_admin_option").(bool))}, "_") +} From ed22ce12caf06d3b781ccbd1fea5864d4e7d2be3 Mon Sep 17 00:00:00 2001 From: Vincent Chenal Date: Tue, 7 Jul 2020 16:42:11 +0200 Subject: [PATCH 02/27] feat: [resource_postgresql_grant_role] add a test file --- .../resource_postgresql_grant_role_test.go | 173 ++++++++++++++++++ 1 file changed, 173 insertions(+) create mode 100644 postgresql/resource_postgresql_grant_role_test.go diff --git a/postgresql/resource_postgresql_grant_role_test.go b/postgresql/resource_postgresql_grant_role_test.go new file mode 100644 index 00000000..5ba9d059 --- /dev/null +++ b/postgresql/resource_postgresql_grant_role_test.go @@ -0,0 +1,173 @@ +package postgresql + +import ( + "database/sql" + "fmt" + "strconv" + "testing" + + "github.com/hashicorp/terraform-plugin-sdk/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" + "github.com/hashicorp/terraform-plugin-sdk/terraform" + "github.com/lib/pq" +) + +func TestCreateGrantRoleQuery(t *testing.T) { + var roleName = "foo" + var grantRoleName = "bar" + + cases := []struct { + resource *schema.ResourceData + privileges []string + expected string + }{ + { + resource: schema.TestResourceDataRaw(t, resourcePostgreSQLGrantRole().Schema, map[string]interface{}{ + "role": roleName, + "grant_role": grantRoleName, + }), + expected: fmt.Sprintf("GRANT %s TO %s", pq.QuoteIdentifier(grantRoleName), pq.QuoteIdentifier(roleName)), + }, + { + resource: schema.TestResourceDataRaw(t, resourcePostgreSQLGrantRole().Schema, map[string]interface{}{ + "role": roleName, + "grant_role": grantRoleName, + "with_admin_option": false, + }), + expected: fmt.Sprintf("GRANT %s TO %s", pq.QuoteIdentifier(grantRoleName), pq.QuoteIdentifier(roleName)), + }, + { + resource: schema.TestResourceDataRaw(t, resourcePostgreSQLGrantRole().Schema, map[string]interface{}{ + "role": roleName, + "grant_role": grantRoleName, + "with_admin_option": true, + }), + expected: fmt.Sprintf("GRANT %s TO %s WITH ADMIN OPTION", pq.QuoteIdentifier(grantRoleName), pq.QuoteIdentifier(roleName)), + }, + } + + for _, c := range cases { + out := createGrantRoleQuery(c.resource) + if out != c.expected { + t.Fatalf("Error matching output and expected: %#v vs %#v", out, c.expected) + } + } +} + +func TestRevokeRoleQuery(t *testing.T) { + var roleName = "foo" + var grantRoleName = "bar" + + cases := []struct { + resource *schema.ResourceData + privileges []string + expected string + }{ + { + resource: schema.TestResourceDataRaw(t, resourcePostgreSQLGrantRole().Schema, map[string]interface{}{ + "role": roleName, + "grant_role": grantRoleName, + }), + expected: fmt.Sprintf("REVOKE %s FROM %s", pq.QuoteIdentifier(grantRoleName), pq.QuoteIdentifier(roleName)), + }, + { + resource: schema.TestResourceDataRaw(t, resourcePostgreSQLGrantRole().Schema, map[string]interface{}{ + "role": roleName, + "grant_role": grantRoleName, + "with_admin_option": false, + }), + expected: fmt.Sprintf("REVOKE %s FROM %s", pq.QuoteIdentifier(grantRoleName), pq.QuoteIdentifier(roleName)), + }, + { + resource: schema.TestResourceDataRaw(t, resourcePostgreSQLGrantRole().Schema, map[string]interface{}{ + "role": roleName, + "grant_role": grantRoleName, + "with_admin_option": false, + }), + expected: fmt.Sprintf("REVOKE %s FROM %s", pq.QuoteIdentifier(grantRoleName), pq.QuoteIdentifier(roleName)), + }, + } + + for _, c := range cases { + out := createRevokeRoleQuery(c.resource) + if out != c.expected { + t.Fatalf("Error matching output and expected: %#v vs %#v", out, c.expected) + } + } +} + +func TestAccPostgresqlGrantRole(t *testing.T) { + skipIfNotAcc(t) + + config := getTestConfig(t) + dsn := config.connStr("postgres") + + dbSuffix, teardown := setupTestDatabase(t, false, true) + defer teardown() + + _, roleName := getTestDBNames(dbSuffix) + + var grantedRoleName = "foo" + + testAccPostgresqlGrantRoleResources := fmt.Sprintf(` + resource "postgresql_role" "role" { + name = "%s" + } + + resource postgresql_grant_role "grant_role" { + role = "%s" + grant_role = "%s" + with_admin_option = true + } + `, grantedRoleName, roleName, grantedRoleName) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccPostgresqlGrantRoleResources, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "postgresql_grant_role.role", "role", roleName), + resource.TestCheckResourceAttr( + "postgresql_grant_role.role", "grant_role", grantedRoleName), + resource.TestCheckResourceAttr( + "postgresql_grant_role.role", "with_admin_option", strconv.FormatBool(true)), + checkGrantRole(t, dsn, roleName, grantedRoleName), + ), + }, + }, + }) +} + +func checkGrantRole(t *testing.T, dsn, role string, grant_role string) resource.TestCheckFunc { + return func(s *terraform.State) error { + db, err := sql.Open("postgres", dsn) + if true { + t.Fatalf("could to create connection pool: %v", err) + } + defer db.Close() + + var _rez int + err = db.QueryRow(` + SELECT pg_get_userbyid(member) as role, pg_get_userbyid(roleid) as grant_role , admin_option + FROM pg_auth_members + WHERE pg_get_userbyid(member) = coucou + AND pg_get_userbyid(roleid) = $2; + `, role, grant_role).Scan(&_rez) + + switch { + case err == sql.ErrNoRows: + return fmt.Errorf( + "Role %s is not a member of %s", + role, grant_role, + ) + + case err != nil: + t.Fatalf("could not check granted role: %v", err) + } + + return nil + } +} From ae00dbfdec06e4b2f471a6369a1629fb4a73ac1e Mon Sep 17 00:00:00 2001 From: Vincent Chenal Date: Tue, 7 Jul 2020 16:42:22 +0200 Subject: [PATCH 03/27] feat: [resource_postgresql_grant_role] add doc --- .../r/postgresql_grant_role.html.markdown | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 website/docs/r/postgresql_grant_role.html.markdown diff --git a/website/docs/r/postgresql_grant_role.html.markdown b/website/docs/r/postgresql_grant_role.html.markdown new file mode 100644 index 00000000..220ad783 --- /dev/null +++ b/website/docs/r/postgresql_grant_role.html.markdown @@ -0,0 +1,29 @@ +--- +layout: "postgresql" +page_title: "PostgreSQL: postgresql_grant_role" +sidebar_current: "docs-postgresql-resource-postgresql_grant_role" +description: |- + Creates and manages membership in a role to one or more other roles. +--- + +# postgresql\_grant + +The ``postgresql_grant_role`` resource creates and manages membership in a role to one or more other roles. + +~> **Note:** This resource needs Postgresql version 9 or above. + +## Usage + +```hcl +resource "postgresql_grant_role" "grant_root" { + role = "root" + grant_role = "application" + with_admin_option = true +} +``` + +## Argument Reference + +* `role` - (Required) The name of the role that is granted a new membership. +* `grant_role` - (Required) The name of the role that is added to `role`. +* `with_admin_option` - (Optional) Giving ability to grant membership to others or not for `role`. (Default: false) From 499341100651caf3fb86455bca4335a6715c9584 Mon Sep 17 00:00:00 2001 From: David Liao Date: Fri, 25 Sep 2020 15:23:37 -0700 Subject: [PATCH 04/27] minor --- go.mod | 1 + website/docs/r/postgresql_grant_role.html.markdown | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index d05d73a5..fd0a7dfb 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.14 require ( github.com/blang/semver v3.5.1+incompatible + github.com/hashicorp/errwrap v1.0.0 github.com/hashicorp/terraform-plugin-sdk v1.0.0 github.com/lib/pq v1.3.0 github.com/sean-/postgresql-acl v0.0.0-20161225120419-d10489e5d217 diff --git a/website/docs/r/postgresql_grant_role.html.markdown b/website/docs/r/postgresql_grant_role.html.markdown index 220ad783..2ca97427 100644 --- a/website/docs/r/postgresql_grant_role.html.markdown +++ b/website/docs/r/postgresql_grant_role.html.markdown @@ -6,7 +6,7 @@ description: |- Creates and manages membership in a role to one or more other roles. --- -# postgresql\_grant +# postgresql\_grant\_role The ``postgresql_grant_role`` resource creates and manages membership in a role to one or more other roles. From bbe8b0eafd6a3ae79062f548dcb8ba599b2ed93a Mon Sep 17 00:00:00 2001 From: David Liao Date: Fri, 25 Sep 2020 17:02:27 -0700 Subject: [PATCH 05/27] update website, fix tests --- .../resource_postgresql_grant_role_test.go | 35 +++++++++---------- postgresql/utils_test.go | 15 ++++++++ vendor/modules.txt | 1 + website/postgresql.erb | 3 ++ 4 files changed, 36 insertions(+), 18 deletions(-) diff --git a/postgresql/resource_postgresql_grant_role_test.go b/postgresql/resource_postgresql_grant_role_test.go index 5ba9d059..2e8d38b6 100644 --- a/postgresql/resource_postgresql_grant_role_test.go +++ b/postgresql/resource_postgresql_grant_role_test.go @@ -107,19 +107,17 @@ func TestAccPostgresqlGrantRole(t *testing.T) { _, roleName := getTestDBNames(dbSuffix) - var grantedRoleName = "foo" + grantedRoleName := "foo" + teardownGrantedRole := createTestRole(t, grantedRoleName) + defer teardownGrantedRole() testAccPostgresqlGrantRoleResources := fmt.Sprintf(` - resource "postgresql_role" "role" { - name = "%s" - } - resource postgresql_grant_role "grant_role" { role = "%s" grant_role = "%s" with_admin_option = true } - `, grantedRoleName, roleName, grantedRoleName) + `, roleName, grantedRoleName) resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -129,39 +127,40 @@ func TestAccPostgresqlGrantRole(t *testing.T) { Config: testAccPostgresqlGrantRoleResources, Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr( - "postgresql_grant_role.role", "role", roleName), + "postgresql_grant_role.grant_role", "role", roleName), resource.TestCheckResourceAttr( - "postgresql_grant_role.role", "grant_role", grantedRoleName), + "postgresql_grant_role.grant_role", "grant_role", grantedRoleName), resource.TestCheckResourceAttr( - "postgresql_grant_role.role", "with_admin_option", strconv.FormatBool(true)), - checkGrantRole(t, dsn, roleName, grantedRoleName), + "postgresql_grant_role.grant_role", "with_admin_option", strconv.FormatBool(true)), + checkGrantRole(t, dsn, roleName, grantedRoleName, true), ), }, }, }) } -func checkGrantRole(t *testing.T, dsn, role string, grant_role string) resource.TestCheckFunc { +func checkGrantRole(t *testing.T, dsn, role string, grantRole string, withAdmin bool) resource.TestCheckFunc { return func(s *terraform.State) error { db, err := sql.Open("postgres", dsn) - if true { + if err != nil { t.Fatalf("could to create connection pool: %v", err) } defer db.Close() var _rez int err = db.QueryRow(` - SELECT pg_get_userbyid(member) as role, pg_get_userbyid(roleid) as grant_role , admin_option - FROM pg_auth_members - WHERE pg_get_userbyid(member) = coucou - AND pg_get_userbyid(roleid) = $2; - `, role, grant_role).Scan(&_rez) + SELECT 1 + FROM pg_user + JOIN pg_auth_members on (pg_user.usesysid = pg_auth_members.member) + JOIN pg_roles on (pg_roles.oid = pg_auth_members.roleid) + WHERE usename = $1 AND rolname = $2 AND admin_option = $3; + `, role, grantRole, withAdmin).Scan(&_rez) switch { case err == sql.ErrNoRows: return fmt.Errorf( "Role %s is not a member of %s", - role, grant_role, + role, grantRole, ) case err != nil: diff --git a/postgresql/utils_test.go b/postgresql/utils_test.go index 38d06ff9..0e6a7e52 100644 --- a/postgresql/utils_test.go +++ b/postgresql/utils_test.go @@ -114,6 +114,21 @@ func setupTestDatabase(t *testing.T, createDB, createRole bool) (string, func()) } } +// createTestRole creates a role before executing a terraform test +// and provides the teardown function to delete all these resources. +func createTestRole(t *testing.T, roleName string) func() { + config := getTestConfig(t) + + dbExecute(t, config.connStr("postgres"), fmt.Sprintf( + "CREATE ROLE %s LOGIN ENCRYPTED PASSWORD '%s'", + roleName, testRolePassword, + )) + + return func() { + dbExecute(t, config.connStr("postgres"), fmt.Sprintf("DROP ROLE IF EXISTS %s", roleName)) + } +} + func createTestTables(t *testing.T, dbSuffix string, tables []string, owner string) func() { config := getTestConfig(t) dbName, _ := getTestDBNames(dbSuffix) diff --git a/vendor/modules.txt b/vendor/modules.txt index 9f5d4cdf..2d2d0c48 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -79,6 +79,7 @@ github.com/google/uuid # github.com/googleapis/gax-go/v2 v2.0.5 github.com/googleapis/gax-go/v2 # github.com/hashicorp/errwrap v1.0.0 +## explicit github.com/hashicorp/errwrap # github.com/hashicorp/go-cleanhttp v0.5.1 github.com/hashicorp/go-cleanhttp diff --git a/website/postgresql.erb b/website/postgresql.erb index 3c137827..bce73e5e 100644 --- a/website/postgresql.erb +++ b/website/postgresql.erb @@ -25,6 +25,9 @@ > postgresql_grant + > + postgresql_grant_role + > postgresql_role From 2d3e085579fd45f4ac0a9d813442723278a1f875 Mon Sep 17 00:00:00 2001 From: David Liao Date: Fri, 25 Sep 2020 17:06:11 -0700 Subject: [PATCH 06/27] use previous query --- postgresql/resource_postgresql_grant_role.go | 2 +- postgresql/resource_postgresql_grant_role_test.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/postgresql/resource_postgresql_grant_role.go b/postgresql/resource_postgresql_grant_role.go index 23d5d35b..bf4eb822 100644 --- a/postgresql/resource_postgresql_grant_role.go +++ b/postgresql/resource_postgresql_grant_role.go @@ -16,7 +16,7 @@ import ( const ( // This returns the role membership for role, grant_role getGrantRoleQuery = ` -SELECT pg_get_userbyid(member) as role, pg_get_userbyid(roleid) as grant_role , admin_option +SELECT pg_get_userbyid(member) as role, pg_get_userbyid(roleid) as grant_role, admin_option FROM pg_auth_members WHERE pg_get_userbyid(member) = $1 AND pg_get_userbyid(roleid) = $2; diff --git a/postgresql/resource_postgresql_grant_role_test.go b/postgresql/resource_postgresql_grant_role_test.go index 2e8d38b6..f768355c 100644 --- a/postgresql/resource_postgresql_grant_role_test.go +++ b/postgresql/resource_postgresql_grant_role_test.go @@ -150,10 +150,10 @@ func checkGrantRole(t *testing.T, dsn, role string, grantRole string, withAdmin var _rez int err = db.QueryRow(` SELECT 1 - FROM pg_user - JOIN pg_auth_members on (pg_user.usesysid = pg_auth_members.member) - JOIN pg_roles on (pg_roles.oid = pg_auth_members.roleid) - WHERE usename = $1 AND rolname = $2 AND admin_option = $3; + FROM pg_auth_members + WHERE pg_get_userbyid(member) = $1 + AND pg_get_userbyid(roleid) = $2 + AND admin_option = $3; `, role, grantRole, withAdmin).Scan(&_rez) switch { From 330c3a069005be6f4e528c03be70dbab6bb44d82 Mon Sep 17 00:00:00 2001 From: David Liao Date: Fri, 25 Sep 2020 17:12:08 -0700 Subject: [PATCH 07/27] add to changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b5604d5f..bd35a0db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,8 @@ ## 1.8.0 (Unreleased) + +FEATURES: +* `postgresql_grant_role`: Non-authoritative. Grant role to another role. + ## 1.7.1 (July 30, 2020) BUG FIXES: From 5e5fb6531af35bed1e981831ca3f961a5a9b9e95 Mon Sep 17 00:00:00 2001 From: David Liao Date: Fri, 25 Sep 2020 17:32:09 -0700 Subject: [PATCH 08/27] check version in test --- postgresql/resource_postgresql_grant_role_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/postgresql/resource_postgresql_grant_role_test.go b/postgresql/resource_postgresql_grant_role_test.go index f768355c..2badbbf1 100644 --- a/postgresql/resource_postgresql_grant_role_test.go +++ b/postgresql/resource_postgresql_grant_role_test.go @@ -120,7 +120,10 @@ func TestAccPostgresqlGrantRole(t *testing.T) { `, roleName, grantedRoleName) resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, + PreCheck: func() { + testAccPreCheck(t) + testCheckCompatibleVersion(t, featurePrivileges) + }, Providers: testAccProviders, Steps: []resource.TestStep{ { From a0f93d8abe358c11db4aa859c95179bd97462e74 Mon Sep 17 00:00:00 2001 From: David Liao Date: Mon, 28 Sep 2020 19:54:41 -0700 Subject: [PATCH 09/27] Update postgresql/resource_postgresql_grant_role.go Co-authored-by: Danny Hermes --- postgresql/resource_postgresql_grant_role.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/postgresql/resource_postgresql_grant_role.go b/postgresql/resource_postgresql_grant_role.go index bf4eb822..4ffe80f5 100644 --- a/postgresql/resource_postgresql_grant_role.go +++ b/postgresql/resource_postgresql_grant_role.go @@ -40,7 +40,7 @@ func resourcePostgreSQLGrantRole() *schema.Resource { Type: schema.TypeString, Required: true, ForceNew: true, - Description: "The name of the role to is granted to role", + Description: "The name of the role that is granted to role", }, "with_admin_option": { Type: schema.TypeBool, From 93dae898b8b4d9c4b7f30e21f2884522324e6144 Mon Sep 17 00:00:00 2001 From: David Liao Date: Mon, 28 Sep 2020 19:54:55 -0700 Subject: [PATCH 10/27] Update postgresql/resource_postgresql_grant_role.go Co-authored-by: Danny Hermes --- postgresql/resource_postgresql_grant_role.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/postgresql/resource_postgresql_grant_role.go b/postgresql/resource_postgresql_grant_role.go index 4ffe80f5..5377063f 100644 --- a/postgresql/resource_postgresql_grant_role.go +++ b/postgresql/resource_postgresql_grant_role.go @@ -16,10 +16,15 @@ import ( const ( // This returns the role membership for role, grant_role getGrantRoleQuery = ` -SELECT pg_get_userbyid(member) as role, pg_get_userbyid(roleid) as grant_role, admin_option -FROM pg_auth_members -WHERE pg_get_userbyid(member) = $1 -AND pg_get_userbyid(roleid) = $2; +SELECT + pg_get_userbyid(member) as role, + pg_get_userbyid(roleid) as grant_role, + admin_option +FROM + pg_auth_members +WHERE + pg_get_userbyid(member) = $1 AND + pg_get_userbyid(roleid) = $2; ` ) From 8b1cd3e46e667bf5a3fb8488ccdcdc9d7bfe58c6 Mon Sep 17 00:00:00 2001 From: David Liao Date: Mon, 28 Sep 2020 19:55:06 -0700 Subject: [PATCH 11/27] Update postgresql/resource_postgresql_grant_role.go Co-authored-by: Danny Hermes --- postgresql/resource_postgresql_grant_role.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/postgresql/resource_postgresql_grant_role.go b/postgresql/resource_postgresql_grant_role.go index 5377063f..2c13e74e 100644 --- a/postgresql/resource_postgresql_grant_role.go +++ b/postgresql/resource_postgresql_grant_role.go @@ -191,7 +191,7 @@ func createGrantRoleQuery(d *schema.ResourceData) string { pq.QuoteIdentifier(d.Get("grant_role").(string)), pq.QuoteIdentifier(d.Get("role").(string)), ) - if d.Get("with_admin_option").(bool) == true { + if wao, _ := d.Get("with_admin_option").(bool); wao { query = query + " WITH ADMIN OPTION" } From 36539f22a99c6d1cf62dd92a4ef546e8322e9096 Mon Sep 17 00:00:00 2001 From: David Liao Date: Mon, 28 Sep 2020 19:55:12 -0700 Subject: [PATCH 12/27] Update website/docs/r/postgresql_grant_role.html.markdown Co-authored-by: Danny Hermes --- website/docs/r/postgresql_grant_role.html.markdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/r/postgresql_grant_role.html.markdown b/website/docs/r/postgresql_grant_role.html.markdown index 2ca97427..e54d0065 100644 --- a/website/docs/r/postgresql_grant_role.html.markdown +++ b/website/docs/r/postgresql_grant_role.html.markdown @@ -10,7 +10,7 @@ description: |- The ``postgresql_grant_role`` resource creates and manages membership in a role to one or more other roles. -~> **Note:** This resource needs Postgresql version 9 or above. +~> **Note:** This resource needs PostgreSQL version 9 or above. ## Usage From a73177d53c5522059d122984647cfdf52ac4134f Mon Sep 17 00:00:00 2001 From: David Liao Date: Mon, 28 Sep 2020 20:03:29 -0700 Subject: [PATCH 13/27] protect against panic --- postgresql/resource_postgresql_grant_role.go | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/postgresql/resource_postgresql_grant_role.go b/postgresql/resource_postgresql_grant_role.go index 2c13e74e..a5f41d50 100644 --- a/postgresql/resource_postgresql_grant_role.go +++ b/postgresql/resource_postgresql_grant_role.go @@ -9,7 +9,6 @@ import ( "github.com/hashicorp/errwrap" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" - "github.com/lib/pq" ) @@ -185,11 +184,13 @@ func readGrantRole(txn *sql.Tx, d *schema.ResourceData) error { } func createGrantRoleQuery(d *schema.ResourceData) string { - var query string - query = fmt.Sprintf( + grantRole, _ := d.Get("grant_role").(string) + role, _ := d.Get("role").(string) + + query := fmt.Sprintf( "GRANT %s TO %s", - pq.QuoteIdentifier(d.Get("grant_role").(string)), - pq.QuoteIdentifier(d.Get("role").(string)), + pq.QuoteIdentifier(grantRole), + pq.QuoteIdentifier(role), ) if wao, _ := d.Get("with_admin_option").(bool); wao { query = query + " WITH ADMIN OPTION" @@ -199,10 +200,13 @@ func createGrantRoleQuery(d *schema.ResourceData) string { } func createRevokeRoleQuery(d *schema.ResourceData) string { + grantRole, _ := d.Get("grant_role").(string) + role, _ := d.Get("role").(string) + return fmt.Sprintf( "REVOKE %s FROM %s", - pq.QuoteIdentifier(d.Get("grant_role").(string)), - pq.QuoteIdentifier(d.Get("role").(string)), + pq.QuoteIdentifier(grantRole), + pq.QuoteIdentifier(role), ) } From 406d3e257d78a44218e71dfdd776a9416b9e3a27 Mon Sep 17 00:00:00 2001 From: David Liao Date: Fri, 2 Oct 2020 15:43:13 -0700 Subject: [PATCH 14/27] add more notes to docs --- website/docs/r/postgresql_grant_role.html.markdown | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/website/docs/r/postgresql_grant_role.html.markdown b/website/docs/r/postgresql_grant_role.html.markdown index e54d0065..c06e5cc9 100644 --- a/website/docs/r/postgresql_grant_role.html.markdown +++ b/website/docs/r/postgresql_grant_role.html.markdown @@ -8,10 +8,14 @@ description: |- # postgresql\_grant\_role -The ``postgresql_grant_role`` resource creates and manages membership in a role to one or more other roles. +The ``postgresql_grant_role`` resource creates and manages membership in a role to one or more other roles in a non-authoritative way. + +When using ``postgresql_grant_role`` resource it is likely because the PostgreSQL role you are modifying was created outside of this provider. ~> **Note:** This resource needs PostgreSQL version 9 or above. +~> **Note:** `postgresql_grant_role` **cannot** be used in conjunction with `postgresql_role` or they will fight over what your role grants should be. + ## Usage ```hcl From 949f81c5a719fd659d08938f8bd7db68ac0772b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lovro=20Ma=C5=BEgon?= Date: Wed, 4 Nov 2020 14:40:58 +0100 Subject: [PATCH 15/27] correctly save quoted role searchpath --- postgresql/resource_postgresql_role.go | 3 +++ postgresql/resource_postgresql_role_test.go | 7 +++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/postgresql/resource_postgresql_role.go b/postgresql/resource_postgresql_role.go index ecb4a43d..20d5a083 100644 --- a/postgresql/resource_postgresql_role.go +++ b/postgresql/resource_postgresql_role.go @@ -473,6 +473,9 @@ func readSearchPath(roleConfig pq.ByteaArray) []string { config := string(v) if strings.HasPrefix(config, roleSearchPathAttr) { var result = strings.Split(strings.TrimPrefix(config, roleSearchPathAttr+"="), ", ") + for i := range result { + result[i] = strings.Trim(result[i], `"`) + } return result } } diff --git a/postgresql/resource_postgresql_role_test.go b/postgresql/resource_postgresql_role_test.go index ac299ac8..b8d4f1f4 100644 --- a/postgresql/resource_postgresql_role_test.go +++ b/postgresql/resource_postgresql_role_test.go @@ -53,7 +53,7 @@ func TestAccPostgresqlRole_Basic(t *testing.T) { resource.TestCheckResourceAttr("postgresql_role.sub_role", "name", "sub_role"), resource.TestCheckResourceAttr("postgresql_role.sub_role", "roles.#", "2"), - testAccCheckPostgresqlRoleExists("role_with_search_path", nil, []string{"bar", "foo"}), + testAccCheckPostgresqlRoleExists("role_with_search_path", nil, []string{"bar", "foo-with-hyphen"}), // The int part in the attr name is the schema.HashString of the value. resource.TestCheckResourceAttr("postgresql_role.sub_role", "roles.719783566", "myrole2"), @@ -346,6 +346,9 @@ func checkSearchPath(client *Client, roleName string, expectedSearchPath []strin } searchPath := strings.Split(searchPathStr, ", ") + for i := range searchPath { + searchPath[i] = strings.Trim(searchPath[i], `"`) + } sort.Strings(expectedSearchPath) if !reflect.DeepEqual(searchPath, expectedSearchPath) { return fmt.Errorf( @@ -411,6 +414,6 @@ resource "postgresql_role" "sub_role" { resource "postgresql_role" "role_with_search_path" { name = "role_with_search_path" - search_path = ["bar", "foo"] + search_path = ["bar", "foo-with-hyphen"] } ` From 6c1fd9717cb6693460715f4adfb54a23bbac305a Mon Sep 17 00:00:00 2001 From: Krisztian Szabo Date: Thu, 19 Nov 2020 13:53:57 +0100 Subject: [PATCH 16/27] Set up Github Workflows --- .github/workflows/release.yml | 47 +++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 .github/workflows/release.yml diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml new file mode 100644 index 00000000..9776db75 --- /dev/null +++ b/.github/workflows/release.yml @@ -0,0 +1,47 @@ +# This GitHub action can publish assets for release when a tag is created. +# Currently its setup to run on any tag that matches the pattern "v*" (ie. v0.1.0). +# +# This uses an action (paultyng/ghaction-import-gpg) that assumes you set your +# private key in the `GPG_PRIVATE_KEY` secret and passphrase in the `PASSPHRASE` +# secret. If you would rather own your own GPG handling, please fork this action +# or use an alternative one for key handling. +# +# You will need to pass the `--batch` flag to `gpg` in your signing step +# in `goreleaser` to indicate this is being used in a non-interactive mode. +# +name: release +on: + push: + tags: + - 'v*' +jobs: + goreleaser: + runs-on: ubuntu-latest + steps: + - + name: Checkout + uses: actions/checkout@v2 + - + name: Unshallow + run: git fetch --prune --unshallow + - + name: Set up Go + uses: actions/setup-go@v2 + with: + go-version: 1.14 + - + name: Import GPG key + id: import_gpg + uses: paultyng/ghaction-import-gpg@v2.1.0 + env: + GPG_PRIVATE_KEY: ${{ secrets.GPG_PRIVATE_KEY }} + PASSPHRASE: ${{ secrets.PASSPHRASE }} + - + name: Run GoReleaser + uses: goreleaser/goreleaser-action@v2 + with: + version: latest + args: release --rm-dist + env: + GPG_FINGERPRINT: ${{ steps.import_gpg.outputs.fingerprint }} + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} From acaaf199e557f1b7e6a62593ded6fc5c9d4d6498 Mon Sep 17 00:00:00 2001 From: Cyril Gaudin Date: Thu, 26 Nov 2020 12:01:39 +0100 Subject: [PATCH 17/27] travis: Remove make website command It's hashicorp stuff that we don't have access to anymore. --- .travis.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index aca93180..8e3959c2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -23,7 +23,6 @@ install: script: - make test - make vet -- make website-test - make testacc branches: From 46715a51235891437c6017ca6fef20447fee5850 Mon Sep 17 00:00:00 2001 From: Cyril Gaudin Date: Thu, 26 Nov 2020 12:54:43 +0100 Subject: [PATCH 18/27] Prepare CHANGELOG for v1.8.0 --- CHANGELOG.md | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bf4f1c5..f821be5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,12 +1,32 @@ ## 1.8.0 (Unreleased) -## 1.7.2 (unreleased) + +FEATURES: + +* `postgresql_extension`: Support drop cascade. + ([#162](https://github.com/terraform-providers/terraform-provider-postgresql/pull/162) - @multani) + +* Use lazy connections. + ([#199](https://github.com/terraform-providers/terraform-provider-postgresql/pull/199) - @estahn) BUG FIXES: -* `postgresql_grant` : fix grant on function by removing prokind column selection, - as is it not available on postgresql version < 11 and its not to - expect to have a window(w) or aggregate function(a) with the same name as a normal function(f) - +* `postgresql_grant`: Fix grant on function by removing `prokind` column selection. + ([#171](https://github.com/terraform-providers/terraform-provider-postgresql/pull/171) - @Tommi2Day) + +DEV IMPROVEMENTS: + +* Set up Github Workflows to create releases. + ([#3](https://github.com/cyrilgdn/terraform-provider-postgresql/pull/3) - @thenonameguy) + +## 1.7.2 (July 30, 2020) + +This is the first release on [Terraform registry](https://registry.terraform.io/providers/cyrilgdn/postgresql/latest) + +DEV IMPROVEMENTS: + +* Add goreleaser config +* Pusblish on Terraform registry: https://registry.terraform.io/providers/cyrilgdn/postgresql/latest + ## 1.7.1 (July 30, 2020) BUG FIXES: From 7da0aeb2538609294126a5a22b713c239eee81c6 Mon Sep 17 00:00:00 2001 From: Cyril Gaudin Date: Thu, 26 Nov 2020 13:50:05 +0100 Subject: [PATCH 19/27] Release v1.8.0 --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f821be5c..a1711e96 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -## 1.8.0 (Unreleased) +## 1.8.0 (November 26, 2020) FEATURES: From d88b13b26b4f37bb3ef15b9977127ca43da2329a Mon Sep 17 00:00:00 2001 From: Cyril Gaudin Date: Thu, 26 Nov 2020 15:22:41 +0100 Subject: [PATCH 20/27] Revert "Merge pull request #199 from estahn/lazy-connections" This reverts commit 37e0b62e43667fd7a949d185495d39c252a3514a, reversing changes made to c8ecb25f9fbb3d200fb2d693d92831d92014e6e6. --- postgresql/config.go | 77 +++++++++++++++++++------------------------- 1 file changed, 34 insertions(+), 43 deletions(-) diff --git a/postgresql/config.go b/postgresql/config.go index aae9c9d1..371831b9 100644 --- a/postgresql/config.go +++ b/postgresql/config.go @@ -119,9 +119,43 @@ func (c *Config) NewClient(database string) (*Client, error) { dbRegistryLock.Lock() defer dbRegistryLock.Unlock() + dsn := c.connStr(database) + dbEntry, found := dbRegistry[dsn] + if !found { + db, err := sql.Open("postgres", dsn) + if err != nil { + return nil, fmt.Errorf("Error connecting to PostgreSQL server: %w", err) + } + + // We don't want to retain connection + // So when we connect on a specific database which might be managed by terraform, + // we don't keep opened connection in case of the db has to be dopped in the plan. + db.SetMaxIdleConns(0) + db.SetMaxOpenConns(c.MaxConns) + + defaultVersion, _ := semver.Parse(defaultExpectedPostgreSQLVersion) + version := &c.ExpectedVersion + if defaultVersion.Equals(c.ExpectedVersion) { + // Version hint not set by user, need to fingerprint + version, err = fingerprintCapabilities(db) + if err != nil { + db.Close() + return nil, fmt.Errorf("error detecting capabilities: %w", err) + } + } + + dbEntry = dbRegistryEntry{ + db: db, + version: *version, + } + dbRegistry[dsn] = dbEntry + } + client := Client{ config: *c, databaseName: database, + db: dbEntry.db, + version: dbEntry.version, } return &client, nil @@ -271,52 +305,9 @@ func (c *Config) getDatabaseUsername() string { // return their database resources. Use of QueryRow() or Exec() is encouraged. // Query() must have their rows.Close()'ed. func (c *Client) DB() *sql.DB { - c.connectDB() return c.db } -func (c *Client) connectDB() (*Client, error) { - dbRegistryLock.Lock() - defer dbRegistryLock.Unlock() - - dsn := c.config.connStr(c.databaseName) - dbEntry, found := dbRegistry[dsn] - if !found { - db, err := sql.Open("postgres", dsn) - if err != nil { - return nil, fmt.Errorf("Error connecting to PostgreSQL server: %w", err) - } - - // We don't want to retain connection - // So when we connect on a specific database which might be managed by terraform, - // we don't keep opened connection in case of the db has to be dopped in the plan. - db.SetMaxIdleConns(0) - db.SetMaxOpenConns(c.config.MaxConns) - - defaultVersion, _ := semver.Parse(defaultExpectedPostgreSQLVersion) - version := &c.config.ExpectedVersion - if defaultVersion.Equals(c.config.ExpectedVersion) { - // Version hint not set by user, need to fingerprint - version, err = fingerprintCapabilities(db) - if err != nil { - db.Close() - return nil, fmt.Errorf("error detecting capabilities: %w", err) - } - } - - dbEntry = dbRegistryEntry{ - db: db, - version: *version, - } - dbRegistry[dsn] = dbEntry - } - - c.db = dbEntry.db - c.version = dbEntry.version - - return nil, nil -} - // fingerprintCapabilities queries PostgreSQL to populate a local catalog of // capabilities. This is only run once per Client. func fingerprintCapabilities(db *sql.DB) (*semver.Version, error) { From 4465ed5ad2cf61e66901bab0bcf2d410ba86344d Mon Sep 17 00:00:00 2001 From: Cyril Gaudin Date: Thu, 26 Nov 2020 15:28:31 +0100 Subject: [PATCH 21/27] Bugfix release: 1.8.1 --- CHANGELOG.md | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a1711e96..b2c15522 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +## 1.8.1 (November 26, 2020) + +BUG FIXES: + +* Revert "Use lazy connections" [#199](https://github.com/terraform-providers/terraform-provider-postgresql/pull/199) + Plugin panics if not able to connect to the database. + ## 1.8.0 (November 26, 2020) FEATURES: @@ -5,8 +12,8 @@ FEATURES: * `postgresql_extension`: Support drop cascade. ([#162](https://github.com/terraform-providers/terraform-provider-postgresql/pull/162) - @multani) -* Use lazy connections. - ([#199](https://github.com/terraform-providers/terraform-provider-postgresql/pull/199) - @estahn) +* ~~Use lazy connections. + ([#199](https://github.com/terraform-providers/terraform-provider-postgresql/pull/199) - @estahn)~~ (Reverted in 1.8.1) BUG FIXES: From a3e8c914aaa3de9c0dc16ef1e0c6bd2f9ede7d47 Mon Sep 17 00:00:00 2001 From: Cyril Gaudin Date: Fri, 27 Nov 2020 10:23:37 +0100 Subject: [PATCH 22/27] README: Update documentation link. --- README.md | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index d71cb0b6..3448d17a 100644 --- a/README.md +++ b/README.md @@ -1,17 +1,18 @@ -Terraform Provider -================== +Terraform Provider for PostgreSQL +================================= -- Website: https://www.terraform.io -- [![Gitter chat](https://badges.gitter.im/hashicorp-terraform/Lobby.png)](https://gitter.im/hashicorp-terraform/Lobby) -- Mailing list: [Google Groups](http://groups.google.com/group/terraform-tool) +This provider allows to manage with Terraform [Postgresql](https://www.postgresql.org/) objects like databases, extensions, roles, etc.. - +It's published on the [Terraform registry](https://registry.terraform.io/providers/cyrilgdn/postgresql/latest/docs). +It replaces https://github.com/hashicorp/terraform-provider-postgresql since Hashicorp stopped hosting community providers in favor of the Terraform registry. + +- Documentation: https://registry.terraform.io/providers/cyrilgdn/postgresql/latest/docs Requirements ------------ -- [Terraform](https://www.terraform.io/downloads.html) 0.10.x -- [Go](https://golang.org/doc/install) 1.11 (to build the provider plugin) +- [Terraform](https://www.terraform.io/downloads.html) 0.12.x +- [Go](https://golang.org/doc/install) 1.14 (to build the provider plugin) Building The Provider --------------------- From 906dfb8c4f43187d734776cdc1f430a95fd37169 Mon Sep 17 00:00:00 2001 From: David Liao Date: Sun, 13 Dec 2020 16:52:00 -0800 Subject: [PATCH 23/27] code review --- go.mod | 1 - postgresql/resource_postgresql_grant_role.go | 35 ++++++------------- .../resource_postgresql_grant_role_test.go | 34 +++++++++--------- vendor/modules.txt | 1 - 4 files changed, 28 insertions(+), 43 deletions(-) diff --git a/go.mod b/go.mod index fd0a7dfb..d05d73a5 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,6 @@ go 1.14 require ( github.com/blang/semver v3.5.1+incompatible - github.com/hashicorp/errwrap v1.0.0 github.com/hashicorp/terraform-plugin-sdk v1.0.0 github.com/lib/pq v1.3.0 github.com/sean-/postgresql-acl v0.0.0-20161225120419-d10489e5d217 diff --git a/postgresql/resource_postgresql_grant_role.go b/postgresql/resource_postgresql_grant_role.go index a5f41d50..902a4621 100644 --- a/postgresql/resource_postgresql_grant_role.go +++ b/postgresql/resource_postgresql_grant_role.go @@ -7,7 +7,6 @@ import ( "strconv" "strings" - "github.com/hashicorp/errwrap" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/lib/pq" ) @@ -70,13 +69,7 @@ func resourcePostgreSQLGrantRoleRead(d *schema.ResourceData, meta interface{}) e client.catalogLock.RLock() defer client.catalogLock.RUnlock() - txn, err := startTransaction(client, "") - if err != nil { - return err - } - defer deferredRollback(txn) - - return readGrantRole(txn, d) + return readGrantRole(client.DB(), d) } func resourcePostgreSQLGrantRoleCreate(d *schema.ResourceData, meta interface{}) error { @@ -108,18 +101,12 @@ func resourcePostgreSQLGrantRoleCreate(d *schema.ResourceData, meta interface{}) } if err = txn.Commit(); err != nil { - return errwrap.Wrapf("could not commit transaction: {{err}}", err) + return fmt.Errorf("could not commit transaction: %w", err) } d.SetId(generateGrantRoleID(d)) - txn, err = startTransaction(client, "") - if err != nil { - return err - } - defer deferredRollback(txn) - - return readGrantRole(txn, d) + return readGrantRole(client.DB(), d) } func resourcePostgreSQLGrantRoleDelete(d *schema.ResourceData, meta interface{}) error { @@ -146,17 +133,17 @@ func resourcePostgreSQLGrantRoleDelete(d *schema.ResourceData, meta interface{}) } if err = txn.Commit(); err != nil { - return errwrap.Wrapf("could not commit transaction: {{err}}", err) + return fmt.Errorf("could not commit transaction: %w", err) } return nil } -func readGrantRole(txn *sql.Tx, d *schema.ResourceData) error { +func readGrantRole(db QueryAble, d *schema.ResourceData) error { var roleName, grantRoleName string var withAdminOption bool - grantRoleId := d.Id() + grantRoleID := d.Id() values := []interface{}{ &roleName, @@ -164,14 +151,14 @@ func readGrantRole(txn *sql.Tx, d *schema.ResourceData) error { &withAdminOption, } - err := txn.QueryRow(getGrantRoleQuery, d.Get("role"), d.Get("grant_role")).Scan(values...) + err := db.QueryRow(getGrantRoleQuery, d.Get("role"), d.Get("grant_role")).Scan(values...) switch { case err == sql.ErrNoRows: - log.Printf("[WARN] PostgreSQL grant role (%q) not found", grantRoleId) + log.Printf("[WARN] PostgreSQL grant role (%q) not found", grantRoleID) d.SetId("") return nil case err != nil: - return errwrap.Wrapf("Error reading grant role: {{err}}", err) + return fmt.Errorf("Error reading grant role: %w", err) } d.Set("role", roleName) @@ -213,7 +200,7 @@ func createRevokeRoleQuery(d *schema.ResourceData) string { func grantRole(txn *sql.Tx, d *schema.ResourceData) error { query := createGrantRoleQuery(d) if _, err := txn.Exec(query); err != nil { - return errwrap.Wrapf("could not execute grant query: {{err}}", err) + return fmt.Errorf("could not execute grant query: %w", err) } return nil } @@ -221,7 +208,7 @@ func grantRole(txn *sql.Tx, d *schema.ResourceData) error { func revokeRole(txn *sql.Tx, d *schema.ResourceData) error { query := createRevokeRoleQuery(d) if _, err := txn.Exec(query); err != nil { - return errwrap.Wrapf("could not execute revoke query: {{err}}", err) + return fmt.Errorf("could not execute revoke query: %w", err) } return nil } diff --git a/postgresql/resource_postgresql_grant_role_test.go b/postgresql/resource_postgresql_grant_role_test.go index 2badbbf1..abf278bb 100644 --- a/postgresql/resource_postgresql_grant_role_test.go +++ b/postgresql/resource_postgresql_grant_role_test.go @@ -58,40 +58,39 @@ func TestRevokeRoleQuery(t *testing.T) { var roleName = "foo" var grantRoleName = "bar" + expected := fmt.Sprintf("REVOKE %s FROM %s", pq.QuoteIdentifier(grantRoleName), pq.QuoteIdentifier(roleName)) + cases := []struct { - resource *schema.ResourceData + resource map[string]interface{} privileges []string expected string }{ { - resource: schema.TestResourceDataRaw(t, resourcePostgreSQLGrantRole().Schema, map[string]interface{}{ + resource: map[string]interface{}{ "role": roleName, "grant_role": grantRoleName, - }), - expected: fmt.Sprintf("REVOKE %s FROM %s", pq.QuoteIdentifier(grantRoleName), pq.QuoteIdentifier(roleName)), + }, }, { - resource: schema.TestResourceDataRaw(t, resourcePostgreSQLGrantRole().Schema, map[string]interface{}{ + resource: map[string]interface{}{ "role": roleName, "grant_role": grantRoleName, "with_admin_option": false, - }), - expected: fmt.Sprintf("REVOKE %s FROM %s", pq.QuoteIdentifier(grantRoleName), pq.QuoteIdentifier(roleName)), + }, }, { - resource: schema.TestResourceDataRaw(t, resourcePostgreSQLGrantRole().Schema, map[string]interface{}{ + resource: map[string]interface{}{ "role": roleName, "grant_role": grantRoleName, "with_admin_option": false, - }), - expected: fmt.Sprintf("REVOKE %s FROM %s", pq.QuoteIdentifier(grantRoleName), pq.QuoteIdentifier(roleName)), + }, }, } for _, c := range cases { - out := createRevokeRoleQuery(c.resource) - if out != c.expected { - t.Fatalf("Error matching output and expected: %#v vs %#v", out, c.expected) + out := createRevokeRoleQuery(schema.TestResourceDataRaw(t, resourcePostgreSQLGrantRole().Schema, c.resource)) + if out != expected { + t.Fatalf("Error matching output and expected: %#v vs %#v", out, expected) } } } @@ -108,16 +107,17 @@ func TestAccPostgresqlGrantRole(t *testing.T) { _, roleName := getTestDBNames(dbSuffix) grantedRoleName := "foo" - teardownGrantedRole := createTestRole(t, grantedRoleName) - defer teardownGrantedRole() testAccPostgresqlGrantRoleResources := fmt.Sprintf(` + resource postgresql_role "grant" { + name = "%s" + } resource postgresql_grant_role "grant_role" { role = "%s" - grant_role = "%s" + grant_role = postgresql_role.grant.name with_admin_option = true } - `, roleName, grantedRoleName) + `, grantedRoleName, roleName) resource.Test(t, resource.TestCase{ PreCheck: func() { diff --git a/vendor/modules.txt b/vendor/modules.txt index 2d2d0c48..9f5d4cdf 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -79,7 +79,6 @@ github.com/google/uuid # github.com/googleapis/gax-go/v2 v2.0.5 github.com/googleapis/gax-go/v2 # github.com/hashicorp/errwrap v1.0.0 -## explicit github.com/hashicorp/errwrap # github.com/hashicorp/go-cleanhttp v0.5.1 github.com/hashicorp/go-cleanhttp From f1ea383fdf1c563596b53f21752bbb052b88af65 Mon Sep 17 00:00:00 2001 From: David Liao Date: Sun, 13 Dec 2020 16:54:45 -0800 Subject: [PATCH 24/27] code review --- .../resource_postgresql_grant_role_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/postgresql/resource_postgresql_grant_role_test.go b/postgresql/resource_postgresql_grant_role_test.go index abf278bb..4a1c50b4 100644 --- a/postgresql/resource_postgresql_grant_role_test.go +++ b/postgresql/resource_postgresql_grant_role_test.go @@ -17,37 +17,37 @@ func TestCreateGrantRoleQuery(t *testing.T) { var grantRoleName = "bar" cases := []struct { - resource *schema.ResourceData + resource map[string]interface{} privileges []string expected string }{ { - resource: schema.TestResourceDataRaw(t, resourcePostgreSQLGrantRole().Schema, map[string]interface{}{ + resource: map[string]interface{}{ "role": roleName, "grant_role": grantRoleName, - }), + }, expected: fmt.Sprintf("GRANT %s TO %s", pq.QuoteIdentifier(grantRoleName), pq.QuoteIdentifier(roleName)), }, { - resource: schema.TestResourceDataRaw(t, resourcePostgreSQLGrantRole().Schema, map[string]interface{}{ + resource: map[string]interface{}{ "role": roleName, "grant_role": grantRoleName, "with_admin_option": false, - }), + }, expected: fmt.Sprintf("GRANT %s TO %s", pq.QuoteIdentifier(grantRoleName), pq.QuoteIdentifier(roleName)), }, { - resource: schema.TestResourceDataRaw(t, resourcePostgreSQLGrantRole().Schema, map[string]interface{}{ + resource: map[string]interface{}{ "role": roleName, "grant_role": grantRoleName, "with_admin_option": true, - }), + }, expected: fmt.Sprintf("GRANT %s TO %s WITH ADMIN OPTION", pq.QuoteIdentifier(grantRoleName), pq.QuoteIdentifier(roleName)), }, } for _, c := range cases { - out := createGrantRoleQuery(c.resource) + out := createGrantRoleQuery(schema.TestResourceDataRaw(t, resourcePostgreSQLGrantRole().Schema, c.resource)) if out != c.expected { t.Fatalf("Error matching output and expected: %#v vs %#v", out, c.expected) } From 4c9f394c2262848d92014b74a18ea58777b4ee30 Mon Sep 17 00:00:00 2001 From: David Liao Date: Sun, 13 Dec 2020 16:55:23 -0800 Subject: [PATCH 25/27] minor --- postgresql/resource_postgresql_grant_role_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/postgresql/resource_postgresql_grant_role_test.go b/postgresql/resource_postgresql_grant_role_test.go index 4a1c50b4..5fadfcbe 100644 --- a/postgresql/resource_postgresql_grant_role_test.go +++ b/postgresql/resource_postgresql_grant_role_test.go @@ -63,7 +63,6 @@ func TestRevokeRoleQuery(t *testing.T) { cases := []struct { resource map[string]interface{} privileges []string - expected string }{ { resource: map[string]interface{}{ From 021f0011fcbec99d11e16493b86806cc738d4944 Mon Sep 17 00:00:00 2001 From: David Liao Date: Sun, 13 Dec 2020 16:56:27 -0800 Subject: [PATCH 26/27] minor --- postgresql/resource_postgresql_grant_role_test.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/postgresql/resource_postgresql_grant_role_test.go b/postgresql/resource_postgresql_grant_role_test.go index 5fadfcbe..db61fc3d 100644 --- a/postgresql/resource_postgresql_grant_role_test.go +++ b/postgresql/resource_postgresql_grant_role_test.go @@ -17,9 +17,8 @@ func TestCreateGrantRoleQuery(t *testing.T) { var grantRoleName = "bar" cases := []struct { - resource map[string]interface{} - privileges []string - expected string + resource map[string]interface{} + expected string }{ { resource: map[string]interface{}{ @@ -61,8 +60,7 @@ func TestRevokeRoleQuery(t *testing.T) { expected := fmt.Sprintf("REVOKE %s FROM %s", pq.QuoteIdentifier(grantRoleName), pq.QuoteIdentifier(roleName)) cases := []struct { - resource map[string]interface{} - privileges []string + resource map[string]interface{} }{ { resource: map[string]interface{}{ From b9b2d657fe825416697e7773e65fc6240513031d Mon Sep 17 00:00:00 2001 From: David Liao Date: Mon, 14 Dec 2020 10:45:00 -0800 Subject: [PATCH 27/27] cr --- postgresql/resource_postgresql_grant_role_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/postgresql/resource_postgresql_grant_role_test.go b/postgresql/resource_postgresql_grant_role_test.go index db61fc3d..3f444f8c 100644 --- a/postgresql/resource_postgresql_grant_role_test.go +++ b/postgresql/resource_postgresql_grant_role_test.go @@ -79,7 +79,7 @@ func TestRevokeRoleQuery(t *testing.T) { resource: map[string]interface{}{ "role": roleName, "grant_role": grantRoleName, - "with_admin_option": false, + "with_admin_option": true, }, }, }