From 60bb91237590ab101019e562c925b9d085c24f50 Mon Sep 17 00:00:00 2001 From: nguyen1 Date: Wed, 1 May 2024 16:03:58 +0700 Subject: [PATCH 1/7] Define view resource attributes --- postgresql/resource_postgresql_view.go | 102 +++++++++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 postgresql/resource_postgresql_view.go diff --git a/postgresql/resource_postgresql_view.go b/postgresql/resource_postgresql_view.go new file mode 100644 index 00000000..655eea9c --- /dev/null +++ b/postgresql/resource_postgresql_view.go @@ -0,0 +1,102 @@ +package postgresql + +import ( + "fmt" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" +) + +const ( + viewNameAttr = "name" + viewSchemaAttr = "schema" + viewRecursiveAttr = "recursive" + viewColumnNamesAttr = "column_names" + viewBodyAttr = "body" + viewCheckOptionAttr = "check_option" +) + +func resourcePostgreSQLView() *schema.Resource { + return &schema.Resource{ + Create: PGResourceFunc(resourcePostgreSQLViewCreate), + Read: PGResourceFunc(resourcePostgreSQLViewCreate), + Update: PGResourceFunc(resourcePostgreSQLViewCreate), + Delete: PGResourceFunc(resourcePostgreSQLViewCreate), + Exists: PGResourceExistsFunc(resourcePostgreSQLFunctionExists), + Importer: &schema.ResourceImporter{ + StateContext: schema.ImportStatePassthroughContext, + }, + + Schema: map[string]*schema.Schema{ + viewSchemaAttr: { + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + Description: "Schema where the view is located. If not specified, the provider default is used.", + + DiffSuppressFunc: defaultDiffSuppressFunc, + }, + viewNameAttr: { + Type: schema.TypeString, + Required: true, + ForceNew: true, + Description: "Name of the view.", + }, + viewRecursiveAttr: { + Type: schema.TypeBool, + Optional: true, + Default: false, + Description: "If the view is a recursive view.", + }, + viewColumnNamesAttr: { + Type: schema.TypeList, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, + Description: "The optional list of names to be used for columns of the view. If not given, the column names are deduced from the query.", + }, + viewBodyAttr: { + Type: schema.TypeString, + Required: true, + Description: "Body of the view.", + + DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { + return normalizeFunctionBody(new) == old + }, + StateFunc: func(val interface{}) string { + return normalizeFunctionBody(val.(string)) + }, + }, + viewCheckOptionAttr: { + Type: schema.TypeString, + Optional: true, + DiffSuppressFunc: defaultDiffSuppressFunc, + ValidateFunc: validation.StringInSlice([]string{"CASCADED", "LOCAL"}, true), + Description: "Controls the behavior of automatically updatable views. One of: CASCADED, LOCAL", + }, + }, + } +} + +func resourcePostgreSQLViewCreate(db *DBConnection, d *schema.ResourceData) error { + if !db.featureSupported(featureFunction) { + return fmt.Errorf( + "postgresql_view resource is not supported for this Postgres version (%s)", + db.version, + ) + } + + if err := createView(db, d, false); err != nil { + return err + } + + return resourcePostgreSQLViewReadImpl(db, d) +} + +func resourcePostgreSQLViewReadImpl(db *DBConnection, d *schema.ResourceData) error { + return nil +} + +func createView(db *DBConnection, d *schema.ResourceData, replace bool) error { + return nil +} From 59042a7a979442658cd48818eec189a32fd56297 Mon Sep 17 00:00:00 2001 From: Nguyen Date: Thu, 2 May 2024 23:09:19 +0700 Subject: [PATCH 2/7] Read view definition from database --- postgresql/resource_postgresql_view.go | 165 +++++++++++++++++++++++-- 1 file changed, 158 insertions(+), 7 deletions(-) diff --git a/postgresql/resource_postgresql_view.go b/postgresql/resource_postgresql_view.go index 655eea9c..0f9e106c 100644 --- a/postgresql/resource_postgresql_view.go +++ b/postgresql/resource_postgresql_view.go @@ -1,18 +1,23 @@ package postgresql import ( + "bytes" + "database/sql" "fmt" + "log" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" + "github.com/lib/pq" ) const ( viewNameAttr = "name" viewSchemaAttr = "schema" + viewDatabaseAttr = "database" viewRecursiveAttr = "recursive" viewColumnNamesAttr = "column_names" - viewBodyAttr = "body" + viewQueryAttr = "query" viewCheckOptionAttr = "check_option" ) @@ -28,12 +33,21 @@ func resourcePostgreSQLView() *schema.Resource { }, Schema: map[string]*schema.Schema{ + viewDatabaseAttr: { + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + Description: "The database where the view is located. If not specified, the provider default database is used.", + + DiffSuppressFunc: defaultDiffSuppressFunc, + }, viewSchemaAttr: { Type: schema.TypeString, Optional: true, Computed: true, ForceNew: true, - Description: "Schema where the view is located. If not specified, the provider default is used.", + Description: "The schema where the view is located. If not specified, the provider default is used.", DiffSuppressFunc: defaultDiffSuppressFunc, }, @@ -41,7 +55,7 @@ func resourcePostgreSQLView() *schema.Resource { Type: schema.TypeString, Required: true, ForceNew: true, - Description: "Name of the view.", + Description: "The name of the view.", }, viewRecursiveAttr: { Type: schema.TypeBool, @@ -50,15 +64,15 @@ func resourcePostgreSQLView() *schema.Resource { Description: "If the view is a recursive view.", }, viewColumnNamesAttr: { - Type: schema.TypeList, + Type: schema.TypeSet, Optional: true, Elem: &schema.Schema{Type: schema.TypeString}, Description: "The optional list of names to be used for columns of the view. If not given, the column names are deduced from the query.", }, - viewBodyAttr: { + viewQueryAttr: { Type: schema.TypeString, Required: true, - Description: "Body of the view.", + Description: "The query of the view.", DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { return normalizeFunctionBody(new) == old @@ -72,7 +86,7 @@ func resourcePostgreSQLView() *schema.Resource { Optional: true, DiffSuppressFunc: defaultDiffSuppressFunc, ValidateFunc: validation.StringInSlice([]string{"CASCADED", "LOCAL"}, true), - Description: "Controls the behavior of automatically updatable views. One of: CASCADED, LOCAL", + Description: "The check option which controls the behavior of automatically updatable views. One of: CASCADED, LOCAL", }, }, } @@ -94,9 +108,146 @@ func resourcePostgreSQLViewCreate(db *DBConnection, d *schema.ResourceData) erro } func resourcePostgreSQLViewReadImpl(db *DBConnection, d *schema.ResourceData) error { + viewID := d.Id() + if viewID == "" { + genViewID, err := genViewID(db, d) + if err != nil { + return err + } + viewID = genViewID + } + + // Query the view definition + databaseName := db.client.databaseName + if databaseAttr, ok := d.GetOk(viewDatabaseAttr); ok { + databaseName = databaseAttr.(string) + } + + query := `` + txn, err := startTransaction(db.client, databaseName) + if err != nil { + return err + } + defer deferredRollback(txn) + + var viewQuery string + err = txn.QueryRow(query).Scan(&viewQuery) + switch { + case err == sql.ErrNoRows: + log.Printf("[WARN] PostgreSQL view: %s", viewID) + d.SetId("") + return nil + case err != nil: + return fmt.Errorf("Error reading view: %w", err) + } + + if err := txn.Commit(); err != nil { + return err + } + + d.Set(viewDatabaseAttr, databaseName) + d.Set(viewSchemaAttr, "") + d.Set(viewNameAttr, "") + d.Set(viewRecursiveAttr, false) + d.Set(viewColumnNamesAttr, "") + d.Set(viewQuery, "") + d.Set(viewCheckOptionAttr, "") + + d.SetId(viewID) + return nil } +func genViewID(db *DBConnection, d *schema.ResourceData) (string, error) { + // Generate with format: .. + b := bytes.NewBufferString("") + if databaseAttr, ok := d.GetOk(viewDatabaseAttr); ok { + fmt.Fprint(b, databaseAttr.(string), ".") + } else { + fmt.Fprint(b, db.client.databaseName, ".") + } + + schemaName := "public" + if v, ok := d.GetOk(viewSchemaAttr); ok { + schemaName = v.(string) + } + viewName := d.Get(viewNameAttr).(string) + + fmt.Fprint(b, schemaName, ".", viewName) + return b.String(), nil +} + func createView(db *DBConnection, d *schema.ResourceData, replace bool) error { + schemaName := "public" + if v, ok := d.GetOk(viewSchemaAttr); ok { + schemaName = v.(string) + } + + name := d.Get(viewNameAttr).(string) + recursive := false + if v, ok := d.GetOk(viewRecursiveAttr); ok { + recursive = v.(bool) + } + + var columnNames []string + if v, ok := d.GetOk(viewColumnNamesAttr); ok { + columnNames = v.([]string) + } + + query := d.Get(viewQueryAttr).(string) + + var checkOption string + if v, ok := d.GetOk(viewCheckOptionAttr); ok { + checkOption = v.(string) + } + + // Construct the view + b := bytes.NewBufferString("CREATE ") + if replace { + b.WriteString("OR REPLACE ") + } + + if recursive { + b.WriteString("RECURSIVE ") + } + b.WriteString("VIEW ") + + fmt.Fprint(b, pq.QuoteIdentifier(schemaName), ".") + fmt.Fprint(b, pq.QuoteIdentifier(name)) + + for idx, columnName := range columnNames { + if idx <= 0 { + b.WriteRune('(') + } + if idx > 0 { + b.WriteRune(',') + } + b.WriteString(columnName) + } + if len(columnNames) > 0 { + b.WriteRune(')') + } + + fmt.Fprint(b, " AS\n", query) + if checkOption == "" { + fmt.Fprint(b, "\nWITH ", checkOption, " CHECK OPTION") + } + b.WriteRune(';') + + sql := b.String() + txn, err := startTransaction(db.client, d.Get(viewDatabaseAttr).(string)) + if err != nil { + return err + } + defer deferredRollback(txn) + + if _, err := txn.Exec(sql); err != nil { + return err + } + + if err := txn.Commit(); err != nil { + return err + } + return nil } From ac0921a94ede3f5f72cc68c40d4c7354e5eaa55c Mon Sep 17 00:00:00 2001 From: Nguyen Date: Sun, 5 May 2024 08:41:39 +0700 Subject: [PATCH 3/7] Add basic implementation for pg view --- postgresql/resource_postgresql_view.go | 286 +++++++++++++++----- postgresql/resource_postgresql_view_test.go | 106 ++++++++ 2 files changed, 326 insertions(+), 66 deletions(-) create mode 100644 postgresql/resource_postgresql_view_test.go diff --git a/postgresql/resource_postgresql_view.go b/postgresql/resource_postgresql_view.go index 0f9e106c..9be87603 100644 --- a/postgresql/resource_postgresql_view.go +++ b/postgresql/resource_postgresql_view.go @@ -5,6 +5,8 @@ import ( "database/sql" "fmt" "log" + "strconv" + "strings" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" @@ -12,22 +14,24 @@ import ( ) const ( - viewNameAttr = "name" - viewSchemaAttr = "schema" - viewDatabaseAttr = "database" - viewRecursiveAttr = "recursive" - viewColumnNamesAttr = "column_names" - viewQueryAttr = "query" - viewCheckOptionAttr = "check_option" + viewNameAttr = "name" + viewSchemaAttr = "schema" + viewDatabaseAttr = "database" + viewQueryAttr = "query" + viewWithCheckOptionAttr = "with_check_option" + viewWithSecurityBarrierAttr = "with_security_barrier" + viewWithSecurityInvokerAttr = "with_security_invoker" + + viewDropCascadeAttr = "drop_cascade" ) func resourcePostgreSQLView() *schema.Resource { return &schema.Resource{ Create: PGResourceFunc(resourcePostgreSQLViewCreate), - Read: PGResourceFunc(resourcePostgreSQLViewCreate), - Update: PGResourceFunc(resourcePostgreSQLViewCreate), - Delete: PGResourceFunc(resourcePostgreSQLViewCreate), - Exists: PGResourceExistsFunc(resourcePostgreSQLFunctionExists), + Read: PGResourceFunc(resourcePostgreSQLViewRead), + Update: PGResourceFunc(resourcePostgreSQLViewUpdate), + Delete: PGResourceFunc(resourcePostgreSQLViewDelete), + Exists: PGResourceExistsFunc(resourcePostgreSQLViewExists), Importer: &schema.ResourceImporter{ StateContext: schema.ImportStatePassthroughContext, }, @@ -57,18 +61,6 @@ func resourcePostgreSQLView() *schema.Resource { ForceNew: true, Description: "The name of the view.", }, - viewRecursiveAttr: { - Type: schema.TypeBool, - Optional: true, - Default: false, - Description: "If the view is a recursive view.", - }, - viewColumnNamesAttr: { - Type: schema.TypeSet, - Optional: true, - Elem: &schema.Schema{Type: schema.TypeString}, - Description: "The optional list of names to be used for columns of the view. If not given, the column names are deduced from the query.", - }, viewQueryAttr: { Type: schema.TypeString, Required: true, @@ -81,13 +73,31 @@ func resourcePostgreSQLView() *schema.Resource { return normalizeFunctionBody(val.(string)) }, }, - viewCheckOptionAttr: { + viewWithCheckOptionAttr: { Type: schema.TypeString, Optional: true, DiffSuppressFunc: defaultDiffSuppressFunc, ValidateFunc: validation.StringInSlice([]string{"CASCADED", "LOCAL"}, true), Description: "The check option which controls the behavior of automatically updatable views. One of: CASCADED, LOCAL", }, + viewWithSecurityBarrierAttr: { + Type: schema.TypeBool, + Optional: true, + Default: false, + Description: "This should be used if the view is intended to provide row-level security.", + }, + viewWithSecurityInvokerAttr: { + Type: schema.TypeBool, + Optional: true, + Default: false, + Description: "This option causes the underlying base relations to be checked against the privileges of the user of the view rather than the view owner.", + }, + viewDropCascadeAttr: { + Type: schema.TypeBool, + Description: "Automatically drop objects that depend on the view (such as other views), and in turn all objects that depend on those objects.", + Optional: true, + Default: false, + }, }, } } @@ -107,6 +117,120 @@ func resourcePostgreSQLViewCreate(db *DBConnection, d *schema.ResourceData) erro return resourcePostgreSQLViewReadImpl(db, d) } +func resourcePostgreSQLViewRead(db *DBConnection, d *schema.ResourceData) error { + if !db.featureSupported(featureFunction) { + return fmt.Errorf( + "postgresql_view resource is not supported for this Postgres version (%s)", + db.version, + ) + } + + return resourcePostgreSQLViewReadImpl(db, d) +} + +func resourcePostgreSQLViewUpdate(db *DBConnection, d *schema.ResourceData) error { + if !db.featureSupported(featureFunction) { + return fmt.Errorf( + "postgresql_view resource is not supported for this Postgres version (%s)", + db.version, + ) + } + + if err := createView(db, d, true); err != nil { + return err + } + + return resourcePostgreSQLViewReadImpl(db, d) +} + +func resourcePostgreSQLViewDelete(db *DBConnection, d *schema.ResourceData) error { + if !db.featureSupported(featureFunction) { + return fmt.Errorf( + "postgresql_view resource is not supported for this Postgres version (%s)", + db.version, + ) + } + + dropMode := "RESTRICT" + if v, ok := d.GetOk(viewDropCascadeAttr); ok && v.(bool) { + dropMode = "CASCADE" + } + + viewParts := strings.Split(d.Id(), ".") + databaseName, schemaName, viewName := viewParts[0], viewParts[1], viewParts[2] + viewIdentifier := fmt.Sprintf("%s.%s", pq.QuoteIdentifier(schemaName), pq.QuoteIdentifier(viewName)) + + sql := fmt.Sprintf("DROP VIEW IF EXISTS %s %s", viewIdentifier, dropMode) + txn, err := startTransaction(db.client, databaseName) + if err != nil { + return err + } + defer deferredRollback(txn) + + if _, err := txn.Exec(sql); err != nil { + return err + } + + if err := txn.Commit(); err != nil { + return err + } + + d.SetId("") + + return nil +} + +func resourcePostgreSQLViewExists(db *DBConnection, d *schema.ResourceData) (bool, error) { + if !db.featureSupported(featureFunction) { + return false, fmt.Errorf( + "postgresql_view resource is not supported for this Postgres version (%s)", + db.version, + ) + } + + viewParts := strings.Split(d.Id(), ".") + databaseName, schemaName, viewName := viewParts[0], viewParts[1], viewParts[2] + viewIdentifier := fmt.Sprintf("%s.%s", pq.QuoteIdentifier(schemaName), pq.QuoteIdentifier(viewName)) + + var viewExists bool + + txn, err := startTransaction(db.client, databaseName) + if err != nil { + return false, err + } + defer deferredRollback(txn) + + query := fmt.Sprintf("SELECT to_regclass('%s') IS NOT NULL AS viewExists", viewIdentifier) + + if err := txn.QueryRow(query).Scan(&viewExists); err != nil { + return false, err + } + + if err := txn.Commit(); err != nil { + return false, err + } + + return viewExists, nil +} + +type PGView struct { + Database string + Schema string + Name string + Query string + WithCheckOption string + WithSecurityBarrier bool + WithSecurityInvoker bool +} + +type ViewInfo struct { + Database string + Schema string + Name string + Query string + Option []string +} + func resourcePostgreSQLViewReadImpl(db *DBConnection, d *schema.ResourceData) error { viewID := d.Id() if viewID == "" { @@ -123,41 +247,89 @@ func resourcePostgreSQLViewReadImpl(db *DBConnection, d *schema.ResourceData) er databaseName = databaseAttr.(string) } - query := `` + query := `SELECT current_database() AS database, ` + + `n.nspname AS schema, ` + + `c.relname AS name, ` + + `pg_get_viewdef(c.oid, true) AS query, ` + + `c.reloptions AS options ` + + `FROM pg_class c ` + + `JOIN pg_namespace n ON c.relnamespace = n.oid ` + + `WHERE c.relkind = 'v' AND n.nspname = '$1' AND c.relname = '$2'` txn, err := startTransaction(db.client, databaseName) if err != nil { return err } defer deferredRollback(txn) - var viewQuery string - err = txn.QueryRow(query).Scan(&viewQuery) + viewIDParts := strings.Split(viewID, ".") + var viewInfo ViewInfo + err = txn.QueryRow(query, viewIDParts[1], viewIDParts[2]).Scan(&viewInfo) switch { case err == sql.ErrNoRows: log.Printf("[WARN] PostgreSQL view: %s", viewID) d.SetId("") return nil case err != nil: - return fmt.Errorf("Error reading view: %w", err) + return fmt.Errorf("error reading view: %w", err) } if err := txn.Commit(); err != nil { return err } - d.Set(viewDatabaseAttr, databaseName) - d.Set(viewSchemaAttr, "") - d.Set(viewNameAttr, "") - d.Set(viewRecursiveAttr, false) - d.Set(viewColumnNamesAttr, "") - d.Set(viewQuery, "") - d.Set(viewCheckOptionAttr, "") + pgView, err := parseView(viewInfo) + if err != nil { + return err + } + + d.Set(viewDatabaseAttr, pgView.Database) + d.Set(viewSchemaAttr, pgView.Schema) + d.Set(viewNameAttr, pgView.Name) + d.Set(viewQueryAttr, pgView.Query) + d.Set(viewWithCheckOptionAttr, pgView.WithCheckOption) + d.Set(viewWithSecurityBarrierAttr, pgView.WithSecurityBarrier) + d.Set(viewWithCheckOptionAttr, pgView.WithSecurityInvoker) d.SetId(viewID) return nil } +func parseView(viewInfo ViewInfo) (PGView, error) { + var pgView PGView + pgView.Database = viewInfo.Database + pgView.Schema = viewInfo.Schema + pgView.Name = viewInfo.Name + pgView.Query = viewInfo.Query + + // Parse options. There are 3 options: + // 1. check_option (enum) - [LOCAL | CASCADED] + // 2. security_barrier (boolean) + // 3. security_invoker (boolean) + options := viewInfo.Option + if viewInfo.Option != nil && len(options) > 0 { + for _, option := range options { + parts := strings.Split(option, "=") + if len(parts) != 2 { + return pgView, fmt.Errorf("invalid view option: %s", option) + } + switch parts[0] { + case "check_option": + pgView.WithCheckOption = parts[1] + case "security_barrier": + val, _ := strconv.ParseBool(parts[1]) + pgView.WithSecurityBarrier = val + case "security_invoker": + val, _ := strconv.ParseBool(parts[1]) + pgView.WithSecurityInvoker = val + default: + log.Printf("[WARN] Unsupported option: %s", parts[0]) + } + } + } + return pgView, nil +} + func genViewID(db *DBConnection, d *schema.ResourceData) (string, error) { // Generate with format: .. b := bytes.NewBufferString("") @@ -184,54 +356,36 @@ func createView(db *DBConnection, d *schema.ResourceData, replace bool) error { } name := d.Get(viewNameAttr).(string) - recursive := false - if v, ok := d.GetOk(viewRecursiveAttr); ok { - recursive = v.(bool) - } - - var columnNames []string - if v, ok := d.GetOk(viewColumnNamesAttr); ok { - columnNames = v.([]string) - } - query := d.Get(viewQueryAttr).(string) - var checkOption string - if v, ok := d.GetOk(viewCheckOptionAttr); ok { - checkOption = v.(string) - } - // Construct the view b := bytes.NewBufferString("CREATE ") if replace { b.WriteString("OR REPLACE ") } - if recursive { - b.WriteString("RECURSIVE ") - } b.WriteString("VIEW ") fmt.Fprint(b, pq.QuoteIdentifier(schemaName), ".") fmt.Fprint(b, pq.QuoteIdentifier(name)) - for idx, columnName := range columnNames { - if idx <= 0 { - b.WriteRune('(') - } - if idx > 0 { - b.WriteRune(',') - } - b.WriteString(columnName) + // With options + var withOptions []string + if v, ok := d.GetOk(viewWithCheckOptionAttr); ok { + withOptions = append(withOptions, fmt.Sprintf("check_option=%s", v.(string))) } - if len(columnNames) > 0 { - b.WriteRune(')') + if v, ok := d.GetOk(viewWithSecurityBarrierAttr); ok { + withOptions = append(withOptions, fmt.Sprintf("security_barrier=%v", v.(bool))) + } + if v, ok := d.GetOk(viewWithSecurityInvokerAttr); ok { + withOptions = append(withOptions, fmt.Sprintf("security_invoker=%v", v.(bool))) + } + if len(withOptions) > 0 { + fmt.Fprint(b, "WITH (", strings.Join(withOptions[:], ","), ")") } + // Query fmt.Fprint(b, " AS\n", query) - if checkOption == "" { - fmt.Fprint(b, "\nWITH ", checkOption, " CHECK OPTION") - } b.WriteRune(';') sql := b.String() diff --git a/postgresql/resource_postgresql_view_test.go b/postgresql/resource_postgresql_view_test.go new file mode 100644 index 00000000..737ac300 --- /dev/null +++ b/postgresql/resource_postgresql_view_test.go @@ -0,0 +1,106 @@ +package postgresql + +import ( + "database/sql" + "fmt" + "testing" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" +) + +func TestAccPostgresqlView_Basic(t *testing.T) { + config := ` +resource "postgresql_view" "basic_view" { + name = "basic_view" + query = "SELECT * FROM tableA" +} +` + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testCheckCompatibleVersion(t, featureFunction) + }, + Providers: testAccProviders, + CheckDestroy: testAccCheckPostgresqlFunctionDestroy, + Steps: []resource.TestStep{ + { + Config: config, + Check: resource.ComposeTestCheckFunc( + testAccCheckPostgresqlViewExists("postgresql_view.basic_view", ""), + resource.TestCheckResourceAttr( + "ppostgresql_view.basic_view", "schema", "public"), + resource.TestCheckResourceAttr( + "postgresql_view.basic_view", "name", "basic_view"), + resource.TestCheckResourceAttr( + "postgresql_view.basic_view", "query", "SELECT * FROM tableA"), + resource.TestCheckResourceAttr( + "postgresql_view.basic_view", "with_check_option", ""), + resource.TestCheckResourceAttr( + "postgresql_view.basic_view", "with_security_barrier", "false"), + resource.TestCheckResourceAttr( + "postgresql_view.basic_view", "with_security_invoker", "false"), + resource.TestCheckResourceAttr( + "postgresql_view.basic_view", "drop_cascade", "false"), + ), + }, + }, + }) +} + +func TestAccPostgresqlFunction_SpecificDatabase(t *testing.T) { + skipIfNotAcc(t) + + dbSuffix, teardown := setupTestDatabase(t, true, true) + defer teardown() + + dbName, _ := getTestDBNames(dbSuffix) +} + +func testAccCheckPostgresqlViewExists(n string, database string) resource.TestCheckFunc { + return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[n] + if !ok { + return fmt.Errorf("Resource not found: %s", n) + } + + if rs.Primary.ID == "" { + return fmt.Errorf("No ID is set") + } + + signature := rs.Primary.ID + + client := testAccProvider.Meta().(*Client) + txn, err := startTransaction(client, database) + if err != nil { + return err + } + defer deferredRollback(txn) + + exists, err := checkFunctionExists(txn, signature) + + if err != nil { + return fmt.Errorf("Error checking function %s", err) + } + + if !exists { + return fmt.Errorf("Function not found") + } + + return nil + } +} + +func checkViewExists(txn *sql.Tx, signature string) (bool, error) { + var _rez bool + err := txn.QueryRow(fmt.Sprintf("SELECT to_regclass('%s') IS NOT NULL", signature)).Scan(&_rez) + switch { + case err == sql.ErrNoRows: + return false, nil + case err != nil: + return false, fmt.Errorf("Error reading info about view: %s", err) + } + + return _rez, nil +} From cb1c0f21791446e0d3dc4146be2ede3cf6907653 Mon Sep 17 00:00:00 2001 From: Nguyen Date: Sun, 5 May 2024 17:00:14 +0700 Subject: [PATCH 4/7] Add unit test for pg view resource --- postgresql/config.go | 3 + postgresql/provider.go | 4 +- postgresql/resource_postgresql_view.go | 81 +++++-- postgresql/resource_postgresql_view_test.go | 215 ++++++++++++++++++- website/docs/r/postgresql_view.html.markdown | 56 +++++ 5 files changed, 328 insertions(+), 31 deletions(-) create mode 100644 website/docs/r/postgresql_view.html.markdown diff --git a/postgresql/config.go b/postgresql/config.go index c4ed30e8..4ebb53d4 100644 --- a/postgresql/config.go +++ b/postgresql/config.go @@ -41,6 +41,7 @@ const ( featurePubWithoutTruncate featureFunction featureServer + featureView ) var ( @@ -112,6 +113,8 @@ var ( featureServer: semver.MustParseRange(">=10.0.0"), featureDatabaseOwnerRole: semver.MustParseRange(">=15.0.0"), + + featureView: semver.MustParseRange(">12.0.0"), } ) diff --git a/postgresql/provider.go b/postgresql/provider.go index 7f15c92e..d5b36a68 100644 --- a/postgresql/provider.go +++ b/postgresql/provider.go @@ -3,9 +3,10 @@ package postgresql import ( "context" "fmt" + "os" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" "github.com/Azure/azure-sdk-for-go/sdk/azidentity" - "os" "github.com/blang/semver" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" @@ -199,6 +200,7 @@ func Provider() *schema.Provider { "postgresql_function": resourcePostgreSQLFunction(), "postgresql_server": resourcePostgreSQLServer(), "postgresql_user_mapping": resourcePostgreSQLUserMapping(), + "postgresql_view": resourcePostgreSQLView(), }, DataSourcesMap: map[string]*schema.Resource{ diff --git a/postgresql/resource_postgresql_view.go b/postgresql/resource_postgresql_view.go index 9be87603..7b843ee2 100644 --- a/postgresql/resource_postgresql_view.go +++ b/postgresql/resource_postgresql_view.go @@ -23,6 +23,9 @@ const ( viewWithSecurityInvokerAttr = "with_security_invoker" viewDropCascadeAttr = "drop_cascade" + + // The private attributes for storing internal states of a view + internalStatesAttr = "internal_states" ) func resourcePostgreSQLView() *schema.Resource { @@ -51,7 +54,7 @@ func resourcePostgreSQLView() *schema.Resource { Optional: true, Computed: true, ForceNew: true, - Description: "The schema where the view is located. If not specified, the provider default is used.", + Description: "The schema where the view is located. If not specified, the provider default schema is used.", DiffSuppressFunc: defaultDiffSuppressFunc, }, @@ -67,10 +70,11 @@ func resourcePostgreSQLView() *schema.Resource { Description: "The query of the view.", DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { - return normalizeFunctionBody(new) == old + internalStates := d.Get(internalStatesAttr).(map[string]interface{}) + return internalStates["original_tf_view_query"] == new && internalStates["original_pg_view_query"] == old }, StateFunc: func(val interface{}) string { - return normalizeFunctionBody(val.(string)) + return val.(string) }, }, viewWithCheckOptionAttr: { @@ -98,27 +102,44 @@ func resourcePostgreSQLView() *schema.Resource { Optional: true, Default: false, }, + internalStatesAttr: { + Type: schema.TypeMap, + Computed: true, + }, }, } } func resourcePostgreSQLViewCreate(db *DBConnection, d *schema.ResourceData) error { - if !db.featureSupported(featureFunction) { + if !db.featureSupported(featureView) { return fmt.Errorf( "postgresql_view resource is not supported for this Postgres version (%s)", db.version, ) } + originalViewQuery := d.Get(viewQueryAttr).(string) + if err := createView(db, d, false); err != nil { return err } - return resourcePostgreSQLViewReadImpl(db, d) + if err := resourcePostgreSQLViewReadImpl(db, d); err != nil { + return err + } + + // Set internal states + pgViewQuery := d.Get(viewQueryAttr).(string) + internalStates := map[string]interface{}{ + "original_tf_view_query": originalViewQuery, + "original_pg_view_query": pgViewQuery, + } + d.Set(internalStatesAttr, internalStates) + return nil } func resourcePostgreSQLViewRead(db *DBConnection, d *schema.ResourceData) error { - if !db.featureSupported(featureFunction) { + if !db.featureSupported(featureView) { return fmt.Errorf( "postgresql_view resource is not supported for this Postgres version (%s)", db.version, @@ -129,22 +150,35 @@ func resourcePostgreSQLViewRead(db *DBConnection, d *schema.ResourceData) error } func resourcePostgreSQLViewUpdate(db *DBConnection, d *schema.ResourceData) error { - if !db.featureSupported(featureFunction) { + if !db.featureSupported(featureView) { return fmt.Errorf( "postgresql_view resource is not supported for this Postgres version (%s)", db.version, ) } + originalViewQuery := d.Get(viewQueryAttr).(string) + if err := createView(db, d, true); err != nil { return err } - return resourcePostgreSQLViewReadImpl(db, d) + if err := resourcePostgreSQLViewReadImpl(db, d); err != nil { + return err + } + + // Set internal states + pgViewQuery := d.Get(viewQueryAttr).(string) + internalStates := map[string]interface{}{ + "original_tf_view_query": originalViewQuery, + "original_pg_view_query": pgViewQuery, + } + d.Set(internalStatesAttr, internalStates) + return nil } func resourcePostgreSQLViewDelete(db *DBConnection, d *schema.ResourceData) error { - if !db.featureSupported(featureFunction) { + if !db.featureSupported(featureView) { return fmt.Errorf( "postgresql_view resource is not supported for this Postgres version (%s)", db.version, @@ -181,7 +215,7 @@ func resourcePostgreSQLViewDelete(db *DBConnection, d *schema.ResourceData) erro } func resourcePostgreSQLViewExists(db *DBConnection, d *schema.ResourceData) (bool, error) { - if !db.featureSupported(featureFunction) { + if !db.featureSupported(featureView) { return false, fmt.Errorf( "postgresql_view resource is not supported for this Postgres version (%s)", db.version, @@ -221,14 +255,16 @@ type PGView struct { WithCheckOption string WithSecurityBarrier bool WithSecurityInvoker bool + + DropCascade bool } type ViewInfo struct { - Database string - Schema string - Name string - Query string - Option []string + Database string `db:"database"` + Schema string `db:"schema"` + Name string `db:"name"` + Query string `db:"query"` + Options []string `db:"options"` } func resourcePostgreSQLViewReadImpl(db *DBConnection, d *schema.ResourceData) error { @@ -254,7 +290,7 @@ func resourcePostgreSQLViewReadImpl(db *DBConnection, d *schema.ResourceData) er `c.reloptions AS options ` + `FROM pg_class c ` + `JOIN pg_namespace n ON c.relnamespace = n.oid ` + - `WHERE c.relkind = 'v' AND n.nspname = '$1' AND c.relname = '$2'` + `WHERE c.relkind = 'v' AND n.nspname = $1 AND c.relname = $2` txn, err := startTransaction(db.client, databaseName) if err != nil { return err @@ -263,7 +299,7 @@ func resourcePostgreSQLViewReadImpl(db *DBConnection, d *schema.ResourceData) er viewIDParts := strings.Split(viewID, ".") var viewInfo ViewInfo - err = txn.QueryRow(query, viewIDParts[1], viewIDParts[2]).Scan(&viewInfo) + err = txn.QueryRow(query, viewIDParts[1], viewIDParts[2]).Scan(&viewInfo.Database, &viewInfo.Schema, &viewInfo.Name, &viewInfo.Query, pq.Array(&viewInfo.Options)) switch { case err == sql.ErrNoRows: log.Printf("[WARN] PostgreSQL view: %s", viewID) @@ -288,7 +324,10 @@ func resourcePostgreSQLViewReadImpl(db *DBConnection, d *schema.ResourceData) er d.Set(viewQueryAttr, pgView.Query) d.Set(viewWithCheckOptionAttr, pgView.WithCheckOption) d.Set(viewWithSecurityBarrierAttr, pgView.WithSecurityBarrier) - d.Set(viewWithCheckOptionAttr, pgView.WithSecurityInvoker) + d.Set(viewWithSecurityInvokerAttr, pgView.WithSecurityInvoker) + if dropCascadeAttr, ok := d.GetOk(viewDropCascadeAttr); ok { + d.Set(viewDropCascadeAttr, dropCascadeAttr.(bool)) + } d.SetId(viewID) @@ -306,8 +345,8 @@ func parseView(viewInfo ViewInfo) (PGView, error) { // 1. check_option (enum) - [LOCAL | CASCADED] // 2. security_barrier (boolean) // 3. security_invoker (boolean) - options := viewInfo.Option - if viewInfo.Option != nil && len(options) > 0 { + options := viewInfo.Options + if len(options) > 0 { for _, option := range options { parts := strings.Split(option, "=") if len(parts) != 2 { @@ -315,7 +354,7 @@ func parseView(viewInfo ViewInfo) (PGView, error) { } switch parts[0] { case "check_option": - pgView.WithCheckOption = parts[1] + pgView.WithCheckOption = strings.ToUpper(parts[1]) case "security_barrier": val, _ := strconv.ParseBool(parts[1]) pgView.WithSecurityBarrier = val diff --git a/postgresql/resource_postgresql_view_test.go b/postgresql/resource_postgresql_view_test.go index 737ac300..f5ba9486 100644 --- a/postgresql/resource_postgresql_view_test.go +++ b/postgresql/resource_postgresql_view_test.go @@ -3,6 +3,7 @@ package postgresql import ( "database/sql" "fmt" + "strings" "testing" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" @@ -13,28 +14,29 @@ func TestAccPostgresqlView_Basic(t *testing.T) { config := ` resource "postgresql_view" "basic_view" { name = "basic_view" - query = "SELECT * FROM tableA" + query = <<-EOF + SELECT * + FROM unnest(ARRAY[1]) AS element; + EOF } ` resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) - testCheckCompatibleVersion(t, featureFunction) + testCheckCompatibleVersion(t, featureView) }, Providers: testAccProviders, - CheckDestroy: testAccCheckPostgresqlFunctionDestroy, + CheckDestroy: testAccCheckPostgresqlViewDestroy, Steps: []resource.TestStep{ { Config: config, Check: resource.ComposeTestCheckFunc( testAccCheckPostgresqlViewExists("postgresql_view.basic_view", ""), resource.TestCheckResourceAttr( - "ppostgresql_view.basic_view", "schema", "public"), + "postgresql_view.basic_view", "schema", "public"), resource.TestCheckResourceAttr( "postgresql_view.basic_view", "name", "basic_view"), - resource.TestCheckResourceAttr( - "postgresql_view.basic_view", "query", "SELECT * FROM tableA"), resource.TestCheckResourceAttr( "postgresql_view.basic_view", "with_check_option", ""), resource.TestCheckResourceAttr( @@ -49,13 +51,176 @@ resource "postgresql_view" "basic_view" { }) } -func TestAccPostgresqlFunction_SpecificDatabase(t *testing.T) { +func TestAccPostgresqlView_SpecificDatabase(t *testing.T) { + skipIfNotAcc(t) + + dbSuffix, teardown := setupTestDatabase(t, true, true) + defer teardown() + + dbName, _ := getTestDBNames(dbSuffix) + + config := ` +resource "postgresql_view" "basic_view" { + database = "%s" + schema = "test_schema" + name = "basic_view" + query = <<-EOF + SELECT * + FROM unnest(ARRAY[1]) AS element; + EOF +} +` + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testCheckCompatibleVersion(t, featureView) + }, + Providers: testAccProviders, + CheckDestroy: testAccCheckPostgresqlViewDestroy, + Steps: []resource.TestStep{ + { + Config: fmt.Sprintf(config, dbName), + Check: resource.ComposeTestCheckFunc( + testAccCheckPostgresqlViewExists("postgresql_view.basic_view", dbName), + resource.TestCheckResourceAttr( + "postgresql_view.basic_view", "database", dbName), + resource.TestCheckResourceAttr( + "postgresql_view.basic_view", "schema", "test_schema"), + resource.TestCheckResourceAttr( + "postgresql_view.basic_view", "name", "basic_view"), + resource.TestCheckResourceAttr( + "postgresql_view.basic_view", "with_check_option", ""), + ), + }, + }, + }) +} + +func TestAccPostgresqlView_AllOptions(t *testing.T) { skipIfNotAcc(t) dbSuffix, teardown := setupTestDatabase(t, true, true) defer teardown() dbName, _ := getTestDBNames(dbSuffix) + + config := ` +resource "postgresql_view" "all_option_view" { + database = "%s" + schema = "test_schema" + name = "all_option_view" + query = <<-EOF + SELECT schemaname, tablename + FROM pg_catalog.pg_tables; + EOF + with_check_option = "CASCADED" + with_security_barrier = true + with_security_invoker = true + drop_cascade = true +} +` + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testCheckCompatibleVersion(t, featureView) + }, + Providers: testAccProviders, + CheckDestroy: testAccCheckPostgresqlViewDestroy, + Steps: []resource.TestStep{ + { + Config: fmt.Sprintf(config, dbName), + Check: resource.ComposeTestCheckFunc( + testAccCheckPostgresqlViewExists("postgresql_view.all_option_view", dbName), + resource.TestCheckResourceAttr( + "postgresql_view.all_option_view", "database", dbName), + resource.TestCheckResourceAttr( + "postgresql_view.all_option_view", "schema", "test_schema"), + resource.TestCheckResourceAttr( + "postgresql_view.all_option_view", "name", "all_option_view"), + resource.TestCheckResourceAttr( + "postgresql_view.all_option_view", "with_check_option", "CASCADED"), + resource.TestCheckResourceAttr( + "postgresql_view.all_option_view", "with_security_barrier", "true"), + resource.TestCheckResourceAttr( + "postgresql_view.all_option_view", "with_security_invoker", "true"), + resource.TestCheckResourceAttr( + "postgresql_view.all_option_view", "drop_cascade", "true"), + ), + }, + }, + }) +} + +func TestAccPostgresqlView_Update(t *testing.T) { + configCreate := ` +resource "postgresql_view" "pg_view" { + name = "pg_view" + query = <<-EOF + SELECT schemaname, tablename + FROM pg_catalog.pg_tables; + EOF +} +` + + configUpdate := ` +resource "postgresql_view" "pg_view" { + name = "pg_view" + query = <<-EOF + SELECT schemaname, tablename, tableowner + FROM pg_catalog.pg_tables; + EOF + with_check_option = "CASCADED" + with_security_barrier = true + with_security_invoker = true + drop_cascade = true +} +` + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testCheckCompatibleVersion(t, featureView) + }, + Providers: testAccProviders, + CheckDestroy: testAccCheckPostgresqlViewDestroy, + Steps: []resource.TestStep{ + { + Config: configCreate, + Check: resource.ComposeTestCheckFunc( + testAccCheckPostgresqlViewExists("postgresql_view.pg_view", ""), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "schema", "public"), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "name", "pg_view"), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "with_check_option", ""), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "with_security_barrier", "false"), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "with_security_invoker", "false"), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "drop_cascade", "false"), + ), + }, + { + Config: configUpdate, + Check: resource.ComposeTestCheckFunc( + testAccCheckPostgresqlViewExists("postgresql_view.pg_view", ""), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "schema", "public"), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "name", "pg_view"), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "with_check_option", "CASCADED"), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "with_security_barrier", "true"), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "with_security_invoker", "true"), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "drop_cascade", "true"), + ), + }, + }, + }) } func testAccCheckPostgresqlViewExists(n string, database string) resource.TestCheckFunc { @@ -78,20 +243,52 @@ func testAccCheckPostgresqlViewExists(n string, database string) resource.TestCh } defer deferredRollback(txn) - exists, err := checkFunctionExists(txn, signature) + exists, err := checkViewExists(txn, signature) if err != nil { return fmt.Errorf("Error checking function %s", err) } if !exists { - return fmt.Errorf("Function not found") + return fmt.Errorf("View not found") } return nil } } +func testAccCheckPostgresqlViewDestroy(s *terraform.State) error { + client := testAccProvider.Meta().(*Client) + + for _, rs := range s.RootModule().Resources { + if rs.Type != "postgresql_view" { + continue + } + + txn, err := startTransaction(client, "") + if err != nil { + return err + } + defer deferredRollback(txn) + + viewParts := strings.Split(rs.Primary.ID, ".") + _, schemaName, viewName := viewParts[0], viewParts[1], viewParts[2] + viewIdentifier := fmt.Sprintf("%s.%s", schemaName, viewName) + + exists, err := checkViewExists(txn, viewIdentifier) + + if err != nil { + return fmt.Errorf("Error checking view %s", err) + } + + if exists { + return fmt.Errorf("View still exists after destroy") + } + } + + return nil +} + func checkViewExists(txn *sql.Tx, signature string) (bool, error) { var _rez bool err := txn.QueryRow(fmt.Sprintf("SELECT to_regclass('%s') IS NOT NULL", signature)).Scan(&_rez) diff --git a/website/docs/r/postgresql_view.html.markdown b/website/docs/r/postgresql_view.html.markdown new file mode 100644 index 00000000..ef728ed6 --- /dev/null +++ b/website/docs/r/postgresql_view.html.markdown @@ -0,0 +1,56 @@ +--- +layout: "postgresql" +page_title: "PostgreSQL: postgresql_view" +sidebar_current: "docs-postgresql-resource-postgresql_view" +description: |- +Creates and manages a view on a PostgreSQL server. +--- + +# postgresql_view + +The `postgresql_view` resource creates and manages a view on a PostgreSQL server. + +## Usage + +```hcl +resource "postgresql_view" "aggregation_view" { + database = "database_name" + schema = "schema_name" + name = "aggregation_view" + query = <<-EOF + SELECT schemaname, tablename + FROM pg_catalog.pg_tables; + EOF + with_check_option = "CASCADED" + with_security_barrier = true + with_security_invoker = true + drop_cascade = true +} +``` + +## Argument Reference + +- `database` - (Optional) The database where the view is located. + If not specified, the view is created in the current database. + +- `schema` - (Optional) The schema where the view is located. + If not specified, the view is created in the current schema. + +- `name` - (Required) The name of the view. + +- `query` - (Required) The query. + +- `with_check_option` - (Optional) The check option which controls the behavior + of automatically updatable views. One of: CASCADED, LOCAL. + +- `with_security_barrier` - (Optional) This should be used if the view is intended to provide row-level security. + +- `with_security_invoker` - (Optional) This option causes the underlying base relations to be + checked against the privileges of the user of the view rather than the view owner. + +- `drop_cascade` - (Optional) - True tp automatically drop objects that depend on the view (such as other views), + and in turn all objects that depend on those objects. Default is false. + +## Postgres documentation + +- https://www.postgresql.org/docs/16/sql-createview.html From 15ab45104c158133017a52b3d1190542ab245f79 Mon Sep 17 00:00:00 2001 From: Nguyen Date: Thu, 9 May 2024 23:43:16 +0700 Subject: [PATCH 5/7] Add default values in docs --- website/docs/r/postgresql_view.html.markdown | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/website/docs/r/postgresql_view.html.markdown b/website/docs/r/postgresql_view.html.markdown index ef728ed6..2f4a3e43 100644 --- a/website/docs/r/postgresql_view.html.markdown +++ b/website/docs/r/postgresql_view.html.markdown @@ -41,12 +41,12 @@ resource "postgresql_view" "aggregation_view" { - `query` - (Required) The query. - `with_check_option` - (Optional) The check option which controls the behavior - of automatically updatable views. One of: CASCADED, LOCAL. + of automatically updatable views. One of: CASCADED, LOCAL. Default is not set. -- `with_security_barrier` - (Optional) This should be used if the view is intended to provide row-level security. +- `with_security_barrier` - (Optional) This should be used if the view is intended to provide row-level security. Default is false. - `with_security_invoker` - (Optional) This option causes the underlying base relations to be - checked against the privileges of the user of the view rather than the view owner. + checked against the privileges of the user of the view rather than the view owner. Default is false. - `drop_cascade` - (Optional) - True tp automatically drop objects that depend on the view (such as other views), and in turn all objects that depend on those objects. Default is false. From 238c12efc673c8c64cbefe968a7f22b743454337 Mon Sep 17 00:00:00 2001 From: Nguyen Date: Sun, 26 May 2024 08:08:59 +0700 Subject: [PATCH 6/7] Drop view if exists before creating/replacing --- postgresql/resource_postgresql_view.go | 84 ++++++++++++--------- postgresql/resource_postgresql_view_test.go | 6 +- 2 files changed, 53 insertions(+), 37 deletions(-) diff --git a/postgresql/resource_postgresql_view.go b/postgresql/resource_postgresql_view.go index 7b843ee2..97d95c0e 100644 --- a/postgresql/resource_postgresql_view.go +++ b/postgresql/resource_postgresql_view.go @@ -24,8 +24,15 @@ const ( viewDropCascadeAttr = "drop_cascade" - // The private attributes for storing internal states of a view - internalStatesAttr = "internal_states" + // Postgres may parse and rewrite the view query with its own rules. + // So the value in viewQueryAttr is enough to detect if a query has been + // modified in Postgres without Terraform's awareness. These two additional + // states need to be stored for the detection: + // + // Stores the current parsed/rewritten query in Postgres. + internalPGParsedQueryAttr = "internal_pg_parsed_query" + // Stores the last updated parsed/rewritten query has been recorded in Terraform. + internalTFParsedQueryAttr = "internal_tf_parsed_query" ) func resourcePostgreSQLView() *schema.Resource { @@ -68,14 +75,6 @@ func resourcePostgreSQLView() *schema.Resource { Type: schema.TypeString, Required: true, Description: "The query of the view.", - - DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { - internalStates := d.Get(internalStatesAttr).(map[string]interface{}) - return internalStates["original_tf_view_query"] == new && internalStates["original_pg_view_query"] == old - }, - StateFunc: func(val interface{}) string { - return val.(string) - }, }, viewWithCheckOptionAttr: { Type: schema.TypeString, @@ -102,8 +101,12 @@ func resourcePostgreSQLView() *schema.Resource { Optional: true, Default: false, }, - internalStatesAttr: { - Type: schema.TypeMap, + internalPGParsedQueryAttr: { + Type: schema.TypeString, + Computed: true, + }, + internalTFParsedQueryAttr: { + Type: schema.TypeString, Computed: true, }, }, @@ -118,8 +121,6 @@ func resourcePostgreSQLViewCreate(db *DBConnection, d *schema.ResourceData) erro ) } - originalViewQuery := d.Get(viewQueryAttr).(string) - if err := createView(db, d, false); err != nil { return err } @@ -128,13 +129,7 @@ func resourcePostgreSQLViewCreate(db *DBConnection, d *schema.ResourceData) erro return err } - // Set internal states - pgViewQuery := d.Get(viewQueryAttr).(string) - internalStates := map[string]interface{}{ - "original_tf_view_query": originalViewQuery, - "original_pg_view_query": pgViewQuery, - } - d.Set(internalStatesAttr, internalStates) + d.Set(internalTFParsedQueryAttr, d.Get(internalPGParsedQueryAttr).(string)) return nil } @@ -146,7 +141,19 @@ func resourcePostgreSQLViewRead(db *DBConnection, d *schema.ResourceData) error ) } - return resourcePostgreSQLViewReadImpl(db, d) + err := resourcePostgreSQLViewReadImpl(db, d) + if err != nil { + return err + } + + // Detect if the query has been modified in Postgres without Terraform's awareness. + // For this kind of error, the users must consolidate manually. + tfQuery := d.Get(internalTFParsedQueryAttr).(string) + pgQuery := d.Get(internalPGParsedQueryAttr).(string) + if tfQuery != pgQuery { + return fmt.Errorf("the view: '%s' has been modified in Postgres", d.Get(viewNameAttr).(string)) + } + return nil } func resourcePostgreSQLViewUpdate(db *DBConnection, d *schema.ResourceData) error { @@ -157,8 +164,6 @@ func resourcePostgreSQLViewUpdate(db *DBConnection, d *schema.ResourceData) erro ) } - originalViewQuery := d.Get(viewQueryAttr).(string) - if err := createView(db, d, true); err != nil { return err } @@ -167,13 +172,7 @@ func resourcePostgreSQLViewUpdate(db *DBConnection, d *schema.ResourceData) erro return err } - // Set internal states - pgViewQuery := d.Get(viewQueryAttr).(string) - internalStates := map[string]interface{}{ - "original_tf_view_query": originalViewQuery, - "original_pg_view_query": pgViewQuery, - } - d.Set(internalStatesAttr, internalStates) + d.Set(internalTFParsedQueryAttr, d.Get(internalPGParsedQueryAttr).(string)) return nil } @@ -321,13 +320,15 @@ func resourcePostgreSQLViewReadImpl(db *DBConnection, d *schema.ResourceData) er d.Set(viewDatabaseAttr, pgView.Database) d.Set(viewSchemaAttr, pgView.Schema) d.Set(viewNameAttr, pgView.Name) - d.Set(viewQueryAttr, pgView.Query) + d.Set(viewQueryAttr, d.Get(viewQueryAttr).(string)) d.Set(viewWithCheckOptionAttr, pgView.WithCheckOption) d.Set(viewWithSecurityBarrierAttr, pgView.WithSecurityBarrier) d.Set(viewWithSecurityInvokerAttr, pgView.WithSecurityInvoker) if dropCascadeAttr, ok := d.GetOk(viewDropCascadeAttr); ok { d.Set(viewDropCascadeAttr, dropCascadeAttr.(bool)) } + // Internal states + d.Set(internalPGParsedQueryAttr, pgView.Query) d.SetId(viewID) @@ -397,6 +398,11 @@ func createView(db *DBConnection, d *schema.ResourceData, replace bool) error { name := d.Get(viewNameAttr).(string) query := d.Get(viewQueryAttr).(string) + fullViewNameBuffer := bytes.NewBufferString(pq.QuoteIdentifier(schemaName)) + fullViewNameBuffer.WriteString(".") + fullViewNameBuffer.WriteString(pq.QuoteIdentifier(name)) + fullViewName := fullViewNameBuffer.String() + // Construct the view b := bytes.NewBufferString("CREATE ") if replace { @@ -405,8 +411,7 @@ func createView(db *DBConnection, d *schema.ResourceData, replace bool) error { b.WriteString("VIEW ") - fmt.Fprint(b, pq.QuoteIdentifier(schemaName), ".") - fmt.Fprint(b, pq.QuoteIdentifier(name)) + fmt.Fprint(b, fullViewName) // With options var withOptions []string @@ -427,6 +432,12 @@ func createView(db *DBConnection, d *schema.ResourceData, replace bool) error { fmt.Fprint(b, " AS\n", query) b.WriteRune(';') + // Drop view command + dropViewSqlBuffer := bytes.NewBufferString("DROP VIEW IF EXISTS ") + dropViewSqlBuffer.WriteString(fullViewName) + dropViewSqlBuffer.WriteString(" RESTRICT;") + dropViewSql := dropViewSqlBuffer.String() + sql := b.String() txn, err := startTransaction(db.client, d.Get(viewDatabaseAttr).(string)) if err != nil { @@ -434,6 +445,11 @@ func createView(db *DBConnection, d *schema.ResourceData, replace bool) error { } defer deferredRollback(txn) + // Drop view if exist + if _, err := txn.Exec(dropViewSql); err != nil { + return err + } + if _, err := txn.Exec(sql); err != nil { return err } diff --git a/postgresql/resource_postgresql_view_test.go b/postgresql/resource_postgresql_view_test.go index f5ba9486..225db1e4 100644 --- a/postgresql/resource_postgresql_view_test.go +++ b/postgresql/resource_postgresql_view_test.go @@ -290,8 +290,8 @@ func testAccCheckPostgresqlViewDestroy(s *terraform.State) error { } func checkViewExists(txn *sql.Tx, signature string) (bool, error) { - var _rez bool - err := txn.QueryRow(fmt.Sprintf("SELECT to_regclass('%s') IS NOT NULL", signature)).Scan(&_rez) + var exists bool + err := txn.QueryRow(fmt.Sprintf("SELECT to_regclass('%s') IS NOT NULL", signature)).Scan(&exists) switch { case err == sql.ErrNoRows: return false, nil @@ -299,5 +299,5 @@ func checkViewExists(txn *sql.Tx, signature string) (bool, error) { return false, fmt.Errorf("Error reading info about view: %s", err) } - return _rez, nil + return exists, nil } From 6b84f2dd075ac186cc67605f5f383b77795ddc17 Mon Sep 17 00:00:00 2001 From: Nguyen Date: Sun, 26 May 2024 22:28:58 +0700 Subject: [PATCH 7/7] Remove unnecessary replace keyword --- postgresql/resource_postgresql_view.go | 14 +- postgresql/resource_postgresql_view_test.go | 211 +++++++++++++++++++- 2 files changed, 210 insertions(+), 15 deletions(-) diff --git a/postgresql/resource_postgresql_view.go b/postgresql/resource_postgresql_view.go index 97d95c0e..90c4971b 100644 --- a/postgresql/resource_postgresql_view.go +++ b/postgresql/resource_postgresql_view.go @@ -121,7 +121,7 @@ func resourcePostgreSQLViewCreate(db *DBConnection, d *schema.ResourceData) erro ) } - if err := createView(db, d, false); err != nil { + if err := createView(db, d); err != nil { return err } @@ -164,7 +164,7 @@ func resourcePostgreSQLViewUpdate(db *DBConnection, d *schema.ResourceData) erro ) } - if err := createView(db, d, true); err != nil { + if err := createView(db, d); err != nil { return err } @@ -389,7 +389,7 @@ func genViewID(db *DBConnection, d *schema.ResourceData) (string, error) { return b.String(), nil } -func createView(db *DBConnection, d *schema.ResourceData, replace bool) error { +func createView(db *DBConnection, d *schema.ResourceData) error { schemaName := "public" if v, ok := d.GetOk(viewSchemaAttr); ok { schemaName = v.(string) @@ -404,13 +404,7 @@ func createView(db *DBConnection, d *schema.ResourceData, replace bool) error { fullViewName := fullViewNameBuffer.String() // Construct the view - b := bytes.NewBufferString("CREATE ") - if replace { - b.WriteString("OR REPLACE ") - } - - b.WriteString("VIEW ") - + b := bytes.NewBufferString("CREATE VIEW ") fmt.Fprint(b, fullViewName) // With options diff --git a/postgresql/resource_postgresql_view_test.go b/postgresql/resource_postgresql_view_test.go index 225db1e4..01beb628 100644 --- a/postgresql/resource_postgresql_view_test.go +++ b/postgresql/resource_postgresql_view_test.go @@ -51,6 +51,88 @@ resource "postgresql_view" "basic_view" { }) } +func TestAccPostgresqlView_CaseSensitiveViewName(t *testing.T) { + config := ` +resource "postgresql_view" "case_sensitive_view_name" { + name = "Case_Sensitive_View_Name" + query = <<-EOF + SELECT 1 AS one; + EOF +} +` + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testCheckCompatibleVersion(t, featureView) + }, + Providers: testAccProviders, + CheckDestroy: testAccCheckPostgresqlViewDestroy, + Steps: []resource.TestStep{ + { + Config: config, + Check: resource.ComposeTestCheckFunc( + testAccCheckPostgresqlViewExists("postgresql_view.case_sensitive_view_name", ""), + resource.TestCheckResourceAttr( + "postgresql_view.case_sensitive_view_name", "schema", "public"), + resource.TestCheckResourceAttr( + "postgresql_view.case_sensitive_view_name", "name", "Case_Sensitive_View_Name"), + resource.TestCheckResourceAttr( + "postgresql_view.case_sensitive_view_name", "with_check_option", ""), + resource.TestCheckResourceAttr( + "postgresql_view.case_sensitive_view_name", "with_security_barrier", "false"), + resource.TestCheckResourceAttr( + "postgresql_view.case_sensitive_view_name", "with_security_invoker", "false"), + resource.TestCheckResourceAttr( + "postgresql_view.case_sensitive_view_name", "drop_cascade", "false"), + ), + }, + }, + }) +} + +func TestAccPostgresqlView_QueryWithDoubleQuotes(t *testing.T) { + config := ` +resource "postgresql_view" "double_quotes_query_view" { + name = "double_quotes_query_view" + query = <<-EOF +SELECT 1 AS "One", 2 AS two; + EOF +} +` + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testCheckCompatibleVersion(t, featureView) + }, + Providers: testAccProviders, + CheckDestroy: testAccCheckPostgresqlViewDestroy, + Steps: []resource.TestStep{ + { + Config: config, + Check: resource.ComposeTestCheckFunc( + testAccCheckPostgresqlViewExists("postgresql_view.double_quotes_query_view", ""), + resource.TestCheckResourceAttr( + "postgresql_view.double_quotes_query_view", "schema", "public"), + resource.TestCheckResourceAttr( + "postgresql_view.double_quotes_query_view", "name", "double_quotes_query_view"), + resource.TestCheckResourceAttr( + "postgresql_view.double_quotes_query_view", "query", "SELECT 1 AS \"One\", 2 AS two;\n"), + resource.TestCheckResourceAttr( + "postgresql_view.double_quotes_query_view", "with_check_option", ""), + resource.TestCheckResourceAttr( + "postgresql_view.double_quotes_query_view", "with_security_barrier", "false"), + resource.TestCheckResourceAttr( + "postgresql_view.double_quotes_query_view", "with_security_invoker", "false"), + resource.TestCheckResourceAttr( + "postgresql_view.double_quotes_query_view", "drop_cascade", "false"), + ), + }, + }, + }) +} + func TestAccPostgresqlView_SpecificDatabase(t *testing.T) { skipIfNotAcc(t) @@ -223,6 +305,124 @@ resource "postgresql_view" "pg_view" { }) } +func TestAccPostgresqlView_QuerySelectColumnChange(t *testing.T) { + configCreate := ` +resource "postgresql_view" "pg_view" { + name = "pg_view" + query = "SELECT 1 as one;" +} +` + + configUpdateAddColumn := ` +resource "postgresql_view" "pg_view" { + name = "pg_view" + query = "SELECT 1 as one, 2 as two;" +}` + + configUpdateReplaceColumn := ` +resource "postgresql_view" "pg_view" { + name = "pg_view" + query = "SELECT 1 as one, 3 as three;" +}` + + configUpdateDeleteColumn := ` +resource "postgresql_view" "pg_view" { + name = "pg_view" + query = "SELECT 1 as one;" +}` + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testCheckCompatibleVersion(t, featureView) + }, + Providers: testAccProviders, + CheckDestroy: testAccCheckPostgresqlViewDestroy, + Steps: []resource.TestStep{ + { + Config: configCreate, + Check: resource.ComposeTestCheckFunc( + testAccCheckPostgresqlViewExists("postgresql_view.pg_view", ""), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "schema", "public"), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "name", "pg_view"), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "query", "SELECT 1 as one;"), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "with_check_option", ""), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "with_security_barrier", "false"), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "with_security_invoker", "false"), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "drop_cascade", "false"), + ), + }, + { + Config: configUpdateAddColumn, + Check: resource.ComposeTestCheckFunc( + testAccCheckPostgresqlViewExists("postgresql_view.pg_view", ""), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "schema", "public"), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "name", "pg_view"), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "query", "SELECT 1 as one, 2 as two;"), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "with_check_option", ""), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "with_security_barrier", "false"), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "with_security_invoker", "false"), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "drop_cascade", "false"), + ), + }, + { + Config: configUpdateReplaceColumn, + Check: resource.ComposeTestCheckFunc( + testAccCheckPostgresqlViewExists("postgresql_view.pg_view", ""), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "schema", "public"), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "name", "pg_view"), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "query", "SELECT 1 as one, 3 as three;"), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "with_check_option", ""), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "with_security_barrier", "false"), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "with_security_invoker", "false"), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "drop_cascade", "false"), + ), + }, + { + Config: configUpdateDeleteColumn, + Check: resource.ComposeTestCheckFunc( + testAccCheckPostgresqlViewExists("postgresql_view.pg_view", ""), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "schema", "public"), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "name", "pg_view"), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "query", "SELECT 1 as one;"), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "with_check_option", ""), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "with_security_barrier", "false"), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "with_security_invoker", "false"), + resource.TestCheckResourceAttr( + "postgresql_view.pg_view", "drop_cascade", "false"), + ), + }, + }, + }) +} + func testAccCheckPostgresqlViewExists(n string, database string) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] @@ -265,16 +465,14 @@ func testAccCheckPostgresqlViewDestroy(s *terraform.State) error { continue } + viewIdentifier := rs.Primary.ID + txn, err := startTransaction(client, "") if err != nil { return err } defer deferredRollback(txn) - viewParts := strings.Split(rs.Primary.ID, ".") - _, schemaName, viewName := viewParts[0], viewParts[1], viewParts[2] - viewIdentifier := fmt.Sprintf("%s.%s", schemaName, viewName) - exists, err := checkViewExists(txn, viewIdentifier) if err != nil { @@ -291,7 +489,10 @@ func testAccCheckPostgresqlViewDestroy(s *terraform.State) error { func checkViewExists(txn *sql.Tx, signature string) (bool, error) { var exists bool - err := txn.QueryRow(fmt.Sprintf("SELECT to_regclass('%s') IS NOT NULL", signature)).Scan(&exists) + signatureParts := strings.Split(signature, ".") + schema := signatureParts[1] + viewName := signatureParts[2] + err := txn.QueryRow(fmt.Sprintf("SELECT viewname IS NOT NULL FROM pg_views where schemaname = '%s' AND viewname = '%s'", schema, viewName)).Scan(&exists) switch { case err == sql.ErrNoRows: return false, nil