From 79bcf81ad07d7b2284ce0c6fdd686e3a02b18e23 Mon Sep 17 00:00:00 2001 From: Abdullah Tariq Date: Sat, 1 Aug 2020 15:48:52 +0200 Subject: [PATCH 01/13] Add with_grant_option to the grants --- pkg/resources/account_grant.go | 13 ++++++ pkg/resources/account_grant_test.go | 30 +++++++------ pkg/resources/database_grant.go | 13 ++++++ pkg/resources/database_grant_test.go | 28 ++++++------ pkg/resources/grant_helpers.go | 23 +++++++--- pkg/resources/grant_helpers_internal_test.go | 23 ++++++---- pkg/resources/integration_grant.go | 13 ++++++ pkg/resources/integration_grant_test.go | 20 +++++---- pkg/resources/resource_monitor_grant.go | 13 ++++++ pkg/resources/resource_monitor_grant_test.go | 20 +++++---- pkg/resources/schema_grant.go | 13 ++++++ pkg/resources/schema_grant_test.go | 32 +++++++------- pkg/resources/share.go | 2 +- pkg/resources/stage_grant.go | 13 ++++++ pkg/resources/stage_grant_test.go | 21 ++++----- pkg/resources/table_grant.go | 14 ++++++ pkg/resources/table_grant_test.go | 45 +++++++++++--------- pkg/resources/view_grant.go | 13 ++++++ pkg/resources/view_grant_test.go | 45 +++++++++++--------- pkg/resources/warehouse_grant.go | 13 ++++++ pkg/snowflake/future_grant.go | 10 ++++- pkg/snowflake/future_grant_test.go | 10 ++--- pkg/snowflake/grant.go | 6 ++- pkg/snowflake/grant_test.go | 30 ++++++------- 24 files changed, 312 insertions(+), 151 deletions(-) diff --git a/pkg/resources/account_grant.go b/pkg/resources/account_grant.go index 8a809129ed..3a055695ee 100644 --- a/pkg/resources/account_grant.go +++ b/pkg/resources/account_grant.go @@ -35,6 +35,13 @@ var accountGrantSchema = map[string]*schema.Schema{ Description: "Grants privilege to these roles.", ForceNew: true, }, + "with_grant_option": { + Type: schema.TypeBool, + Optional: true, + Description: "When this is set to true, allows the recipient role to grant the privileges to other roles.", + Default: false, + ForceNew: true, + }, } // ViewGrant returns a pointer to the resource representing a view grant @@ -51,6 +58,7 @@ func AccountGrant() *schema.Resource { // CreateAccountGrant implements schema.CreateFunc func CreateAccountGrant(data *schema.ResourceData, meta interface{}) error { priv := data.Get("privilege").(string) + grantOption := data.Get("with_grant_option").(bool) builder := snowflake.AccountGrant() @@ -62,6 +70,7 @@ func CreateAccountGrant(data *schema.ResourceData, meta interface{}) error { grantID := &grantID{ ResourceName: "ACCOUNT", Privilege: priv, + GrantOption: grantOption, } dataIDInput, err := grantID.String() if err != nil { @@ -82,6 +91,10 @@ func ReadAccountGrant(data *schema.ResourceData, meta interface{}) error { if err != nil { return err } + err = data.Set("with_grant_option", grantID.GrantOption) + if err != nil { + return err + } builder := snowflake.AccountGrant() diff --git a/pkg/resources/account_grant_test.go b/pkg/resources/account_grant_test.go index 86151dea45..bae4f57009 100644 --- a/pkg/resources/account_grant_test.go +++ b/pkg/resources/account_grant_test.go @@ -25,15 +25,16 @@ func TestAccountGrantCreate(t *testing.T) { r := require.New(t) in := map[string]interface{}{ - "privilege": "CREATE DATABASE", - "roles": []interface{}{"test-role-1", "test-role-2"}, + "privilege": "CREATE DATABASE", + "roles": []interface{}{"test-role-1", "test-role-2"}, + "with_grant_option": true, } d := schema.TestResourceDataRaw(t, resources.AccountGrant().Schema, in) r.NotNil(d) WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) { - mock.ExpectExec(`^GRANT CREATE DATABASE ON ACCOUNT TO ROLE "test-role-1"$`).WillReturnResult(sqlmock.NewResult(1, 1)) - mock.ExpectExec(`^GRANT CREATE DATABASE ON ACCOUNT TO ROLE "test-role-2"$`).WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec(`^GRANT CREATE DATABASE ON ACCOUNT TO ROLE "test-role-1" WITH GRANT OPTION$`).WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec(`^GRANT CREATE DATABASE ON ACCOUNT TO ROLE "test-role-2" WITH GRANT OPTION$`).WillReturnResult(sqlmock.NewResult(1, 1)) expectReadAccountGrant(mock) err := resources.CreateAccountGrant(d, db) r.NoError(err) @@ -43,9 +44,10 @@ func TestAccountGrantCreate(t *testing.T) { func TestAccountGrantRead(t *testing.T) { r := require.New(t) - d := accountGrant(t, "ACCOUNT|||MANAGE GRANTS", map[string]interface{}{ - "privilege": "MANAGE GRANTS", - "roles": []interface{}{"test-role-1", "test-role-2"}, + d := accountGrant(t, "ACCOUNT|||MANAGE GRANTS|true", map[string]interface{}{ + "privilege": "MANAGE GRANTS", + "roles": []interface{}{"test-role-1", "test-role-2"}, + "with_grant_option": true, }) r.NotNil(d) @@ -60,9 +62,10 @@ func TestAccountGrantRead(t *testing.T) { func TestMonitorExecution(t *testing.T) { r := require.New(t) - d := accountGrant(t, "ACCOUNT|||MONITOR EXECUTION", map[string]interface{}{ - "privilege": "MONITOR EXECUTION", - "roles": []interface{}{"test-role-1", "test-role-2"}, + d := accountGrant(t, "ACCOUNT|||MONITOR EXECUTION|true", map[string]interface{}{ + "privilege": "MONITOR EXECUTION", + "roles": []interface{}{"test-role-1", "test-role-2"}, + "with_grant_option": true, }) r.NotNil(d) @@ -77,9 +80,10 @@ func TestMonitorExecution(t *testing.T) { func TestExecuteTask(t *testing.T) { r := require.New(t) - d := accountGrant(t, "ACCOUNT|||EXECUTE TASK", map[string]interface{}{ - "privilege": "EXECUTE TASK", - "roles": []interface{}{"test-role-1", "test-role-2"}, + d := accountGrant(t, "ACCOUNT|||EXECUTE TASK|false", map[string]interface{}{ + "privilege": "EXECUTE TASK", + "roles": []interface{}{"test-role-1", "test-role-2"}, + "with_grant_option": false, }) r.NotNil(d) diff --git a/pkg/resources/database_grant.go b/pkg/resources/database_grant.go index 202ed8736f..3cbebbc943 100644 --- a/pkg/resources/database_grant.go +++ b/pkg/resources/database_grant.go @@ -46,6 +46,13 @@ var databaseGrantSchema = map[string]*schema.Schema{ Description: "Grants privilege to these shares.", ForceNew: true, }, + "with_grant_option": { + Type: schema.TypeBool, + Optional: true, + Description: "When this is set to true, allows the recipient role to grant the privileges to other roles.", + Default: false, + ForceNew: true, + }, } // DatabaseGrant returns a pointer to the resource representing a database grant @@ -67,6 +74,7 @@ func CreateDatabaseGrant(data *schema.ResourceData, meta interface{}) error { dbName := data.Get("database_name").(string) builder := snowflake.DatabaseGrant(dbName) priv := data.Get("privilege").(string) + grantOption := data.Get("with_grant_option").(bool) err := createGenericGrant(data, meta, builder) if err != nil { @@ -76,6 +84,7 @@ func CreateDatabaseGrant(data *schema.ResourceData, meta interface{}) error { grant := &grantID{ ResourceName: dbName, Privilege: priv, + GrantOption: grantOption, } dataIDInput, err := grant.String() if err != nil { @@ -100,6 +109,10 @@ func ReadDatabaseGrant(data *schema.ResourceData, meta interface{}) error { if err != nil { return err } + err = data.Set("with_grant_option", grantID.GrantOption) + if err != nil { + return err + } // IMPORTED PRIVILEGES is not a real resource, so we can't actually verify // that it is still there. Just exit for now diff --git a/pkg/resources/database_grant_test.go b/pkg/resources/database_grant_test.go index 7fe29c4bd3..6c0d50e68a 100644 --- a/pkg/resources/database_grant_test.go +++ b/pkg/resources/database_grant_test.go @@ -25,19 +25,20 @@ func TestDatabaseGrantCreate(t *testing.T) { r := require.New(t) in := map[string]interface{}{ - "database_name": "test-database", - "privilege": "USAGE", - "roles": []interface{}{"test-role-1", "test-role-2"}, - "shares": []interface{}{"test-share-1", "test-share-2"}, + "database_name": "test-database", + "privilege": "USAGE", + "roles": []interface{}{"test-role-1", "test-role-2"}, + "shares": []interface{}{"test-share-1", "test-share-2"}, + "with_grant_option": true, } d := schema.TestResourceDataRaw(t, resources.DatabaseGrant().Schema, in) r.NotNil(d) WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) { - mock.ExpectExec(`^GRANT USAGE ON DATABASE "test-database" TO ROLE "test-role-1"$`).WillReturnResult(sqlmock.NewResult(1, 1)) - mock.ExpectExec(`^GRANT USAGE ON DATABASE "test-database" TO ROLE "test-role-2"$`).WillReturnResult(sqlmock.NewResult(1, 1)) - mock.ExpectExec(`^GRANT USAGE ON DATABASE "test-database" TO SHARE "test-share-1"$`).WillReturnResult(sqlmock.NewResult(1, 1)) - mock.ExpectExec(`^GRANT USAGE ON DATABASE "test-database" TO SHARE "test-share-2"$`).WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec(`^GRANT USAGE ON DATABASE "test-database" TO ROLE "test-role-1" WITH GRANT OPTION$`).WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec(`^GRANT USAGE ON DATABASE "test-database" TO ROLE "test-role-2" WITH GRANT OPTION$`).WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec(`^GRANT USAGE ON DATABASE "test-database" TO SHARE "test-share-1" WITH GRANT OPTION$`).WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec(`^GRANT USAGE ON DATABASE "test-database" TO SHARE "test-share-2" WITH GRANT OPTION$`).WillReturnResult(sqlmock.NewResult(1, 1)) expectReadDatabaseGrant(mock) err := resources.CreateDatabaseGrant(d, db) r.NoError(err) @@ -47,11 +48,12 @@ func TestDatabaseGrantCreate(t *testing.T) { func TestDatabaseGrantRead(t *testing.T) { r := require.New(t) - d := databaseGrant(t, "test-database|||IMPORTED PRIVILIGES", map[string]interface{}{ - "database_name": "test-database", - "privilege": "IMPORTED PRIVILIGES", - "roles": []interface{}{"test-role-1", "test-role-2"}, - "shares": []interface{}{"test-share-1", "test-share-2"}, + d := databaseGrant(t, "test-database|||IMPORTED PRIVILIGES|false", map[string]interface{}{ + "database_name": "test-database", + "privilege": "IMPORTED PRIVILIGES", + "roles": []interface{}{"test-role-1", "test-role-2"}, + "shares": []interface{}{"test-share-1", "test-share-2"}, + "with_grant_option": false, }) r.NotNil(d) diff --git a/pkg/resources/grant_helpers.go b/pkg/resources/grant_helpers.go index 7624980734..7dcddc4408 100644 --- a/pkg/resources/grant_helpers.go +++ b/pkg/resources/grant_helpers.go @@ -61,6 +61,7 @@ type grantID struct { SchemaName string ObjectName string Privilege string + GrantOption bool } // Because none of the grants currently have a privilege of "ALL", rather they explicitly say @@ -80,6 +81,7 @@ func filterALLGrants(grantList []*grant, validPrivs privilegeSet) []*grant { GrantName: g.GrantName, GranteeType: g.GranteeType, GranteeName: g.GranteeName, + GrantOption: g.GrantOption, } if _, ok := groupedByRole[id]; !ok { groupedByRole[id] = privilegeSet{} @@ -108,6 +110,7 @@ func filterALLGrants(grantList []*grant, validPrivs privilegeSet) []*grant { GrantName: g.GrantName, GranteeType: g.GranteeType, GranteeName: g.GranteeName, + GrantOption: g.GrantOption, } // Already added it with the "ALL" privilege, so skip if _, ok := groupedByRole[id]; ok { @@ -119,12 +122,13 @@ func filterALLGrants(grantList []*grant, validPrivs privilegeSet) []*grant { } // String() takes in a grantID object and returns a pipe-delimited string: -// resourceName|schemaName|ObjectName|Privilege +// resourceName|schemaName|ObjectName|Privilege|GrantOption func (gi *grantID) String() (string, error) { var buf bytes.Buffer csvWriter := csv.NewWriter(&buf) csvWriter.Comma = grantIDDelimiter - dataIdentifiers := [][]string{{gi.ResourceName, gi.SchemaName, gi.ObjectName, gi.Privilege}} + grantOption := fmt.Sprintf("%v", gi.GrantOption) + dataIdentifiers := [][]string{{gi.ResourceName, gi.SchemaName, gi.ObjectName, gi.Privilege, grantOption}} err := csvWriter.WriteAll(dataIdentifiers) if err != nil { return "", err @@ -146,8 +150,8 @@ func grantIDFromString(stringID string) (*grantID, error) { if len(lines) != 1 { return nil, fmt.Errorf("1 line per grant") } - if len(lines[0]) != 4 { - return nil, fmt.Errorf("4 fields allowed") + if len(lines[0]) != 5 { + return nil, fmt.Errorf("5 fields allowed") } grantResult := &grantID{ @@ -155,6 +159,7 @@ func grantIDFromString(stringID string) (*grantID, error) { SchemaName: lines[0][1], ObjectName: lines[0][2], Privilege: lines[0][3], + GrantOption: lines[0][4] == "true", } return grantResult, nil } @@ -163,6 +168,7 @@ func createGenericGrant(data *schema.ResourceData, meta interface{}, builder sno db := meta.(*sql.DB) priv := data.Get("privilege").(string) + grantOption := data.Get("with_grant_option").(bool) roles, shares := expandRolesAndShares(data) @@ -171,14 +177,14 @@ func createGenericGrant(data *schema.ResourceData, meta interface{}, builder sno } for _, role := range roles { - err := snowflake.Exec(db, builder.Role(role).Grant(priv)) + err := snowflake.Exec(db, builder.Role(role).Grant(priv, grantOption)) if err != nil { return err } } for _, share := range shares { - err := snowflake.Exec(db, builder.Share(share).Grant(priv)) + err := snowflake.Exec(db, builder.Share(share).Grant(priv, grantOption)) if err != nil { return err } @@ -200,6 +206,7 @@ func readGenericGrant(data *schema.ResourceData, meta interface{}, builder snowf return err } priv := data.Get("privilege").(string) + grantOption := data.Get("with_grant_option").(bool) // We re-aggregate grants that would be equivalent to the "ALL" grant grants = filterALLGrants(grants, validPrivileges) @@ -264,6 +271,10 @@ func readGenericGrant(data *schema.ResourceData, meta interface{}, builder snowf return err } } + err = data.Set("with_grant_option", grantOption) + if err != nil { + return err + } return nil } diff --git a/pkg/resources/grant_helpers_internal_test.go b/pkg/resources/grant_helpers_internal_test.go index e48684373d..22441d9453 100644 --- a/pkg/resources/grant_helpers_internal_test.go +++ b/pkg/resources/grant_helpers_internal_test.go @@ -10,7 +10,7 @@ import ( func TestGrantIDFromString(t *testing.T) { r := require.New(t) // Vanilla - id := "database_name|schema|view_name|privilege" + id := "database_name|schema|view_name|privilege|true" grant, err := grantIDFromString(id) r.NoError(err) @@ -18,30 +18,32 @@ func TestGrantIDFromString(t *testing.T) { r.Equal("schema", grant.SchemaName) r.Equal("view_name", grant.ObjectName) r.Equal("privilege", grant.Privilege) + r.Equal(true, grant.GrantOption) // No view - id = "database_name|||privilege" + id = "database_name|||privilege|" grant, err = grantIDFromString(id) r.NoError(err) r.Equal("database_name", grant.ResourceName) r.Equal("", grant.SchemaName) r.Equal("", grant.ObjectName) r.Equal("privilege", grant.Privilege) + r.Equal(false, grant.GrantOption) // Bad ID -- not enough fields id = "database|name-privilege" _, err = grantIDFromString(id) - r.Equal(fmt.Errorf("4 fields allowed"), err) + r.Equal(fmt.Errorf("5 fields allowed"), err) // Bad ID -- privilege in wrong area - id = "database||||name-privilege" + id = "database|||name-privilege" _, err = grantIDFromString(id) - r.Equal(fmt.Errorf("4 fields allowed"), err) + r.Equal(fmt.Errorf("5 fields allowed"), err) // too many fields - id = "database_name|schema|view_name|privilege|extra" + id = "database_name|schema|view_name|privilege|false|2" _, err = grantIDFromString(id) - r.Equal(fmt.Errorf("4 fields allowed"), err) + r.Equal(fmt.Errorf("5 fields allowed"), err) // 0 lines id = "" @@ -64,16 +66,17 @@ func TestGrantStruct(t *testing.T) { SchemaName: "schema", ObjectName: "view_name", Privilege: "priv", + GrantOption: true, } gID, err := grant.String() r.NoError(err) - r.Equal("database_name|schema|view_name|priv", gID) + r.Equal("database_name|schema|view_name|priv|true", gID) // Empty grant grant = &grantID{} gID, err = grant.String() r.NoError(err) - r.Equal("|||", gID) + r.Equal("||||false", gID) // Grant with extra delimiters grant = &grantID{ @@ -81,6 +84,7 @@ func TestGrantStruct(t *testing.T) { SchemaName: "schema|name", ObjectName: "view|name", Privilege: "priv", + GrantOption: false, } gID, err = grant.String() r.NoError(err) @@ -90,4 +94,5 @@ func TestGrantStruct(t *testing.T) { r.Equal("schema|name", newGrant.SchemaName) r.Equal("view|name", newGrant.ObjectName) r.Equal("priv", newGrant.Privilege) + r.Equal(false, newGrant.GrantOption) } diff --git a/pkg/resources/integration_grant.go b/pkg/resources/integration_grant.go index a58feb231a..06d0c04a5b 100644 --- a/pkg/resources/integration_grant.go +++ b/pkg/resources/integration_grant.go @@ -33,6 +33,13 @@ var integrationGrantSchema = map[string]*schema.Schema{ Description: "Grants privilege to these roles.", ForceNew: true, }, + "with_grant_option": { + Type: schema.TypeBool, + Optional: true, + Description: "When this is set to true, allows the recipient role to grant the privileges to other roles.", + Default: false, + ForceNew: true, + }, } // IntegrationGrant returns a pointer to the resource representing a integration grant @@ -50,6 +57,7 @@ func IntegrationGrant() *schema.Resource { func CreateIntegrationGrant(data *schema.ResourceData, meta interface{}) error { w := data.Get("integration_name").(string) priv := data.Get("privilege").(string) + grantOption := data.Get("with_grant_option").(bool) builder := snowflake.IntegrationGrant(w) err := createGenericGrant(data, meta, builder) @@ -60,6 +68,7 @@ func CreateIntegrationGrant(data *schema.ResourceData, meta interface{}) error { grant := &grantID{ ResourceName: w, Privilege: priv, + GrantOption: grantOption, } dataIDInput, err := grant.String() if err != nil { @@ -87,6 +96,10 @@ func ReadIntegrationGrant(data *schema.ResourceData, meta interface{}) error { if err != nil { return err } + err = data.Set("with_grant_option", grantID.GrantOption) + if err != nil { + return err + } builder := snowflake.IntegrationGrant(w) diff --git a/pkg/resources/integration_grant_test.go b/pkg/resources/integration_grant_test.go index 0af5e2f663..18d80cd707 100644 --- a/pkg/resources/integration_grant_test.go +++ b/pkg/resources/integration_grant_test.go @@ -25,16 +25,17 @@ func TestIntegrationGrantCreate(t *testing.T) { r := require.New(t) in := map[string]interface{}{ - "integration_name": "test-integration", - "privilege": "USAGE", - "roles": []interface{}{"test-role-1", "test-role-2"}, + "integration_name": "test-integration", + "privilege": "USAGE", + "roles": []interface{}{"test-role-1", "test-role-2"}, + "with_grant_option": true, } d := schema.TestResourceDataRaw(t, resources.IntegrationGrant().Schema, in) r.NotNil(d) WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) { - mock.ExpectExec(`^GRANT USAGE ON INTEGRATION "test-integration" TO ROLE "test-role-1"$`).WillReturnResult(sqlmock.NewResult(1, 1)) - mock.ExpectExec(`^GRANT USAGE ON INTEGRATION "test-integration" TO ROLE "test-role-2"$`).WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec(`^GRANT USAGE ON INTEGRATION "test-integration" TO ROLE "test-role-1" WITH GRANT OPTION$`).WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec(`^GRANT USAGE ON INTEGRATION "test-integration" TO ROLE "test-role-2" WITH GRANT OPTION$`).WillReturnResult(sqlmock.NewResult(1, 1)) expectReadIntegrationGrant(mock) err := resources.CreateIntegrationGrant(d, db) r.NoError(err) @@ -44,10 +45,11 @@ func TestIntegrationGrantCreate(t *testing.T) { func TestIntegrationGrantRead(t *testing.T) { r := require.New(t) - d := integrationGrant(t, "test-integration|||IMPORTED PRIVILIGES", map[string]interface{}{ - "integration_name": "test-integration", - "privilege": "IMPORTED PRIVILIGES", - "roles": []interface{}{"test-role-1", "test-role-2"}, + d := integrationGrant(t, "test-integration|||IMPORTED PRIVILIGES|false", map[string]interface{}{ + "integration_name": "test-integration", + "privilege": "IMPORTED PRIVILIGES", + "roles": []interface{}{"test-role-1", "test-role-2"}, + "with_grant_option": false, }) r.NotNil(d) diff --git a/pkg/resources/resource_monitor_grant.go b/pkg/resources/resource_monitor_grant.go index f244a63d33..f91d795dcc 100644 --- a/pkg/resources/resource_monitor_grant.go +++ b/pkg/resources/resource_monitor_grant.go @@ -34,6 +34,13 @@ var resourceMonitorGrantSchema = map[string]*schema.Schema{ Description: "Grants privilege to these roles.", ForceNew: true, }, + "with_grant_option": { + Type: schema.TypeBool, + Optional: true, + Description: "When this is set to true, allows the recipient role to grant the privileges to other roles.", + Default: false, + ForceNew: true, + }, } // ResourceMonitorGrant returns a pointer to the resource representing a resource monitor grant @@ -51,6 +58,7 @@ func ResourceMonitorGrant() *schema.Resource { func CreateResourceMonitorGrant(data *schema.ResourceData, meta interface{}) error { w := data.Get("monitor_name").(string) priv := data.Get("privilege").(string) + grantOption := data.Get("with_grant_option").(bool) builder := snowflake.ResourceMonitorGrant(w) err := createGenericGrant(data, meta, builder) @@ -61,6 +69,7 @@ func CreateResourceMonitorGrant(data *schema.ResourceData, meta interface{}) err grant := &grantID{ ResourceName: w, Privilege: priv, + GrantOption: grantOption, } dataIDInput, err := grant.String() if err != nil { @@ -88,6 +97,10 @@ func ReadResourceMonitorGrant(data *schema.ResourceData, meta interface{}) error if err != nil { return err } + err = data.Set("with_grant_option", grantID.GrantOption) + if err != nil { + return err + } builder := snowflake.ResourceMonitorGrant(w) diff --git a/pkg/resources/resource_monitor_grant_test.go b/pkg/resources/resource_monitor_grant_test.go index 2abf80ce8c..7315d2c4c1 100644 --- a/pkg/resources/resource_monitor_grant_test.go +++ b/pkg/resources/resource_monitor_grant_test.go @@ -25,16 +25,17 @@ func TestResourceMonitorGrantCreate(t *testing.T) { r := require.New(t) in := map[string]interface{}{ - "monitor_name": "test-monitor", - "privilege": "MONITOR", - "roles": []interface{}{"test-role-1", "test-role-2"}, + "monitor_name": "test-monitor", + "privilege": "MONITOR", + "roles": []interface{}{"test-role-1", "test-role-2"}, + "with_grant_option": true, } d := schema.TestResourceDataRaw(t, resources.ResourceMonitorGrant().Schema, in) r.NotNil(d) WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) { - mock.ExpectExec(`^GRANT MONITOR ON RESOURCE MONITOR "test-monitor" TO ROLE "test-role-1"$`).WillReturnResult(sqlmock.NewResult(1, 1)) - mock.ExpectExec(`^GRANT MONITOR ON RESOURCE MONITOR "test-monitor" TO ROLE "test-role-2"$`).WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec(`^GRANT MONITOR ON RESOURCE MONITOR "test-monitor" TO ROLE "test-role-1" WITH GRANT OPTION$`).WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec(`^GRANT MONITOR ON RESOURCE MONITOR "test-monitor" TO ROLE "test-role-2" WITH GRANT OPTION$`).WillReturnResult(sqlmock.NewResult(1, 1)) expectReadResourceMonitorGrant(mock) err := resources.CreateResourceMonitorGrant(d, db) r.NoError(err) @@ -44,10 +45,11 @@ func TestResourceMonitorGrantCreate(t *testing.T) { func TestResourceMonitorGrantRead(t *testing.T) { r := require.New(t) - d := resourceMonitorGrant(t, "test-monitor|||MONITOR", map[string]interface{}{ - "monitor_name": "test-monitor", - "privilege": "MONITOR", - "roles": []interface{}{"test-role-1", "test-role-2"}, + d := resourceMonitorGrant(t, "test-monitor|||MONITOR|false", map[string]interface{}{ + "monitor_name": "test-monitor", + "privilege": "MONITOR", + "roles": []interface{}{"test-role-1", "test-role-2"}, + "with_grant_option": false, }) r.NotNil(d) diff --git a/pkg/resources/schema_grant.go b/pkg/resources/schema_grant.go index 5a94412b99..1538c3204d 100644 --- a/pkg/resources/schema_grant.go +++ b/pkg/resources/schema_grant.go @@ -74,6 +74,13 @@ var schemaGrantSchema = map[string]*schema.Schema{ ForceNew: true, ConflictsWith: []string{"schema_name", "shares"}, }, + "with_grant_option": { + Type: schema.TypeBool, + Optional: true, + Description: "When this is set to true, allows the recipient role to grant the privileges to other roles.", + Default: false, + ForceNew: true, + }, } // SchemaGrant returns a pointer to the resource representing a view grant @@ -101,6 +108,7 @@ func CreateSchemaGrant(data *schema.ResourceData, meta interface{}) error { db := data.Get("database_name").(string) priv := data.Get("privilege").(string) onFuture := data.Get("on_future").(bool) + grantOption := data.Get("with_grant_option").(bool) if (schema == "") && !onFuture { return errors.New("schema_name must be set unless on_future is true.") @@ -122,6 +130,7 @@ func CreateSchemaGrant(data *schema.ResourceData, meta interface{}) error { ResourceName: db, SchemaName: schema, Privilege: priv, + GrantOption: grantOption, } dataIDInput, err := grantID.String() if err != nil { @@ -161,6 +170,10 @@ func ReadSchemaGrant(data *schema.ResourceData, meta interface{}) error { if err != nil { return err } + err = data.Set("with_grant_option", grantID.GrantOption) + if err != nil { + return err + } var builder snowflake.GrantBuilder if onFuture { diff --git a/pkg/resources/schema_grant_test.go b/pkg/resources/schema_grant_test.go index 70d76f447e..031600bf62 100644 --- a/pkg/resources/schema_grant_test.go +++ b/pkg/resources/schema_grant_test.go @@ -25,27 +25,28 @@ func TestSchemaGrantCreate(t *testing.T) { for _, test_priv := range []string{"USAGE", "MODIFY"} { in := map[string]interface{}{ - "schema_name": "test-schema", - "database_name": "test-db", - "privilege": test_priv, - "roles": []interface{}{"test-role-1", "test-role-2"}, - "shares": []interface{}{"test-share-1", "test-share-2"}, + "schema_name": "test-schema", + "database_name": "test-db", + "privilege": test_priv, + "roles": []interface{}{"test-role-1", "test-role-2"}, + "shares": []interface{}{"test-share-1", "test-share-2"}, + "with_grant_option": true, } d := schema.TestResourceDataRaw(t, resources.SchemaGrant().Schema, in) r.NotNil(d) WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) { mock.ExpectExec( - fmt.Sprintf(`^GRANT %s ON SCHEMA "test-db"."test-schema" TO ROLE "test-role-1"$`, test_priv), + fmt.Sprintf(`^GRANT %s ON SCHEMA "test-db"."test-schema" TO ROLE "test-role-1" WITH GRANT OPTION$`, test_priv), ).WillReturnResult(sqlmock.NewResult(1, 1)) mock.ExpectExec( - fmt.Sprintf(`^GRANT %s ON SCHEMA "test-db"."test-schema" TO ROLE "test-role-2"$`, test_priv), + fmt.Sprintf(`^GRANT %s ON SCHEMA "test-db"."test-schema" TO ROLE "test-role-2" WITH GRANT OPTION$`, test_priv), ).WillReturnResult(sqlmock.NewResult(1, 1)) mock.ExpectExec( - fmt.Sprintf(`^GRANT %s ON SCHEMA "test-db"."test-schema" TO SHARE "test-share-1"$`, test_priv), + fmt.Sprintf(`^GRANT %s ON SCHEMA "test-db"."test-schema" TO SHARE "test-share-1" WITH GRANT OPTION$`, test_priv), ).WillReturnResult(sqlmock.NewResult(1, 1)) mock.ExpectExec( - fmt.Sprintf(`^GRANT %s ON SCHEMA "test-db"."test-schema" TO SHARE "test-share-2"$`, test_priv), + fmt.Sprintf(`^GRANT %s ON SCHEMA "test-db"."test-schema" TO SHARE "test-share-2" WITH GRANT OPTION$`, test_priv), ).WillReturnResult(sqlmock.NewResult(1, 1)) expectReadSchemaGrant(mock, test_priv) err := resources.CreateSchemaGrant(d, db) @@ -73,20 +74,21 @@ func TestFutureSchemaGrantCreate(t *testing.T) { r := require.New(t) in := map[string]interface{}{ - "on_future": true, - "database_name": "test-db", - "privilege": "USAGE", - "roles": []interface{}{"test-role-1", "test-role-2"}, + "on_future": true, + "database_name": "test-db", + "privilege": "USAGE", + "roles": []interface{}{"test-role-1", "test-role-2"}, + "with_grant_option": true, } d := schema.TestResourceDataRaw(t, resources.SchemaGrant().Schema, in) r.NotNil(d) WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) { mock.ExpectExec( - `^GRANT USAGE ON FUTURE SCHEMAS IN DATABASE "test-db" TO ROLE "test-role-1"$`, + `^GRANT USAGE ON FUTURE SCHEMAS IN DATABASE "test-db" TO ROLE "test-role-1" WITH GRANT OPTION$`, ).WillReturnResult(sqlmock.NewResult(1, 1)) mock.ExpectExec( - `^GRANT USAGE ON FUTURE SCHEMAS IN DATABASE "test-db" TO ROLE "test-role-2"$`, + `^GRANT USAGE ON FUTURE SCHEMAS IN DATABASE "test-db" TO ROLE "test-role-2" WITH GRANT OPTION$`, ).WillReturnResult(sqlmock.NewResult(1, 1)) expectReadFutureSchemaGrant(mock) err := resources.CreateSchemaGrant(d, db) diff --git a/pkg/resources/share.go b/pkg/resources/share.go index 82733c3717..8457abe8fc 100644 --- a/pkg/resources/share.go +++ b/pkg/resources/share.go @@ -99,7 +99,7 @@ func setAccounts(data *schema.ResourceData, meta interface{}) error { // 2. Create temporary DB grant to the share tempDBGrant := snowflake.DatabaseGrant(tempName) - err = snowflake.Exec(db, tempDBGrant.Share(name).Grant("USAGE")) + err = snowflake.Exec(db, tempDBGrant.Share(name).Grant("USAGE", false)) if err != nil { return errors.Wrapf(err, "error creating temporary DB grant %v", tempName) } diff --git a/pkg/resources/stage_grant.go b/pkg/resources/stage_grant.go index 72c1f907d5..8cec3f1e59 100644 --- a/pkg/resources/stage_grant.go +++ b/pkg/resources/stage_grant.go @@ -57,6 +57,13 @@ var stageGrantSchema = map[string]*schema.Schema{ Description: "Grants privilege to these shares.", ForceNew: true, }, + "with_grant_option": { + Type: schema.TypeBool, + Optional: true, + Description: "When this is set to true, allows the recipient role to grant the privileges to other roles.", + Default: false, + ForceNew: true, + }, } // StageGrant returns a pointer to the resource representing a stage grant @@ -84,6 +91,7 @@ func CreateStageGrant(data *schema.ResourceData, meta interface{}) error { schemaName := data.Get("schema_name").(string) dbName := data.Get("database_name").(string) priv := data.Get("privilege").(string) + grantOption := data.Get("with_grant_option").(bool) var builder snowflake.GrantBuilder builder = snowflake.StageGrant(dbName, schemaName, stageName) @@ -98,6 +106,7 @@ func CreateStageGrant(data *schema.ResourceData, meta interface{}) error { SchemaName: schemaName, ObjectName: stageName, Privilege: priv, + GrantOption: grantOption, } dataIDInput, err := grant.String() if err != nil { @@ -135,6 +144,10 @@ func ReadStageGrant(data *schema.ResourceData, meta interface{}) error { if err != nil { return err } + err = data.Set("with_grant_option", grantID.GrantOption) + if err != nil { + return err + } builder := snowflake.StageGrant(dbName, schemaName, stageName) diff --git a/pkg/resources/stage_grant_test.go b/pkg/resources/stage_grant_test.go index 1f374e4ffd..fd292a37f4 100644 --- a/pkg/resources/stage_grant_test.go +++ b/pkg/resources/stage_grant_test.go @@ -25,21 +25,22 @@ func TestStageGrantCreate(t *testing.T) { for _, test_priv := range []string{"USAGE", "READ"} { in := map[string]interface{}{ - "stage_name": "test-stage", - "schema_name": "test-schema", - "database_name": "test-db", - "privilege": test_priv, - "roles": []interface{}{"test-role-1", "test-role-2"}, - "shares": []interface{}{"test-share-1", "test-share-2"}, + "stage_name": "test-stage", + "schema_name": "test-schema", + "database_name": "test-db", + "privilege": test_priv, + "roles": []interface{}{"test-role-1", "test-role-2"}, + "shares": []interface{}{"test-share-1", "test-share-2"}, + "with_grant_option": true, } d := schema.TestResourceDataRaw(t, resources.StageGrant().Schema, in) r.NotNil(d) WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) { - mock.ExpectExec(fmt.Sprintf(`^GRANT %s ON STAGE "test-db"."test-schema"."test-stage" TO ROLE "test-role-1"$`, test_priv)).WillReturnResult(sqlmock.NewResult(1, 1)) - mock.ExpectExec(fmt.Sprintf(`^GRANT %s ON STAGE "test-db"."test-schema"."test-stage" TO ROLE "test-role-2"$`, test_priv)).WillReturnResult(sqlmock.NewResult(1, 1)) - mock.ExpectExec(fmt.Sprintf(`^GRANT %s ON STAGE "test-db"."test-schema"."test-stage" TO SHARE "test-share-1"$`, test_priv)).WillReturnResult(sqlmock.NewResult(1, 1)) - mock.ExpectExec(fmt.Sprintf(`^GRANT %s ON STAGE "test-db"."test-schema"."test-stage" TO SHARE "test-share-2"$`, test_priv)).WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec(fmt.Sprintf(`^GRANT %s ON STAGE "test-db"."test-schema"."test-stage" TO ROLE "test-role-1" WITH GRANT OPTION$`, test_priv)).WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec(fmt.Sprintf(`^GRANT %s ON STAGE "test-db"."test-schema"."test-stage" TO ROLE "test-role-2" WITH GRANT OPTION$`, test_priv)).WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec(fmt.Sprintf(`^GRANT %s ON STAGE "test-db"."test-schema"."test-stage" TO SHARE "test-share-1" WITH GRANT OPTION$`, test_priv)).WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec(fmt.Sprintf(`^GRANT %s ON STAGE "test-db"."test-schema"."test-stage" TO SHARE "test-share-2" WITH GRANT OPTION$`, test_priv)).WillReturnResult(sqlmock.NewResult(1, 1)) expectReadStageGrant(mock, test_priv) err := resources.CreateStageGrant(d, db) r.NoError(err) diff --git a/pkg/resources/table_grant.go b/pkg/resources/table_grant.go index 945c61dd3e..16de9f4387 100644 --- a/pkg/resources/table_grant.go +++ b/pkg/resources/table_grant.go @@ -66,6 +66,13 @@ var tableGrantSchema = map[string]*schema.Schema{ ForceNew: true, ConflictsWith: []string{"table_name", "shares"}, }, + "with_grant_option": { + Type: schema.TypeBool, + Optional: true, + Description: "When this is set to true, allows the recipient role to grant the privileges to other roles.", + Default: false, + ForceNew: true, + }, } // TableGrant returns a pointer to the resource representing a Table grant @@ -101,6 +108,7 @@ func CreateTableGrant(data *schema.ResourceData, meta interface{}) error { dbName := data.Get("database_name").(string) priv := data.Get("privilege").(string) onFuture := data.Get("on_future").(bool) + grantOption := data.Get("with_grant_option").(bool) if (schemaName == "") && !onFuture { return errors.New("schema_name must be set unless on_future is true.") @@ -127,6 +135,7 @@ func CreateTableGrant(data *schema.ResourceData, meta interface{}) error { ResourceName: dbName, SchemaName: schemaName, Privilege: priv, + GrantOption: grantOption, } if !onFuture { grantID.ObjectName = tableName @@ -151,6 +160,7 @@ func ReadTableGrant(data *schema.ResourceData, meta interface{}) error { schemaName := grantID.SchemaName tableName := grantID.ObjectName priv := grantID.Privilege + err = data.Set("database_name", dbName) if err != nil { return err @@ -175,6 +185,10 @@ func ReadTableGrant(data *schema.ResourceData, meta interface{}) error { if err != nil { return err } + err = data.Set("with_grant_option", grantID.GrantOption) + if err != nil { + return err + } var builder snowflake.GrantBuilder if onFuture { diff --git a/pkg/resources/table_grant_test.go b/pkg/resources/table_grant_test.go index aad54d1800..33e202e3e3 100644 --- a/pkg/resources/table_grant_test.go +++ b/pkg/resources/table_grant_test.go @@ -25,21 +25,22 @@ func TestTableGrantCreate(t *testing.T) { r := require.New(t) in := map[string]interface{}{ - "table_name": "test-table", - "schema_name": "PUBLIC", - "database_name": "test-db", - "privilege": "SELECT", - "roles": []interface{}{"test-role-1", "test-role-2"}, - "shares": []interface{}{"test-share-1", "test-share-2"}, + "table_name": "test-table", + "schema_name": "PUBLIC", + "database_name": "test-db", + "privilege": "SELECT", + "roles": []interface{}{"test-role-1", "test-role-2"}, + "shares": []interface{}{"test-share-1", "test-share-2"}, + "with_grant_option": true, } d := schema.TestResourceDataRaw(t, resources.TableGrant().Schema, in) r.NotNil(d) WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) { - mock.ExpectExec(`^GRANT SELECT ON TABLE "test-db"."PUBLIC"."test-table" TO ROLE "test-role-1"$`).WillReturnResult(sqlmock.NewResult(1, 1)) - mock.ExpectExec(`^GRANT SELECT ON TABLE "test-db"."PUBLIC"."test-table" TO ROLE "test-role-2"$`).WillReturnResult(sqlmock.NewResult(1, 1)) - mock.ExpectExec(`^GRANT SELECT ON TABLE "test-db"."PUBLIC"."test-table" TO SHARE "test-share-1"$`).WillReturnResult(sqlmock.NewResult(1, 1)) - mock.ExpectExec(`^GRANT SELECT ON TABLE "test-db"."PUBLIC"."test-table" TO SHARE "test-share-2"$`).WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec(`^GRANT SELECT ON TABLE "test-db"."PUBLIC"."test-table" TO ROLE "test-role-1" WITH GRANT OPTION$`).WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec(`^GRANT SELECT ON TABLE "test-db"."PUBLIC"."test-table" TO ROLE "test-role-2" WITH GRANT OPTION$`).WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec(`^GRANT SELECT ON TABLE "test-db"."PUBLIC"."test-table" TO SHARE "test-share-1" WITH GRANT OPTION$`).WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec(`^GRANT SELECT ON TABLE "test-db"."PUBLIC"."test-table" TO SHARE "test-share-2" WITH GRANT OPTION$`).WillReturnResult(sqlmock.NewResult(1, 1)) expectReadTableGrant(mock) err := resources.CreateTableGrant(d, db) r.NoError(err) @@ -65,21 +66,22 @@ func TestFutureTableGrantCreate(t *testing.T) { r := require.New(t) in := map[string]interface{}{ - "on_future": true, - "schema_name": "PUBLIC", - "database_name": "test-db", - "privilege": "SELECT", - "roles": []interface{}{"test-role-1", "test-role-2"}, + "on_future": true, + "schema_name": "PUBLIC", + "database_name": "test-db", + "privilege": "SELECT", + "roles": []interface{}{"test-role-1", "test-role-2"}, + "with_grant_option": true, } d := schema.TestResourceDataRaw(t, resources.TableGrant().Schema, in) r.NotNil(d) WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) { mock.ExpectExec( - `^GRANT SELECT ON FUTURE TABLES IN SCHEMA "test-db"."PUBLIC" TO ROLE "test-role-1"$`, + `^GRANT SELECT ON FUTURE TABLES IN SCHEMA "test-db"."PUBLIC" TO ROLE "test-role-1" WITH GRANT OPTION$`, ).WillReturnResult(sqlmock.NewResult(1, 1)) mock.ExpectExec( - `^GRANT SELECT ON FUTURE TABLES IN SCHEMA "test-db"."PUBLIC" TO ROLE "test-role-2"$`, + `^GRANT SELECT ON FUTURE TABLES IN SCHEMA "test-db"."PUBLIC" TO ROLE "test-role-2" WITH GRANT OPTION$`, ).WillReturnResult(sqlmock.NewResult(1, 1)) expectReadFutureTableGrant(mock) err := resources.CreateTableGrant(d, db) @@ -89,10 +91,11 @@ func TestFutureTableGrantCreate(t *testing.T) { b := require.New(t) in = map[string]interface{}{ - "on_future": true, - "database_name": "test-db", - "privilege": "SELECT", - "roles": []interface{}{"test-role-1", "test-role-2"}, + "on_future": true, + "database_name": "test-db", + "privilege": "SELECT", + "roles": []interface{}{"test-role-1", "test-role-2"}, + "with_grant_option": false, } d = schema.TestResourceDataRaw(t, resources.TableGrant().Schema, in) b.NotNil(d) diff --git a/pkg/resources/view_grant.go b/pkg/resources/view_grant.go index c44020865f..09f2ae032c 100644 --- a/pkg/resources/view_grant.go +++ b/pkg/resources/view_grant.go @@ -61,6 +61,13 @@ var viewGrantSchema = map[string]*schema.Schema{ ForceNew: true, ConflictsWith: []string{"view_name", "shares"}, }, + "with_grant_option": { + Type: schema.TypeBool, + Optional: true, + Description: "When this is set to true, allows the recipient role to grant the privileges to other roles.", + Default: false, + ForceNew: true, + }, } // ViewGrant returns a pointer to the resource representing a view grant @@ -96,6 +103,7 @@ func CreateViewGrant(data *schema.ResourceData, meta interface{}) error { dbName := data.Get("database_name").(string) priv := data.Get("privilege").(string) futureViews := data.Get("on_future").(bool) + grantOption := data.Get("with_grant_option").(bool) if (schemaName == "") && !futureViews { return errors.New("schema_name must be set unless on_future is true.") @@ -125,6 +133,7 @@ func CreateViewGrant(data *schema.ResourceData, meta interface{}) error { SchemaName: schemaName, ObjectName: viewName, Privilege: priv, + GrantOption: grantOption, } dataIDInput, err := grant.String() if err != nil { @@ -170,6 +179,10 @@ func ReadViewGrant(data *schema.ResourceData, meta interface{}) error { if err != nil { return err } + err = data.Set("with_grant_option", grantID.GrantOption) + if err != nil { + return err + } var builder snowflake.GrantBuilder if futureViewsEnabled { diff --git a/pkg/resources/view_grant_test.go b/pkg/resources/view_grant_test.go index 099b402722..c08c62b8e5 100644 --- a/pkg/resources/view_grant_test.go +++ b/pkg/resources/view_grant_test.go @@ -23,21 +23,22 @@ func TestViewGrantCreate(t *testing.T) { r := require.New(t) in := map[string]interface{}{ - "view_name": "test-view", - "schema_name": "PUBLIC", - "database_name": "test-db", - "privilege": "SELECT", - "roles": []interface{}{"test-role-1", "test-role-2"}, - "shares": []interface{}{"test-share-1", "test-share-2"}, + "view_name": "test-view", + "schema_name": "PUBLIC", + "database_name": "test-db", + "privilege": "SELECT", + "roles": []interface{}{"test-role-1", "test-role-2"}, + "shares": []interface{}{"test-share-1", "test-share-2"}, + "with_grant_option": true, } d := schema.TestResourceDataRaw(t, resources.ViewGrant().Schema, in) r.NotNil(d) WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) { - mock.ExpectExec(`^GRANT SELECT ON VIEW "test-db"."PUBLIC"."test-view" TO ROLE "test-role-1"$`).WillReturnResult(sqlmock.NewResult(1, 1)) - mock.ExpectExec(`^GRANT SELECT ON VIEW "test-db"."PUBLIC"."test-view" TO ROLE "test-role-2"$`).WillReturnResult(sqlmock.NewResult(1, 1)) - mock.ExpectExec(`^GRANT SELECT ON VIEW "test-db"."PUBLIC"."test-view" TO SHARE "test-share-1"$`).WillReturnResult(sqlmock.NewResult(1, 1)) - mock.ExpectExec(`^GRANT SELECT ON VIEW "test-db"."PUBLIC"."test-view" TO SHARE "test-share-2"$`).WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec(`^GRANT SELECT ON VIEW "test-db"."PUBLIC"."test-view" TO ROLE "test-role-1" WITH GRANT OPTION$`).WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec(`^GRANT SELECT ON VIEW "test-db"."PUBLIC"."test-view" TO ROLE "test-role-2" WITH GRANT OPTION$`).WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec(`^GRANT SELECT ON VIEW "test-db"."PUBLIC"."test-view" TO SHARE "test-share-1" WITH GRANT OPTION$`).WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec(`^GRANT SELECT ON VIEW "test-db"."PUBLIC"."test-view" TO SHARE "test-share-2" WITH GRANT OPTION$`).WillReturnResult(sqlmock.NewResult(1, 1)) expectReadViewGrant(mock) err := resources.CreateViewGrant(d, db) r.NoError(err) @@ -63,21 +64,22 @@ func TestFutureViewGrantCreate(t *testing.T) { r := require.New(t) in := map[string]interface{}{ - "on_future": true, - "schema_name": "PUBLIC", - "database_name": "test-db", - "privilege": "SELECT", - "roles": []interface{}{"test-role-1", "test-role-2"}, + "on_future": true, + "schema_name": "PUBLIC", + "database_name": "test-db", + "privilege": "SELECT", + "roles": []interface{}{"test-role-1", "test-role-2"}, + "with_grant_option": true, } d := schema.TestResourceDataRaw(t, resources.ViewGrant().Schema, in) r.NotNil(d) WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) { mock.ExpectExec( - `^GRANT SELECT ON FUTURE VIEWS IN SCHEMA "test-db"."PUBLIC" TO ROLE "test-role-1"$`, + `^GRANT SELECT ON FUTURE VIEWS IN SCHEMA "test-db"."PUBLIC" TO ROLE "test-role-1" WITH GRANT OPTION$`, ).WillReturnResult(sqlmock.NewResult(1, 1)) mock.ExpectExec( - `^GRANT SELECT ON FUTURE VIEWS IN SCHEMA "test-db"."PUBLIC" TO ROLE "test-role-2"$`, + `^GRANT SELECT ON FUTURE VIEWS IN SCHEMA "test-db"."PUBLIC" TO ROLE "test-role-2" WITH GRANT OPTION$`, ).WillReturnResult(sqlmock.NewResult(1, 1)) expectReadFutureViewGrant(mock) err := resources.CreateViewGrant(d, db) @@ -87,10 +89,11 @@ func TestFutureViewGrantCreate(t *testing.T) { b := require.New(t) in = map[string]interface{}{ - "on_future": true, - "database_name": "test-db", - "privilege": "SELECT", - "roles": []interface{}{"test-role-1", "test-role-2"}, + "on_future": true, + "database_name": "test-db", + "privilege": "SELECT", + "roles": []interface{}{"test-role-1", "test-role-2"}, + "with_grant_option": false, } d = schema.TestResourceDataRaw(t, resources.ViewGrant().Schema, in) b.NotNil(d) diff --git a/pkg/resources/warehouse_grant.go b/pkg/resources/warehouse_grant.go index 32baa79b03..1de16fc3b4 100644 --- a/pkg/resources/warehouse_grant.go +++ b/pkg/resources/warehouse_grant.go @@ -36,6 +36,13 @@ var warehouseGrantSchema = map[string]*schema.Schema{ Description: "Grants privilege to these roles.", ForceNew: true, }, + "with_grant_option": { + Type: schema.TypeBool, + Optional: true, + Description: "When this is set to true, allows the recipient role to grant the privileges to other roles.", + Default: false, + ForceNew: true, + }, } // WarehouseGrant returns a pointer to the resource representing a warehouse grant @@ -57,6 +64,7 @@ func WarehouseGrant() *schema.Resource { func CreateWarehouseGrant(data *schema.ResourceData, meta interface{}) error { w := data.Get("warehouse_name").(string) priv := data.Get("privilege").(string) + grantOption := data.Get("with_grant_option").(bool) builder := snowflake.WarehouseGrant(w) err := createGenericGrant(data, meta, builder) @@ -67,6 +75,7 @@ func CreateWarehouseGrant(data *schema.ResourceData, meta interface{}) error { grant := &grantID{ ResourceName: w, Privilege: priv, + GrantOption: grantOption, } dataIDInput, err := grant.String() if err != nil { @@ -94,6 +103,10 @@ func ReadWarehouseGrant(data *schema.ResourceData, meta interface{}) error { if err != nil { return err } + err = data.Set("with_grant_option", grantID.GrantOption) + if err != nil { + return err + } builder := snowflake.WarehouseGrant(w) diff --git a/pkg/snowflake/future_grant.go b/pkg/snowflake/future_grant.go index 1be9cf639d..77de5d0af1 100644 --- a/pkg/snowflake/future_grant.go +++ b/pkg/snowflake/future_grant.go @@ -107,8 +107,14 @@ func (gb *FutureGrantBuilder) Share(n string) GrantExecutable { } // Grant returns the SQL that will grant future privileges on the grant to the grantee -func (fge *FutureGrantExecutable) Grant(p string) string { - return fmt.Sprintf(`GRANT %v ON FUTURE %vS IN %v %v TO ROLE "%v"`, +func (fge *FutureGrantExecutable) Grant(p string, w bool) string { + var template string + if w == true { + template = `GRANT %v ON FUTURE %vS IN %v %v TO ROLE "%v" WITH GRANT OPTION` + } else { + template = `GRANT %v ON FUTURE %vS IN %v %v TO ROLE "%v"` + } + return fmt.Sprintf(template, p, fge.futureGrantType, fge.futureGrantTarget, fge.grantName, fge.granteeName) } diff --git a/pkg/snowflake/future_grant_test.go b/pkg/snowflake/future_grant_test.go index ac6bbdd5fd..c74a4fff06 100644 --- a/pkg/snowflake/future_grant_test.go +++ b/pkg/snowflake/future_grant_test.go @@ -15,7 +15,7 @@ func TestFutureSchemaGrant(t *testing.T) { s := fvg.Show() r.Equal(`SHOW FUTURE GRANTS IN DATABASE "test_db"`, s) - s = fvg.Role("bob").Grant("USAGE") + s = fvg.Role("bob").Grant("USAGE", false) r.Equal(`GRANT USAGE ON FUTURE SCHEMAS IN DATABASE "test_db" TO ROLE "bob"`, s) s = fvg.Role("bob").Revoke("USAGE") @@ -30,7 +30,7 @@ func TestFutureTableGrant(t *testing.T) { s := fvg.Show() r.Equal(`SHOW FUTURE GRANTS IN SCHEMA "test_db"."PUBLIC"`, s) - s = fvg.Role("bob").Grant("USAGE") + s = fvg.Role("bob").Grant("USAGE", false) r.Equal(`GRANT USAGE ON FUTURE TABLES IN SCHEMA "test_db"."PUBLIC" TO ROLE "bob"`, s) s = fvg.Role("bob").Revoke("USAGE") @@ -43,7 +43,7 @@ func TestFutureTableGrant(t *testing.T) { s = fvgd.Show() b.Equal(`SHOW FUTURE GRANTS IN DATABASE "test_db"`, s) - s = fvgd.Role("bob").Grant("USAGE") + s = fvgd.Role("bob").Grant("USAGE", false) b.Equal(`GRANT USAGE ON FUTURE TABLES IN DATABASE "test_db" TO ROLE "bob"`, s) s = fvgd.Role("bob").Revoke("USAGE") @@ -58,7 +58,7 @@ func TestFutureViewGrant(t *testing.T) { s := fvg.Show() r.Equal(`SHOW FUTURE GRANTS IN SCHEMA "test_db"."PUBLIC"`, s) - s = fvg.Role("bob").Grant("USAGE") + s = fvg.Role("bob").Grant("USAGE", false) r.Equal(`GRANT USAGE ON FUTURE VIEWS IN SCHEMA "test_db"."PUBLIC" TO ROLE "bob"`, s) s = fvg.Role("bob").Revoke("USAGE") @@ -71,7 +71,7 @@ func TestFutureViewGrant(t *testing.T) { s = fvgd.Show() b.Equal(`SHOW FUTURE GRANTS IN DATABASE "test_db"`, s) - s = fvgd.Role("bob").Grant("USAGE") + s = fvgd.Role("bob").Grant("USAGE", false) b.Equal(`GRANT USAGE ON FUTURE VIEWS IN DATABASE "test_db" TO ROLE "bob"`, s) s = fvgd.Role("bob").Revoke("USAGE") diff --git a/pkg/snowflake/grant.go b/pkg/snowflake/grant.go index 8f48a04ae7..ca48df3c1b 100644 --- a/pkg/snowflake/grant.go +++ b/pkg/snowflake/grant.go @@ -21,7 +21,7 @@ const ( ) type GrantExecutable interface { - Grant(p string) string + Grant(p string, w bool) string Revoke(p string) string Show() string } @@ -167,10 +167,12 @@ func (gb *CurrentGrantBuilder) Share(n string) GrantExecutable { } // Grant returns the SQL that will grant privileges on the grant to the grantee -func (ge *CurrentGrantExecutable) Grant(p string) string { +func (ge *CurrentGrantExecutable) Grant(p string, w bool) string { var template string if p == `OWNERSHIP` { template = `GRANT %v ON %v %v TO %v "%v" COPY CURRENT GRANTS` + } else if w == true { + template = `GRANT %v ON %v %v TO %v "%v" WITH GRANT OPTION` } else { template = `GRANT %v ON %v %v TO %v "%v"` } diff --git a/pkg/snowflake/grant_test.go b/pkg/snowflake/grant_test.go index 23c8f419fa..bdaacca106 100644 --- a/pkg/snowflake/grant_test.go +++ b/pkg/snowflake/grant_test.go @@ -15,16 +15,16 @@ func TestDatabaseGrant(t *testing.T) { s := dg.Show() r.Equal(`SHOW GRANTS ON DATABASE "testDB"`, s) - s = dg.Role("bob").Grant("USAGE") + s = dg.Role("bob").Grant("USAGE", false) r.Equal(`GRANT USAGE ON DATABASE "testDB" TO ROLE "bob"`, s) s = dg.Role("bob").Revoke("USAGE") r.Equal(`REVOKE USAGE ON DATABASE "testDB" FROM ROLE "bob"`, s) - s = dg.Role("bob").Grant("OWNERSHIP") + s = dg.Role("bob").Grant("OWNERSHIP", false) r.Equal(`GRANT OWNERSHIP ON DATABASE "testDB" TO ROLE "bob" COPY CURRENT GRANTS`, s) - s = dg.Share("bob").Grant("USAGE") + s = dg.Share("bob").Grant("USAGE", false) r.Equal(`GRANT USAGE ON DATABASE "testDB" TO SHARE "bob"`, s) s = dg.Share("bob").Revoke("USAGE") @@ -39,19 +39,19 @@ func TestSchemaGrant(t *testing.T) { s := sg.Show() r.Equal(`SHOW GRANTS ON SCHEMA "test_db"."testSchema"`, s) - s = sg.Role("bob").Grant("USAGE") + s = sg.Role("bob").Grant("USAGE", false) r.Equal(`GRANT USAGE ON SCHEMA "test_db"."testSchema" TO ROLE "bob"`, s) s = sg.Role("bob").Revoke("USAGE") r.Equal(`REVOKE USAGE ON SCHEMA "test_db"."testSchema" FROM ROLE "bob"`, s) - s = sg.Share("bob").Grant("USAGE") + s = sg.Share("bob").Grant("USAGE", false) r.Equal(`GRANT USAGE ON SCHEMA "test_db"."testSchema" TO SHARE "bob"`, s) s = sg.Share("bob").Revoke("USAGE") r.Equal(`REVOKE USAGE ON SCHEMA "test_db"."testSchema" FROM SHARE "bob"`, s) - s = sg.Role("bob").Grant("OWNERSHIP") + s = sg.Role("bob").Grant("OWNERSHIP", false) r.Equal(`GRANT OWNERSHIP ON SCHEMA "test_db"."testSchema" TO ROLE "bob" COPY CURRENT GRANTS`, s) } @@ -63,19 +63,19 @@ func TestViewGrant(t *testing.T) { s := vg.Show() r.Equal(`SHOW GRANTS ON VIEW "test_db"."PUBLIC"."testView"`, s) - s = vg.Role("bob").Grant("USAGE") + s = vg.Role("bob").Grant("USAGE", false) r.Equal(`GRANT USAGE ON VIEW "test_db"."PUBLIC"."testView" TO ROLE "bob"`, s) s = vg.Role("bob").Revoke("USAGE") r.Equal(`REVOKE USAGE ON VIEW "test_db"."PUBLIC"."testView" FROM ROLE "bob"`, s) - s = vg.Share("bob").Grant("USAGE") + s = vg.Share("bob").Grant("USAGE", false) r.Equal(`GRANT USAGE ON VIEW "test_db"."PUBLIC"."testView" TO SHARE "bob"`, s) s = vg.Share("bob").Revoke("USAGE") r.Equal(`REVOKE USAGE ON VIEW "test_db"."PUBLIC"."testView" FROM SHARE "bob"`, s) - s = vg.Role("bob").Grant("OWNERSHIP") + s = vg.Role("bob").Grant("OWNERSHIP", false) r.Equal(`GRANT OWNERSHIP ON VIEW "test_db"."PUBLIC"."testView" TO ROLE "bob" COPY CURRENT GRANTS`, s) } @@ -87,13 +87,13 @@ func TestWarehouseGrant(t *testing.T) { s := wg.Show() r.Equal(`SHOW GRANTS ON WAREHOUSE "test_warehouse"`, s) - s = wg.Role("bob").Grant("USAGE") + s = wg.Role("bob").Grant("USAGE", false) r.Equal(`GRANT USAGE ON WAREHOUSE "test_warehouse" TO ROLE "bob"`, s) s = wg.Role("bob").Revoke("USAGE") r.Equal(`REVOKE USAGE ON WAREHOUSE "test_warehouse" FROM ROLE "bob"`, s) - s = wg.Role("bob").Grant("OWNERSHIP") + s = wg.Role("bob").Grant("OWNERSHIP", false) r.Equal(`GRANT OWNERSHIP ON WAREHOUSE "test_warehouse" TO ROLE "bob" COPY CURRENT GRANTS`, s) } @@ -109,7 +109,7 @@ func TestAccountGrant(t *testing.T) { s := wg.Show() r.Equal("SHOW GRANTS ON ACCOUNT ", s) - s = wg.Role("bob").Grant("MANAGE GRANTS") + s = wg.Role("bob").Grant("MANAGE GRANTS", false) r.Equal(`GRANT MANAGE GRANTS ON ACCOUNT TO ROLE "bob"`, s) s = wg.Role("bob").Revoke("MONITOR USAGE") @@ -124,13 +124,13 @@ func TestIntegrationGrant(t *testing.T) { s := wg.Show() r.Equal(`SHOW GRANTS ON INTEGRATION "test_integration"`, s) - s = wg.Role("bob").Grant("USAGE") + s = wg.Role("bob").Grant("USAGE", false) r.Equal(`GRANT USAGE ON INTEGRATION "test_integration" TO ROLE "bob"`, s) s = wg.Role("bob").Revoke("USAGE") r.Equal(`REVOKE USAGE ON INTEGRATION "test_integration" FROM ROLE "bob"`, s) - s = wg.Role("bob").Grant("OWNERSHIP") + s = wg.Role("bob").Grant("OWNERSHIP", false) r.Equal(`GRANT OWNERSHIP ON INTEGRATION "test_integration" TO ROLE "bob" COPY CURRENT GRANTS`, s) } @@ -142,7 +142,7 @@ func TestResourceMonitorGrant(t *testing.T) { s := wg.Show() r.Equal(`SHOW GRANTS ON RESOURCE MONITOR "test_monitor"`, s) - s = wg.Role("bob").Grant("MONITOR") + s = wg.Role("bob").Grant("MONITOR", false) r.Equal(`GRANT MONITOR ON RESOURCE MONITOR "test_monitor" TO ROLE "bob"`, s) s = wg.Role("bob").Revoke("MODIFY") From bd764b8118d6cef5163c4b7878732452cf9bfbe2 Mon Sep 17 00:00:00 2001 From: Abdullah Tariq Date: Sat, 1 Aug 2020 15:54:55 +0200 Subject: [PATCH 02/13] Update go.mod and go.sum files --- go.mod | 1 + go.sum | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/go.mod b/go.mod index d7709b0929..9ab7666925 100644 --- a/go.mod +++ b/go.mod @@ -18,4 +18,5 @@ require ( github.com/stretchr/testify v1.5.1 github.com/vmihailenco/msgpack v4.0.4+incompatible // indirect golang.org/x/crypto v0.0.0-20200707235045-ab33eee955e0 + golang.org/x/tools v0.0.0-20200731060945-b5fad4ed8dd6 // indirect ) diff --git a/go.sum b/go.sum index 25319129e9..666e3da134 100644 --- a/go.sum +++ b/go.sum @@ -350,6 +350,7 @@ github.com/vmihailenco/msgpack/v4 v4.3.12/go.mod h1:gborTTJjAo/GWTqqRjrLCn9pgNN+ github.com/vmihailenco/tagparser v0.1.1/go.mod h1:OeAg3pn3UbLjkWt+rN9oFYB6u/cQgqMEUPoW2WPyhdI= github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU= github.com/xordataexchange/crypt v0.0.3-0.20170626215501-b2862e3d0a77/go.mod h1:aYKd//L2LvnjZzWKhF00oedf4jCCReLcmhLdhm1A27Q= +github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/zalando/go-keyring v0.0.0-20200121091418-667557018717/go.mod h1:RaxNwUITJaHVdQ0VC7pELPZ3tOWn13nr0gZMZEhpVU0= github.com/zclconf/go-cty v1.0.0/go.mod h1:xnAOWiHeOqg2nWS62VtQ7pbOu17FtxJNW8RLEih+O3s= github.com/zclconf/go-cty v1.1.0 h1:uJwc9HiBOCpoKIObTQaLR+tsEXx1HBHnOsOOpcdhZgw= @@ -370,6 +371,7 @@ golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACk golang.org/x/crypto v0.0.0-20190426145343-a29dc8fdc734/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20190605123033-f99c8df09eb5/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20190820162420-60c769a6c586/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= +golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20200707235045-ab33eee955e0 h1:eIYIE7EC5/Wv5Kbz8bJPaq+TN3kq3W8S+LSm62vM0DY= golang.org/x/crypto v0.0.0-20200707235045-ab33eee955e0/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= @@ -384,6 +386,8 @@ golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3/go.mod h1:6SW0HCj/g11FgYtHl golang.org/x/lint v0.0.0-20190409202823-959b441ac422 h1:QzoH/1pFpZguR8NrRHLcO6jKqfv2zpuSqZLgdm7ZmjI= golang.org/x/lint v0.0.0-20190409202823-959b441ac422/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= golang.org/x/mobile v0.0.0-20190312151609-d3739f865fa6/go.mod h1:z+o9i4GpDbdi3rU15maQ/Ox0txvL9dWGYEHz965HBQE= +golang.org/x/mod v0.3.0 h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4= +golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180811021610-c39426892332/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -403,6 +407,7 @@ golang.org/x/net v0.0.0-20191009170851-d66e71096ffb/go.mod h1:z5CRVTTTmAJ677TzLL golang.org/x/net v0.0.0-20200202094626-16171245cfb2/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20200301022130-244492dfa37a/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20200324143707-d3edc9973b7e/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A= +golang.org/x/net v0.0.0-20200625001655-4c5254603344/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA= golang.org/x/net v0.0.0-20200707034311-ab3426394381 h1:VXak5I6aEWmAXeQjA+QSZzlgNrpq9mjcfDemuexIKsU= golang.org/x/net v0.0.0-20200707034311-ab3426394381/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= @@ -417,6 +422,8 @@ golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20190227155943-e225da77a7e6/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20190423024810-112230192c58 h1:8gQV6CLnAEikrhgkHFbMAEhagSSnXWGV915qUMm9mrU= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208 h1:qwRHBd0NqMbJxfbotnDhm2ByMI1Shq4Y6oRJo21SGJA= +golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20180823144017-11551d06cbcc/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -458,6 +465,11 @@ golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135/go.mod h1:RgjU9mgBXZiqYHBn golang.org/x/tools v0.0.0-20190606124116-d0a3d012864b/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc= golang.org/x/tools v0.0.0-20190628153133-6cdbf07be9d0 h1:Dh6fw+p6FyRl5x/FvNswO1ji0lIGzm3KP8Y9VkS9PTE= golang.org/x/tools v0.0.0-20190628153133-6cdbf07be9d0/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc= +golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= +golang.org/x/tools v0.0.0-20200731060945-b5fad4ed8dd6 h1:qKpj8TpV+LEhel7H/fR788J+KvhWZ3o3V6N2fU/iuLU= +golang.org/x/tools v0.0.0-20200731060945-b5fad4ed8dd6/go.mod h1:njjCfa9FT2d7l9Bc6FUM5FLjQPp3cFF28FI3qnDFljA= +golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/api v0.4.0/go.mod h1:8k5glujaEP+g9n7WNsDg8QP6cUVNI86fCNMcbazEtwE= From 68f80243930c58fa6382e57439b0be6eda185d7f Mon Sep 17 00:00:00 2001 From: Abdullah Tariq Date: Sat, 1 Aug 2020 15:57:43 +0200 Subject: [PATCH 03/13] Update docs related to with_grant_option property --- docs/resources/account_grant.md | 9 +++++---- docs/resources/database_grant.md | 13 +++++++------ docs/resources/integration_grant.md | 11 ++++++----- docs/resources/resource_monitor_grant.md | 11 ++++++----- docs/resources/schema_grant.md | 17 +++++++++-------- docs/resources/stage_grant.md | 17 +++++++++-------- docs/resources/table_grant.md | 19 ++++++++++--------- docs/resources/view_grant.md | 19 ++++++++++--------- docs/resources/warehouse_grant.md | 11 ++++++----- 9 files changed, 68 insertions(+), 59 deletions(-) diff --git a/docs/resources/account_grant.md b/docs/resources/account_grant.md index 7ce685f9c9..1fdb9991d4 100644 --- a/docs/resources/account_grant.md +++ b/docs/resources/account_grant.md @@ -11,7 +11,8 @@ ## properties -| NAME | TYPE | DESCRIPTION | OPTIONAL | REQUIRED | COMPUTED | DEFAULT | -|-----------|--------|---------------------------------------|----------|-----------|----------|---------| -| privilege | string | The privilege to grant on the schema. | true | false | false | "USAGE" | -| roles | set | Grants privilege to these roles. | true | false | false | | +| NAME | TYPE | DESCRIPTION | OPTIONAL | REQUIRED | COMPUTED | DEFAULT | +|-------------------|--------|---------------------------------------------------------------------------------------------|----------|-----------|----------|---------| +| privilege | string | The privilege to grant on the schema. | true | false | false | "USAGE" | +| roles | set | Grants privilege to these roles. | true | false | false | | +| with_grant_option | bool | When this is set to true, allows the recipient role to grant the privileges to other roles. | true | false | false | false | diff --git a/docs/resources/database_grant.md b/docs/resources/database_grant.md index d600dd871d..97da62336f 100644 --- a/docs/resources/database_grant.md +++ b/docs/resources/database_grant.md @@ -11,9 +11,10 @@ ## properties -| NAME | TYPE | DESCRIPTION | OPTIONAL | REQUIRED | COMPUTED | DEFAULT | -|---------------|--------|--------------------------------------------------------|----------|-----------|----------|---------| -| database_name | string | The name of the database on which to grant privileges. | false | true | false | | -| privilege | string | The privilege to grant on the database. | true | false | false | "USAGE" | -| roles | set | Grants privilege to these roles. | true | false | false | | -| shares | set | Grants privilege to these shares. | true | false | false | | +| NAME | TYPE | DESCRIPTION | OPTIONAL | REQUIRED | COMPUTED | DEFAULT | +|-------------------|--------|---------------------------------------------------------------------------------------------|----------|-----------|----------|---------| +| database_name | string | The name of the database on which to grant privileges. | false | true | false | | +| privilege | string | The privilege to grant on the database. | true | false | false | "USAGE" | +| roles | set | Grants privilege to these roles. | true | false | false | | +| shares | set | Grants privilege to these shares. | true | false | false | | +| with_grant_option | bool | When this is set to true, allows the recipient role to grant the privileges to other roles. | true | false | false | false | diff --git a/docs/resources/integration_grant.md b/docs/resources/integration_grant.md index a04eeb4941..5633b53734 100644 --- a/docs/resources/integration_grant.md +++ b/docs/resources/integration_grant.md @@ -11,8 +11,9 @@ ## properties -| NAME | TYPE | DESCRIPTION | OPTIONAL | REQUIRED | COMPUTED | DEFAULT | -|------------------|--------|------------------------------------------------------------------|----------|-----------|----------|---------| -| integration_name | string | Identifier for the integration; must be unique for your account. | false | true | false | | -| privilege | string | The privilege to grant on the integration. | true | false | false | "USAGE" | -| roles | set | Grants privilege to these roles. | true | false | false | | +| NAME | TYPE | DESCRIPTION | OPTIONAL | REQUIRED | COMPUTED | DEFAULT | +|-------------------|--------|---------------------------------------------------------------------------------------------|----------|-----------|----------|---------| +| integration_name | string | Identifier for the integration; must be unique for your account. | false | true | false | | +| privilege | string | The privilege to grant on the integration. | true | false | false | "USAGE" | +| roles | set | Grants privilege to these roles. | true | false | false | | +| with_grant_option | bool | When this is set to true, allows the recipient role to grant the privileges to other roles. | true | false | false | false | diff --git a/docs/resources/resource_monitor_grant.md b/docs/resources/resource_monitor_grant.md index dc135aacc8..37812676b4 100644 --- a/docs/resources/resource_monitor_grant.md +++ b/docs/resources/resource_monitor_grant.md @@ -11,8 +11,9 @@ ## properties -| NAME | TYPE | DESCRIPTION | OPTIONAL | REQUIRED | COMPUTED | DEFAULT | -|--------------|--------|-----------------------------------------------------------------------|----------|-----------|----------|-----------| -| monitor_name | string | Identifier for the resource monitor; must be unique for your account. | false | true | false | | -| privilege | string | The privilege to grant on the resource monitor. | true | false | false | "MONITOR" | -| roles | set | Grants privilege to these roles. | true | false | false | | +| NAME | TYPE | DESCRIPTION | OPTIONAL | REQUIRED | COMPUTED | DEFAULT | +|-------------------|--------|---------------------------------------------------------------------------------------------|----------|-----------|----------|-----------| +| monitor_name | string | Identifier for the resource monitor; must be unique for your account. | false | true | false | | +| privilege | string | The privilege to grant on the resource monitor. | true | false | false | "MONITOR" | +| roles | set | Grants privilege to these roles. | true | false | false | | +| with_grant_option | bool | When this is set to true, allows the recipient role to grant the privileges to other roles. | true | false | false | false | diff --git a/docs/resources/schema_grant.md b/docs/resources/schema_grant.md index 5e6344291f..e170267b4f 100644 --- a/docs/resources/schema_grant.md +++ b/docs/resources/schema_grant.md @@ -11,11 +11,12 @@ ## properties -| NAME | TYPE | DESCRIPTION | OPTIONAL | REQUIRED | COMPUTED | DEFAULT | -|---------------|--------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------|-----------|----------|---------| -| database_name | string | The name of the database containing the schema on which to grant privileges. | false | true | false | | -| on_future | bool | When this is set to true, apply this grant on all future schemas in the given database. The schema_name and shares fields must be unset in order to use on_future. | true | false | false | false | -| privilege | string | The privilege to grant on the current or future schema. Note that if "OWNERSHIP" is specified, ensure that the role that terraform is using is granted access. | true | false | false | "USAGE" | -| roles | set | Grants privilege to these roles. | true | false | false | | -| schema_name | string | The name of the schema on which to grant privileges. | true | false | false | | -| shares | set | Grants privilege to these shares (only valid if on_future is unset). | true | false | false | | +| NAME | TYPE | DESCRIPTION | OPTIONAL | REQUIRED | COMPUTED | DEFAULT | +|-------------------|--------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------|-----------|----------|---------| +| database_name | string | The name of the database containing the schema on which to grant privileges. | false | true | false | | +| on_future | bool | When this is set to true, apply this grant on all future schemas in the given database. The schema_name and shares fields must be unset in order to use on_future. | true | false | false | false | +| privilege | string | The privilege to grant on the current or future schema. Note that if "OWNERSHIP" is specified, ensure that the role that terraform is using is granted access. | true | false | false | "USAGE" | +| roles | set | Grants privilege to these roles. | true | false | false | | +| schema_name | string | The name of the schema on which to grant privileges. | true | false | false | | +| shares | set | Grants privilege to these shares (only valid if on_future is unset). | true | false | false | | +| with_grant_option | bool | When this is set to true, allows the recipient role to grant the privileges to other roles. | true | false | false | false | diff --git a/docs/resources/stage_grant.md b/docs/resources/stage_grant.md index 71086027f9..df9c1cea45 100644 --- a/docs/resources/stage_grant.md +++ b/docs/resources/stage_grant.md @@ -11,11 +11,12 @@ ## properties -| NAME | TYPE | DESCRIPTION | OPTIONAL | REQUIRED | COMPUTED | DEFAULT | -|---------------|--------|-------------------------------------------------------------------------------------|----------|-----------|----------|---------| -| database_name | string | The name of the database containing the current stage on which to grant privileges. | false | true | false | | -| privilege | string | The privilege to grant on the stage. | true | false | false | "USAGE" | -| roles | set | Grants privilege to these roles. | true | false | false | | -| schema_name | string | The name of the schema containing the current stage on which to grant privileges. | false | true | false | | -| shares | set | Grants privilege to these shares. | true | false | false | | -| stage_name | string | The name of the stage on which to grant privileges. | false | true | false | | +| NAME | TYPE | DESCRIPTION | OPTIONAL | REQUIRED | COMPUTED | DEFAULT | +|-------------------|--------|---------------------------------------------------------------------------------------------|----------|-----------|----------|---------| +| database_name | string | The name of the database containing the current stage on which to grant privileges. | false | true | false | | +| privilege | string | The privilege to grant on the stage. | true | false | false | "USAGE" | +| roles | set | Grants privilege to these roles. | true | false | false | | +| schema_name | string | The name of the schema containing the current stage on which to grant privileges. | false | true | false | | +| shares | set | Grants privilege to these shares. | true | false | false | | +| stage_name | string | The name of the stage on which to grant privileges. | false | true | false | | +| with_grant_option | bool | When this is set to true, allows the recipient role to grant the privileges to other roles. | true | false | false | false | diff --git a/docs/resources/table_grant.md b/docs/resources/table_grant.md index 782511048e..c88a651b1c 100644 --- a/docs/resources/table_grant.md +++ b/docs/resources/table_grant.md @@ -11,12 +11,13 @@ ## properties -| NAME | TYPE | DESCRIPTION | OPTIONAL | REQUIRED | COMPUTED | DEFAULT | -|---------------|--------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------|-----------|----------|----------| -| database_name | string | The name of the database containing the current or future tables on which to grant privileges. | false | true | false | | -| on_future | bool | When this is set to true and a schema_name is provided, apply this grant on all future tables in the given schema. When this is true and no schema_name is provided apply this grant on all future tables in the given database. The table_name and shares fields must be unset in order to use on_future. | true | false | false | false | -| privilege | string | The privilege to grant on the current or future table. | true | false | false | "SELECT" | -| roles | set | Grants privilege to these roles. | true | false | false | | -| schema_name | string | The name of the schema containing the current or future tables on which to grant privileges. | true | false | false | | -| shares | set | Grants privilege to these shares (only valid if on_future is unset). | true | false | false | | -| table_name | string | The name of the table on which to grant privileges immediately (only valid if on_future is unset). | true | false | false | | +| NAME | TYPE | DESCRIPTION | OPTIONAL | REQUIRED | COMPUTED | DEFAULT | +|-------------------|--------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------|-----------|----------|----------| +| database_name | string | The name of the database containing the current or future tables on which to grant privileges. | false | true | false | | +| on_future | bool | When this is set to true and a schema_name is provided, apply this grant on all future tables in the given schema. When this is true and no schema_name is provided apply this grant on all future tables in the given database. The table_name and shares fields must be unset in order to use on_future. | true | false | false | false | +| privilege | string | The privilege to grant on the current or future table. | true | false | false | "SELECT" | +| roles | set | Grants privilege to these roles. | true | false | false | | +| schema_name | string | The name of the schema containing the current or future tables on which to grant privileges. | true | false | false | | +| shares | set | Grants privilege to these shares (only valid if on_future is unset). | true | false | false | | +| table_name | string | The name of the table on which to grant privileges immediately (only valid if on_future is unset). | true | false | false | | +| with_grant_option | bool | When this is set to true, allows the recipient role to grant the privileges to other roles. | true | false | false | false | diff --git a/docs/resources/view_grant.md b/docs/resources/view_grant.md index c4b30672b6..cc32edaaa6 100644 --- a/docs/resources/view_grant.md +++ b/docs/resources/view_grant.md @@ -11,12 +11,13 @@ ## properties -| NAME | TYPE | DESCRIPTION | OPTIONAL | REQUIRED | COMPUTED | DEFAULT | -|---------------|--------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------|-----------|----------|----------| -| database_name | string | The name of the database containing the current or future views on which to grant privileges. | false | true | false | | -| on_future | bool | When this is set to true and a schema_name is provided, apply this grant on all future views in the given schema. When this is true and no schema_name is provided apply this grant on all future views in the given database. The view_name and shares fields must be unset in order to use on_future. | true | false | false | false | -| privilege | string | The privilege to grant on the current or future view. | true | false | false | "SELECT" | -| roles | set | Grants privilege to these roles. | true | false | false | | -| schema_name | string | The name of the schema containing the current or future views on which to grant privileges. | true | false | false | | -| shares | set | Grants privilege to these shares (only valid if on_future is unset). | true | false | false | | -| view_name | string | The name of the view on which to grant privileges immediately (only valid if on_future is unset). | true | false | false | | +| NAME | TYPE | DESCRIPTION | OPTIONAL | REQUIRED | COMPUTED | DEFAULT | +|-------------------|--------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------|-----------|----------|----------| +| database_name | string | The name of the database containing the current or future views on which to grant privileges. | false | true | false | | +| on_future | bool | When this is set to true and a schema_name is provided, apply this grant on all future views in the given schema. When this is true and no schema_name is provided apply this grant on all future views in the given database. The view_name and shares fields must be unset in order to use on_future. | true | false | false | false | +| privilege | string | The privilege to grant on the current or future view. | true | false | false | "SELECT" | +| roles | set | Grants privilege to these roles. | true | false | false | | +| schema_name | string | The name of the schema containing the current or future views on which to grant privileges. | true | false | false | | +| shares | set | Grants privilege to these shares (only valid if on_future is unset). | true | false | false | | +| view_name | string | The name of the view on which to grant privileges immediately (only valid if on_future is unset). | true | false | false | | +| with_grant_option | bool | When this is set to true, allows the recipient role to grant the privileges to other roles. | true | false | false | false | diff --git a/docs/resources/warehouse_grant.md b/docs/resources/warehouse_grant.md index f8b755b02c..e91f2298cc 100644 --- a/docs/resources/warehouse_grant.md +++ b/docs/resources/warehouse_grant.md @@ -11,8 +11,9 @@ ## properties -| NAME | TYPE | DESCRIPTION | OPTIONAL | REQUIRED | COMPUTED | DEFAULT | -|----------------|--------|---------------------------------------------------------|----------|-----------|----------|---------| -| privilege | string | The privilege to grant on the warehouse. | true | false | false | "USAGE" | -| roles | set | Grants privilege to these roles. | true | false | false | | -| warehouse_name | string | The name of the warehouse on which to grant privileges. | false | true | false | | +| NAME | TYPE | DESCRIPTION | OPTIONAL | REQUIRED | COMPUTED | DEFAULT | +|-------------------|--------|---------------------------------------------------------------------------------------------|----------|-----------|----------|---------| +| privilege | string | The privilege to grant on the warehouse. | true | false | false | "USAGE" | +| roles | set | Grants privilege to these roles. | true | false | false | | +| warehouse_name | string | The name of the warehouse on which to grant privileges. | false | true | false | | +| with_grant_option | bool | When this is set to true, allows the recipient role to grant the privileges to other roles. | true | false | false | false | From 8ec32c73c4fa6cb090a4c07625895115d9217bf8 Mon Sep 17 00:00:00 2001 From: John Peter Harvey Date: Tue, 11 Aug 2020 08:06:53 +0100 Subject: [PATCH 04/13] Start of table addition, some unit tests failing, no examples etc yet --- README.md | 2 +- pkg/provider/provider.go | 1 + pkg/resources/helpers_test.go | 8 + pkg/resources/table.go | 304 +++++++++++++++++++++++++++ pkg/resources/table_internal_test.go | 72 +++++++ pkg/resources/table_test.go | 70 ++++++ pkg/snowflake/table.go | 136 ++++++++++++ pkg/snowflake/table_test.go | 42 ++++ 8 files changed, 634 insertions(+), 1 deletion(-) create mode 100644 pkg/resources/table.go create mode 100644 pkg/resources/table_internal_test.go create mode 100644 pkg/resources/table_test.go create mode 100644 pkg/snowflake/table.go create mode 100644 pkg/snowflake/table_test.go diff --git a/README.md b/README.md index 8b8f4244e4..4bb41f6898 100644 --- a/README.md +++ b/README.md @@ -54,7 +54,7 @@ For the Terraform resources, there are 3 levels of testing - internal, unit and The 'internal' tests are run in the `github.com/chanzuckerberg/terraform-provider-snowflake/pkg/resources` package so that they can test functions that are not exported. These tests are intended to be limited to unit tests for simple functions. -The 'unit' tests are run in `github.com/chanzuckerberg/terraform-provider-snowflake/pkg/resources_test`, so they only have access to the exported methods of `resources`. These tests exercise the CRUD methods that on the terraform resources. Note that all tests here make use of database mocking and are run locally. This means the tests are fast, but are liable to be wrong in suble ways (since the mocks are unlikely to be perfect). +The 'unit' tests are run in `github.com/chanzuckerberg/terraform-provider-snowflake/pkg/resources_test`, so they only have access to the exported methods of `resources`. These tests exercise the CRUD methods that on the terraform resources. Note that all tests here make use of database mocking and are run locally. This means the tests are fast, but are liable to be wrong in subtle ways (since the mocks are unlikely to be perfect). You can run these first two sets of tests with `make test`. diff --git a/pkg/provider/provider.go b/pkg/provider/provider.go index 3db0bdd5cb..fea1483689 100644 --- a/pkg/provider/provider.go +++ b/pkg/provider/provider.go @@ -87,6 +87,7 @@ func Provider() *schema.Provider { "snowflake_view": resources.View(), "snowflake_view_grant": resources.ViewGrant(), "snowflake_task": resources.Task(), + "snowflake_table": resources.Table(), "snowflake_table_grant": resources.TableGrant(), "snowflake_warehouse": resources.Warehouse(), "snowflake_warehouse_grant": resources.WarehouseGrant(), diff --git a/pkg/resources/helpers_test.go b/pkg/resources/helpers_test.go index 8369185a37..edfca8048e 100644 --- a/pkg/resources/helpers_test.go +++ b/pkg/resources/helpers_test.go @@ -81,6 +81,14 @@ func storageIntegration(t *testing.T, id string, params map[string]interface{}) return d } +func table(t *testing.T, id string, params map[string]interface{}) *schema.ResourceData { + r := require.New(t) + d := schema.TestResourceDataRaw(t, resources.Table().Schema, params) + r.NotNil(d) + d.SetId(id) + return d +} + func user(t *testing.T, id string, params map[string]interface{}) *schema.ResourceData { r := require.New(t) d := schema.TestResourceDataRaw(t, resources.User().Schema, params) diff --git a/pkg/resources/table.go b/pkg/resources/table.go new file mode 100644 index 0000000000..45e54f2671 --- /dev/null +++ b/pkg/resources/table.go @@ -0,0 +1,304 @@ +package resources + +import ( + "bytes" + "database/sql" + "encoding/csv" + "fmt" + "strings" + + "github.com/chanzuckerberg/terraform-provider-snowflake/pkg/snowflake" + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" + "github.com/pkg/errors" +) + +const ( + tableIDDelimiter = '|' +) + +var tableSchema = map[string]*schema.Schema{ + "name": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + Description: "Specifies the identifier for the table; must be unique for the database and schema in which the pipe is created.", + }, + "schema": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + Description: "The schema in which to create the table.", + }, + "database": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + Description: "The database in which to create the table.", + }, + "columns": { + Type: schema.TypeMap, + Required: true, + ForceNew: true, + Description: "The column names and types to create.", + }, + "comment": { + Type: schema.TypeString, + Optional: true, + Description: "Specifies a comment for the table.", + }, + "owner": { + Type: schema.TypeString, + Computed: true, + Description: "Name of the role that owns the pipe.", + }, +} + +func Table() *schema.Resource { + return &schema.Resource{ + Create: CreateTable, + Read: ReadTable, + Update: UpdateTable, + Delete: DeleteTable, + Exists: TableExists, + + Schema: tableSchema, + Importer: &schema.ResourceImporter{ + State: schema.ImportStatePassthrough, + }, + } +} + +type tableID struct { + DatabaseName string + SchemaName string + TableName string +} + +//String() takes in a tableID object and returns a pipe-delimited string: +//DatabaseName|SchemaName|TableName +func (si *tableID) String() (string, error) { + var buf bytes.Buffer + csvWriter := csv.NewWriter(&buf) + csvWriter.Comma = tableIDDelimiter + dataIdentifiers := [][]string{{si.DatabaseName, si.SchemaName, si.TableName}} + err := csvWriter.WriteAll(dataIdentifiers) + if err != nil { + return "", err + } + strTableID := strings.TrimSpace(buf.String()) + return strTableID, nil +} + +// tableIDFromString() takes in a pipe-delimited string: DatabaseName|SchemaName|TableName +// and returns a tableID object +func tableIDFromString(stringID string) (*tableID, error) { + reader := csv.NewReader(strings.NewReader(stringID)) + reader.Comma = tableIDDelimiter + lines, err := reader.ReadAll() + if err != nil { + return nil, fmt.Errorf("Not CSV compatible") + } + + if len(lines) != 1 { + return nil, fmt.Errorf("1 line at a time") + } + if len(lines[0]) != 3 { + return nil, fmt.Errorf("3 fields allowed") + } + + tableResult := &tableID{ + DatabaseName: lines[0][0], + SchemaName: lines[0][1], + TableName: lines[0][2], + } + return tableResult, nil +} + +// CreateTable implements schema.CreateFunc +func CreateTable(data *schema.ResourceData, meta interface{}) error { + db := meta.(*sql.DB) + database := data.Get("database").(string) + schema := data.Get("schema").(string) + name := data.Get("name").(string) + columns := data.Get("columns").(map[string]interface{}) + + columnDefinitions := map[string]string{} + for columnName, columnType := range columns { + columnDefinitions[columnName] = columnType.(string) + } + + builder := snowflake.TableWithColumnDefinition(name, database, schema, columnDefinitions) + + // Set optionals + if v, ok := data.GetOk("comment"); ok { + builder.WithComment(v.(string)) + } + + q := builder.Create() + + err := snowflake.Exec(db, q) + if err != nil { + return errors.Wrapf(err, "error creating table %v", name) + } + + tableID := &tableID{ + DatabaseName: database, + SchemaName: schema, + TableName: name, + } + dataIDInput, err := tableID.String() + if err != nil { + return err + } + data.SetId(dataIDInput) + + return ReadTable(data, meta) +} + +// ReadTable implements schema.ReadFunc +func ReadTable(data *schema.ResourceData, meta interface{}) error { + db := meta.(*sql.DB) + tableID, err := tableIDFromString(data.Id()) + if err != nil { + return err + } + + dbName := tableID.DatabaseName + schema := tableID.SchemaName + name := tableID.TableName + + sq := snowflake.Table(name, dbName, schema).Show() + row := snowflake.QueryRow(db, sq) + table, err := snowflake.ScanTable(row) + if err != nil { + return err + } + + err = data.Set("name", table.Name) + if err != nil { + return err + } + + err = data.Set("type", table.Type) + if err != nil { + return err + } + + err = data.Set("kind", table.Kind) + if err != nil { + return err + } + + err = data.Set("null?", table.Null) + if err != nil { + return err + } + + err = data.Set("primary key", table.PrimaryKey) + if err != nil { + return err + } + + err = data.Set("unique key", table.UniqueKey) + if err != nil { + return err + } + + err = data.Set("check", table.Check) + if err != nil { + return err + } + + err = data.Set("expression", table.Expression) + if err != nil { + return err + } + + err = data.Set("comment", table.Comment) + if err != nil { + return err + } + + return nil +} + +// UpdateTable implements schema.UpdateFunc +func UpdateTable(data *schema.ResourceData, meta interface{}) error { + // https://www.terraform.io/docs/extend/writing-custom-providers.html#error-handling-amp-partial-state + data.Partial(true) + + tableID, err := tableIDFromString(data.Id()) + if err != nil { + return err + } + + dbName := tableID.DatabaseName + schema := tableID.SchemaName + tableName := tableID.TableName + + builder := snowflake.Table(tableName, dbName, schema) + + db := meta.(*sql.DB) + if data.HasChange("comment") { + _, comment := data.GetChange("comment") + q := builder.ChangeComment(comment.(string)) + err := snowflake.Exec(db, q) + if err != nil { + return errors.Wrapf(err, "error updating table comment on %v", data.Id()) + } + + data.SetPartial("comment") + } + + return ReadTable(data, meta) +} + +// DeleteTable implements schema.DeleteFunc +func DeleteTable(data *schema.ResourceData, meta interface{}) error { + db := meta.(*sql.DB) + tableID, err := tableIDFromString(data.Id()) + if err != nil { + return err + } + + dbName := tableID.DatabaseName + schema := tableID.SchemaName + tableName := tableID.TableName + + q := snowflake.Table(tableName, dbName, schema).Drop() + + err = snowflake.Exec(db, q) + if err != nil { + return errors.Wrapf(err, "error deleting pipe %v", data.Id()) + } + + data.SetId("") + + return nil +} + +// TableExists implements schema.ExistsFunc +func TableExists(data *schema.ResourceData, meta interface{}) (bool, error) { + db := meta.(*sql.DB) + tableID, err := tableIDFromString(data.Id()) + if err != nil { + return false, err + } + + dbName := tableID.DatabaseName + schema := tableID.SchemaName + tableName := tableID.TableName + + q := snowflake.Table(tableName, dbName, schema).Show() + rows, err := db.Query(q) + if err != nil { + return false, err + } + defer rows.Close() + + if rows.Next() { + return true, nil + } + + return false, nil +} diff --git a/pkg/resources/table_internal_test.go b/pkg/resources/table_internal_test.go new file mode 100644 index 0000000000..f35c953025 --- /dev/null +++ b/pkg/resources/table_internal_test.go @@ -0,0 +1,72 @@ +package resources + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestTableIDFromString(t *testing.T) { + r := require.New(t) + // Vanilla + id := "database_name|schema_name|table" + table, err := tableIDFromString(id) + r.NoError(err) + r.Equal("database_name", table.DatabaseName) + r.Equal("schema_name", table.SchemaName) + r.Equal("table", table.TableName) + + // Bad ID -- not enough fields + id = "database" + _, err = tableIDFromString(id) + r.Equal(fmt.Errorf("3 fields allowed"), err) + + // Bad ID + id = "||" + _, err = tableIDFromString(id) + r.NoError(err) + + // 0 lines + id = "" + _, err = tableIDFromString(id) + r.Equal(fmt.Errorf("1 line at a time"), err) + + // 2 lines + id = `database_name|schema_name|table + database_name|schema_name|table` + _, err = tableIDFromString(id) + r.Equal(fmt.Errorf("1 line at a time"), err) +} + +func TestTableStruct(t *testing.T) { + r := require.New(t) + + // Vanilla + table := &tableID{ + DatabaseName: "database_name", + SchemaName: "schema_name", + TableName: "table", + } + sID, err := table.String() + r.NoError(err) + r.Equal("database_name|schema_name|table", sID) + + // Empty grant + table = &tableID{} + sID, err = table.String() + r.NoError(err) + r.Equal("||", sID) + + // Grant with extra delimiters + table = &tableID{ + DatabaseName: "database|name", + TableName: "table|name", + } + sID, err = table.String() + r.NoError(err) + newTable, err := tableIDFromString(sID) + r.NoError(err) + r.Equal("database|name", newTable.DatabaseName) + r.Equal("table|name", newTable.TableName) +} diff --git a/pkg/resources/table_test.go b/pkg/resources/table_test.go new file mode 100644 index 0000000000..0a6abbd2cd --- /dev/null +++ b/pkg/resources/table_test.go @@ -0,0 +1,70 @@ +package resources_test + +import ( + "database/sql" + "testing" + + sqlmock "github.com/DATA-DOG/go-sqlmock" + "github.com/chanzuckerberg/terraform-provider-snowflake/pkg/provider" + "github.com/chanzuckerberg/terraform-provider-snowflake/pkg/resources" + . "github.com/chanzuckerberg/terraform-provider-snowflake/pkg/testhelpers" + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" + "github.com/stretchr/testify/require" +) + +func TestTable(t *testing.T) { + r := require.New(t) + err := resources.Table().InternalValidate(provider.Provider().Schema, true) + r.NoError(err) +} + +func TestTableCreate(t *testing.T) { + r := require.New(t) + + in := map[string]interface{}{ + "name": "good_name", + "comment": "great comment", + "columns": map[string]interface{}{"column1": "VARCHAR"}, + } + d := schema.TestResourceDataRaw(t, resources.Table().Schema, in) + r.NotNil(d) + + WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) { + mock.ExpectExec(`CREATE TABLE "good_name" ("column1" VARCHAR) COMMENT = 'great comment'`).WillReturnResult(sqlmock.NewResult(1, 1)) + expectTableRead(mock) + err := resources.CreateTable(d, db) + r.NoError(err) + }) +} + +func expectTableRead(mock sqlmock.Sqlmock) { + rows := sqlmock.NewRows([]string{"name", "type", "kind", "null?", "primary key", "unique key", "check", "expression", "comment"}).AddRow("good_name", "VARCHAR()", "COLUMN", "Y", "NULL", "N", "N", "NULL", "mock comment") + mock.ExpectQuery(`SHOW TABLES LIKE 'good_name' IN DATABASE "database_name"`).WillReturnRows(rows) +} + +func TestTableRead(t *testing.T) { + r := require.New(t) + + d := table(t, "database_name|schema_name|good_name", map[string]interface{}{"name": "good_name"}) + + WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) { + expectTableRead(mock) + + err := resources.ReadTable(d, db) + r.NoError(err) + r.Equal("good_name", d.Get("name").(string)) + r.Equal("mock comment", d.Get("comment").(string)) + }) +} + +func TestTableDelete(t *testing.T) { + r := require.New(t) + + d := table(t, "database_name|schema_name|drop_it", map[string]interface{}{"name": "drop_it"}) + + WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) { + mock.ExpectExec(`DROP TABLE "database_name"."schema_name"."drop_it"`).WillReturnResult(sqlmock.NewResult(1, 1)) + err := resources.DeleteTable(d, db) + r.NoError(err) + }) +} diff --git a/pkg/snowflake/table.go b/pkg/snowflake/table.go new file mode 100644 index 0000000000..b9f9bf0682 --- /dev/null +++ b/pkg/snowflake/table.go @@ -0,0 +1,136 @@ +package snowflake + +import ( + "fmt" + "strings" + + "github.com/jmoiron/sqlx" +) + +//CREATE TABLE "FULFIL_TEST"."PUBLIC"."TABLENAME" ("C1" STRING); + +// TableBuilder abstracts the creation of SQL queries for a Snowflake schema +type TableBuilder struct { + name string + db string + schema string + columns map[string]string + comment string +} + +// QualifiedName prepends the db and schema if set and escapes everything nicely +func (tb *TableBuilder) QualifiedName() string { + var n strings.Builder + + if tb.db != "" && tb.schema != "" { + n.WriteString(fmt.Sprintf(`"%v"."%v".`, tb.db, tb.schema)) + } + + if tb.db != "" && tb.schema == "" { + n.WriteString(fmt.Sprintf(`"%v"..`, tb.db)) + } + + if tb.db == "" && tb.schema != "" { + n.WriteString(fmt.Sprintf(`"%v".`, tb.schema)) + } + + n.WriteString(fmt.Sprintf(`"%v"`, tb.name)) + + return n.String() +} + +// WithComment adds a comment to the TableBuilder +func (tb *TableBuilder) WithComment(c string) *TableBuilder { + tb.comment = c + return tb +} + +// Table returns a pointer to a Builder that abstracts the DDL operations for a table. +// +// Supported DDL operations are: +// - ALTER TABLE +// - DROP TABLE +// - SHOW TABLES +// +// [Snowflake Reference](https://docs.snowflake.com/en/sql-reference/ddl-table.html) +func Table(name, db, schema string) *TableBuilder { + return &TableBuilder{ + name: name, + db: db, + schema: schema, + } +} + +// Table returns a pointer to a Builder that abstracts the DDL operations for a table. +// +// Supported DDL operations are: +// - CREATE TABLE +// +// [Snowflake Reference](https://docs.snowflake.com/en/sql-reference/ddl-table.html) +func TableWithColumnDefinition(name, db, schema string, columns map[string]string) *TableBuilder { + return &TableBuilder{ + name: name, + db: db, + schema: schema, + columns: columns, + } +} + +// Create returns the SQL statement required to create a table +func (tb *TableBuilder) Create() string { + q := strings.Builder{} + q.WriteString(`CREATE`) + + q.WriteString(fmt.Sprintf(` TABLE %v`, tb.QualifiedName())) + q.WriteString(fmt.Sprintf(` (`)) + columnDefinitions := []string{} + for columnName, columnType := range tb.columns { + columnDefinitions = append(columnDefinitions, fmt.Sprintf(`"%v" %v`, columnName, columnType)) + } + q.WriteString(strings.Join(columnDefinitions, ", ")) + q.WriteString(fmt.Sprintf(`)`)) + + if tb.comment != "" { + q.WriteString(fmt.Sprintf(` COMMENT = '%v'`, EscapeString(tb.comment))) + } + + return q.String() +} + +// ChangeComment returns the SQL query that will update the comment on the table. +func (tb *TableBuilder) ChangeComment(c string) string { + return fmt.Sprintf(`ALTER TABLE %v SET COMMENT = '%v'`, tb.QualifiedName(), EscapeString(c)) +} + +// RemoveComment returns the SQL query that will remove the comment on the table. +func (tb *TableBuilder) RemoveComment() string { + return fmt.Sprintf(`ALTER TABLE %v UNSET COMMENT`, tb.QualifiedName()) +} + +// Drop returns the SQL query that will drop a table. +func (tb *TableBuilder) Drop() string { + return fmt.Sprintf(`DROP TABLE %v`, tb.QualifiedName()) +} + +// Show returns the SQL query that will show a table. +func (tb *TableBuilder) Show() string { + return fmt.Sprintf(`SHOW TABLES LIKE '%v' IN DATABASE "%v"`, tb.name, tb.db) +} + +type table struct { + Name string `db:"name"` + Type string `db:"type"` + Kind string `db:"kind"` + Null string `db:"null?"` + PrimaryKey string `db:"primary key"` + UniqueKey string `db:"unique key"` + Check string `db:"check"` + Expression string `db:"expression"` + Comment string `db:"comment"` +} + +func ScanTable(row *sqlx.Row) (*table, error) { + t := &table{} + e := row.StructScan(t) + return t, e +} diff --git a/pkg/snowflake/table_test.go b/pkg/snowflake/table_test.go new file mode 100644 index 0000000000..ca62178b2e --- /dev/null +++ b/pkg/snowflake/table_test.go @@ -0,0 +1,42 @@ +package snowflake + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestTableCreate(t *testing.T) { + r := require.New(t) + s := TableWithColumnDefinition("test_table", "test_db", "test_schema", map[string]string{"column1": "OBJECT", "column2": "VARCHAR(255)"}) + r.Equal(s.QualifiedName(), `"test_db"."test_schema"."test_table"`) + + r.Equal(s.Create(), `CREATE TABLE "test_db"."test_schema"."test_table" ("column1" OBJECT, "column2" VARCHAR(255))`) + + s.WithComment("Test Comment") + r.Equal(s.Create(), `CREATE TABLE "test_db"."test_schema"."test_table" ("column1" OBJECT, "column2" VARCHAR(255)) COMMENT = 'Test Comment'`) +} + +func TestTableChangeComment(t *testing.T) { + r := require.New(t) + s := Table("test_table", "test_db", "test_schema") + r.Equal(s.ChangeComment("new table comment"), `ALTER TABLE "test_db"."test_schema"."test_table" SET COMMENT = 'new table comment'`) +} + +func TestTableRemoveComment(t *testing.T) { + r := require.New(t) + s := Table("test_table", "test_db", "test_schema") + r.Equal(s.RemoveComment(), `ALTER TABLE "test_db"."test_schema"."test_table" UNSET COMMENT`) +} + +func TestTableDrop(t *testing.T) { + r := require.New(t) + s := Table("test_table", "test_db", "test_schema") + r.Equal(s.Drop(), `DROP TABLE "test_db"."test_schema"."test_table"`) +} + +func TestTableShow(t *testing.T) { + r := require.New(t) + s := Table("test_table", "test_db", "test_schema") + r.Equal(s.Show(), `SHOW TABLES LIKE 'test_table' IN DATABASE "test_db"`) +} From 668cb1d3640fb74dc578e1face46e8f08a3e236d Mon Sep 17 00:00:00 2001 From: Abdullah Tariq Date: Fri, 14 Aug 2020 10:24:53 +0200 Subject: [PATCH 05/13] Make old state files compatible --- pkg/resources/grant_helpers.go | 11 +++++++--- pkg/resources/grant_helpers_internal_test.go | 23 +++++++++++++++----- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/pkg/resources/grant_helpers.go b/pkg/resources/grant_helpers.go index 7dcddc4408..e1f0ebbdde 100644 --- a/pkg/resources/grant_helpers.go +++ b/pkg/resources/grant_helpers.go @@ -150,8 +150,13 @@ func grantIDFromString(stringID string) (*grantID, error) { if len(lines) != 1 { return nil, fmt.Errorf("1 line per grant") } - if len(lines[0]) != 5 { - return nil, fmt.Errorf("5 fields allowed") + if len(lines[0]) != 4 && len(lines[0]) != 5 { + return nil, fmt.Errorf("4 or 5 fields allowed") + } + + grantOption := false + if len(lines[0]) == 5 && lines[0][4] == "true" { + grantOption = true } grantResult := &grantID{ @@ -159,7 +164,7 @@ func grantIDFromString(stringID string) (*grantID, error) { SchemaName: lines[0][1], ObjectName: lines[0][2], Privilege: lines[0][3], - GrantOption: lines[0][4] == "true", + GrantOption: grantOption, } return grantResult, nil } diff --git a/pkg/resources/grant_helpers_internal_test.go b/pkg/resources/grant_helpers_internal_test.go index 22441d9453..aa4b0e77a8 100644 --- a/pkg/resources/grant_helpers_internal_test.go +++ b/pkg/resources/grant_helpers_internal_test.go @@ -9,11 +9,22 @@ import ( func TestGrantIDFromString(t *testing.T) { r := require.New(t) - // Vanilla - id := "database_name|schema|view_name|privilege|true" + // Vanilla without GrantOption + id := "database_name|schema|view_name|privilege" grant, err := grantIDFromString(id) r.NoError(err) + r.Equal("database_name", grant.ResourceName) + r.Equal("schema", grant.SchemaName) + r.Equal("view_name", grant.ObjectName) + r.Equal("privilege", grant.Privilege) + r.Equal(false, grant.GrantOption) + + // Vanilla with GrantOption + id = "database_name|schema|view_name|privilege|true" + grant, err = grantIDFromString(id) + r.NoError(err) + r.Equal("database_name", grant.ResourceName) r.Equal("schema", grant.SchemaName) r.Equal("view_name", grant.ObjectName) @@ -33,17 +44,17 @@ func TestGrantIDFromString(t *testing.T) { // Bad ID -- not enough fields id = "database|name-privilege" _, err = grantIDFromString(id) - r.Equal(fmt.Errorf("5 fields allowed"), err) + r.Equal(fmt.Errorf("4 or 5 fields allowed"), err) // Bad ID -- privilege in wrong area - id = "database|||name-privilege" + id = "database||name-privilege" _, err = grantIDFromString(id) - r.Equal(fmt.Errorf("5 fields allowed"), err) + r.Equal(fmt.Errorf("4 or 5 fields allowed"), err) // too many fields id = "database_name|schema|view_name|privilege|false|2" _, err = grantIDFromString(id) - r.Equal(fmt.Errorf("5 fields allowed"), err) + r.Equal(fmt.Errorf("4 or 5 fields allowed"), err) // 0 lines id = "" From 9e4f8cdd80e8319222b521827fae32aeb2a73063 Mon Sep 17 00:00:00 2001 From: John Peter Harvey Date: Mon, 17 Aug 2020 09:09:59 +0100 Subject: [PATCH 06/13] Test still failing --- go.sum | 2 ++ pkg/resources/table.go | 53 ++++++++++++++++++++++++++++++++----- pkg/resources/table_test.go | 15 ++++++----- pkg/snowflake/exec.go | 4 +-- pkg/snowflake/table.go | 22 +++++++-------- pkg/snowflake/table_test.go | 6 ++--- 6 files changed, 74 insertions(+), 28 deletions(-) diff --git a/go.sum b/go.sum index 25319129e9..14d3df05c9 100644 --- a/go.sum +++ b/go.sum @@ -16,6 +16,8 @@ github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03 github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo= github.com/DATA-DOG/go-sqlmock v1.4.1 h1:ThlnYciV1iM/V0OSF/dtkqWb6xo5qITT1TJBG1MRDJM= github.com/DATA-DOG/go-sqlmock v1.4.1/go.mod h1:f/Ixk793poVmq4qj/V1dPUg2JEAKC73Q5eFN3EC/SaM= +github.com/DATA-DOG/go-sqlmock v1.5.0 h1:Shsta01QNfFxHCfpW6YH2STWB0MudeXXEWMr20OEh60= +github.com/DATA-DOG/go-sqlmock v1.5.0/go.mod h1:f/Ixk793poVmq4qj/V1dPUg2JEAKC73Q5eFN3EC/SaM= github.com/DataDog/zstd v1.4.4/go.mod h1:1jcaCB/ufaK+sKp1NBhlGmpz41jOoPQ35bpF36t7BBo= github.com/ExpansiveWorlds/instrumentedsql v0.0.0-20171218214018-45abb4b1947d h1:r+whow+VHd9kAd4UTtQ/rtvcvmwkdryKUcGofpYOp+8= github.com/ExpansiveWorlds/instrumentedsql v0.0.0-20171218214018-45abb4b1947d/go.mod h1:Lm6NFlzU3HvZIo5l8GykZn6MH8/wq/A/X/d+7P/hgZU= diff --git a/pkg/resources/table.go b/pkg/resources/table.go index 45e54f2671..284ae6c75f 100644 --- a/pkg/resources/table.go +++ b/pkg/resources/table.go @@ -51,6 +51,42 @@ var tableSchema = map[string]*schema.Schema{ Computed: true, Description: "Name of the role that owns the pipe.", }, + "type": { + Type: schema.TypeString, + Computed: true, + Description: "Data type.", + }, + "kind": { + Type: schema.TypeString, + Computed: true, + Description: "Kind of object.", + }, + "null": { + Type: schema.TypeString, + Computed: true, + }, + "default": { + Type: schema.TypeString, + Computed: true, + }, + "primary_key": { + Type: schema.TypeBool, + Computed: true, + Description: "Is this a primary key?", + }, + "unique_key": { + Type: schema.TypeBool, + Computed: true, + Description: "Is this a unique key?", + }, + "check": { + Type: schema.TypeString, + Computed: true, + }, + "expression": { + Type: schema.TypeString, + Computed: true, + }, } func Table() *schema.Resource { @@ -189,32 +225,37 @@ func ReadTable(data *schema.ResourceData, meta interface{}) error { return err } - err = data.Set("null?", table.Null) + err = data.Set("null", table.Null) + if err != nil { + return err + } + + err = data.Set("default", table.Default.String) if err != nil { return err } - err = data.Set("primary key", table.PrimaryKey) + err = data.Set("primary_key", table.PrimaryKey == "Y") if err != nil { return err } - err = data.Set("unique key", table.UniqueKey) + err = data.Set("unique_key", table.UniqueKey == "Y") if err != nil { return err } - err = data.Set("check", table.Check) + err = data.Set("check", table.Check.String) if err != nil { return err } - err = data.Set("expression", table.Expression) + err = data.Set("expression", table.Expression.String) if err != nil { return err } - err = data.Set("comment", table.Comment) + err = data.Set("comment", table.Comment.String) if err != nil { return err } diff --git a/pkg/resources/table_test.go b/pkg/resources/table_test.go index 0a6abbd2cd..9476b4ca99 100644 --- a/pkg/resources/table_test.go +++ b/pkg/resources/table_test.go @@ -2,6 +2,7 @@ package resources_test import ( "database/sql" + "fmt" "testing" sqlmock "github.com/DATA-DOG/go-sqlmock" @@ -22,15 +23,17 @@ func TestTableCreate(t *testing.T) { r := require.New(t) in := map[string]interface{}{ - "name": "good_name", - "comment": "great comment", - "columns": map[string]interface{}{"column1": "VARCHAR"}, + "name": "test_name", + "database": "test_db", + "schema": "test_schema", + "comment": "great comment", + "columns": map[string]interface{}{"column1": "VARCHAR"}, } d := schema.TestResourceDataRaw(t, resources.Table().Schema, in) r.NotNil(d) WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) { - mock.ExpectExec(`CREATE TABLE "good_name" ("column1" VARCHAR) COMMENT = 'great comment'`).WillReturnResult(sqlmock.NewResult(1, 1)) + fmt.Println(mock.ExpectExec(`CREATE TABLE "test_db"."test_schema"."test_name" ("column1" VARCHAR) COMMENT = 'great comment`).WillReturnResult(sqlmock.NewResult(1, 1))) expectTableRead(mock) err := resources.CreateTable(d, db) r.NoError(err) @@ -38,8 +41,8 @@ func TestTableCreate(t *testing.T) { } func expectTableRead(mock sqlmock.Sqlmock) { - rows := sqlmock.NewRows([]string{"name", "type", "kind", "null?", "primary key", "unique key", "check", "expression", "comment"}).AddRow("good_name", "VARCHAR()", "COLUMN", "Y", "NULL", "N", "N", "NULL", "mock comment") - mock.ExpectQuery(`SHOW TABLES LIKE 'good_name' IN DATABASE "database_name"`).WillReturnRows(rows) + rows := sqlmock.NewRows([]string{"name", "type", "kind", "null?", "default", "primary key", "unique key", "check", "expression", "comment"}).AddRow("good_name", "VARCHAR()", "COLUMN", "Y", "NULL", "NULL", "N", "N", "NULL", "mock comment") + fmt.Println(mock.ExpectQuery(`SHOW TABLES LIKE 'good_name' IN DATABASE "database_name"`).WillReturnRows(rows)) } func TestTableRead(t *testing.T) { diff --git a/pkg/snowflake/exec.go b/pkg/snowflake/exec.go index 684fb02376..1150e5594a 100644 --- a/pkg/snowflake/exec.go +++ b/pkg/snowflake/exec.go @@ -8,7 +8,7 @@ import ( ) func Exec(db *sql.DB, query string) error { - log.Print("[DEBUG] stmt ", query) + log.Print("[DEBUG] exec stmt ", query) _, err := db.Exec(query) return err @@ -18,7 +18,7 @@ func Exec(db *sql.DB, query string) error { // [DB.Unsafe](https://godoc.org/github.com/jmoiron/sqlx#DB.Unsafe) so that we can scan to structs // without worrying about newly introduced columns func QueryRow(db *sql.DB, stmt string) *sqlx.Row { - log.Print("[DEBUG] stmt ", stmt) + log.Print("[DEBUG] query stmt ", stmt) sdb := sqlx.NewDb(db, "snowflake").Unsafe() return sdb.QueryRowx(stmt) } diff --git a/pkg/snowflake/table.go b/pkg/snowflake/table.go index b9f9bf0682..bff26d04db 100644 --- a/pkg/snowflake/table.go +++ b/pkg/snowflake/table.go @@ -1,14 +1,13 @@ package snowflake import ( + "database/sql" "fmt" "strings" "github.com/jmoiron/sqlx" ) -//CREATE TABLE "FULFIL_TEST"."PUBLIC"."TABLENAME" ("C1" STRING); - // TableBuilder abstracts the creation of SQL queries for a Snowflake schema type TableBuilder struct { name string @@ -118,15 +117,16 @@ func (tb *TableBuilder) Show() string { } type table struct { - Name string `db:"name"` - Type string `db:"type"` - Kind string `db:"kind"` - Null string `db:"null?"` - PrimaryKey string `db:"primary key"` - UniqueKey string `db:"unique key"` - Check string `db:"check"` - Expression string `db:"expression"` - Comment string `db:"comment"` + Name string `db:"name"` + Type string `db:"type"` + Kind string `db:"kind"` + Null string `db:"null?"` + Default sql.NullString `db:"default"` + PrimaryKey string `db:"primary key"` + UniqueKey string `db:"unique key"` + Check sql.NullString `db:"check"` + Expression sql.NullString `db:"expression"` + Comment sql.NullString `db:"comment"` } func ScanTable(row *sqlx.Row) (*table, error) { diff --git a/pkg/snowflake/table_test.go b/pkg/snowflake/table_test.go index ca62178b2e..0c94287b1d 100644 --- a/pkg/snowflake/table_test.go +++ b/pkg/snowflake/table_test.go @@ -8,13 +8,13 @@ import ( func TestTableCreate(t *testing.T) { r := require.New(t) - s := TableWithColumnDefinition("test_table", "test_db", "test_schema", map[string]string{"column1": "OBJECT", "column2": "VARCHAR(255)"}) + s := TableWithColumnDefinition("test_table", "test_db", "test_schema", map[string]string{"column1": "OBJECT"}) r.Equal(s.QualifiedName(), `"test_db"."test_schema"."test_table"`) - r.Equal(s.Create(), `CREATE TABLE "test_db"."test_schema"."test_table" ("column1" OBJECT, "column2" VARCHAR(255))`) + r.Equal(s.Create(), `CREATE TABLE "test_db"."test_schema"."test_table" ("column1" OBJECT)`) s.WithComment("Test Comment") - r.Equal(s.Create(), `CREATE TABLE "test_db"."test_schema"."test_table" ("column1" OBJECT, "column2" VARCHAR(255)) COMMENT = 'Test Comment'`) + r.Equal(s.Create(), `CREATE TABLE "test_db"."test_schema"."test_table" ("column1" OBJECT) COMMENT = 'Test Comment'`) } func TestTableChangeComment(t *testing.T) { From 91142d77d7780aadec5452cf25d7d77b418a77bf Mon Sep 17 00:00:00 2001 From: John Peter Harvey Date: Tue, 18 Aug 2020 08:41:44 +0100 Subject: [PATCH 07/13] Improvements, still got one failing test --- docs/resources/table.md | 15 ++++++ go.sum | 2 - pkg/resources/table.go | 93 ++++--------------------------------- pkg/resources/table_test.go | 10 ++-- pkg/snowflake/table.go | 26 ++++++----- 5 files changed, 41 insertions(+), 105 deletions(-) create mode 100644 docs/resources/table.md diff --git a/docs/resources/table.md b/docs/resources/table.md new file mode 100644 index 0000000000..30975ae4b6 --- /dev/null +++ b/docs/resources/table.md @@ -0,0 +1,15 @@ + +# snowflake_table + + + +## properties + +| NAME | TYPE | DESCRIPTION | OPTIONAL | REQUIRED | COMPUTED | DEFAULT | +|----------|--------|-------------------------------------------------------------------------------------------------------------------|----------|-----------|----------|---------| +| columns | map | Map of the column names and types to create. e.g. { "column1" = "OBJECT", "column2" = "VARCHAR" } | false | true | false | | +| comment | string | Specifies a comment for the table. | true | false | false | | +| database | string | The database in which to create the table. | false | true | false | | +| name | string | Specifies the identifier for the table; must be unique for the database and schema in which the table is created. | false | true | false | | +| owner | string | Name of the role that owns the table. | false | false | true | | +| schema | string | The schema in which to create the table. | false | true | false | | diff --git a/go.sum b/go.sum index 14d3df05c9..25319129e9 100644 --- a/go.sum +++ b/go.sum @@ -16,8 +16,6 @@ github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03 github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo= github.com/DATA-DOG/go-sqlmock v1.4.1 h1:ThlnYciV1iM/V0OSF/dtkqWb6xo5qITT1TJBG1MRDJM= github.com/DATA-DOG/go-sqlmock v1.4.1/go.mod h1:f/Ixk793poVmq4qj/V1dPUg2JEAKC73Q5eFN3EC/SaM= -github.com/DATA-DOG/go-sqlmock v1.5.0 h1:Shsta01QNfFxHCfpW6YH2STWB0MudeXXEWMr20OEh60= -github.com/DATA-DOG/go-sqlmock v1.5.0/go.mod h1:f/Ixk793poVmq4qj/V1dPUg2JEAKC73Q5eFN3EC/SaM= github.com/DataDog/zstd v1.4.4/go.mod h1:1jcaCB/ufaK+sKp1NBhlGmpz41jOoPQ35bpF36t7BBo= github.com/ExpansiveWorlds/instrumentedsql v0.0.0-20171218214018-45abb4b1947d h1:r+whow+VHd9kAd4UTtQ/rtvcvmwkdryKUcGofpYOp+8= github.com/ExpansiveWorlds/instrumentedsql v0.0.0-20171218214018-45abb4b1947d/go.mod h1:Lm6NFlzU3HvZIo5l8GykZn6MH8/wq/A/X/d+7P/hgZU= diff --git a/pkg/resources/table.go b/pkg/resources/table.go index 284ae6c75f..4b073d86b2 100644 --- a/pkg/resources/table.go +++ b/pkg/resources/table.go @@ -21,7 +21,7 @@ var tableSchema = map[string]*schema.Schema{ Type: schema.TypeString, Required: true, ForceNew: true, - Description: "Specifies the identifier for the table; must be unique for the database and schema in which the pipe is created.", + Description: "Specifies the identifier for the table; must be unique for the database and schema in which the table is created.", }, "schema": { Type: schema.TypeString, @@ -39,7 +39,7 @@ var tableSchema = map[string]*schema.Schema{ Type: schema.TypeMap, Required: true, ForceNew: true, - Description: "The column names and types to create.", + Description: "Map of the column names and types to create. e.g. { \"column1\" = \"OBJECT\", \"column2\" = \"VARCHAR\" }", }, "comment": { Type: schema.TypeString, @@ -49,43 +49,7 @@ var tableSchema = map[string]*schema.Schema{ "owner": { Type: schema.TypeString, Computed: true, - Description: "Name of the role that owns the pipe.", - }, - "type": { - Type: schema.TypeString, - Computed: true, - Description: "Data type.", - }, - "kind": { - Type: schema.TypeString, - Computed: true, - Description: "Kind of object.", - }, - "null": { - Type: schema.TypeString, - Computed: true, - }, - "default": { - Type: schema.TypeString, - Computed: true, - }, - "primary_key": { - Type: schema.TypeBool, - Computed: true, - Description: "Is this a primary key?", - }, - "unique_key": { - Type: schema.TypeBool, - Computed: true, - Description: "Is this a unique key?", - }, - "check": { - Type: schema.TypeString, - Computed: true, - }, - "expression": { - Type: schema.TypeString, - Computed: true, + Description: "Name of the role that owns the table.", }, } @@ -170,9 +134,8 @@ func CreateTable(data *schema.ResourceData, meta interface{}) error { builder.WithComment(v.(string)) } - q := builder.Create() - - err := snowflake.Exec(db, q) + stmt := builder.Create() + err := snowflake.Exec(db, stmt) if err != nil { return errors.Wrapf(err, "error creating table %v", name) } @@ -203,8 +166,8 @@ func ReadTable(data *schema.ResourceData, meta interface{}) error { schema := tableID.SchemaName name := tableID.TableName - sq := snowflake.Table(name, dbName, schema).Show() - row := snowflake.QueryRow(db, sq) + stmt := snowflake.Table(name, dbName, schema).Show() + row := snowflake.QueryRow(db, stmt) table, err := snowflake.ScanTable(row) if err != nil { return err @@ -215,47 +178,7 @@ func ReadTable(data *schema.ResourceData, meta interface{}) error { return err } - err = data.Set("type", table.Type) - if err != nil { - return err - } - - err = data.Set("kind", table.Kind) - if err != nil { - return err - } - - err = data.Set("null", table.Null) - if err != nil { - return err - } - - err = data.Set("default", table.Default.String) - if err != nil { - return err - } - - err = data.Set("primary_key", table.PrimaryKey == "Y") - if err != nil { - return err - } - - err = data.Set("unique_key", table.UniqueKey == "Y") - if err != nil { - return err - } - - err = data.Set("check", table.Check.String) - if err != nil { - return err - } - - err = data.Set("expression", table.Expression.String) - if err != nil { - return err - } - - err = data.Set("comment", table.Comment.String) + err = data.Set("owner", table.Owner.String) if err != nil { return err } diff --git a/pkg/resources/table_test.go b/pkg/resources/table_test.go index 9476b4ca99..2621905bf0 100644 --- a/pkg/resources/table_test.go +++ b/pkg/resources/table_test.go @@ -2,14 +2,12 @@ package resources_test import ( "database/sql" - "fmt" "testing" sqlmock "github.com/DATA-DOG/go-sqlmock" "github.com/chanzuckerberg/terraform-provider-snowflake/pkg/provider" "github.com/chanzuckerberg/terraform-provider-snowflake/pkg/resources" . "github.com/chanzuckerberg/terraform-provider-snowflake/pkg/testhelpers" - "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/stretchr/testify/require" ) @@ -29,20 +27,20 @@ func TestTableCreate(t *testing.T) { "comment": "great comment", "columns": map[string]interface{}{"column1": "VARCHAR"}, } - d := schema.TestResourceDataRaw(t, resources.Table().Schema, in) - r.NotNil(d) + d := table(t, "database_name|schema_name|good_name", in) WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) { - fmt.Println(mock.ExpectExec(`CREATE TABLE "test_db"."test_schema"."test_name" ("column1" VARCHAR) COMMENT = 'great comment`).WillReturnResult(sqlmock.NewResult(1, 1))) + mock.ExpectExec(`CREATE TABLE "test_db"."test_schema"."test_name" ("column1" VARCHAR) COMMENT = 'great comment'`).WillReturnResult(sqlmock.NewResult(1, 1)) expectTableRead(mock) err := resources.CreateTable(d, db) r.NoError(err) + r.Equal("good_name", d.Get("name").(string)) }) } func expectTableRead(mock sqlmock.Sqlmock) { rows := sqlmock.NewRows([]string{"name", "type", "kind", "null?", "default", "primary key", "unique key", "check", "expression", "comment"}).AddRow("good_name", "VARCHAR()", "COLUMN", "Y", "NULL", "NULL", "N", "N", "NULL", "mock comment") - fmt.Println(mock.ExpectQuery(`SHOW TABLES LIKE 'good_name' IN DATABASE "database_name"`).WillReturnRows(rows)) + mock.ExpectQuery(`SHOW TABLES LIKE 'good_name' IN DATABASE "database_name"`).WillReturnRows(rows) } func TestTableRead(t *testing.T) { diff --git a/pkg/snowflake/table.go b/pkg/snowflake/table.go index bff26d04db..d284841944 100644 --- a/pkg/snowflake/table.go +++ b/pkg/snowflake/table.go @@ -78,9 +78,8 @@ func TableWithColumnDefinition(name, db, schema string, columns map[string]strin // Create returns the SQL statement required to create a table func (tb *TableBuilder) Create() string { q := strings.Builder{} - q.WriteString(`CREATE`) + q.WriteString(fmt.Sprintf(`CREATE TABLE %v`, tb.QualifiedName())) - q.WriteString(fmt.Sprintf(` TABLE %v`, tb.QualifiedName())) q.WriteString(fmt.Sprintf(` (`)) columnDefinitions := []string{} for columnName, columnType := range tb.columns { @@ -117,16 +116,19 @@ func (tb *TableBuilder) Show() string { } type table struct { - Name string `db:"name"` - Type string `db:"type"` - Kind string `db:"kind"` - Null string `db:"null?"` - Default sql.NullString `db:"default"` - PrimaryKey string `db:"primary key"` - UniqueKey string `db:"unique key"` - Check sql.NullString `db:"check"` - Expression sql.NullString `db:"expression"` - Comment sql.NullString `db:"comment"` + CreatedOn sql.NullString `db:"created_on"` + Name sql.NullString `db:"name"` + DatabaseName sql.NullString `db:"database_name"` + SchemaName sql.NullString `db:"schema_name"` + Kind sql.NullString `db:"kind"` + Comment sql.NullString `db:"comment"` + ClusterBy sql.NullString `db:"cluster_by"` + Rows sql.NullString `db:"row"` + Bytes sql.NullString `db:"bytes"` + Owner sql.NullString `db:"owner"` + RetentionTime sql.NullString `db:"retention_time"` + AutomaticClustering sql.NullString `db:"automatic_clustering"` + ChangeTracking sql.NullString `db:"change_tracking"` } func ScanTable(row *sqlx.Row) (*table, error) { From e7c58ebbf204b9c0bd6b5470ccd533a2145e5353 Mon Sep 17 00:00:00 2001 From: John Peter Harvey Date: Thu, 20 Aug 2020 21:24:19 +0100 Subject: [PATCH 08/13] Fix unit test --- pkg/resources/table.go | 4 ++-- pkg/resources/table_test.go | 11 ++++++----- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/pkg/resources/table.go b/pkg/resources/table.go index 4b073d86b2..381cad8e94 100644 --- a/pkg/resources/table.go +++ b/pkg/resources/table.go @@ -39,7 +39,7 @@ var tableSchema = map[string]*schema.Schema{ Type: schema.TypeMap, Required: true, ForceNew: true, - Description: "Map of the column names and types to create. e.g. { \"column1\" = \"OBJECT\", \"column2\" = \"VARCHAR\" }", + Description: "Map of the column names and types to create. e.g. { \"column1\" = \"VARIANT\", \"column2\" = \"VARCHAR\" }", }, "comment": { Type: schema.TypeString, @@ -173,7 +173,7 @@ func ReadTable(data *schema.ResourceData, meta interface{}) error { return err } - err = data.Set("name", table.Name) + err = data.Set("name", table.Name.String) if err != nil { return err } diff --git a/pkg/resources/table_test.go b/pkg/resources/table_test.go index 2621905bf0..2af835631c 100644 --- a/pkg/resources/table_test.go +++ b/pkg/resources/table_test.go @@ -21,16 +21,17 @@ func TestTableCreate(t *testing.T) { r := require.New(t) in := map[string]interface{}{ - "name": "test_name", - "database": "test_db", - "schema": "test_schema", + "name": "good_name", + "database": "database_name", + "schema": "schema_name", "comment": "great comment", "columns": map[string]interface{}{"column1": "VARCHAR"}, } d := table(t, "database_name|schema_name|good_name", in) WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) { - mock.ExpectExec(`CREATE TABLE "test_db"."test_schema"."test_name" ("column1" VARCHAR) COMMENT = 'great comment'`).WillReturnResult(sqlmock.NewResult(1, 1)) + // mock.ExpectQuery(`CREATE TABLE "test_db"."test_schema"."test_name" ("column1" VARCHAR) COMMENT = 'great comment'`) + mock.ExpectExec(`CREATE TABLE "database_name"."schema_name"."good_name" \("column1" VARCHAR\) COMMENT = 'great comment'`).WillReturnResult(sqlmock.NewResult(1, 1)) expectTableRead(mock) err := resources.CreateTable(d, db) r.NoError(err) @@ -46,7 +47,7 @@ func expectTableRead(mock sqlmock.Sqlmock) { func TestTableRead(t *testing.T) { r := require.New(t) - d := table(t, "database_name|schema_name|good_name", map[string]interface{}{"name": "good_name"}) + d := table(t, "database_name|schema_name|good_name", map[string]interface{}{"name": "good_name", "comment": "mock comment"}) WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) { expectTableRead(mock) From d6d2446fd49b39a6f81c67ce35c4ab2a4410153a Mon Sep 17 00:00:00 2001 From: John Peter Harvey Date: Thu, 20 Aug 2020 21:44:36 +0100 Subject: [PATCH 09/13] Documentation regeneration --- docs/resources/table.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/resources/table.md b/docs/resources/table.md index 30975ae4b6..4074c2c4e2 100644 --- a/docs/resources/table.md +++ b/docs/resources/table.md @@ -7,7 +7,7 @@ | NAME | TYPE | DESCRIPTION | OPTIONAL | REQUIRED | COMPUTED | DEFAULT | |----------|--------|-------------------------------------------------------------------------------------------------------------------|----------|-----------|----------|---------| -| columns | map | Map of the column names and types to create. e.g. { "column1" = "OBJECT", "column2" = "VARCHAR" } | false | true | false | | +| columns | map | Map of the column names and types to create. e.g. { "column1" = "VARIANT", "column2" = "VARCHAR" } | false | true | false | | | comment | string | Specifies a comment for the table. | true | false | false | | | database | string | The database in which to create the table. | false | true | false | | | name | string | Specifies the identifier for the table; must be unique for the database and schema in which the table is created. | false | true | false | | From 6aec38240ea39e99948ced79e7ebcf19c6508932 Mon Sep 17 00:00:00 2001 From: John Peter Harvey Date: Sat, 22 Aug 2020 17:35:19 +0100 Subject: [PATCH 10/13] Initial acceptance test, just verifying the basic --- pkg/resources/table_acceptance_test.go | 57 ++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 pkg/resources/table_acceptance_test.go diff --git a/pkg/resources/table_acceptance_test.go b/pkg/resources/table_acceptance_test.go new file mode 100644 index 0000000000..d1f8b5704d --- /dev/null +++ b/pkg/resources/table_acceptance_test.go @@ -0,0 +1,57 @@ +package resources_test + +import ( + "fmt" + "testing" + + "github.com/hashicorp/terraform-plugin-sdk/helper/acctest" + "github.com/hashicorp/terraform-plugin-sdk/helper/resource" +) + +func TestAccTable(t *testing.T) { + accName := acctest.RandStringFromCharSet(10, acctest.CharSetAlpha) + + resource.Test(t, resource.TestCase{ + Providers: providers(), + Steps: []resource.TestStep{ + { + Config: tableConfig(accName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_table.test_table", "database", accName), + resource.TestCheckResourceAttr("snowflake_table.test_table", "schema", accName), + resource.TestCheckResourceAttr("snowflake_table.test_table", "name", accName), + resource.TestCheckResourceAttr("snowflake_table.test_table", "comment", "Terraform acceptance test"), + checkBool("snowflake_table.test_table", "is_transient", false), // this is from user_acceptance_test.go + checkBool("snowflake_table.test_table", "is_managed", false), + ), + }, + }, + }) +} + +func tableConfig(name string) string { + s := ` +resource "snowflake_database" "test_database" { + name = "%v" + comment = "Terraform acceptance test" +} + +resource "snowflake_schema" "test_schema" { + name = "%v" + database = snowflake_database.test_database.name + comment = "Terraform acceptance test" +} + +resource "snowflake_table" "test_table" { + database = snowflake_database.test_database.name + schema = snowflake_schema.test_schema.name + name = "%v" + comment = "Terraform acceptance test" + columns = { + "column1" = "VARIANT", + "column2" = "VARCHAR" + } +} +` + return fmt.Sprintf(s, name) +} From 0da5218251cd67c0a1d4da6a7ec913adccfd173c Mon Sep 17 00:00:00 2001 From: John Peter Harvey Date: Mon, 24 Aug 2020 22:32:50 +0100 Subject: [PATCH 11/13] Fix acceptance test, change definition of resource to allow for future extension, as per review comment --- docs/resources/table.md | 2 +- pkg/resources/table.go | 42 ++++++++++++++++++++------ pkg/resources/table_acceptance_test.go | 22 ++++++++------ pkg/resources/table_test.go | 5 ++- pkg/snowflake/table.go | 16 +++++++--- pkg/snowflake/table_test.go | 7 +++-- 6 files changed, 63 insertions(+), 31 deletions(-) diff --git a/docs/resources/table.md b/docs/resources/table.md index 4074c2c4e2..d23939d044 100644 --- a/docs/resources/table.md +++ b/docs/resources/table.md @@ -7,7 +7,7 @@ | NAME | TYPE | DESCRIPTION | OPTIONAL | REQUIRED | COMPUTED | DEFAULT | |----------|--------|-------------------------------------------------------------------------------------------------------------------|----------|-----------|----------|---------| -| columns | map | Map of the column names and types to create. e.g. { "column1" = "VARIANT", "column2" = "VARCHAR" } | false | true | false | | +| column | list | Definitions of a column to create in the table. Minimum one required. | false | true | false | | | comment | string | Specifies a comment for the table. | true | false | false | | | database | string | The database in which to create the table. | false | true | false | | | name | string | Specifies the identifier for the table; must be unique for the database and schema in which the table is created. | false | true | false | | diff --git a/pkg/resources/table.go b/pkg/resources/table.go index 381cad8e94..a4da5bdd8c 100644 --- a/pkg/resources/table.go +++ b/pkg/resources/table.go @@ -35,11 +35,26 @@ var tableSchema = map[string]*schema.Schema{ ForceNew: true, Description: "The database in which to create the table.", }, - "columns": { - Type: schema.TypeMap, + "column": { + Type: schema.TypeList, Required: true, + MinItems: 1, ForceNew: true, - Description: "Map of the column names and types to create. e.g. { \"column1\" = \"VARIANT\", \"column2\" = \"VARCHAR\" }", + Description: "Definitions of a column to create in the table. Minimum one required.", + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "name": { + Type: schema.TypeString, + Required: true, + Description: "Column name", + }, + "type": { + Type: schema.TypeString, + Required: true, + Description: "Column type, e.g. VARIANT", + }, + }, + }, }, "comment": { Type: schema.TypeString, @@ -120,14 +135,23 @@ func CreateTable(data *schema.ResourceData, meta interface{}) error { database := data.Get("database").(string) schema := data.Get("schema").(string) name := data.Get("name").(string) - columns := data.Get("columns").(map[string]interface{}) - columnDefinitions := map[string]string{} - for columnName, columnType := range columns { - columnDefinitions[columnName] = columnType.(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.TableWithColumnDefinitions(name, database, schema, columns) - builder := snowflake.TableWithColumnDefinition(name, database, schema, columnDefinitions) + // if len(columns) < 1 { + // return errors.New("At least one column definition is required") + // } + // builder.WithColumns(columns) // Set optionals if v, ok := data.GetOk("comment"); ok { @@ -173,7 +197,7 @@ func ReadTable(data *schema.ResourceData, meta interface{}) error { return err } - err = data.Set("name", table.Name.String) + err = data.Set("name", table.TableName.String) if err != nil { return err } diff --git a/pkg/resources/table_acceptance_test.go b/pkg/resources/table_acceptance_test.go index d1f8b5704d..6c85a3f4fe 100644 --- a/pkg/resources/table_acceptance_test.go +++ b/pkg/resources/table_acceptance_test.go @@ -17,12 +17,10 @@ func TestAccTable(t *testing.T) { { Config: tableConfig(accName), Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_table.test_table", "name", accName), resource.TestCheckResourceAttr("snowflake_table.test_table", "database", accName), resource.TestCheckResourceAttr("snowflake_table.test_table", "schema", accName), - resource.TestCheckResourceAttr("snowflake_table.test_table", "name", accName), resource.TestCheckResourceAttr("snowflake_table.test_table", "comment", "Terraform acceptance test"), - checkBool("snowflake_table.test_table", "is_transient", false), // this is from user_acceptance_test.go - checkBool("snowflake_table.test_table", "is_managed", false), ), }, }, @@ -32,12 +30,12 @@ func TestAccTable(t *testing.T) { func tableConfig(name string) string { s := ` resource "snowflake_database" "test_database" { - name = "%v" + name = "%s" comment = "Terraform acceptance test" } resource "snowflake_schema" "test_schema" { - name = "%v" + name = "%s" database = snowflake_database.test_database.name comment = "Terraform acceptance test" } @@ -45,13 +43,17 @@ resource "snowflake_schema" "test_schema" { resource "snowflake_table" "test_table" { database = snowflake_database.test_database.name schema = snowflake_schema.test_schema.name - name = "%v" + name = "%s" comment = "Terraform acceptance test" - columns = { - "column1" = "VARIANT", - "column2" = "VARCHAR" + column { + name = "column1" + type = "VARIANT" + } + column { + name = "column2" + type = "VARCHAR" } } ` - return fmt.Sprintf(s, name) + return fmt.Sprintf(s, name, name, name) } diff --git a/pkg/resources/table_test.go b/pkg/resources/table_test.go index 2af835631c..42f33619c0 100644 --- a/pkg/resources/table_test.go +++ b/pkg/resources/table_test.go @@ -25,13 +25,12 @@ func TestTableCreate(t *testing.T) { "database": "database_name", "schema": "schema_name", "comment": "great comment", - "columns": map[string]interface{}{"column1": "VARCHAR"}, + "column": []interface{}{map[string]interface{}{"name": "column1", "type": "OBJECT"}, map[string]interface{}{"name": "column2", "type": "VARCHAR"}}, } d := table(t, "database_name|schema_name|good_name", in) WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) { - // mock.ExpectQuery(`CREATE TABLE "test_db"."test_schema"."test_name" ("column1" VARCHAR) COMMENT = 'great comment'`) - mock.ExpectExec(`CREATE TABLE "database_name"."schema_name"."good_name" \("column1" VARCHAR\) COMMENT = 'great comment'`).WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec(`CREATE TABLE "database_name"."schema_name"."good_name" \("column1" OBJECT, "column2" VARCHAR\) COMMENT = 'great comment'`).WillReturnResult(sqlmock.NewResult(1, 1)) expectTableRead(mock) err := resources.CreateTable(d, db) r.NoError(err) diff --git a/pkg/snowflake/table.go b/pkg/snowflake/table.go index d284841944..767e4ccc5c 100644 --- a/pkg/snowflake/table.go +++ b/pkg/snowflake/table.go @@ -13,7 +13,7 @@ type TableBuilder struct { name string db string schema string - columns map[string]string + columns []map[string]string comment string } @@ -44,6 +44,12 @@ func (tb *TableBuilder) WithComment(c string) *TableBuilder { return tb } +// WithColumns sets the column definitions on the TableBuilder +func (tb *TableBuilder) WithColumns(c []map[string]string) *TableBuilder { + tb.columns = c + return tb +} + // Table returns a pointer to a Builder that abstracts the DDL operations for a table. // // Supported DDL operations are: @@ -66,7 +72,7 @@ func Table(name, db, schema string) *TableBuilder { // - CREATE TABLE // // [Snowflake Reference](https://docs.snowflake.com/en/sql-reference/ddl-table.html) -func TableWithColumnDefinition(name, db, schema string, columns map[string]string) *TableBuilder { +func TableWithColumnDefinitions(name, db, schema string, columns []map[string]string) *TableBuilder { return &TableBuilder{ name: name, db: db, @@ -82,8 +88,8 @@ func (tb *TableBuilder) Create() string { q.WriteString(fmt.Sprintf(` (`)) columnDefinitions := []string{} - for columnName, columnType := range tb.columns { - columnDefinitions = append(columnDefinitions, fmt.Sprintf(`"%v" %v`, columnName, columnType)) + for _, columnDefinition := range tb.columns { + columnDefinitions = append(columnDefinitions, fmt.Sprintf(`"%v" %v`, columnDefinition["name"], columnDefinition["type"])) } q.WriteString(strings.Join(columnDefinitions, ", ")) q.WriteString(fmt.Sprintf(`)`)) @@ -117,7 +123,7 @@ func (tb *TableBuilder) Show() string { type table struct { CreatedOn sql.NullString `db:"created_on"` - Name sql.NullString `db:"name"` + TableName sql.NullString `db:"name"` DatabaseName sql.NullString `db:"database_name"` SchemaName sql.NullString `db:"schema_name"` Kind sql.NullString `db:"kind"` diff --git a/pkg/snowflake/table_test.go b/pkg/snowflake/table_test.go index 0c94287b1d..7479a0109e 100644 --- a/pkg/snowflake/table_test.go +++ b/pkg/snowflake/table_test.go @@ -8,13 +8,14 @@ import ( func TestTableCreate(t *testing.T) { r := require.New(t) - s := TableWithColumnDefinition("test_table", "test_db", "test_schema", map[string]string{"column1": "OBJECT"}) + s := Table("test_table", "test_db", "test_schema") + s.WithColumns([]map[string]string{{"name": "column1", "type": "OBJECT"}, {"name": "column2", "type": "VARCHAR"}}) r.Equal(s.QualifiedName(), `"test_db"."test_schema"."test_table"`) - r.Equal(s.Create(), `CREATE TABLE "test_db"."test_schema"."test_table" ("column1" OBJECT)`) + r.Equal(`CREATE TABLE "test_db"."test_schema"."test_table" ("column1" OBJECT, "column2" VARCHAR)`, s.Create()) s.WithComment("Test Comment") - r.Equal(s.Create(), `CREATE TABLE "test_db"."test_schema"."test_table" ("column1" OBJECT) COMMENT = 'Test Comment'`) + r.Equal(s.Create(), `CREATE TABLE "test_db"."test_schema"."test_table" ("column1" OBJECT, "column2" VARCHAR) COMMENT = 'Test Comment'`) } func TestTableChangeComment(t *testing.T) { From 174264b134651d22598983726a59c0c741cdc02d Mon Sep 17 00:00:00 2001 From: John Peter Harvey Date: Sun, 6 Sep 2020 17:56:37 +0100 Subject: [PATCH 12/13] Remove commented out lines --- pkg/resources/table.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pkg/resources/table.go b/pkg/resources/table.go index a4da5bdd8c..fe266e7910 100644 --- a/pkg/resources/table.go +++ b/pkg/resources/table.go @@ -148,11 +148,6 @@ func CreateTable(data *schema.ResourceData, meta interface{}) error { } builder := snowflake.TableWithColumnDefinitions(name, database, schema, columns) - // if len(columns) < 1 { - // return errors.New("At least one column definition is required") - // } - // builder.WithColumns(columns) - // Set optionals if v, ok := data.GetOk("comment"); ok { builder.WithComment(v.(string)) From 9900d8a7b5a8bfd6214b621dd916266f61651f4c Mon Sep 17 00:00:00 2001 From: John Peter Harvey Date: Fri, 2 Oct 2020 11:34:58 +0100 Subject: [PATCH 13/13] Escape the definitions of columns --- pkg/snowflake/table.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/snowflake/table.go b/pkg/snowflake/table.go index 767e4ccc5c..dd0bf995ee 100644 --- a/pkg/snowflake/table.go +++ b/pkg/snowflake/table.go @@ -89,7 +89,7 @@ func (tb *TableBuilder) Create() string { q.WriteString(fmt.Sprintf(` (`)) columnDefinitions := []string{} for _, columnDefinition := range tb.columns { - columnDefinitions = append(columnDefinitions, fmt.Sprintf(`"%v" %v`, columnDefinition["name"], columnDefinition["type"])) + columnDefinitions = append(columnDefinitions, fmt.Sprintf(`"%v" %v`, EscapeString(columnDefinition["name"]), EscapeString(columnDefinition["type"]))) } q.WriteString(strings.Join(columnDefinitions, ", ")) q.WriteString(fmt.Sprintf(`)`))