From 3e8b1cfc41ab31d0ee991dfc09591223230b480c Mon Sep 17 00:00:00 2001 From: Scott Winkler Date: Wed, 22 Dec 2021 15:17:16 -0800 Subject: [PATCH 1/2] fix tagging for db, external_table, schema --- pkg/resources/database.go | 3 +- pkg/resources/external_table.go | 70 +++++++++++++++++++++++++++++++++ pkg/resources/resource.go | 18 ++++++--- pkg/resources/tag.go | 5 --- pkg/snowflake/external_table.go | 44 +++++++++++++++++++++ pkg/snowflake/generic.go | 44 ++++++++++++++++----- 6 files changed, 162 insertions(+), 22 deletions(-) diff --git a/pkg/resources/database.go b/pkg/resources/database.go index aec76ebf8a..fae960ec44 100644 --- a/pkg/resources/database.go +++ b/pkg/resources/database.go @@ -44,7 +44,7 @@ var databaseSchema = map[string]*schema.Schema{ "tag": tagReferenceSchema, } -var databaseProperties = []string{"comment", "data_retention_time_in_days"} +var databaseProperties = []string{"comment", "data_retention_time_in_days", "tag"} // Database returns a pointer to the resource representing a database func Database() *schema.Resource { @@ -152,6 +152,7 @@ func ReadDatabase(d *schema.ResourceData, meta interface{}) error { } func UpdateDatabase(d *schema.ResourceData, meta interface{}) error { + log.Printf("[DEBUG] updating database %v", d.Id()) return UpdateResource("database", databaseProperties, databaseSchema, snowflake.Database, ReadDatabase)(d, meta) } diff --git a/pkg/resources/external_table.go b/pkg/resources/external_table.go index 4a98a8827e..075c100fb6 100644 --- a/pkg/resources/external_table.go +++ b/pkg/resources/external_table.go @@ -135,6 +135,7 @@ func ExternalTable() *schema.Resource { return &schema.Resource{ Create: CreateExternalTable, Read: ReadExternalTable, + Update: UpdateExternalTable, Delete: DeleteExternalTable, Schema: externalTableSchema, @@ -291,6 +292,75 @@ func ReadExternalTable(data *schema.ResourceData, meta interface{}) error { return nil } +// UpdateExternalTable implements schema.UpdateFunc +func UpdateExternalTable(data *schema.ResourceData, meta interface{}) error { + db := meta.(*sql.DB) + database := data.Get("database").(string) + dbSchema := data.Get("schema").(string) + name := data.Get("name").(string) + + // This type conversion is due to the test framework in the terraform-plugin-sdk having limited support + // for data types in the HCL2ValueFromConfigValue method. + columns := []map[string]string{} + for _, column := range data.Get("column").([]interface{}) { + columnDef := map[string]string{} + for key, val := range column.(map[string]interface{}) { + columnDef[key] = val.(string) + } + columns = append(columns, columnDef) + } + builder := snowflake.ExternalTable(name, database, dbSchema) + builder.WithColumns(columns) + builder.WithFileFormat(data.Get("file_format").(string)) + builder.WithLocation(data.Get("location").(string)) + + builder.WithAutoRefresh(data.Get("auto_refresh").(bool)) + builder.WithRefreshOnCreate(data.Get("refresh_on_create").(bool)) + builder.WithCopyGrants(data.Get("copy_grants").(bool)) + + // Set optionals + if v, ok := data.GetOk("partition_by"); ok { + partitionBys := expandStringList(v.([]interface{})) + builder.WithPartitionBys(partitionBys) + } + + if v, ok := data.GetOk("pattern"); ok { + builder.WithPattern(v.(string)) + } + + if v, ok := data.GetOk("aws_sns_topic"); ok { + builder.WithAwsSNSTopic(v.(string)) + } + + if v, ok := data.GetOk("comment"); ok { + builder.WithComment(v.(string)) + } + + if v, ok := data.GetOk("tag"); ok { + tags := getTags(v) + builder.WithTags(tags.toSnowflakeTagValues()) + } + + stmt := builder.Update() + err := snowflake.Exec(db, stmt) + if err != nil { + return errors.Wrapf(err, "error updating externalTable %v", name) + } + + externalTableID := &externalTableID{ + DatabaseName: database, + SchemaName: dbSchema, + ExternalTableName: name, + } + dataIDInput, err := externalTableID.String() + if err != nil { + return err + } + data.SetId(dataIDInput) + + return ReadExternalTable(data, meta) +} + // DeleteExternalTable implements schema.DeleteFunc func DeleteExternalTable(data *schema.ResourceData, meta interface{}) error { db := meta.(*sql.DB) diff --git a/pkg/resources/resource.go b/pkg/resources/resource.go index 23edbe95fb..6d31497019 100644 --- a/pkg/resources/resource.go +++ b/pkg/resources/resource.go @@ -2,6 +2,7 @@ package resources import ( "database/sql" + "log" "github.com/chanzuckerberg/terraform-provider-snowflake/pkg/snowflake" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" @@ -34,15 +35,13 @@ func CreateResource( case schema.TypeInt: valInt := val.(int) qb.SetInt(field, valInt) - case schema.TypeList: - tags, ok := val.([]snowflake.TagValue) - if !ok { - continue - } - qb.SetTags(tags) } } } + if v, ok := d.GetOk("tag"); ok { + tags := getTags(v) + qb.SetTags(tags.toSnowflakeTagValues()) + } err := snowflake.Exec(db, qb.Statement()) if err != nil { @@ -103,12 +102,19 @@ func UpdateResource( qb.SetInt(field, valInt) } } + if d.HasChange("tag") { + log.Printf("[DEBUG] updating tags") + v := d.Get("tag") + tags := getTags(v) + qb.SetTags(tags.toSnowflakeTagValues()) + } err := snowflake.Exec(db, qb.Statement()) if err != nil { return errors.Wrapf(err, "error altering %s", t) } } + log.Printf("[DEBUG] performing read") return read(d, meta) } } diff --git a/pkg/resources/tag.go b/pkg/resources/tag.go index cf28e65e46..f073737a2f 100644 --- a/pkg/resources/tag.go +++ b/pkg/resources/tag.go @@ -48,7 +48,6 @@ var tagReferenceSchema = &schema.Schema{ Type: schema.TypeList, Required: false, Optional: true, - ForceNew: true, MinItems: 0, Description: "Definitions of a tag to associate with the resource.", Elem: &schema.Resource{ @@ -56,27 +55,23 @@ var tagReferenceSchema = &schema.Schema{ "name": { Type: schema.TypeString, Required: true, - ForceNew: true, Description: "Tag name, e.g. department.", }, "value": { Type: schema.TypeString, Required: true, - ForceNew: true, Description: "Tag value, e.g. marketing_info.", }, "database": { Type: schema.TypeString, Required: false, Optional: true, - ForceNew: true, Description: "Name of the database that the tag was created in.", }, "schema": { Type: schema.TypeString, Required: false, Optional: true, - ForceNew: true, Description: "Name of the schema that the tag was created in.", }, }, diff --git a/pkg/snowflake/external_table.go b/pkg/snowflake/external_table.go index 64209434d0..eba7813233 100644 --- a/pkg/snowflake/external_table.go +++ b/pkg/snowflake/external_table.go @@ -158,6 +158,50 @@ func (tb *ExternalTableBuilder) Create() string { return q.String() } +// Update returns the SQL statement required to update an externalTable +func (tb *ExternalTableBuilder) Update() string { + q := strings.Builder{} + q.WriteString(fmt.Sprintf(`ALTER EXTERNAL TABLE %v`, tb.QualifiedName())) + + q.WriteString(fmt.Sprintf(` (`)) + columnDefinitions := []string{} + for _, columnDefinition := range tb.columns { + columnDefinitions = append(columnDefinitions, fmt.Sprintf(`"%v" %v AS %v`, EscapeString(columnDefinition["name"]), EscapeString(columnDefinition["type"]), columnDefinition["as"])) + } + q.WriteString(strings.Join(columnDefinitions, ", ")) + q.WriteString(fmt.Sprintf(`)`)) + + if len(tb.partitionBys) > 0 { + q.WriteString(` PARTITION BY ( `) + q.WriteString(EscapeString(strings.Join(tb.partitionBys, ", "))) + q.WriteString(` )`) + } + + q.WriteString(` WITH LOCATION = ` + EscapeString(tb.location)) + q.WriteString(fmt.Sprintf(` REFRESH_ON_CREATE = %t`, tb.refreshOnCreate)) + q.WriteString(fmt.Sprintf(` AUTO_REFRESH = %t`, tb.autoRefresh)) + + if tb.pattern != "" { + q.WriteString(fmt.Sprintf(` PATTERN = '%v'`, EscapeString(tb.pattern))) + } + + q.WriteString(fmt.Sprintf(` FILE_FORMAT = ( %v )`, EscapeString(tb.fileFormat))) + + if tb.awsSNSTopic != "" { + q.WriteString(fmt.Sprintf(` AWS_SNS_TOPIC = '%v'`, EscapeString(tb.awsSNSTopic))) + } + + if tb.copyGrants { + q.WriteString(" COPY GRANTS") + } + + if tb.comment != "" { + q.WriteString(fmt.Sprintf(` COMMENT = '%v'`, EscapeString(tb.comment))) + } + + return q.String() +} + // Drop returns the SQL query that will drop a externalTable. func (tb *ExternalTableBuilder) Drop() string { return fmt.Sprintf(`DROP EXTERNAL TABLE %v`, tb.QualifiedName()) diff --git a/pkg/snowflake/generic.go b/pkg/snowflake/generic.go index 86772e5b43..83507f2622 100644 --- a/pkg/snowflake/generic.go +++ b/pkg/snowflake/generic.go @@ -108,6 +108,21 @@ func (b *AlterPropertiesBuilder) SetTags(tags []TagValue) { b.tags = tags } +func (ab *AlterPropertiesBuilder) GetTagValueString() string { + var q strings.Builder + for _, v := range ab.tags { + fmt.Println(v) + if v.Schema != "" { + if v.Database != "" { + q.WriteString(fmt.Sprintf(`"%v".`, v.Database)) + } + q.WriteString(fmt.Sprintf(`"%v".`, v.Schema)) + } + q.WriteString(fmt.Sprintf(`"%v" = "%v", `, v.Name, v.Value)) + } + return strings.TrimSuffix(q.String(), ", ") +} + func (ab *AlterPropertiesBuilder) Statement() string { var sb strings.Builder sb.WriteString(fmt.Sprintf(`ALTER %s "%s" SET`, ab.entityType, ab.name)) // TODO handle error @@ -134,10 +149,9 @@ func (ab *AlterPropertiesBuilder) Statement() string { sb.WriteString(fmt.Sprintf(" %s=%.2f", strings.ToUpper(k), ab.floatProperties[k])) } - for _, t := range sortTags(ab.tags) { - sb.WriteString(fmt.Sprintf(` TAG "%v"."%v"."%v" = "%v"`, t.Database, t.Schema, t.Name, t.Value)) + if len(ab.tags) > 0 { + sb.WriteString(fmt.Sprintf(` TAG %s`, ab.GetTagValueString())) } - return sb.String() } @@ -195,6 +209,21 @@ func (b *CreateBuilder) SetTags(tags []TagValue) { b.tags = tags } +func (b *CreateBuilder) GetTagValueString() string { + var q strings.Builder + for _, v := range b.tags { + fmt.Println(v) + if v.Schema != "" { + if v.Database != "" { + q.WriteString(fmt.Sprintf(`"%v".`, v.Database)) + } + q.WriteString(fmt.Sprintf(`"%v".`, v.Schema)) + } + q.WriteString(fmt.Sprintf(`"%v" = "%v", `, v.Name, v.Value)) + } + return strings.TrimSuffix(q.String(), ", ") +} + func (b *CreateBuilder) Statement() string { var sb strings.Builder sb.WriteString(fmt.Sprintf(`CREATE %s "%s"`, b.entityType, b.name)) // TODO handle error @@ -221,13 +250,8 @@ func (b *CreateBuilder) Statement() string { sb.WriteString(fmt.Sprintf(" %s=%.2f", strings.ToUpper(k), b.floatProperties[k])) } - for i, t := range sortTags(b.tags) { - if i == 0 { - sb.WriteString(` TAG`) - } else if i < len(b.tags)-1 { - sb.WriteString(`, `) - } - sb.WriteString(fmt.Sprintf(`"%v"."%v"."%v" = "%v"`, t.Database, t.Schema, t.Name, t.Value)) + if len(b.tags) > 0 { + sb.WriteString(fmt.Sprintf(` WITH TAG (%s)`, b.GetTagValueString())) } return sb.String() From dffc3acef5597e8d31afb66fbb702cfb3bd48895 Mon Sep 17 00:00:00 2001 From: Scott Winkler Date: Thu, 23 Dec 2021 13:07:28 -0800 Subject: [PATCH 2/2] add test for external table --- pkg/resources/external_table.go | 38 +------------------ pkg/snowflake/external_table.go | 56 +++++++++++----------------- pkg/snowflake/external_table_test.go | 7 ++++ 3 files changed, 31 insertions(+), 70 deletions(-) diff --git a/pkg/resources/external_table.go b/pkg/resources/external_table.go index 075c100fb6..ac359370eb 100644 --- a/pkg/resources/external_table.go +++ b/pkg/resources/external_table.go @@ -299,44 +299,10 @@ func UpdateExternalTable(data *schema.ResourceData, meta interface{}) error { dbSchema := data.Get("schema").(string) name := data.Get("name").(string) - // This type conversion is due to the test framework in the terraform-plugin-sdk having limited support - // for data types in the HCL2ValueFromConfigValue method. - columns := []map[string]string{} - for _, column := range data.Get("column").([]interface{}) { - columnDef := map[string]string{} - for key, val := range column.(map[string]interface{}) { - columnDef[key] = val.(string) - } - columns = append(columns, columnDef) - } builder := snowflake.ExternalTable(name, database, dbSchema) - builder.WithColumns(columns) - builder.WithFileFormat(data.Get("file_format").(string)) - builder.WithLocation(data.Get("location").(string)) - - builder.WithAutoRefresh(data.Get("auto_refresh").(bool)) - builder.WithRefreshOnCreate(data.Get("refresh_on_create").(bool)) - builder.WithCopyGrants(data.Get("copy_grants").(bool)) - - // Set optionals - if v, ok := data.GetOk("partition_by"); ok { - partitionBys := expandStringList(v.([]interface{})) - builder.WithPartitionBys(partitionBys) - } - - if v, ok := data.GetOk("pattern"); ok { - builder.WithPattern(v.(string)) - } - if v, ok := data.GetOk("aws_sns_topic"); ok { - builder.WithAwsSNSTopic(v.(string)) - } - - if v, ok := data.GetOk("comment"); ok { - builder.WithComment(v.(string)) - } - - if v, ok := data.GetOk("tag"); ok { + if data.HasChange("tag") { + v := data.Get("tag") tags := getTags(v) builder.WithTags(tags.toSnowflakeTagValues()) } diff --git a/pkg/snowflake/external_table.go b/pkg/snowflake/external_table.go index eba7813233..efebcdad75 100644 --- a/pkg/snowflake/external_table.go +++ b/pkg/snowflake/external_table.go @@ -155,6 +155,10 @@ func (tb *ExternalTableBuilder) Create() string { q.WriteString(fmt.Sprintf(` COMMENT = '%v'`, EscapeString(tb.comment))) } + if len(tb.tags) > 0 { + q.WriteString(fmt.Sprintf(` WITH TAG (%s)`, tb.GetTagValueString())) + } + return q.String() } @@ -163,40 +167,8 @@ func (tb *ExternalTableBuilder) Update() string { q := strings.Builder{} q.WriteString(fmt.Sprintf(`ALTER EXTERNAL TABLE %v`, tb.QualifiedName())) - q.WriteString(fmt.Sprintf(` (`)) - columnDefinitions := []string{} - for _, columnDefinition := range tb.columns { - columnDefinitions = append(columnDefinitions, fmt.Sprintf(`"%v" %v AS %v`, EscapeString(columnDefinition["name"]), EscapeString(columnDefinition["type"]), columnDefinition["as"])) - } - q.WriteString(strings.Join(columnDefinitions, ", ")) - q.WriteString(fmt.Sprintf(`)`)) - - if len(tb.partitionBys) > 0 { - q.WriteString(` PARTITION BY ( `) - q.WriteString(EscapeString(strings.Join(tb.partitionBys, ", "))) - q.WriteString(` )`) - } - - q.WriteString(` WITH LOCATION = ` + EscapeString(tb.location)) - q.WriteString(fmt.Sprintf(` REFRESH_ON_CREATE = %t`, tb.refreshOnCreate)) - q.WriteString(fmt.Sprintf(` AUTO_REFRESH = %t`, tb.autoRefresh)) - - if tb.pattern != "" { - q.WriteString(fmt.Sprintf(` PATTERN = '%v'`, EscapeString(tb.pattern))) - } - - q.WriteString(fmt.Sprintf(` FILE_FORMAT = ( %v )`, EscapeString(tb.fileFormat))) - - if tb.awsSNSTopic != "" { - q.WriteString(fmt.Sprintf(` AWS_SNS_TOPIC = '%v'`, EscapeString(tb.awsSNSTopic))) - } - - if tb.copyGrants { - q.WriteString(" COPY GRANTS") - } - - if tb.comment != "" { - q.WriteString(fmt.Sprintf(` COMMENT = '%v'`, EscapeString(tb.comment))) + if len(tb.tags) > 0 { + q.WriteString(fmt.Sprintf(` TAG %s`, tb.GetTagValueString())) } return q.String() @@ -212,6 +184,22 @@ func (tb *ExternalTableBuilder) Show() string { return fmt.Sprintf(`SHOW EXTERNAL TABLES LIKE '%v' IN SCHEMA "%v"."%v"`, tb.name, tb.db, tb.schema) } +func (tb *ExternalTableBuilder) GetTagValueString() string { + var q strings.Builder + for _, v := range tb.tags { + fmt.Println(v) + if v.Schema != "" { + if v.Database != "" { + q.WriteString(fmt.Sprintf(`"%v".`, v.Database)) + } + q.WriteString(fmt.Sprintf(`"%v".`, v.Schema)) + } + q.WriteString(fmt.Sprintf(`"%v" = "%v", `, v.Name, v.Value)) + } + return strings.TrimSuffix(q.String(), ", ") +} + + type externalTable struct { CreatedOn sql.NullString `db:"created_on"` ExternalTableName sql.NullString `db:"name"` diff --git a/pkg/snowflake/external_table_test.go b/pkg/snowflake/external_table_test.go index 4e220904e8..f4d6e02b8f 100644 --- a/pkg/snowflake/external_table_test.go +++ b/pkg/snowflake/external_table_test.go @@ -21,6 +21,13 @@ func TestExternalTableCreate(t *testing.T) { r.Equal(s.Create(), `CREATE EXTERNAL TABLE "test_db"."test_schema"."test_table" ("column1" OBJECT AS expression1, "column2" VARCHAR AS expression2) WITH LOCATION = location REFRESH_ON_CREATE = false AUTO_REFRESH = false PATTERN = 'pattern' FILE_FORMAT = ( file format ) COMMENT = 'Test Comment'`) } +func TestExternalTableUpdate(t *testing.T){ + r := require.New(t) + s := ExternalTable("test_table", "test_db", "test_schema") + s.WithTags([]TagValue{{Name: "tag1", Value: "value1", Schema: "test_schema", Database: "test_db"}}) + r.Equal(s.Update(), `ALTER EXTERNAL TABLE "test_db"."test_schema"."test_table" TAG "test_db"."test_schema"."tag1" = "value1"`) +} + func TestExternalTableDrop(t *testing.T) { r := require.New(t) s := ExternalTable("test_table", "test_db", "test_schema")