Skip to content

Commit

Permalink
fix: be more cautious when setting db id to empty (#1725)
Browse files Browse the repository at this point in the history 
* be more cautious when setting db id to empty

* database read

* fix tests to allow for two reads

* Update database.go

* format

* Delete object_type_helpers.go

* Update snowflake.go

* Update password_policy.go
  • Loading branch information
@sfc-gh-swinkler
sfc-gh-swinkler authored Apr 19, 2023
1 parent 791dd0b commit e78e0c8
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 18 deletions.
41 changes: 26 additions & 15 deletions pkg/resources/database.go
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package resources

import (
"context"
"database/sql"
"errors"
"fmt"
Expand All @@ -19,7 +18,6 @@ var databaseSchema = map[string]*schema.Schema{
"name": {
Type: schema.TypeString,
Required: true,
ForceNew: false,
},
"comment": {
Type: schema.TypeString,
Expand All @@ -37,7 +35,7 @@ var databaseSchema = map[string]*schema.Schema{
Type: schema.TypeInt,
Optional: true,
Description: "Number of days for which Snowflake retains historical data for performing Time Travel actions (SELECT, CLONE, UNDROP) on the object. A value of 0 effectively disables Time Travel for the specified database, schema, or table. For more information, see Understanding & Using Time Travel.",
Computed: true,
Default: 1,
},
"from_share": {
Type: schema.TypeMap,
Expand Down Expand Up @@ -95,12 +93,7 @@ func Database() *schema.Resource {

Schema: databaseSchema,
Importer: &schema.ResourceImporter{
StateContext: func(ctx context.Context, d *schema.ResourceData, m interface{}) ([]*schema.ResourceData, error) {
if err := d.Set("name", d.Id()); err != nil {
return nil, err
}
return []*schema.ResourceData{d}, nil
},
StateContext: schema.ImportStatePassthroughContext,
},
}
}
Expand Down Expand Up @@ -227,11 +220,26 @@ func createDatabaseFromReplica(d *schema.ResourceData, meta interface{}) error {
func ReadDatabase(d *schema.ResourceData, meta interface{}) error {
db := meta.(*sql.DB)
dbx := sqlx.NewDb(db, "snowflake")
database, err := snowflake.ListDatabase(dbx, d.Get("name").(string))
name := d.Id()

// perform a "show database" command to ensure that the database is actually there.
stmt := snowflake.NewDatabaseBuilder(name).Show()
row := snowflake.QueryRow(db, stmt)
_, err := snowflake.ScanDatabase(row)
if err != nil {
if errors.Is(err, sql.ErrNoRows) {
// If not found, mark resource to be removed from statefile during apply or refresh
log.Printf("[DEBUG] database (%s) not found", name)
d.SetId("")
return nil
}
return fmt.Errorf("unable to scan row for SHOW DATABASES")
}

// there may be more than one database found, so we need to filter. this could probably be combined with the above query
database, err := snowflake.ListDatabase(dbx, name)
if err != nil {
log.Println("[DEBUG] list database failed to decode")
d.SetId("")
return nil
return err
}
if err := d.Set("name", database.DBName.String); err != nil {
return err
Expand All @@ -244,6 +252,9 @@ func ReadDatabase(d *schema.ResourceData, meta interface{}) error {
if err != nil {
return err
}
if err := d.Set("data_retention_time_in_days", i); err != nil {
return err
}

// reset the options before reading back from the DB
if err := d.Set("is_transient", false); err != nil {
Expand All @@ -260,7 +271,7 @@ func ReadDatabase(d *schema.ResourceData, meta interface{}) error {
}
}

return d.Set("data_retention_time_in_days", i)
return nil
}

func UpdateDatabase(d *schema.ResourceData, meta interface{}) error {
Expand Down Expand Up @@ -307,7 +318,7 @@ func UpdateDatabase(d *schema.ResourceData, meta interface{}) error {
if err := snowflake.Exec(db, q); err != nil {
return fmt.Errorf("error updating database name on %v err = %w", d.Id(), err)
}
d.SetId(fmt.Sprintf("%v", name.(string)))
d.SetId(name.(string))
}

if d.HasChange("comment") {
Expand Down
14 changes: 11 additions & 3 deletions pkg/resources/database_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ func TestDatabaseCreate(t *testing.T) {
r.NotNil(d)

WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) {
mock.ExpectExec(`CREATE DATABASE "tst-terraform-good_name" COMMENT = 'great comment'`).WillReturnResult(sqlmock.NewResult(1, 1))
mock.ExpectExec(`CREATE DATABASE "tst-terraform-good_name" DATA_RETENTION_TIME_IN_DAYS = 1 COMMENT = 'great comment'`).WillReturnResult(sqlmock.NewResult(1, 1))
expectRead(mock)
expectRead(mock)
err := resources.CreateDatabase(d, db)
r.NoError(err)
Expand All @@ -52,9 +53,10 @@ func TestDatabase_Create_WithValidReplicationConfiguration(t *testing.T) {
r.NotNil(d)

WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) {
mock.ExpectExec(`CREATE DATABASE "tst-terraform-good_name" COMMENT = 'great comment'`).WillReturnResult(sqlmock.NewResult(1, 1))
mock.ExpectExec(`CREATE DATABASE "tst-terraform-good_name" DATA_RETENTION_TIME_IN_DAYS = 1 COMMENT = 'great comment'`).WillReturnResult(sqlmock.NewResult(1, 1))
mock.ExpectExec(`ALTER DATABASE "tst-terraform-good_name" ENABLE REPLICATION TO ACCOUNTS account1, account2`).WillReturnResult(sqlmock.NewResult(1, 1))
expectRead(mock)
expectRead(mock)

err := resources.CreateDatabase(d, db)
r.NoError(err)
Expand Down Expand Up @@ -94,6 +96,7 @@ func TestDatabaseRead(t *testing.T) {
})

WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) {
expectRead(mock)
expectRead(mock)
err := resources.ReadDatabase(d, db)
r.NoError(err)
Expand Down Expand Up @@ -131,6 +134,7 @@ func TestDatabaseCreateFromShare(t *testing.T) {
WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) {
mock.ExpectExec(`CREATE DATABASE "tst-terraform-good_name" FROM SHARE "abc123"."my_share"`).WillReturnResult(sqlmock.NewResult(1, 1))
expectRead(mock)
expectRead(mock)
err := resources.CreateDatabase(d, db)
r.NoError(err)
})
Expand All @@ -149,6 +153,7 @@ func TestDatabaseCreateFromDatabase(t *testing.T) {
WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) {
mock.ExpectExec(`CREATE DATABASE "tst-terraform-good_name" CLONE "abc123"`).WillReturnResult(sqlmock.NewResult(1, 1))
expectRead(mock)
expectRead(mock)
err := resources.CreateDatabase(d, db)
r.NoError(err)
})
Expand All @@ -167,6 +172,7 @@ func TestDatabaseCreateFromReplica(t *testing.T) {
WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) {
mock.ExpectExec(`CREATE DATABASE "tst-terraform-good_name" AS REPLICA OF "abc123"`).WillReturnResult(sqlmock.NewResult(1, 1))
expectRead(mock)
expectRead(mock)
err := resources.CreateDatabase(d, db)
r.NoError(err)
})
Expand All @@ -184,7 +190,8 @@ func TestDatabaseCreateTransient(t *testing.T) {
r.NotNil(d)

WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) {
mock.ExpectExec(`CREATE TRANSIENT DATABASE "tst-terraform-good_name" COMMENT = 'great comment'`).WillReturnResult(sqlmock.NewResult(1, 1))
mock.ExpectExec(`CREATE TRANSIENT DATABASE "tst-terraform-good_name" DATA_RETENTION_TIME_IN_DAYS = 1 COMMENT = 'great comment'`).WillReturnResult(sqlmock.NewResult(1, 1))
expectRead(mock)
expectRead(mock)
err := resources.CreateDatabase(d, db)
r.NoError(err)
Expand All @@ -205,6 +212,7 @@ func TestDatabaseCreateTransientFromDatabase(t *testing.T) {
WithMockDb(t, func(db *sql.DB, mock sqlmock.Sqlmock) {
mock.ExpectExec(`CREATE TRANSIENT DATABASE "tst-terraform-good_name" CLONE "abc123"`).WillReturnResult(sqlmock.NewResult(1, 1))
expectRead(mock)
expectRead(mock)
err := resources.CreateDatabase(d, db)
r.NoError(err)
})
Expand Down

0 comments on commit e78e0c8

Please sign in to comment.