Skip to content

Commit

Permalink
feat: Add identifier parsers (#2957)
Browse files Browse the repository at this point in the history
Changes:
- Add and test identifier parsers for object and account identifiers
with different separators (only dot was added as identifier parser,
description provided below).

The encoder/parser for resource identifiers was in the end implemented
as simple string join/split by pipe, because of the following:
- Identifiers that have only one part that is represented by the
`sdk.ObjectIdentifier` should be encoded as `FullyQualifiedName` and
parsed with one of the newly introduced parsers. This should improve a
bit the support for tools that generate import commands (or import
blocks) because the resource identifier is the same as the Snowflake
one.
- More complicated identifiers having more parts should use the provided
encoder/parser to create or parse resource identifiers into parts. This
could cause problems for things that may contain pipe, but we should
generally avoid it or document it (can be done in the usage part of
identifier rework). Also, CSV reader/writer cannot be used for resource
identifiers, because this would make identifiers more complex. For
example, a fully qualified identifier with quotes would have to be
imported as `terraform import snowflake_table.test_table
'"""database_name"".""schema_name"".table_name"""'`. To have this as an
identifier would also mean creating a state upgrader for every resource
(or general one that could be reused) that would transform the
identifiers into CSV-compliant ones.

Notes: 
- Implementation with the use of `identifeirParsingFunc` was added
because initially, I had two parsers (one with dot as csv.Comma and the
other one with pipe) and later on removed the one with the pipe as
csv.Comma (I can merge the unexported parsers with exported ones or
leave it as is if we would like to introduce another internal parser).
- ExternalObjectIdentifier and AccountIdentifier parsers were created
with certain assumptions (described in code).

## Test Plan
<!-- detail ways in which this PR has been tested or needs to be tested
-->
* [x] unit tests

## References
<!-- issues documentation links, etc  -->

* SNOW-1495051
  • Loading branch information
sfc-gh-jcieslak authored Aug 5, 2024
1 parent 0d90cc9 commit 824ec52
Show file tree
Hide file tree
Showing 29 changed files with 832 additions and 184 deletions.
16 changes: 11 additions & 5 deletions pkg/acceptance/helpers/database_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,21 @@ func (c *DatabaseClient) CreateDatabaseWithOptions(t *testing.T, id sdk.AccountO
}

func (c *DatabaseClient) DropDatabaseFunc(t *testing.T, id sdk.AccountObjectIdentifier) func() {
t.Helper()
return func() { require.NoError(t, c.DropDatabase(t, id)) }
}

func (c *DatabaseClient) DropDatabase(t *testing.T, id sdk.AccountObjectIdentifier) error {
t.Helper()
ctx := context.Background()

return func() {
err := c.client().Drop(ctx, id, &sdk.DropDatabaseOptions{IfExists: sdk.Bool(true)})
require.NoError(t, err)
err = c.context.client.Sessions.UseSchema(ctx, c.ids.SchemaId())
require.NoError(t, err)
if err := c.client().Drop(ctx, id, &sdk.DropDatabaseOptions{IfExists: sdk.Bool(true)}); err != nil {
return err
}
if err := c.context.client.Sessions.UseSchema(ctx, c.ids.SchemaId()); err != nil {
return err
}
return nil
}

func (c *DatabaseClient) CreateSecondaryDatabaseWithOptions(t *testing.T, id sdk.AccountObjectIdentifier, externalId sdk.ExternalObjectIdentifier, opts *sdk.CreateSecondaryDatabaseOptions) (*sdk.Database, func()) {
Expand Down
45 changes: 45 additions & 0 deletions pkg/acceptance/helpers/grant_client.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package helpers

import (
"context"
"testing"

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

type GrantClient struct {
context *TestClientContext
ids *IdsGenerator
}

func NewGrantClient(context *TestClientContext, idsGenerator *IdsGenerator) *GrantClient {
return &GrantClient{
context: context,
ids: idsGenerator,
}
}

func (c *GrantClient) client() sdk.Grants {
return c.context.client.Grants
}

func (c *GrantClient) GrantOnSchemaToAccountRole(t *testing.T, schemaId sdk.DatabaseObjectIdentifier, accountRoleId sdk.AccountObjectIdentifier, privileges ...sdk.SchemaPrivilege) {
t.Helper()
ctx := context.Background()

err := c.client().GrantPrivilegesToAccountRole(
ctx,
&sdk.AccountRoleGrantPrivileges{
SchemaPrivileges: privileges,
},
&sdk.AccountRoleGrantOn{
Schema: &sdk.GrantOnSchema{
Schema: &schemaId,
},
},
accountRoleId,
new(sdk.GrantPrivilegesToAccountRoleOptions),
)
require.NoError(t, err)
}
2 changes: 2 additions & 0 deletions pkg/acceptance/helpers/test_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ type TestClient struct {
ExternalVolume *ExternalVolumeClient
FailoverGroup *FailoverGroupClient
FileFormat *FileFormatClient
Grant *GrantClient
MaskingPolicy *MaskingPolicyClient
MaterializedView *MaterializedViewClient
NetworkPolicy *NetworkPolicyClient
Expand Down Expand Up @@ -77,6 +78,7 @@ func NewTestClient(c *sdk.Client, database string, schema string, warehouse stri
ExternalVolume: NewExternalVolumeClient(context, idsGenerator),
FailoverGroup: NewFailoverGroupClient(context, idsGenerator),
FileFormat: NewFileFormatClient(context, idsGenerator),
Grant: NewGrantClient(context, idsGenerator),
MaskingPolicy: NewMaskingPolicyClient(context, idsGenerator),
MaterializedView: NewMaterializedViewClient(context, idsGenerator),
NetworkPolicy: NewNetworkPolicyClient(context, idsGenerator),
Expand Down
6 changes: 5 additions & 1 deletion pkg/datasources/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,11 @@ func ReadDatabase(d *schema.ResourceData, meta interface{}) error {
if err := d.Set("is_current", database.IsCurrent); err != nil {
return err
}
if err := d.Set("origin", database.Origin); err != nil {
var origin string
if database.Origin != nil {
origin = database.Origin.FullyQualifiedName()
}
if err := d.Set("origin", origin); err != nil {
return err
}
if err := d.Set("retention_time", database.RetentionTime); err != nil {
Expand Down
6 changes: 3 additions & 3 deletions pkg/helpers/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func DecodeSnowflakeID(id string) sdk.ObjectIdentifier {
// The following configuration { "some_identifier": "db.name" } will be parsed as an object called "name" that lives
// inside database called "db", not a database called "db.name". In this case quotes should be used.
func DecodeSnowflakeParameterID(identifier string) (sdk.ObjectIdentifier, error) {
parts, err := ParseIdentifierString(identifier)
parts, err := sdk.ParseIdentifierString(identifier)
if err != nil {
return nil, err
}
Expand All @@ -126,7 +126,7 @@ func DecodeSnowflakeParameterID(identifier string) (sdk.ObjectIdentifier, error)
// DecodeSnowflakeAccountIdentifier decodes account identifier (usually passed as one of the parameter in tf configuration) into sdk.AccountIdentifier.
// Check more in https://docs.snowflake.com/en/sql-reference/sql/create-account#required-parameters.
func DecodeSnowflakeAccountIdentifier(identifier string) (sdk.AccountIdentifier, error) {
parts, err := ParseIdentifierString(identifier)
parts, err := sdk.ParseIdentifierString(identifier)
if err != nil {
return sdk.AccountIdentifier{}, err
}
Expand Down Expand Up @@ -166,7 +166,7 @@ func ConcatSlices[T any](slices ...[]T) []T {
// TODO(SNOW-999049): address during identifiers rework
func ParseRootLocation(location string) (sdk.SchemaObjectIdentifier, string, error) {
location = strings.TrimPrefix(location, "@")
parts, err := parseIdentifierStringWithOpts(location, func(r *csv.Reader) {
parts, err := sdk.ParseIdentifierStringWithOpts(location, func(r *csv.Reader) {
r.Comma = '.'
r.LazyQuotes = true
})
Expand Down
32 changes: 0 additions & 32 deletions pkg/helpers/identifier_string_parser.go

This file was deleted.

94 changes: 0 additions & 94 deletions pkg/helpers/identifier_string_parser_test.go

This file was deleted.

18 changes: 18 additions & 0 deletions pkg/helpers/resource_identifier.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package helpers

import (
"strings"
)

const ResourceIdDelimiter = '|'

func ParseResourceIdentifier(identifier string) []string {
if identifier == "" {
return make([]string, 0)
}
return strings.Split(identifier, string(ResourceIdDelimiter))
}

func EncodeResourceIdentifier(parts ...string) string {
return strings.Join(parts, string(ResourceIdDelimiter))
}
39 changes: 39 additions & 0 deletions pkg/helpers/resource_identifier_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package helpers

import (
"fmt"
"testing"

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

func Test_Encoding_And_Parsing_Of_ResourceIdentifier(t *testing.T) {
testCases := []struct {
Input []string
Expected string
ExpectedAfterDecoding []string
}{
{Input: []string{sdk.NewSchemaObjectIdentifier("a", "b", "c").FullyQualifiedName(), "info"}, Expected: `"a"."b"."c"|info`},
{Input: []string{}, Expected: ``},
{Input: []string{"", "", ""}, Expected: `||`},
{Input: []string{"a", "b", "c"}, Expected: `a|b|c`},
// If one of the parts contains a separator sign (pipe in this case),
// we can end up with more parts than we started with.
{Input: []string{"a", "b", "c|d"}, Expected: `a|b|c|d`, ExpectedAfterDecoding: []string{"a", "b", "c", "d"}},
}

for _, testCase := range testCases {
t.Run(fmt.Sprintf(`Encoding and parsing %s resource identifier`, testCase.Input), func(t *testing.T) {
encodedIdentifier := EncodeResourceIdentifier(testCase.Input...)
assert.Equal(t, testCase.Expected, encodedIdentifier)

parsedIdentifier := ParseResourceIdentifier(encodedIdentifier)
if testCase.ExpectedAfterDecoding != nil {
assert.Equal(t, testCase.ExpectedAfterDecoding, parsedIdentifier)
} else {
assert.Equal(t, testCase.Input, parsedIdentifier)
}
})
}
}
7 changes: 3 additions & 4 deletions pkg/resources/grant_ownership_identifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package resources

import (
"fmt"
"strings"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
Expand Down Expand Up @@ -59,7 +58,7 @@ func (g *OnObjectGrantOwnershipData) String() string {
var parts []string
parts = append(parts, g.ObjectType.String())
parts = append(parts, g.ObjectName.FullyQualifiedName())
return strings.Join(parts, helpers.IDDelimiter)
return helpers.EncodeResourceIdentifier(parts...)
}

func (g *GrantOwnershipId) String() string {
Expand All @@ -81,13 +80,13 @@ func (g *GrantOwnershipId) String() string {
if len(data) > 0 {
parts = append(parts, data)
}
return strings.Join(parts, helpers.IDDelimiter)
return helpers.EncodeResourceIdentifier(parts...)
}

func ParseGrantOwnershipId(id string) (*GrantOwnershipId, error) {
grantOwnershipId := new(GrantOwnershipId)

parts := strings.Split(id, helpers.IDDelimiter)
parts := helpers.ParseResourceIdentifier(id)
if len(parts) < 5 {
return grantOwnershipId, sdk.NewError(`grant ownership identifier should hold at least 5 parts "<target_role_kind>|<role_name>|<outbound_privileges_behavior>|<grant_type>|<grant_data>"`)
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/resources/grant_privileges_identifier_commons.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func (d *OnSchemaGrantData) String() string {
case OnAllSchemasInDatabaseSchemaGrantKind, OnFutureSchemasInDatabaseSchemaGrantKind:
parts = append(parts, d.DatabaseName.FullyQualifiedName())
}
return strings.Join(parts, helpers.IDDelimiter)
return helpers.EncodeResourceIdentifier(parts...)
}

type OnSchemaObjectGrantData struct {
Expand All @@ -57,7 +57,7 @@ func (d *OnSchemaObjectGrantData) String() string {
case OnAllSchemaObjectGrantKind, OnFutureSchemaObjectGrantKind:
parts = append(parts, d.OnAllOrFuture.String())
}
return strings.Join(parts, helpers.IDDelimiter)
return helpers.EncodeResourceIdentifier(parts...)
}

type BulkOperationGrantKind string
Expand All @@ -84,7 +84,7 @@ func (d *BulkOperationGrantData) String() string {
case InSchemaBulkOperationGrantKind:
parts = append(parts, d.Schema.FullyQualifiedName())
}
return strings.Join(parts, helpers.IDDelimiter)
return helpers.EncodeResourceIdentifier(parts...)
}

func getBulkOperationGrantData(in *sdk.GrantOnSchemaObjectIn) *BulkOperationGrantData {
Expand Down
Loading

0 comments on commit 824ec52

Please sign in to comment.