Skip to content

Commit

Permalink
chore: Update readme and objects rework state (#3046)
Browse files Browse the repository at this point in the history
- Update list of essential objects rework
- Update the known issues for the finished objects
- Fix the database behavior
- Add logging
- Fix formatting and linter problems
- Add (unfinished) list of the unfinished topics
  • Loading branch information
sfc-gh-asawicki authored Sep 5, 2024
1 parent 2844a30 commit 5802dca
Show file tree
Hide file tree
Showing 11 changed files with 112 additions and 39 deletions.
3 changes: 3 additions & 0 deletions pkg/resources/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package resources
import (
"context"
"fmt"
"log"
"strings"
"time"

Expand Down Expand Up @@ -299,6 +300,7 @@ func CreateAccount(d *schema.ResourceData, meta interface{}) error {
err = util.Retry(5, 3*time.Second, func() (error, bool) {
account, err = client.Accounts.ShowByID(ctx, objectIdentifier)
if err != nil {
log.Printf("[DEBUG] retryable operation resulted in error: %v\n", err)
return nil, false
}
return nil, true
Expand All @@ -323,6 +325,7 @@ func ReadAccount(d *schema.ResourceData, meta interface{}) error {
err = util.Retry(5, 3*time.Second, func() (error, bool) {
acc, err = client.Accounts.ShowByID(ctx, id)
if err != nil {
log.Printf("[DEBUG] retryable operation resulted in error: %v\n", err)
return nil, false
}
return nil, true
Expand Down
4 changes: 2 additions & 2 deletions pkg/resources/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -366,12 +366,12 @@ func ReadDatabase(ctx context.Context, d *schema.ResourceData, meta any) diag.Di

database, err := client.Databases.ShowByID(ctx, id)
if err != nil {
if errors.Is(err, sdk.ErrObjectNotFound) {
if errors.Is(err, sdk.ErrObjectNotExistOrAuthorized) || errors.Is(err, sdk.ErrObjectNotFound) {
d.SetId("")
return diag.Diagnostics{
diag.Diagnostic{
Severity: diag.Warning,
Summary: "Failed to query secondary database. Marking the resource as removed.",
Summary: "Failed to query database. Marking the resource as removed.",
Detail: fmt.Sprintf("DatabaseName: %s, Err: %s", id.FullyQualifiedName(), err),
},
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/resources/managed_account.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package resources
import (
"context"
"fmt"
"log"
"time"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers"
Expand Down Expand Up @@ -137,6 +138,7 @@ func ReadManagedAccount(d *schema.ResourceData, meta interface{}) error {
err = util.Retry(5, 3*time.Second, func() (error, bool) {
managedAccount, err = client.ManagedAccounts.ShowByID(ctx, id)
if err != nil {
log.Printf("[DEBUG] retryable operation resulted in error: %v\n", err)
return nil, false
}
return nil, true
Expand Down
67 changes: 56 additions & 11 deletions pkg/resources/schema_acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,24 @@ import (
"strconv"
"testing"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers"

acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance"
acchelpers "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/helpers"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/planchecks"
r "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/resources"
"github.com/stretchr/testify/require"
tfjson "github.com/hashicorp/terraform-json"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/importchecks"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/planchecks"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/testenvs"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
tfjson "github.com/hashicorp/terraform-json"
"github.com/hashicorp/terraform-plugin-testing/config"
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
"github.com/hashicorp/terraform-plugin-testing/plancheck"
"github.com/hashicorp/terraform-plugin-testing/terraform"
"github.com/hashicorp/terraform-plugin-testing/tfversion"
"github.com/stretchr/testify/require"
)

func TestAcc_Schema_basic(t *testing.T) {
Expand Down Expand Up @@ -788,7 +788,7 @@ func TestAcc_Schema_DefaultDataRetentionTime_SetOutsideOfTerraform(t *testing.T)
})
}

func TestAcc_Schema_RemoveDatabaseOutsideOfTerraform(t *testing.T) {
func TestAcc_Schema_RemoveSchemaOutsideOfTerraform(t *testing.T) {
schemaId := acc.TestClient().Ids.RandomDatabaseObjectIdentifier()
configVariables := map[string]config.Variable{
"schema_name": config.StringVariable(schemaId.Name()),
Expand Down Expand Up @@ -823,13 +823,12 @@ func TestAcc_Schema_RemoveDatabaseOutsideOfTerraform(t *testing.T) {
})
}

func TestAcc_Schema_RemoveSchemaOutsideOfTerraform(t *testing.T) {
func TestAcc_Schema_RemoveDatabaseOutsideOfTerraform(t *testing.T) {
databaseId := acc.TestClient().Ids.RandomAccountObjectIdentifier()
databaseName := databaseId.Name()
schemaName := acc.TestClient().Ids.Alpha()
schemaId := acc.TestClient().Ids.RandomDatabaseObjectIdentifierInDatabase(databaseId)
configVariables := map[string]config.Variable{
"schema_name": config.StringVariable(schemaName),
"database_name": config.StringVariable(databaseName),
"database_name": config.StringVariable(databaseId.Name()),
"schema_name": config.StringVariable(schemaId.Name()),
}

var cleanupDatabase func()
Expand Down Expand Up @@ -863,6 +862,52 @@ func TestAcc_Schema_RemoveSchemaOutsideOfTerraform(t *testing.T) {
})
}

func TestAcc_Schema_RemoveDatabaseOutsideOfTerraform_dbInConfig(t *testing.T) {
databaseId := acc.TestClient().Ids.RandomAccountObjectIdentifier()
schemaId := acc.TestClient().Ids.RandomDatabaseObjectIdentifierInDatabase(databaseId)
configVariables := map[string]config.Variable{
"database_name": config.StringVariable(databaseId.Name()),
"schema_name": config.StringVariable(schemaId.Name()),
}

resource.Test(t, resource.TestCase{
ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
PreCheck: func() { acc.TestAccPreCheck(t) },
TerraformVersionChecks: []tfversion.TerraformVersionCheck{
tfversion.RequireAbove(tfversion.Version1_5_0),
},
CheckDestroy: acc.CheckDestroy(t, resources.Schema),
Steps: []resource.TestStep{
{
ConfigDirectory: acc.ConfigurationDirectory("TestAcc_Schema_RemoveOutsideOfTerraform_dbInConfig"),
ConfigVariables: configVariables,
Check: assert.AssertThat(t,
assert.Check(resource.TestCheckResourceAttr("snowflake_database.test", "name", databaseId.Name())),
assert.Check(resource.TestCheckResourceAttr("snowflake_schema.test", "name", schemaId.Name())),
),
},
{
PreConfig: func() {
err := acc.TestClient().Database.DropDatabase(t, databaseId)
require.NoError(t, err)
},
ConfigPlanChecks: resource.ConfigPlanChecks{
PreApply: []plancheck.PlanCheck{
plancheck.ExpectResourceAction("snowflake_database.test", plancheck.ResourceActionCreate),
plancheck.ExpectResourceAction("snowflake_schema.test", plancheck.ResourceActionCreate),
},
},
ConfigDirectory: acc.ConfigurationDirectory("TestAcc_Schema_RemoveOutsideOfTerraform_dbInConfig"),
ConfigVariables: configVariables,
Check: assert.AssertThat(t,
assert.Check(resource.TestCheckResourceAttr("snowflake_database.test", "name", databaseId.Name())),
assert.Check(resource.TestCheckResourceAttr("snowflake_schema.test", "name", schemaId.Name())),
),
},
},
})
}

func checkDatabaseAndSchemaDataRetentionTime(t *testing.T, schemaId sdk.DatabaseObjectIdentifier, expectedDatabaseRetentionsDays int, expectedSchemaRetentionDays int) func(state *terraform.State) error {
t.Helper()
return func(state *terraform.State) error {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
resource "snowflake_database" "test" {
name = var.database_name
}

resource "snowflake_schema" "test" {
database = snowflake_database.test.fully_qualified_name
name = var.schema_name
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
variable "schema_name" {
type = string
}

variable "database_name" {
type = string
}
6 changes: 0 additions & 6 deletions pkg/sdk/external_volumes_impl_gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,9 @@ func (r *AlterExternalVolumeRequest) toOpts() *AlterExternalVolumeOptions {
}

if r.AddStorageLocation != nil {

opts.AddStorageLocation = &ExternalVolumeStorageLocation{}

if r.AddStorageLocation.S3StorageLocationParams != nil {

opts.AddStorageLocation.S3StorageLocationParams = &S3StorageLocationParams{
Name: r.AddStorageLocation.S3StorageLocationParams.Name,
StorageProvider: r.AddStorageLocation.S3StorageLocationParams.StorageProvider,
Expand All @@ -104,11 +102,9 @@ func (r *AlterExternalVolumeRequest) toOpts() *AlterExternalVolumeOptions {
KmsKeyId: r.AddStorageLocation.S3StorageLocationParams.Encryption.KmsKeyId,
}
}

}

if r.AddStorageLocation.GCSStorageLocationParams != nil {

opts.AddStorageLocation.GCSStorageLocationParams = &GCSStorageLocationParams{
Name: r.AddStorageLocation.GCSStorageLocationParams.Name,
StorageBaseUrl: r.AddStorageLocation.GCSStorageLocationParams.StorageBaseUrl,
Expand All @@ -120,7 +116,6 @@ func (r *AlterExternalVolumeRequest) toOpts() *AlterExternalVolumeOptions {
KmsKeyId: r.AddStorageLocation.GCSStorageLocationParams.Encryption.KmsKeyId,
}
}

}

if r.AddStorageLocation.AzureStorageLocationParams != nil {
Expand All @@ -130,7 +125,6 @@ func (r *AlterExternalVolumeRequest) toOpts() *AlterExternalVolumeOptions {
StorageBaseUrl: r.AddStorageLocation.AzureStorageLocationParams.StorageBaseUrl,
}
}

}

return opts
Expand Down
20 changes: 12 additions & 8 deletions pkg/sdk/testint/external_volumes_gen_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,11 @@ func TestInt_ExternalVolumes(t *testing.T) {
var externalVolumePropNameValue []ExternalVolumePropNameValue
for _, p := range props {
if strings.Contains(p.Name, "STORAGE_LOCATION_") {
if strings.Contains(p.Value, `"STORAGE_PROVIDER":"S3"`) {
switch {
case strings.Contains(p.Value, `"STORAGE_PROVIDER":"S3"`):
s3StorageLocation := S3StorageLocation{}
json.Unmarshal([]byte(p.Value), &s3StorageLocation)
err := json.Unmarshal([]byte(p.Value), &s3StorageLocation)
require.NoError(t, err)
s3StorageLocationTrimmed := S3StorageLocationTrimmed{
Name: s3StorageLocation.Name,
StorageProvider: s3StorageLocation.StorageProvider,
Expand All @@ -125,9 +127,10 @@ func TestInt_ExternalVolumes(t *testing.T) {
externalVolumePropNameValue,
ExternalVolumePropNameValue{Name: p.Name, Value: string(s3StorageLocationTrimmedMarshaled)},
)
} else if strings.Contains(p.Value, `"STORAGE_PROVIDER":"GCS"`) {
case strings.Contains(p.Value, `"STORAGE_PROVIDER":"GCS"`):
gcsStorageLocation := GCSStorageLocation{}
json.Unmarshal([]byte(p.Value), &gcsStorageLocation)
err := json.Unmarshal([]byte(p.Value), &gcsStorageLocation)
require.NoError(t, err)
gcsStorageLocationTrimmed := GCSStorageLocationTrimmed{
Name: gcsStorageLocation.Name,
StorageProvider: gcsStorageLocation.StorageProvider,
Expand All @@ -141,9 +144,10 @@ func TestInt_ExternalVolumes(t *testing.T) {
externalVolumePropNameValue,
ExternalVolumePropNameValue{Name: p.Name, Value: string(gcsStorageLocationTrimmedMarshaled)},
)
} else if strings.Contains(p.Value, `"STORAGE_PROVIDER":"AZURE"`) {
case strings.Contains(p.Value, `"STORAGE_PROVIDER":"AZURE"`):
azureStorageLocation := AzureStorageLocation{}
json.Unmarshal([]byte(p.Value), &azureStorageLocation)
err := json.Unmarshal([]byte(p.Value), &azureStorageLocation)
require.NoError(t, err)
azureStorageLocationTrimmed := AzureStorageLocationTrimmed{
Name: azureStorageLocation.Name,
StorageProvider: azureStorageLocation.StorageProvider,
Expand All @@ -156,8 +160,8 @@ func TestInt_ExternalVolumes(t *testing.T) {
externalVolumePropNameValue,
ExternalVolumePropNameValue{Name: p.Name, Value: string(azureStorageLocationTrimmedMarshaled)},
)
} else {
panic("Unrecognised storage provider in storage location property")
default:
panic("Unrecognized storage provider in storage location property")
}
} else {
externalVolumePropNameValue = append(externalVolumePropNameValue, ExternalVolumePropNameValue{Name: p.Name, Value: p.Value})
Expand Down
9 changes: 7 additions & 2 deletions pkg/sdk/warehouses.go
Original file line number Diff line number Diff line change
Expand Up @@ -329,15 +329,20 @@ func (c *warehouses) Alter(ctx context.Context, id AccountObjectIdentifier, opts
}()

// needed to make sure that warehouse is suspended
var warehouseSuspensionErrs []error
err = util.Retry(3, 1*time.Second, func() (error, bool) {
warehouse, err = c.ShowByID(ctx, id)
if err != nil || warehouse.State != WarehouseStateSuspended {
if err != nil {
warehouseSuspensionErrs = append(warehouseSuspensionErrs, err)
return nil, false
}
if warehouse.State != WarehouseStateSuspended {
return nil, false
}
return nil, true
})
if err != nil {
return err
return fmt.Errorf("warehouse suspension failed, err: %w, original errors: %w", err, errors.Join(warehouseSuspensionErrs...))
}
}
}
Expand Down
Loading

0 comments on commit 5802dca

Please sign in to comment.