From ccb7cd9906cb926dfbd25b391394fc783a3f948b Mon Sep 17 00:00:00 2001 From: Jakub Michalak Date: Tue, 10 Sep 2024 14:59:12 +0200 Subject: [PATCH 1/9] Row access policy resource v1 --- MIGRATION_GUIDE.md | 28 +- docs/resources/row_access_policy.md | 62 ++- .../snowflake_row_access_policy/import.sh | 3 +- .../snowflake_row_access_policy/resource.tf | 9 +- .../assert/objectassert/gen/sdk_object_def.go | 5 + .../helpers/row_access_policy_client.go | 8 + pkg/resources/row_access_policy.go | 245 ++++++++---- .../row_access_policy_acceptance_test.go | 371 ++++++++++++++++-- .../row_access_policy_state_upgraders.go | 27 ++ .../TestAcc_RowAccessPolicy/1/test.tf | 14 +- .../TestAcc_RowAccessPolicy/2/test.tf | 14 +- .../TestAcc_RowAccessPolicy/3/test.tf | 14 +- .../TestAcc_RowAccessPolicy/4/test.tf | 15 + .../TestAcc_RowAccessPolicy/4/variables.tf | 11 + .../TestAcc_RowAccessPolicy/basic/test.tf | 13 + .../basic/variables.tf | 15 + pkg/schemas/row_access_policy.go | 34 ++ templates/resources/row_access_policy.md.tmpl | 35 ++ 18 files changed, 788 insertions(+), 135 deletions(-) create mode 100644 pkg/resources/row_access_policy_state_upgraders.go create mode 100644 pkg/resources/testdata/TestAcc_RowAccessPolicy/4/test.tf create mode 100644 pkg/resources/testdata/TestAcc_RowAccessPolicy/4/variables.tf create mode 100644 pkg/resources/testdata/TestAcc_RowAccessPolicy/basic/test.tf create mode 100644 pkg/resources/testdata/TestAcc_RowAccessPolicy/basic/variables.tf create mode 100644 pkg/schemas/row_access_policy.go create mode 100644 templates/resources/row_access_policy.md.tmpl diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index 22d421a5e7..72c608ae91 100644 --- a/MIGRATION_GUIDE.md +++ b/MIGRATION_GUIDE.md @@ -4,6 +4,26 @@ This document is meant to help you migrate your Terraform config to the new newe describe deprecations or breaking changes and help you to change your configuration to keep the same (or similar) behavior across different versions. +## v0.95.x ➞ v0.96.0 +### snowflake_row_access_policy resource changes +New fields: + - `show_output` field that holds the response from SHOW ROW ACCESS POLICIES. + - `describe_output` field that holds the response from DESCRIBE ROW ACCESS POLICIES. + +#### *(breaking change)* Renamed fields in snowflake_row_access_policy resource +Renamed fields: + - `row_access_expression` to `body` + - `signature` to `arguments`. Also, the field type is changed from map to a list of arguments. +to align with Snowflake docs. Please rename this field in your configuration files. State will be migrated automatically. + +#### *(breaking change)* Identifiers related changes +During [identifiers rework](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/ROADMAP.md#identifiers-rework) we decided to +migrate resource ids from pipe-separated to regular Snowflake identifiers (e.g. `|` -> `"".""`). + +Also, we added diff suppress function that prevents Terraform from showing differences, when only quoting is different. + +No change is required, the state will be migrated automatically. + ## v0.94.x ➞ v0.95.0 ### *(breaking change)* database roles data source; field rename, schema structure changes, and adding missing filtering options @@ -37,7 +57,7 @@ New output fields Breaking changes: - `database` and `schema` are right now under `in` field -- `views` field now organizes output of show under `show_output` field and the output of describe under `describe_output` field. +- `views` field now organizes output of show under `show_output` field and the output of describe under `describe_output` field. ### snowflake_view resource changes New fields: @@ -133,7 +153,7 @@ Because of introducing a new `fully_qualified_name` field for all of the resourc Correctly handle the situation when stage was rename/deleted externally (earlier it resulted in a permanent loop). No action is required on the user's side. -Connected issues: [#2972](https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2972) +Connected issues: [#2972](https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2972) ### snowflake_table resource changes @@ -289,8 +309,8 @@ Type changes: - `disabled`: bool -> string (To easily handle three-value logic (true, false, unknown) in provider's configs, read more in https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/751239b7d2fee4757471db6c03b952d4728ee099/v1-preparations/CHANGES_BEFORE_V1.md?plain=1#L24) #### *(breaking change)* refactored snowflake_users datasource -> **IMPORTANT NOTE:** when querying users you don't have permissions to, the querying options are limited. -You won't get almost any field in `show_output` (only empty or default values), the DESCRIBE command cannot be called, so you have to set `with_describe = false`. +> **IMPORTANT NOTE:** when querying users you don't have permissions to, the querying options are limited. +You won't get almost any field in `show_output` (only empty or default values), the DESCRIBE command cannot be called, so you have to set `with_describe = false`. Only `parameters` output is not affected by the lack of privileges. Changes: diff --git a/docs/resources/row_access_policy.md b/docs/resources/row_access_policy.md index f7339bd879..49ced88b61 100644 --- a/docs/resources/row_access_policy.md +++ b/docs/resources/row_access_policy.md @@ -5,6 +5,8 @@ description: |- --- +!> **V1 release candidate** This resource was reworked and is a release candidate for the V1. We do not expect significant changes in it before the V1. We will welcome any feedback and adjust the resource if needed. Any errors reported will be resolved with a higher priority. We encourage checking this resource out before the V1 release. Please follow the [migration guide](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/MIGRATION_GUIDE.md#v0950--v0960) to use it. + # snowflake_row_access_policy (Resource) @@ -16,14 +18,14 @@ resource "snowflake_row_access_policy" "example_row_access_policy" { name = "EXAMPLE_ROW_ACCESS_POLICY" database = "EXAMPLE_DB" schema = "EXAMPLE_SCHEMA" - signature = { - A = "VARCHAR", - B = "VARCHAR" + argument { + name = "ARG1" + type = "VARCHAR" } - row_access_expression = "case when current_role() in ('ANALYST') then true else false end" + body = "case when current_role() in ('ANALYST') then true else false end" + comment = "comment" } ``` - -> **Note** Instead of using fully_qualified_name, you can reference objects managed outside Terraform by constructing a correct ID, consult [identifiers guide](https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs/guides/identifiers#new-computed-fully-qualified-name-field-in-resources). @@ -32,11 +34,11 @@ resource "snowflake_row_access_policy" "example_row_access_policy" { ### Required -- `database` (String) The database in which to create the row access policy. -- `name` (String) Specifies the identifier for the row access policy; must be unique for the database and schema in which the row access policy is created. -- `row_access_expression` (String) Specifies the SQL expression. The expression can be any boolean-valued SQL expression. -- `schema` (String) The schema in which to create the row access policy. -- `signature` (Map of String) Specifies signature (arguments) for the row access policy (uppercase and sorted to avoid recreation of resource). A signature specifies a set of attributes that must be considered to determine whether the row is accessible. The attribute values come from the database object (e.g. table or view) to be protected by the row access policy. +- `argument` (Block List, Min: 1) List of the arguments for the row access policy. A signature specifies a set of attributes that must be considered to determine whether the row is accessible. The attribute values come from the database object (e.g. table or view) to be protected by the row access policy. If any argument name or type is changed, the resource is recreated. (see [below for nested schema](#nestedblock--argument)) +- `body` (String) Specifies the SQL expression. The expression can be any boolean-valued SQL expression. +- `database` (String) The database in which to create the row access policy. Due to technical limitations (read more [here](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/technical-documentation/identifiers_rework_design_decisions.md#known-limitations-and-identifier-recommendations)), avoid using the following characters: `|`, `.`, `(`, `)`, `"` +- `name` (String) Specifies the identifier for the row access policy; must be unique for the database and schema in which the row access policy is created. Due to technical limitations (read more [here](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/technical-documentation/identifiers_rework_design_decisions.md#known-limitations-and-identifier-recommendations)), avoid using the following characters: `|`, `.`, `(`, `)`, `"` +- `schema` (String) The schema in which to create the row access policy. Due to technical limitations (read more [here](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/technical-documentation/identifiers_rework_design_decisions.md#known-limitations-and-identifier-recommendations)), avoid using the following characters: `|`, `.`, `(`, `)`, `"` ### Optional @@ -44,14 +46,50 @@ resource "snowflake_row_access_policy" "example_row_access_policy" { ### Read-Only +- `describe_output` (List of Object) Outputs the result of `DESCRIBE ROW ACCESS POLICY` for the given row access policy. (see [below for nested schema](#nestedatt--describe_output)) - `fully_qualified_name` (String) Fully qualified name of the resource. For more information, see [object name resolution](https://docs.snowflake.com/en/sql-reference/name-resolution). - `id` (String) The ID of this resource. +- `show_output` (List of Object) Outputs the result of `SHOW ROW ACCESS POLICY` for the given row access policy. (see [below for nested schema](#nestedatt--show_output)) + + +### Nested Schema for `argument` + +Required: + +- `name` (String) The argument name +- `type` (String) The argument type + + + +### Nested Schema for `describe_output` + +Read-Only: + +- `body` (String) +- `name` (String) +- `return_type` (String) +- `signature` (String) + + + +### Nested Schema for `show_output` + +Read-Only: + +- `comment` (String) +- `created_on` (String) +- `database_name` (String) +- `kind` (String) +- `name` (String) +- `options` (String) +- `owner` (String) +- `owner_role_type` (String) +- `schema_name` (String) ## Import Import is supported using the following syntax: ```shell -# format is database name | schema name | policy name -terraform import snowflake_row_access_policy.example 'dbName|schemaName|policyName' +terraform import snowflake_row_access_policy.example '""."".""' ``` diff --git a/examples/resources/snowflake_row_access_policy/import.sh b/examples/resources/snowflake_row_access_policy/import.sh index e314b353a6..efc882a08e 100644 --- a/examples/resources/snowflake_row_access_policy/import.sh +++ b/examples/resources/snowflake_row_access_policy/import.sh @@ -1,2 +1 @@ -# format is database name | schema name | policy name -terraform import snowflake_row_access_policy.example 'dbName|schemaName|policyName' +terraform import snowflake_row_access_policy.example '""."".""' diff --git a/examples/resources/snowflake_row_access_policy/resource.tf b/examples/resources/snowflake_row_access_policy/resource.tf index a3f68a570a..7e256f9700 100644 --- a/examples/resources/snowflake_row_access_policy/resource.tf +++ b/examples/resources/snowflake_row_access_policy/resource.tf @@ -2,9 +2,10 @@ resource "snowflake_row_access_policy" "example_row_access_policy" { name = "EXAMPLE_ROW_ACCESS_POLICY" database = "EXAMPLE_DB" schema = "EXAMPLE_SCHEMA" - signature = { - A = "VARCHAR", - B = "VARCHAR" + argument { + name = "ARG1" + type = "VARCHAR" } - row_access_expression = "case when current_role() in ('ANALYST') then true else false end" + body = "case when current_role() in ('ANALYST') then true else false end" + comment = "comment" } diff --git a/pkg/acceptance/bettertestspoc/assert/objectassert/gen/sdk_object_def.go b/pkg/acceptance/bettertestspoc/assert/objectassert/gen/sdk_object_def.go index 798b2dbdcc..93ac5b4338 100644 --- a/pkg/acceptance/bettertestspoc/assert/objectassert/gen/sdk_object_def.go +++ b/pkg/acceptance/bettertestspoc/assert/objectassert/gen/sdk_object_def.go @@ -22,6 +22,11 @@ var allStructs = []SdkObjectDef{ ObjectType: sdk.ObjectTypeDatabaseRole, ObjectStruct: sdk.DatabaseRole{}, }, + { + IdType: "sdk.SchemaObjectIdentifier", + ObjectType: sdk.ObjectTypeRowAccessPolicy, + ObjectStruct: sdk.RowAccessPolicy{}, + }, { IdType: "sdk.AccountObjectIdentifier", ObjectType: sdk.ObjectTypeUser, diff --git a/pkg/acceptance/helpers/row_access_policy_client.go b/pkg/acceptance/helpers/row_access_policy_client.go index 2ae9be2099..e5452be71f 100644 --- a/pkg/acceptance/helpers/row_access_policy_client.go +++ b/pkg/acceptance/helpers/row_access_policy_client.go @@ -47,6 +47,14 @@ func (c *RowAccessPolicyClient) CreateRowAccessPolicyWithDataType(t *testing.T, return rowAccessPolicy, c.DropRowAccessPolicyFunc(t, id) } +func (c *RowAccessPolicyClient) Alter(t *testing.T, req sdk.AlterRowAccessPolicyRequest) { + t.Helper() + ctx := context.Background() + + err := c.client().Alter(ctx, &req) + require.NoError(t, err) +} + func (c *RowAccessPolicyClient) DropRowAccessPolicyFunc(t *testing.T, id sdk.SchemaObjectIdentifier) func() { t.Helper() ctx := context.Background() diff --git a/pkg/resources/row_access_policy.go b/pkg/resources/row_access_policy.go index 544ec1cad2..0878036da2 100644 --- a/pkg/resources/row_access_policy.go +++ b/pkg/resources/row_access_policy.go @@ -11,81 +11,148 @@ import ( "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" + "github.com/hashicorp/go-cty/cty" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) var rowAccessPolicySchema = map[string]*schema.Schema{ "name": { - Type: schema.TypeString, - Required: true, - Description: "Specifies the identifier for the row access policy; must be unique for the database and schema in which the row access policy is created.", - ForceNew: true, + Type: schema.TypeString, + Required: true, + Description: blocklistedCharactersFieldDescription("Specifies the identifier for the row access policy; must be unique for the database and schema in which the row access policy is created."), + DiffSuppressFunc: suppressIdentifierQuoting, }, "database": { - Type: schema.TypeString, - Required: true, - Description: "The database in which to create the row access policy.", - ForceNew: true, + Type: schema.TypeString, + Required: true, + Description: blocklistedCharactersFieldDescription("The database in which to create the row access policy."), + ForceNew: true, + DiffSuppressFunc: suppressIdentifierQuoting, }, "schema": { - Type: schema.TypeString, - Required: true, - Description: "The schema in which to create the row access policy.", - ForceNew: true, + Type: schema.TypeString, + Required: true, + Description: blocklistedCharactersFieldDescription("The schema in which to create the row access policy."), + ForceNew: true, + DiffSuppressFunc: suppressIdentifierQuoting, }, - // TODO [SNOW-1020074]: Implement DiffSuppressFunc and test after https://github.com/hashicorp/terraform-plugin-sdk/issues/477 is solved. - "signature": { - Type: schema.TypeMap, - Elem: &schema.Schema{Type: schema.TypeString}, + "argument": { + Type: schema.TypeList, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "name": { + Type: schema.TypeString, + Required: true, + Description: "The argument name", + ForceNew: true, + }, + // TODO(SNOW-1596962): Fully support VECTOR data type sdk.ParseFunctionArgumentsFromString could be a base for another function that takes argument names into consideration. + "type": { + Type: schema.TypeString, + Required: true, + DiffSuppressFunc: func(key, oldValue, newValue string, _ *schema.ResourceData) bool { + res := NormalizeAndCompare(sdk.ToDataType)(key, oldValue, newValue, nil) + return res + }, + ValidateDiagFunc: sdkValidation(sdk.ToDataType), + Description: "The argument type", + ForceNew: true, + }, + }, + }, Required: true, + Description: "List of the arguments for the row access policy. A signature specifies a set of attributes that must be considered to determine whether the row is accessible. The attribute values come from the database object (e.g. table or view) to be protected by the row access policy. If any argument name or type is changed, the resource is recreated.", ForceNew: true, - Description: "Specifies signature (arguments) for the row access policy (uppercase and sorted to avoid recreation of resource). A signature specifies a set of attributes that must be considered to determine whether the row is accessible. The attribute values come from the database object (e.g. table or view) to be protected by the row access policy.", + DiffSuppressFunc: func(key, oldValue, newValue string, _ *schema.ResourceData) bool { + if !strings.HasSuffix(key, ".type") { + return false + } + return NormalizeAndCompare(sdk.ToDataType)(key, oldValue, newValue, nil) + }, }, - "row_access_expression": { - Type: schema.TypeString, - Required: true, - Description: "Specifies the SQL expression. The expression can be any boolean-valued SQL expression.", + "body": { + Type: schema.TypeString, + Required: true, + Description: "Specifies the SQL expression. The expression can be any boolean-valued SQL expression.", + DiffSuppressFunc: DiffSuppressStatement, }, "comment": { Type: schema.TypeString, Optional: true, Description: "Specifies a comment for the row access policy.", }, + ShowOutputAttributeName: { + Type: schema.TypeList, + Computed: true, + Description: "Outputs the result of `SHOW ROW ACCESS POLICY` for the given row access policy.", + Elem: &schema.Resource{ + Schema: schemas.ShowRowAccessPolicySchema, + }, + }, + DescribeOutputAttributeName: { + Type: schema.TypeList, + Computed: true, + Description: "Outputs the result of `DESCRIBE ROW ACCESS POLICY` for the given row access policy.", + Elem: &schema.Resource{ + Schema: schemas.RowAccessPolicyDescribeSchema, + }, + }, FullyQualifiedNameAttributeName: schemas.FullyQualifiedNameSchema, } // RowAccessPolicy returns a pointer to the resource representing a row access policy. func RowAccessPolicy() *schema.Resource { return &schema.Resource{ - Create: CreateRowAccessPolicy, - Read: ReadRowAccessPolicy, - Update: UpdateRowAccessPolicy, - Delete: DeleteRowAccessPolicy, + SchemaVersion: 1, + + CreateContext: CreateRowAccessPolicy, + ReadContext: ReadRowAccessPolicy, + UpdateContext: UpdateRowAccessPolicy, + DeleteContext: DeleteRowAccessPolicy, Schema: rowAccessPolicySchema, Importer: &schema.ResourceImporter{ StateContext: schema.ImportStatePassthroughContext, }, + + CustomizeDiff: customdiff.All( + ComputedIfAnyAttributeChanged(rowAccessPolicySchema, ShowOutputAttributeName, "comment"), + ComputedIfAnyAttributeChanged(rowAccessPolicySchema, DescribeOutputAttributeName, "body"), + ComputedIfAnyAttributeChanged(rowAccessPolicySchema, FullyQualifiedNameAttributeName, "name"), + ), + + StateUpgraders: []schema.StateUpgrader{ + { + Version: 0, + // setting type to cty.EmptyObject is a bit hacky here but following https://developer.hashicorp.com/terraform/plugin/framework/migrating/resources/state-upgrade#sdkv2-1 would require lots of repetitive code; this should work with cty.EmptyObject + Type: cty.EmptyObject, + Upgrade: v0_95_0_RowAccessPolicyStateUpgrader, + }, + }, } } -// CreateRowAccessPolicy implements schema.CreateFunc. -func CreateRowAccessPolicy(d *schema.ResourceData, meta interface{}) error { +func CreateRowAccessPolicy(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { client := meta.(*provider.Context).Client - ctx := context.Background() databaseName := d.Get("database").(string) schemaName := d.Get("schema").(string) name := d.Get("name").(string) id := sdk.NewSchemaObjectIdentifier(databaseName, schemaName, name) - signature := d.Get("signature").(map[string]any) - rowAccessExpression := d.Get("row_access_expression").(string) + arguments := d.Get("argument").([]any) + rowAccessExpression := d.Get("body").(string) args := make([]sdk.CreateRowAccessPolicyArgsRequest, 0) - for k, v := range signature { - dataType := sdk.DataType(v.(string)) - args = append(args, *sdk.NewCreateRowAccessPolicyArgsRequest(k, dataType)) + for _, arg := range arguments { + v := arg.(map[string]any) + dataType, err := sdk.ToDataType(v["type"].(string)) + if err != nil { + return diag.FromErr(err) + } + args = append(args, *sdk.NewCreateRowAccessPolicyArgsRequest(v["name"].(string), dataType)) } createRequest := sdk.NewCreateRowAccessPolicyRequest(id, args, rowAccessExpression) @@ -97,19 +164,20 @@ func CreateRowAccessPolicy(d *schema.ResourceData, meta interface{}) error { err := client.RowAccessPolicies.Create(ctx, createRequest) if err != nil { - return fmt.Errorf("error creating row access policy %v err = %w", name, err) + return diag.FromErr(fmt.Errorf("error creating row access policy %v err = %w", name, err)) } - d.SetId(helpers.EncodeSnowflakeID(id)) + d.SetId(helpers.EncodeResourceIdentifier(id)) - return ReadRowAccessPolicy(d, meta) + return ReadRowAccessPolicy(ctx, d, meta) } -// ReadRowAccessPolicy implements schema.ReadFunc. -func ReadRowAccessPolicy(d *schema.ResourceData, meta interface{}) error { +func ReadRowAccessPolicy(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { client := meta.(*provider.Context).Client - ctx := context.Background() - id := helpers.DecodeSnowflakeID(d.Id()).(sdk.SchemaObjectIdentifier) + id, err := sdk.ParseSchemaObjectIdentifier(d.Id()) + if err != nil { + return diag.FromErr(err) + } rowAccessPolicy, err := client.RowAccessPolicies.ShowByID(ctx, id) if err != nil { @@ -118,82 +186,109 @@ func ReadRowAccessPolicy(d *schema.ResourceData, meta interface{}) error { return nil } if err := d.Set(FullyQualifiedNameAttributeName, id.FullyQualifiedName()); err != nil { - return err + return diag.FromErr(err) } if err := d.Set("name", rowAccessPolicy.Name); err != nil { - return err + return diag.FromErr(err) } if err := d.Set("database", rowAccessPolicy.DatabaseName); err != nil { - return err + return diag.FromErr(err) } if err := d.Set("schema", rowAccessPolicy.SchemaName); err != nil { - return err + return diag.FromErr(err) } if err := d.Set("comment", rowAccessPolicy.Comment); err != nil { - return err + return diag.FromErr(err) } rowAccessPolicyDescription, err := client.RowAccessPolicies.Describe(ctx, id) if err != nil { - return err + return diag.FromErr(err) } - if err := d.Set("row_access_expression", rowAccessPolicyDescription.Body); err != nil { - return err + if err := d.Set("body", rowAccessPolicyDescription.Body); err != nil { + return diag.FromErr(err) } - if err := d.Set("signature", parseSignature(rowAccessPolicyDescription.Signature)); err != nil { - return err + if err := d.Set("argument", parseSignature(rowAccessPolicyDescription.Signature)); err != nil { + return diag.FromErr(err) } - - return err + if err = d.Set(ShowOutputAttributeName, []map[string]any{schemas.RowAccessPolicyToSchema(rowAccessPolicy)}); err != nil { + return diag.FromErr(err) + } + if err = d.Set(DescribeOutputAttributeName, []map[string]any{schemas.RowAccessPolicyDescriptionToSchema(*rowAccessPolicyDescription)}); err != nil { + return diag.FromErr(err) + } + return diag.FromErr(err) } // UpdateRowAccessPolicy implements schema.UpdateFunc. -func UpdateRowAccessPolicy(d *schema.ResourceData, meta interface{}) error { +func UpdateRowAccessPolicy(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { client := meta.(*provider.Context).Client - ctx := context.Background() - id := helpers.DecodeSnowflakeID(d.Id()).(sdk.SchemaObjectIdentifier) + id, err := sdk.ParseSchemaObjectIdentifier(d.Id()) + if err != nil { + return diag.FromErr(err) + } + + if d.HasChange("name") { + newId := sdk.NewSchemaObjectIdentifierInSchema(id.SchemaId(), d.Get("name").(string)) + + err := client.RowAccessPolicies.Alter(ctx, sdk.NewAlterRowAccessPolicyRequest(id).WithRenameTo(&newId)) + if err != nil { + return diag.FromErr(fmt.Errorf("error renaming view %v err = %w", d.Id(), err)) + } + + d.SetId(helpers.EncodeResourceIdentifier(newId)) + id = newId + } if d.HasChange("comment") { comment := d.Get("comment") if c := comment.(string); c == "" { err := client.RowAccessPolicies.Alter(ctx, sdk.NewAlterRowAccessPolicyRequest(id).WithUnsetComment(sdk.Bool(true))) if err != nil { - return fmt.Errorf("error unsetting comment for row access policy on %v err = %w", d.Id(), err) + return diag.FromErr(fmt.Errorf("error unsetting comment for row access policy on %v err = %w", d.Id(), err)) } } else { err := client.RowAccessPolicies.Alter(ctx, sdk.NewAlterRowAccessPolicyRequest(id).WithSetComment(sdk.String(c))) if err != nil { - return fmt.Errorf("error updating comment for row access policy on %v err = %w", d.Id(), err) + return diag.FromErr(fmt.Errorf("error updating comment for row access policy on %v err = %w", d.Id(), err)) } } } - if d.HasChange("row_access_expression") { - rowAccessExpression := d.Get("row_access_expression").(string) + if d.HasChange("body") { + rowAccessExpression := d.Get("body").(string) err := client.RowAccessPolicies.Alter(ctx, sdk.NewAlterRowAccessPolicyRequest(id).WithSetBody(sdk.String(rowAccessExpression))) if err != nil { - return fmt.Errorf("error updating row access policy expression on %v err = %w", d.Id(), err) + return diag.FromErr(fmt.Errorf("error updating row access policy expression on %v err = %w", d.Id(), err)) } } - return ReadRowAccessPolicy(d, meta) + return ReadRowAccessPolicy(ctx, d, meta) } -// DeleteRowAccessPolicy implements schema.DeleteFunc. -func DeleteRowAccessPolicy(d *schema.ResourceData, meta interface{}) error { +func DeleteRowAccessPolicy(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { + id, err := sdk.ParseSchemaObjectIdentifier(d.Id()) + if err != nil { + return diag.FromErr(err) + } + client := meta.(*provider.Context).Client - ctx := context.Background() - id := helpers.DecodeSnowflakeID(d.Id()).(sdk.SchemaObjectIdentifier) - err := client.RowAccessPolicies.Drop(ctx, sdk.NewDropRowAccessPolicyRequest(id)) + err = client.RowAccessPolicies.Drop(ctx, sdk.NewDropRowAccessPolicyRequest(id).WithIfExists(sdk.Pointer(true))) if err != nil { - return err + return diag.Diagnostics{ + diag.Diagnostic{ + Severity: diag.Error, + Summary: "Error deleting row access policy", + Detail: fmt.Sprintf("id %v err = %v", id.Name(), err), + }, + } } d.SetId("") @@ -202,17 +297,21 @@ func DeleteRowAccessPolicy(d *schema.ResourceData, meta interface{}) error { } // TODO [SNOW-1020074]: should we put signature parsing to the SDK? -func parseSignature(signature string) map[string]interface{} { +func parseSignature(signature string) []map[string]any { // Format in database is `(column )` plainSignature := strings.ReplaceAll(signature, "(", "") plainSignature = strings.ReplaceAll(plainSignature, ")", "") signatureParts := strings.Split(plainSignature, ", ") - signatureMap := map[string]interface{}{} + // signatureMap := map[string]interface{}{} + arguments := make([]map[string]any, len(signatureParts)) - for _, e := range signatureParts { + for i, e := range signatureParts { parts := strings.Split(e, " ") - signatureMap[parts[0]] = parts[1] + // signatureMap[parts[0]] = parts[1] + arguments[i] = map[string]any{ + "name": parts[0], + "type": parts[1], + } } - - return signatureMap + return arguments } diff --git a/pkg/resources/row_access_policy_acceptance_test.go b/pkg/resources/row_access_policy_acceptance_test.go index c9544c203a..abf9033c05 100644 --- a/pkg/resources/row_access_policy_acceptance_test.go +++ b/pkg/resources/row_access_policy_acceptance_test.go @@ -1,18 +1,24 @@ package resources_test import ( + "fmt" + "regexp" "testing" acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources" "github.com/hashicorp/terraform-plugin-testing/config" "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/plancheck" "github.com/hashicorp/terraform-plugin-testing/tfversion" ) func TestAcc_RowAccessPolicy(t *testing.T) { id := acc.TestClient().Ids.RandomSchemaObjectIdentifier() + resourceName := "snowflake_row_access_policy.test" + m := func() map[string]config.Variable { return map[string]config.Variable{ "name": config.StringVariable(id.Name()), @@ -33,14 +39,17 @@ func TestAcc_RowAccessPolicy(t *testing.T) { ConfigDirectory: config.TestStepDirectory(), ConfigVariables: m(), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("snowflake_row_access_policy.test", "name", id.Name()), - resource.TestCheckResourceAttr("snowflake_row_access_policy.test", "fully_qualified_name", id.FullyQualifiedName()), - resource.TestCheckResourceAttr("snowflake_row_access_policy.test", "database", acc.TestDatabaseName), - resource.TestCheckResourceAttr("snowflake_row_access_policy.test", "schema", acc.TestSchemaName), - resource.TestCheckResourceAttr("snowflake_row_access_policy.test", "comment", "Terraform acceptance test"), - resource.TestCheckResourceAttr("snowflake_row_access_policy.test", "row_access_expression", "case when current_role() in ('ANALYST') then true else false end"), - resource.TestCheckResourceAttr("snowflake_row_access_policy.test", "signature.N", "VARCHAR"), - resource.TestCheckResourceAttr("snowflake_row_access_policy.test", "signature.V", "VARCHAR"), + resource.TestCheckResourceAttr(resourceName, "name", id.Name()), + resource.TestCheckResourceAttr(resourceName, "fully_qualified_name", id.FullyQualifiedName()), + resource.TestCheckResourceAttr(resourceName, "database", acc.TestDatabaseName), + resource.TestCheckResourceAttr(resourceName, "schema", acc.TestSchemaName), + resource.TestCheckResourceAttr(resourceName, "comment", "Terraform acceptance test"), + resource.TestCheckResourceAttr(resourceName, "body", "case when current_role() in ('ANALYST') then true else false end"), + resource.TestCheckResourceAttr(resourceName, "argument.#", "2"), + resource.TestCheckResourceAttr(resourceName, "argument.0.name", "N"), + resource.TestCheckResourceAttr(resourceName, "argument.0.type", "VARCHAR"), + resource.TestCheckResourceAttr(resourceName, "argument.1.name", "V"), + resource.TestCheckResourceAttr(resourceName, "argument.1.type", "VARCHAR"), ), }, // change comment and expression @@ -48,14 +57,16 @@ func TestAcc_RowAccessPolicy(t *testing.T) { ConfigDirectory: config.TestStepDirectory(), ConfigVariables: m(), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("snowflake_row_access_policy.test", "name", id.Name()), - resource.TestCheckResourceAttr("snowflake_row_access_policy.test", "fully_qualified_name", id.FullyQualifiedName()), - resource.TestCheckResourceAttr("snowflake_row_access_policy.test", "database", acc.TestDatabaseName), - resource.TestCheckResourceAttr("snowflake_row_access_policy.test", "schema", acc.TestSchemaName), - resource.TestCheckResourceAttr("snowflake_row_access_policy.test", "comment", "Terraform acceptance test - changed comment"), - resource.TestCheckResourceAttr("snowflake_row_access_policy.test", "row_access_expression", "case when current_role() in ('ANALYST') then false else true end"), - resource.TestCheckResourceAttr("snowflake_row_access_policy.test", "signature.N", "VARCHAR"), - resource.TestCheckResourceAttr("snowflake_row_access_policy.test", "signature.V", "VARCHAR"), + resource.TestCheckResourceAttr(resourceName, "name", id.Name()), + resource.TestCheckResourceAttr(resourceName, "fully_qualified_name", id.FullyQualifiedName()), + resource.TestCheckResourceAttr(resourceName, "database", acc.TestDatabaseName), + resource.TestCheckResourceAttr(resourceName, "schema", acc.TestSchemaName), + resource.TestCheckResourceAttr(resourceName, "comment", "Terraform acceptance test - changed comment"), + resource.TestCheckResourceAttr(resourceName, "body", "case when current_role() in ('ANALYST') then false else true end"), + resource.TestCheckResourceAttr(resourceName, "argument.0.name", "N"), + resource.TestCheckResourceAttr(resourceName, "argument.0.type", "VARCHAR"), + resource.TestCheckResourceAttr(resourceName, "argument.1.name", "V"), + resource.TestCheckResourceAttr(resourceName, "argument.1.type", "VARCHAR"), ), }, // change signature @@ -63,23 +74,333 @@ func TestAcc_RowAccessPolicy(t *testing.T) { ConfigDirectory: config.TestStepDirectory(), ConfigVariables: m(), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("snowflake_row_access_policy.test", "name", id.Name()), - resource.TestCheckResourceAttr("snowflake_row_access_policy.test", "fully_qualified_name", id.FullyQualifiedName()), - resource.TestCheckResourceAttr("snowflake_row_access_policy.test", "database", acc.TestDatabaseName), - resource.TestCheckResourceAttr("snowflake_row_access_policy.test", "schema", acc.TestSchemaName), - resource.TestCheckResourceAttr("snowflake_row_access_policy.test", "comment", "Terraform acceptance test - changed comment"), - resource.TestCheckResourceAttr("snowflake_row_access_policy.test", "row_access_expression", "case when current_role() in ('ANALYST') then false else true end"), - resource.TestCheckResourceAttr("snowflake_row_access_policy.test", "signature.V", "BOOLEAN"), - resource.TestCheckResourceAttr("snowflake_row_access_policy.test", "signature.X", "TIMESTAMP_NTZ"), + resource.TestCheckResourceAttr(resourceName, "name", id.Name()), + resource.TestCheckResourceAttr(resourceName, "fully_qualified_name", id.FullyQualifiedName()), + resource.TestCheckResourceAttr(resourceName, "database", acc.TestDatabaseName), + resource.TestCheckResourceAttr(resourceName, "schema", acc.TestSchemaName), + resource.TestCheckResourceAttr(resourceName, "comment", "Terraform acceptance test - changed comment"), + resource.TestCheckResourceAttr(resourceName, "body", "case when current_role() in ('ANALYST') then false else true end"), + resource.TestCheckResourceAttr(resourceName, "argument.0.name", "V"), + resource.TestCheckResourceAttr(resourceName, "argument.0.type", "BOOLEAN"), + resource.TestCheckResourceAttr(resourceName, "argument.1.name", "X"), + resource.TestCheckResourceAttr(resourceName, "argument.1.type", "TIMESTAMP_NTZ"), + ), + }, + // external change on body + { + ConfigDirectory: config.TestStepDirectory(), + ConfigVariables: m(), + PreConfig: func() { + acc.TestClient().RowAccessPolicy.Alter(t, *sdk.NewAlterRowAccessPolicyRequest(id).WithSetBody(sdk.Pointer("case when current_role() in ('EXTERNAL') then false else true end"))) + }, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(resourceName, "name", id.Name()), + resource.TestCheckResourceAttr(resourceName, "fully_qualified_name", id.FullyQualifiedName()), + resource.TestCheckResourceAttr(resourceName, "database", acc.TestDatabaseName), + resource.TestCheckResourceAttr(resourceName, "schema", acc.TestSchemaName), + resource.TestCheckResourceAttr(resourceName, "comment", "Terraform acceptance test - changed comment"), + resource.TestCheckResourceAttr(resourceName, "body", "case when current_role() in ('ANALYST') then false else true end"), + resource.TestCheckResourceAttr(resourceName, "argument.0.name", "V"), + resource.TestCheckResourceAttr(resourceName, "argument.0.type", "BOOLEAN"), + resource.TestCheckResourceAttr(resourceName, "argument.1.name", "X"), + resource.TestCheckResourceAttr(resourceName, "argument.1.type", "TIMESTAMP_NTZ"), ), }, // IMPORT { ConfigVariables: m(), - ResourceName: "snowflake_row_access_policy.test", + ResourceName: resourceName, ImportState: true, ImportStateVerify: true, }, }, }) } + +// proves https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2053 is fixed +func TestAcc_RowAccessPolicy_Issue2053(t *testing.T) { + id := acc.TestClient().Ids.RandomSchemaObjectIdentifier() + resourceName := "snowflake_row_access_policy.test" + m := func() map[string]config.Variable { + return map[string]config.Variable{ + "name": config.StringVariable(id.Name()), + "database": config.StringVariable(acc.TestDatabaseName), + "schema": config.StringVariable(acc.TestSchemaName), + "arguments": config.SetVariable( + config.MapVariable(map[string]config.Variable{ + "name": config.StringVariable("A"), + "type": config.StringVariable("VARCHAR"), + }), + ), + } + } + resource.Test(t, resource.TestCase{ + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + PreCheck: func() { acc.TestAccPreCheck(t) }, + Steps: []resource.TestStep{ + { + ExternalProviders: map[string]resource.ExternalProvider{ + "snowflake": { + VersionConstraint: "=0.95.0", + Source: "Snowflake-Labs/snowflake", + }, + }, + Config: rowAccessPolicy_v0_95_0_WithHeredoc(id, ` case + when current_role() in ('ANALYST') then true + else false + end +`), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PostApplyPostRefresh: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate), + }, + }, + ExpectNonEmptyPlan: true, + }, + { + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_RowAccessPolicy/basic"), + ConfigVariables: m(), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionNoop), + }, + }, + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr(resourceName, "name", id.Name()), + resource.TestCheckResourceAttr(resourceName, "body", `case + when current_role() in ('ANALYST') then true + else false +end`), + ), + }, + }, + }) +} + +func TestAcc_RowAccessPolicy_Rename(t *testing.T) { + id := acc.TestClient().Ids.RandomSchemaObjectIdentifier() + newId := acc.TestClient().Ids.RandomSchemaObjectIdentifier() + resourceName := "snowflake_row_access_policy.test" + m := func(identifier sdk.SchemaObjectIdentifier) config.Variables { + return config.Variables{ + "name": config.StringVariable(identifier.Name()), + "database": config.StringVariable(identifier.DatabaseName()), + "schema": config.StringVariable(identifier.SchemaName()), + "arguments": config.SetVariable( + config.MapVariable(map[string]config.Variable{ + "name": config.StringVariable("A"), + "type": config.StringVariable("VARCHAR"), + }), + ), + } + } + + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + PreCheck: func() { acc.TestAccPreCheck(t) }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + CheckDestroy: acc.CheckDestroy(t, resources.RowAccessPolicy), + Steps: []resource.TestStep{ + { + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_RowAccessPolicy/basic"), + ConfigVariables: m(id), + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr(resourceName, "name", id.Name()), + resource.TestCheckResourceAttr(resourceName, "fully_qualified_name", id.FullyQualifiedName()), + ), + }, + // rename with one param changed + { + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_RowAccessPolicy/basic"), + ConfigVariables: m(newId), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate), + }, + }, + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr(resourceName, "name", newId.Name()), + resource.TestCheckResourceAttr(resourceName, "fully_qualified_name", newId.FullyQualifiedName()), + ), + }, + }, + }) +} + +func rowAccessPolicy_v0_95_0(id sdk.SchemaObjectIdentifier, expr string) string { + return fmt.Sprintf(` +resource "snowflake_row_access_policy" "test" { + name = "%s" + database = "%s" + schema = "%s" + signature = { + A = "VARCHAR", + } + row_access_expression = "%s" +}`, id.Name(), id.DatabaseName(), id.SchemaName(), expr) +} + +func rowAccessPolicy_v0_95_0_WithHeredoc(id sdk.SchemaObjectIdentifier, expr string) string { + return fmt.Sprintf(` +resource "snowflake_row_access_policy" "test" { + name = "%s" + database = "%s" + schema = "%s" + signature = { + A = "VARCHAR", + } + row_access_expression = <<-EOT +%s +EOT +}`, id.Name(), id.DatabaseName(), id.SchemaName(), expr) +} + +func rowAccessPolicy_v0_96_0(id sdk.SchemaObjectIdentifier) string { + return fmt.Sprintf(` +resource "snowflake_row_access_policy" "test" { + name = "%s" + database = "%s" + schema = "%s" + argument { + name = "A" + type = "VARCHAR" + } + row_access_expression = <<-EOT + case + when current_role() in ('ANALYST') then true + else false + end +EOT +}`, id.Name(), id.DatabaseName(), id.SchemaName()) +} + +func TestAcc_RowAccessPolicy_InvalidDataType(t *testing.T) { + id := acc.TestClient().Ids.RandomAccountObjectIdentifier() + + m := func() map[string]config.Variable { + return map[string]config.Variable{ + "name": config.StringVariable(id.Name()), + "database": config.StringVariable(acc.TestDatabaseName), + "schema": config.StringVariable(acc.TestSchemaName), + "arguments": config.SetVariable( + config.MapVariable(map[string]config.Variable{ + "name": config.StringVariable("A"), + "type": config.StringVariable("invalid-int"), + }), + ), + } + } + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + PreCheck: func() { acc.TestAccPreCheck(t) }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + Steps: []resource.TestStep{ + { + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_RowAccessPolicy/basic"), + ConfigVariables: m(), + ExpectError: regexp.MustCompile(`invalid data type: invalid-int`), + }, + }, + }) +} + +func TestAcc_RowAccessPolicy_DataTypeAliases(t *testing.T) { + id := acc.TestClient().Ids.RandomAccountObjectIdentifier() + resourceName := "snowflake_row_access_policy.test" + m := func() map[string]config.Variable { + return map[string]config.Variable{ + "name": config.StringVariable(id.Name()), + "database": config.StringVariable(acc.TestDatabaseName), + "schema": config.StringVariable(acc.TestSchemaName), + "arguments": config.SetVariable( + config.MapVariable(map[string]config.Variable{ + "name": config.StringVariable("A"), + "type": config.StringVariable("TEXT"), + }), + ), + } + } + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + PreCheck: func() { acc.TestAccPreCheck(t) }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + Steps: []resource.TestStep{ + { + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_RowAccessPolicy/basic"), + ConfigVariables: m(), + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr(resourceName, "name", id.Name()), + resource.TestCheckResourceAttr(resourceName, "argument.#", "1"), + resource.TestCheckResourceAttr(resourceName, "argument.0.type", "VARCHAR")), + }, + }, + }) +} + +func TestAcc_view_migrateFromVersion_0_95_0(t *testing.T) { + id := acc.TestClient().Ids.RandomSchemaObjectIdentifier() + resourceName := "snowflake_row_access_policy.test" + m := config.Variables{ + "name": config.StringVariable(id.Name()), + "database": config.StringVariable(id.DatabaseName()), + "schema": config.StringVariable(id.SchemaName()), + "arguments": config.SetVariable( + config.MapVariable(map[string]config.Variable{ + "name": config.StringVariable("A"), + "type": config.StringVariable("VARCHAR"), + }), + ), + } + + resource.Test(t, resource.TestCase{ + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + PreCheck: func() { acc.TestAccPreCheck(t) }, + Steps: []resource.TestStep{ + { + ExternalProviders: map[string]resource.ExternalProvider{ + "snowflake": { + VersionConstraint: "=0.95.0", + Source: "Snowflake-Labs/snowflake", + }, + }, + Config: rowAccessPolicy_v0_95_0(id, "case when current_role() in ('ANALYST') then true else false end"), + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr(resourceName, "name", id.Name()), + resource.TestCheckResourceAttr(resourceName, "fully_qualified_name", id.FullyQualifiedName()), + resource.TestCheckResourceAttr(resourceName, "database", acc.TestDatabaseName), + resource.TestCheckResourceAttr(resourceName, "schema", acc.TestSchemaName), + resource.TestCheckResourceAttr(resourceName, "row_access_expression", "case when current_role() in ('ANALYST') then true else false end"), + resource.TestCheckResourceAttr(resourceName, "signature.A", "VARCHAR"), + ), + }, + { + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_RowAccessPolicy/basic"), + ConfigVariables: m, + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr(resourceName, "name", id.Name()), + resource.TestCheckResourceAttr(resourceName, "fully_qualified_name", id.FullyQualifiedName()), + resource.TestCheckResourceAttr(resourceName, "database", acc.TestDatabaseName), + resource.TestCheckResourceAttr(resourceName, "schema", acc.TestSchemaName), + resource.TestCheckResourceAttr(resourceName, "body", "case when current_role() in ('ANALYST') then true else false end"), + resource.TestCheckNoResourceAttr(resourceName, "row_access_expression"), + resource.TestCheckResourceAttr(resourceName, "argument.#", "1"), + resource.TestCheckResourceAttr(resourceName, "argument.0.name", "A"), + resource.TestCheckResourceAttr(resourceName, "argument.0.type", "VARCHAR"), + resource.TestCheckNoResourceAttr(resourceName, "signature.A"), + ), + }, + }, + }) +} diff --git a/pkg/resources/row_access_policy_state_upgraders.go b/pkg/resources/row_access_policy_state_upgraders.go new file mode 100644 index 0000000000..c1de13d6a9 --- /dev/null +++ b/pkg/resources/row_access_policy_state_upgraders.go @@ -0,0 +1,27 @@ +package resources + +import ( + "context" +) + +func v0_95_0_RowAccessPolicyStateUpgrader(ctx context.Context, rawState map[string]any, meta any) (map[string]any, error) { + if rawState == nil { + return rawState, nil + } + + rawState["body"] = rawState["row_access_expression"] + delete(rawState, "row_access_expression") + + signature := rawState["signature"].(map[string]any) + args := make([]map[string]any, 0) + for k, v := range signature { + args = append(args, map[string]any{ + "name": k, + "type": v, + }) + } + rawState["argument"] = args + delete(rawState, "signature") + + return migratePipeSeparatedObjectIdentifierResourceIdToFullyQualifiedName(ctx, rawState, meta) +} diff --git a/pkg/resources/testdata/TestAcc_RowAccessPolicy/1/test.tf b/pkg/resources/testdata/TestAcc_RowAccessPolicy/1/test.tf index 96fd17c5ac..fee1c9aeee 100644 --- a/pkg/resources/testdata/TestAcc_RowAccessPolicy/1/test.tf +++ b/pkg/resources/testdata/TestAcc_RowAccessPolicy/1/test.tf @@ -2,10 +2,14 @@ resource "snowflake_row_access_policy" "test" { name = var.name database = var.database schema = var.schema - signature = { - N = "VARCHAR" - V = "VARCHAR", + argument { + name = "N" + type = "VARCHAR" } - row_access_expression = "case when current_role() in ('ANALYST') then true else false end" - comment = "Terraform acceptance test" + argument { + name = "V" + type = "VARCHAR" + } + body = "case when current_role() in ('ANALYST') then true else false end" + comment = "Terraform acceptance test" } diff --git a/pkg/resources/testdata/TestAcc_RowAccessPolicy/2/test.tf b/pkg/resources/testdata/TestAcc_RowAccessPolicy/2/test.tf index e1976bdbd4..e0541f4a2c 100644 --- a/pkg/resources/testdata/TestAcc_RowAccessPolicy/2/test.tf +++ b/pkg/resources/testdata/TestAcc_RowAccessPolicy/2/test.tf @@ -2,10 +2,14 @@ resource "snowflake_row_access_policy" "test" { name = var.name database = var.database schema = var.schema - signature = { - N = "VARCHAR" - V = "VARCHAR", + argument { + name = "N" + type = "VARCHAR" } - row_access_expression = "case when current_role() in ('ANALYST') then false else true end" - comment = "Terraform acceptance test - changed comment" + argument { + name = "V" + type = "VARCHAR" + } + body = "case when current_role() in ('ANALYST') then false else true end" + comment = "Terraform acceptance test - changed comment" } diff --git a/pkg/resources/testdata/TestAcc_RowAccessPolicy/3/test.tf b/pkg/resources/testdata/TestAcc_RowAccessPolicy/3/test.tf index 7e0fb12032..82be361ac5 100644 --- a/pkg/resources/testdata/TestAcc_RowAccessPolicy/3/test.tf +++ b/pkg/resources/testdata/TestAcc_RowAccessPolicy/3/test.tf @@ -2,10 +2,14 @@ resource "snowflake_row_access_policy" "test" { name = var.name database = var.database schema = var.schema - signature = { - V = "BOOLEAN", - X = "TIMESTAMP_NTZ" + argument { + name = "V" + type = "BOOLEAN" } - row_access_expression = "case when current_role() in ('ANALYST') then false else true end" - comment = "Terraform acceptance test - changed comment" + argument { + name = "X" + type = "TIMESTAMP_NTZ" + } + body = "case when current_role() in ('ANALYST') then false else true end" + comment = "Terraform acceptance test - changed comment" } diff --git a/pkg/resources/testdata/TestAcc_RowAccessPolicy/4/test.tf b/pkg/resources/testdata/TestAcc_RowAccessPolicy/4/test.tf new file mode 100644 index 0000000000..82be361ac5 --- /dev/null +++ b/pkg/resources/testdata/TestAcc_RowAccessPolicy/4/test.tf @@ -0,0 +1,15 @@ +resource "snowflake_row_access_policy" "test" { + name = var.name + database = var.database + schema = var.schema + argument { + name = "V" + type = "BOOLEAN" + } + argument { + name = "X" + type = "TIMESTAMP_NTZ" + } + body = "case when current_role() in ('ANALYST') then false else true end" + comment = "Terraform acceptance test - changed comment" +} diff --git a/pkg/resources/testdata/TestAcc_RowAccessPolicy/4/variables.tf b/pkg/resources/testdata/TestAcc_RowAccessPolicy/4/variables.tf new file mode 100644 index 0000000000..31dd643cf2 --- /dev/null +++ b/pkg/resources/testdata/TestAcc_RowAccessPolicy/4/variables.tf @@ -0,0 +1,11 @@ +variable "name" { + type = string +} + +variable "database" { + type = string +} + +variable "schema" { + type = string +} diff --git a/pkg/resources/testdata/TestAcc_RowAccessPolicy/basic/test.tf b/pkg/resources/testdata/TestAcc_RowAccessPolicy/basic/test.tf new file mode 100644 index 0000000000..bb157daed8 --- /dev/null +++ b/pkg/resources/testdata/TestAcc_RowAccessPolicy/basic/test.tf @@ -0,0 +1,13 @@ +resource "snowflake_row_access_policy" "test" { + name = var.name + database = var.database + schema = var.schema + dynamic "argument" { + for_each = var.arguments + content { + name = argument.value["name"] + type = argument.value["type"] + } + } + body = "case when current_role() in ('ANALYST') then true else false end" +} diff --git a/pkg/resources/testdata/TestAcc_RowAccessPolicy/basic/variables.tf b/pkg/resources/testdata/TestAcc_RowAccessPolicy/basic/variables.tf new file mode 100644 index 0000000000..eb5ac2abc9 --- /dev/null +++ b/pkg/resources/testdata/TestAcc_RowAccessPolicy/basic/variables.tf @@ -0,0 +1,15 @@ +variable "name" { + type = string +} + +variable "database" { + type = string +} + +variable "schema" { + type = string +} + +variable "arguments" { + type = set(map(string)) +} diff --git a/pkg/schemas/row_access_policy.go b/pkg/schemas/row_access_policy.go new file mode 100644 index 0000000000..cc98576aec --- /dev/null +++ b/pkg/schemas/row_access_policy.go @@ -0,0 +1,34 @@ +package schemas + +import ( + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +var RowAccessPolicyDescribeSchema = map[string]*schema.Schema{ + "name": { + Type: schema.TypeString, + Computed: true, + }, + "signature": { + Type: schema.TypeString, + Computed: true, + }, + "return_type": { + Type: schema.TypeString, + Computed: true, + }, + "body": { + Type: schema.TypeString, + Computed: true, + }, +} + +func RowAccessPolicyDescriptionToSchema(description sdk.RowAccessPolicyDescription) map[string]any { + return map[string]any{ + "name": description.Name, + "signature": description.Signature, + "return_type": description.ReturnType, + "body": description.Body, + } +} diff --git a/templates/resources/row_access_policy.md.tmpl b/templates/resources/row_access_policy.md.tmpl new file mode 100644 index 0000000000..e09e58f03f --- /dev/null +++ b/templates/resources/row_access_policy.md.tmpl @@ -0,0 +1,35 @@ +--- +page_title: "{{.Name}} {{.Type}} - {{.ProviderName}}" +subcategory: "" +description: |- +{{ if gt (len (split .Description "")) 1 -}} +{{ index (split .Description "") 1 | plainmarkdown | trimspace | prefixlines " " }} +{{- else -}} +{{ .Description | plainmarkdown | trimspace | prefixlines " " }} +{{- end }} +--- + +!> **V1 release candidate** This resource was reworked and is a release candidate for the V1. We do not expect significant changes in it before the V1. We will welcome any feedback and adjust the resource if needed. Any errors reported will be resolved with a higher priority. We encourage checking this resource out before the V1 release. Please follow the [migration guide](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/MIGRATION_GUIDE.md#v0950--v0960) to use it. + +# {{.Name}} ({{.Type}}) + +{{ .Description | trimspace }} + +{{ if .HasExample -}} +## Example Usage + +{{ tffile (printf "examples/resources/%s/resource.tf" .Name)}} +-> **Note** Instead of using fully_qualified_name, you can reference objects managed outside Terraform by constructing a correct ID, consult [identifiers guide](https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs/guides/identifiers#new-computed-fully-qualified-name-field-in-resources). + + +{{- end }} + +{{ .SchemaMarkdown | trimspace }} +{{- if .HasImport }} + +## Import + +Import is supported using the following syntax: + +{{ codefile "shell" (printf "examples/resources/%s/import.sh" .Name)}} +{{- end }} From 0026f04d097abd0177e0cc2cf531778adfa53f34 Mon Sep 17 00:00:00 2001 From: Jakub Michalak Date: Wed, 11 Sep 2024 10:35:25 +0200 Subject: [PATCH 2/9] Cleanup --- MIGRATION_GUIDE.md | 21 +- .../row_access_policy_snowflake_gen.go | 130 ++++++++++++ .../row_access_policy_resource_gen.go | 107 ++++++++++ .../resourceassert/view_resource_gen.go | 10 + .../row_access_policy_show_output_ext.go | 10 + .../row_access_policy_show_output_gen.go | 86 ++++++++ .../config/model/view_model_gen.go | 8 + .../helpers/row_access_policy_client.go | 7 + pkg/resources/doc_helpers.go | 4 + pkg/resources/row_access_policy.go | 36 +--- .../row_access_policy_acceptance_test.go | 192 ++++++++++-------- .../TestAcc_RowAccessPolicy/basic/test.tf | 3 +- .../basic/variables.tf | 4 + pkg/sdk/row_access_policies_gen.go | 21 +- pkg/sdk/row_access_policies_gen_test.go | 64 +++++- 15 files changed, 579 insertions(+), 124 deletions(-) create mode 100644 pkg/acceptance/bettertestspoc/assert/objectassert/row_access_policy_snowflake_gen.go create mode 100644 pkg/acceptance/bettertestspoc/assert/resourceassert/row_access_policy_resource_gen.go create mode 100644 pkg/acceptance/bettertestspoc/assert/resourceshowoutputassert/row_access_policy_show_output_ext.go create mode 100644 pkg/acceptance/bettertestspoc/assert/resourceshowoutputassert/row_access_policy_show_output_gen.go diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index 72c608ae91..6db30f023c 100644 --- a/MIGRATION_GUIDE.md +++ b/MIGRATION_GUIDE.md @@ -4,17 +4,30 @@ This document is meant to help you migrate your Terraform config to the new newe describe deprecations or breaking changes and help you to change your configuration to keep the same (or similar) behavior across different versions. -## v0.95.x ➞ v0.96.0 +## v0.95.0 ➞ v0.96.0 ### snowflake_row_access_policy resource changes New fields: - `show_output` field that holds the response from SHOW ROW ACCESS POLICIES. - - `describe_output` field that holds the response from DESCRIBE ROW ACCESS POLICIES. + - `describe_output` field that holds the response from DESCRIBE ROW ACCESS POLICY. #### *(breaking change)* Renamed fields in snowflake_row_access_policy resource Renamed fields: - `row_access_expression` to `body` - - `signature` to `arguments`. Also, the field type is changed from map to a list of arguments. -to align with Snowflake docs. Please rename this field in your configuration files. State will be migrated automatically. + - `signature` to `arguments` +Please rename these fields in your configuration files. State will be migrated automatically. + +#### *(breaking change)* Adjusted behavior of arguments/signature +Now, arguments are stored as a list, instead of a map. Please adjust that in your configs. State is migrated automatically. Also, this means that order of the items matters and may be adjusted. + +Argument names are now case sensitive. + +#### *(breaking change)* Adjusted behavior on changing name +Previously, after changing `name` field, the resource was recreated. Now, the object is renamed with `RENAME TO`. + +#### *(breaking change)* Adjusted behavior on changing name +Previously, `body` of a policy was compared as a raw string. This led to permament diff because of leading newlines (see https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2053). + +Now, similarly to handling statements in other resources, we replace blank characters with a space. The provider can cause false positives in cases where a change in case or run of whitespace is semantically significant. #### *(breaking change)* Identifiers related changes During [identifiers rework](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/ROADMAP.md#identifiers-rework) we decided to diff --git a/pkg/acceptance/bettertestspoc/assert/objectassert/row_access_policy_snowflake_gen.go b/pkg/acceptance/bettertestspoc/assert/objectassert/row_access_policy_snowflake_gen.go new file mode 100644 index 0000000000..285ad82d0f --- /dev/null +++ b/pkg/acceptance/bettertestspoc/assert/objectassert/row_access_policy_snowflake_gen.go @@ -0,0 +1,130 @@ +// Code generated by assertions generator; DO NOT EDIT. + +package objectassert + +import ( + "fmt" + "testing" + + acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance" + + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" +) + +type RowAccessPolicyAssert struct { + *assert.SnowflakeObjectAssert[sdk.RowAccessPolicy, sdk.SchemaObjectIdentifier] +} + +func RowAccessPolicy(t *testing.T, id sdk.SchemaObjectIdentifier) *RowAccessPolicyAssert { + t.Helper() + return &RowAccessPolicyAssert{ + assert.NewSnowflakeObjectAssertWithProvider(sdk.ObjectTypeRowAccessPolicy, id, acc.TestClient().RowAccessPolicy.Show), + } +} + +func RowAccessPolicyFromObject(t *testing.T, rowAccessPolicy *sdk.RowAccessPolicy) *RowAccessPolicyAssert { + t.Helper() + return &RowAccessPolicyAssert{ + assert.NewSnowflakeObjectAssertWithObject(sdk.ObjectTypeRowAccessPolicy, rowAccessPolicy.ID(), rowAccessPolicy), + } +} + +func (r *RowAccessPolicyAssert) HasCreatedOn(expected string) *RowAccessPolicyAssert { + r.AddAssertion(func(t *testing.T, o *sdk.RowAccessPolicy) error { + t.Helper() + if o.CreatedOn != expected { + return fmt.Errorf("expected created on: %v; got: %v", expected, o.CreatedOn) + } + return nil + }) + return r +} + +func (r *RowAccessPolicyAssert) HasName(expected string) *RowAccessPolicyAssert { + r.AddAssertion(func(t *testing.T, o *sdk.RowAccessPolicy) error { + t.Helper() + if o.Name != expected { + return fmt.Errorf("expected name: %v; got: %v", expected, o.Name) + } + return nil + }) + return r +} + +func (r *RowAccessPolicyAssert) HasDatabaseName(expected string) *RowAccessPolicyAssert { + r.AddAssertion(func(t *testing.T, o *sdk.RowAccessPolicy) error { + t.Helper() + if o.DatabaseName != expected { + return fmt.Errorf("expected database name: %v; got: %v", expected, o.DatabaseName) + } + return nil + }) + return r +} + +func (r *RowAccessPolicyAssert) HasSchemaName(expected string) *RowAccessPolicyAssert { + r.AddAssertion(func(t *testing.T, o *sdk.RowAccessPolicy) error { + t.Helper() + if o.SchemaName != expected { + return fmt.Errorf("expected schema name: %v; got: %v", expected, o.SchemaName) + } + return nil + }) + return r +} + +func (r *RowAccessPolicyAssert) HasKind(expected string) *RowAccessPolicyAssert { + r.AddAssertion(func(t *testing.T, o *sdk.RowAccessPolicy) error { + t.Helper() + if o.Kind != expected { + return fmt.Errorf("expected kind: %v; got: %v", expected, o.Kind) + } + return nil + }) + return r +} + +func (r *RowAccessPolicyAssert) HasOwner(expected string) *RowAccessPolicyAssert { + r.AddAssertion(func(t *testing.T, o *sdk.RowAccessPolicy) error { + t.Helper() + if o.Owner != expected { + return fmt.Errorf("expected owner: %v; got: %v", expected, o.Owner) + } + return nil + }) + return r +} + +func (r *RowAccessPolicyAssert) HasComment(expected string) *RowAccessPolicyAssert { + r.AddAssertion(func(t *testing.T, o *sdk.RowAccessPolicy) error { + t.Helper() + if o.Comment != expected { + return fmt.Errorf("expected comment: %v; got: %v", expected, o.Comment) + } + return nil + }) + return r +} + +func (r *RowAccessPolicyAssert) HasOptions(expected string) *RowAccessPolicyAssert { + r.AddAssertion(func(t *testing.T, o *sdk.RowAccessPolicy) error { + t.Helper() + if o.Options != expected { + return fmt.Errorf("expected options: %v; got: %v", expected, o.Options) + } + return nil + }) + return r +} + +func (r *RowAccessPolicyAssert) HasOwnerRoleType(expected string) *RowAccessPolicyAssert { + r.AddAssertion(func(t *testing.T, o *sdk.RowAccessPolicy) error { + t.Helper() + if o.OwnerRoleType != expected { + return fmt.Errorf("expected owner role type: %v; got: %v", expected, o.OwnerRoleType) + } + return nil + }) + return r +} diff --git a/pkg/acceptance/bettertestspoc/assert/resourceassert/row_access_policy_resource_gen.go b/pkg/acceptance/bettertestspoc/assert/resourceassert/row_access_policy_resource_gen.go new file mode 100644 index 0000000000..a73404790b --- /dev/null +++ b/pkg/acceptance/bettertestspoc/assert/resourceassert/row_access_policy_resource_gen.go @@ -0,0 +1,107 @@ +// Code generated by assertions generator; DO NOT EDIT. + +package resourceassert + +import ( + "testing" + + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert" +) + +type RowAccessPolicyResourceAssert struct { + *assert.ResourceAssert +} + +func RowAccessPolicyResource(t *testing.T, name string) *RowAccessPolicyResourceAssert { + t.Helper() + + return &RowAccessPolicyResourceAssert{ + ResourceAssert: assert.NewResourceAssert(name, "resource"), + } +} + +func ImportedRowAccessPolicyResource(t *testing.T, id string) *RowAccessPolicyResourceAssert { + t.Helper() + + return &RowAccessPolicyResourceAssert{ + ResourceAssert: assert.NewImportedResourceAssert(id, "imported resource"), + } +} + +/////////////////////////////////// +// Attribute value string checks // +/////////////////////////////////// + +func (r *RowAccessPolicyResourceAssert) HasArgumentString(expected string) *RowAccessPolicyResourceAssert { + r.AddAssertion(assert.ValueSet("argument", expected)) + return r +} + +func (r *RowAccessPolicyResourceAssert) HasBodyString(expected string) *RowAccessPolicyResourceAssert { + r.AddAssertion(assert.ValueSet("body", expected)) + return r +} + +func (r *RowAccessPolicyResourceAssert) HasCommentString(expected string) *RowAccessPolicyResourceAssert { + r.AddAssertion(assert.ValueSet("comment", expected)) + return r +} + +func (r *RowAccessPolicyResourceAssert) HasDatabaseString(expected string) *RowAccessPolicyResourceAssert { + r.AddAssertion(assert.ValueSet("database", expected)) + return r +} + +func (r *RowAccessPolicyResourceAssert) HasFullyQualifiedNameString(expected string) *RowAccessPolicyResourceAssert { + r.AddAssertion(assert.ValueSet("fully_qualified_name", expected)) + return r +} + +func (r *RowAccessPolicyResourceAssert) HasNameString(expected string) *RowAccessPolicyResourceAssert { + r.AddAssertion(assert.ValueSet("name", expected)) + return r +} + +func (r *RowAccessPolicyResourceAssert) HasSchemaString(expected string) *RowAccessPolicyResourceAssert { + r.AddAssertion(assert.ValueSet("schema", expected)) + return r +} + +//////////////////////////// +// Attribute empty checks // +//////////////////////////// + +func (r *RowAccessPolicyResourceAssert) HasNoArgument() *RowAccessPolicyResourceAssert { + r.AddAssertion(assert.ValueNotSet("argument")) + return r +} + +func (r *RowAccessPolicyResourceAssert) HasNoBody() *RowAccessPolicyResourceAssert { + r.AddAssertion(assert.ValueNotSet("body")) + return r +} + +func (r *RowAccessPolicyResourceAssert) HasNoComment() *RowAccessPolicyResourceAssert { + r.AddAssertion(assert.ValueNotSet("comment")) + return r +} + +func (r *RowAccessPolicyResourceAssert) HasNoDatabase() *RowAccessPolicyResourceAssert { + r.AddAssertion(assert.ValueNotSet("database")) + return r +} + +func (r *RowAccessPolicyResourceAssert) HasNoFullyQualifiedName() *RowAccessPolicyResourceAssert { + r.AddAssertion(assert.ValueNotSet("fully_qualified_name")) + return r +} + +func (r *RowAccessPolicyResourceAssert) HasNoName() *RowAccessPolicyResourceAssert { + r.AddAssertion(assert.ValueNotSet("name")) + return r +} + +func (r *RowAccessPolicyResourceAssert) HasNoSchema() *RowAccessPolicyResourceAssert { + r.AddAssertion(assert.ValueNotSet("schema")) + return r +} diff --git a/pkg/acceptance/bettertestspoc/assert/resourceassert/view_resource_gen.go b/pkg/acceptance/bettertestspoc/assert/resourceassert/view_resource_gen.go index 9b2994e865..f95069aa91 100644 --- a/pkg/acceptance/bettertestspoc/assert/resourceassert/view_resource_gen.go +++ b/pkg/acceptance/bettertestspoc/assert/resourceassert/view_resource_gen.go @@ -42,6 +42,11 @@ func (v *ViewResourceAssert) HasChangeTrackingString(expected string) *ViewResou return v } +func (v *ViewResourceAssert) HasColumnString(expected string) *ViewResourceAssert { + v.AddAssertion(assert.ValueSet("column", expected)) + return v +} + func (v *ViewResourceAssert) HasCommentString(expected string) *ViewResourceAssert { v.AddAssertion(assert.ValueSet("comment", expected)) return v @@ -121,6 +126,11 @@ func (v *ViewResourceAssert) HasNoChangeTracking() *ViewResourceAssert { return v } +func (v *ViewResourceAssert) HasNoColumn() *ViewResourceAssert { + v.AddAssertion(assert.ValueNotSet("column")) + return v +} + func (v *ViewResourceAssert) HasNoComment() *ViewResourceAssert { v.AddAssertion(assert.ValueNotSet("comment")) return v diff --git a/pkg/acceptance/bettertestspoc/assert/resourceshowoutputassert/row_access_policy_show_output_ext.go b/pkg/acceptance/bettertestspoc/assert/resourceshowoutputassert/row_access_policy_show_output_ext.go new file mode 100644 index 0000000000..a63b1e3b23 --- /dev/null +++ b/pkg/acceptance/bettertestspoc/assert/resourceshowoutputassert/row_access_policy_show_output_ext.go @@ -0,0 +1,10 @@ +package resourceshowoutputassert + +import ( + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert" +) + +func (r *RowAccessPolicyShowOutputAssert) HasCreatedOnNotEmpty() *RowAccessPolicyShowOutputAssert { + r.AddAssertion(assert.ResourceShowOutputValuePresent("created_on")) + return r +} diff --git a/pkg/acceptance/bettertestspoc/assert/resourceshowoutputassert/row_access_policy_show_output_gen.go b/pkg/acceptance/bettertestspoc/assert/resourceshowoutputassert/row_access_policy_show_output_gen.go new file mode 100644 index 0000000000..b08670ce73 --- /dev/null +++ b/pkg/acceptance/bettertestspoc/assert/resourceshowoutputassert/row_access_policy_show_output_gen.go @@ -0,0 +1,86 @@ +// Code generated by assertions generator; DO NOT EDIT. + +package resourceshowoutputassert + +import ( + "testing" + + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" +) + +// to ensure sdk package is used +var _ = sdk.Object{} + +type RowAccessPolicyShowOutputAssert struct { + *assert.ResourceAssert +} + +func RowAccessPolicyShowOutput(t *testing.T, name string) *RowAccessPolicyShowOutputAssert { + t.Helper() + + r := RowAccessPolicyShowOutputAssert{ + ResourceAssert: assert.NewResourceAssert(name, "show_output"), + } + r.AddAssertion(assert.ValueSet("show_output.#", "1")) + return &r +} + +func ImportedRowAccessPolicyShowOutput(t *testing.T, id string) *RowAccessPolicyShowOutputAssert { + t.Helper() + + r := RowAccessPolicyShowOutputAssert{ + ResourceAssert: assert.NewImportedResourceAssert(id, "show_output"), + } + r.AddAssertion(assert.ValueSet("show_output.#", "1")) + return &r +} + +//////////////////////////// +// Attribute value checks // +//////////////////////////// + +func (r *RowAccessPolicyShowOutputAssert) HasCreatedOn(expected string) *RowAccessPolicyShowOutputAssert { + r.AddAssertion(assert.ResourceShowOutputValueSet("created_on", expected)) + return r +} + +func (r *RowAccessPolicyShowOutputAssert) HasName(expected string) *RowAccessPolicyShowOutputAssert { + r.AddAssertion(assert.ResourceShowOutputValueSet("name", expected)) + return r +} + +func (r *RowAccessPolicyShowOutputAssert) HasDatabaseName(expected string) *RowAccessPolicyShowOutputAssert { + r.AddAssertion(assert.ResourceShowOutputValueSet("database_name", expected)) + return r +} + +func (r *RowAccessPolicyShowOutputAssert) HasSchemaName(expected string) *RowAccessPolicyShowOutputAssert { + r.AddAssertion(assert.ResourceShowOutputValueSet("schema_name", expected)) + return r +} + +func (r *RowAccessPolicyShowOutputAssert) HasKind(expected string) *RowAccessPolicyShowOutputAssert { + r.AddAssertion(assert.ResourceShowOutputValueSet("kind", expected)) + return r +} + +func (r *RowAccessPolicyShowOutputAssert) HasOwner(expected string) *RowAccessPolicyShowOutputAssert { + r.AddAssertion(assert.ResourceShowOutputValueSet("owner", expected)) + return r +} + +func (r *RowAccessPolicyShowOutputAssert) HasComment(expected string) *RowAccessPolicyShowOutputAssert { + r.AddAssertion(assert.ResourceShowOutputValueSet("comment", expected)) + return r +} + +func (r *RowAccessPolicyShowOutputAssert) HasOptions(expected string) *RowAccessPolicyShowOutputAssert { + r.AddAssertion(assert.ResourceShowOutputValueSet("options", expected)) + return r +} + +func (r *RowAccessPolicyShowOutputAssert) HasOwnerRoleType(expected string) *RowAccessPolicyShowOutputAssert { + r.AddAssertion(assert.ResourceShowOutputValueSet("owner_role_type", expected)) + return r +} diff --git a/pkg/acceptance/bettertestspoc/config/model/view_model_gen.go b/pkg/acceptance/bettertestspoc/config/model/view_model_gen.go index 2dc0920dd1..1afe8859d8 100644 --- a/pkg/acceptance/bettertestspoc/config/model/view_model_gen.go +++ b/pkg/acceptance/bettertestspoc/config/model/view_model_gen.go @@ -12,6 +12,7 @@ import ( type ViewModel struct { AggregationPolicy tfconfig.Variable `json:"aggregation_policy,omitempty"` ChangeTracking tfconfig.Variable `json:"change_tracking,omitempty"` + Column tfconfig.Variable `json:"column,omitempty"` Comment tfconfig.Variable `json:"comment,omitempty"` CopyGrants tfconfig.Variable `json:"copy_grants,omitempty"` DataMetricFunction tfconfig.Variable `json:"data_metric_function,omitempty"` @@ -73,6 +74,8 @@ func (v *ViewModel) WithChangeTracking(changeTracking string) *ViewModel { return v } +// column attribute type is not yet supported, so WithColumn can't be generated + func (v *ViewModel) WithComment(comment string) *ViewModel { v.Comment = tfconfig.StringVariable(comment) return v @@ -143,6 +146,11 @@ func (v *ViewModel) WithChangeTrackingValue(value tfconfig.Variable) *ViewModel return v } +func (v *ViewModel) WithColumnValue(value tfconfig.Variable) *ViewModel { + v.Column = value + return v +} + func (v *ViewModel) WithCommentValue(value tfconfig.Variable) *ViewModel { v.Comment = value return v diff --git a/pkg/acceptance/helpers/row_access_policy_client.go b/pkg/acceptance/helpers/row_access_policy_client.go index e5452be71f..89b71a49b5 100644 --- a/pkg/acceptance/helpers/row_access_policy_client.go +++ b/pkg/acceptance/helpers/row_access_policy_client.go @@ -64,3 +64,10 @@ func (c *RowAccessPolicyClient) DropRowAccessPolicyFunc(t *testing.T, id sdk.Sch require.NoError(t, err) } } + +func (c *RowAccessPolicyClient) Show(t *testing.T, id sdk.SchemaObjectIdentifier) (*sdk.RowAccessPolicy, error) { + t.Helper() + ctx := context.Background() + + return c.client().ShowByID(ctx, id) +} diff --git a/pkg/resources/doc_helpers.go b/pkg/resources/doc_helpers.go index 9fe20cf817..d85e68f047 100644 --- a/pkg/resources/doc_helpers.go +++ b/pkg/resources/doc_helpers.go @@ -36,3 +36,7 @@ func withPrivilegedRolesDescription(description, paramName string) string { func blocklistedCharactersFieldDescription(description string) string { return fmt.Sprintf(`%s Due to technical limitations (read more [here](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/technical-documentation/identifiers_rework_design_decisions.md#known-limitations-and-identifier-recommendations)), avoid using the following characters: %s`, description, characterList([]rune{'|', '.', '(', ')', '"'})) } + +func diffSuppressStatementFieldDescription(description string) string { + return fmt.Sprintf(`%s To mitigate permadiff on this field, the provider replaces blank characters with a space. This can lead to false positives in cases where a change in case or run of whitespace is semantically significant.`, description) +} diff --git a/pkg/resources/row_access_policy.go b/pkg/resources/row_access_policy.go index 0878036da2..cd14408096 100644 --- a/pkg/resources/row_access_policy.go +++ b/pkg/resources/row_access_policy.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "log" - "strings" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/provider" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/schemas" @@ -53,8 +52,7 @@ var rowAccessPolicySchema = map[string]*schema.Schema{ Type: schema.TypeString, Required: true, DiffSuppressFunc: func(key, oldValue, newValue string, _ *schema.ResourceData) bool { - res := NormalizeAndCompare(sdk.ToDataType)(key, oldValue, newValue, nil) - return res + return NormalizeAndCompare(sdk.ToDataType)(key, oldValue, newValue, nil) }, ValidateDiagFunc: sdkValidation(sdk.ToDataType), Description: "The argument type", @@ -65,17 +63,11 @@ var rowAccessPolicySchema = map[string]*schema.Schema{ Required: true, Description: "List of the arguments for the row access policy. A signature specifies a set of attributes that must be considered to determine whether the row is accessible. The attribute values come from the database object (e.g. table or view) to be protected by the row access policy. If any argument name or type is changed, the resource is recreated.", ForceNew: true, - DiffSuppressFunc: func(key, oldValue, newValue string, _ *schema.ResourceData) bool { - if !strings.HasSuffix(key, ".type") { - return false - } - return NormalizeAndCompare(sdk.ToDataType)(key, oldValue, newValue, nil) - }, }, "body": { Type: schema.TypeString, Required: true, - Description: "Specifies the SQL expression. The expression can be any boolean-valued SQL expression.", + Description: diffSuppressStatementFieldDescription("Specifies the SQL expression. The expression can be any boolean-valued SQL expression."), DiffSuppressFunc: DiffSuppressStatement, }, "comment": { @@ -214,7 +206,7 @@ func ReadRowAccessPolicy(ctx context.Context, d *schema.ResourceData, meta any) return diag.FromErr(err) } - if err := d.Set("argument", parseSignature(rowAccessPolicyDescription.Signature)); err != nil { + if err := d.Set("argument", rowAccessPolicyDescription.Arguments()); err != nil { return diag.FromErr(err) } if err = d.Set(ShowOutputAttributeName, []map[string]any{schemas.RowAccessPolicyToSchema(rowAccessPolicy)}); err != nil { @@ -223,7 +215,7 @@ func ReadRowAccessPolicy(ctx context.Context, d *schema.ResourceData, meta any) if err = d.Set(DescribeOutputAttributeName, []map[string]any{schemas.RowAccessPolicyDescriptionToSchema(*rowAccessPolicyDescription)}); err != nil { return diag.FromErr(err) } - return diag.FromErr(err) + return nil } // UpdateRowAccessPolicy implements schema.UpdateFunc. @@ -295,23 +287,3 @@ func DeleteRowAccessPolicy(ctx context.Context, d *schema.ResourceData, meta any return nil } - -// TODO [SNOW-1020074]: should we put signature parsing to the SDK? -func parseSignature(signature string) []map[string]any { - // Format in database is `(column )` - plainSignature := strings.ReplaceAll(signature, "(", "") - plainSignature = strings.ReplaceAll(plainSignature, ")", "") - signatureParts := strings.Split(plainSignature, ", ") - // signatureMap := map[string]interface{}{} - arguments := make([]map[string]any, len(signatureParts)) - - for i, e := range signatureParts { - parts := strings.Split(e, " ") - // signatureMap[parts[0]] = parts[1] - arguments[i] = map[string]any{ - "name": parts[0], - "type": parts[1], - } - } - return arguments -} diff --git a/pkg/resources/row_access_policy_acceptance_test.go b/pkg/resources/row_access_policy_acceptance_test.go index abf9033c05..cf9692c32b 100644 --- a/pkg/resources/row_access_policy_acceptance_test.go +++ b/pkg/resources/row_access_policy_acceptance_test.go @@ -6,6 +6,10 @@ import ( "testing" acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert/resourceassert" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert/resourceshowoutputassert" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/snowflakeroles" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources" @@ -38,52 +42,68 @@ func TestAcc_RowAccessPolicy(t *testing.T) { { ConfigDirectory: config.TestStepDirectory(), ConfigVariables: m(), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr(resourceName, "name", id.Name()), - resource.TestCheckResourceAttr(resourceName, "fully_qualified_name", id.FullyQualifiedName()), - resource.TestCheckResourceAttr(resourceName, "database", acc.TestDatabaseName), - resource.TestCheckResourceAttr(resourceName, "schema", acc.TestSchemaName), - resource.TestCheckResourceAttr(resourceName, "comment", "Terraform acceptance test"), - resource.TestCheckResourceAttr(resourceName, "body", "case when current_role() in ('ANALYST') then true else false end"), - resource.TestCheckResourceAttr(resourceName, "argument.#", "2"), - resource.TestCheckResourceAttr(resourceName, "argument.0.name", "N"), - resource.TestCheckResourceAttr(resourceName, "argument.0.type", "VARCHAR"), - resource.TestCheckResourceAttr(resourceName, "argument.1.name", "V"), - resource.TestCheckResourceAttr(resourceName, "argument.1.type", "VARCHAR"), + Check: assert.AssertThat(t, resourceassert.RowAccessPolicyResource(t, resourceName). + HasNameString(id.Name()). + HasDatabaseString(id.DatabaseName()). + HasSchemaString(id.SchemaName()). + HasFullyQualifiedNameString(id.FullyQualifiedName()). + HasCommentString("Terraform acceptance test"). + HasBodyString("case when current_role() in ('ANALYST') then true else false end"), + assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.#", "2")), + assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.0.name", "N")), + assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.0.type", "VARCHAR")), + assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.1.name", "V")), + assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.1.type", "VARCHAR")), + resourceshowoutputassert.RowAccessPolicyShowOutput(t, resourceName). + HasCreatedOnNotEmpty(). + HasDatabaseName(id.DatabaseName()). + HasKind(string(sdk.PolicyKindRowAccessPolicy)). + HasName(id.Name()). + HasOptions(""). + HasOwner(snowflakeroles.Accountadmin.Name()). + HasOwnerRoleType("ROLE"). + HasSchemaName(id.SchemaName()). + HasComment("Terraform acceptance test"), + assert.Check(resource.TestCheckResourceAttr(resourceName, "describe_output.0.body", "case when current_role() in ('ANALYST') then true else false end")), + assert.Check(resource.TestCheckResourceAttr(resourceName, "describe_output.0.name", id.Name())), + assert.Check(resource.TestCheckResourceAttr(resourceName, "describe_output.0.return_type", "BOOLEAN")), + assert.Check(resource.TestCheckResourceAttr(resourceName, "describe_output.0.signature", "(N VARCHAR, V VARCHAR)")), ), }, // change comment and expression { ConfigDirectory: config.TestStepDirectory(), ConfigVariables: m(), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr(resourceName, "name", id.Name()), - resource.TestCheckResourceAttr(resourceName, "fully_qualified_name", id.FullyQualifiedName()), - resource.TestCheckResourceAttr(resourceName, "database", acc.TestDatabaseName), - resource.TestCheckResourceAttr(resourceName, "schema", acc.TestSchemaName), - resource.TestCheckResourceAttr(resourceName, "comment", "Terraform acceptance test - changed comment"), - resource.TestCheckResourceAttr(resourceName, "body", "case when current_role() in ('ANALYST') then false else true end"), - resource.TestCheckResourceAttr(resourceName, "argument.0.name", "N"), - resource.TestCheckResourceAttr(resourceName, "argument.0.type", "VARCHAR"), - resource.TestCheckResourceAttr(resourceName, "argument.1.name", "V"), - resource.TestCheckResourceAttr(resourceName, "argument.1.type", "VARCHAR"), + Check: assert.AssertThat(t, resourceassert.RowAccessPolicyResource(t, resourceName). + HasNameString(id.Name()). + HasDatabaseString(id.DatabaseName()). + HasSchemaString(id.SchemaName()). + HasFullyQualifiedNameString(id.FullyQualifiedName()). + HasCommentString("Terraform acceptance test - changed comment"). + HasBodyString("case when current_role() in ('ANALYST') then false else true end"), + assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.#", "2")), + assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.0.name", "N")), + assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.0.type", "VARCHAR")), + assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.1.name", "V")), + assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.1.type", "VARCHAR")), ), }, // change signature { ConfigDirectory: config.TestStepDirectory(), ConfigVariables: m(), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr(resourceName, "name", id.Name()), - resource.TestCheckResourceAttr(resourceName, "fully_qualified_name", id.FullyQualifiedName()), - resource.TestCheckResourceAttr(resourceName, "database", acc.TestDatabaseName), - resource.TestCheckResourceAttr(resourceName, "schema", acc.TestSchemaName), - resource.TestCheckResourceAttr(resourceName, "comment", "Terraform acceptance test - changed comment"), - resource.TestCheckResourceAttr(resourceName, "body", "case when current_role() in ('ANALYST') then false else true end"), - resource.TestCheckResourceAttr(resourceName, "argument.0.name", "V"), - resource.TestCheckResourceAttr(resourceName, "argument.0.type", "BOOLEAN"), - resource.TestCheckResourceAttr(resourceName, "argument.1.name", "X"), - resource.TestCheckResourceAttr(resourceName, "argument.1.type", "TIMESTAMP_NTZ"), + Check: assert.AssertThat(t, resourceassert.RowAccessPolicyResource(t, resourceName). + HasNameString(id.Name()). + HasDatabaseString(id.DatabaseName()). + HasSchemaString(id.SchemaName()). + HasFullyQualifiedNameString(id.FullyQualifiedName()). + HasCommentString("Terraform acceptance test - changed comment"). + HasBodyString("case when current_role() in ('ANALYST') then false else true end"), + assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.#", "2")), + assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.0.name", "V")), + assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.0.type", "BOOLEAN")), + assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.1.name", "X")), + assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.1.type", "TIMESTAMP_NTZ")), ), }, // external change on body @@ -93,17 +113,18 @@ func TestAcc_RowAccessPolicy(t *testing.T) { PreConfig: func() { acc.TestClient().RowAccessPolicy.Alter(t, *sdk.NewAlterRowAccessPolicyRequest(id).WithSetBody(sdk.Pointer("case when current_role() in ('EXTERNAL') then false else true end"))) }, - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr(resourceName, "name", id.Name()), - resource.TestCheckResourceAttr(resourceName, "fully_qualified_name", id.FullyQualifiedName()), - resource.TestCheckResourceAttr(resourceName, "database", acc.TestDatabaseName), - resource.TestCheckResourceAttr(resourceName, "schema", acc.TestSchemaName), - resource.TestCheckResourceAttr(resourceName, "comment", "Terraform acceptance test - changed comment"), - resource.TestCheckResourceAttr(resourceName, "body", "case when current_role() in ('ANALYST') then false else true end"), - resource.TestCheckResourceAttr(resourceName, "argument.0.name", "V"), - resource.TestCheckResourceAttr(resourceName, "argument.0.type", "BOOLEAN"), - resource.TestCheckResourceAttr(resourceName, "argument.1.name", "X"), - resource.TestCheckResourceAttr(resourceName, "argument.1.type", "TIMESTAMP_NTZ"), + Check: assert.AssertThat(t, resourceassert.RowAccessPolicyResource(t, resourceName). + HasNameString(id.Name()). + HasDatabaseString(id.DatabaseName()). + HasSchemaString(id.SchemaName()). + HasFullyQualifiedNameString(id.FullyQualifiedName()). + HasCommentString("Terraform acceptance test - changed comment"). + HasBodyString("case when current_role() in ('ANALYST') then false else true end"), + assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.#", "2")), + assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.0.name", "V")), + assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.0.type", "BOOLEAN")), + assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.1.name", "X")), + assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.1.type", "TIMESTAMP_NTZ")), ), }, // IMPORT @@ -132,6 +153,7 @@ func TestAcc_RowAccessPolicy_Issue2053(t *testing.T) { "type": config.StringVariable("VARCHAR"), }), ), + "body": config.StringVariable("case when current_role() in ('ANALYST') then true else false end"), } } resource.Test(t, resource.TestCase{ @@ -147,6 +169,7 @@ func TestAcc_RowAccessPolicy_Issue2053(t *testing.T) { Source: "Snowflake-Labs/snowflake", }, }, + // these configs have "weird" format on purpose - to test against handling new lines during diff correctly Config: rowAccessPolicy_v0_95_0_WithHeredoc(id, ` case when current_role() in ('ANALYST') then true else false @@ -168,9 +191,9 @@ func TestAcc_RowAccessPolicy_Issue2053(t *testing.T) { plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionNoop), }, }, - Check: resource.ComposeAggregateTestCheckFunc( - resource.TestCheckResourceAttr(resourceName, "name", id.Name()), - resource.TestCheckResourceAttr(resourceName, "body", `case + Check: assert.AssertThat(t, resourceassert.RowAccessPolicyResource(t, resourceName). + HasNameString(id.Name()). + HasBodyString(`case when current_role() in ('ANALYST') then true else false end`), @@ -191,10 +214,11 @@ func TestAcc_RowAccessPolicy_Rename(t *testing.T) { "schema": config.StringVariable(identifier.SchemaName()), "arguments": config.SetVariable( config.MapVariable(map[string]config.Variable{ - "name": config.StringVariable("A"), + "name": config.StringVariable("a"), "type": config.StringVariable("VARCHAR"), }), ), + "body": config.StringVariable("case when current_role() in ('ANALYST') then true else false end"), } } @@ -209,12 +233,12 @@ func TestAcc_RowAccessPolicy_Rename(t *testing.T) { { ConfigDirectory: acc.ConfigurationDirectory("TestAcc_RowAccessPolicy/basic"), ConfigVariables: m(id), - Check: resource.ComposeAggregateTestCheckFunc( - resource.TestCheckResourceAttr(resourceName, "name", id.Name()), - resource.TestCheckResourceAttr(resourceName, "fully_qualified_name", id.FullyQualifiedName()), + Check: assert.AssertThat(t, resourceassert.RowAccessPolicyResource(t, resourceName). + HasNameString(id.Name()). + HasFullyQualifiedNameString(id.FullyQualifiedName()), ), }, - // rename with one param changed + // rename { ConfigDirectory: acc.ConfigurationDirectory("TestAcc_RowAccessPolicy/basic"), ConfigVariables: m(newId), @@ -223,9 +247,9 @@ func TestAcc_RowAccessPolicy_Rename(t *testing.T) { plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate), }, }, - Check: resource.ComposeAggregateTestCheckFunc( - resource.TestCheckResourceAttr(resourceName, "name", newId.Name()), - resource.TestCheckResourceAttr(resourceName, "fully_qualified_name", newId.FullyQualifiedName()), + Check: assert.AssertThat(t, resourceassert.RowAccessPolicyResource(t, resourceName). + HasNameString(newId.Name()). + HasFullyQualifiedNameString(newId.FullyQualifiedName()), ), }, }, @@ -293,6 +317,7 @@ func TestAcc_RowAccessPolicy_InvalidDataType(t *testing.T) { "type": config.StringVariable("invalid-int"), }), ), + "body": config.StringVariable("case when current_role() in ('ANALYST') then true else false end"), } } resource.Test(t, resource.TestCase{ @@ -312,19 +337,20 @@ func TestAcc_RowAccessPolicy_InvalidDataType(t *testing.T) { } func TestAcc_RowAccessPolicy_DataTypeAliases(t *testing.T) { - id := acc.TestClient().Ids.RandomAccountObjectIdentifier() + id := acc.TestClient().Ids.RandomSchemaObjectIdentifier() resourceName := "snowflake_row_access_policy.test" m := func() map[string]config.Variable { return map[string]config.Variable{ "name": config.StringVariable(id.Name()), - "database": config.StringVariable(acc.TestDatabaseName), - "schema": config.StringVariable(acc.TestSchemaName), + "database": config.StringVariable(id.DatabaseName()), + "schema": config.StringVariable(id.SchemaName()), "arguments": config.SetVariable( config.MapVariable(map[string]config.Variable{ "name": config.StringVariable("A"), "type": config.StringVariable("TEXT"), }), ), + "body": config.StringVariable("case when current_role() in ('ANALYST') then true else false end"), } } resource.Test(t, resource.TestCase{ @@ -337,10 +363,11 @@ func TestAcc_RowAccessPolicy_DataTypeAliases(t *testing.T) { { ConfigDirectory: acc.ConfigurationDirectory("TestAcc_RowAccessPolicy/basic"), ConfigVariables: m(), - Check: resource.ComposeAggregateTestCheckFunc( - resource.TestCheckResourceAttr(resourceName, "name", id.Name()), - resource.TestCheckResourceAttr(resourceName, "argument.#", "1"), - resource.TestCheckResourceAttr(resourceName, "argument.0.type", "VARCHAR")), + Check: assert.AssertThat(t, resourceassert.RowAccessPolicyResource(t, resourceName). + HasNameString(id.Name()), + assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.#", "1")), + assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.0.type", "VARCHAR")), + ), }, }, }) @@ -349,6 +376,7 @@ func TestAcc_RowAccessPolicy_DataTypeAliases(t *testing.T) { func TestAcc_view_migrateFromVersion_0_95_0(t *testing.T) { id := acc.TestClient().Ids.RandomSchemaObjectIdentifier() resourceName := "snowflake_row_access_policy.test" + body := "case when current_role() in ('ANALYST') then true else false end" m := config.Variables{ "name": config.StringVariable(id.Name()), "database": config.StringVariable(id.DatabaseName()), @@ -359,6 +387,7 @@ func TestAcc_view_migrateFromVersion_0_95_0(t *testing.T) { "type": config.StringVariable("VARCHAR"), }), ), + "body": config.StringVariable("case when current_role() in ('ANALYST') then true else false end"), } resource.Test(t, resource.TestCase{ @@ -374,31 +403,30 @@ func TestAcc_view_migrateFromVersion_0_95_0(t *testing.T) { Source: "Snowflake-Labs/snowflake", }, }, - Config: rowAccessPolicy_v0_95_0(id, "case when current_role() in ('ANALYST') then true else false end"), - Check: resource.ComposeAggregateTestCheckFunc( - resource.TestCheckResourceAttr(resourceName, "name", id.Name()), - resource.TestCheckResourceAttr(resourceName, "fully_qualified_name", id.FullyQualifiedName()), - resource.TestCheckResourceAttr(resourceName, "database", acc.TestDatabaseName), - resource.TestCheckResourceAttr(resourceName, "schema", acc.TestSchemaName), - resource.TestCheckResourceAttr(resourceName, "row_access_expression", "case when current_role() in ('ANALYST') then true else false end"), - resource.TestCheckResourceAttr(resourceName, "signature.A", "VARCHAR"), + Config: rowAccessPolicy_v0_95_0(id, body), + Check: assert.AssertThat(t, resourceassert.RowAccessPolicyResource(t, resourceName). + HasNameString(id.Name()). + HasDatabaseString(id.DatabaseName()). + HasSchemaString(id.SchemaName()). + HasFullyQualifiedNameString(id.FullyQualifiedName()), + assert.Check(resource.TestCheckResourceAttr(resourceName, "row_access_expression", body)), ), }, { ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, ConfigDirectory: acc.ConfigurationDirectory("TestAcc_RowAccessPolicy/basic"), ConfigVariables: m, - Check: resource.ComposeAggregateTestCheckFunc( - resource.TestCheckResourceAttr(resourceName, "name", id.Name()), - resource.TestCheckResourceAttr(resourceName, "fully_qualified_name", id.FullyQualifiedName()), - resource.TestCheckResourceAttr(resourceName, "database", acc.TestDatabaseName), - resource.TestCheckResourceAttr(resourceName, "schema", acc.TestSchemaName), - resource.TestCheckResourceAttr(resourceName, "body", "case when current_role() in ('ANALYST') then true else false end"), - resource.TestCheckNoResourceAttr(resourceName, "row_access_expression"), - resource.TestCheckResourceAttr(resourceName, "argument.#", "1"), - resource.TestCheckResourceAttr(resourceName, "argument.0.name", "A"), - resource.TestCheckResourceAttr(resourceName, "argument.0.type", "VARCHAR"), - resource.TestCheckNoResourceAttr(resourceName, "signature.A"), + Check: assert.AssertThat(t, resourceassert.RowAccessPolicyResource(t, resourceName). + HasNameString(id.Name()). + HasDatabaseString(id.DatabaseName()). + HasSchemaString(id.SchemaName()). + HasFullyQualifiedNameString(id.FullyQualifiedName()). + HasBodyString(body), + assert.Check(resource.TestCheckNoResourceAttr(resourceName, "row_access_expression")), + assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.#", "1")), + assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.0.name", "A")), + assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.0.type", "VARCHAR")), + assert.Check(resource.TestCheckNoResourceAttr(resourceName, "signature.A")), ), }, }, diff --git a/pkg/resources/testdata/TestAcc_RowAccessPolicy/basic/test.tf b/pkg/resources/testdata/TestAcc_RowAccessPolicy/basic/test.tf index bb157daed8..c66b9ae65c 100644 --- a/pkg/resources/testdata/TestAcc_RowAccessPolicy/basic/test.tf +++ b/pkg/resources/testdata/TestAcc_RowAccessPolicy/basic/test.tf @@ -9,5 +9,6 @@ resource "snowflake_row_access_policy" "test" { type = argument.value["type"] } } - body = "case when current_role() in ('ANALYST') then true else false end" + # body = "case when current_role() in ('ANALYST') then true else false end" + body = var.body } diff --git a/pkg/resources/testdata/TestAcc_RowAccessPolicy/basic/variables.tf b/pkg/resources/testdata/TestAcc_RowAccessPolicy/basic/variables.tf index eb5ac2abc9..fb0dfe1f59 100644 --- a/pkg/resources/testdata/TestAcc_RowAccessPolicy/basic/variables.tf +++ b/pkg/resources/testdata/TestAcc_RowAccessPolicy/basic/variables.tf @@ -13,3 +13,7 @@ variable "schema" { variable "arguments" { type = set(map(string)) } + +variable "body" { + type = string +} diff --git a/pkg/sdk/row_access_policies_gen.go b/pkg/sdk/row_access_policies_gen.go index 7b99767eb3..ff434b24ad 100644 --- a/pkg/sdk/row_access_policies_gen.go +++ b/pkg/sdk/row_access_policies_gen.go @@ -3,6 +3,7 @@ package sdk import ( "context" "database/sql" + "strings" ) type RowAccessPolicies interface { @@ -29,7 +30,7 @@ type CreateRowAccessPolicyOptions struct { } type CreateRowAccessPolicyArgs struct { - Name string `ddl:"keyword,no_quotes"` + Name string `ddl:"keyword,double_quotes"` Type DataType `ddl:"keyword,no_quotes"` } @@ -110,3 +111,21 @@ type RowAccessPolicyDescription struct { ReturnType string Body string } + +// TODO(SNOW-1596962): Fully support VECTOR data type +func (d *RowAccessPolicyDescription) Arguments() []map[string]any { + // Format in database is `(column )` + plainSignature := strings.ReplaceAll(d.Signature, "(", "") + plainSignature = strings.ReplaceAll(plainSignature, ")", "") + signatureParts := strings.Split(plainSignature, ", ") + arguments := make([]map[string]any, len(signatureParts)) + + for i, e := range signatureParts { + parts := strings.Split(e, " ") + arguments[i] = map[string]any{ + "name": strings.Join(parts[:len(parts)-1], " "), + "type": parts[len(parts)-1], + } + } + return arguments +} diff --git a/pkg/sdk/row_access_policies_gen_test.go b/pkg/sdk/row_access_policies_gen_test.go index 71e8e68395..e1006fdd0c 100644 --- a/pkg/sdk/row_access_policies_gen_test.go +++ b/pkg/sdk/row_access_policies_gen_test.go @@ -1,6 +1,10 @@ package sdk -import "testing" +import ( + "testing" + + "github.com/stretchr/testify/require" +) func TestRowAccessPolicies_Create(t *testing.T) { id := randomSchemaObjectIdentifier() @@ -49,7 +53,7 @@ func TestRowAccessPolicies_Create(t *testing.T) { t.Run("one parameter", func(t *testing.T) { opts := defaultOpts() - assertOptsValidAndSQLEquals(t, opts, "CREATE ROW ACCESS POLICY %s AS (n VARCHAR) RETURNS BOOLEAN -> true", id.FullyQualifiedName()) + assertOptsValidAndSQLEquals(t, opts, `CREATE ROW ACCESS POLICY %s AS ("n" VARCHAR) RETURNS BOOLEAN -> true`, id.FullyQualifiedName()) }) t.Run("two parameters", func(t *testing.T) { @@ -61,14 +65,14 @@ func TestRowAccessPolicies_Create(t *testing.T) { Name: "h", Type: DataTypeVARCHAR, }} - assertOptsValidAndSQLEquals(t, opts, "CREATE ROW ACCESS POLICY %s AS (n VARCHAR, h VARCHAR) RETURNS BOOLEAN -> true", id.FullyQualifiedName()) + assertOptsValidAndSQLEquals(t, opts, `CREATE ROW ACCESS POLICY %s AS ("n" VARCHAR, "h" VARCHAR) RETURNS BOOLEAN -> true`, id.FullyQualifiedName()) }) t.Run("all options", func(t *testing.T) { opts := defaultOpts() opts.OrReplace = Bool(true) opts.Comment = String("some comment") - assertOptsValidAndSQLEquals(t, opts, "CREATE OR REPLACE ROW ACCESS POLICY %s AS (n VARCHAR) RETURNS BOOLEAN -> true COMMENT = 'some comment'", id.FullyQualifiedName()) + assertOptsValidAndSQLEquals(t, opts, `CREATE OR REPLACE ROW ACCESS POLICY %s AS ("n" VARCHAR) RETURNS BOOLEAN -> true COMMENT = 'some comment'`, id.FullyQualifiedName()) }) } @@ -243,3 +247,55 @@ func TestRowAccessPolicies_Describe(t *testing.T) { assertOptsValidAndSQLEquals(t, opts, "DESCRIBE ROW ACCESS POLICY %s", id.FullyQualifiedName()) }) } + +func TestRowAccessPolicyDescription_Arguments(t *testing.T) { + tests := []struct { + name string + signature string + want []map[string]any + }{ + { + name: "signature with 1 arg", + signature: "(A VARCHAR)", + want: []map[string]any{ + { + "name": "A", + "type": "VARCHAR", + }, + }, + }, + { + name: "signature with multiple args", + signature: "(A VARCHAR, B BOOLEAN)", + want: []map[string]any{ + { + "name": "A", + "type": "VARCHAR", + }, + { + "name": "B", + "type": "BOOLEAN", + }, + }, + }, + { + name: "signature with complex name", + signature: "(a B VARCHAR)", + want: []map[string]any{ + { + "name": "a B", + "type": "VARCHAR", + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + d := &RowAccessPolicyDescription{ + Signature: tt.signature, + } + got := d.Arguments() + require.Equal(t, tt.want, got) + }) + } +} From 145a0d069e3e8cc7677c26a46a7ee5cac526838a Mon Sep 17 00:00:00 2001 From: Jakub Michalak Date: Wed, 11 Sep 2024 10:41:49 +0200 Subject: [PATCH 3/9] Cleanup --- MIGRATION_GUIDE.md | 2 +- docs/resources/row_access_policy.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index fd83c26129..1600061373 100644 --- a/MIGRATION_GUIDE.md +++ b/MIGRATION_GUIDE.md @@ -24,7 +24,7 @@ Argument names are now case sensitive. #### *(breaking change)* Adjusted behavior on changing name Previously, after changing `name` field, the resource was recreated. Now, the object is renamed with `RENAME TO`. -#### *(breaking change)* Adjusted behavior on changing name +#### *(breaking change)* Mitigating permadiff on `body` Previously, `body` of a policy was compared as a raw string. This led to permament diff because of leading newlines (see https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2053). Now, similarly to handling statements in other resources, we replace blank characters with a space. The provider can cause false positives in cases where a change in case or run of whitespace is semantically significant. diff --git a/docs/resources/row_access_policy.md b/docs/resources/row_access_policy.md index 49ced88b61..ce612535fc 100644 --- a/docs/resources/row_access_policy.md +++ b/docs/resources/row_access_policy.md @@ -35,7 +35,7 @@ resource "snowflake_row_access_policy" "example_row_access_policy" { ### Required - `argument` (Block List, Min: 1) List of the arguments for the row access policy. A signature specifies a set of attributes that must be considered to determine whether the row is accessible. The attribute values come from the database object (e.g. table or view) to be protected by the row access policy. If any argument name or type is changed, the resource is recreated. (see [below for nested schema](#nestedblock--argument)) -- `body` (String) Specifies the SQL expression. The expression can be any boolean-valued SQL expression. +- `body` (String) Specifies the SQL expression. The expression can be any boolean-valued SQL expression. To mitigate permadiff on this field, the provider replaces blank characters with a space. This can lead to false positives in cases where a change in case or run of whitespace is semantically significant. - `database` (String) The database in which to create the row access policy. Due to technical limitations (read more [here](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/technical-documentation/identifiers_rework_design_decisions.md#known-limitations-and-identifier-recommendations)), avoid using the following characters: `|`, `.`, `(`, `)`, `"` - `name` (String) Specifies the identifier for the row access policy; must be unique for the database and schema in which the row access policy is created. Due to technical limitations (read more [here](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/technical-documentation/identifiers_rework_design_decisions.md#known-limitations-and-identifier-recommendations)), avoid using the following characters: `|`, `.`, `(`, `)`, `"` - `schema` (String) The schema in which to create the row access policy. Due to technical limitations (read more [here](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/technical-documentation/identifiers_rework_design_decisions.md#known-limitations-and-identifier-recommendations)), avoid using the following characters: `|`, `.`, `(`, `)`, `"` From f67fe87e6d07933bb96a3a9729aba425541707e1 Mon Sep 17 00:00:00 2001 From: Jakub Michalak Date: Wed, 11 Sep 2024 15:19:14 +0200 Subject: [PATCH 4/9] Review suggestions --- MIGRATION_GUIDE.md | 4 +- .../snowflake_row_access_policy/import.sh | 2 +- .../snowflake_row_access_policy/resource.tf | 8 + .../resourceassert/gen/resource_schema_def.go | 4 + .../row_access_policy_resource_ext.go | 18 + .../model/row_access_policy_model_ext.go | 19 + .../model/row_access_policy_model_gen.go | 135 +++++++ .../helpers/row_access_policy_client.go | 13 + pkg/resources/row_access_policy.go | 62 +++- .../row_access_policy_acceptance_test.go | 334 +++++++++++------- .../row_access_policy_state_upgraders.go | 3 +- .../TestAcc_RowAccessPolicy/1/test.tf | 15 - .../TestAcc_RowAccessPolicy/1/variables.tf | 11 - .../TestAcc_RowAccessPolicy/2/test.tf | 15 - .../TestAcc_RowAccessPolicy/2/variables.tf | 11 - .../TestAcc_RowAccessPolicy/3/test.tf | 15 - .../TestAcc_RowAccessPolicy/3/variables.tf | 11 - .../TestAcc_RowAccessPolicy/4/test.tf | 15 - .../TestAcc_RowAccessPolicy/4/variables.tf | 11 - .../TestAcc_RowAccessPolicy/basic/test.tf | 3 +- .../basic/variables.tf | 2 +- .../TestAcc_RowAccessPolicy/complete/test.tf | 14 + .../complete/variables.tf | 23 ++ pkg/schemas/row_access_policy.go | 11 + pkg/sdk/row_access_policies_gen.go | 20 +- pkg/sdk/row_access_policies_gen_test.go | 26 +- ...ow_access_policies_gen_integration_test.go | 57 +++ 27 files changed, 612 insertions(+), 250 deletions(-) create mode 100644 pkg/acceptance/bettertestspoc/assert/resourceassert/row_access_policy_resource_ext.go create mode 100644 pkg/acceptance/bettertestspoc/config/model/row_access_policy_model_ext.go create mode 100644 pkg/acceptance/bettertestspoc/config/model/row_access_policy_model_gen.go delete mode 100644 pkg/resources/testdata/TestAcc_RowAccessPolicy/1/test.tf delete mode 100644 pkg/resources/testdata/TestAcc_RowAccessPolicy/1/variables.tf delete mode 100644 pkg/resources/testdata/TestAcc_RowAccessPolicy/2/test.tf delete mode 100644 pkg/resources/testdata/TestAcc_RowAccessPolicy/2/variables.tf delete mode 100644 pkg/resources/testdata/TestAcc_RowAccessPolicy/3/test.tf delete mode 100644 pkg/resources/testdata/TestAcc_RowAccessPolicy/3/variables.tf delete mode 100644 pkg/resources/testdata/TestAcc_RowAccessPolicy/4/test.tf delete mode 100644 pkg/resources/testdata/TestAcc_RowAccessPolicy/4/variables.tf create mode 100644 pkg/resources/testdata/TestAcc_RowAccessPolicy/complete/test.tf create mode 100644 pkg/resources/testdata/TestAcc_RowAccessPolicy/complete/variables.tf diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index 1600061373..6d297f864b 100644 --- a/MIGRATION_GUIDE.md +++ b/MIGRATION_GUIDE.md @@ -19,7 +19,7 @@ Please rename these fields in your configuration files. State will be migrated a #### *(breaking change)* Adjusted behavior of arguments/signature Now, arguments are stored as a list, instead of a map. Please adjust that in your configs. State is migrated automatically. Also, this means that order of the items matters and may be adjusted. -Argument names are now case sensitive. +Argument names are now case sensitive. All policies created previously in the provider have upper case argument names. If you used lower case before, please adjust your configs. #### *(breaking change)* Adjusted behavior on changing name Previously, after changing `name` field, the resource was recreated. Now, the object is renamed with `RENAME TO`. @@ -31,7 +31,7 @@ Now, similarly to handling statements in other resources, we replace blank chara #### *(breaking change)* Identifiers related changes During [identifiers rework](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/ROADMAP.md#identifiers-rework) we decided to -migrate resource ids from pipe-separated to regular Snowflake identifiers (e.g. `|` -> `"".""`). +migrate resource ids from pipe-separated to regular Snowflake identifiers (e.g. `|` -> `"".""`). Importing resources also needs to be adjusted (see [example](https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs/resources/row_access_policy#import)). Also, we added diff suppress function that prevents Terraform from showing differences, when only quoting is different. diff --git a/examples/resources/snowflake_row_access_policy/import.sh b/examples/resources/snowflake_row_access_policy/import.sh index efc882a08e..2bf4d01b96 100644 --- a/examples/resources/snowflake_row_access_policy/import.sh +++ b/examples/resources/snowflake_row_access_policy/import.sh @@ -1 +1 @@ -terraform import snowflake_row_access_policy.example '""."".""' +terraform import snowflake_row_access_policy.example '""."".""' diff --git a/examples/resources/snowflake_row_access_policy/resource.tf b/examples/resources/snowflake_row_access_policy/resource.tf index 7e256f9700..c4ff60b7be 100644 --- a/examples/resources/snowflake_row_access_policy/resource.tf +++ b/examples/resources/snowflake_row_access_policy/resource.tf @@ -6,6 +6,14 @@ resource "snowflake_row_access_policy" "example_row_access_policy" { name = "ARG1" type = "VARCHAR" } + argument { + name = "ARG2" + type = "NUMBER" + } + argument { + name = "ARG3" + type = "TIMESTAMP_NTZ" + } body = "case when current_role() in ('ANALYST') then true else false end" comment = "comment" } diff --git a/pkg/acceptance/bettertestspoc/assert/resourceassert/gen/resource_schema_def.go b/pkg/acceptance/bettertestspoc/assert/resourceassert/gen/resource_schema_def.go index ab4b7bb538..e805b4c30a 100644 --- a/pkg/acceptance/bettertestspoc/assert/resourceassert/gen/resource_schema_def.go +++ b/pkg/acceptance/bettertestspoc/assert/resourceassert/gen/resource_schema_def.go @@ -41,4 +41,8 @@ var allResourceSchemaDefs = []ResourceSchemaDef{ name: "ResourceMonitor", schema: resources.ResourceMonitor().Schema, }, + { + name: "RowAccessPolicy", + schema: resources.RowAccessPolicy().Schema, + }, } diff --git a/pkg/acceptance/bettertestspoc/assert/resourceassert/row_access_policy_resource_ext.go b/pkg/acceptance/bettertestspoc/assert/resourceassert/row_access_policy_resource_ext.go new file mode 100644 index 0000000000..ec54107493 --- /dev/null +++ b/pkg/acceptance/bettertestspoc/assert/resourceassert/row_access_policy_resource_ext.go @@ -0,0 +1,18 @@ +package resourceassert + +import ( + "fmt" + "strconv" + + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" +) + +func (r *RowAccessPolicyResourceAssert) HasArguments(args []sdk.RowAccessPolicyArgument) *RowAccessPolicyResourceAssert { + r.AddAssertion(assert.ValueSet("argument.#", strconv.FormatInt(int64(len(args)), 10))) + for i, v := range args { + r.AddAssertion(assert.ValueSet(fmt.Sprintf("argument.%d.name", i), v.Name)) + r.AddAssertion(assert.ValueSet(fmt.Sprintf("argument.%d.type", i), string(v.Type))) + } + return r +} diff --git a/pkg/acceptance/bettertestspoc/config/model/row_access_policy_model_ext.go b/pkg/acceptance/bettertestspoc/config/model/row_access_policy_model_ext.go new file mode 100644 index 0000000000..fc11800ba3 --- /dev/null +++ b/pkg/acceptance/bettertestspoc/config/model/row_access_policy_model_ext.go @@ -0,0 +1,19 @@ +package model + +import ( + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" + "github.com/hashicorp/terraform-plugin-testing/config" + tfconfig "github.com/hashicorp/terraform-plugin-testing/config" +) + +func (r *RowAccessPolicyModel) WithArgument(argument []sdk.RowAccessPolicyArgument) *RowAccessPolicyModel { + maps := make([]config.Variable, len(argument)) + for i, v := range argument { + maps[i] = config.MapVariable(map[string]config.Variable{ + "name": config.StringVariable(v.Name), + "type": config.StringVariable(v.Type), + }) + } + r.Argument = tfconfig.SetVariable(maps...) + return r +} diff --git a/pkg/acceptance/bettertestspoc/config/model/row_access_policy_model_gen.go b/pkg/acceptance/bettertestspoc/config/model/row_access_policy_model_gen.go new file mode 100644 index 0000000000..e4a8460de2 --- /dev/null +++ b/pkg/acceptance/bettertestspoc/config/model/row_access_policy_model_gen.go @@ -0,0 +1,135 @@ +// Code generated by config model builder generator; DO NOT EDIT. + +package model + +import ( + tfconfig "github.com/hashicorp/terraform-plugin-testing/config" + + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/config" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources" +) + +type RowAccessPolicyModel struct { + Argument tfconfig.Variable `json:"argument,omitempty"` + Body tfconfig.Variable `json:"body,omitempty"` + Comment tfconfig.Variable `json:"comment,omitempty"` + Database tfconfig.Variable `json:"database,omitempty"` + FullyQualifiedName tfconfig.Variable `json:"fully_qualified_name,omitempty"` + Name tfconfig.Variable `json:"name,omitempty"` + Schema tfconfig.Variable `json:"schema,omitempty"` + + *config.ResourceModelMeta +} + +///////////////////////////////////////////////// +// Basic builders (resource name and required) // +///////////////////////////////////////////////// + +func RowAccessPolicy( + resourceName string, + argument []sdk.RowAccessPolicyArgument, + body string, + database string, + name string, + schema string, +) *RowAccessPolicyModel { + r := &RowAccessPolicyModel{ResourceModelMeta: config.Meta(resourceName, resources.RowAccessPolicy)} + r.WithArgument(argument) + r.WithBody(body) + r.WithDatabase(database) + r.WithName(name) + r.WithSchema(schema) + return r +} + +func RowAccessPolicyWithDefaultMeta( + argument []sdk.RowAccessPolicyArgument, + body string, + database string, + name string, + schema string, +) *RowAccessPolicyModel { + r := &RowAccessPolicyModel{ResourceModelMeta: config.DefaultMeta(resources.RowAccessPolicy)} + r.WithArgument(argument) + r.WithBody(body) + r.WithDatabase(database) + r.WithName(name) + r.WithSchema(schema) + return r +} + +///////////////////////////////// +// below all the proper values // +///////////////////////////////// + +// argument attribute type is not yet supported, so WithArgument can't be generated + +func (r *RowAccessPolicyModel) WithBody(body string) *RowAccessPolicyModel { + r.Body = tfconfig.StringVariable(body) + return r +} + +func (r *RowAccessPolicyModel) WithComment(comment string) *RowAccessPolicyModel { + r.Comment = tfconfig.StringVariable(comment) + return r +} + +func (r *RowAccessPolicyModel) WithDatabase(database string) *RowAccessPolicyModel { + r.Database = tfconfig.StringVariable(database) + return r +} + +func (r *RowAccessPolicyModel) WithFullyQualifiedName(fullyQualifiedName string) *RowAccessPolicyModel { + r.FullyQualifiedName = tfconfig.StringVariable(fullyQualifiedName) + return r +} + +func (r *RowAccessPolicyModel) WithName(name string) *RowAccessPolicyModel { + r.Name = tfconfig.StringVariable(name) + return r +} + +func (r *RowAccessPolicyModel) WithSchema(schema string) *RowAccessPolicyModel { + r.Schema = tfconfig.StringVariable(schema) + return r +} + +////////////////////////////////////////// +// below it's possible to set any value // +////////////////////////////////////////// + +func (r *RowAccessPolicyModel) WithArgumentValue(value tfconfig.Variable) *RowAccessPolicyModel { + r.Argument = value + return r +} + +func (r *RowAccessPolicyModel) WithBodyValue(value tfconfig.Variable) *RowAccessPolicyModel { + r.Body = value + return r +} + +func (r *RowAccessPolicyModel) WithCommentValue(value tfconfig.Variable) *RowAccessPolicyModel { + r.Comment = value + return r +} + +func (r *RowAccessPolicyModel) WithDatabaseValue(value tfconfig.Variable) *RowAccessPolicyModel { + r.Database = value + return r +} + +func (r *RowAccessPolicyModel) WithFullyQualifiedNameValue(value tfconfig.Variable) *RowAccessPolicyModel { + r.FullyQualifiedName = value + return r +} + +func (r *RowAccessPolicyModel) WithNameValue(value tfconfig.Variable) *RowAccessPolicyModel { + r.Name = value + return r +} + +func (r *RowAccessPolicyModel) WithSchemaValue(value tfconfig.Variable) *RowAccessPolicyModel { + r.Schema = value + return r +} diff --git a/pkg/acceptance/helpers/row_access_policy_client.go b/pkg/acceptance/helpers/row_access_policy_client.go index 89b71a49b5..fe5d97eccd 100644 --- a/pkg/acceptance/helpers/row_access_policy_client.go +++ b/pkg/acceptance/helpers/row_access_policy_client.go @@ -47,6 +47,19 @@ func (c *RowAccessPolicyClient) CreateRowAccessPolicyWithDataType(t *testing.T, return rowAccessPolicy, c.DropRowAccessPolicyFunc(t, id) } +func (c *RowAccessPolicyClient) CreateOrReplaceRowAccessPolicy(t *testing.T, req sdk.CreateRowAccessPolicyRequest) (*sdk.RowAccessPolicy, func()) { + t.Helper() + ctx := context.Background() + + err := c.client().Create(ctx, req.WithOrReplace(sdk.Pointer(true))) + require.NoError(t, err) + + rowAccessPolicy, err := c.client().ShowByID(ctx, req.GetName()) + require.NoError(t, err) + + return rowAccessPolicy, c.DropRowAccessPolicyFunc(t, req.GetName()) +} + func (c *RowAccessPolicyClient) Alter(t *testing.T, req sdk.AlterRowAccessPolicyRequest) { t.Helper() ctx := context.Background() diff --git a/pkg/resources/row_access_policy.go b/pkg/resources/row_access_policy.go index cd14408096..5b15d9774d 100644 --- a/pkg/resources/row_access_policy.go +++ b/pkg/resources/row_access_policy.go @@ -49,11 +49,9 @@ var rowAccessPolicySchema = map[string]*schema.Schema{ }, // TODO(SNOW-1596962): Fully support VECTOR data type sdk.ParseFunctionArgumentsFromString could be a base for another function that takes argument names into consideration. "type": { - Type: schema.TypeString, - Required: true, - DiffSuppressFunc: func(key, oldValue, newValue string, _ *schema.ResourceData) bool { - return NormalizeAndCompare(sdk.ToDataType)(key, oldValue, newValue, nil) - }, + Type: schema.TypeString, + Required: true, + DiffSuppressFunc: NormalizeAndCompare(sdk.ToDataType), ValidateDiagFunc: sdkValidation(sdk.ToDataType), Description: "The argument type", ForceNew: true, @@ -106,12 +104,12 @@ func RowAccessPolicy() *schema.Resource { Schema: rowAccessPolicySchema, Importer: &schema.ResourceImporter{ - StateContext: schema.ImportStatePassthroughContext, + StateContext: ImportRowAccessPolicy, }, CustomizeDiff: customdiff.All( - ComputedIfAnyAttributeChanged(rowAccessPolicySchema, ShowOutputAttributeName, "comment"), - ComputedIfAnyAttributeChanged(rowAccessPolicySchema, DescribeOutputAttributeName, "body"), + ComputedIfAnyAttributeChanged(rowAccessPolicySchema, ShowOutputAttributeName, "comment", "name"), + ComputedIfAnyAttributeChanged(rowAccessPolicySchema, DescribeOutputAttributeName, "body", "name", "signature"), ComputedIfAnyAttributeChanged(rowAccessPolicySchema, FullyQualifiedNameAttributeName, "name"), ), @@ -126,6 +124,47 @@ func RowAccessPolicy() *schema.Resource { } } +func ImportRowAccessPolicy(ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) { + log.Printf("[DEBUG] Starting row access policy import") + client := meta.(*provider.Context).Client + id, err := sdk.ParseSchemaObjectIdentifier(d.Id()) + if err != nil { + return nil, err + } + + policy, err := client.RowAccessPolicies.ShowByID(ctx, id) + if err != nil { + return nil, err + } + if err := d.Set("name", id.Name()); err != nil { + return nil, err + } + if err := d.Set("database", id.DatabaseName()); err != nil { + return nil, err + } + if err := d.Set("schema", id.SchemaName()); err != nil { + return nil, err + } + if err := d.Set("comment", policy.Comment); err != nil { + return nil, err + } + policyDescription, err := client.RowAccessPolicies.Describe(ctx, id) + if err != nil { + return nil, err + } + if err := d.Set("body", policyDescription.Body); err != nil { + return nil, err + } + args, err := policyDescription.Arguments() + if err != nil { + return nil, err + } + if err := d.Set("argument", schemas.RowAccessPolicyArgumentsToSchema(args)); err != nil { + return nil, err + } + return []*schema.ResourceData{d}, nil +} + func CreateRowAccessPolicy(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { client := meta.(*provider.Context).Client @@ -205,8 +244,11 @@ func ReadRowAccessPolicy(ctx context.Context, d *schema.ResourceData, meta any) if err := d.Set("body", rowAccessPolicyDescription.Body); err != nil { return diag.FromErr(err) } - - if err := d.Set("argument", rowAccessPolicyDescription.Arguments()); err != nil { + args, err := rowAccessPolicyDescription.Arguments() + if err != nil { + return diag.FromErr(err) + } + if err := d.Set("argument", schemas.RowAccessPolicyArgumentsToSchema(args)); err != nil { return diag.FromErr(err) } if err = d.Set(ShowOutputAttributeName, []map[string]any{schemas.RowAccessPolicyToSchema(rowAccessPolicy)}); err != nil { diff --git a/pkg/resources/row_access_policy_acceptance_test.go b/pkg/resources/row_access_policy_acceptance_test.go index cf9692c32b..b2b033425e 100644 --- a/pkg/resources/row_access_policy_acceptance_test.go +++ b/pkg/resources/row_access_policy_acceptance_test.go @@ -9,11 +9,12 @@ import ( "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert/resourceassert" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert/resourceshowoutputassert" + tfconfig "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/config" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/config/model" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/snowflakeroles" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources" - "github.com/hashicorp/terraform-plugin-testing/config" "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/plancheck" "github.com/hashicorp/terraform-plugin-testing/tfversion" @@ -23,13 +24,18 @@ func TestAcc_RowAccessPolicy(t *testing.T) { id := acc.TestClient().Ids.RandomSchemaObjectIdentifier() resourceName := "snowflake_row_access_policy.test" - m := func() map[string]config.Variable { - return map[string]config.Variable{ - "name": config.StringVariable(id.Name()), - "database": config.StringVariable(acc.TestDatabaseName), - "schema": config.StringVariable(acc.TestSchemaName), - } - } + body := "case when current_role() in ('ANALYST') then true else false end" + changedBody := "case when current_role() in ('CHANGED') then true else false end" + policyModel := model.RowAccessPolicy("test", []sdk.RowAccessPolicyArgument{ + { + Name: "N", + Type: string(sdk.DataTypeVARCHAR), + }, + { + Name: "V", + Type: string(sdk.DataTypeVARCHAR), + }, + }, body, id.DatabaseName(), id.Name(), id.SchemaName()).WithComment("Terraform acceptance test") resource.Test(t, resource.TestCase{ ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, @@ -40,20 +46,25 @@ func TestAcc_RowAccessPolicy(t *testing.T) { CheckDestroy: acc.CheckDestroy(t, resources.RowAccessPolicy), Steps: []resource.TestStep{ { - ConfigDirectory: config.TestStepDirectory(), - ConfigVariables: m(), + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_RowAccessPolicy/complete"), + ConfigVariables: tfconfig.ConfigVariablesFromModel(t, policyModel), Check: assert.AssertThat(t, resourceassert.RowAccessPolicyResource(t, resourceName). HasNameString(id.Name()). HasDatabaseString(id.DatabaseName()). HasSchemaString(id.SchemaName()). HasFullyQualifiedNameString(id.FullyQualifiedName()). HasCommentString("Terraform acceptance test"). - HasBodyString("case when current_role() in ('ANALYST') then true else false end"), - assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.#", "2")), - assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.0.name", "N")), - assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.0.type", "VARCHAR")), - assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.1.name", "V")), - assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.1.type", "VARCHAR")), + HasBodyString(body). + HasArguments([]sdk.RowAccessPolicyArgument{ + { + Name: "N", + Type: string(sdk.DataTypeVARCHAR), + }, + { + Name: "V", + Type: string(sdk.DataTypeVARCHAR), + }, + }), resourceshowoutputassert.RowAccessPolicyShowOutput(t, resourceName). HasCreatedOnNotEmpty(). HasDatabaseName(id.DatabaseName()). @@ -64,7 +75,7 @@ func TestAcc_RowAccessPolicy(t *testing.T) { HasOwnerRoleType("ROLE"). HasSchemaName(id.SchemaName()). HasComment("Terraform acceptance test"), - assert.Check(resource.TestCheckResourceAttr(resourceName, "describe_output.0.body", "case when current_role() in ('ANALYST') then true else false end")), + assert.Check(resource.TestCheckResourceAttr(resourceName, "describe_output.0.body", body)), assert.Check(resource.TestCheckResourceAttr(resourceName, "describe_output.0.name", id.Name())), assert.Check(resource.TestCheckResourceAttr(resourceName, "describe_output.0.return_type", "BOOLEAN")), assert.Check(resource.TestCheckResourceAttr(resourceName, "describe_output.0.signature", "(N VARCHAR, V VARCHAR)")), @@ -72,33 +83,80 @@ func TestAcc_RowAccessPolicy(t *testing.T) { }, // change comment and expression { - ConfigDirectory: config.TestStepDirectory(), - ConfigVariables: m(), + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_RowAccessPolicy/complete"), + ConfigVariables: tfconfig.ConfigVariablesFromModel(t, policyModel.WithBody(changedBody).WithComment("Terraform acceptance test - changed comment")), Check: assert.AssertThat(t, resourceassert.RowAccessPolicyResource(t, resourceName). HasNameString(id.Name()). HasDatabaseString(id.DatabaseName()). HasSchemaString(id.SchemaName()). HasFullyQualifiedNameString(id.FullyQualifiedName()). HasCommentString("Terraform acceptance test - changed comment"). - HasBodyString("case when current_role() in ('ANALYST') then false else true end"), - assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.#", "2")), - assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.0.name", "N")), - assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.0.type", "VARCHAR")), - assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.1.name", "V")), - assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.1.type", "VARCHAR")), + HasBodyString(changedBody). + HasArguments([]sdk.RowAccessPolicyArgument{ + { + Name: "N", + Type: string(sdk.DataTypeVARCHAR), + }, + { + Name: "V", + Type: string(sdk.DataTypeVARCHAR), + }, + }), ), }, // change signature { - ConfigDirectory: config.TestStepDirectory(), - ConfigVariables: m(), + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_RowAccessPolicy/complete"), + ConfigVariables: tfconfig.ConfigVariablesFromModel(t, policyModel.WithArgument([]sdk.RowAccessPolicyArgument{ + { + Name: "V", + Type: string(sdk.DataTypeBoolean), + }, + { + Name: "X", + Type: string(sdk.DataTypeTimestampNTZ), + }, + })), Check: assert.AssertThat(t, resourceassert.RowAccessPolicyResource(t, resourceName). HasNameString(id.Name()). HasDatabaseString(id.DatabaseName()). HasSchemaString(id.SchemaName()). HasFullyQualifiedNameString(id.FullyQualifiedName()). HasCommentString("Terraform acceptance test - changed comment"). - HasBodyString("case when current_role() in ('ANALYST') then false else true end"), + HasBodyString(changedBody). + HasArguments([]sdk.RowAccessPolicyArgument{ + { + Name: "V", + Type: string(sdk.DataTypeBoolean), + }, + { + Name: "X", + Type: string(sdk.DataTypeTimestampNTZ), + }, + }), + ), + }, + // external change on signature + { + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_RowAccessPolicy/complete"), + ConfigVariables: tfconfig.ConfigVariablesFromModel(t, policyModel), + PreConfig: func() { + arg := sdk.NewCreateRowAccessPolicyArgsRequest("A", sdk.DataTypeBoolean) + createRequest := sdk.NewCreateRowAccessPolicyRequest(id, []sdk.CreateRowAccessPolicyArgsRequest{*arg}, "case when current_role() in ('ANALYST') then false else true end") + acc.TestClient().RowAccessPolicy.CreateOrReplaceRowAccessPolicy(t, *createRequest) + }, + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionDestroyBeforeCreate), + }, + }, + Check: assert.AssertThat(t, resourceassert.RowAccessPolicyResource(t, resourceName). + HasNameString(id.Name()). + HasDatabaseString(id.DatabaseName()). + HasSchemaString(id.SchemaName()). + HasFullyQualifiedNameString(id.FullyQualifiedName()). + HasCommentString("Terraform acceptance test - changed comment"). + HasBodyString(changedBody), assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.#", "2")), assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.0.name", "V")), assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.0.type", "BOOLEAN")), @@ -108,8 +166,8 @@ func TestAcc_RowAccessPolicy(t *testing.T) { }, // external change on body { - ConfigDirectory: config.TestStepDirectory(), - ConfigVariables: m(), + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_RowAccessPolicy/complete"), + ConfigVariables: tfconfig.ConfigVariablesFromModel(t, policyModel), PreConfig: func() { acc.TestClient().RowAccessPolicy.Alter(t, *sdk.NewAlterRowAccessPolicyRequest(id).WithSetBody(sdk.Pointer("case when current_role() in ('EXTERNAL') then false else true end"))) }, @@ -119,17 +177,54 @@ func TestAcc_RowAccessPolicy(t *testing.T) { HasSchemaString(id.SchemaName()). HasFullyQualifiedNameString(id.FullyQualifiedName()). HasCommentString("Terraform acceptance test - changed comment"). - HasBodyString("case when current_role() in ('ANALYST') then false else true end"), - assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.#", "2")), - assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.0.name", "V")), - assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.0.type", "BOOLEAN")), - assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.1.name", "X")), - assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.1.type", "TIMESTAMP_NTZ")), + HasBodyString(changedBody). + HasArguments([]sdk.RowAccessPolicyArgument{ + { + Name: "V", + Type: string(sdk.DataTypeBoolean), + }, + { + Name: "X", + Type: string(sdk.DataTypeTimestampNTZ), + }, + }), + ), + }, + // unset comment + { + ConfigVariables: tfconfig.ConfigVariablesFromModel(t, policyModel), + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_RowAccessPolicy/complete"), + ConfigVariables: tfconfig.ConfigVariablesFromModel(t, policyModel.WithComment("")), + PreConfig: func() { + acc.TestClient().RowAccessPolicy.Alter(t, *sdk.NewAlterRowAccessPolicyRequest(id).WithSetBody(sdk.Pointer("case when current_role() in ('EXTERNAL') then false else true end"))) + }, + Check: assert.AssertThat(t, resourceassert.RowAccessPolicyResource(t, resourceName). + HasNameString(id.Name()). + HasDatabaseString(id.DatabaseName()). + HasSchemaString(id.SchemaName()). + HasFullyQualifiedNameString(id.FullyQualifiedName()). + HasCommentString(""). + HasBodyString(changedBody). + HasArguments([]sdk.RowAccessPolicyArgument{ + { + Name: "V", + Type: string(sdk.DataTypeBoolean), + }, + { + Name: "X", + Type: string(sdk.DataTypeTimestampNTZ), + }, + }), ), }, // IMPORT { - ConfigVariables: m(), + ConfigVariables: tfconfig.ConfigVariablesFromModel(t, policyModel), ResourceName: resourceName, ImportState: true, ImportStateVerify: true, @@ -142,20 +237,13 @@ func TestAcc_RowAccessPolicy(t *testing.T) { func TestAcc_RowAccessPolicy_Issue2053(t *testing.T) { id := acc.TestClient().Ids.RandomSchemaObjectIdentifier() resourceName := "snowflake_row_access_policy.test" - m := func() map[string]config.Variable { - return map[string]config.Variable{ - "name": config.StringVariable(id.Name()), - "database": config.StringVariable(acc.TestDatabaseName), - "schema": config.StringVariable(acc.TestSchemaName), - "arguments": config.SetVariable( - config.MapVariable(map[string]config.Variable{ - "name": config.StringVariable("A"), - "type": config.StringVariable("VARCHAR"), - }), - ), - "body": config.StringVariable("case when current_role() in ('ANALYST') then true else false end"), - } - } + body := "case when current_role() in ('ANALYST') then true else false end" + policyModel := model.RowAccessPolicy("test", []sdk.RowAccessPolicyArgument{ + { + Name: "A", + Type: string(sdk.DataTypeVARCHAR), + }, + }, body, id.DatabaseName(), id.Name(), id.SchemaName()) resource.Test(t, resource.TestCase{ TerraformVersionChecks: []tfversion.TerraformVersionCheck{ tfversion.RequireAbove(tfversion.Version1_5_0), @@ -185,7 +273,7 @@ func TestAcc_RowAccessPolicy_Issue2053(t *testing.T) { { ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, ConfigDirectory: acc.ConfigurationDirectory("TestAcc_RowAccessPolicy/basic"), - ConfigVariables: m(), + ConfigVariables: tfconfig.ConfigVariablesFromModel(t, policyModel), ConfigPlanChecks: resource.ConfigPlanChecks{ PreApply: []plancheck.PlanCheck{ plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionNoop), @@ -207,20 +295,13 @@ func TestAcc_RowAccessPolicy_Rename(t *testing.T) { id := acc.TestClient().Ids.RandomSchemaObjectIdentifier() newId := acc.TestClient().Ids.RandomSchemaObjectIdentifier() resourceName := "snowflake_row_access_policy.test" - m := func(identifier sdk.SchemaObjectIdentifier) config.Variables { - return config.Variables{ - "name": config.StringVariable(identifier.Name()), - "database": config.StringVariable(identifier.DatabaseName()), - "schema": config.StringVariable(identifier.SchemaName()), - "arguments": config.SetVariable( - config.MapVariable(map[string]config.Variable{ - "name": config.StringVariable("a"), - "type": config.StringVariable("VARCHAR"), - }), - ), - "body": config.StringVariable("case when current_role() in ('ANALYST') then true else false end"), - } - } + body := "case when current_role() in ('ANALYST') then true else false end" + policyModel := model.RowAccessPolicy("test", []sdk.RowAccessPolicyArgument{ + { + Name: "a", + Type: string(sdk.DataTypeVARCHAR), + }, + }, body, id.DatabaseName(), id.Name(), id.SchemaName()) resource.Test(t, resource.TestCase{ ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, @@ -232,7 +313,7 @@ func TestAcc_RowAccessPolicy_Rename(t *testing.T) { Steps: []resource.TestStep{ { ConfigDirectory: acc.ConfigurationDirectory("TestAcc_RowAccessPolicy/basic"), - ConfigVariables: m(id), + ConfigVariables: tfconfig.ConfigVariablesFromModel(t, policyModel), Check: assert.AssertThat(t, resourceassert.RowAccessPolicyResource(t, resourceName). HasNameString(id.Name()). HasFullyQualifiedNameString(id.FullyQualifiedName()), @@ -241,7 +322,7 @@ func TestAcc_RowAccessPolicy_Rename(t *testing.T) { // rename { ConfigDirectory: acc.ConfigurationDirectory("TestAcc_RowAccessPolicy/basic"), - ConfigVariables: m(newId), + ConfigVariables: tfconfig.ConfigVariablesFromModel(t, policyModel.WithName(newId.Name())), ConfigPlanChecks: resource.ConfigPlanChecks{ PreApply: []plancheck.PlanCheck{ plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate), @@ -264,6 +345,7 @@ resource "snowflake_row_access_policy" "test" { schema = "%s" signature = { A = "VARCHAR", + b = "VARCHAR", } row_access_expression = "%s" }`, id.Name(), id.DatabaseName(), id.SchemaName(), expr) @@ -304,22 +386,15 @@ EOT } func TestAcc_RowAccessPolicy_InvalidDataType(t *testing.T) { - id := acc.TestClient().Ids.RandomAccountObjectIdentifier() + id := acc.TestClient().Ids.RandomSchemaObjectIdentifier() - m := func() map[string]config.Variable { - return map[string]config.Variable{ - "name": config.StringVariable(id.Name()), - "database": config.StringVariable(acc.TestDatabaseName), - "schema": config.StringVariable(acc.TestSchemaName), - "arguments": config.SetVariable( - config.MapVariable(map[string]config.Variable{ - "name": config.StringVariable("A"), - "type": config.StringVariable("invalid-int"), - }), - ), - "body": config.StringVariable("case when current_role() in ('ANALYST') then true else false end"), - } - } + body := "case when current_role() in ('ANALYST') then true else false end" + policyModel := model.RowAccessPolicy("test", []sdk.RowAccessPolicyArgument{ + { + Name: "a", + Type: "invalid-type", + }, + }, body, id.DatabaseName(), id.Name(), id.SchemaName()) resource.Test(t, resource.TestCase{ ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, PreCheck: func() { acc.TestAccPreCheck(t) }, @@ -329,8 +404,8 @@ func TestAcc_RowAccessPolicy_InvalidDataType(t *testing.T) { Steps: []resource.TestStep{ { ConfigDirectory: acc.ConfigurationDirectory("TestAcc_RowAccessPolicy/basic"), - ConfigVariables: m(), - ExpectError: regexp.MustCompile(`invalid data type: invalid-int`), + ConfigVariables: tfconfig.ConfigVariablesFromModel(t, policyModel), + ExpectError: regexp.MustCompile(`invalid data type: invalid-type`), }, }, }) @@ -339,20 +414,14 @@ func TestAcc_RowAccessPolicy_InvalidDataType(t *testing.T) { func TestAcc_RowAccessPolicy_DataTypeAliases(t *testing.T) { id := acc.TestClient().Ids.RandomSchemaObjectIdentifier() resourceName := "snowflake_row_access_policy.test" - m := func() map[string]config.Variable { - return map[string]config.Variable{ - "name": config.StringVariable(id.Name()), - "database": config.StringVariable(id.DatabaseName()), - "schema": config.StringVariable(id.SchemaName()), - "arguments": config.SetVariable( - config.MapVariable(map[string]config.Variable{ - "name": config.StringVariable("A"), - "type": config.StringVariable("TEXT"), - }), - ), - "body": config.StringVariable("case when current_role() in ('ANALYST') then true else false end"), - } - } + body := "case when current_role() in ('ANALYST') then true else false end" + policyModel := model.RowAccessPolicy("test", []sdk.RowAccessPolicyArgument{ + { + Name: "A", + Type: "TEXT", + }, + }, body, id.DatabaseName(), id.Name(), id.SchemaName()) + resource.Test(t, resource.TestCase{ ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, PreCheck: func() { acc.TestAccPreCheck(t) }, @@ -362,11 +431,15 @@ func TestAcc_RowAccessPolicy_DataTypeAliases(t *testing.T) { Steps: []resource.TestStep{ { ConfigDirectory: acc.ConfigurationDirectory("TestAcc_RowAccessPolicy/basic"), - ConfigVariables: m(), + ConfigVariables: tfconfig.ConfigVariablesFromModel(t, policyModel), Check: assert.AssertThat(t, resourceassert.RowAccessPolicyResource(t, resourceName). - HasNameString(id.Name()), - assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.#", "1")), - assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.0.type", "VARCHAR")), + HasNameString(id.Name()). + HasArguments([]sdk.RowAccessPolicyArgument{ + { + Name: "A", + Type: string(sdk.DataTypeVARCHAR), + }, + }), ), }, }, @@ -377,18 +450,16 @@ func TestAcc_view_migrateFromVersion_0_95_0(t *testing.T) { id := acc.TestClient().Ids.RandomSchemaObjectIdentifier() resourceName := "snowflake_row_access_policy.test" body := "case when current_role() in ('ANALYST') then true else false end" - m := config.Variables{ - "name": config.StringVariable(id.Name()), - "database": config.StringVariable(id.DatabaseName()), - "schema": config.StringVariable(id.SchemaName()), - "arguments": config.SetVariable( - config.MapVariable(map[string]config.Variable{ - "name": config.StringVariable("A"), - "type": config.StringVariable("VARCHAR"), - }), - ), - "body": config.StringVariable("case when current_role() in ('ANALYST') then true else false end"), - } + policyModel := model.RowAccessPolicy("test", []sdk.RowAccessPolicyArgument{ + { + Name: "A", + Type: string(sdk.DataTypeVARCHAR), + }, + { + Name: "b", + Type: string(sdk.DataTypeVARCHAR), + }, + }, body, id.DatabaseName(), id.Name(), id.SchemaName()) resource.Test(t, resource.TestCase{ TerraformVersionChecks: []tfversion.TerraformVersionCheck{ @@ -404,29 +475,48 @@ func TestAcc_view_migrateFromVersion_0_95_0(t *testing.T) { }, }, Config: rowAccessPolicy_v0_95_0(id, body), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PostApplyPostRefresh: []plancheck.PlanCheck{ + // expect change - arg name is lower case which causes a diff + plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionDestroyBeforeCreate), + }, + }, + ExpectNonEmptyPlan: true, Check: assert.AssertThat(t, resourceassert.RowAccessPolicyResource(t, resourceName). HasNameString(id.Name()). HasDatabaseString(id.DatabaseName()). HasSchemaString(id.SchemaName()). HasFullyQualifiedNameString(id.FullyQualifiedName()), assert.Check(resource.TestCheckResourceAttr(resourceName, "row_access_expression", body)), + assert.Check(resource.TestCheckResourceAttr(resourceName, "signature.A", "VARCHAR")), + assert.Check(resource.TestCheckResourceAttr(resourceName, "signature.B", "VARCHAR")), ), }, { ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, ConfigDirectory: acc.ConfigurationDirectory("TestAcc_RowAccessPolicy/basic"), - ConfigVariables: m, + ConfigVariables: tfconfig.ConfigVariablesFromModel(t, policyModel), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PostApplyPostRefresh: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionNoop), + }, + }, Check: assert.AssertThat(t, resourceassert.RowAccessPolicyResource(t, resourceName). HasNameString(id.Name()). HasDatabaseString(id.DatabaseName()). HasSchemaString(id.SchemaName()). HasFullyQualifiedNameString(id.FullyQualifiedName()). - HasBodyString(body), - assert.Check(resource.TestCheckNoResourceAttr(resourceName, "row_access_expression")), - assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.#", "1")), - assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.0.name", "A")), - assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.0.type", "VARCHAR")), - assert.Check(resource.TestCheckNoResourceAttr(resourceName, "signature.A")), + HasBodyString(body). + HasArguments([]sdk.RowAccessPolicyArgument{ + { + Name: "A", + Type: string(sdk.DataTypeVARCHAR), + }, + { + Name: "b", + Type: string(sdk.DataTypeVARCHAR), + }, + }), ), }, }, diff --git a/pkg/resources/row_access_policy_state_upgraders.go b/pkg/resources/row_access_policy_state_upgraders.go index c1de13d6a9..a61efd6f9c 100644 --- a/pkg/resources/row_access_policy_state_upgraders.go +++ b/pkg/resources/row_access_policy_state_upgraders.go @@ -2,6 +2,7 @@ package resources import ( "context" + "strings" ) func v0_95_0_RowAccessPolicyStateUpgrader(ctx context.Context, rawState map[string]any, meta any) (map[string]any, error) { @@ -16,7 +17,7 @@ func v0_95_0_RowAccessPolicyStateUpgrader(ctx context.Context, rawState map[stri args := make([]map[string]any, 0) for k, v := range signature { args = append(args, map[string]any{ - "name": k, + "name": strings.ToUpper(k), "type": v, }) } diff --git a/pkg/resources/testdata/TestAcc_RowAccessPolicy/1/test.tf b/pkg/resources/testdata/TestAcc_RowAccessPolicy/1/test.tf deleted file mode 100644 index fee1c9aeee..0000000000 --- a/pkg/resources/testdata/TestAcc_RowAccessPolicy/1/test.tf +++ /dev/null @@ -1,15 +0,0 @@ -resource "snowflake_row_access_policy" "test" { - name = var.name - database = var.database - schema = var.schema - argument { - name = "N" - type = "VARCHAR" - } - argument { - name = "V" - type = "VARCHAR" - } - body = "case when current_role() in ('ANALYST') then true else false end" - comment = "Terraform acceptance test" -} diff --git a/pkg/resources/testdata/TestAcc_RowAccessPolicy/1/variables.tf b/pkg/resources/testdata/TestAcc_RowAccessPolicy/1/variables.tf deleted file mode 100644 index 31dd643cf2..0000000000 --- a/pkg/resources/testdata/TestAcc_RowAccessPolicy/1/variables.tf +++ /dev/null @@ -1,11 +0,0 @@ -variable "name" { - type = string -} - -variable "database" { - type = string -} - -variable "schema" { - type = string -} diff --git a/pkg/resources/testdata/TestAcc_RowAccessPolicy/2/test.tf b/pkg/resources/testdata/TestAcc_RowAccessPolicy/2/test.tf deleted file mode 100644 index e0541f4a2c..0000000000 --- a/pkg/resources/testdata/TestAcc_RowAccessPolicy/2/test.tf +++ /dev/null @@ -1,15 +0,0 @@ -resource "snowflake_row_access_policy" "test" { - name = var.name - database = var.database - schema = var.schema - argument { - name = "N" - type = "VARCHAR" - } - argument { - name = "V" - type = "VARCHAR" - } - body = "case when current_role() in ('ANALYST') then false else true end" - comment = "Terraform acceptance test - changed comment" -} diff --git a/pkg/resources/testdata/TestAcc_RowAccessPolicy/2/variables.tf b/pkg/resources/testdata/TestAcc_RowAccessPolicy/2/variables.tf deleted file mode 100644 index 31dd643cf2..0000000000 --- a/pkg/resources/testdata/TestAcc_RowAccessPolicy/2/variables.tf +++ /dev/null @@ -1,11 +0,0 @@ -variable "name" { - type = string -} - -variable "database" { - type = string -} - -variable "schema" { - type = string -} diff --git a/pkg/resources/testdata/TestAcc_RowAccessPolicy/3/test.tf b/pkg/resources/testdata/TestAcc_RowAccessPolicy/3/test.tf deleted file mode 100644 index 82be361ac5..0000000000 --- a/pkg/resources/testdata/TestAcc_RowAccessPolicy/3/test.tf +++ /dev/null @@ -1,15 +0,0 @@ -resource "snowflake_row_access_policy" "test" { - name = var.name - database = var.database - schema = var.schema - argument { - name = "V" - type = "BOOLEAN" - } - argument { - name = "X" - type = "TIMESTAMP_NTZ" - } - body = "case when current_role() in ('ANALYST') then false else true end" - comment = "Terraform acceptance test - changed comment" -} diff --git a/pkg/resources/testdata/TestAcc_RowAccessPolicy/3/variables.tf b/pkg/resources/testdata/TestAcc_RowAccessPolicy/3/variables.tf deleted file mode 100644 index 31dd643cf2..0000000000 --- a/pkg/resources/testdata/TestAcc_RowAccessPolicy/3/variables.tf +++ /dev/null @@ -1,11 +0,0 @@ -variable "name" { - type = string -} - -variable "database" { - type = string -} - -variable "schema" { - type = string -} diff --git a/pkg/resources/testdata/TestAcc_RowAccessPolicy/4/test.tf b/pkg/resources/testdata/TestAcc_RowAccessPolicy/4/test.tf deleted file mode 100644 index 82be361ac5..0000000000 --- a/pkg/resources/testdata/TestAcc_RowAccessPolicy/4/test.tf +++ /dev/null @@ -1,15 +0,0 @@ -resource "snowflake_row_access_policy" "test" { - name = var.name - database = var.database - schema = var.schema - argument { - name = "V" - type = "BOOLEAN" - } - argument { - name = "X" - type = "TIMESTAMP_NTZ" - } - body = "case when current_role() in ('ANALYST') then false else true end" - comment = "Terraform acceptance test - changed comment" -} diff --git a/pkg/resources/testdata/TestAcc_RowAccessPolicy/4/variables.tf b/pkg/resources/testdata/TestAcc_RowAccessPolicy/4/variables.tf deleted file mode 100644 index 31dd643cf2..0000000000 --- a/pkg/resources/testdata/TestAcc_RowAccessPolicy/4/variables.tf +++ /dev/null @@ -1,11 +0,0 @@ -variable "name" { - type = string -} - -variable "database" { - type = string -} - -variable "schema" { - type = string -} diff --git a/pkg/resources/testdata/TestAcc_RowAccessPolicy/basic/test.tf b/pkg/resources/testdata/TestAcc_RowAccessPolicy/basic/test.tf index c66b9ae65c..32b7731ead 100644 --- a/pkg/resources/testdata/TestAcc_RowAccessPolicy/basic/test.tf +++ b/pkg/resources/testdata/TestAcc_RowAccessPolicy/basic/test.tf @@ -3,12 +3,11 @@ resource "snowflake_row_access_policy" "test" { database = var.database schema = var.schema dynamic "argument" { - for_each = var.arguments + for_each = var.argument content { name = argument.value["name"] type = argument.value["type"] } } - # body = "case when current_role() in ('ANALYST') then true else false end" body = var.body } diff --git a/pkg/resources/testdata/TestAcc_RowAccessPolicy/basic/variables.tf b/pkg/resources/testdata/TestAcc_RowAccessPolicy/basic/variables.tf index fb0dfe1f59..6de3eb1d18 100644 --- a/pkg/resources/testdata/TestAcc_RowAccessPolicy/basic/variables.tf +++ b/pkg/resources/testdata/TestAcc_RowAccessPolicy/basic/variables.tf @@ -10,7 +10,7 @@ variable "schema" { type = string } -variable "arguments" { +variable "argument" { type = set(map(string)) } diff --git a/pkg/resources/testdata/TestAcc_RowAccessPolicy/complete/test.tf b/pkg/resources/testdata/TestAcc_RowAccessPolicy/complete/test.tf new file mode 100644 index 0000000000..bf7bb7d9e9 --- /dev/null +++ b/pkg/resources/testdata/TestAcc_RowAccessPolicy/complete/test.tf @@ -0,0 +1,14 @@ +resource "snowflake_row_access_policy" "test" { + name = var.name + database = var.database + schema = var.schema + dynamic "argument" { + for_each = var.argument + content { + name = argument.value["name"] + type = argument.value["type"] + } + } + body = var.body + comment = var.comment +} diff --git a/pkg/resources/testdata/TestAcc_RowAccessPolicy/complete/variables.tf b/pkg/resources/testdata/TestAcc_RowAccessPolicy/complete/variables.tf new file mode 100644 index 0000000000..c1ee696364 --- /dev/null +++ b/pkg/resources/testdata/TestAcc_RowAccessPolicy/complete/variables.tf @@ -0,0 +1,23 @@ +variable "name" { + type = string +} + +variable "database" { + type = string +} + +variable "schema" { + type = string +} + +variable "argument" { + type = set(map(string)) +} + +variable "body" { + type = string +} + +variable "comment" { + type = string +} diff --git a/pkg/schemas/row_access_policy.go b/pkg/schemas/row_access_policy.go index cc98576aec..ddb04f49aa 100644 --- a/pkg/schemas/row_access_policy.go +++ b/pkg/schemas/row_access_policy.go @@ -32,3 +32,14 @@ func RowAccessPolicyDescriptionToSchema(description sdk.RowAccessPolicyDescripti "body": description.Body, } } + +func RowAccessPolicyArgumentsToSchema(args []sdk.RowAccessPolicyArgument) []map[string]any { + schema := make([]map[string]any, len(args)) + for i, v := range args { + schema[i] = map[string]any{ + "name": v.Name, + "type": v.Type, + } + } + return schema +} diff --git a/pkg/sdk/row_access_policies_gen.go b/pkg/sdk/row_access_policies_gen.go index ff434b24ad..c515528fef 100644 --- a/pkg/sdk/row_access_policies_gen.go +++ b/pkg/sdk/row_access_policies_gen.go @@ -3,6 +3,7 @@ package sdk import ( "context" "database/sql" + "fmt" "strings" ) @@ -111,21 +112,28 @@ type RowAccessPolicyDescription struct { ReturnType string Body string } +type RowAccessPolicyArgument struct { + Name string + Type string +} // TODO(SNOW-1596962): Fully support VECTOR data type -func (d *RowAccessPolicyDescription) Arguments() []map[string]any { +func (d *RowAccessPolicyDescription) Arguments() ([]RowAccessPolicyArgument, error) { // Format in database is `(column )` plainSignature := strings.ReplaceAll(d.Signature, "(", "") plainSignature = strings.ReplaceAll(plainSignature, ")", "") signatureParts := strings.Split(plainSignature, ", ") - arguments := make([]map[string]any, len(signatureParts)) + arguments := make([]RowAccessPolicyArgument, len(signatureParts)) for i, e := range signatureParts { parts := strings.Split(e, " ") - arguments[i] = map[string]any{ - "name": strings.Join(parts[:len(parts)-1], " "), - "type": parts[len(parts)-1], + if len(parts) < 2 { + return nil, fmt.Errorf("parsing policy arguments: expected argument name and type, got %s", e) + } + arguments[i] = RowAccessPolicyArgument{ + Name: strings.Join(parts[:len(parts)-1], " "), + Type: parts[len(parts)-1], } } - return arguments + return arguments, nil } diff --git a/pkg/sdk/row_access_policies_gen_test.go b/pkg/sdk/row_access_policies_gen_test.go index e1006fdd0c..d94713ac69 100644 --- a/pkg/sdk/row_access_policies_gen_test.go +++ b/pkg/sdk/row_access_policies_gen_test.go @@ -294,8 +294,32 @@ func TestRowAccessPolicyDescription_Arguments(t *testing.T) { d := &RowAccessPolicyDescription{ Signature: tt.signature, } - got := d.Arguments() + got, err := d.Arguments() + require.NoError(t, err) require.Equal(t, tt.want, got) }) } } + +func TestRowAccessPolicyDescription_ArgumentsInvalid(t *testing.T) { + tests := []struct { + name string + signature string + err string + }{ + { + name: "signature without data type", + signature: "(A)", + err: "parsing policy arguments: expected argument name and type, got A", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + d := &RowAccessPolicyDescription{ + Signature: tt.signature, + } + _, err := d.Arguments() + require.ErrorContains(t, err, tt.err) + }) + } +} diff --git a/pkg/sdk/testint/row_access_policies_gen_integration_test.go b/pkg/sdk/testint/row_access_policies_gen_integration_test.go index ad6dc6f1d6..4fd38bae52 100644 --- a/pkg/sdk/testint/row_access_policies_gen_integration_test.go +++ b/pkg/sdk/testint/row_access_policies_gen_integration_test.go @@ -350,3 +350,60 @@ func TestInt_RowAccessPoliciesShowByID(t *testing.T) { require.Equal(t, id2, e2.ID()) }) } + +func TestInt_RowAccessPoliciesDescribe(t *testing.T) { + client := testClient(t) + ctx := testContext(t) + + cleanupRowAccessPolicyHandle := func(id sdk.SchemaObjectIdentifier) func() { + return func() { + err := client.RowAccessPolicies.Drop(ctx, sdk.NewDropRowAccessPolicyRequest(id)) + if errors.Is(err, sdk.ErrObjectNotExistOrAuthorized) { + return + } + require.NoError(t, err) + } + } + + createRowAccessPolicyHandle := func(t *testing.T, id sdk.SchemaObjectIdentifier, args []sdk.CreateRowAccessPolicyArgsRequest) { + t.Helper() + + err := client.RowAccessPolicies.Create(ctx, sdk.NewCreateRowAccessPolicyRequest(id, args, "true")) + require.NoError(t, err) + t.Cleanup(cleanupRowAccessPolicyHandle(id)) + } + + t.Run("describe", func(t *testing.T) { + id := testClientHelper().Ids.RandomSchemaObjectIdentifier() + + arg1 := sdk.NewCreateRowAccessPolicyArgsRequest(random.AlphaN(5), sdk.DataTypeVARCHAR) + arg2 := sdk.NewCreateRowAccessPolicyArgsRequest(random.AlphaN(5)+" foo", "TEXT") + arg3 := sdk.NewCreateRowAccessPolicyArgsRequest(random.AlphaN(5), "NUMBER(10, 5)") + args := []sdk.CreateRowAccessPolicyArgsRequest{*arg1, *arg2, *arg3} + + createRowAccessPolicyHandle(t, id, args) + + policy, err := client.RowAccessPolicies.Describe(ctx, id) + require.NoError(t, err) + assert.Equal(t, "true", policy.Body) + assert.Equal(t, id.Name(), policy.Name) + assert.Equal(t, "BOOLEAN", policy.ReturnType) + assert.Equal(t, fmt.Sprintf("(%s VARCHAR, %s VARCHAR, %s NUMBER)", arg1.Name, arg2.Name, arg3.Name), policy.Signature) + gotArgs, err := policy.Arguments() + require.NoError(t, err) + assert.Equal(t, []sdk.RowAccessPolicyArgument{ + { + Name: arg1.Name, + Type: "VARCHAR", + }, + { + Name: arg2.Name, + Type: "VARCHAR", + }, + { + Name: arg3.Name, + Type: "NUMBER", + }, + }, gotArgs) + }) +} From b9951a6afa1d861fe7d827af224c17411dd68b28 Mon Sep 17 00:00:00 2001 From: Jakub Michalak Date: Wed, 11 Sep 2024 15:20:29 +0200 Subject: [PATCH 5/9] Pre push --- docs/resources/row_access_policy.md | 10 +++++++++- .../resourceassert/row_access_policy_resource_ext.go | 2 +- .../config/model/row_access_policy_model_gen.go | 2 +- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/docs/resources/row_access_policy.md b/docs/resources/row_access_policy.md index ce612535fc..10823dd631 100644 --- a/docs/resources/row_access_policy.md +++ b/docs/resources/row_access_policy.md @@ -22,6 +22,14 @@ resource "snowflake_row_access_policy" "example_row_access_policy" { name = "ARG1" type = "VARCHAR" } + argument { + name = "ARG2" + type = "NUMBER" + } + argument { + name = "ARG3" + type = "TIMESTAMP_NTZ" + } body = "case when current_role() in ('ANALYST') then true else false end" comment = "comment" } @@ -91,5 +99,5 @@ Read-Only: Import is supported using the following syntax: ```shell -terraform import snowflake_row_access_policy.example '""."".""' +terraform import snowflake_row_access_policy.example '""."".""' ``` diff --git a/pkg/acceptance/bettertestspoc/assert/resourceassert/row_access_policy_resource_ext.go b/pkg/acceptance/bettertestspoc/assert/resourceassert/row_access_policy_resource_ext.go index ec54107493..725e0012ea 100644 --- a/pkg/acceptance/bettertestspoc/assert/resourceassert/row_access_policy_resource_ext.go +++ b/pkg/acceptance/bettertestspoc/assert/resourceassert/row_access_policy_resource_ext.go @@ -12,7 +12,7 @@ func (r *RowAccessPolicyResourceAssert) HasArguments(args []sdk.RowAccessPolicyA r.AddAssertion(assert.ValueSet("argument.#", strconv.FormatInt(int64(len(args)), 10))) for i, v := range args { r.AddAssertion(assert.ValueSet(fmt.Sprintf("argument.%d.name", i), v.Name)) - r.AddAssertion(assert.ValueSet(fmt.Sprintf("argument.%d.type", i), string(v.Type))) + r.AddAssertion(assert.ValueSet(fmt.Sprintf("argument.%d.type", i), v.Type)) } return r } diff --git a/pkg/acceptance/bettertestspoc/config/model/row_access_policy_model_gen.go b/pkg/acceptance/bettertestspoc/config/model/row_access_policy_model_gen.go index e4a8460de2..589bf1660b 100644 --- a/pkg/acceptance/bettertestspoc/config/model/row_access_policy_model_gen.go +++ b/pkg/acceptance/bettertestspoc/config/model/row_access_policy_model_gen.go @@ -6,8 +6,8 @@ import ( tfconfig "github.com/hashicorp/terraform-plugin-testing/config" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/config" - "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" ) type RowAccessPolicyModel struct { From 6cbc5b0849b2f8f4edd82771d3d5a478eea12446 Mon Sep 17 00:00:00 2001 From: Jakub Michalak Date: Wed, 11 Sep 2024 16:19:58 +0200 Subject: [PATCH 6/9] Review suggestions --- .../row_access_policy_acceptance_test.go | 176 +++++++++++------- 1 file changed, 106 insertions(+), 70 deletions(-) diff --git a/pkg/resources/row_access_policy_acceptance_test.go b/pkg/resources/row_access_policy_acceptance_test.go index b2b033425e..b6d4a7ea66 100644 --- a/pkg/resources/row_access_policy_acceptance_test.go +++ b/pkg/resources/row_access_policy_acceptance_test.go @@ -26,7 +26,17 @@ func TestAcc_RowAccessPolicy(t *testing.T) { body := "case when current_role() in ('ANALYST') then true else false end" changedBody := "case when current_role() in ('CHANGED') then true else false end" - policyModel := model.RowAccessPolicy("test", []sdk.RowAccessPolicyArgument{ + argument := []sdk.RowAccessPolicyArgument{ + { + Name: "N", + Type: string(sdk.DataTypeVARCHAR), + }, + { + Name: "V", + Type: string(sdk.DataTypeVARCHAR), + }, + } + changedArgument := []sdk.RowAccessPolicyArgument{ { Name: "N", Type: string(sdk.DataTypeVARCHAR), @@ -35,7 +45,8 @@ func TestAcc_RowAccessPolicy(t *testing.T) { Name: "V", Type: string(sdk.DataTypeVARCHAR), }, - }, body, id.DatabaseName(), id.Name(), id.SchemaName()).WithComment("Terraform acceptance test") + } + policyModel := model.RowAccessPolicy("test", argument, body, id.DatabaseName(), id.Name(), id.SchemaName()).WithComment("Terraform acceptance test") resource.Test(t, resource.TestCase{ ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, @@ -55,16 +66,7 @@ func TestAcc_RowAccessPolicy(t *testing.T) { HasFullyQualifiedNameString(id.FullyQualifiedName()). HasCommentString("Terraform acceptance test"). HasBodyString(body). - HasArguments([]sdk.RowAccessPolicyArgument{ - { - Name: "N", - Type: string(sdk.DataTypeVARCHAR), - }, - { - Name: "V", - Type: string(sdk.DataTypeVARCHAR), - }, - }), + HasArguments(argument), resourceshowoutputassert.RowAccessPolicyShowOutput(t, resourceName). HasCreatedOnNotEmpty(). HasDatabaseName(id.DatabaseName()). @@ -92,31 +94,13 @@ func TestAcc_RowAccessPolicy(t *testing.T) { HasFullyQualifiedNameString(id.FullyQualifiedName()). HasCommentString("Terraform acceptance test - changed comment"). HasBodyString(changedBody). - HasArguments([]sdk.RowAccessPolicyArgument{ - { - Name: "N", - Type: string(sdk.DataTypeVARCHAR), - }, - { - Name: "V", - Type: string(sdk.DataTypeVARCHAR), - }, - }), + HasArguments(argument), ), }, // change signature { ConfigDirectory: acc.ConfigurationDirectory("TestAcc_RowAccessPolicy/complete"), - ConfigVariables: tfconfig.ConfigVariablesFromModel(t, policyModel.WithArgument([]sdk.RowAccessPolicyArgument{ - { - Name: "V", - Type: string(sdk.DataTypeBoolean), - }, - { - Name: "X", - Type: string(sdk.DataTypeTimestampNTZ), - }, - })), + ConfigVariables: tfconfig.ConfigVariablesFromModel(t, policyModel.WithArgument(changedArgument)), Check: assert.AssertThat(t, resourceassert.RowAccessPolicyResource(t, resourceName). HasNameString(id.Name()). HasDatabaseString(id.DatabaseName()). @@ -124,16 +108,7 @@ func TestAcc_RowAccessPolicy(t *testing.T) { HasFullyQualifiedNameString(id.FullyQualifiedName()). HasCommentString("Terraform acceptance test - changed comment"). HasBodyString(changedBody). - HasArguments([]sdk.RowAccessPolicyArgument{ - { - Name: "V", - Type: string(sdk.DataTypeBoolean), - }, - { - Name: "X", - Type: string(sdk.DataTypeTimestampNTZ), - }, - }), + HasArguments(changedArgument), ), }, // external change on signature @@ -156,12 +131,8 @@ func TestAcc_RowAccessPolicy(t *testing.T) { HasSchemaString(id.SchemaName()). HasFullyQualifiedNameString(id.FullyQualifiedName()). HasCommentString("Terraform acceptance test - changed comment"). - HasBodyString(changedBody), - assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.#", "2")), - assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.0.name", "V")), - assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.0.type", "BOOLEAN")), - assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.1.name", "X")), - assert.Check(resource.TestCheckResourceAttr(resourceName, "argument.1.type", "TIMESTAMP_NTZ")), + HasBodyString(changedBody). + HasArguments(changedArgument), ), }, // external change on body @@ -178,25 +149,16 @@ func TestAcc_RowAccessPolicy(t *testing.T) { HasFullyQualifiedNameString(id.FullyQualifiedName()). HasCommentString("Terraform acceptance test - changed comment"). HasBodyString(changedBody). - HasArguments([]sdk.RowAccessPolicyArgument{ - { - Name: "V", - Type: string(sdk.DataTypeBoolean), - }, - { - Name: "X", - Type: string(sdk.DataTypeTimestampNTZ), - }, - }), + HasArguments(changedArgument), ), }, - // unset comment { ConfigVariables: tfconfig.ConfigVariablesFromModel(t, policyModel), ResourceName: resourceName, ImportState: true, ImportStateVerify: true, }, + // unset comment { ConfigDirectory: acc.ConfigurationDirectory("TestAcc_RowAccessPolicy/complete"), ConfigVariables: tfconfig.ConfigVariablesFromModel(t, policyModel.WithComment("")), @@ -210,16 +172,7 @@ func TestAcc_RowAccessPolicy(t *testing.T) { HasFullyQualifiedNameString(id.FullyQualifiedName()). HasCommentString(""). HasBodyString(changedBody). - HasArguments([]sdk.RowAccessPolicyArgument{ - { - Name: "V", - Type: string(sdk.DataTypeBoolean), - }, - { - Name: "X", - Type: string(sdk.DataTypeTimestampNTZ), - }, - }), + HasArguments(changedArgument), ), }, // IMPORT @@ -446,7 +399,7 @@ func TestAcc_RowAccessPolicy_DataTypeAliases(t *testing.T) { }) } -func TestAcc_view_migrateFromVersion_0_95_0(t *testing.T) { +func TestAcc_view_migrateFromVersion_0_95_0_LowercaseArgName(t *testing.T) { id := acc.TestClient().Ids.RandomSchemaObjectIdentifier() resourceName := "snowflake_row_access_policy.test" body := "case when current_role() in ('ANALYST') then true else false end" @@ -497,6 +450,9 @@ func TestAcc_view_migrateFromVersion_0_95_0(t *testing.T) { ConfigDirectory: acc.ConfigurationDirectory("TestAcc_RowAccessPolicy/basic"), ConfigVariables: tfconfig.ConfigVariablesFromModel(t, policyModel), ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionDestroyBeforeCreate), + }, PostApplyPostRefresh: []plancheck.PlanCheck{ plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionNoop), }, @@ -522,3 +478,83 @@ func TestAcc_view_migrateFromVersion_0_95_0(t *testing.T) { }, }) } + +func TestAcc_view_migrateFromVersion_0_95_0_UppercaseArgName(t *testing.T) { + id := acc.TestClient().Ids.RandomSchemaObjectIdentifier() + resourceName := "snowflake_row_access_policy.test" + body := "case when current_role() in ('ANALYST') then true else false end" + policyModel := model.RowAccessPolicy("test", []sdk.RowAccessPolicyArgument{ + { + Name: "A", + Type: string(sdk.DataTypeVARCHAR), + }, + { + Name: "B", + Type: string(sdk.DataTypeVARCHAR), + }, + }, body, id.DatabaseName(), id.Name(), id.SchemaName()) + + resource.Test(t, resource.TestCase{ + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + PreCheck: func() { acc.TestAccPreCheck(t) }, + Steps: []resource.TestStep{ + { + ExternalProviders: map[string]resource.ExternalProvider{ + "snowflake": { + VersionConstraint: "=0.95.0", + Source: "Snowflake-Labs/snowflake", + }, + }, + Config: rowAccessPolicy_v0_95_0(id, body), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PostApplyPostRefresh: []plancheck.PlanCheck{ + // expect change - arg name is lower case which causes a diff + plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionDestroyBeforeCreate), + }, + }, + ExpectNonEmptyPlan: true, + Check: assert.AssertThat(t, resourceassert.RowAccessPolicyResource(t, resourceName). + HasNameString(id.Name()). + HasDatabaseString(id.DatabaseName()). + HasSchemaString(id.SchemaName()). + HasFullyQualifiedNameString(id.FullyQualifiedName()), + assert.Check(resource.TestCheckResourceAttr(resourceName, "row_access_expression", body)), + assert.Check(resource.TestCheckResourceAttr(resourceName, "signature.A", "VARCHAR")), + assert.Check(resource.TestCheckResourceAttr(resourceName, "signature.B", "VARCHAR")), + ), + }, + { + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_RowAccessPolicy/basic"), + ConfigVariables: tfconfig.ConfigVariablesFromModel(t, policyModel), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionNoop), + }, + PostApplyPostRefresh: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionNoop), + }, + }, + Check: assert.AssertThat(t, resourceassert.RowAccessPolicyResource(t, resourceName). + HasNameString(id.Name()). + HasDatabaseString(id.DatabaseName()). + HasSchemaString(id.SchemaName()). + HasFullyQualifiedNameString(id.FullyQualifiedName()). + HasBodyString(body). + HasArguments([]sdk.RowAccessPolicyArgument{ + { + Name: "A", + Type: string(sdk.DataTypeVARCHAR), + }, + { + Name: "B", + Type: string(sdk.DataTypeVARCHAR), + }, + }), + ), + }, + }, + }) +} From 6bfb6ed431c2e55f6e6241da32b679b7ae2930de Mon Sep 17 00:00:00 2001 From: Jakub Michalak Date: Wed, 11 Sep 2024 16:24:51 +0200 Subject: [PATCH 7/9] Linka a jira --- pkg/sdk/row_access_policies_gen.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/sdk/row_access_policies_gen.go b/pkg/sdk/row_access_policies_gen.go index c515528fef..6fdb9af2fb 100644 --- a/pkg/sdk/row_access_policies_gen.go +++ b/pkg/sdk/row_access_policies_gen.go @@ -118,6 +118,7 @@ type RowAccessPolicyArgument struct { } // TODO(SNOW-1596962): Fully support VECTOR data type +// TODO(SNOW-1660588): Use ParseFunctionArgumentsFromString func (d *RowAccessPolicyDescription) Arguments() ([]RowAccessPolicyArgument, error) { // Format in database is `(column )` plainSignature := strings.ReplaceAll(d.Signature, "(", "") From 04df0b04525ef06d0c09b71ba6bebb1d4dafd976 Mon Sep 17 00:00:00 2001 From: Jakub Michalak Date: Thu, 12 Sep 2024 14:52:49 +0200 Subject: [PATCH 8/9] Fix --- MIGRATION_GUIDE.md | 2 +- docs/resources/row_access_policy.md | 2 +- .../helpers/row_access_policy_client.go | 14 +- pkg/resources/row_access_policy.go | 2 +- .../row_access_policy_acceptance_test.go | 16 +- pkg/sdk/row_access_policies_gen_test.go | 24 +-- ...ow_access_policies_gen_integration_test.go | 204 ++++++++---------- 7 files changed, 122 insertions(+), 142 deletions(-) diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index 6d297f864b..9a933195c2 100644 --- a/MIGRATION_GUIDE.md +++ b/MIGRATION_GUIDE.md @@ -19,7 +19,7 @@ Please rename these fields in your configuration files. State will be migrated a #### *(breaking change)* Adjusted behavior of arguments/signature Now, arguments are stored as a list, instead of a map. Please adjust that in your configs. State is migrated automatically. Also, this means that order of the items matters and may be adjusted. -Argument names are now case sensitive. All policies created previously in the provider have upper case argument names. If you used lower case before, please adjust your configs. +Argument names are now case sensitive. All policies created previously in the provider have upper case argument names. If you used lower case before, please adjust your configs. Values in the state will be migrated to uppercase automatically. #### *(breaking change)* Adjusted behavior on changing name Previously, after changing `name` field, the resource was recreated. Now, the object is renamed with `RENAME TO`. diff --git a/docs/resources/row_access_policy.md b/docs/resources/row_access_policy.md index 10823dd631..17173abf53 100644 --- a/docs/resources/row_access_policy.md +++ b/docs/resources/row_access_policy.md @@ -65,7 +65,7 @@ resource "snowflake_row_access_policy" "example_row_access_policy" { Required: - `name` (String) The argument name -- `type` (String) The argument type +- `type` (String) The argument type. VECTOR data types are not yet supported. For more information about data types, check [Snowflake docs](https://docs.snowflake.com/en/sql-reference/intro-summary-data-types). diff --git a/pkg/acceptance/helpers/row_access_policy_client.go b/pkg/acceptance/helpers/row_access_policy_client.go index fe5d97eccd..b8fbfbfdc9 100644 --- a/pkg/acceptance/helpers/row_access_policy_client.go +++ b/pkg/acceptance/helpers/row_access_policy_client.go @@ -31,12 +31,18 @@ func (c *RowAccessPolicyClient) CreateRowAccessPolicy(t *testing.T) (*sdk.RowAcc func (c *RowAccessPolicyClient) CreateRowAccessPolicyWithDataType(t *testing.T, datatype sdk.DataType) (*sdk.RowAccessPolicy, func()) { t.Helper() + + arg := sdk.NewCreateRowAccessPolicyArgsRequest("A", datatype) + return c.CreateRowAccessPolicyWithArguments(t, []sdk.CreateRowAccessPolicyArgsRequest{*arg}) +} + +func (c *RowAccessPolicyClient) CreateRowAccessPolicyWithArguments(t *testing.T, args []sdk.CreateRowAccessPolicyArgsRequest) (*sdk.RowAccessPolicy, func()) { + t.Helper() ctx := context.Background() id := c.ids.RandomSchemaObjectIdentifier() - arg := sdk.NewCreateRowAccessPolicyArgsRequest("A", datatype) body := "true" - createRequest := sdk.NewCreateRowAccessPolicyRequest(id, []sdk.CreateRowAccessPolicyArgsRequest{*arg}, body) + createRequest := sdk.NewCreateRowAccessPolicyRequest(id, args, body) err := c.client().Create(ctx, createRequest) require.NoError(t, err) @@ -47,11 +53,11 @@ func (c *RowAccessPolicyClient) CreateRowAccessPolicyWithDataType(t *testing.T, return rowAccessPolicy, c.DropRowAccessPolicyFunc(t, id) } -func (c *RowAccessPolicyClient) CreateOrReplaceRowAccessPolicy(t *testing.T, req sdk.CreateRowAccessPolicyRequest) (*sdk.RowAccessPolicy, func()) { +func (c *RowAccessPolicyClient) CreateRowAccessPolicyWithRequest(t *testing.T, req sdk.CreateRowAccessPolicyRequest) (*sdk.RowAccessPolicy, func()) { t.Helper() ctx := context.Background() - err := c.client().Create(ctx, req.WithOrReplace(sdk.Pointer(true))) + err := c.client().Create(ctx, &req) require.NoError(t, err) rowAccessPolicy, err := c.client().ShowByID(ctx, req.GetName()) diff --git a/pkg/resources/row_access_policy.go b/pkg/resources/row_access_policy.go index 5b15d9774d..b137635922 100644 --- a/pkg/resources/row_access_policy.go +++ b/pkg/resources/row_access_policy.go @@ -53,7 +53,7 @@ var rowAccessPolicySchema = map[string]*schema.Schema{ Required: true, DiffSuppressFunc: NormalizeAndCompare(sdk.ToDataType), ValidateDiagFunc: sdkValidation(sdk.ToDataType), - Description: "The argument type", + Description: "The argument type. VECTOR data types are not yet supported. For more information about data types, check [Snowflake docs](https://docs.snowflake.com/en/sql-reference/intro-summary-data-types).", ForceNew: true, }, }, diff --git a/pkg/resources/row_access_policy_acceptance_test.go b/pkg/resources/row_access_policy_acceptance_test.go index b6d4a7ea66..796f1df167 100644 --- a/pkg/resources/row_access_policy_acceptance_test.go +++ b/pkg/resources/row_access_policy_acceptance_test.go @@ -28,22 +28,22 @@ func TestAcc_RowAccessPolicy(t *testing.T) { changedBody := "case when current_role() in ('CHANGED') then true else false end" argument := []sdk.RowAccessPolicyArgument{ { - Name: "N", + Name: "A", Type: string(sdk.DataTypeVARCHAR), }, { - Name: "V", + Name: "B", Type: string(sdk.DataTypeVARCHAR), }, } changedArgument := []sdk.RowAccessPolicyArgument{ { - Name: "N", - Type: string(sdk.DataTypeVARCHAR), + Name: "C", + Type: string(sdk.DataTypeBoolean), }, { - Name: "V", - Type: string(sdk.DataTypeVARCHAR), + Name: "D", + Type: string(sdk.DataTypeTimestampNTZ), }, } policyModel := model.RowAccessPolicy("test", argument, body, id.DatabaseName(), id.Name(), id.SchemaName()).WithComment("Terraform acceptance test") @@ -80,7 +80,7 @@ func TestAcc_RowAccessPolicy(t *testing.T) { assert.Check(resource.TestCheckResourceAttr(resourceName, "describe_output.0.body", body)), assert.Check(resource.TestCheckResourceAttr(resourceName, "describe_output.0.name", id.Name())), assert.Check(resource.TestCheckResourceAttr(resourceName, "describe_output.0.return_type", "BOOLEAN")), - assert.Check(resource.TestCheckResourceAttr(resourceName, "describe_output.0.signature", "(N VARCHAR, V VARCHAR)")), + assert.Check(resource.TestCheckResourceAttr(resourceName, "describe_output.0.signature", "(A VARCHAR, B VARCHAR)")), ), }, // change comment and expression @@ -118,7 +118,7 @@ func TestAcc_RowAccessPolicy(t *testing.T) { PreConfig: func() { arg := sdk.NewCreateRowAccessPolicyArgsRequest("A", sdk.DataTypeBoolean) createRequest := sdk.NewCreateRowAccessPolicyRequest(id, []sdk.CreateRowAccessPolicyArgsRequest{*arg}, "case when current_role() in ('ANALYST') then false else true end") - acc.TestClient().RowAccessPolicy.CreateOrReplaceRowAccessPolicy(t, *createRequest) + acc.TestClient().RowAccessPolicy.CreateRowAccessPolicyWithRequest(t, *createRequest.WithOrReplace(sdk.Pointer(true))) }, ConfigPlanChecks: resource.ConfigPlanChecks{ PreApply: []plancheck.PlanCheck{ diff --git a/pkg/sdk/row_access_policies_gen_test.go b/pkg/sdk/row_access_policies_gen_test.go index d94713ac69..4363fac206 100644 --- a/pkg/sdk/row_access_policies_gen_test.go +++ b/pkg/sdk/row_access_policies_gen_test.go @@ -252,39 +252,39 @@ func TestRowAccessPolicyDescription_Arguments(t *testing.T) { tests := []struct { name string signature string - want []map[string]any + want []RowAccessPolicyArgument }{ { name: "signature with 1 arg", signature: "(A VARCHAR)", - want: []map[string]any{ + want: []RowAccessPolicyArgument{ { - "name": "A", - "type": "VARCHAR", + Name: "A", + Type: "VARCHAR", }, }, }, { name: "signature with multiple args", signature: "(A VARCHAR, B BOOLEAN)", - want: []map[string]any{ + want: []RowAccessPolicyArgument{ { - "name": "A", - "type": "VARCHAR", + Name: "A", + Type: "VARCHAR", }, { - "name": "B", - "type": "BOOLEAN", + Name: "B", + Type: "BOOLEAN", }, }, }, { name: "signature with complex name", signature: "(a B VARCHAR)", - want: []map[string]any{ + want: []RowAccessPolicyArgument{ { - "name": "a B", - "type": "VARCHAR", + Name: "a B", + Type: "VARCHAR", }, }, }, diff --git a/pkg/sdk/testint/row_access_policies_gen_integration_test.go b/pkg/sdk/testint/row_access_policies_gen_integration_test.go index 4fd38bae52..8e89b9ebf0 100644 --- a/pkg/sdk/testint/row_access_policies_gen_integration_test.go +++ b/pkg/sdk/testint/row_access_policies_gen_integration_test.go @@ -1,9 +1,7 @@ package testint import ( - "errors" "fmt" - "strings" "testing" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/helpers/random" @@ -40,13 +38,6 @@ func TestInt_RowAccessPolicies(t *testing.T) { }, *rowAccessPolicyDescription) } - cleanupRowAccessPolicyProvider := func(id sdk.SchemaObjectIdentifier) func() { - return func() { - err := client.RowAccessPolicies.Drop(ctx, sdk.NewDropRowAccessPolicyRequest(id)) - require.NoError(t, err) - } - } - createRowAccessPolicyRequest := func(t *testing.T, args []sdk.CreateRowAccessPolicyArgsRequest, body string) *sdk.CreateRowAccessPolicyRequest { t.Helper() id := testClientHelper().Ids.RandomSchemaObjectIdentifier() @@ -66,38 +57,20 @@ func TestInt_RowAccessPolicies(t *testing.T) { return createRowAccessPolicyRequest(t, []sdk.CreateRowAccessPolicyArgsRequest{*args}, body) } - createRowAccessPolicyWithRequest := func(t *testing.T, request *sdk.CreateRowAccessPolicyRequest) *sdk.RowAccessPolicy { - t.Helper() - id := request.GetName() - - err := client.RowAccessPolicies.Create(ctx, request) - require.NoError(t, err) - t.Cleanup(cleanupRowAccessPolicyProvider(id)) - - rowAccessPolicy, err := client.RowAccessPolicies.ShowByID(ctx, id) - require.NoError(t, err) - - return rowAccessPolicy - } - - createRowAccessPolicy := func(t *testing.T) *sdk.RowAccessPolicy { - t.Helper() - return createRowAccessPolicyWithRequest(t, createRowAccessPolicyBasicRequest(t)) - } - t.Run("create row access policy: no optionals", func(t *testing.T) { request := createRowAccessPolicyBasicRequest(t) - rowAccessPolicy := createRowAccessPolicyWithRequest(t, request) + rowAccessPolicy, cleanup := testClientHelper().RowAccessPolicy.CreateRowAccessPolicyWithRequest(t, *request) + t.Cleanup(cleanup) assertRowAccessPolicy(t, rowAccessPolicy, request.GetName(), "") }) t.Run("create row access policy: full", func(t *testing.T) { - request := createRowAccessPolicyBasicRequest(t) - request.Comment = sdk.String("some comment") + request := createRowAccessPolicyBasicRequest(t).WithComment(sdk.Pointer("some comment")) - rowAccessPolicy := createRowAccessPolicyWithRequest(t, request) + rowAccessPolicy, cleanup := testClientHelper().RowAccessPolicy.CreateRowAccessPolicyWithRequest(t, *request) + t.Cleanup(cleanup) assertRowAccessPolicy(t, rowAccessPolicy, request.GetName(), "some comment") }) @@ -133,9 +106,9 @@ func TestInt_RowAccessPolicies(t *testing.T) { err = client.RowAccessPolicies.Alter(ctx, alterRequest) if err != nil { - t.Cleanup(cleanupRowAccessPolicyProvider(id)) + t.Cleanup(testClientHelper().RowAccessPolicy.DropRowAccessPolicyFunc(t, id)) } else { - t.Cleanup(cleanupRowAccessPolicyProvider(newId)) + t.Cleanup(testClientHelper().RowAccessPolicy.DropRowAccessPolicyFunc(t, newId)) } require.NoError(t, err) @@ -149,7 +122,8 @@ func TestInt_RowAccessPolicies(t *testing.T) { }) t.Run("alter row access policy: set and unset comment", func(t *testing.T) { - rowAccessPolicy := createRowAccessPolicy(t) + rowAccessPolicy, cleanup := testClientHelper().RowAccessPolicy.CreateRowAccessPolicy(t) + t.Cleanup(cleanup) id := rowAccessPolicy.ID() alterRequest := sdk.NewAlterRowAccessPolicyRequest(id).WithSetComment(sdk.String("new comment")) @@ -172,7 +146,8 @@ func TestInt_RowAccessPolicies(t *testing.T) { }) t.Run("alter row access policy: set body", func(t *testing.T) { - rowAccessPolicy := createRowAccessPolicy(t) + rowAccessPolicy, cleanup := testClientHelper().RowAccessPolicy.CreateRowAccessPolicy(t) + t.Cleanup(cleanup) id := rowAccessPolicy.ID() alterRequest := sdk.NewAlterRowAccessPolicyRequest(id).WithSetBody(sdk.String("false")) @@ -198,7 +173,8 @@ func TestInt_RowAccessPolicies(t *testing.T) { tag, tagCleanup := testClientHelper().Tag.CreateTag(t) t.Cleanup(tagCleanup) - rowAccessPolicy := createRowAccessPolicy(t) + rowAccessPolicy, cleanup := testClientHelper().RowAccessPolicy.CreateRowAccessPolicy(t) + t.Cleanup(cleanup) id := rowAccessPolicy.ID() tagValue := "abc" @@ -231,8 +207,10 @@ func TestInt_RowAccessPolicies(t *testing.T) { }) t.Run("show row access policy: default", func(t *testing.T) { - rowAccessPolicy1 := createRowAccessPolicy(t) - rowAccessPolicy2 := createRowAccessPolicy(t) + rowAccessPolicy1, cleanup1 := testClientHelper().RowAccessPolicy.CreateRowAccessPolicy(t) + t.Cleanup(cleanup1) + rowAccessPolicy2, cleanup2 := testClientHelper().RowAccessPolicy.CreateRowAccessPolicy(t) + t.Cleanup(cleanup2) showRequest := sdk.NewShowRowAccessPolicyRequest() returnedRowAccessPolicies, err := client.RowAccessPolicies.Show(ctx, showRequest) @@ -243,8 +221,10 @@ func TestInt_RowAccessPolicies(t *testing.T) { }) t.Run("show row access policy: with options", func(t *testing.T) { - rowAccessPolicy1 := createRowAccessPolicy(t) - rowAccessPolicy2 := createRowAccessPolicy(t) + rowAccessPolicy1, cleanup1 := testClientHelper().RowAccessPolicy.CreateRowAccessPolicy(t) + t.Cleanup(cleanup1) + rowAccessPolicy2, cleanup2 := testClientHelper().RowAccessPolicy.CreateRowAccessPolicy(t) + t.Cleanup(cleanup2) showRequest := sdk.NewShowRowAccessPolicyRequest(). WithLike(&sdk.Like{Pattern: &rowAccessPolicy1.Name}). @@ -264,42 +244,45 @@ func TestInt_RowAccessPolicies(t *testing.T) { body := "true" request := createRowAccessPolicyRequest(t, []sdk.CreateRowAccessPolicyArgsRequest{*args}, body) - rowAccessPolicy := createRowAccessPolicyWithRequest(t, request) + rowAccessPolicy, cleanup := testClientHelper().RowAccessPolicy.CreateRowAccessPolicyWithRequest(t, *request) + t.Cleanup(cleanup) returnedRowAccessPolicyDescription, err := client.RowAccessPolicies.Describe(ctx, rowAccessPolicy.ID()) require.NoError(t, err) - assertRowAccessPolicyDescription(t, returnedRowAccessPolicyDescription, rowAccessPolicy.ID(), fmt.Sprintf("(%s %s)", strings.ToUpper(argName), argType), body) + assertRowAccessPolicyDescription(t, returnedRowAccessPolicyDescription, rowAccessPolicy.ID(), fmt.Sprintf("(%s %s)", argName, argType), body) }) - t.Run("describe row access policy: with data type normalization", func(t *testing.T) { + t.Run("describe row access policy: with timestamp data type normalization", func(t *testing.T) { argName := random.AlphaN(5) argType := sdk.DataTypeTimestamp args := sdk.NewCreateRowAccessPolicyArgsRequest(argName, argType) body := "true" request := createRowAccessPolicyRequest(t, []sdk.CreateRowAccessPolicyArgsRequest{*args}, body) - rowAccessPolicy := createRowAccessPolicyWithRequest(t, request) + rowAccessPolicy, cleanup := testClientHelper().RowAccessPolicy.CreateRowAccessPolicyWithRequest(t, *request) + t.Cleanup(cleanup) returnedRowAccessPolicyDescription, err := client.RowAccessPolicies.Describe(ctx, rowAccessPolicy.ID()) require.NoError(t, err) - assertRowAccessPolicyDescription(t, returnedRowAccessPolicyDescription, rowAccessPolicy.ID(), fmt.Sprintf("(%s %s)", strings.ToUpper(argName), sdk.DataTypeTimestampNTZ), body) + assertRowAccessPolicyDescription(t, returnedRowAccessPolicyDescription, rowAccessPolicy.ID(), fmt.Sprintf("(%s %s)", argName, sdk.DataTypeTimestampNTZ), body) }) - t.Run("describe row access policy: with data type normalization", func(t *testing.T) { + t.Run("describe row access policy: with varchar data type normalization", func(t *testing.T) { argName := random.AlphaN(5) argType := sdk.DataType("VARCHAR(200)") args := sdk.NewCreateRowAccessPolicyArgsRequest(argName, argType) body := "true" request := createRowAccessPolicyRequest(t, []sdk.CreateRowAccessPolicyArgsRequest{*args}, body) - rowAccessPolicy := createRowAccessPolicyWithRequest(t, request) + rowAccessPolicy, cleanup := testClientHelper().RowAccessPolicy.CreateRowAccessPolicyWithRequest(t, *request) + t.Cleanup(cleanup) returnedRowAccessPolicyDescription, err := client.RowAccessPolicies.Describe(ctx, rowAccessPolicy.ID()) require.NoError(t, err) - assertRowAccessPolicyDescription(t, returnedRowAccessPolicyDescription, rowAccessPolicy.ID(), fmt.Sprintf("(%s %s)", strings.ToUpper(argName), sdk.DataTypeVARCHAR), body) + assertRowAccessPolicyDescription(t, returnedRowAccessPolicyDescription, rowAccessPolicy.ID(), fmt.Sprintf("(%s %s)", argName, sdk.DataTypeVARCHAR), body) }) t.Run("describe row access policy: non-existing", func(t *testing.T) { @@ -312,25 +295,6 @@ func TestInt_RowAccessPoliciesShowByID(t *testing.T) { client := testClient(t) ctx := testContext(t) - cleanupRowAccessPolicyHandle := func(id sdk.SchemaObjectIdentifier) func() { - return func() { - err := client.RowAccessPolicies.Drop(ctx, sdk.NewDropRowAccessPolicyRequest(id)) - if errors.Is(err, sdk.ErrObjectNotExistOrAuthorized) { - return - } - require.NoError(t, err) - } - } - - createRowAccessPolicyHandle := func(t *testing.T, id sdk.SchemaObjectIdentifier) { - t.Helper() - - args := sdk.NewCreateRowAccessPolicyArgsRequest(random.AlphaN(5), sdk.DataTypeVARCHAR) - err := client.RowAccessPolicies.Create(ctx, sdk.NewCreateRowAccessPolicyRequest(id, []sdk.CreateRowAccessPolicyArgsRequest{*args}, "true")) - require.NoError(t, err) - t.Cleanup(cleanupRowAccessPolicyHandle(id)) - } - t.Run("show by id - same name in different schemas", func(t *testing.T) { schema, schemaCleanup := testClientHelper().Schema.CreateSchema(t) t.Cleanup(schemaCleanup) @@ -338,8 +302,17 @@ func TestInt_RowAccessPoliciesShowByID(t *testing.T) { id1 := testClientHelper().Ids.RandomSchemaObjectIdentifier() id2 := testClientHelper().Ids.NewSchemaObjectIdentifierInSchema(id1.Name(), schema.ID()) - createRowAccessPolicyHandle(t, id1) - createRowAccessPolicyHandle(t, id2) + body := "true" + argName := random.AlphaN(5) + argType := sdk.DataTypeVARCHAR + arg := sdk.NewCreateRowAccessPolicyArgsRequest(argName, argType) + + req1 := sdk.NewCreateRowAccessPolicyRequest(id1, []sdk.CreateRowAccessPolicyArgsRequest{*arg}, body) + req2 := sdk.NewCreateRowAccessPolicyRequest(id2, []sdk.CreateRowAccessPolicyArgsRequest{*arg}, body) + _, cleanup1 := testClientHelper().RowAccessPolicy.CreateRowAccessPolicyWithRequest(t, *req1) + t.Cleanup(cleanup1) + _, cleanup2 := testClientHelper().RowAccessPolicy.CreateRowAccessPolicyWithRequest(t, *req2) + t.Cleanup(cleanup2) e1, err := client.RowAccessPolicies.ShowByID(ctx, id1) require.NoError(t, err) @@ -355,55 +328,56 @@ func TestInt_RowAccessPoliciesDescribe(t *testing.T) { client := testClient(t) ctx := testContext(t) - cleanupRowAccessPolicyHandle := func(id sdk.SchemaObjectIdentifier) func() { - return func() { - err := client.RowAccessPolicies.Drop(ctx, sdk.NewDropRowAccessPolicyRequest(id)) - if errors.Is(err, sdk.ErrObjectNotExistOrAuthorized) { - return - } - require.NoError(t, err) - } - } - - createRowAccessPolicyHandle := func(t *testing.T, id sdk.SchemaObjectIdentifier, args []sdk.CreateRowAccessPolicyArgsRequest) { - t.Helper() - - err := client.RowAccessPolicies.Create(ctx, sdk.NewCreateRowAccessPolicyRequest(id, args, "true")) - require.NoError(t, err) - t.Cleanup(cleanupRowAccessPolicyHandle(id)) - } - t.Run("describe", func(t *testing.T) { - id := testClientHelper().Ids.RandomSchemaObjectIdentifier() - - arg1 := sdk.NewCreateRowAccessPolicyArgsRequest(random.AlphaN(5), sdk.DataTypeVARCHAR) - arg2 := sdk.NewCreateRowAccessPolicyArgsRequest(random.AlphaN(5)+" foo", "TEXT") - arg3 := sdk.NewCreateRowAccessPolicyArgsRequest(random.AlphaN(5), "NUMBER(10, 5)") - args := []sdk.CreateRowAccessPolicyArgsRequest{*arg1, *arg2, *arg3} + args := []sdk.CreateRowAccessPolicyArgsRequest{ + *sdk.NewCreateRowAccessPolicyArgsRequest("A", "NUMBER(2, 0)"), + *sdk.NewCreateRowAccessPolicyArgsRequest("B", "DECIMAL"), + *sdk.NewCreateRowAccessPolicyArgsRequest("C", "INTEGER"), + *sdk.NewCreateRowAccessPolicyArgsRequest("D", sdk.DataTypeFloat), + *sdk.NewCreateRowAccessPolicyArgsRequest("E", "DOUBLE"), + *sdk.NewCreateRowAccessPolicyArgsRequest("F", "VARCHAR(20)"), + *sdk.NewCreateRowAccessPolicyArgsRequest("G", "CHAR"), + *sdk.NewCreateRowAccessPolicyArgsRequest("H", sdk.DataTypeString), + *sdk.NewCreateRowAccessPolicyArgsRequest("I", "TEXT"), + *sdk.NewCreateRowAccessPolicyArgsRequest("J", sdk.DataTypeBinary), + *sdk.NewCreateRowAccessPolicyArgsRequest("K", "VARBINARY"), + *sdk.NewCreateRowAccessPolicyArgsRequest("L", sdk.DataTypeBoolean), + *sdk.NewCreateRowAccessPolicyArgsRequest("M", sdk.DataTypeDate), + *sdk.NewCreateRowAccessPolicyArgsRequest("N", "DATETIME"), + *sdk.NewCreateRowAccessPolicyArgsRequest("O", sdk.DataTypeTime), + *sdk.NewCreateRowAccessPolicyArgsRequest("P", sdk.DataTypeTimestamp), + *sdk.NewCreateRowAccessPolicyArgsRequest("R", sdk.DataTypeTimestampLTZ), + *sdk.NewCreateRowAccessPolicyArgsRequest("S", sdk.DataTypeTimestampNTZ), + *sdk.NewCreateRowAccessPolicyArgsRequest("T", sdk.DataTypeTimestampTZ), + *sdk.NewCreateRowAccessPolicyArgsRequest("U", sdk.DataTypeVariant), + *sdk.NewCreateRowAccessPolicyArgsRequest("V", sdk.DataTypeObject), + *sdk.NewCreateRowAccessPolicyArgsRequest("W", sdk.DataTypeArray), + *sdk.NewCreateRowAccessPolicyArgsRequest("X", sdk.DataTypeGeography), + *sdk.NewCreateRowAccessPolicyArgsRequest("Y", sdk.DataTypeGeometry), + // TODO(SNOW-1596962): Fully support VECTOR data type sdk.ParseFunctionArgumentsFromString could be a base for another function that takes argument names into consideration. + // *sdk.NewCreateRowAccessPolicyArgsRequest("Z", "VECTOR(INT, 16)"), + } - createRowAccessPolicyHandle(t, id, args) + policy, cleanup := testClientHelper().RowAccessPolicy.CreateRowAccessPolicyWithArguments(t, args) + t.Cleanup(cleanup) - policy, err := client.RowAccessPolicies.Describe(ctx, id) + id := policy.ID() + policyDetails, err := client.RowAccessPolicies.Describe(ctx, id) require.NoError(t, err) - assert.Equal(t, "true", policy.Body) - assert.Equal(t, id.Name(), policy.Name) - assert.Equal(t, "BOOLEAN", policy.ReturnType) - assert.Equal(t, fmt.Sprintf("(%s VARCHAR, %s VARCHAR, %s NUMBER)", arg1.Name, arg2.Name, arg3.Name), policy.Signature) - gotArgs, err := policy.Arguments() + assert.Equal(t, "true", policyDetails.Body) + assert.Equal(t, id.Name(), policyDetails.Name) + assert.Equal(t, "BOOLEAN", policyDetails.ReturnType) + gotArgs, err := policyDetails.Arguments() require.NoError(t, err) - assert.Equal(t, []sdk.RowAccessPolicyArgument{ - { - Name: arg1.Name, - Type: "VARCHAR", - }, - { - Name: arg2.Name, - Type: "VARCHAR", - }, - { - Name: arg3.Name, - Type: "NUMBER", - }, - }, gotArgs) + wantArgs := make([]sdk.RowAccessPolicyArgument, len(args)) + for i, arg := range args { + dataType, err := sdk.ToDataType(string(arg.Type)) + require.NoError(t, err) + wantArgs[i] = sdk.RowAccessPolicyArgument{ + Name: arg.Name, + Type: string(dataType), + } + } + assert.Equal(t, wantArgs, gotArgs) }) } From 497b25e0184f1696bfc597228fac6c2e6e330dcc Mon Sep 17 00:00:00 2001 From: Jakub Michalak Date: Thu, 12 Sep 2024 16:01:25 +0200 Subject: [PATCH 9/9] Fix --- pkg/resources/row_access_policy.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/pkg/resources/row_access_policy.go b/pkg/resources/row_access_policy.go index b137635922..c9fd812af3 100644 --- a/pkg/resources/row_access_policy.go +++ b/pkg/resources/row_access_policy.go @@ -220,18 +220,6 @@ func ReadRowAccessPolicy(ctx context.Context, d *schema.ResourceData, meta any) return diag.FromErr(err) } - if err := d.Set("name", rowAccessPolicy.Name); err != nil { - return diag.FromErr(err) - } - - if err := d.Set("database", rowAccessPolicy.DatabaseName); err != nil { - return diag.FromErr(err) - } - - if err := d.Set("schema", rowAccessPolicy.SchemaName); err != nil { - return diag.FromErr(err) - } - if err := d.Set("comment", rowAccessPolicy.Comment); err != nil { return diag.FromErr(err) }