From 6472ad164340c2d18028aad52deba39c881caa6d Mon Sep 17 00:00:00 2001 From: librucha Date: Sat, 12 Oct 2024 09:49:28 +0200 Subject: [PATCH] fix(postgresql_role): Postgresql 16 compatibility (with new roles management) (#407) --- .github/workflows/golangci.yml | 4 +- .github/workflows/release.yml | 2 +- .github/workflows/test.yml | 4 +- .gitignore | 154 ++++++++++- README.md | 2 +- examples/issues/407/dev.tfrc | 13 + examples/issues/407/test.tf | 25 ++ go.mod | 2 +- postgresql/config.go | 5 + postgresql/provider.go | 37 ++- postgresql/provider_test.go | 31 +++ .../resource_postgresql_database_test.go | 13 +- ...urce_postgresql_default_privileges_test.go | 17 +- postgresql/resource_postgresql_role.go | 95 +++++++ postgresql/resource_postgresql_role_test.go | 251 ++++++++++++++++-- postgresql/utils_test.go | 42 ++- tests/docker-compose.yml | 2 - tests/switch_rds.sh | 27 +- website/docs/index.html.markdown | 12 + website/docs/r/postgresql_role.html.markdown | 3 + 20 files changed, 677 insertions(+), 64 deletions(-) create mode 100644 examples/issues/407/dev.tfrc create mode 100644 examples/issues/407/test.tf diff --git a/.github/workflows/golangci.yml b/.github/workflows/golangci.yml index b52b2499..13ff1c64 100644 --- a/.github/workflows/golangci.yml +++ b/.github/workflows/golangci.yml @@ -15,11 +15,11 @@ jobs: - name: Setup Go uses: actions/setup-go@v4 with: - go-version: 1.20.x + go-version: 1.23.x - run: go mod vendor - name: Run golangci-lint uses: golangci/golangci-lint-action@v3 with: - version: v1.53 + version: v1.61 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 43738d29..d56d8cc8 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -28,7 +28,7 @@ jobs: name: Set up Go uses: actions/setup-go@v2 with: - go-version: '1.20' + go-version: '1.23' - name: Import GPG key id: import_gpg diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9d1b63bc..8ed64cdb 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -12,7 +12,7 @@ jobs: strategy: matrix: - pgversion: [15, 14, 13, 12, 11] + pgversion: [16, 15, 14, 13, 12, 11] env: PGVERSION: ${{ matrix.pgversion }} @@ -24,7 +24,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v4 with: - go-version: '1.20' + go-version: '1.23' - name: test run: make test diff --git a/.gitignore b/.gitignore index 68f7ab57..ed20777d 100644 --- a/.gitignore +++ b/.gitignore @@ -1,7 +1,149 @@ -./*.tfstate -.terraform/ -.terraform.lock.hcl* -*.log -.*.swp -tests/docker-compose.*.yml terraform-provider-postgresql + +### JetBrains+all template +# Covers JetBrains IDEs: IntelliJ, RubyMine, PhpStorm, AppCode, PyCharm, CLion, Android Studio, WebStorm and Rider +# Reference: https://intellij-support.jetbrains.com/hc/en-us/articles/206544839 + +# User-specific stuff +.idea/**/workspace.xml +.idea/**/tasks.xml +.idea/**/usage.statistics.xml +.idea/**/dictionaries +.idea/**/shelf + +# AWS User-specific +.idea/**/aws.xml + +# Generated files +.idea/**/contentModel.xml + +# Sensitive or high-churn files +.idea/**/dataSources/ +.idea/**/dataSources.ids +.idea/**/dataSources.local.xml +.idea/**/sqlDataSources.xml +.idea/**/dynamic.xml +.idea/**/uiDesigner.xml +.idea/**/dbnavigator.xml + +# Gradle +.idea/**/gradle.xml +.idea/**/libraries + +# Gradle and Maven with auto-import +# When using Gradle or Maven with auto-import, you should exclude module files, +# since they will be recreated, and may cause churn. Uncomment if using +# auto-import. +# .idea/artifacts +# .idea/compiler.xml +# .idea/jarRepositories.xml +# .idea/modules.xml +# .idea/*.iml +# .idea/modules +# *.iml +# *.ipr + +# CMake +cmake-build-*/ + +# Mongo Explorer plugin +.idea/**/mongoSettings.xml + +# File-based project format +*.iws + +# IntelliJ +out/ + +# mpeltonen/sbt-idea plugin +.idea_modules/ + +# JIRA plugin +atlassian-ide-plugin.xml + +# Cursive Clojure plugin +.idea/replstate.xml + +# SonarLint plugin +.idea/sonarlint/ + +# Crashlytics plugin (for Android Studio and IntelliJ) +com_crashlytics_export_strings.xml +crashlytics.properties +crashlytics-build.properties +fabric.properties + +# Editor-based Rest Client +.idea/httpRequests + +# Android studio 3.1+ serialized cache file +.idea/caches/build_file_checksums.ser + + +### Linux template +*~ + +# temporary files which can be created if a process still has a handle open of a deleted file +.fuse_hidden* + +# KDE directory preferences +.directory + +# Linux trash folder which might appear on any partition or disk +.Trash-* + +# .nfs files are created when an open file is removed but is still being accessed +.nfs* + +### Go template +# If you prefer the allow list template instead of the deny list, see community template: +# https://github.com/github/gitignore/blob/main/community/Golang/Go.AllowList.gitignore +# +# Binaries for programs and plugins +*.exe +*.exe~ +*.dll +*.so +*.dylib + +# Test binary, built with `go test -c` +*.test + +# Output of the go coverage tool, specifically when used with LiteIDE +*.out + +# Dependency directories (remove the comment below to include it) +# vendor/ + +# Go workspace file +go.work + +### Windows template +# Windows thumbnail cache files +Thumbs.db +Thumbs.db:encryptable +ehthumbs.db +ehthumbs_vista.db + +# Dump file +*.stackdump + +# Folder config file +[Dd]esktop.ini + +# Recycle Bin used on file shares +$RECYCLE.BIN/ + +# Windows Installer files +*.cab +*.msi +*.msix +*.msm +*.msp + +# Windows shortcuts +*.lnk + +.terraform +.terraform.lock.hcl +terraform.tfstate* \ No newline at end of file diff --git a/README.md b/README.md index 7df471fb..0014327e 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ Requirements ------------ - [Terraform](https://www.terraform.io/downloads.html) 0.12.x -- [Go](https://golang.org/doc/install) 1.16 (to build the provider plugin) +- [Go](https://golang.org/doc/install) 1.21 (to build the provider plugin) Building The Provider --------------------- diff --git a/examples/issues/407/dev.tfrc b/examples/issues/407/dev.tfrc new file mode 100644 index 00000000..8d61e525 --- /dev/null +++ b/examples/issues/407/dev.tfrc @@ -0,0 +1,13 @@ +# See https://www.terraform.io/cli/config/config-file#development-overrides-for-provider-developers +# Use `go build -o ./examples/issues/407/postgresql/terraform-provider-postgresql` in the project root to build the provider. +# Then run terraform in this example directory. + +# terraform init +# TF_CLI_CONFIG_FILE=dev.tfrc terraform apply + +provider_installation { + dev_overrides { + "cyrilgdn/postgresql" = "./postgresql" + } + direct {} +} diff --git a/examples/issues/407/test.tf b/examples/issues/407/test.tf new file mode 100644 index 00000000..e292259a --- /dev/null +++ b/examples/issues/407/test.tf @@ -0,0 +1,25 @@ +terraform { + required_version = ">= 1.0" + + required_providers { + postgresql = { + source = "cyrilgdn/postgresql" + version = "~>1" + } + } +} + +provider "postgresql" { + superuser = false + port = 25432 + username = "rds" + password = "rds" + sslmode = "disable" +} + +resource "postgresql_role" "test_role_with_createrole_self_grant" { + name = "test_role_with_createrole_self_grant" + parameters { + createrole_self_grant = "set,inherit" + } +} diff --git a/go.mod b/go.mod index 5d4290d5..3203b7f5 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/terraform-providers/terraform-provider-postgresql -go 1.20 +go 1.23 require ( github.com/Azure/azure-sdk-for-go/sdk/azcore v1.11.1 diff --git a/postgresql/config.go b/postgresql/config.go index 17668c0e..0b225323 100644 --- a/postgresql/config.go +++ b/postgresql/config.go @@ -44,6 +44,7 @@ const ( featurePubWithoutTruncate featureFunction featureServer + featureCreateRoleSelfGrant ) var ( @@ -115,6 +116,10 @@ var ( featureServer: semver.MustParseRange(">=10.0.0"), featureDatabaseOwnerRole: semver.MustParseRange(">=15.0.0"), + + // New privileges rules in version 16 + // https://www.postgresql.org/docs/16/release-16.html#RELEASE-16-PRIVILEGES + featureCreateRoleSelfGrant: semver.MustParseRange(">=16.0.0"), } ) diff --git a/postgresql/provider.go b/postgresql/provider.go index 2743d7b6..5ac93759 100644 --- a/postgresql/provider.go +++ b/postgresql/provider.go @@ -2,12 +2,14 @@ package postgresql import ( "context" + "database/sql" "fmt" + "github.com/hashicorp/terraform-plugin-log/tflog" "os" + "regexp" "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" "github.com/Azure/azure-sdk-for-go/sdk/azidentity" - "github.com/blang/semver" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" @@ -363,6 +365,39 @@ func providerConfigure(d *schema.ResourceData) (interface{}, error) { } } + err := checkCreateRoleSelfGrant(config) + if err != nil { + return nil, err + } + client := config.NewClient(d.Get("database").(string)) return client, nil } + +func checkCreateRoleSelfGrant(config Config) error { + client := config.NewClient("postgres") + connect, err := client.Connect() + if err != nil { + return err + } + + username := config.Username + + if connect.featureSupported(featureCreateRoleSelfGrant) && !config.Superuser { + var paramValue string + query := `SELECT coalesce(replace(jsonb_path_query_first(to_jsonb(rolconfig),'$[*] ? (@ like_regex "^createrole_self_grant=")')::TEXT,'%[1]s=',''), '') FROM pg_catalog.pg_roles WHERE rolname = $1` + err := connect.QueryRow(query, username).Scan(¶mValue) + switch { + case err == sql.ErrNoRows: + // They don't have a parameter, just skip + break + case err != nil: + tflog.Debug(context.Background(), fmt.Sprintf("User %s parameters cannot be checked: %v", username, err)) + } + if !regexp.MustCompile(`^set\s*,\s*inherit$|^inherit\s*,\s*set$`).MatchString(paramValue) { + msg := fmt.Sprintf("Provider user %[1]s has not configured property 'createrole_self_grant' properly. Creating new roles will not create non admin grants by default. You can set it by \"ALTER ROLE '%[1]s' SET createrole_self_grant 'set, inherit';\"", username) + tflog.Warn(context.Background(), msg) + } + } + return nil +} diff --git a/postgresql/provider_test.go b/postgresql/provider_test.go index 44714db4..1798d663 100644 --- a/postgresql/provider_test.go +++ b/postgresql/provider_test.go @@ -29,6 +29,37 @@ func TestProvider_impl(t *testing.T) { var _ *schema.Provider = Provider() } +func TestAccProviderSetCreateRoleSelfGrant(t *testing.T) { + skipIfNotAcc(t) + + config := getTestConfig(t) + client := config.NewClient("postgres") + db, err := client.Connect() + if err != nil { + t.Fatal(err) + } + + if db.featureSupported(featureCreateRoleSelfGrant) { + t.Skipf("Skip tests for unsuported feature %d in Postgres %s", featureCreateRoleSelfGrant, db.version) + } + + // Create NON superuser role + if _, err = db.Exec("CREATE ROLE rds_srg LOGIN CREATEDB CREATEROLE PASSWORD 'rds_srg'"); err != nil { + t.Fatalf("could not create role for test user paramaters: %v", err) + } + defer func() { + _, _ = db.Exec("DROP ROLE rds_srg") + }() + + provider := Provider() + provider.Configure(context.Background(), terraform.NewResourceConfigRaw( + map[string]interface{}{ + "username": "rds_srg", + "password": "rds_srg", + }, + )) +} + func testAccPreCheck(t *testing.T) { var host string if host = os.Getenv("PGHOST"); host == "" { diff --git a/postgresql/resource_postgresql_database_test.go b/postgresql/resource_postgresql_database_test.go index 0ac97445..82b237c3 100644 --- a/postgresql/resource_postgresql_database_test.go +++ b/postgresql/resource_postgresql_database_test.go @@ -220,8 +220,17 @@ resource postgresql_database "test_db" { resource.TestCheckResourceAttr("postgresql_database.test_db", "name", "test_db"), resource.TestCheckResourceAttr("postgresql_database.test_db", "owner", "test_owner"), - // check if connected user does not have test_owner granted anymore. - checkUserMembership(t, dsn, config.Username, "test_owner", false), + func(state *terraform.State) error { + connect, _ := config.NewClient("postgres").Connect() + if connect.featureSupported(featureCreateRoleSelfGrant) { + // in PG 16 all created roles have creator grant with admin option + checkUserMembership(t, dsn, config.Username, "test_owner", true) + } else { + // check if connected user does not have test_owner granted anymore. + checkUserMembership(t, dsn, config.Username, "test_owner", false) + } + return nil + }, ), }, }, diff --git a/postgresql/resource_postgresql_default_privileges_test.go b/postgresql/resource_postgresql_default_privileges_test.go index 5c779d06..3c5fa985 100644 --- a/postgresql/resource_postgresql_default_privileges_test.go +++ b/postgresql/resource_postgresql_default_privileges_test.go @@ -187,16 +187,25 @@ resource "postgresql_default_privileges" "test_ro" { resource.TestCheckResourceAttr("postgresql_default_privileges.test_ro", "privileges.#", "1"), resource.TestCheckResourceAttr("postgresql_default_privileges.test_ro", "privileges.0", "SELECT"), - // check if connected user does not have test_owner granted anymore. - checkUserMembership(t, dsn, config.Username, "test_owner", false), + func(state *terraform.State) error { + connect, _ := config.NewClient("postgres").Connect() + if connect.featureSupported(featureCreateRoleSelfGrant) { + // in PG 16 all created roles have creator grant with admin option by default + checkUserMembership(t, dsn, config.Username, "test_owner", true) + } else { + // check if connected user does not have test_owner granted anymore. + checkUserMembership(t, dsn, config.Username, "test_owner", false) + } + return nil + }, ), }, }, }) } -// Test the case where we define default priviliges without specifying a schema. These -// priviliges should apply to newly created resources for the named role in all schema. +// Test the case where we define default privileges without specifying a schema. These +// privileges should apply to newly created resources for the named role in all schema. func TestAccPostgresqlDefaultPrivileges_NoSchema(t *testing.T) { skipIfNotAcc(t) diff --git a/postgresql/resource_postgresql_role.go b/postgresql/resource_postgresql_role.go index b7cb0fab..9581fd39 100644 --- a/postgresql/resource_postgresql_role.go +++ b/postgresql/resource_postgresql_role.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "log" + "slices" "strconv" "strings" @@ -35,6 +36,8 @@ const ( roleSearchPathAttr = "search_path" roleStatementTimeoutAttr = "statement_timeout" roleAssumeRoleAttr = "assume_role" + roleParametersAttr = "parameters" + roleCreateRoleSelfGrantAttr = "createrole_self_grant" // Deprecated options roleDepEncryptedAttr = "encrypted" @@ -173,6 +176,14 @@ func resourcePostgreSQLRole() *schema.Resource { Optional: true, Description: "Role to switch to at login", }, + roleParametersAttr: { + Type: schema.TypeMap, + Optional: true, + Description: "User parameters", + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + }, }, } } @@ -311,6 +322,10 @@ func resourcePostgreSQLRoleCreate(db *DBConnection, d *schema.ResourceData) erro return err } + if err = setParameters(txn, d); err != nil { + return err + } + if err = txn.Commit(); err != nil { return fmt.Errorf("could not commit transaction: %w", err) } @@ -480,6 +495,38 @@ func resourcePostgreSQLRoleReadImpl(db *DBConnection, d *schema.ResourceData) er } d.Set(rolePasswordAttr, password) + + if err = readRoleParameters(db, d); err != nil { + return err + } + + return nil +} + +func readRoleParameters(db *DBConnection, d *schema.ResourceData) error { + roleName := d.Get(roleNameAttr).(string) + + if paramsSchema, ok := d.GetOk(roleParametersAttr); ok { + if roleParams, ok := paramsSchema.(map[string]interface{}); ok { + for paramKey, _ := range roleParams { + if !slices.Contains(supportedRoleParameters, paramKey) { + return fmt.Errorf("parameter %s is not supported, only %v parameters are supported yet", paramKey, supportedRoleParameters) + } + var paramValue string + query := fmt.Sprintf(`SELECT coalesce(trim(replace(jsonb_path_query_first(to_jsonb(rolconfig),'$[*] ? (@ like_regex "^%[1]s=")')::text,'%[1]s=',''),'"'), '') FROM pg_catalog.pg_roles WHERE rolname = $1`, paramKey) + err := db.QueryRow(query, roleName).Scan(¶mValue) + switch { + case err == sql.ErrNoRows: + // They don't have a parameter, just skip + break + case err != nil: + return fmt.Errorf("Error reading role parameters: %w", err) + default: + roleParams[paramKey] = paramValue + } + } + } + } return nil } @@ -689,6 +736,10 @@ func resourcePostgreSQLRoleUpdate(db *DBConnection, d *schema.ResourceData) erro return err } + if err = setParameters(txn, d); err != nil { + return err + } + if err = txn.Commit(); err != nil { return fmt.Errorf("could not commit transaction: %w", err) } @@ -1062,3 +1113,47 @@ func setAssumeRole(txn *sql.Tx, d *schema.ResourceData) error { } return nil } + +func setParameters(txn *sql.Tx, d *schema.ResourceData) error { + if !d.HasChange(roleParametersAttr) { + return nil + } + + roleName := d.Get(roleNameAttr).(string) + parameters := map[string]interface{}{} + if paramsSchema, ok := d.GetOk(roleParametersAttr); ok { + if roleParams, ok := paramsSchema.(map[string]interface{}); ok { + for paramKey, paramValue := range roleParams { + if !slices.Contains(supportedRoleParameters, paramKey) { + return fmt.Errorf("parameter %s is not supported, only %v parameters are supported yet", paramKey, supportedRoleParameters) + } + parameters[paramKey] = paramValue + } + parameters = roleParams + } + } + + return setRoleParameters(txn, roleName, parameters) +} + +// supportedRoleParameters user role supported parameters +var supportedRoleParameters = []string{roleCreateRoleSelfGrantAttr} + +// setRoleParameters set given parameter for role +// only string values are supported yet, others will be ignored +func setRoleParameters(txn *sql.Tx, roleName string, parameters map[string]interface{}) error { + for k, v := range parameters { + strValue, ok := v.(string) + if slices.Contains(supportedRoleParameters, k) && ok { + valParam := pq.QuoteLiteral(strValue) + roleParam := pq.QuoteIdentifier(roleName) + query := fmt.Sprintf( + "ALTER ROLE %s SET %s TO %s", roleParam, k, valParam, + ) + if _, err := txn.Exec(query); err != nil { + return fmt.Errorf("could not set %s parameter %s for %s: %w", k, valParam, roleParam, err) + } + } + } + return nil +} diff --git a/postgresql/resource_postgresql_role_test.go b/postgresql/resource_postgresql_role_test.go index ef502f00..d7f69258 100644 --- a/postgresql/resource_postgresql_role_test.go +++ b/postgresql/resource_postgresql_role_test.go @@ -2,9 +2,11 @@ package postgresql import ( "database/sql" + "errors" "fmt" "os" "reflect" + "regexp" "sort" "strings" "testing" @@ -96,6 +98,109 @@ resource "postgresql_role" "role_with_superuser" { }) } +func TestAccPostgresqlRole_CreateRoleSelfGrant(t *testing.T) { + var configCreate = ` +resource "postgresql_role" "role_with_createrole_self_grant" { + name = "role_with_createrole_self_grant" + parameters = { + createrole_self_grant = "set, inherit" + } +} +` + var configUpdate = ` +resource "postgresql_role" "role_with_createrole_self_grant" { + name = "role_with_createrole_self_grant" + parameters = { + createrole_self_grant = "set" + } +} +` + var configReset = ` +resource "postgresql_role" "role_with_createrole_self_grant" { + name = "role_with_createrole_self_grant" + parameters = {} +} +` + var configNoParams = ` +resource "postgresql_role" "role_with_createrole_self_grant" { + name = "role_with_createrole_self_grant" +} +` + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testCheckCompatibleVersion(t, featurePrivileges) + testCheckCompatibleVersion(t, featureCreateRoleSelfGrant) + }, + Providers: testAccProviders, + CheckDestroy: testAccCheckPostgresqlRoleDestroy, + Steps: []resource.TestStep{ + { + Config: configCreate, + Check: resource.ComposeTestCheckFunc( + testAccCheckPostgresqlRoleExists("role_with_createrole_self_grant", nil, nil), + testAccCheckPostgresqlRoleParameters("role_with_createrole_self_grant", map[string]string{"createrole_self_grant": "set, inherit"}), + ), + }, + { + Config: configUpdate, + Check: resource.ComposeTestCheckFunc( + testAccCheckPostgresqlRoleExists("role_with_createrole_self_grant", nil, nil), + testAccCheckPostgresqlRoleParameters("role_with_createrole_self_grant", map[string]string{"createrole_self_grant": "set"}), + ), + }, + { + Config: configReset, + Check: resource.ComposeTestCheckFunc( + testAccCheckPostgresqlRoleExists("role_with_createrole_self_grant", nil, nil), + testAccCheckPostgresqlRoleParameters("role_with_createrole_self_grant", map[string]string{}), + ), + }, + // check parameters are reset by deleting parameters container + { + Config: configCreate, + Check: resource.ComposeTestCheckFunc( + testAccCheckPostgresqlRoleExists("role_with_createrole_self_grant", nil, nil), + testAccCheckPostgresqlRoleParameters("role_with_createrole_self_grant", map[string]string{"createrole_self_grant": "set, inherit"}), + ), + }, + { + Config: configNoParams, + Check: resource.ComposeTestCheckFunc( + testAccCheckPostgresqlRoleExists("role_with_createrole_self_grant", nil, nil), + testAccCheckPostgresqlRoleParameters("role_with_createrole_self_grant", map[string]string{}), + ), + }, + }, + }) +} + +func TestAccPostgresqlRole_UnsupportedRoleParam(t *testing.T) { + var configCreate = ` +resource "postgresql_role" "role_with_unsupported_param_lol" { + name = "role_with_unsupported_param_lol" + parameters = { + unsupported_param_lol = "N/A" + } +} +` + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testCheckCompatibleVersion(t, featurePrivileges) + testCheckCompatibleVersion(t, featureCreateRoleSelfGrant) + }, + Providers: testAccProviders, + CheckDestroy: testAccCheckPostgresqlRoleDestroy, + Steps: []resource.TestStep{ + { + Config: configCreate, + ExpectError: regexp.MustCompile(`parameter unsupported_param_lol is not supported, only \[.+] parameters are supported yet`), + }, + }, + }) +} + func TestAccPostgresqlRole_Update(t *testing.T) { var configCreate = ` @@ -213,6 +318,7 @@ resource "postgresql_role" "test_role" { PreCheck: func() { testAccPreCheck(t) testCheckCompatibleVersion(t, featurePrivileges) + testCheckCompatibleVersionRange(t, "<16.0.0") // PG 16: Only roles with the ADMIN option on role "" may grant this role. }, Providers: testAccProviders, CheckDestroy: testAccCheckPostgresqlRoleDestroy, @@ -278,6 +384,25 @@ func testAccCheckPostgresqlRoleExists(roleName string, grantedRoles []string, se } } +func testAccCheckPostgresqlRoleParameters(roleName string, parameters map[string]string) resource.TestCheckFunc { + return func(s *terraform.State) error { + client := testAccProvider.Meta().(*Client) + + if parameters != nil { + var errs []error + for k, v := range parameters { + if err := checkRoleParameter(client, roleName, k, v); err != nil { + errs = append(errs, err) + } + } + if len(errs) > 0 { + return errors.Join(errs...) + } + } + return nil + } +} + func checkRoleExists(client *Client, roleName string) (bool, error) { db, err := client.Connect() if err != nil { @@ -311,6 +436,41 @@ func testAccCheckRoleCanLogin(t *testing.T, role, password string) resource.Test } } +func TestAccPostgresqlRole_DbOwner(t *testing.T) { + config := getTestConfig(t) + dsn := config.connStr("postgres") + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testCheckCompatibleVersion(t, featurePrivileges) + }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccPostgresqlRoleDbOwnerConfig, + Check: resource.ComposeTestCheckFunc( + testAccCheckPostgresqlRoleExists("owned_db_owner", nil, nil), + resource.TestCheckResourceAttr("postgresql_database.owned_db", "owner", "owned_db_owner"), + resource.TestCheckResourceAttr("postgresql_database.owned_db", "name", "owned_db"), + + func(state *terraform.State) error { + connect, _ := config.NewClient("postgres").Connect() + if connect.featureSupported(featureCreateRoleSelfGrant) { + // in PG 16 all created roles have creator grant with admin option + checkUserMembership(t, dsn, config.Username, "owned_db_owner", true) + } else { + // check if connected user does not have test_owner granted anymore. + checkUserMembership(t, dsn, config.Username, "owned_db_owner", false) + } + return nil + }, + ), + }, + }, + }) +} + func checkGrantedRoles(client *Client, roleName string, expectedRoles []string) error { db, err := client.Connect() if err != nil { @@ -378,22 +538,40 @@ func checkSearchPath(client *Client, roleName string, expectedSearchPath []strin return nil } +func checkRoleParameter(client *Client, roleName string, name string, value string) error { + db, err := client.Connect() + if err != nil { + return err + } + + expectedParameter := fmt.Sprintf("%s=%s", name, value) + query := fmt.Sprintf(`SELECT 1 FROM pg_catalog.pg_roles WHERE rolname = $1 AND jsonb_path_exists(to_jsonb(rolconfig), '$[*] ? (@ like_regex "^%s$")')`, expectedParameter) + result, err := db.Query(query, roleName) + if err != nil { + return err + } + if !result.Next() { + return fmt.Errorf("role '%s' parameters are not set to expected value. expected '%v' but nothing found", roleName, expectedParameter) + } + return nil +} + var testAccPostgresqlRoleConfig = ` resource "postgresql_role" "myrole2" { - name = "myrole2" + name = "myrole2" login = true } resource "postgresql_role" "role_with_pwd" { - name = "role_with_pwd" - login = true + name = "role_with_pwd" + login = true password = "mypass" } resource "postgresql_role" "role_with_pwd_encr" { - name = "role_with_pwd_encr" - login = true - password = "mypass" + name = "role_with_pwd_encr" + login = true + password = "mypass" encrypted_password = true } @@ -402,39 +580,56 @@ resource "postgresql_role" "role_simple" { } resource "postgresql_role" "role_with_defaults" { - name = "role_default" - superuser = false - create_database = false - create_role = false - inherit = false - login = false - replication = false - bypass_row_level_security = false - connection_limit = -1 - encrypted_password = true - password = "" - skip_drop_role = false - valid_until = "infinity" - statement_timeout = 0 + name = "role_default" + superuser = false + create_database = false + create_role = false + inherit = false + login = false + replication = false + bypass_row_level_security = false + connection_limit = -1 + encrypted_password = true + password = "" + skip_drop_role = false + valid_until = "infinity" + statement_timeout = 0 idle_in_transaction_session_timeout = 0 - assume_role = "" + assume_role = "" } resource "postgresql_role" "role_with_create_database" { - name = "role_with_create_database" + name = "role_with_create_database" create_database = true } resource "postgresql_role" "sub_role" { - name = "sub_role" - roles = [ - "${postgresql_role.myrole2.id}", - "${postgresql_role.role_simple.id}", - ] + name = "sub_role" + roles = [ + "${postgresql_role.myrole2.id}", + "${postgresql_role.role_simple.id}", + ] } resource "postgresql_role" "role_with_search_path" { - name = "role_with_search_path" + name = "role_with_search_path" search_path = ["bar", "foo-with-hyphen"] } ` + +// based on comment for issue 407 +// https://github.com/cyrilgdn/terraform-provider-postgresql/issues/407#issue-2127498162 +var testAccPostgresqlRoleDbOwnerConfig = ` +resource "postgresql_role" "owned_db_owner" { + name = "owned_db_owner" + login = true + password = "veryS3cret" +} + +resource "postgresql_database" "owned_db" { + name = "owned_db" + owner = postgresql_role.owned_db_owner.name + lc_collate = "en_US.utf8" + allow_connections = true +} +` diff --git a/postgresql/utils_test.go b/postgresql/utils_test.go index 8b5b462f..2312faed 100644 --- a/postgresql/utils_test.go +++ b/postgresql/utils_test.go @@ -3,6 +3,7 @@ package postgresql import ( "database/sql" "fmt" + "github.com/blang/semver" "os" "strconv" "strings" @@ -26,7 +27,19 @@ func testCheckCompatibleVersion(t *testing.T, feature featureName) { t.Fatalf("could connect to database: %v", err) } if !db.featureSupported(feature) { - t.Skipf("Skip extension tests for Postgres %s", db.version) + t.Skipf("Skip tests for unsuported feature %d in Postgres %s", feature, db.version) + } +} + +// Can be used in a PreCheck function to disable test based on Postgresql version range. +func testCheckCompatibleVersionRange(t *testing.T, versionRange string) { + client := testAccProvider.Meta().(*Client) + db, err := client.Connect() + if err != nil { + t.Fatalf("could connect to database: %v", err) + } + if !semver.MustParseRange(versionRange)(db.version) { + t.Skipf("Skip tests for unexpected version range %s in Postgres %s", versionRange, db.version) } } @@ -175,8 +188,13 @@ func createTestTables(t *testing.T, dbSuffix string, tables []string, owner stri } } if owner != "" && !config.Superuser { - if _, err := db.Exec(fmt.Sprintf("SET ROLE %s; REVOKE %s FROM %s", adminUser, owner, adminUser)); err != nil { - t.Fatalf("could not revoke role %s from %s: %v", owner, adminUser, err) + if _, err := db.Exec(fmt.Sprintf("SET ROLE %s", adminUser)); err != nil { + t.Fatalf("could not set role %s : %v", adminUser, err) + } + if !createRoleSelfGrantEnabled(t) { + if _, err := db.Exec(fmt.Sprintf("REVOKE %s FROM %s", owner, adminUser)); err != nil { + t.Fatalf("could not revoke role %s from %s: %v", owner, adminUser, err) + } } } @@ -200,8 +218,13 @@ func createTestTables(t *testing.T, dbSuffix string, tables []string, owner stri } } if owner != "" && !config.Superuser { - if _, err := db.Exec(fmt.Sprintf("SET ROLE %s; REVOKE %s FROM %s", adminUser, owner, adminUser)); err != nil { - t.Fatalf("could not revoke role %s from %s: %v", owner, adminUser, err) + if _, err := db.Exec(fmt.Sprintf("SET ROLE %s", adminUser)); err != nil { + t.Fatalf("could not set role %s: %v", adminUser, err) + } + if !createRoleSelfGrantEnabled(t) { + if _, err := db.Exec(fmt.Sprintf("REVOKE %s FROM %s", owner, adminUser)); err != nil { + t.Fatalf("could not revoke role %s from %s: %v", owner, adminUser, err) + } } } @@ -438,3 +461,12 @@ func testCheckColumnPrivileges(t *testing.T, dbName, roleName string, tables []s } return nil } + +func createRoleSelfGrantEnabled(t *testing.T) bool { + client := testAccProvider.Meta().(*Client) + db, err := client.Connect() + if err != nil { + t.Fatalf("could connect to database: %v", err) + } + return db.featureSupported(featureCreateRoleSelfGrant) +} diff --git a/tests/docker-compose.yml b/tests/docker-compose.yml index 177994bf..6852399b 100644 --- a/tests/docker-compose.yml +++ b/tests/docker-compose.yml @@ -1,5 +1,3 @@ -version: "3" - services: postgres: image: postgres:${PGVERSION:-latest} diff --git a/tests/switch_rds.sh b/tests/switch_rds.sh index dd112fd0..8d5f01b4 100755 --- a/tests/switch_rds.sh +++ b/tests/switch_rds.sh @@ -6,15 +6,24 @@ source "$HERE/switch_superuser.sh" echo "Switching to an RDS-like environment" psql -d postgres > /dev/null <= 160000 THEN + ALTER ROLE rds SET createrole_self_grant TO 'set, inherit'; + END IF; + END; +\$BODY\$; EOS psql -d template1 > /dev/null <