From 8a178eeaf8dffacfc103f62e88acaeb5c455c3eb Mon Sep 17 00:00:00 2001 From: Scott Winkler Date: Fri, 31 Mar 2023 12:22:20 -0700 Subject: [PATCH 1/5] add on_account to session and object params --- docs/data-sources/parameters.md | 1 + docs/resources/object_parameter.md | 6 +- docs/resources/session_parameter.md | 12 ++ .../snowflake_object_parameter/resource.tf | 5 +- .../snowflake_session_parameter/resource.tf | 7 + pkg/datasources/parameters.go | 19 +- pkg/resources/account_parameter.go | 4 +- pkg/resources/object_parameter.go | 39 +++-- .../object_parameter_acceptance_test.go | 29 ++-- pkg/resources/session_parameter.go | 70 ++++++-- .../session_parameter_acceptance_test.go | 45 ++++- pkg/snowflake/parameters.go | 162 ++++++++++++++---- 12 files changed, 318 insertions(+), 81 deletions(-) diff --git a/docs/data-sources/parameters.md b/docs/data-sources/parameters.md index b6f0eea2b2..c2b0445ef2 100644 --- a/docs/data-sources/parameters.md +++ b/docs/data-sources/parameters.md @@ -46,6 +46,7 @@ data "snowflake_parameters" "p3" { - `object_type` (String) If parameter_type is set to "OBJECT" then object_type is the type of object to display object parameters for. Valid values are any object supported by the IN clause of the [SHOW PARAMETERS](https://docs.snowflake.com/en/sql-reference/sql/show-parameters.html#parameters) statement, including: WAREHOUSE | DATABASE | SCHEMA | TASK | TABLE - `parameter_type` (String) The type of parameter to filter by. Valid values are: "ACCOUNT", "SESSION", "OBJECT". - `pattern` (String) Allows limiting the list of parameters by name using LIKE clause. Refer to [Limiting the List of Parameters by Name](https://docs.snowflake.com/en/sql-reference/parameters.html#limiting-the-list-of-parameters-by-name) +- `user` (String) If parameter_type is set to "SESSION" then user is the name of the user to display session parameters for. ### Read-Only diff --git a/docs/resources/object_parameter.md b/docs/resources/object_parameter.md index b234f7a838..d04f1b01d5 100644 --- a/docs/resources/object_parameter.md +++ b/docs/resources/object_parameter.md @@ -64,8 +64,9 @@ resource "snowflake_object_parameter" "o3" { // Setting object parameter at account level resource "snowflake_object_parameter" "o4" { - key = "DATA_RETENTION_TIME_IN_DAYS" - value = "89" + key = "DATA_RETENTION_TIME_IN_DAYS" + value = "89" + on_account = true } ``` @@ -81,6 +82,7 @@ resource "snowflake_object_parameter" "o4" { - `object_identifier` (Block List) Specifies the object identifier for the object parameter. If no value is provided, then the resource will default to setting the object parameter at account level. (see [below for nested schema](#nestedblock--object_identifier)) - `object_type` (String) Type of object to which the parameter applies. Valid values are those in [object types](https://docs.snowflake.com/en/sql-reference/parameters.html#object-types). If no value is provided, then the resource will default to setting the object parameter at account level. +- `on_account` (Boolean) If true, the object parameter will be set on the account level. ### Read-Only diff --git a/docs/resources/session_parameter.md b/docs/resources/session_parameter.md index 5ad236adf3..3e2f798d8c 100644 --- a/docs/resources/session_parameter.md +++ b/docs/resources/session_parameter.md @@ -16,6 +16,13 @@ description: |- resource "snowflake_session_parameter" "s" { key = "AUTOCOMMIT" value = "false" + user = "TEST_USER" +} + +resource "snowflake_session_parameter" "s2" { + key = "BINARY_OUTPUT_FORMAT" + value = "BASE64" + on_account = true } ``` @@ -27,6 +34,11 @@ resource "snowflake_session_parameter" "s" { - `key` (String) Name of session parameter. Valid values are those in [session parameters](https://docs.snowflake.com/en/sql-reference/parameters.html#session-parameters). - `value` (String) Value of session parameter, as a string. Constraints are the same as those for the parameters in Snowflake documentation. +### Optional + +- `on_account` (Boolean) If true, the session parameter will be set on the account level. +- `user` (String) The user to set the session parameter for. Required if on_account is false + ### Read-Only - `id` (String) The ID of this resource. diff --git a/examples/resources/snowflake_object_parameter/resource.tf b/examples/resources/snowflake_object_parameter/resource.tf index ab7aef606e..a42afa04c4 100644 --- a/examples/resources/snowflake_object_parameter/resource.tf +++ b/examples/resources/snowflake_object_parameter/resource.tf @@ -49,6 +49,7 @@ resource "snowflake_object_parameter" "o3" { // Setting object parameter at account level resource "snowflake_object_parameter" "o4" { - key = "DATA_RETENTION_TIME_IN_DAYS" - value = "89" + key = "DATA_RETENTION_TIME_IN_DAYS" + value = "89" + on_account = true } diff --git a/examples/resources/snowflake_session_parameter/resource.tf b/examples/resources/snowflake_session_parameter/resource.tf index 1f399e8642..c7c0dc2b30 100644 --- a/examples/resources/snowflake_session_parameter/resource.tf +++ b/examples/resources/snowflake_session_parameter/resource.tf @@ -1,4 +1,11 @@ resource "snowflake_session_parameter" "s" { key = "AUTOCOMMIT" value = "false" + user = "TEST_USER" +} + +resource "snowflake_session_parameter" "s2" { + key = "BINARY_OUTPUT_FORMAT" + value = "BASE64" + on_account = true } diff --git a/pkg/datasources/parameters.go b/pkg/datasources/parameters.go index 31740ea7ad..fd966ebba1 100644 --- a/pkg/datasources/parameters.go +++ b/pkg/datasources/parameters.go @@ -3,6 +3,7 @@ package datasources import ( "database/sql" "errors" + "fmt" "log" "strings" @@ -24,6 +25,11 @@ var parametersSchema = map[string]*schema.Schema{ Optional: true, Description: "Allows limiting the list of parameters by name using LIKE clause. Refer to [Limiting the List of Parameters by Name](https://docs.snowflake.com/en/sql-reference/parameters.html#limiting-the-list-of-parameters-by-name)", }, + "user": { + Type: schema.TypeString, + Optional: true, + Description: "If parameter_type is set to \"SESSION\" then user is the name of the user to display session parameters for.", + }, "object_type": { Type: schema.TypeString, Optional: true, @@ -120,7 +126,18 @@ func ReadParameters(d *schema.ResourceData, meta interface{}) error { } return d.Set("parameters", params) } - parameters, err := snowflake.ListParameters(db, parameterType, pattern) + var parameters []snowflake.Parameter + var err error + switch parameterType { + case snowflake.ParameterTypeAccount: + parameters, err = snowflake.ListAccountParameters(db, pattern) + case snowflake.ParameterTypeSession: + user := d.Get("user").(string) + if user == "" { + return fmt.Errorf("user is required when parameter_type is set to SESSION") + } + parameters, err = snowflake.ListSessionParameters(db, user, pattern) + } if errors.Is(err, sql.ErrNoRows) { log.Printf("[DEBUG] parameters not found") d.SetId("") diff --git a/pkg/resources/account_parameter.go b/pkg/resources/account_parameter.go index c58cc0e74c..03a2fe805a 100644 --- a/pkg/resources/account_parameter.go +++ b/pkg/resources/account_parameter.go @@ -59,7 +59,7 @@ func CreateAccountParameter(d *schema.ResourceData, meta interface{}) error { value = fmt.Sprintf("'%s'", snowflake.EscapeString(value)) } - builder := snowflake.NewParameter(key, value, snowflake.ParameterTypeAccount, db) + builder := snowflake.NewAccountParameter(key, value, db) err := builder.SetParameter() if err != nil { return fmt.Errorf("error creating account parameter err = %w", err) @@ -111,7 +111,7 @@ func DeleteAccountParameter(d *schema.ResourceData, meta interface{}) error { if reflect.TypeOf(parameterDefault.DefaultValue) == typeString { value = fmt.Sprintf("'%s'", value) } - builder := snowflake.NewParameter(key, value, snowflake.ParameterTypeAccount, db) + builder := snowflake.NewAccountParameter(key, value, db) err := builder.SetParameter() if err != nil { return fmt.Errorf("error creating account parameter err = %w", err) diff --git a/pkg/resources/object_parameter.go b/pkg/resources/object_parameter.go index fbd0c7f45d..4cfbd24d84 100644 --- a/pkg/resources/object_parameter.go +++ b/pkg/resources/object_parameter.go @@ -27,6 +27,12 @@ var objectParameterSchema = map[string]*schema.Schema{ Required: true, Description: "Value of object parameter, as a string. Constraints are the same as those for the parameters in Snowflake documentation.", }, + "on_account": { + Type: schema.TypeBool, + Optional: true, + Default: false, + Description: "If true, the object parameter will be set on the account level.", + }, "object_type": { Type: schema.TypeString, Optional: true, @@ -99,7 +105,12 @@ func CreateObjectParameter(d *schema.ResourceData, meta interface{}) error { value = fmt.Sprintf("'%s'", snowflake.EscapeString(value)) } - builder := snowflake.NewParameter(key, value, snowflake.ParameterTypeObject, db) + builder := snowflake.NewObjectParameter(key, value, db) + + onAccount := d.Get("on_account").(bool) + if onAccount { + builder.SetOnAccount(onAccount) + } var fullyQualifierObjectIdentifier string if v, ok := d.GetOk("object_identifier"); ok { @@ -121,7 +132,7 @@ func CreateObjectParameter(d *schema.ResourceData, meta interface{}) error { if err != nil { return fmt.Errorf("error creating object parameter err = %w", err) } - id := fmt.Sprintf("%v❄️%v❄️%v", key, objectType, fullyQualifierObjectIdentifier) + id := fmt.Sprintf("%v|%v|%v", key, objectType, fullyQualifierObjectIdentifier) d.SetId(id) var p *snowflake.Parameter if fullyQualifierObjectIdentifier != "" { @@ -143,9 +154,12 @@ func CreateObjectParameter(d *schema.ResourceData, meta interface{}) error { func ReadObjectParameter(d *schema.ResourceData, meta interface{}) error { db := meta.(*sql.DB) id := d.Id() - parts := strings.Split(id, "❄️") + parts := strings.Split(id, "|") + if len(parts) != 3 { + parts = strings.Split(id, "❄️") // for backwards compatibility + } if len(parts) != 3 { - return fmt.Errorf("unexpected format of ID (%v), expected key❄️object_type❄️object_identifier", id) + return fmt.Errorf("unexpected format of ID (%v), expected key|object_type|object_identifier", id) } key := parts[0] var p *snowflake.Parameter @@ -185,7 +199,12 @@ func DeleteObjectParameter(d *schema.ResourceData, meta interface{}) error { if reflect.TypeOf(parameterDefault.DefaultValue) == typeString { value = fmt.Sprintf("'%s'", value) } - builder := snowflake.NewParameter(key, value, snowflake.ParameterTypeObject, db) + builder := snowflake.NewObjectParameter(key, value, db) + + onAccount := d.Get("on_account").(bool) + if onAccount { + builder.SetOnAccount(onAccount) + } var fullyQualifierObjectIdentifier string if v, ok := d.GetOk("object_identifier"); ok { @@ -193,6 +212,7 @@ func DeleteObjectParameter(d *schema.ResourceData, meta interface{}) error { fullyQualifierObjectIdentifier = snowflakeValidation.FormatFullyQualifiedObjectID(objectDatabase, objectSchema, objectName) builder.WithObjectIdentifier(fullyQualifierObjectIdentifier) } + var objectType snowflake.ObjectType if v, ok := d.GetOk("object_type"); ok { objectType = snowflake.ObjectType(v.(string)) @@ -201,18 +221,11 @@ func DeleteObjectParameter(d *schema.ResourceData, meta interface{}) error { } builder.WithObjectType(objectType) } + err := builder.SetParameter() if err != nil { return fmt.Errorf("error deleting object parameter err = %w", err) } - if fullyQualifierObjectIdentifier != "" { - _, err = snowflake.ShowObjectParameter(db, key, objectType, fullyQualifierObjectIdentifier) - } else { - _, err = snowflake.ShowAccountParameter(db, key) - } - if err != nil { - return fmt.Errorf("error reading object parameter err = %w", err) - } d.SetId("") return nil diff --git a/pkg/resources/object_parameter_acceptance_test.go b/pkg/resources/object_parameter_acceptance_test.go index 9e23b7b59c..5e66a9e99e 100644 --- a/pkg/resources/object_parameter_acceptance_test.go +++ b/pkg/resources/object_parameter_acceptance_test.go @@ -16,10 +16,11 @@ func TestAcc_ObjectParameter(t *testing.T) { CheckDestroy: nil, Steps: []resource.TestStep{ { - Config: objectParameterBasic(prefix, "ENABLE_STREAM_TASK_REPLICATION", "true"), + Config: objectParameterConfigBasic(prefix, "ENABLE_STREAM_TASK_REPLICATION", "true"), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("snowflake_object_parameter.p", "key", "ENABLE_STREAM_TASK_REPLICATION"), resource.TestCheckResourceAttr("snowflake_object_parameter.p", "value", "true"), + resource.TestCheckResourceAttr("snowflake_object_parameter.p", "on_account", "false"), ), }, }, @@ -27,40 +28,34 @@ func TestAcc_ObjectParameter(t *testing.T) { } func TestAcc_ObjectParameterAccount(t *testing.T) { - prefix := "tst-terraform" + strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) resource.ParallelTest(t, resource.TestCase{ Providers: providers(), CheckDestroy: nil, Steps: []resource.TestStep{ { - Config: objectParameterAccount(prefix, "ENABLE_STREAM_TASK_REPLICATION", "true"), + Config: objectParameterConfigOnAccount("DATA_RETENTION_TIME_IN_DAYS", "0"), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("snowflake_object_parameter.p", "key", "ENABLE_STREAM_TASK_REPLICATION"), - resource.TestCheckResourceAttr("snowflake_object_parameter.p", "value", "true"), + resource.TestCheckResourceAttr("snowflake_object_parameter.p", "key", "DATA_RETENTION_TIME_IN_DAYS"), + resource.TestCheckResourceAttr("snowflake_object_parameter.p", "value", "0"), + resource.TestCheckResourceAttr("snowflake_object_parameter.p", "on_account", "true"), ), }, }, }) } -func objectParameterBasic(prefix, key, value string) string { +func objectParameterConfigOnAccount(key, value string) string { s := ` -resource "snowflake_database" "d" { - name = "%s" -} resource "snowflake_object_parameter" "p" { key = "%s" value = "%s" - object_type = "DATABASE" - object_identifier { - name = snowflake_database.d.name - } + on_account = true } ` - return fmt.Sprintf(s, prefix, key, value) + return fmt.Sprintf(s, key, value) } -func objectParameterAccount(prefix, key, value string) string { +func objectParameterConfigBasic(prefix, key, value string) string { s := ` resource "snowflake_database" "d" { name = "%s" @@ -68,6 +63,10 @@ resource "snowflake_database" "d" { resource "snowflake_object_parameter" "p" { key = "%s" value = "%s" + object_type = "DATABASE" + object_identifier { + name = snowflake_database.d.name + } } ` return fmt.Sprintf(s, prefix, key, value) diff --git a/pkg/resources/session_parameter.go b/pkg/resources/session_parameter.go index fc42fd8f7f..525291a36a 100644 --- a/pkg/resources/session_parameter.go +++ b/pkg/resources/session_parameter.go @@ -24,6 +24,17 @@ var sessionParameterSchema = map[string]*schema.Schema{ Required: true, Description: "Value of session parameter, as a string. Constraints are the same as those for the parameters in Snowflake documentation.", }, + "on_account": { + Type: schema.TypeBool, + Optional: true, + Default: false, + Description: "If true, the session parameter will be set on the account level.", + }, + "user": { + Type: schema.TypeString, + Optional: true, + Description: "The user to set the session parameter for. Required if on_account is false", + }, } func SessionParameter() *schema.Resource { @@ -59,17 +70,34 @@ func CreateSessionParameter(d *schema.ResourceData, meta interface{}) error { value = fmt.Sprintf("'%s'", snowflake.EscapeString(value)) } - builder := snowflake.NewParameter(key, value, snowflake.ParameterTypeSession, db) + builder := snowflake.NewSessionParameter(key, value, db) + + onAccount := d.Get("on_account").(bool) + user := d.Get("user").(string) + + if onAccount { + builder.SetOnAccount(onAccount) + } else { + if user == "" { + return fmt.Errorf("user is required if on_account is false") + } + builder.SetUser(user) + } + err := builder.SetParameter() if err != nil { return fmt.Errorf("error creating session parameter err = %w", err) } d.SetId(key) - p, err := snowflake.ShowAccountParameter(db, key) - if err != nil { - return fmt.Errorf("error reading session parameter err = %w", err) + + var p *snowflake.Parameter + if onAccount { + p, err = snowflake.ShowAccountParameter(db, key) + } else { + p, err = snowflake.ShowSessionParameter(db, key, user) } + err = d.Set("value", p.Value.String) if err != nil { return fmt.Errorf("error setting session parameter err = %w", err) @@ -81,9 +109,21 @@ func CreateSessionParameter(d *schema.ResourceData, meta interface{}) error { func ReadSessionParameter(d *schema.ResourceData, meta interface{}) error { db := meta.(*sql.DB) key := d.Id() - p, err := snowflake.ShowAccountParameter(db, key) - if err != nil { - return fmt.Errorf("error reading session parameter err = %w", err) + + onAccount := d.Get("on_account").(bool) + var p *snowflake.Parameter + var err error + if onAccount { + p, err = snowflake.ShowAccountParameter(db, key) + if err != nil { + return fmt.Errorf("error reading session parameter err = %w", err) + } + } else { + user := d.Get("user").(string) + p, err = snowflake.ShowSessionParameter(db, key, user) + if err != nil { + return fmt.Errorf("error reading session parameter err = %w", err) + } } err = d.Set("value", p.Value.String) if err != nil { @@ -111,16 +151,18 @@ func DeleteSessionParameter(d *schema.ResourceData, meta interface{}) error { if reflect.TypeOf(parameterDefault.DefaultValue) == typeString { value = fmt.Sprintf("'%s'", value) } - builder := snowflake.NewParameter(key, value, snowflake.ParameterTypeSession, db) - err := builder.SetParameter() - if err != nil { - return fmt.Errorf("error creating account parameter err = %w", err) + builder := snowflake.NewSessionParameter(key, value, db) + onAccount := d.Get("on_account").(bool) + if onAccount { + builder.SetOnAccount(onAccount) + } else { + user := d.Get("user").(string) + builder.SetUser(user) } - _, err = snowflake.ShowAccountParameter(db, key) + err := builder.SetParameter() if err != nil { - return fmt.Errorf("error reading a parameter err = %w", err) + return fmt.Errorf("error resetting session parameter err = %w", err) } - d.SetId("") return nil } diff --git a/pkg/resources/session_parameter_acceptance_test.go b/pkg/resources/session_parameter_acceptance_test.go index 21067c4c88..3588fdae30 100644 --- a/pkg/resources/session_parameter_acceptance_test.go +++ b/pkg/resources/session_parameter_acceptance_test.go @@ -2,32 +2,71 @@ package resources_test import ( "fmt" + "strings" "testing" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" ) -func TestAcc_SessionParameter(t *testing.T) { +func TestAcc_SessionParameterWithUser(t *testing.T) { + prefix := "TEST_USER_" + strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) + resource.ParallelTest(t, resource.TestCase{ Providers: providers(), CheckDestroy: nil, Steps: []resource.TestStep{ { - Config: sessionParameterBasic("AUTOCOMMIT", "false"), + Config: sessionParameterWithUser(prefix, "BINARY_OUTPUT_FORMAT", "BASE64"), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_session_parameter.p", "key", "BINARY_OUTPUT_FORMAT"), + resource.TestCheckResourceAttr("snowflake_session_parameter.p", "value", "BASE64"), + resource.TestCheckResourceAttr("snowflake_session_parameter.p", "user", prefix), + resource.TestCheckResourceAttr("snowflake_session_parameter.p", "on_account", "false"), + ), + }, + }, + }) +} + +func TestAcc_SessionParameterOnAccount(t *testing.T) { + resource.ParallelTest(t, resource.TestCase{ + Providers: providers(), + CheckDestroy: nil, + Steps: []resource.TestStep{ + { + Config: sessionParameterOnAccount("AUTOCOMMIT", "false"), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("snowflake_session_parameter.p", "key", "AUTOCOMMIT"), resource.TestCheckResourceAttr("snowflake_session_parameter.p", "value", "false"), + resource.TestCheckResourceAttr("snowflake_session_parameter.p", "on_account", "true"), ), }, }, }) } -func sessionParameterBasic(key, value string) string { +func sessionParameterWithUser(user, key, value string) string { + s := ` +resource "snowflake_user" "u" { + name = "%s" +} + +resource "snowflake_session_parameter" "p" { + key = "%s" + value = "%s" + user = snowflake_user.u.name +} +` + return fmt.Sprintf(s, user, key, value) +} + +func sessionParameterOnAccount(key, value string) string { s := ` resource "snowflake_session_parameter" "p" { key = "%s" value = "%s" + on_account = true } ` return fmt.Sprintf(s, key, value) diff --git a/pkg/snowflake/parameters.go b/pkg/snowflake/parameters.go index 6813019adf..76df34bf3d 100644 --- a/pkg/snowflake/parameters.go +++ b/pkg/snowflake/parameters.go @@ -683,52 +683,119 @@ func GetParameterDefault(key string) ParameterDefault { return ParameterDefaults()[key] } -// ParameterBuilder abstracts the creation of SQL queries for Snowflake parameters. -type ParameterBuilder struct { +// AccountParameterBuilder abstracts the creation of SQL queries for Snowflake account parameters. +type AccountParameterBuilder struct { + key string + value string + db *sql.DB +} + +func NewAccountParameter(key, value string, db *sql.DB) *AccountParameterBuilder { + return &AccountParameterBuilder{ + key: key, + value: value, + db: db, + } +} + +func (v *AccountParameterBuilder) SetParameter() error { + stmt := fmt.Sprintf("ALTER ACCOUNT SET %s = %s", v.key, v.value) + _, err := v.db.Exec(stmt) + return err +} + +// SessionParameterBuilder abstracts the creation of SQL queries for Snowflake session parameters. +type SessionParameterBuilder struct { + key string + value string + onAccount bool + user string + db *sql.DB +} + +func NewSessionParameter(key, value string, db *sql.DB) *SessionParameterBuilder { + return &SessionParameterBuilder{ + key: key, + value: value, + db: db, + } +} + +func (v *SessionParameterBuilder) SetOnAccount(onAccount bool) *SessionParameterBuilder { + v.onAccount = onAccount + return v +} + +func (v *SessionParameterBuilder) SetUser(user string) *SessionParameterBuilder { + v.user = user + return v +} + +func (v *SessionParameterBuilder) SetParameter() error { + if v.onAccount { + // prepared statements do not work here for some reason. We already validate inputs so its okay + stmt := fmt.Sprintf("ALTER ACCOUNT SET %s = %s", v.key, v.value) + _, err := v.db.Exec(stmt) + return err + } + if v.user == "" { + return fmt.Errorf("user is required when setting session parameters on a user") + } + stmt := fmt.Sprintf("ALTER USER %s SET %s = %s", v.user, v.key, v.value) + _, err := v.db.Exec(stmt) + return err +} + +// ObjectParameterBuilder abstracts the creation of SQL queries for Snowflake object parameters. +type ObjectParameterBuilder struct { key string value string - parameterType ParameterType + onAccount bool objectType ObjectType objectIdentifier string db *sql.DB } -func NewParameter(key, value string, parameterType ParameterType, db *sql.DB) *ParameterBuilder { - return &ParameterBuilder{ - key: key, - value: value, - parameterType: parameterType, - db: db, +func NewObjectParameter(key, value string, db *sql.DB) *ObjectParameterBuilder { + return &ObjectParameterBuilder{ + key: key, + value: value, + db: db, } } -func (v *ParameterBuilder) WithObjectType(objectType ObjectType) *ParameterBuilder { +func (v *ObjectParameterBuilder) SetOnAccount(onAccount bool) *ObjectParameterBuilder { + v.onAccount = onAccount + return v +} + +func (v *ObjectParameterBuilder) WithObjectType(objectType ObjectType) *ObjectParameterBuilder { v.objectType = objectType return v } -func (v *ParameterBuilder) WithObjectIdentifier(objectIdentifier string) *ParameterBuilder { +func (v *ObjectParameterBuilder) WithObjectIdentifier(objectIdentifier string) *ObjectParameterBuilder { v.objectIdentifier = objectIdentifier return v } -func (v *ParameterBuilder) SetParameter() error { - // Should this be set on the account level? - setOnAccount := v.parameterType == ParameterTypeAccount || v.parameterType == ParameterTypeSession || v.parameterType == ParameterTypeObject && v.objectIdentifier == "" - if setOnAccount { +func (v *ObjectParameterBuilder) SetParameter() error { + if v.onAccount { // prepared statements do not work here for some reason. We already validate inputs so its okay stmt := fmt.Sprintf("ALTER ACCOUNT SET %s = %s", v.key, v.value) _, err := v.db.Exec(stmt) return err } - if v.parameterType == ParameterTypeObject { - stmt := fmt.Sprintf("ALTER %s %s SET %s = %s", v.objectType, v.objectIdentifier, v.key, v.value) - _, err := v.db.Exec(stmt) - if err != nil { - return err - } + if v.objectType == "" { + return fmt.Errorf("object type is required when setting object parameters") + } + if v.objectIdentifier == "" { + return fmt.Errorf("object identifier is required when setting object parameters") } - return nil + + stmt := fmt.Sprintf("ALTER %s %s SET %s = %s", v.objectType, v.objectIdentifier, v.key, v.value) + _, err := v.db.Exec(stmt) + return err } type Parameter struct { @@ -759,6 +826,24 @@ func ShowAccountParameter(db *sql.DB, key string) (*Parameter, error) { return ¶ms[0], nil } +func ShowSessionParameter(db *sql.DB, key string, user string) (*Parameter, error) { + stmt := fmt.Sprintf("SHOW PARAMETERS LIKE '%s' IN USER %s", key, user) + rows, err := db.Query(stmt) + if err != nil { + return nil, err + } + defer rows.Close() + params := []Parameter{} + if err := sqlx.StructScan(rows, ¶ms); err != nil { + if errors.Is(err, sql.ErrNoRows) { + return nil, nil + } + return nil, fmt.Errorf("unable to scan row for %s err = %w", stmt, err) + } + + return ¶ms[0], nil +} + func ShowObjectParameter(db *sql.DB, key string, objectType ObjectType, objectIdentifier string) (*Parameter, error) { var value Parameter stmt := fmt.Sprintf("SHOW PARAMETERS LIKE '%s' IN %s %s", key, objectType.String(), objectIdentifier) @@ -779,16 +864,35 @@ func ShowObjectParameter(db *sql.DB, key string, objectType ObjectType, objectId return &value, nil } -func ListParameters(db *sql.DB, parameterType ParameterType, pattern string) ([]Parameter, error) { +func ListAccountParameters(db *sql.DB, pattern string) ([]Parameter, error) { var stmt string - if parameterType == ParameterTypeAccount || parameterType == ParameterTypeSession { - if pattern != "" { - stmt = fmt.Sprintf("SHOW PARAMETERS LIKE '%s' IN %v", pattern, parameterType) - } else { - stmt = fmt.Sprintf("SHOW PARAMETERS IN %v", parameterType) + if pattern != "" { + stmt = fmt.Sprintf("SHOW PARAMETERS LIKE '%s' IN ACCOUNT", pattern) + } else { + stmt = "SHOW PARAMETERS IN ACCOUNT" + } + log.Printf("[DEBUG] query = %s", stmt) + rows, err := db.Query(stmt) + if err != nil { + return nil, err + } + defer rows.Close() + params := []Parameter{} + if err := sqlx.StructScan(rows, ¶ms); err != nil { + if errors.Is(err, sql.ErrNoRows) { + return nil, nil } + return nil, fmt.Errorf("unable to scan row for %s err = %w", stmt, err) + } + return params, nil +} + +func ListSessionParameters(db *sql.DB, pattern string, user string) ([]Parameter, error) { + var stmt string + if pattern != "" { + stmt = fmt.Sprintf("SHOW PARAMETERS LIKE '%s' FOR USER %s", pattern, user) } else { - return nil, fmt.Errorf("unsupported parameter type %s", parameterType) + stmt = fmt.Sprintf("SHOW PARAMETERS FOR USER %s", user) } log.Printf("[DEBUG] query = %s", stmt) rows, err := db.Query(stmt) From 3cff2eb3def90f33a840e78d8fd45c0223ce38b1 Mon Sep 17 00:00:00 2001 From: Scott Winkler Date: Fri, 31 Mar 2023 12:38:54 -0700 Subject: [PATCH 2/5] add on_account to session and object params --- pkg/resources/session_parameter.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/resources/session_parameter.go b/pkg/resources/session_parameter.go index 525291a36a..4c376cba23 100644 --- a/pkg/resources/session_parameter.go +++ b/pkg/resources/session_parameter.go @@ -97,7 +97,9 @@ func CreateSessionParameter(d *schema.ResourceData, meta interface{}) error { } else { p, err = snowflake.ShowSessionParameter(db, key, user) } - + if err != nil { + return fmt.Errorf("error reading session parameter err = %w", err) + } err = d.Set("value", p.Value.String) if err != nil { return fmt.Errorf("error setting session parameter err = %w", err) From a0b47aff7f01ef03a603f4a5762075d29d23dcd7 Mon Sep 17 00:00:00 2001 From: Scott Winkler Date: Fri, 31 Mar 2023 13:12:18 -0700 Subject: [PATCH 3/5] fix parameters data source acc tests --- docs/data-sources/parameters.md | 1 + .../snowflake_parameters/data-source.tf | 1 + pkg/datasources/parameters.go | 40 ++-------- pkg/datasources/parameters_acceptance_test.go | 80 ++++++++++++++++++- 4 files changed, 86 insertions(+), 36 deletions(-) diff --git a/docs/data-sources/parameters.md b/docs/data-sources/parameters.md index c2b0445ef2..25e998d07a 100644 --- a/docs/data-sources/parameters.md +++ b/docs/data-sources/parameters.md @@ -34,6 +34,7 @@ data "snowflake_parameters" "p2" { data "snowflake_parameters" "p3" { parameter_type = "SESSION" pattern = "ROWS_PER_RESULTSET" + user = "TEST_USER" } ``` diff --git a/examples/data-sources/snowflake_parameters/data-source.tf b/examples/data-sources/snowflake_parameters/data-source.tf index 72b0607e06..ad5c2df58a 100644 --- a/examples/data-sources/snowflake_parameters/data-source.tf +++ b/examples/data-sources/snowflake_parameters/data-source.tf @@ -19,4 +19,5 @@ data "snowflake_parameters" "p2" { data "snowflake_parameters" "p3" { parameter_type = "SESSION" pattern = "ROWS_PER_RESULTSET" + user = "TEST_USER" } diff --git a/pkg/datasources/parameters.go b/pkg/datasources/parameters.go index fd966ebba1..7634e827f2 100644 --- a/pkg/datasources/parameters.go +++ b/pkg/datasources/parameters.go @@ -16,7 +16,7 @@ var parametersSchema = map[string]*schema.Schema{ "parameter_type": { Type: schema.TypeString, Optional: true, - Default: "SESSION", + Default: "ACCOUNT", Description: "The type of parameter to filter by. Valid values are: \"ACCOUNT\", \"SESSION\", \"OBJECT\".", ValidateFunc: validation.StringInSlice([]string{"ACCOUNT", "SESSION", "OBJECT"}, true), }, @@ -96,38 +96,9 @@ func ReadParameters(d *schema.ResourceData, meta interface{}) error { if ok { pattern = p.(string) } - parameterType := snowflake.ParameterType(strings.ToUpper(d.Get("parameter_type").(string))) - if parameterType == snowflake.ParameterTypeObject { - oType := d.Get("object_type").(string) - objectType := snowflake.ObjectType(oType) - objectName := d.Get("object_name").(string) - parameters, err := snowflake.ListObjectParameters(db, objectType, objectName, pattern) - if errors.Is(err, sql.ErrNoRows) { - log.Printf("[DEBUG] parameters not found") - d.SetId("") - return nil - } else if err != nil { - log.Printf("[DEBUG] error occurred during read: %v", err.Error()) - return err - } - d.SetId("parameters") - params := []map[string]interface{}{} - for _, param := range parameters { - paramMap := map[string]interface{}{} - - paramMap["key"] = param.Key.String - paramMap["value"] = param.Value.String - paramMap["default"] = param.Default.String - paramMap["level"] = param.Level.String - paramMap["description"] = param.Description.String - paramMap["type"] = param.PType.String - - params = append(params, paramMap) - } - return d.Set("parameters", params) - } var parameters []snowflake.Parameter var err error + parameterType := snowflake.ParameterType(strings.ToUpper(d.Get("parameter_type").(string))) switch parameterType { case snowflake.ParameterTypeAccount: parameters, err = snowflake.ListAccountParameters(db, pattern) @@ -136,7 +107,12 @@ func ReadParameters(d *schema.ResourceData, meta interface{}) error { if user == "" { return fmt.Errorf("user is required when parameter_type is set to SESSION") } - parameters, err = snowflake.ListSessionParameters(db, user, pattern) + parameters, err = snowflake.ListSessionParameters(db, pattern, user) + case snowflake.ParameterTypeObject: + oType := d.Get("object_type").(string) + objectType := snowflake.ObjectType(oType) + objectName := d.Get("object_name").(string) + parameters, err = snowflake.ListObjectParameters(db, objectType, objectName, pattern) } if errors.Is(err, sql.ErrNoRows) { log.Printf("[DEBUG] parameters not found") diff --git a/pkg/datasources/parameters_acceptance_test.go b/pkg/datasources/parameters_acceptance_test.go index b4b68df8b4..70ae13fb39 100644 --- a/pkg/datasources/parameters_acceptance_test.go +++ b/pkg/datasources/parameters_acceptance_test.go @@ -1,28 +1,100 @@ package datasources_test import ( + "fmt" + "strings" "testing" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" ) -func TestAcc_Parameters(t *testing.T) { +func TestAcc_ParametersOnAccount(t *testing.T) { resource.ParallelTest(t, resource.TestCase{ Providers: providers(), CheckDestroy: nil, Steps: []resource.TestStep{ { - Config: parameters(), + Config: parametersConfigOnAccount(), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("data.snowflake_parameters.p", "pattern", "AUTOCOMMIT"), + resource.TestCheckResourceAttr("data.snowflake_parameters.p", "parameters.#", "1"), + resource.TestCheckResourceAttr("data.snowflake_parameters.p", "parameters.0.key", "AUTOCOMMIT"), + resource.TestCheckResourceAttrSet("data.snowflake_parameters.p", "parameters.0.value"), + ), + }, + }, + }) +} + +func TestAcc_ParametersOnSession(t *testing.T) { + userName := "TEST_USER_" + strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) + resource.ParallelTest(t, resource.TestCase{ + Providers: providers(), + CheckDestroy: nil, + Steps: []resource.TestStep{ + { + Config: parametersConfigOnSession(userName), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttrSet("data.snowflake_parameters.p", "parameters.#"), resource.TestCheckResourceAttrSet("data.snowflake_parameters.p", "parameters.0.key"), resource.TestCheckResourceAttrSet("data.snowflake_parameters.p", "parameters.0.value"), + resource.TestCheckResourceAttr("data.snowflake_parameters.p", "user", userName), ), }, }, }) } -func parameters() string { - return `data "snowflake_parameters" "p" {}` +func TestAcc_ParametersOnObject(t *testing.T) { + dbName := "TEST_DB_" + strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) + resource.ParallelTest(t, resource.TestCase{ + Providers: providers(), + CheckDestroy: nil, + Steps: []resource.TestStep{ + { + Config: parametersConfigOnObject(dbName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrSet("data.snowflake_parameters.p", "parameters.#"), + resource.TestCheckResourceAttrSet("data.snowflake_parameters.p", "parameters.0.key"), + resource.TestCheckResourceAttrSet("data.snowflake_parameters.p", "parameters.0.value"), + resource.TestCheckResourceAttr("data.snowflake_parameters.p", "object_type", "DATABASE"), + resource.TestCheckResourceAttr("data.snowflake_parameters.p", "object_name", dbName), + ), + }, + }, + }) +} + +func parametersConfigOnAccount() string { + return `data "snowflake_parameters" "p" { + parameter_type = "ACCOUNT" + pattern = "AUTOCOMMIT" + }` +} + +func parametersConfigOnSession(user string) string { + s := ` + resource "snowflake_user" "u" { + name = "%s" + } + + data "snowflake_parameters" "p" { + parameter_type = "SESSION" + user = snowflake_user.u.name + }` + return fmt.Sprintf(s, user) +} + +func parametersConfigOnObject(name string) string { + stmt := ` + resource "snowflake_database" "d" { + name = "%s" + } + data "snowflake_parameters" "p" { + parameter_type = "OBJECT" + object_type = "DATABASE" + object_name = snowflake_database.d.name + }` + return fmt.Sprintf(stmt, name) } From ae21f4dc832cb1f919b807bc91a39a28c356d6dc Mon Sep 17 00:00:00 2001 From: Scott Winkler Date: Fri, 31 Mar 2023 18:06:39 -0700 Subject: [PATCH 4/5] refactor parameters to remove code duplication --- pkg/snowflake/parameters.go | 168 ++++++++++++++++-------------------- 1 file changed, 73 insertions(+), 95 deletions(-) diff --git a/pkg/snowflake/parameters.go b/pkg/snowflake/parameters.go index 76df34bf3d..af42a1fed1 100644 --- a/pkg/snowflake/parameters.go +++ b/pkg/snowflake/parameters.go @@ -4,7 +4,6 @@ import ( "database/sql" "errors" "fmt" - "log" "reflect" "strconv" "time" @@ -683,25 +682,66 @@ func GetParameterDefault(key string) ParameterDefault { return ParameterDefaults()[key] } +type ParameterExecutor struct { + db *sql.DB +} + +func NewParameterExecutor(db *sql.DB) *ParameterExecutor { + return &ParameterExecutor{ + db: db, + } +} + +func (v *ParameterExecutor) Execute(stmt string, args ...interface{}) error { + _, err := v.db.Exec(stmt, args...) + return err +} + +func (v *ParameterExecutor) Query(stmt string) ([]Parameter, error) { + rows, err := v.db.Query(stmt) + if err != nil { + return nil, err + } + defer rows.Close() + params := []Parameter{} + if err := sqlx.StructScan(rows, ¶ms); err != nil { + if errors.Is(err, sql.ErrNoRows) { + return nil, nil + } + return nil, fmt.Errorf("unable to scan row for %s err = %w", stmt, err) + } + return params, nil +} + +func (v *ParameterExecutor) QueryOne(stmt string) (*Parameter, error) { + params, error := v.Query(stmt) + if error != nil { + return nil, error + } + if len(params) == 0 { + return nil, nil + } + return ¶ms[0], nil +} + // AccountParameterBuilder abstracts the creation of SQL queries for Snowflake account parameters. type AccountParameterBuilder struct { - key string - value string - db *sql.DB + key string + value string + executor *ParameterExecutor } func NewAccountParameter(key, value string, db *sql.DB) *AccountParameterBuilder { return &AccountParameterBuilder{ - key: key, - value: value, - db: db, + key: key, + value: value, + executor: NewParameterExecutor(db), } } func (v *AccountParameterBuilder) SetParameter() error { stmt := fmt.Sprintf("ALTER ACCOUNT SET %s = %s", v.key, v.value) - _, err := v.db.Exec(stmt) - return err + return v.executor.Execute(stmt) } // SessionParameterBuilder abstracts the creation of SQL queries for Snowflake session parameters. @@ -710,14 +750,14 @@ type SessionParameterBuilder struct { value string onAccount bool user string - db *sql.DB + executor *ParameterExecutor } func NewSessionParameter(key, value string, db *sql.DB) *SessionParameterBuilder { return &SessionParameterBuilder{ - key: key, - value: value, - db: db, + key: key, + value: value, + executor: NewParameterExecutor(db), } } @@ -733,17 +773,14 @@ func (v *SessionParameterBuilder) SetUser(user string) *SessionParameterBuilder func (v *SessionParameterBuilder) SetParameter() error { if v.onAccount { - // prepared statements do not work here for some reason. We already validate inputs so its okay stmt := fmt.Sprintf("ALTER ACCOUNT SET %s = %s", v.key, v.value) - _, err := v.db.Exec(stmt) - return err + return v.executor.Execute(stmt) } if v.user == "" { return fmt.Errorf("user is required when setting session parameters on a user") } stmt := fmt.Sprintf("ALTER USER %s SET %s = %s", v.user, v.key, v.value) - _, err := v.db.Exec(stmt) - return err + return v.executor.Execute(stmt) } // ObjectParameterBuilder abstracts the creation of SQL queries for Snowflake object parameters. @@ -753,14 +790,14 @@ type ObjectParameterBuilder struct { onAccount bool objectType ObjectType objectIdentifier string - db *sql.DB + executor *ParameterExecutor } func NewObjectParameter(key, value string, db *sql.DB) *ObjectParameterBuilder { return &ObjectParameterBuilder{ - key: key, - value: value, - db: db, + key: key, + value: value, + executor: NewParameterExecutor(db), } } @@ -781,10 +818,8 @@ func (v *ObjectParameterBuilder) WithObjectIdentifier(objectIdentifier string) * func (v *ObjectParameterBuilder) SetParameter() error { if v.onAccount { - // prepared statements do not work here for some reason. We already validate inputs so its okay stmt := fmt.Sprintf("ALTER ACCOUNT SET %s = %s", v.key, v.value) - _, err := v.db.Exec(stmt) - return err + return v.executor.Execute(stmt) } if v.objectType == "" { return fmt.Errorf("object type is required when setting object parameters") @@ -794,8 +829,7 @@ func (v *ObjectParameterBuilder) SetParameter() error { } stmt := fmt.Sprintf("ALTER %s %s SET %s = %s", v.objectType, v.objectIdentifier, v.key, v.value) - _, err := v.db.Exec(stmt) - return err + return v.executor.Execute(stmt) } type Parameter struct { @@ -809,20 +843,14 @@ type Parameter struct { func ShowAccountParameter(db *sql.DB, key string) (*Parameter, error) { stmt := fmt.Sprintf("SHOW PARAMETERS LIKE '%s' IN ACCOUNT", key) - - rows, err := db.Query(stmt) + executor := NewParameterExecutor(db) + params, err := executor.Query(stmt) if err != nil { return nil, err } - defer rows.Close() - params := []Parameter{} - if err := sqlx.StructScan(rows, ¶ms); err != nil { - if errors.Is(err, sql.ErrNoRows) { - return nil, nil - } - return nil, fmt.Errorf("unable to scan row for %s err = %w", stmt, err) + if len(params) == 0 { + return nil, nil } - return ¶ms[0], nil } @@ -845,23 +873,9 @@ func ShowSessionParameter(db *sql.DB, key string, user string) (*Parameter, erro } func ShowObjectParameter(db *sql.DB, key string, objectType ObjectType, objectIdentifier string) (*Parameter, error) { - var value Parameter stmt := fmt.Sprintf("SHOW PARAMETERS LIKE '%s' IN %s %s", key, objectType.String(), objectIdentifier) - rows, err := db.Query(stmt) - if err != nil { - return nil, err - } - defer rows.Close() - params := []Parameter{} - if err := sqlx.StructScan(rows, ¶ms); err != nil { - if errors.Is(err, sql.ErrNoRows) { - return nil, nil - } - return nil, fmt.Errorf("unable to scan row for %s err = %w", stmt, err) - } - value = params[0] - - return &value, nil + executor := NewParameterExecutor(db) + return executor.QueryOne(stmt) } func ListAccountParameters(db *sql.DB, pattern string) ([]Parameter, error) { @@ -871,20 +885,8 @@ func ListAccountParameters(db *sql.DB, pattern string) ([]Parameter, error) { } else { stmt = "SHOW PARAMETERS IN ACCOUNT" } - log.Printf("[DEBUG] query = %s", stmt) - rows, err := db.Query(stmt) - if err != nil { - return nil, err - } - defer rows.Close() - params := []Parameter{} - if err := sqlx.StructScan(rows, ¶ms); err != nil { - if errors.Is(err, sql.ErrNoRows) { - return nil, nil - } - return nil, fmt.Errorf("unable to scan row for %s err = %w", stmt, err) - } - return params, nil + executor := NewParameterExecutor(db) + return executor.Query(stmt) } func ListSessionParameters(db *sql.DB, pattern string, user string) ([]Parameter, error) { @@ -894,20 +896,8 @@ func ListSessionParameters(db *sql.DB, pattern string, user string) ([]Parameter } else { stmt = fmt.Sprintf("SHOW PARAMETERS FOR USER %s", user) } - log.Printf("[DEBUG] query = %s", stmt) - rows, err := db.Query(stmt) - if err != nil { - return nil, err - } - defer rows.Close() - params := []Parameter{} - if err := sqlx.StructScan(rows, ¶ms); err != nil { - if errors.Is(err, sql.ErrNoRows) { - return nil, nil - } - return nil, fmt.Errorf("unable to scan row for %s err = %w", stmt, err) - } - return params, nil + executor := NewParameterExecutor(db) + return executor.Query(stmt) } func ListObjectParameters(db *sql.DB, objectType ObjectType, objectIdentifier, pattern string) ([]Parameter, error) { @@ -917,18 +907,6 @@ func ListObjectParameters(db *sql.DB, objectType ObjectType, objectIdentifier, p } else { stmt = fmt.Sprintf("SHOW PARAMETERS IN %s %s", objectType.String(), objectIdentifier) } - log.Printf("[DEBUG] query = %s", stmt) - rows, err := db.Query(stmt) - if err != nil { - return nil, err - } - defer rows.Close() - params := []Parameter{} - if err := sqlx.StructScan(rows, ¶ms); err != nil { - if errors.Is(err, sql.ErrNoRows) { - return nil, nil - } - return nil, fmt.Errorf("unable to scan row for %s err = %w", stmt, err) - } - return params, nil + executor := NewParameterExecutor(db) + return executor.Query(stmt) } From 3449e6a0bd14e5ec0d928db9d1830c7db29d64a1 Mon Sep 17 00:00:00 2001 From: Scott Winkler Date: Fri, 31 Mar 2023 18:17:23 -0700 Subject: [PATCH 5/5] refactor parameters to remove code duplication --- pkg/snowflake/parameters.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/snowflake/parameters.go b/pkg/snowflake/parameters.go index af42a1fed1..1c36eb554b 100644 --- a/pkg/snowflake/parameters.go +++ b/pkg/snowflake/parameters.go @@ -714,9 +714,9 @@ func (v *ParameterExecutor) Query(stmt string) ([]Parameter, error) { } func (v *ParameterExecutor) QueryOne(stmt string) (*Parameter, error) { - params, error := v.Query(stmt) - if error != nil { - return nil, error + params, err := v.Query(stmt) + if err != nil { + return nil, err } if len(params) == 0 { return nil, nil