Skip to content

Commit

Permalink
chore: Continue random ids rework (#2819)
Browse files Browse the repository at this point in the history
Continuation of #2747 (topics connected with
`NewAccountObjectIdentifier`):
- `alterMaterializedViewQueryExternally` - moved to test helper
- `checkDatabaseAndSchemaDataRetentionTime` - id part reworked, method
left for assertions introduction
- `checkDatabaseSchemaAndTableDataRetentionTime` - id part reworked,
method left for assertions introduction
- `unsafe_execute_acceptance_test.go` - id part reworked, some methods
left for assertions introduction
- `alterViewOwnershipExternally` - moved to test helper
- check `app_role_1` - test role name extracted and parametrized in
tests
- check hardcoded `filter_by_role`, `stproc1`, `sp_pi`, `filterByRole`,
`records` - added to procedure rework
- ask @sfc-gh-jcieslak about
`sdk.NewAccountObjectIdentifier("S3_STORAGE_INTEGRATION")`,
`sdk.NewAccountObjectIdentifier("GCP_STORAGE_INTEGRATION")`,
`sdk.NewAccountObjectIdentifier("AZURE_STORAGE_INTEGRATION")` -
precreated integrations extracted
- extract common helpers for
`sdk.NewAccountObjectIdentifier("does_not_exist")` and similar -
extracted
- Fixed setup for multiple acceptance tests (to use random ids not
random strings)

Still left (connected with `NewAccountObjectIdentifier`):
- `CreateDatabaseWithName` (and other similar helpers)
- check `setup_test.go`

Additional changes:
- changed return type for `CurrentRole` and `CurrentUser` to ids
- Fixed ShowByID for application roles
- Added FAQ entry about possible identifier error
- Introduced `DatabaseId()` and `SchemaId()` helpers to identifiers and
convenience methods to use them
- Introduced some additional placeholders (like role ids for
ACCOUNTADMIN and ORGADMIN)
- Moved check destroy to a common place
  • Loading branch information
sfc-gh-asawicki authored May 20, 2024
1 parent 751e5d3 commit f20940c
Show file tree
Hide file tree
Showing 107 changed files with 745 additions and 636 deletions.
14 changes: 14 additions & 0 deletions CREATING_ISSUES.md
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,20 @@ which is highly recommended for large-scale migrations.

**Solution:** Some fields may expect different types of identifiers, when in doubt check [our documentation](https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs) for the field or the [official Snowflake documentation](https://docs.snowflake.com/) what type of identifier is needed.

### Incorrect identifier type (panic: interface conversion)
**Problem** When getting stack traces similar to:
```text
panic: interface conversion: sdk.ObjectIdentifier is sdk.AccountObjectIdentifier, not sdk.DatabaseObjectIdentifier
```

[GitHub issue reference](https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2779)

**Solution:** Some fields may expect different types of identifiers, when in doubt check [our documentation](https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs) for the field or the [official Snowflake documentation](https://docs.snowflake.com/) what type of identifier is needed. Quick reference:
- AccountObjectIdentifier - `<name>`
- DatabaseObjectIdentifier - `<database>.<name>`
- SchemaObjectIdentifier - `<database>.<schema>.<name>`
- TableColumnIdentifier - `<database>.<schema>.<table>.<name>`

### Incorrect account identifier (snowflake_database.from_share)
**Problem:** From 0.87.0 version, we are quoting incoming external account identifier correctly, which may break configurations that specified account identifier as `<org_name>.<acc_name>` that worked previously by accident.

Expand Down
35 changes: 35 additions & 0 deletions pkg/acceptance/check_destroy.go
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,41 @@ func CheckDatabaseRolePrivilegesRevoked(t *testing.T) func(*terraform.State) err
}
}

