From 9bf65db66df275882503eab61aaea7d1db8fd50b Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Thu, 7 Nov 2024 11:26:25 +0100 Subject: [PATCH] Fix grants datasource and improve parsers References: #3127 --- pkg/datasources/grants.go | 79 +++++++++++++++++-- pkg/datasources/grants_acceptance_test.go | 14 ++-- .../snowflake_grants_on_schema_object.tf | 6 ++ .../SchemaObject_WithArguments/variables.tf | 3 + pkg/resources/grant_ownership.go | 9 +-- .../grant_privileges_to_account_role.go | 1 - .../grant_privileges_to_database_role.go | 1 - pkg/resources/grant_privileges_to_share.go | 11 ++- pkg/sdk/identifier_parsers.go | 2 +- 9 files changed, 101 insertions(+), 25 deletions(-) create mode 100644 pkg/datasources/testdata/TestAcc_Grants/On/SchemaObject_WithArguments/snowflake_grants_on_schema_object.tf create mode 100644 pkg/datasources/testdata/TestAcc_Grants/On/SchemaObject_WithArguments/variables.tf diff --git a/pkg/datasources/grants.go b/pkg/datasources/grants.go index 6cb81a7498..a2c0d340b1 100644 --- a/pkg/datasources/grants.go +++ b/pkg/datasources/grants.go @@ -378,14 +378,27 @@ func buildOptsForGrantsOn(grantsOn map[string]any) (*sdk.ShowGrantOptions, error if objectType == "" || objectName == "" { return nil, fmt.Errorf("object_type (%s) or object_name (%s) missing", objectType, objectName) } - // TODO [this PR]: implement - objectId, err := helpers.DecodeSnowflakeParameterID(objectName) - if err != nil { - return nil, err + + sdkObjectType := sdk.ObjectType(objectType) + var objectId sdk.ObjectIdentifier + var err error + // TODO [SNOW-1569535]: use a mapper from object type to parsing function + // TODO [SNOW-1569535]: grant_ownership#getOnObjectIdentifier could be used but it is limited only to ownership-transferable objects (according to the docs) - we should add an integration test to verify if the docs are complete + if sdkObjectType.IsWithArguments() { + objectId, err = sdk.ParseSchemaObjectIdentifierWithArguments(objectName) + if err != nil { + return nil, err + } + } else { + objectId, err = helpers.DecodeSnowflakeParameterID(objectName) + if err != nil { + return nil, err + } } + opts.On = &sdk.ShowGrantsOn{ Object: &sdk.Object{ - ObjectType: sdk.ObjectType(objectType), + ObjectType: sdkObjectType, Name: objectId, }, } @@ -393,6 +406,62 @@ func buildOptsForGrantsOn(grantsOn map[string]any) (*sdk.ShowGrantOptions, error return opts, nil } +// TODO(SNOW-1229218): Make sdk.ObjectType + string objectName to sdk.ObjectIdentifier mapping available in the sdk (for all object types). +func getOnObjectIdentifier(objectType sdk.ObjectType, objectName string) (sdk.ObjectIdentifier, error) { + switch objectType { + case sdk.ObjectTypeComputePool, + sdk.ObjectTypeDatabase, + sdk.ObjectTypeExternalVolume, + sdk.ObjectTypeFailoverGroup, + sdk.ObjectTypeIntegration, + sdk.ObjectTypeNetworkPolicy, + sdk.ObjectTypeReplicationGroup, + sdk.ObjectTypeRole, + sdk.ObjectTypeUser, + sdk.ObjectTypeWarehouse: + return sdk.ParseAccountObjectIdentifier(objectName) + case sdk.ObjectTypeDatabaseRole, + sdk.ObjectTypeSchema: + return sdk.ParseDatabaseObjectIdentifier(objectName) + case sdk.ObjectTypeAggregationPolicy, + sdk.ObjectTypeAlert, + sdk.ObjectTypeAuthenticationPolicy, + sdk.ObjectTypeDataMetricFunction, + sdk.ObjectTypeDynamicTable, + sdk.ObjectTypeEventTable, + sdk.ObjectTypeExternalTable, + sdk.ObjectTypeFileFormat, + sdk.ObjectTypeGitRepository, + sdk.ObjectTypeHybridTable, + sdk.ObjectTypeIcebergTable, + sdk.ObjectTypeImageRepository, + sdk.ObjectTypeMaterializedView, + sdk.ObjectTypeNetworkRule, + sdk.ObjectTypePackagesPolicy, + sdk.ObjectTypePipe, + sdk.ObjectTypeMaskingPolicy, + sdk.ObjectTypePasswordPolicy, + sdk.ObjectTypeProjectionPolicy, + sdk.ObjectTypeRowAccessPolicy, + sdk.ObjectTypeSessionPolicy, + sdk.ObjectTypeSecret, + sdk.ObjectTypeSequence, + sdk.ObjectTypeStage, + sdk.ObjectTypeStream, + sdk.ObjectTypeTable, + sdk.ObjectTypeTag, + sdk.ObjectTypeTask, + sdk.ObjectTypeView: + return sdk.ParseSchemaObjectIdentifier(objectName) + case sdk.ObjectTypeFunction, + sdk.ObjectTypeProcedure, + sdk.ObjectTypeExternalFunction: + return sdk.ParseSchemaObjectIdentifierWithArguments(objectName) + default: + return nil, sdk.NewError(fmt.Sprintf("object_type %s is not supported, please create a feature request for the provider if given object_type should be supported", objectType)) + } +} + func buildOptsForGrantsTo(grantsTo map[string]any) (*sdk.ShowGrantOptions, error) { opts := new(sdk.ShowGrantOptions) diff --git a/pkg/datasources/grants_acceptance_test.go b/pkg/datasources/grants_acceptance_test.go index 5badd1dfdd..6e9774e285 100644 --- a/pkg/datasources/grants_acceptance_test.go +++ b/pkg/datasources/grants_acceptance_test.go @@ -6,6 +6,8 @@ import ( acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/testenvs" + "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/tfversion" @@ -99,13 +101,13 @@ func TestAcc_Grants_On_SchemaObject(t *testing.T) { }) } -// TODO [this PR]: implement func TestAcc_Grants_On_SchemaObject_WithArguments(t *testing.T) { - tableName := acc.TestClient().Ids.Alpha() + _ = testenvs.GetOrSkipTest(t, testenvs.EnableAcceptance) + acc.TestAccPreCheck(t) + + function := acc.TestClient().Function.Create(t, sdk.DataTypeVARCHAR) configVariables := config.Variables{ - "database": config.StringVariable(acc.TestDatabaseName), - "schema": config.StringVariable(acc.TestSchemaName), - "table": config.StringVariable(tableName), + "fully_qualified_function_name": config.StringVariable(function.ID().FullyQualifiedName()), } resource.Test(t, resource.TestCase{ @@ -117,7 +119,7 @@ func TestAcc_Grants_On_SchemaObject_WithArguments(t *testing.T) { CheckDestroy: nil, Steps: []resource.TestStep{ { - ConfigDirectory: acc.ConfigurationDirectory("TestAcc_Grants/On/SchemaObject"), + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_Grants/On/SchemaObject_WithArguments"), ConfigVariables: configVariables, Check: checkAtLeastOneGrantPresent(), }, diff --git a/pkg/datasources/testdata/TestAcc_Grants/On/SchemaObject_WithArguments/snowflake_grants_on_schema_object.tf b/pkg/datasources/testdata/TestAcc_Grants/On/SchemaObject_WithArguments/snowflake_grants_on_schema_object.tf new file mode 100644 index 0000000000..80d7df5fad --- /dev/null +++ b/pkg/datasources/testdata/TestAcc_Grants/On/SchemaObject_WithArguments/snowflake_grants_on_schema_object.tf @@ -0,0 +1,6 @@ +data "snowflake_grants" "test" { + grants_on { + object_name = var.fully_qualified_function_name + object_type = "FUNCTION" + } +} diff --git a/pkg/datasources/testdata/TestAcc_Grants/On/SchemaObject_WithArguments/variables.tf b/pkg/datasources/testdata/TestAcc_Grants/On/SchemaObject_WithArguments/variables.tf new file mode 100644 index 0000000000..8a903b6d62 --- /dev/null +++ b/pkg/datasources/testdata/TestAcc_Grants/On/SchemaObject_WithArguments/variables.tf @@ -0,0 +1,3 @@ +variable "fully_qualified_function_name" { + type = string +} diff --git a/pkg/resources/grant_ownership.go b/pkg/resources/grant_ownership.go index d767485746..a79e9679ae 100644 --- a/pkg/resources/grant_ownership.go +++ b/pkg/resources/grant_ownership.go @@ -68,11 +68,10 @@ var grantOwnershipSchema = map[string]*schema.Schema{ ValidateFunc: validation.StringInSlice(sdk.ValidGrantOwnershipObjectTypesString, true), }, "object_name": { - Type: schema.TypeString, - Optional: true, - ForceNew: true, - Description: "Specifies the identifier for the object on which you are transferring ownership.", - // TODO [this PR]: implement + Type: schema.TypeString, + Optional: true, + ForceNew: true, + Description: "Specifies the identifier for the object on which you are transferring ownership.", DiffSuppressFunc: suppressIdentifierQuoting, RequiredWith: []string{ "on.0.object_type", diff --git a/pkg/resources/grant_privileges_to_account_role.go b/pkg/resources/grant_privileges_to_account_role.go index 1e1b668593..9d8fd49a97 100644 --- a/pkg/resources/grant_privileges_to_account_role.go +++ b/pkg/resources/grant_privileges_to_account_role.go @@ -225,7 +225,6 @@ var grantPrivilegesToAccountRoleSchema = map[string]*schema.Schema{ "on_schema_object.0.all", "on_schema_object.0.future", }, - // TODO [this PR]: implement DiffSuppressFunc: suppressIdentifierQuoting, }, "all": { diff --git a/pkg/resources/grant_privileges_to_database_role.go b/pkg/resources/grant_privileges_to_database_role.go index 08714aab41..b69b089a97 100644 --- a/pkg/resources/grant_privileges_to_database_role.go +++ b/pkg/resources/grant_privileges_to_database_role.go @@ -177,7 +177,6 @@ var grantPrivilegesToDatabaseRoleSchema = map[string]*schema.Schema{ "on_schema_object.0.all", "on_schema_object.0.future", }, - // TODO [this PR]: implement DiffSuppressFunc: suppressIdentifierQuoting, }, "all": { diff --git a/pkg/resources/grant_privileges_to_share.go b/pkg/resources/grant_privileges_to_share.go index 1e0cc6d4b8..ee3ab1eaab 100644 --- a/pkg/resources/grant_privileges_to_share.go +++ b/pkg/resources/grant_privileges_to_share.go @@ -94,12 +94,11 @@ var grantPrivilegesToShareSchema = map[string]*schema.Schema{ ExactlyOneOf: grantPrivilegesToShareGrantExactlyOneOfValidation, }, "on_function": { - Type: schema.TypeString, - Optional: true, - ForceNew: true, - Description: "The fully qualified name of the function on which privileges will be granted.", - ExactlyOneOf: grantPrivilegesToShareGrantExactlyOneOfValidation, - // TODO [this PR]: implement + Type: schema.TypeString, + Optional: true, + ForceNew: true, + Description: "The fully qualified name of the function on which privileges will be granted.", + ExactlyOneOf: grantPrivilegesToShareGrantExactlyOneOfValidation, DiffSuppressFunc: suppressIdentifierQuoting, }, } diff --git a/pkg/sdk/identifier_parsers.go b/pkg/sdk/identifier_parsers.go index 8764c6e082..291d861b9e 100644 --- a/pkg/sdk/identifier_parsers.go +++ b/pkg/sdk/identifier_parsers.go @@ -45,7 +45,7 @@ func ParseIdentifierString(identifier string) ([]string, error) { return parts, nil } -func parseIdentifier[T ObjectIdentifier](identifier string, expectedParts int, expectedFormat string, constructFromParts func(parts []string) T) (T, error) { +func parseIdentifier[T AccountIdentifier | AccountObjectIdentifier | DatabaseObjectIdentifier | ExternalObjectIdentifier | SchemaObjectIdentifier | TableColumnIdentifier](identifier string, expectedParts int, expectedFormat string, constructFromParts func(parts []string) T) (T, error) { var emptyIdentifier T parts, err := ParseIdentifierString(identifier) if err != nil {