Skip to content

Commit

Permalink
Fix grants datasource and improve parsers
Browse files Browse the repository at this point in the history
References: #3127
  • Loading branch information
sfc-gh-asawicki committed Nov 7, 2024
1 parent 87ff7a7 commit 9bf65db
Show file tree
Hide file tree
Showing 9 changed files with 101 additions and 25 deletions.
79 changes: 74 additions & 5 deletions pkg/datasources/grants.go
Original file line number Diff line number Diff line change
Expand Up @@ -378,21 +378,90 @@ 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,
},
}
}
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)

Expand Down
14 changes: 8 additions & 6 deletions pkg/datasources/grants_acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{
Expand All @@ -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(),
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
data "snowflake_grants" "test" {
grants_on {
object_name = var.fully_qualified_function_name
object_type = "FUNCTION"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
variable "fully_qualified_function_name" {
type = string
}
9 changes: 4 additions & 5 deletions pkg/resources/grant_ownership.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 0 additions & 1 deletion pkg/resources/grant_privileges_to_account_role.go
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
1 change: 0 additions & 1 deletion pkg/resources/grant_privileges_to_database_role.go
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
11 changes: 5 additions & 6 deletions pkg/resources/grant_privileges_to_share.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/sdk/identifier_parsers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 9bf65db

Please sign in to comment.