// CheckSharePrivilegesRevoked is a custom checks that should be later incorporated into generic CheckDestroy
func CheckSharePrivilegesRevoked(t *testing.T) func(*terraform.State) error {
t.Helper()
client := Client(t)

return func(state *terraform.State) error {
for _, rs := range state.RootModule().Resources {
if rs.Type != "snowflake_grant_privileges_to_share" {
continue
}
ctx := context.Background()

id := sdk.NewExternalObjectIdentifierFromFullyQualifiedName(rs.Primary.Attributes["to_share"])
grants, err := client.Grants.Show(ctx, &sdk.ShowGrantOptions{
To: &sdk.ShowGrantsTo{
Share: &sdk.ShowGrantsToShare{
Name: sdk.NewAccountObjectIdentifier(id.Name()),
},
},
})
if err != nil {
return err
}
var grantedPrivileges []string
for _, grant := range grants {
grantedPrivileges = append(grantedPrivileges, grant.Privilege)
}
if len(grantedPrivileges) > 0 {
return fmt.Errorf("share (%s) is still granted with privileges: %v", id.FullyQualifiedName(), grantedPrivileges)
}
}
return nil
}
}

// CheckUserPasswordPolicyAttachmentDestroy is a custom checks that should be later incorporated into generic CheckDestroy
func CheckUserPasswordPolicyAttachmentDestroy(t *testing.T) func(*terraform.State) error {
t.Helper()
Expand Down
4 changes: 2 additions & 2 deletions pkg/acceptance/helpers/context_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func (c *ContextClient) CurrentAccount(t *testing.T) string {
return currentAccount
}

func (c *ContextClient) CurrentRole(t *testing.T) string {
func (c *ContextClient) CurrentRole(t *testing.T) sdk.AccountObjectIdentifier {
t.Helper()
ctx := context.Background()

Expand All @@ -52,7 +52,7 @@ func (c *ContextClient) CurrentRegion(t *testing.T) string {
return currentRegion
}

func (c *ContextClient) CurrentUser(t *testing.T) string {
func (c *ContextClient) CurrentUser(t *testing.T) sdk.AccountObjectIdentifier {
t.Helper()
ctx := context.Background()

Expand Down
2 changes: 1 addition & 1 deletion pkg/acceptance/helpers/database_role_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func (c *DatabaseRoleClient) CleanupDatabaseRoleFunc(t *testing.T, id sdk.Databa

return func() {
// to prevent error when db was removed before the role
_, err := c.context.client.Databases.ShowByID(ctx, sdk.NewAccountObjectIdentifier(id.DatabaseName()))
_, err := c.context.client.Databases.ShowByID(ctx, id.DatabaseId())
if errors.Is(err, sdk.ErrObjectNotExistOrAuthorized) {
return
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/acceptance/helpers/dynamic_table_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func (c *DynamicTableClient) CreateDynamicTable(t *testing.T, tableId sdk.Schema

func (c *DynamicTableClient) CreateDynamicTableWithOptions(t *testing.T, schemaId sdk.DatabaseObjectIdentifier, name string, warehouseId sdk.AccountObjectIdentifier, tableId sdk.SchemaObjectIdentifier) (*sdk.DynamicTable, func()) {
t.Helper()
id := sdk.NewSchemaObjectIdentifier(schemaId.DatabaseName(), schemaId.Name(), name)
id := sdk.NewSchemaObjectIdentifierInSchema(schemaId, name)
targetLag := sdk.TargetLag{
MaximumDuration: sdk.String("2 minutes"),
}
Expand Down
16 changes: 12 additions & 4 deletions pkg/acceptance/helpers/ids_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ func (c *IdsGenerator) AccountIdentifierWithLocator() sdk.AccountIdentifier {
return sdk.NewAccountIdentifierFromAccountLocator(c.context.client.GetAccountLocator())
}

func (c *IdsGenerator) newSchemaObjectIdentifier(name string) sdk.SchemaObjectIdentifier {
return sdk.NewSchemaObjectIdentifier(c.context.database, c.context.schema, name)
func (c *IdsGenerator) NewSchemaObjectIdentifier(name string) sdk.SchemaObjectIdentifier {
return sdk.NewSchemaObjectIdentifierInSchema(c.SchemaId(), name)
}

func (c *IdsGenerator) RandomAccountObjectIdentifier() sdk.AccountObjectIdentifier {
Expand All @@ -49,8 +49,16 @@ func (c *IdsGenerator) RandomAccountObjectIdentifierContaining(part string) sdk.
return sdk.NewAccountObjectIdentifier(c.AlphaContaining(part))
}

func (c *IdsGenerator) RandomDatabaseObjectIdentifier() sdk.DatabaseObjectIdentifier {
return sdk.NewDatabaseObjectIdentifier(c.DatabaseId().Name(), c.Alpha())
}

func (c *IdsGenerator) RandomSchemaObjectIdentifier() sdk.SchemaObjectIdentifier {
return c.newSchemaObjectIdentifier(c.Alpha())
return c.RandomSchemaObjectIdentifierInSchema(c.SchemaId())
}

func (c *IdsGenerator) RandomSchemaObjectIdentifierInSchema(schemaId sdk.DatabaseObjectIdentifier) sdk.SchemaObjectIdentifier {
return sdk.NewSchemaObjectIdentifierInSchema(schemaId, c.Alpha())
}

func (c *IdsGenerator) Alpha() string {
Expand All @@ -66,7 +74,7 @@ func (c *IdsGenerator) AlphaContaining(part string) string {
}

func (c *IdsGenerator) AlphaWithPrefix(prefix string) string {
return c.withTestObjectSuffix(prefix + c.Alpha())
return prefix + c.Alpha()
}

func (c *IdsGenerator) withTestObjectSuffix(text string) string {
Expand Down
3 changes: 1 addition & 2 deletions pkg/acceptance/helpers/masking_policy_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,7 @@ func (c *MaskingPolicyClient) CreateMaskingPolicyWithOptions(t *testing.T, schem
t.Helper()
ctx := context.Background()

name := random.String()
id := sdk.NewSchemaObjectIdentifier(schemaId.DatabaseName(), schemaId.Name(), name)
id := c.ids.RandomSchemaObjectIdentifierInSchema(schemaId)

err := c.client().Create(ctx, id, signature, returns, expression, options)
require.NoError(t, err)
Expand Down
53 changes: 53 additions & 0 deletions pkg/acceptance/helpers/materialized_view_client.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package helpers

import (
"context"
"testing"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
"github.com/stretchr/testify/require"
)

type MaterializedViewClient struct {
context *TestClientContext
ids *IdsGenerator
}

func NewMaterializedViewClient(context *TestClientContext, idsGenerator *IdsGenerator) *MaterializedViewClient {
return &MaterializedViewClient{
context: context,
ids: idsGenerator,
}
}

func (c *MaterializedViewClient) client() sdk.MaterializedViews {
return c.context.client.MaterializedViews
}

func (c *MaterializedViewClient) CreateMaterializedView(t *testing.T, query string, orReplace bool) (*sdk.MaterializedView, func()) {
t.Helper()
return c.CreateMaterializedViewWithName(t, c.ids.RandomSchemaObjectIdentifier(), query, orReplace)
}

func (c *MaterializedViewClient) CreateMaterializedViewWithName(t *testing.T, id sdk.SchemaObjectIdentifier, query string, orReplace bool) (*sdk.MaterializedView, func()) {
t.Helper()
ctx := context.Background()

err := c.client().Create(ctx, sdk.NewCreateMaterializedViewRequest(id, query).WithOrReplace(sdk.Bool(orReplace)))
require.NoError(t, err)

view, err := c.client().ShowByID(ctx, id)
require.NoError(t, err)

return view, c.DropMaterializedViewFunc(t, id)
}

func (c *MaterializedViewClient) DropMaterializedViewFunc(t *testing.T, id sdk.SchemaObjectIdentifier) func() {
t.Helper()
ctx := context.Background()

return func() {
err := c.client().Drop(ctx, sdk.NewDropMaterializedViewRequest(id).WithIfExists(sdk.Bool(true)))
require.NoError(t, err)
}
}
4 changes: 1 addition & 3 deletions pkg/acceptance/helpers/password_policy_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"testing"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/helpers/random"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -44,8 +43,7 @@ func (c *PasswordPolicyClient) CreatePasswordPolicyInSchemaWithOptions(t *testin
t.Helper()
ctx := context.Background()

name := random.AlphanumericN(12)
id := sdk.NewSchemaObjectIdentifier(schemaId.DatabaseName(), schemaId.Name(), name)
id := c.ids.RandomSchemaObjectIdentifierInSchema(schemaId)

err := c.client().Create(ctx, id, options)
require.NoError(t, err)
Expand Down
3 changes: 1 addition & 2 deletions pkg/acceptance/helpers/pipe_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"testing"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/helpers/random"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -34,7 +33,7 @@ func (c *PipeClient) CreatePipeInSchema(t *testing.T, schemaId sdk.DatabaseObjec
t.Helper()
ctx := context.Background()

id := sdk.NewSchemaObjectIdentifier(schemaId.DatabaseName(), schemaId.Name(), random.AlphaN(20))
id := c.ids.RandomSchemaObjectIdentifierInSchema(schemaId)

err := c.client().Create(ctx, id, copyStatement, &sdk.CreatePipeOptions{})
require.NoError(t, err)
Expand Down
62 changes: 52 additions & 10 deletions pkg/acceptance/helpers/role_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,18 @@ func (c *RoleClient) client() sdk.Roles {
return c.context.client.Roles
}

func (c *RoleClient) UseRole(t *testing.T, roleName string) func() {
func (c *RoleClient) UseRole(t *testing.T, roleId sdk.AccountObjectIdentifier) func() {
t.Helper()
ctx := context.Background()

currentRole, err := c.context.client.ContextFunctions.CurrentRole(ctx)
require.NoError(t, err)

err = c.context.client.Sessions.UseRole(ctx, sdk.NewAccountObjectIdentifier(roleName))
err = c.context.client.Sessions.UseRole(ctx, roleId)
require.NoError(t, err)

return func() {
err = c.context.client.Sessions.UseRole(ctx, sdk.NewAccountObjectIdentifier(currentRole))
err = c.context.client.Sessions.UseRole(ctx, currentRole)
require.NoError(t, err)
}
}
Expand All @@ -53,9 +53,14 @@ func (c *RoleClient) CreateRoleWithName(t *testing.T, name string) (*sdk.Role, f

func (c *RoleClient) CreateRoleGrantedToCurrentUser(t *testing.T) (*sdk.Role, func()) {
t.Helper()
ctx := context.Background()

role, roleCleanup := c.CreateRole(t)
c.GrantRoleToCurrentUser(t, role.ID())

currentUser, err := c.context.client.ContextFunctions.CurrentUser(ctx)
require.NoError(t, err)

c.GrantRoleToUser(t, role.ID(), currentUser)
return role, roleCleanup
}

Expand All @@ -80,15 +85,12 @@ func (c *RoleClient) DropRoleFunc(t *testing.T, id sdk.AccountObjectIdentifier)
}
}

func (c *RoleClient) GrantRoleToCurrentUser(t *testing.T, id sdk.AccountObjectIdentifier) {
func (c *RoleClient) GrantRoleToUser(t *testing.T, id sdk.AccountObjectIdentifier, userId sdk.AccountObjectIdentifier) {
t.Helper()
ctx := context.Background()

currentUser, err := c.context.client.ContextFunctions.CurrentUser(ctx)
require.NoError(t, err)

err = c.client().Grant(ctx, sdk.NewGrantRoleRequest(id, sdk.GrantRole{
User: sdk.Pointer(sdk.NewAccountObjectIdentifier(currentUser)),
err := c.client().Grant(ctx, sdk.NewGrantRoleRequest(id, sdk.GrantRole{
User: sdk.Pointer(userId),
}))
require.NoError(t, err)
}
Expand All @@ -114,6 +116,31 @@ func (c *RoleClient) GrantOwnershipOnAccountObject(t *testing.T, roleId sdk.Acco
require.NoError(t, err)
}

// TODO: move later to grants client
func (c *RoleClient) GrantOwnershipOnSchemaObject(t *testing.T, roleId sdk.AccountObjectIdentifier, objectId sdk.SchemaObjectIdentifier, objectType sdk.ObjectType, outboundPrivileges sdk.OwnershipCurrentGrantsOutboundPrivileges) {
t.Helper()
ctx := context.Background()

err := c.context.client.Grants.GrantOwnership(
ctx,
sdk.OwnershipGrantOn{
Object: &sdk.Object{
ObjectType: objectType,
Name: objectId,
},
},
sdk.OwnershipGrantTo{
AccountRoleName: sdk.Pointer(roleId),
},
&sdk.GrantOwnershipOptions{
CurrentGrants: &sdk.OwnershipCurrentGrants{
OutboundPrivileges: outboundPrivileges,
},
},
)
require.NoError(t, err)
}

// TODO: move later to grants client
func (c *RoleClient) GrantPrivilegeOnDatabaseToShare(t *testing.T, databaseId sdk.AccountObjectIdentifier, shareId sdk.AccountObjectIdentifier) {
t.Helper()
Expand All @@ -122,3 +149,18 @@ func (c *RoleClient) GrantPrivilegeOnDatabaseToShare(t *testing.T, databaseId sd
err := c.context.client.Grants.GrantPrivilegeToShare(ctx, []sdk.ObjectPrivilege{sdk.ObjectPrivilegeReferenceUsage}, &sdk.ShareGrantOn{Database: databaseId}, shareId)
require.NoError(t, err)
}

// TODO: move later to grants client
func (c *RoleClient) ShowGrantsTo(t *testing.T, roleId sdk.AccountObjectIdentifier) []sdk.Grant {
t.Helper()
ctx := context.Background()

grants, err := c.context.client.Grants.Show(ctx, &sdk.ShowGrantOptions{
To: &sdk.ShowGrantsTo{
Role: roleId,
},
})
require.NoError(t, err)

return grants
}
3 changes: 1 addition & 2 deletions pkg/acceptance/helpers/stage_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"path/filepath"
"testing"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/helpers/random"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -58,7 +57,7 @@ func (c *StageClient) CreateStage(t *testing.T) (*sdk.Stage, func()) {

func (c *StageClient) CreateStageInSchema(t *testing.T, schemaId sdk.DatabaseObjectIdentifier) (*sdk.Stage, func()) {
t.Helper()
id := sdk.NewSchemaObjectIdentifier(schemaId.DatabaseName(), schemaId.Name(), random.AlphaN(8))
id := c.ids.RandomSchemaObjectIdentifierInSchema(schemaId)
return c.CreateStageWithRequest(t, sdk.NewCreateInternalStageRequest(id))
}

Expand Down
10 changes: 9 additions & 1 deletion pkg/acceptance/helpers/table_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (c *TableClient) CreateTableInSchema(t *testing.T, schemaId sdk.DatabaseObj
func (c *TableClient) CreateTableWithColumns(t *testing.T, schemaId sdk.DatabaseObjectIdentifier, name string, columns []sdk.TableColumnRequest) (*sdk.Table, func()) {
t.Helper()

id := sdk.NewSchemaObjectIdentifier(schemaId.DatabaseName(), schemaId.Name(), name)
id := sdk.NewSchemaObjectIdentifierInSchema(schemaId, name)
ctx := context.Background()

dbCreateRequest := sdk.NewCreateTableRequest(id, columns)
Expand Down Expand Up @@ -75,6 +75,14 @@ func (c *TableClient) DropTableFunc(t *testing.T, id sdk.SchemaObjectIdentifier)
}
}

func (c *TableClient) SetDataRetentionTime(t *testing.T, id sdk.SchemaObjectIdentifier, days int) {
t.Helper()
ctx := context.Background()

err := c.client().Alter(ctx, sdk.NewAlterTableRequest(id).WithSet(sdk.NewTableSetRequest().WithDataRetentionTimeInDays(sdk.Int(days))))
require.NoError(t, err)
}

// GetTableColumnsFor is based on https://docs.snowflake.com/en/sql-reference/info-schema/columns.
// TODO: extract getting table columns as resource (like getting tag in system functions)
func (c *TableClient) GetTableColumnsFor(t *testing.T, tableId sdk.SchemaObjectIdentifier) []InformationSchemaColumns {
Expand Down
Loading

0 comments on commit f20940c

Please sign in to comment.