From 2fa78afdecc08e63ac5a09bc6b47716923fa5cf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Menassa?= Date: Thu, 5 Dec 2019 16:53:06 +0100 Subject: [PATCH 1/4] [add] set statement_timeout to role --- postgresql/helpers.go | 8 ++++++ postgresql/resource_postgresql_role.go | 31 ++++++++++++++++++++- postgresql/resource_postgresql_role_test.go | 6 ++++ 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/postgresql/helpers.go b/postgresql/helpers.go index 7dc96a29..86f42e79 100644 --- a/postgresql/helpers.go +++ b/postgresql/helpers.go @@ -37,6 +37,14 @@ func validateConnLimit(v interface{}, key string) (warnings []string, errors []e return } +func validateStatementTimeout(v interface{}, key string) (warnings []string, errors []error) { + value := v.(int) + if value < 0 { + errors = append(errors, fmt.Errorf("%s can not be less than 0", key)) + } + return +} + func isRoleMember(db QueryAble, role, member string) (bool, error) { var _rez int err := db.QueryRow( diff --git a/postgresql/resource_postgresql_role.go b/postgresql/resource_postgresql_role.go index 94795a90..629cff89 100644 --- a/postgresql/resource_postgresql_role.go +++ b/postgresql/resource_postgresql_role.go @@ -31,6 +31,7 @@ const ( roleValidUntilAttr = "valid_until" roleRolesAttr = "roles" roleSearchPathAttr = "search_path" + roleStatementTimeoutAttr = "statement_timeout" // Deprecated options roleDepEncryptedAttr = "encrypted" @@ -152,6 +153,13 @@ func resourcePostgreSQLRole() *schema.Resource { Default: false, Description: "Skip actually running the REASSIGN OWNED command when removing a role from PostgreSQL", }, + roleStatementTimeoutAttr: { + Type: schema.TypeInt, + Optional: true, + Default: 0, + Description: "Abort any statement that takes more than the specified number of milliseconds", + ValidateFunc: validateStatementTimeout, + }, }, } } @@ -361,7 +369,7 @@ func resourcePostgreSQLRoleRead(d *schema.ResourceData, meta interface{}) error } func resourcePostgreSQLRoleReadImpl(c *Client, d *schema.ResourceData) error { - var roleSuperuser, roleInherit, roleCreateRole, roleCreateDB, roleCanLogin, roleReplication, roleBypassRLS bool + var roleSuperuser, roleInherit, roleCreateRole, roleCreateDB, roleCanLogin, roleReplication, roleBypassRLS, roleStatementTimeout bool var roleConnLimit int var roleName, roleValidUntil string var roleRoles, roleConfig pq.ByteaArray @@ -391,6 +399,7 @@ func resourcePostgreSQLRoleReadImpl(c *Client, d *schema.ResourceData) error { &roleConnLimit, &roleValidUntil, &roleConfig, + &roleStatementTimeout, } if c.featureSupported(featureReplication) { @@ -436,6 +445,7 @@ func resourcePostgreSQLRoleReadImpl(c *Client, d *schema.ResourceData) error { d.Set(roleReplicationAttr, roleBypassRLS) d.Set(roleRolesAttr, pgArrayToSet(roleRoles)) d.Set(roleSearchPathAttr, readSearchPath(roleConfig)) + d.Set(roleStatementTimeoutAttr, roleStatementTimeout) d.SetId(roleName) @@ -582,6 +592,10 @@ func resourcePostgreSQLRoleUpdate(d *schema.ResourceData, meta interface{}) erro return err } + if err = setStatementTimeout(txn, d); err != nil { + return err + } + if err = txn.Commit(); err != nil { return errwrap.Wrapf("could not commit transaction: {{err}}", err) } @@ -880,3 +894,18 @@ func alterSearchPath(txn *sql.Tx, d *schema.ResourceData) error { } return nil } + +func setStatementTimeout(txn *sql.Tx, d *schema.ResourceData) error { + if !d.HasChange(roleStatementTimeoutAttr) { + return nil + } + + statementTimeout := d.Get(roleStatementTimeoutAttr).(int) + roleName := d.Get(roleNameAttr).(string) + sql := fmt.Sprintf("ALTER ROLE %s SET statement_timeout TO %d", pq.QuoteIdentifier(roleName), statementTimeout) + if _, err := txn.Exec(sql); err != nil { + return errwrap.Wrapf("Error updating role STATEMENT TIMEOUT: {{err}}", err) + } + + return nil +} diff --git a/postgresql/resource_postgresql_role_test.go b/postgresql/resource_postgresql_role_test.go index 26156b48..9b361c06 100644 --- a/postgresql/resource_postgresql_role_test.go +++ b/postgresql/resource_postgresql_role_test.go @@ -43,6 +43,7 @@ func TestAccPostgresqlRole_Basic(t *testing.T) { resource.TestCheckResourceAttr("postgresql_role.role_with_defaults", "valid_until", "infinity"), resource.TestCheckResourceAttr("postgresql_role.role_with_defaults", "skip_drop_role", "false"), resource.TestCheckResourceAttr("postgresql_role.role_with_defaults", "skip_reassign_owned", "false"), + resource.TestCheckResourceAttr("postgresql_role.role_with_defaults", "statement_timeout", "0"), resource.TestCheckResourceAttr("postgresql_role.role_with_create_database", "name", "role_with_create_database"), resource.TestCheckResourceAttr("postgresql_role.role_with_create_database", "create_database", "true"), @@ -88,6 +89,7 @@ resource "postgresql_role" "update_role" { password = "titi" roles = ["${postgresql_role.group_role.name}"] search_path = ["mysearchpath"] + statement_timeout = 30000 } ` resource.Test(t, resource.TestCase{ @@ -109,6 +111,7 @@ resource "postgresql_role" "update_role" { resource.TestCheckResourceAttr("postgresql_role.update_role", "valid_until", "2099-05-04 12:00:00+00"), resource.TestCheckResourceAttr("postgresql_role.update_role", "roles.#", "0"), resource.TestCheckResourceAttr("postgresql_role.update_role", "search_path.#", "0"), + resource.TestCheckResourceAttr("postgresql_role.update_role", "statement_timeout", "0"), testAccCheckRoleCanLogin(t, "update_role", "toto"), ), }, @@ -130,6 +133,7 @@ resource "postgresql_role" "update_role" { ), resource.TestCheckResourceAttr("postgresql_role.update_role", "search_path.#", "1"), resource.TestCheckResourceAttr("postgresql_role.update_role", "search_path.0", "mysearchpath"), + resource.TestCheckResourceAttr("postgresql_role.update_role", "statement_timeout", "30000"), testAccCheckRoleCanLogin(t, "update_role2", "titi"), ), }, @@ -145,6 +149,7 @@ resource "postgresql_role" "update_role" { resource.TestCheckResourceAttr("postgresql_role.update_role", "password", "toto"), resource.TestCheckResourceAttr("postgresql_role.update_role", "roles.#", "0"), resource.TestCheckResourceAttr("postgresql_role.update_role", "search_path.#", "0"), + resource.TestCheckResourceAttr("postgresql_role.update_role", "statement_timeout", "0"), testAccCheckRoleCanLogin(t, "update_role", "toto"), ), }, @@ -323,6 +328,7 @@ resource "postgresql_role" "role_with_defaults" { skip_drop_role = false skip_reassign_owned = false valid_until = "infinity" + statement_timeout = 0 } resource "postgresql_role" "role_with_create_database" { From 2813a22000dcfca37501dd794d0ba9b05b6df406 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Menassa?= Date: Fri, 6 Dec 2019 15:45:41 +0100 Subject: [PATCH 2/4] [fix] tests --- postgresql/resource_postgresql_role.go | 39 ++++++++++++++++++++------ 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/postgresql/resource_postgresql_role.go b/postgresql/resource_postgresql_role.go index 629cff89..20bcdd87 100644 --- a/postgresql/resource_postgresql_role.go +++ b/postgresql/resource_postgresql_role.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "log" + "strconv" "strings" "github.com/hashicorp/errwrap" @@ -156,7 +157,6 @@ func resourcePostgreSQLRole() *schema.Resource { roleStatementTimeoutAttr: { Type: schema.TypeInt, Optional: true, - Default: 0, Description: "Abort any statement that takes more than the specified number of milliseconds", ValidateFunc: validateStatementTimeout, }, @@ -290,6 +290,10 @@ func resourcePostgreSQLRoleCreate(d *schema.ResourceData, meta interface{}) erro return err } + if err = setStatementTimeout(txn, d); err != nil { + return err + } + if err = txn.Commit(); err != nil { return errwrap.Wrapf("could not commit transaction: {{err}}", err) } @@ -369,7 +373,7 @@ func resourcePostgreSQLRoleRead(d *schema.ResourceData, meta interface{}) error } func resourcePostgreSQLRoleReadImpl(c *Client, d *schema.ResourceData) error { - var roleSuperuser, roleInherit, roleCreateRole, roleCreateDB, roleCanLogin, roleReplication, roleBypassRLS, roleStatementTimeout bool + var roleSuperuser, roleInherit, roleCreateRole, roleCreateDB, roleCanLogin, roleReplication, roleBypassRLS bool var roleConnLimit int var roleName, roleValidUntil string var roleRoles, roleConfig pq.ByteaArray @@ -399,7 +403,6 @@ func resourcePostgreSQLRoleReadImpl(c *Client, d *schema.ResourceData) error { &roleConnLimit, &roleValidUntil, &roleConfig, - &roleStatementTimeout, } if c.featureSupported(featureReplication) { @@ -445,7 +448,7 @@ func resourcePostgreSQLRoleReadImpl(c *Client, d *schema.ResourceData) error { d.Set(roleReplicationAttr, roleBypassRLS) d.Set(roleRolesAttr, pgArrayToSet(roleRoles)) d.Set(roleSearchPathAttr, readSearchPath(roleConfig)) - d.Set(roleStatementTimeoutAttr, roleStatementTimeout) + d.Set(roleStatementTimeoutAttr, readStatementTimeout(roleConfig)) d.SetId(roleName) @@ -464,12 +467,30 @@ func readSearchPath(roleConfig pq.ByteaArray) []string { for _, v := range roleConfig { config := string(v) if strings.HasPrefix(config, roleSearchPathAttr) { - return strings.Split(strings.TrimPrefix(config, roleSearchPathAttr+"="), ", ") + var result = strings.Split(strings.TrimPrefix(config, roleSearchPathAttr+"="), ", ") + return result } } return nil } +// readStatementTimeout searches for a statement_timeout entry in the rolconfig array. +// In case no such value is present, it returns nil. +func readStatementTimeout(roleConfig pq.ByteaArray) int { + for _, v := range roleConfig { + config := string(v) + if strings.HasPrefix(config, roleStatementTimeoutAttr) { + var result = strings.Split(strings.TrimPrefix(config, roleStatementTimeoutAttr+"="), ", ") + res, err := strconv.Atoi(result[0]) + if err != nil { + fmt.Println(err) + } + return res + } + } + return 0 +} + // readRolePassword reads password either from Postgres if admin user is a superuser // or only from Terraform state. func readRolePassword(c *Client, d *schema.ResourceData, roleCanLogin bool) (string, error) { @@ -900,11 +921,13 @@ func setStatementTimeout(txn *sql.Tx, d *schema.ResourceData) error { return nil } - statementTimeout := d.Get(roleStatementTimeoutAttr).(int) roleName := d.Get(roleNameAttr).(string) - sql := fmt.Sprintf("ALTER ROLE %s SET statement_timeout TO %d", pq.QuoteIdentifier(roleName), statementTimeout) + statementTimeout := d.Get(roleStatementTimeoutAttr).(int) + sql := fmt.Sprintf( + "ALTER ROLE %s SET statement_timeout TO %d", pq.QuoteIdentifier(roleName), statementTimeout, + ) if _, err := txn.Exec(sql); err != nil { - return errwrap.Wrapf("Error updating role STATEMENT TIMEOUT: {{err}}", err) + return errwrap.Wrapf(fmt.Sprintf("could not set statementtimeout %d for %s: {{err}}", statementTimeout, roleName), err) } return nil From bd07b518a368a03bfd907d188d373ae26f0b5ea4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Menassa?= Date: Mon, 23 Dec 2019 17:45:46 +0100 Subject: [PATCH 3/4] [enh] return error & handle reset statement_timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Cédric Menassa --- postgresql/resource_postgresql_role.go | 36 ++++++++++++++++++-------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/postgresql/resource_postgresql_role.go b/postgresql/resource_postgresql_role.go index 20bcdd87..96092e0c 100644 --- a/postgresql/resource_postgresql_role.go +++ b/postgresql/resource_postgresql_role.go @@ -448,7 +448,13 @@ func resourcePostgreSQLRoleReadImpl(c *Client, d *schema.ResourceData) error { d.Set(roleReplicationAttr, roleBypassRLS) d.Set(roleRolesAttr, pgArrayToSet(roleRoles)) d.Set(roleSearchPathAttr, readSearchPath(roleConfig)) - d.Set(roleStatementTimeoutAttr, readStatementTimeout(roleConfig)) + + statementTimeout, err := readStatementTimeout(roleConfig) + if err != nil { + return err + } + + d.Set(roleStatementTimeoutAttr, statementTimeout) d.SetId(roleName) @@ -476,19 +482,19 @@ func readSearchPath(roleConfig pq.ByteaArray) []string { // readStatementTimeout searches for a statement_timeout entry in the rolconfig array. // In case no such value is present, it returns nil. -func readStatementTimeout(roleConfig pq.ByteaArray) int { +func readStatementTimeout(roleConfig pq.ByteaArray) (int, error) { for _, v := range roleConfig { config := string(v) if strings.HasPrefix(config, roleStatementTimeoutAttr) { var result = strings.Split(strings.TrimPrefix(config, roleStatementTimeoutAttr+"="), ", ") res, err := strconv.Atoi(result[0]) if err != nil { - fmt.Println(err) + return -1, err } - return res + return res, nil } } - return 0 + return 0, nil } // readRolePassword reads password either from Postgres if admin user is a superuser @@ -923,12 +929,20 @@ func setStatementTimeout(txn *sql.Tx, d *schema.ResourceData) error { roleName := d.Get(roleNameAttr).(string) statementTimeout := d.Get(roleStatementTimeoutAttr).(int) - sql := fmt.Sprintf( - "ALTER ROLE %s SET statement_timeout TO %d", pq.QuoteIdentifier(roleName), statementTimeout, - ) - if _, err := txn.Exec(sql); err != nil { - return errwrap.Wrapf(fmt.Sprintf("could not set statementtimeout %d for %s: {{err}}", statementTimeout, roleName), err) + if statementTimeout != 0 { + sql := fmt.Sprintf( + "ALTER ROLE %s SET statement_timeout TO %d", pq.QuoteIdentifier(roleName), statementTimeout, + ) + if _, err := txn.Exec(sql); err != nil { + return errwrap.Wrapf(fmt.Sprintf("could not set statement_timeout %d for %s: {{err}}", statementTimeout, roleName), err) + } + } else { + sql := fmt.Sprintf( + "ALTER ROLE %s RESET statement_timeout", pq.QuoteIdentifier(roleName), + ) + if _, err := txn.Exec(sql); err != nil { + return errwrap.Wrapf(fmt.Sprintf("could not reset statement_timeout for %s: {{err}}", roleName), err) + } } - return nil } From 3cd9a6b404e4b6fd458218aa8188928c4198a478 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Menassa?= Date: Thu, 26 Dec 2019 19:10:34 +0100 Subject: [PATCH 4/4] [enh] wrap error in reading statement_timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Cédric Menassa --- postgresql/resource_postgresql_role.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/postgresql/resource_postgresql_role.go b/postgresql/resource_postgresql_role.go index 96092e0c..ff6c4951 100644 --- a/postgresql/resource_postgresql_role.go +++ b/postgresql/resource_postgresql_role.go @@ -489,7 +489,7 @@ func readStatementTimeout(roleConfig pq.ByteaArray) (int, error) { var result = strings.Split(strings.TrimPrefix(config, roleStatementTimeoutAttr+"="), ", ") res, err := strconv.Atoi(result[0]) if err != nil { - return -1, err + return -1, errwrap.Wrapf("Error reading statement_timeout: {{err}}", err) } return res, nil }