From 824ec52228afcdec282672018fc41c35a42a429d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= Date: Mon, 5 Aug 2024 10:43:18 +0200 Subject: [PATCH] feat: Add identifier parsers (#2957) 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 * [x] unit tests ## References * SNOW-1495051 --- pkg/acceptance/helpers/database_client.go | 16 +- pkg/acceptance/helpers/grant_client.go | 45 ++++ pkg/acceptance/helpers/test_client.go | 2 + pkg/datasources/database.go | 6 +- pkg/helpers/helpers.go | 6 +- pkg/helpers/identifier_string_parser.go | 32 --- pkg/helpers/identifier_string_parser_test.go | 94 ------- pkg/helpers/resource_identifier.go | 18 ++ pkg/helpers/resource_identifier_test.go | 39 +++ pkg/resources/grant_ownership_identifier.go | 7 +- .../grant_privileges_identifier_commons.go | 6 +- ...t_privileges_to_account_role_identifier.go | 9 +- ..._privileges_to_database_role_identifier.go | 4 +- .../grant_privileges_to_share_identifier.go | 6 +- pkg/resources/network_policy.go | 8 +- .../network_policy_acceptance_test.go | 45 ++++ pkg/resources/secondary_database.go | 6 +- .../secondary_database_acceptance_test.go | 22 +- pkg/resources/shared_database.go | 6 +- .../shared_database_acceptance_test.go | 24 +- .../user_password_policy_attachment.go | 14 +- pkg/schemas/database_gen.go | 4 +- pkg/sdk/databases.go | 13 +- pkg/sdk/identifier_helpers.go | 12 + pkg/sdk/identifier_parsers.go | 142 ++++++++++ pkg/sdk/identifier_parsers_test.go | 252 ++++++++++++++++++ pkg/sdk/replication_functions.go | 12 +- .../testint/identifier_integration_test.go | 163 +++++++++++ .../replication_functions_integration_test.go | 3 +- 29 files changed, 832 insertions(+), 184 deletions(-) create mode 100644 pkg/acceptance/helpers/grant_client.go delete mode 100644 pkg/helpers/identifier_string_parser.go delete mode 100644 pkg/helpers/identifier_string_parser_test.go create mode 100644 pkg/helpers/resource_identifier.go create mode 100644 pkg/helpers/resource_identifier_test.go create mode 100644 pkg/sdk/identifier_parsers.go create mode 100644 pkg/sdk/identifier_parsers_test.go create mode 100644 pkg/sdk/testint/identifier_integration_test.go diff --git a/pkg/acceptance/helpers/database_client.go b/pkg/acceptance/helpers/database_client.go index 2e9c71cf23..bc657a1fa3 100644 --- a/pkg/acceptance/helpers/database_client.go +++ b/pkg/acceptance/helpers/database_client.go @@ -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()) { diff --git a/pkg/acceptance/helpers/grant_client.go b/pkg/acceptance/helpers/grant_client.go new file mode 100644 index 0000000000..fa7ba85ccc --- /dev/null +++ b/pkg/acceptance/helpers/grant_client.go @@ -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) +} diff --git a/pkg/acceptance/helpers/test_client.go b/pkg/acceptance/helpers/test_client.go index ad1b2a982b..c629c37a0e 100644 --- a/pkg/acceptance/helpers/test_client.go +++ b/pkg/acceptance/helpers/test_client.go @@ -24,6 +24,7 @@ type TestClient struct { ExternalVolume *ExternalVolumeClient FailoverGroup *FailoverGroupClient FileFormat *FileFormatClient + Grant *GrantClient MaskingPolicy *MaskingPolicyClient MaterializedView *MaterializedViewClient NetworkPolicy *NetworkPolicyClient @@ -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), diff --git a/pkg/datasources/database.go b/pkg/datasources/database.go index e3caa58b04..dcf0ec536f 100644 --- a/pkg/datasources/database.go +++ b/pkg/datasources/database.go @@ -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 { diff --git a/pkg/helpers/helpers.go b/pkg/helpers/helpers.go index 96866792c4..a3e0f04737 100644 --- a/pkg/helpers/helpers.go +++ b/pkg/helpers/helpers.go @@ -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 } @@ -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 } @@ -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 }) diff --git a/pkg/helpers/identifier_string_parser.go b/pkg/helpers/identifier_string_parser.go deleted file mode 100644 index c5b1ee2049..0000000000 --- a/pkg/helpers/identifier_string_parser.go +++ /dev/null @@ -1,32 +0,0 @@ -package helpers - -import ( - "encoding/csv" - "fmt" - "strings" -) - -const ( - ParameterIDDelimiter = '.' -) - -func ParseIdentifierString(identifier string) ([]string, error) { - return parseIdentifierStringWithOpts(identifier, func(r *csv.Reader) { - r.Comma = ParameterIDDelimiter - }) -} - -func parseIdentifierStringWithOpts(identifier string, opts func(*csv.Reader)) ([]string, error) { - reader := csv.NewReader(strings.NewReader(identifier)) - if opts != nil { - opts(reader) - } - lines, err := reader.ReadAll() - if err != nil { - return nil, fmt.Errorf("unable to read identifier: %s, err = %w", identifier, err) - } - if len(lines) != 1 { - return nil, fmt.Errorf("incompatible identifier: %s", identifier) - } - return lines[0], nil -} diff --git a/pkg/helpers/identifier_string_parser_test.go b/pkg/helpers/identifier_string_parser_test.go deleted file mode 100644 index 1635808290..0000000000 --- a/pkg/helpers/identifier_string_parser_test.go +++ /dev/null @@ -1,94 +0,0 @@ -package helpers - -import ( - "testing" - - "github.com/stretchr/testify/require" -) - -// TODO [SNOW-999049]: add more fancy cases -func Test_ParseIdentifierString(t *testing.T) { - containsAll := func(t *testing.T, parts, expectedParts []string) { - t.Helper() - require.Len(t, parts, len(expectedParts)) - for _, part := range expectedParts { - require.Contains(t, parts, part) - } - } - - t.Run("returns read error", func(t *testing.T) { - input := `ab"c` - - _, err := ParseIdentifierString(input) - - require.ErrorContains(t, err, "unable to read identifier") - require.ErrorContains(t, err, `bare " in non-quoted-field`) - }) - - t.Run("returns error for empty input", func(t *testing.T) { - input := "" - - _, err := ParseIdentifierString(input) - - require.ErrorContains(t, err, "incompatible identifier") - }) - - t.Run("returns error for multiple lines", func(t *testing.T) { - input := "abc\ndef" - - _, err := ParseIdentifierString(input) - - require.ErrorContains(t, err, "incompatible identifier") - }) - - t.Run("returns parts correctly without quoting", func(t *testing.T) { - input := "abc.def" - expected := []string{"abc", "def"} - - parts, err := ParseIdentifierString(input) - - require.NoError(t, err) - containsAll(t, parts, expected) - }) - - t.Run("returns parts correctly with quoting", func(t *testing.T) { - input := `"abc"."def"` - expected := []string{"abc", "def"} - - parts, err := ParseIdentifierString(input) - - require.NoError(t, err) - containsAll(t, parts, expected) - }) - - t.Run("returns parts correctly with mixed quoting", func(t *testing.T) { - input := `"abc".def."ghi"` - expected := []string{"abc", "def", "ghi"} - - parts, err := ParseIdentifierString(input) - - require.NoError(t, err) - containsAll(t, parts, expected) - }) - - // Quote inside must have a preceding quote (https://docs.snowflake.com/en/sql-reference/identifiers-syntax). - t.Run("returns parts correctly with quote inside", func(t *testing.T) { - input := `"ab""c".def` - expected := []string{`ab"c`, "def"} - - parts, err := ParseIdentifierString(input) - - require.NoError(t, err) - containsAll(t, parts, expected) - }) - - t.Run("returns parts correctly with dots inside", func(t *testing.T) { - input := `"ab.c".def` - expected := []string{`ab.c`, "def"} - - parts, err := ParseIdentifierString(input) - - require.NoError(t, err) - containsAll(t, parts, expected) - }) -} diff --git a/pkg/helpers/resource_identifier.go b/pkg/helpers/resource_identifier.go new file mode 100644 index 0000000000..684b6ecd55 --- /dev/null +++ b/pkg/helpers/resource_identifier.go @@ -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)) +} diff --git a/pkg/helpers/resource_identifier_test.go b/pkg/helpers/resource_identifier_test.go new file mode 100644 index 0000000000..f453cb8339 --- /dev/null +++ b/pkg/helpers/resource_identifier_test.go @@ -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) + } + }) + } +} diff --git a/pkg/resources/grant_ownership_identifier.go b/pkg/resources/grant_ownership_identifier.go index a6bc3277c3..2b897f262c 100644 --- a/pkg/resources/grant_ownership_identifier.go +++ b/pkg/resources/grant_ownership_identifier.go @@ -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" @@ -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 { @@ -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 "||||"`) } diff --git a/pkg/resources/grant_privileges_identifier_commons.go b/pkg/resources/grant_privileges_identifier_commons.go index 885b5d61c5..81779c08d9 100644 --- a/pkg/resources/grant_privileges_identifier_commons.go +++ b/pkg/resources/grant_privileges_identifier_commons.go @@ -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 { @@ -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 @@ -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 { diff --git a/pkg/resources/grant_privileges_to_account_role_identifier.go b/pkg/resources/grant_privileges_to_account_role_identifier.go index 03f91dee4d..024e18bc11 100644 --- a/pkg/resources/grant_privileges_to_account_role_identifier.go +++ b/pkg/resources/grant_privileges_to_account_role_identifier.go @@ -30,10 +30,7 @@ type OnAccountObjectGrantData struct { } func (d *OnAccountObjectGrantData) String() string { - return strings.Join([]string{ - d.ObjectType.String(), - d.ObjectName.FullyQualifiedName(), - }, helpers.IDDelimiter) + return helpers.EncodeResourceIdentifier(d.ObjectType.String(), d.ObjectName.FullyQualifiedName()) } type GrantPrivilegesToAccountRoleId struct { @@ -61,13 +58,13 @@ func (g *GrantPrivilegesToAccountRoleId) String() string { if len(data) > 0 { parts = append(parts, data) } - return strings.Join(parts, helpers.IDDelimiter) + return helpers.EncodeResourceIdentifier(parts...) } func ParseGrantPrivilegesToAccountRoleId(id string) (GrantPrivilegesToAccountRoleId, error) { var accountRoleId GrantPrivilegesToAccountRoleId - parts := strings.Split(id, helpers.IDDelimiter) + parts := helpers.ParseResourceIdentifier(id) if len(parts) < 5 { return accountRoleId, sdk.NewError(`account role identifier should hold at least 5 parts "||||"`) } diff --git a/pkg/resources/grant_privileges_to_database_role_identifier.go b/pkg/resources/grant_privileges_to_database_role_identifier.go index 56e4ec2044..973b7ab024 100644 --- a/pkg/resources/grant_privileges_to_database_role_identifier.go +++ b/pkg/resources/grant_privileges_to_database_role_identifier.go @@ -39,7 +39,7 @@ func (g *GrantPrivilegesToDatabaseRoleId) String() string { } parts = append(parts, string(g.Kind)) parts = append(parts, g.Data.String()) - return strings.Join(parts, helpers.IDDelimiter) + return helpers.EncodeResourceIdentifier(parts...) } type OnDatabaseGrantData struct { @@ -53,7 +53,7 @@ func (d *OnDatabaseGrantData) String() string { func ParseGrantPrivilegesToDatabaseRoleId(id string) (GrantPrivilegesToDatabaseRoleId, error) { var databaseRoleId GrantPrivilegesToDatabaseRoleId - parts := strings.Split(id, helpers.IDDelimiter) + parts := helpers.ParseResourceIdentifier(id) if len(parts) < 6 { return databaseRoleId, sdk.NewError(`database role identifier should hold at least 6 parts "|||||"`) } diff --git a/pkg/resources/grant_privileges_to_share_identifier.go b/pkg/resources/grant_privileges_to_share_identifier.go index 744facc1b2..9996c38add 100644 --- a/pkg/resources/grant_privileges_to_share_identifier.go +++ b/pkg/resources/grant_privileges_to_share_identifier.go @@ -29,18 +29,18 @@ type GrantPrivilegesToShareId struct { } func (id *GrantPrivilegesToShareId) String() string { - return strings.Join([]string{ + return helpers.EncodeResourceIdentifier( id.ShareName.FullyQualifiedName(), strings.Join(id.Privileges, ","), string(id.Kind), id.Identifier.FullyQualifiedName(), - }, helpers.IDDelimiter) + ) } func ParseGrantPrivilegesToShareId(idString string) (GrantPrivilegesToShareId, error) { var grantPrivilegesToShareId GrantPrivilegesToShareId - parts := strings.Split(idString, helpers.IDDelimiter) + parts := helpers.ParseResourceIdentifier(idString) if len(parts) != 4 { return grantPrivilegesToShareId, sdk.NewError(fmt.Sprintf(`snowflake_grant_privileges_to_share id is composed out of 4 parts "|||", but got %d parts: %v`, len(parts), parts)) } diff --git a/pkg/resources/network_policy.go b/pkg/resources/network_policy.go index 5d84e96dd2..8ca3e82f94 100644 --- a/pkg/resources/network_policy.go +++ b/pkg/resources/network_policy.go @@ -92,10 +92,10 @@ func NetworkPolicy() *schema.Resource { Description: "Resource used to control network traffic. For more information, check an [official guide](https://docs.snowflake.com/en/user-guide/network-policies) on controlling network traffic with network policies.", CustomizeDiff: customdiff.All( - // For now, allowed_network_rule_list and blocked_network_rule_list have to stay commented and the implementation - // for ComputedIfAnyAttributeChanged has to be adjusted. The main issue lays in fields that have diff suppression. - // When the value in state and the value in config are different (which is normal with diff suppressions) show - // and describe outputs are constantly recomputed (which will appear in every terraform plan). + // For now, allowed_network_rule_list and blocked_network_rule_list have to stay commented. + // The main issue lays in the old Terraform SDK and how its handling DiffSuppression and CustomizeDiff + // for complex types like Sets, Lists, and Maps. When every element of the Set is suppressed in custom diff, + // it returns true for d.HasChange anyway (it returns false for suppressed changes on primitive types like Number, Bool, String, etc.). ComputedIfAnyAttributeChanged( ShowOutputAttributeName, // "allowed_network_rule_list", diff --git a/pkg/resources/network_policy_acceptance_test.go b/pkg/resources/network_policy_acceptance_test.go index 638b58dfbd..e552fe84e5 100644 --- a/pkg/resources/network_policy_acceptance_test.go +++ b/pkg/resources/network_policy_acceptance_test.go @@ -363,6 +363,37 @@ func TestAcc_NetworkPolicy_InvalidBlockedIpListValue(t *testing.T) { }) } +func TestAcc_NetworkPolicy_InvalidNetworkRuleIds(t *testing.T) { + id := acc.TestClient().Ids.RandomAccountObjectIdentifier() + + 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.NetworkPolicy), + Steps: []resource.TestStep{ + { + Config: networkPolicyConfigInvalidAllowedNetworkRules(id.Name()), + ExpectError: regexp.MustCompile(`sdk\.TableColumnIdentifier\. The correct form of the fully qualified name for`), + }, + { + Config: networkPolicyConfigInvalidAllowedNetworkRules(id.Name()), + ExpectError: regexp.MustCompile(`sdk\.DatabaseObjectIdentifier\. The correct form of the fully qualified name`), + }, + { + Config: networkPolicyConfigInvalidBlockedNetworkRules(id.Name()), + ExpectError: regexp.MustCompile(`sdk\.TableColumnIdentifier\. The correct form of the fully qualified name for`), + }, + { + Config: networkPolicyConfigInvalidBlockedNetworkRules(id.Name()), + ExpectError: regexp.MustCompile(`sdk\.DatabaseObjectIdentifier\. The correct form of the fully qualified name`), + }, + }, + }) +} + func networkPolicyConfigBasic(name string) string { return fmt.Sprintf(`resource "snowflake_network_policy" "test" { name = "%v" @@ -376,6 +407,20 @@ func networkPolicyConfigInvalidBlockedIpListValue(name string) string { }`, name) } +func networkPolicyConfigInvalidAllowedNetworkRules(name string) string { + return fmt.Sprintf(`resource "snowflake_network_policy" "test" { + name = "%v" + allowed_network_rule_list = ["a.b", "a.b.c.d"] + }`, name) +} + +func networkPolicyConfigInvalidBlockedNetworkRules(name string) string { + return fmt.Sprintf(`resource "snowflake_network_policy" "test" { + name = "%v" + blocked_network_rule_list = ["a.b", "a.b.c.d"] + }`, name) +} + func networkPolicyConfigComplete( name string, allowedRuleList []string, diff --git a/pkg/resources/secondary_database.go b/pkg/resources/secondary_database.go index a135663732..cae95c5ccd 100644 --- a/pkg/resources/secondary_database.go +++ b/pkg/resources/secondary_database.go @@ -207,8 +207,10 @@ func ReadSecondaryDatabase(ctx context.Context, d *schema.ResourceData, meta any return diag.FromErr(err) } - if err := d.Set("as_replica_of", sdk.NewExternalObjectIdentifierFromFullyQualifiedName(replicationPrimaryDatabase.PrimaryDatabase).FullyQualifiedName()); err != nil { - return diag.FromErr(err) + if replicationPrimaryDatabase.PrimaryDatabase != nil { + if err := d.Set("as_replica_of", replicationPrimaryDatabase.PrimaryDatabase.FullyQualifiedName()); err != nil { + return diag.FromErr(err) + } } if err := d.Set("is_transient", secondaryDatabase.Transient); err != nil { diff --git a/pkg/resources/secondary_database_acceptance_test.go b/pkg/resources/secondary_database_acceptance_test.go index 921025ef90..edac458ec5 100644 --- a/pkg/resources/secondary_database_acceptance_test.go +++ b/pkg/resources/secondary_database_acceptance_test.go @@ -3,6 +3,7 @@ package resources_test import ( "context" "testing" + "time" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/helpers" @@ -20,10 +21,13 @@ func TestAcc_CreateSecondaryDatabase_Basic(t *testing.T) { id := acc.TestClient().Ids.RandomAccountObjectIdentifier() comment := random.Comment() - _, externalPrimaryId, primaryDatabaseCleanup := acc.SecondaryTestClient().Database.CreatePrimaryDatabase(t, []sdk.AccountIdentifier{ + primaryDatabase, externalPrimaryId, _ := acc.SecondaryTestClient().Database.CreatePrimaryDatabase(t, []sdk.AccountIdentifier{ acc.TestClient().Account.GetAccountIdentifier(t), }) - t.Cleanup(primaryDatabaseCleanup) + t.Cleanup(func() { + // TODO(SNOW-1562172): Create a better solution for this type of situations + require.Eventually(t, func() bool { return acc.SecondaryTestClient().Database.DropDatabase(t, primaryDatabase.ID()) == nil }, time.Second*5, time.Second) + }) newId := acc.TestClient().Ids.RandomAccountObjectIdentifier() newComment := random.Comment() @@ -151,10 +155,13 @@ func TestAcc_CreateSecondaryDatabase_complete(t *testing.T) { id := acc.TestClient().Ids.RandomAccountObjectIdentifier() comment := random.Comment() - _, externalPrimaryId, primaryDatabaseCleanup := acc.SecondaryTestClient().Database.CreatePrimaryDatabase(t, []sdk.AccountIdentifier{ + primaryDatabase, externalPrimaryId, _ := acc.SecondaryTestClient().Database.CreatePrimaryDatabase(t, []sdk.AccountIdentifier{ sdk.NewAccountIdentifierFromAccountLocator(acc.Client(t).GetAccountLocator()), }) - t.Cleanup(primaryDatabaseCleanup) + t.Cleanup(func() { + // TODO(SNOW-1562172): Create a better solution for this type of situations + require.Eventually(t, func() bool { return acc.SecondaryTestClient().Database.DropDatabase(t, primaryDatabase.ID()) == nil }, time.Second*5, time.Second) + }) externalVolumeId, externalVolumeCleanup := acc.TestClient().ExternalVolume.Create(t) t.Cleanup(externalVolumeCleanup) @@ -391,10 +398,13 @@ func TestAcc_CreateSecondaryDatabase_complete(t *testing.T) { func TestAcc_CreateSecondaryDatabase_DataRetentionTimeInDays(t *testing.T) { id := acc.TestClient().Ids.RandomAccountObjectIdentifier() - _, externalPrimaryId, primaryDatabaseCleanup := acc.SecondaryTestClient().Database.CreatePrimaryDatabase(t, []sdk.AccountIdentifier{ + primaryDatabase, externalPrimaryId, _ := acc.SecondaryTestClient().Database.CreatePrimaryDatabase(t, []sdk.AccountIdentifier{ sdk.NewAccountIdentifierFromAccountLocator(acc.Client(t).GetAccountLocator()), }) - t.Cleanup(primaryDatabaseCleanup) + t.Cleanup(func() { + // TODO(SNOW-1562172): Create a better solution for this type of situations + require.Eventually(t, func() bool { return acc.SecondaryTestClient().Database.DropDatabase(t, primaryDatabase.ID()) == nil }, time.Second*5, time.Second) + }) accountDataRetentionTimeInDays, err := acc.Client(t).Parameters.ShowAccountParameter(context.Background(), sdk.AccountParameterDataRetentionTimeInDays) require.NoError(t, err) diff --git a/pkg/resources/shared_database.go b/pkg/resources/shared_database.go index a9c26400b6..73ee42be03 100644 --- a/pkg/resources/shared_database.go +++ b/pkg/resources/shared_database.go @@ -171,8 +171,10 @@ func ReadSharedDatabase(ctx context.Context, d *schema.ResourceData, meta any) d return diag.FromErr(err) } - if err := d.Set("from_share", sdk.NewExternalObjectIdentifierFromFullyQualifiedName(database.Origin).FullyQualifiedName()); err != nil { - return diag.FromErr(err) + if database.Origin != nil { + if err := d.Set("from_share", database.Origin.FullyQualifiedName()); err != nil { + return diag.FromErr(err) + } } // TODO(SNOW-1325381) diff --git a/pkg/resources/shared_database_acceptance_test.go b/pkg/resources/shared_database_acceptance_test.go index fc0ec0cc1d..d700a25646 100644 --- a/pkg/resources/shared_database_acceptance_test.go +++ b/pkg/resources/shared_database_acceptance_test.go @@ -4,16 +4,16 @@ import ( "context" "regexp" "testing" - - "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/helpers" - "github.com/hashicorp/terraform-plugin-testing/plancheck" + "time" acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/helpers" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/helpers/random" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" "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/tfversion" "github.com/stretchr/testify/require" ) @@ -174,6 +174,24 @@ func TestAcc_CreateSharedDatabase_complete(t *testing.T) { "enable_console_output": config.BoolVariable(true), } + // TODO(SNOW-1562172): Create a better solution for this type of situations + // We have to create test database from share before the actual test to check if the newly created share is ready + // after previous test (there's some kind of issue or delay between cleaning up a share and creating a new one right after). + testId := acc.TestClient().Ids.RandomAccountObjectIdentifier() + err := acc.Client(t).Databases.CreateShared(context.Background(), testId, externalShareId, new(sdk.CreateSharedDatabaseOptions)) + require.NoError(t, err) + + require.Eventually(t, func() bool { + database, err := acc.TestClient().Database.Show(t, testId) + if err != nil { + return false + } + // Origin is returned as "" in those cases, because it's not valid sdk.ExternalObjectIdentifier parser sets it as nil. + // Once it turns into valid sdk.ExternalObjectIdentifier, we're ready to proceed with the actual test. + return database.Origin != nil + }, time.Minute, time.Second*6) + acc.TestClient().Database.DropDatabaseFunc(t, testId)() + resource.Test(t, resource.TestCase{ ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, TerraformVersionChecks: []tfversion.TerraformVersionCheck{ diff --git a/pkg/resources/user_password_policy_attachment.go b/pkg/resources/user_password_policy_attachment.go index 567f6a40e5..12882103a3 100644 --- a/pkg/resources/user_password_policy_attachment.go +++ b/pkg/resources/user_password_policy_attachment.go @@ -3,7 +3,6 @@ package resources import ( "context" "fmt" - "strings" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/provider" @@ -14,10 +13,11 @@ import ( var userPasswordPolicyAttachmentSchema = map[string]*schema.Schema{ "user_name": { - Type: schema.TypeString, - Required: true, - ForceNew: true, - Description: "User name of the user you want to attach the password policy to", + Type: schema.TypeString, + Required: true, + ForceNew: true, + Description: "User name of the user you want to attach the password policy to", + ValidateDiagFunc: IsValidIdentifier[sdk.AccountObjectIdentifier](), }, "password_policy_name": { Type: schema.TypeString, @@ -57,7 +57,7 @@ func CreateUserPasswordPolicyAttachment(d *schema.ResourceData, meta any) error return err } - d.SetId(helpers.EncodeSnowflakeID(userName.FullyQualifiedName(), passwordPolicy.FullyQualifiedName())) + d.SetId(helpers.EncodeResourceIdentifier(userName.FullyQualifiedName(), passwordPolicy.FullyQualifiedName())) return ReadUserPasswordPolicyAttachment(d, meta) } @@ -66,7 +66,7 @@ func ReadUserPasswordPolicyAttachment(d *schema.ResourceData, meta any) error { client := meta.(*provider.Context).Client ctx := context.Background() - parts := strings.Split(d.Id(), helpers.IDDelimiter) + parts := helpers.ParseResourceIdentifier(d.Id()) if len(parts) != 2 { return fmt.Errorf("required id format 'user_name|password_policy_name', but got: '%s'", d.Id()) } diff --git a/pkg/schemas/database_gen.go b/pkg/schemas/database_gen.go index f4a5596b14..5ad77e4895 100644 --- a/pkg/schemas/database_gen.go +++ b/pkg/schemas/database_gen.go @@ -75,7 +75,9 @@ func DatabaseToSchema(database *sdk.Database) map[string]any { databaseSchema["name"] = database.Name databaseSchema["is_default"] = database.IsDefault databaseSchema["is_current"] = database.IsCurrent - databaseSchema["origin"] = database.Origin + if database.Origin != nil { + databaseSchema["origin"] = database.Origin.FullyQualifiedName() + } databaseSchema["owner"] = database.Owner databaseSchema["comment"] = database.Comment databaseSchema["options"] = database.Options diff --git a/pkg/sdk/databases.go b/pkg/sdk/databases.go index e1c2bd3300..8245ea6b3e 100644 --- a/pkg/sdk/databases.go +++ b/pkg/sdk/databases.go @@ -5,6 +5,7 @@ import ( "database/sql" "errors" "fmt" + "log" "strconv" "strings" "time" @@ -49,7 +50,7 @@ type Database struct { Name string IsDefault bool IsCurrent bool - Origin string + Origin *ExternalObjectIdentifier Owner string Comment string Options string @@ -96,8 +97,14 @@ func (row databaseRow) convert() *Database { if row.IsCurrent.Valid { database.IsCurrent = row.IsCurrent.String == "Y" } - if row.Origin.Valid { - database.Origin = row.Origin.String + if row.Origin.Valid && row.Origin.String != "" { + originId, err := ParseExternalObjectIdentifier(row.Origin.String) + if err != nil { + // TODO(SNOW-1561641): Return error + log.Printf("[DEBUG] unable to parse origin ID: %v", row.Origin.String) + } else { + database.Origin = &originId + } } if row.Owner.Valid { database.Owner = row.Owner.String diff --git a/pkg/sdk/identifier_helpers.go b/pkg/sdk/identifier_helpers.go index dde7b86686..c7632347da 100644 --- a/pkg/sdk/identifier_helpers.go +++ b/pkg/sdk/identifier_helpers.go @@ -100,6 +100,10 @@ func NewExternalObjectIdentifierFromFullyQualifiedName(fullyQualifiedName string } } +func (i ExternalObjectIdentifier) AccountIdentifier() AccountIdentifier { + return i.accountIdentifier +} + func (i ExternalObjectIdentifier) Name() string { return i.objectIdentifier.Name() } @@ -137,6 +141,14 @@ func NewAccountIdentifierFromFullyQualifiedName(fullyQualifiedName string) Accou return NewAccountIdentifier(organizationName, accountName) } +func (i AccountIdentifier) OrganizationName() string { + return i.organizationName +} + +func (i AccountIdentifier) AccountName() string { + return i.accountName +} + func (i AccountIdentifier) Name() string { if i.organizationName != "" && i.accountName != "" { return fmt.Sprintf("%s.%s", i.organizationName, i.accountName) diff --git a/pkg/sdk/identifier_parsers.go b/pkg/sdk/identifier_parsers.go new file mode 100644 index 0000000000..584761b0be --- /dev/null +++ b/pkg/sdk/identifier_parsers.go @@ -0,0 +1,142 @@ +package sdk + +import ( + "encoding/csv" + "fmt" + "strings" +) + +const IdDelimiter = '.' + +// TODO(SNOW-1495053): Temporarily exported, make as private +func ParseIdentifierStringWithOpts(identifier string, opts func(*csv.Reader)) ([]string, error) { + reader := csv.NewReader(strings.NewReader(identifier)) + if opts != nil { + opts(reader) + } + lines, err := reader.ReadAll() + if err != nil { + return nil, fmt.Errorf("unable to read identifier: %s, err = %w", identifier, err) + } + if len(lines) != 1 { + return nil, fmt.Errorf("incompatible identifier: %s", identifier) + } + return lines[0], nil +} + +// TODO(SNOW-1495053): Temporarily exported, make as private +func ParseIdentifierString(identifier string) ([]string, error) { + parts, err := ParseIdentifierStringWithOpts(identifier, func(r *csv.Reader) { + r.Comma = IdDelimiter + }) + if err != nil { + return nil, err + } + for _, part := range parts { + // TODO(SNOW-1571674): Remove the validation + if strings.Contains(part, `"`) { + return nil, fmt.Errorf(`unable to parse identifier: %s, currently identifiers containing double quotes are not supported in the provider`, identifier) + } + } + return parts, nil +} + +func parseIdentifier[T ObjectIdentifier](identifier string, expectedParts int, expectedFormat string, constructFromParts func(parts []string) T) (T, error) { + var emptyIdentifier T + parts, err := ParseIdentifierString(identifier) + if err != nil { + return emptyIdentifier, err + } + if len(parts) != expectedParts { + return emptyIdentifier, fmt.Errorf(`unexpected number of parts %[1]d in identifier %[2]s, expected %[3]d in a form of "%[4]s"`, len(parts), identifier, expectedParts, expectedFormat) + } + return constructFromParts(parts), nil +} + +func ParseAccountObjectIdentifier(identifier string) (AccountObjectIdentifier, error) { + return parseIdentifier[AccountObjectIdentifier]( + identifier, 1, "", + func(parts []string) AccountObjectIdentifier { + return NewAccountObjectIdentifier(parts[0]) + }, + ) +} + +// TODO(SNOW-1495053): Replace ParseObjectIdentifier +// ParseObjectIdentifierString tries to guess the identifier by the number of parts it contains. +// Because of the overlapping, in some cases, the output ObjectIdentifier can be one of the following implementations: +// - AccountObjectIdentifier for one part +// - DatabaseObjectIdentifier for two parts +// - SchemaObjectIdentifier for three parts (overlaps with ExternalObjectIdentifier) +// - TableColumnIdentifier for four parts +func ParseObjectIdentifierString(identifier string) (ObjectIdentifier, error) { + parts, err := ParseIdentifierString(identifier) + if err != nil { + return nil, err + } + switch len(parts) { + case 1: + return NewAccountObjectIdentifier(parts[0]), nil + case 2: + return NewDatabaseObjectIdentifier(parts[0], parts[1]), nil + case 3: + return NewSchemaObjectIdentifier(parts[0], parts[1], parts[2]), nil + case 4: + return NewTableColumnIdentifier(parts[0], parts[1], parts[2], parts[3]), nil + default: + return nil, fmt.Errorf("unsupported identifier: %[1]s (number of parts: %[2]d)", identifier, len(parts)) + } +} + +func ParseDatabaseObjectIdentifier(identifier string) (DatabaseObjectIdentifier, error) { + return parseIdentifier[DatabaseObjectIdentifier]( + identifier, 2, ".", + func(parts []string) DatabaseObjectIdentifier { + return NewDatabaseObjectIdentifier(parts[0], parts[1]) + }, + ) +} + +func ParseSchemaObjectIdentifier(identifier string) (SchemaObjectIdentifier, error) { + return parseIdentifier[SchemaObjectIdentifier]( + identifier, 3, "..", + func(parts []string) SchemaObjectIdentifier { + return NewSchemaObjectIdentifier(parts[0], parts[1], parts[2]) + }, + ) +} + +func ParseTableColumnIdentifier(identifier string) (TableColumnIdentifier, error) { + return parseIdentifier[TableColumnIdentifier]( + identifier, 4, "...", + func(parts []string) TableColumnIdentifier { + return NewTableColumnIdentifier(parts[0], parts[1], parts[2], parts[3]) + }, + ) +} + +// ParseAccountIdentifier is implemented with an assumption that the recommended format is used that contains two parts, +// organization name and account name. +func ParseAccountIdentifier(identifier string) (AccountIdentifier, error) { + return parseIdentifier[AccountIdentifier]( + identifier, 2, ".", + func(parts []string) AccountIdentifier { + return NewAccountIdentifier(parts[0], parts[1]) + }, + ) +} + +// ParseExternalObjectIdentifier is implemented with an assumption that the identifier consists of three parts, because: +// - After identifier rework, we expect account identifiers to always have two parts ".". +// - So far, the only external things that we referred to with external identifiers had only one part (not including the account identifier), +// meaning it will always be represented as sdk.AccountObjectIdentifier. Documentation also doesn't describe any case where +// account identifier would be used as part of the identifier that would refer to the "lower level" object. +// Reference: https://docs.snowflake.com/en/user-guide/admin-account-identifier#where-are-account-identifiers-used. +func ParseExternalObjectIdentifier(identifier string) (ExternalObjectIdentifier, error) { + return parseIdentifier[ExternalObjectIdentifier]( + identifier, 3, "..", + func(parts []string) ExternalObjectIdentifier { + return NewExternalObjectIdentifier(NewAccountIdentifier(parts[0], parts[1]), NewAccountObjectIdentifier(parts[2])) + }, + ) +} diff --git a/pkg/sdk/identifier_parsers_test.go b/pkg/sdk/identifier_parsers_test.go new file mode 100644 index 0000000000..de86bbc9dc --- /dev/null +++ b/pkg/sdk/identifier_parsers_test.go @@ -0,0 +1,252 @@ +package sdk + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_ParseIdentifierString(t *testing.T) { + containsAll := func(t *testing.T, parts, expectedParts []string) { + t.Helper() + require.Len(t, parts, len(expectedParts)) + for _, part := range expectedParts { + require.Contains(t, parts, part) + } + } + + t.Run("returns read error", func(t *testing.T) { + input := `ab"c` + + _, err := ParseIdentifierString(input) + + require.ErrorContains(t, err, "unable to read identifier") + require.ErrorContains(t, err, `bare " in non-quoted-field`) + }) + + t.Run("returns error for empty input", func(t *testing.T) { + input := "" + + _, err := ParseIdentifierString(input) + + require.ErrorContains(t, err, "incompatible identifier") + }) + + t.Run("returns error for multiple lines", func(t *testing.T) { + input := "abc\ndef" + + _, err := ParseIdentifierString(input) + + require.ErrorContains(t, err, "incompatible identifier") + }) + + t.Run("returns parts correctly without quoting", func(t *testing.T) { + input := "abc.def" + expected := []string{"abc", "def"} + + parts, err := ParseIdentifierString(input) + + require.NoError(t, err) + containsAll(t, parts, expected) + }) + + t.Run("returns parts correctly with quoting", func(t *testing.T) { + input := `"abc"."def"` + expected := []string{"abc", "def"} + + parts, err := ParseIdentifierString(input) + + require.NoError(t, err) + containsAll(t, parts, expected) + }) + + t.Run("returns parts correctly with mixed quoting", func(t *testing.T) { + input := `"abc".def."ghi"` + expected := []string{"abc", "def", "ghi"} + + parts, err := ParseIdentifierString(input) + + require.NoError(t, err) + containsAll(t, parts, expected) + }) + + // Quote inside must have a preceding quote (https://docs.snowflake.com/en/sql-reference/identifiers-syntax). + t.Run("returns parts correctly with quote inside", func(t *testing.T) { + input := `"ab""c".def` + _, err := ParseIdentifierString(input) + + require.ErrorContains(t, err, `unable to parse identifier: "ab""c".def, currently identifiers containing double quotes are not supported in the provider`) + }) + + t.Run("returns parts correctly with dots inside", func(t *testing.T) { + input := `"ab.c".def` + expected := []string{`ab.c`, "def"} + + parts, err := ParseIdentifierString(input) + + require.NoError(t, err) + containsAll(t, parts, expected) + }) + + t.Run("empty identifier", func(t *testing.T) { + input := `""` + expected := []string{""} + + parts, err := ParseIdentifierString(input) + + require.NoError(t, err) + containsAll(t, parts, expected) + }) + + t.Run("handled correctly double quotes", func(t *testing.T) { + input := `""."."".".".""."".""."".""."".""."""""` + + _, err := ParseIdentifierString(input) + require.ErrorContains(t, err, `unable to parse identifier: "".".""."."."".""."".""."".""."".""""", currently identifiers containing double quotes are not supported in the provider`) + }) +} + +func Test_IdentifierParsers(t *testing.T) { + testCases := []struct { + IdentifierType string + Input string + Expected ObjectIdentifier + Error string + }{ + {IdentifierType: "AccountObjectIdentifier", Input: ``, Error: "incompatible identifier: "}, + {IdentifierType: "AccountObjectIdentifier", Input: "a\nb", Error: "incompatible identifier: a\nb"}, + {IdentifierType: "AccountObjectIdentifier", Input: `a"b`, Error: "unable to read identifier: a\"b, err = parse error on line 1, column 2: bare \" in non-quoted-field"}, + {IdentifierType: "AccountObjectIdentifier", Input: `abc.cde`, Error: `unexpected number of parts 2 in identifier abc.cde, expected 1 in a form of ""`}, + {IdentifierType: "AccountObjectIdentifier", Input: `""""`, Error: `unable to parse identifier: """", currently identifiers containing double quotes are not supported in the provider`}, + {IdentifierType: "AccountObjectIdentifier", Input: `"a""bc"`, Error: `unable to parse identifier: "a""bc", currently identifiers containing double quotes are not supported in the provider`}, + {IdentifierType: "AccountObjectIdentifier", Input: `""`, Expected: NewAccountObjectIdentifier(``)}, + {IdentifierType: "AccountObjectIdentifier", Input: `abc`, Expected: NewAccountObjectIdentifier(`abc`)}, + {IdentifierType: "AccountObjectIdentifier", Input: `"abc"`, Expected: NewAccountObjectIdentifier(`abc`)}, + {IdentifierType: "AccountObjectIdentifier", Input: `"ab.c"`, Expected: NewAccountObjectIdentifier(`ab.c`)}, + + {IdentifierType: "DatabaseObjectIdentifier", Input: ``, Error: "incompatible identifier: "}, + {IdentifierType: "DatabaseObjectIdentifier", Input: "a\nb.cde", Error: "unable to read identifier: a\nb.cde, err = record on line 2: wrong number of fields"}, + {IdentifierType: "DatabaseObjectIdentifier", Input: `a"b.cde`, Error: "unable to read identifier: a\"b.cde, err = parse error on line 1, column 2: bare \" in non-quoted-field"}, + {IdentifierType: "DatabaseObjectIdentifier", Input: `abc.cde.efg`, Error: `unexpected number of parts 3 in identifier abc.cde.efg, expected 2 in a form of "."`}, + {IdentifierType: "DatabaseObjectIdentifier", Input: `abc`, Error: `unexpected number of parts 1 in identifier abc, expected 2 in a form of "."`}, + {IdentifierType: "DatabaseObjectIdentifier", Input: `"""".""""`, Error: `unable to parse identifier: """"."""", currently identifiers containing double quotes are not supported in the provider`}, + {IdentifierType: "DatabaseObjectIdentifier", Input: `"a""bc"."cd""e"`, Error: `unable to parse identifier: "a""bc"."cd""e", currently identifiers containing double quotes are not supported in the provider`}, + {IdentifierType: "DatabaseObjectIdentifier", Input: `"".""`, Expected: NewDatabaseObjectIdentifier(``, ``)}, + {IdentifierType: "DatabaseObjectIdentifier", Input: `abc.cde`, Expected: NewDatabaseObjectIdentifier(`abc`, `cde`)}, + {IdentifierType: "DatabaseObjectIdentifier", Input: `"abc"."cde"`, Expected: NewDatabaseObjectIdentifier(`abc`, `cde`)}, + {IdentifierType: "DatabaseObjectIdentifier", Input: `"ab.c"."cd.e"`, Expected: NewDatabaseObjectIdentifier(`ab.c`, `cd.e`)}, + + {IdentifierType: "SchemaObjectIdentifier", Input: ``, Error: "incompatible identifier: "}, + {IdentifierType: "SchemaObjectIdentifier", Input: "a\nb.cde.efg", Error: "unable to read identifier: a\nb.cde.efg, err = record on line 2: wrong number of fields"}, + {IdentifierType: "SchemaObjectIdentifier", Input: `a"b.cde.efg`, Error: "unable to read identifier: a\"b.cde.efg, err = parse error on line 1, column 2: bare \" in non-quoted-field"}, + {IdentifierType: "SchemaObjectIdentifier", Input: `abc.cde.efg.ghi`, Error: `unexpected number of parts 4 in identifier abc.cde.efg.ghi, expected 3 in a form of ".."`}, + {IdentifierType: "SchemaObjectIdentifier", Input: `abc.cde`, Error: `unexpected number of parts 2 in identifier abc.cde, expected 3 in a form of ".."`}, + {IdentifierType: "SchemaObjectIdentifier", Input: `""""."""".""""`, Error: `unable to parse identifier: """".""""."""", currently identifiers containing double quotes are not supported in the provider`}, + {IdentifierType: "SchemaObjectIdentifier", Input: `"a""bc"."cd""e"."ef""g"`, Error: `unable to parse identifier: "a""bc"."cd""e"."ef""g", currently identifiers containing double quotes are not supported in the provider`}, + {IdentifierType: "SchemaObjectIdentifier", Input: `""."".""`, Expected: NewSchemaObjectIdentifier(``, ``, ``)}, + {IdentifierType: "SchemaObjectIdentifier", Input: `abc.cde.efg`, Expected: NewSchemaObjectIdentifier(`abc`, `cde`, `efg`)}, + {IdentifierType: "SchemaObjectIdentifier", Input: `"abc"."cde"."efg"`, Expected: NewSchemaObjectIdentifier(`abc`, `cde`, `efg`)}, + {IdentifierType: "SchemaObjectIdentifier", Input: `"ab.c"."cd.e"."ef.g"`, Expected: NewSchemaObjectIdentifier(`ab.c`, `cd.e`, `ef.g`)}, + + {IdentifierType: "TableColumnIdentifier", Input: ``, Error: "incompatible identifier: "}, + {IdentifierType: "TableColumnIdentifier", Input: "a\nb.cde.efg.ghi", Error: "unable to read identifier: a\nb.cde.efg.ghi, err = record on line 2: wrong number of fields"}, + {IdentifierType: "TableColumnIdentifier", Input: `a"b.cde.efg.ghi`, Error: "unable to read identifier: a\"b.cde.efg.ghi, err = parse error on line 1, column 2: bare \" in non-quoted-field"}, + {IdentifierType: "TableColumnIdentifier", Input: `abc.cde.efg.ghi.ijk`, Error: `unexpected number of parts 5 in identifier abc.cde.efg.ghi.ijk, expected 4 in a form of "..."`}, + {IdentifierType: "TableColumnIdentifier", Input: `abc.cde`, Error: `unexpected number of parts 2 in identifier abc.cde, expected 4 in a form of "..."`}, + {IdentifierType: "TableColumnIdentifier", Input: `"""".""""."""".""""`, Error: `unable to parse identifier: """"."""".""""."""", currently identifiers containing double quotes are not supported in the provider`}, + {IdentifierType: "TableColumnIdentifier", Input: `"a""bc"."cd""e"."ef""g"."gh""i"`, Error: `unable to parse identifier: "a""bc"."cd""e"."ef""g"."gh""i", currently identifiers containing double quotes are not supported in the provider`}, + {IdentifierType: "TableColumnIdentifier", Input: `"".""."".""`, Expected: NewTableColumnIdentifier(``, ``, ``, ``)}, + {IdentifierType: "TableColumnIdentifier", Input: `abc.cde.efg.ghi`, Expected: NewTableColumnIdentifier(`abc`, `cde`, `efg`, `ghi`)}, + {IdentifierType: "TableColumnIdentifier", Input: `"abc"."cde"."efg"."ghi"`, Expected: NewTableColumnIdentifier(`abc`, `cde`, `efg`, `ghi`)}, + {IdentifierType: "TableColumnIdentifier", Input: `"ab.c"."cd.e"."ef.g"."gh.i"`, Expected: NewTableColumnIdentifier(`ab.c`, `cd.e`, `ef.g`, `gh.i`)}, + + {IdentifierType: "AccountIdentifier", Input: ``, Error: "incompatible identifier: "}, + {IdentifierType: "AccountIdentifier", Input: "a\nb.cde", Error: "unable to read identifier: a\nb.cde, err = record on line 2: wrong number of fields"}, + {IdentifierType: "AccountIdentifier", Input: `a"b.cde`, Error: "unable to read identifier: a\"b.cde, err = parse error on line 1, column 2: bare \" in non-quoted-field"}, + {IdentifierType: "AccountIdentifier", Input: `abc.cde.efg`, Error: `unexpected number of parts 3 in identifier abc.cde.efg, expected 2 in a form of "."`}, + {IdentifierType: "AccountIdentifier", Input: `abc`, Error: `unexpected number of parts 1 in identifier abc, expected 2 in a form of "."`}, + {IdentifierType: "AccountIdentifier", Input: `"""".""""`, Error: `unable to parse identifier: """"."""", currently identifiers containing double quotes are not supported in the provider`}, + {IdentifierType: "AccountIdentifier", Input: `"a""bc"."cd""e"`, Error: `unable to parse identifier: "a""bc"."cd""e", currently identifiers containing double quotes are not supported in the provider`}, + {IdentifierType: "AccountIdentifier", Input: `"".""`, Expected: NewAccountIdentifier(``, ``)}, + {IdentifierType: "AccountIdentifier", Input: `abc.cde`, Expected: NewAccountIdentifier(`abc`, `cde`)}, + {IdentifierType: "AccountIdentifier", Input: `"abc"."cde"`, Expected: NewAccountIdentifier(`abc`, `cde`)}, + {IdentifierType: "AccountIdentifier", Input: `"ab.c"."cd.e"`, Expected: NewAccountIdentifier(`ab.c`, `cd.e`)}, + + {IdentifierType: "ExternalObjectIdentifier", Input: ``, Error: "incompatible identifier: "}, + {IdentifierType: "ExternalObjectIdentifier", Input: "a\nb.cde.efg", Error: "unable to read identifier: a\nb.cde.efg, err = record on line 2: wrong number of fields"}, + {IdentifierType: "ExternalObjectIdentifier", Input: `a"b.cde.efg`, Error: "unable to read identifier: a\"b.cde.efg, err = parse error on line 1, column 2: bare \" in non-quoted-field"}, + {IdentifierType: "ExternalObjectIdentifier", Input: `abc.cde.efg.ghi`, Error: `unexpected number of parts 4 in identifier abc.cde.efg.ghi, expected 3 in a form of ".."`}, + {IdentifierType: "ExternalObjectIdentifier", Input: `abc.cde`, Error: `unexpected number of parts 2 in identifier abc.cde, expected 3 in a form of ".."`}, + {IdentifierType: "ExternalObjectIdentifier", Input: `""""."""".""""`, Error: `unable to parse identifier: """".""""."""", currently identifiers containing double quotes are not supported in the provider`}, + {IdentifierType: "ExternalObjectIdentifier", Input: `"a""bc"."cd""e"."ef""g"`, Error: `unable to parse identifier: "a""bc"."cd""e"."ef""g", currently identifiers containing double quotes are not supported in the provider`}, + {IdentifierType: "ExternalObjectIdentifier", Input: `""."".""`, Expected: NewExternalObjectIdentifier(NewAccountIdentifier(``, ``), NewAccountObjectIdentifier(``))}, + {IdentifierType: "ExternalObjectIdentifier", Input: `abc.cde.efg`, Expected: NewExternalObjectIdentifier(NewAccountIdentifier(`abc`, `cde`), NewAccountObjectIdentifier(`efg`))}, + {IdentifierType: "ExternalObjectIdentifier", Input: `"abc"."cde"."efg"`, Expected: NewExternalObjectIdentifier(NewAccountIdentifier(`abc`, `cde`), NewAccountObjectIdentifier(`efg`))}, + {IdentifierType: "ExternalObjectIdentifier", Input: `"ab.c"."cd.e"."ef.g"`, Expected: NewExternalObjectIdentifier(NewAccountIdentifier(`ab.c`, `cd.e`), NewAccountObjectIdentifier(`ef.g`))}, + } + + for _, testCase := range testCases { + t.Run(fmt.Sprintf(`Parsing %s with input: "%s"`, testCase.IdentifierType, testCase.Input), func(t *testing.T) { + var id ObjectIdentifier + var err error + + switch testCase.IdentifierType { + case "AccountObjectIdentifier": + id, err = ParseAccountObjectIdentifier(testCase.Input) + case "DatabaseObjectIdentifier": + id, err = ParseDatabaseObjectIdentifier(testCase.Input) + case "SchemaObjectIdentifier": + id, err = ParseSchemaObjectIdentifier(testCase.Input) + case "TableColumnIdentifier": + id, err = ParseTableColumnIdentifier(testCase.Input) + case "AccountIdentifier": + id, err = ParseAccountIdentifier(testCase.Input) + case "ExternalObjectIdentifier": + id, err = ParseExternalObjectIdentifier(testCase.Input) + default: + t.Fatalf("unknown identifier type: %s", testCase.IdentifierType) + } + + if testCase.Error != "" { + assert.ErrorContains(t, err, testCase.Error) + } else { + assert.Equal(t, testCase.Expected, id) + assert.NoError(t, err) + } + }) + } +} + +func Test_ParseObjectIdentifierString(t *testing.T) { + testCases := []struct { + Input string + Expected ObjectIdentifier + Error string + }{ + {Input: `to.many.parts.for.identifier`, Error: "unsupported identifier: to.many.parts.for.identifier (number of parts: 5)"}, + {Input: "a\nb.cde.efg", Error: "unable to read identifier: a\nb.cde.efg, err = record on line 2: wrong number of fields"}, + {Input: `a"b.cde.efg`, Error: "unable to read identifier: a\"b.cde.efg, err = parse error on line 1, column 2: bare \" in non-quoted-field"}, + {Input: ``, Error: "incompatible identifier: "}, + {Input: `abc`, Expected: NewAccountObjectIdentifier(`abc`)}, + {Input: `abc.def`, Expected: NewDatabaseObjectIdentifier(`abc`, `def`)}, + {Input: `abc.def.ghi`, Expected: NewSchemaObjectIdentifier(`abc`, `def`, `ghi`)}, + {Input: `abc."d.e.f".ghi`, Expected: NewSchemaObjectIdentifier(`abc`, `d.e.f`, `ghi`)}, + {Input: `abc."d""e""f".ghi`, Expected: NewSchemaObjectIdentifier(`abc`, `d"e"f`, `ghi`), Error: `unable to parse identifier: abc."d""e""f".ghi, currently identifiers containing double quotes are not supported in the provider`}, + {Input: `abc.def.ghi.jkl`, Expected: NewTableColumnIdentifier(`abc`, `def`, `ghi`, `jkl`)}, + } + + for _, testCase := range testCases { + t.Run(fmt.Sprintf("ParseObjectIdentifierString for input %s", testCase.Input), func(t *testing.T) { + id, err := ParseObjectIdentifierString(testCase.Input) + + if testCase.Error != "" { + assert.ErrorContains(t, err, testCase.Error) + } else { + assert.Equal(t, testCase.Expected, id) + assert.NoError(t, err) + } + }) + } +} diff --git a/pkg/sdk/replication_functions.go b/pkg/sdk/replication_functions.go index 23bb07115f..c23aa974d0 100644 --- a/pkg/sdk/replication_functions.go +++ b/pkg/sdk/replication_functions.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "errors" + "log" "time" ) @@ -83,7 +84,7 @@ type ReplicationDatabase struct { Name string Comment string IsPrimary bool - PrimaryDatabase string + PrimaryDatabase *ExternalObjectIdentifier ReplicationAllowedToAccounts string FailoverAllowedToAccounts string OrganizationName string @@ -97,10 +98,17 @@ func (row replicationDatabaseRow) convert() *ReplicationDatabase { AccountName: row.AccountName, Name: row.Name, IsPrimary: row.IsPrimary, - PrimaryDatabase: row.PrimaryDatabase, OrganizationName: row.OrganizationName, AccountLocator: row.AccountLocator, } + if row.PrimaryDatabase != "" { + primaryDatabaseId, err := ParseExternalObjectIdentifier(row.PrimaryDatabase) + if err != nil { + log.Printf("unable to parse primary database identifier: %v, err = %s", row.PrimaryDatabase, err) + } else { + db.PrimaryDatabase = &primaryDatabaseId + } + } if row.RegionGroup.Valid { db.RegionGroup = row.RegionGroup.String } diff --git a/pkg/sdk/testint/identifier_integration_test.go b/pkg/sdk/testint/identifier_integration_test.go new file mode 100644 index 0000000000..f4153a5252 --- /dev/null +++ b/pkg/sdk/testint/identifier_integration_test.go @@ -0,0 +1,163 @@ +package testint + +import ( + "context" + "fmt" + "testing" + + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk/internal/collections" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestInt_IdentifiersForOnePartIdentifierAsNameAndReference(t *testing.T) { + testCases := []struct { + Name sdk.AccountObjectIdentifier + ShowName string + Error string + }{ + // special cases + {Name: sdk.NewAccountObjectIdentifier(``), Error: "invalid object identifier"}, + {Name: sdk.NewAccountObjectIdentifier(`"`), Error: "invalid object identifier"}, + // This is a valid identifier, but because in NewXIdentifier functions we're trimming double quotes it won't work + {Name: sdk.NewAccountObjectIdentifier(`""`), Error: "invalid object identifier"}, + // This is a valid identifier, but because in NewXIdentifier functions we're trimming double quotes it won't work + {Name: sdk.NewAccountObjectIdentifier(`""""`), Error: "invalid object identifier"}, + {Name: sdk.NewAccountObjectIdentifier(`"."`), ShowName: `.`}, + + // lower case + {Name: sdk.NewAccountObjectIdentifier(`abc`), ShowName: `abc`}, + {Name: sdk.NewAccountObjectIdentifier(`ab.c`), ShowName: `ab.c`}, + {Name: sdk.NewAccountObjectIdentifier(`a"bc`), Error: `unexpected '"`}, + {Name: sdk.NewAccountObjectIdentifier(`"a""bc"`), ShowName: `a"bc`}, + + // upper case + {Name: sdk.NewAccountObjectIdentifier(`ABC`), ShowName: `ABC`}, + {Name: sdk.NewAccountObjectIdentifier(`AB.C`), ShowName: `AB.C`}, + {Name: sdk.NewAccountObjectIdentifier(`A"BC`), Error: `unexpected '"`}, + {Name: sdk.NewAccountObjectIdentifier(`"A""BC"`), ShowName: `A"BC`}, + + // mixed case + {Name: sdk.NewAccountObjectIdentifier(`AbC`), ShowName: `AbC`}, + {Name: sdk.NewAccountObjectIdentifier(`Ab.C`), ShowName: `Ab.C`}, + {Name: sdk.NewAccountObjectIdentifier(`A"bC`), Error: `unexpected '"`}, + {Name: sdk.NewAccountObjectIdentifier(`"A""bC"`), ShowName: `A"bC`}, + } + + for _, testCase := range testCases { + testCase := testCase + + t.Run(fmt.Sprintf("one part identifier name and reference for input: %s", testCase.Name.FullyQualifiedName()), func(t *testing.T) { + ctx := context.Background() + + err := testClient(t).ResourceMonitors.Create(ctx, testCase.Name, new(sdk.CreateResourceMonitorOptions)) + if testCase.Error != "" { + require.ErrorContains(t, err, testCase.Error) + } else { + t.Cleanup(testClientHelper().ResourceMonitor.DropResourceMonitorFunc(t, testCase.Name)) + } + + err = testClient(t).Warehouses.Create(ctx, testCase.Name, &sdk.CreateWarehouseOptions{ + ResourceMonitor: &testCase.Name, + }) + if testCase.Error != "" { + require.ErrorContains(t, err, testCase.Error) + } else { + require.NoError(t, err) + t.Cleanup(testClientHelper().Warehouse.DropWarehouseFunc(t, testCase.Name)) + var result struct { + Name string `db:"name"` + ResourceMonitor string `db:"resource_monitor"` + } + err = testClient(t).QueryOneForTests(ctx, &result, fmt.Sprintf("SHOW WAREHOUSES LIKE '%s'", testCase.ShowName)) + require.NoError(t, err) + + // For one part identifiers, we expect Snowflake to return unescaped identifiers (just like the ones we used for SHOW) + assert.Equal(t, testCase.ShowName, result.Name) + assert.Equal(t, testCase.ShowName, result.ResourceMonitor) + } + }) + } +} + +func TestInt_IdentifiersForTwoPartIdentifierAsReference(t *testing.T) { + type RawGrantOutput struct { + Name string `db:"name"` + Privilege string `db:"privilege"` + } + + testCases := []struct { + Name sdk.DatabaseObjectIdentifier + OverrideExpectedSnowflakeOutput string + Error string + }{ + // special cases + {Name: sdk.NewDatabaseObjectIdentifier(``, ``), Error: "invalid object identifier"}, + {Name: sdk.NewDatabaseObjectIdentifier(`"`, `"`), Error: "invalid object identifier"}, + // This is a valid identifier, but because in NewXIdentifier functions we're trimming double quotes it won't work + {Name: sdk.NewDatabaseObjectIdentifier(`""`, `""`), Error: "invalid object identifier"}, + // This is a valid identifier, but because in NewXIdentifier functions we're trimming double quotes it won't work + {Name: sdk.NewDatabaseObjectIdentifier(`""""`, `""""`), Error: "invalid object identifier"}, + {Name: sdk.NewDatabaseObjectIdentifier(`"."`, `"."`)}, + + // lower case + {Name: sdk.NewDatabaseObjectIdentifier(`abc`, `abc`)}, + {Name: sdk.NewDatabaseObjectIdentifier(`ab.c`, `ab.c`)}, + {Name: sdk.NewDatabaseObjectIdentifier(`a"bc`, `a"bc`), Error: `unexpected '"`}, + {Name: sdk.NewDatabaseObjectIdentifier(`"a""bc"`, `"a""bc"`)}, + + // upper case + {Name: sdk.NewDatabaseObjectIdentifier(`ABC`, `ABC`), OverrideExpectedSnowflakeOutput: `ABC.ABC`}, + {Name: sdk.NewDatabaseObjectIdentifier(`AB.C`, `AB.C`)}, + {Name: sdk.NewDatabaseObjectIdentifier(`A"BC`, `A"BC`), Error: `unexpected '"`}, + {Name: sdk.NewDatabaseObjectIdentifier(`"A""BC"`, `"A""BC"`)}, + + // mixed case + {Name: sdk.NewDatabaseObjectIdentifier(`AbC`, `AbC`)}, + {Name: sdk.NewDatabaseObjectIdentifier(`Ab.C`, `Ab.C`)}, + {Name: sdk.NewDatabaseObjectIdentifier(`A"bC`, `A"bC`), Error: `unexpected '"`}, + {Name: sdk.NewDatabaseObjectIdentifier(`"A""bC"`, `"A""bC"`)}, + } + + role, roleCleanup := testClientHelper().Role.CreateRole(t) + t.Cleanup(roleCleanup) + + for _, testCase := range testCases { + t.Run(fmt.Sprintf("two part identifier reference for input: %s", testCase.Name.FullyQualifiedName()), func(t *testing.T) { + ctx := context.Background() + + err := testClient(t).Databases.Create(ctx, testCase.Name.DatabaseId(), new(sdk.CreateDatabaseOptions)) + if testCase.Error != "" { + require.ErrorContains(t, err, testCase.Error) + } else { + t.Cleanup(testClientHelper().Database.DropDatabaseFunc(t, testCase.Name.DatabaseId())) + } + + err = testClient(t).Schemas.Create(ctx, testCase.Name, new(sdk.CreateSchemaOptions)) + if testCase.Error != "" { + require.ErrorContains(t, err, testCase.Error) + } else { + require.NoError(t, err) + t.Cleanup(testClientHelper().Schema.DropSchemaFunc(t, testCase.Name)) + + testClientHelper().Grant.GrantOnSchemaToAccountRole(t, testCase.Name, role.ID(), sdk.SchemaPrivilegeCreateTable) + + var grants []RawGrantOutput + err = testClient(t).QueryForTests(ctx, &grants, fmt.Sprintf("SHOW GRANTS ON SCHEMA %s", testCase.Name.FullyQualifiedName())) + require.NoError(t, err) + + createTableGrant, err := collections.FindOne(grants, func(output RawGrantOutput) bool { return output.Privilege == sdk.SchemaPrivilegeCreateTable.String() }) + require.NoError(t, err) + + // For two part identifiers, we expect Snowflake to return escaped identifiers with exception + // to identifiers that don't have any lowercase character and special symbol in it. + if testCase.OverrideExpectedSnowflakeOutput != "" { + assert.Equal(t, testCase.OverrideExpectedSnowflakeOutput, createTableGrant.Name) + } else { + assert.Equal(t, testCase.Name.FullyQualifiedName(), createTableGrant.Name) + } + } + }) + } +} diff --git a/pkg/sdk/testint/replication_functions_integration_test.go b/pkg/sdk/testint/replication_functions_integration_test.go index d9a2a05d05..d6a58dd01d 100644 --- a/pkg/sdk/testint/replication_functions_integration_test.go +++ b/pkg/sdk/testint/replication_functions_integration_test.go @@ -55,7 +55,8 @@ func TestInt_ShowReplicationDatabases(t *testing.T) { require.NotEmpty(t, rdb.SnowflakeRegion) require.NotEmpty(t, rdb.CreatedOn) require.NotEmpty(t, rdb.AccountName) - require.NotEmpty(t, rdb.PrimaryDatabase) + require.NotNil(t, rdb.PrimaryDatabase) + require.NotEmpty(t, rdb.PrimaryDatabase.FullyQualifiedName()) if expectedIsPrimary { require.NotEmpty(t, rdb.ReplicationAllowedToAccounts) require.NotEmpty(t, rdb.FailoverAllowedToAccounts)