Skip to content

Commit

Permalink
feat: identifier validation (#2269)
Browse files Browse the repository at this point in the history
  • Loading branch information
sfc-gh-jcieslak authored Dec 15, 2023
1 parent 6b852c2 commit 9687972
Show file tree
Hide file tree
Showing 7 changed files with 398 additions and 94 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ require (
github.com/buger/jsonparser v1.1.1
github.com/google/uuid v1.4.0
github.com/gookit/color v1.5.4
github.com/hashicorp/go-cty v1.4.1-0.20200414143053-d3edf31b6320
github.com/hashicorp/go-uuid v1.0.3
github.com/hashicorp/terraform-plugin-framework v1.4.2
github.com/hashicorp/terraform-plugin-framework-validators v0.12.0
Expand Down Expand Up @@ -78,7 +79,6 @@ require (
github.com/hashicorp/errwrap v1.1.0 // indirect
github.com/hashicorp/go-checkpoint v0.5.0 // indirect
github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
github.com/hashicorp/go-cty v1.4.1-0.20200414143053-d3edf31b6320 // indirect
github.com/hashicorp/go-hclog v1.5.0 // indirect
github.com/hashicorp/go-multierror v1.1.1 // indirect
github.com/hashicorp/go-plugin v1.5.2 // indirect
Expand Down
75 changes: 41 additions & 34 deletions pkg/resources/grant_privileges_to_role.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,10 @@ var grantPrivilegesToRoleSchema = map[string]*schema.Schema{
}, true),
},
"object_name": {
Type: schema.TypeString,
Required: true,
Description: "The fully qualified name of the object on which privileges will be granted.",
Type: schema.TypeString,
Required: true,
Description: "The fully qualified name of the object on which privileges will be granted.",
ValidateDiagFunc: IsValidIdentifier[sdk.AccountObjectIdentifier](),
},
},
},
Expand All @@ -86,11 +87,12 @@ var grantPrivilegesToRoleSchema = map[string]*schema.Schema{
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"schema_name": {
Type: schema.TypeString,
Optional: true,
Description: "The fully qualified name of the schema.",
ConflictsWith: []string{"on_schema.0.all_schemas_in_database", "on_schema.0.future_schemas_in_database"},
ForceNew: true,
Type: schema.TypeString,
Optional: true,
Description: "The fully qualified name of the schema.",
ConflictsWith: []string{"on_schema.0.all_schemas_in_database", "on_schema.0.future_schemas_in_database"},
ValidateDiagFunc: IsValidIdentifier[sdk.DatabaseObjectIdentifier](),
ForceNew: true,
},
"all_schemas_in_database": {
Type: schema.TypeString,
Expand Down Expand Up @@ -151,12 +153,13 @@ var grantPrivilegesToRoleSchema = map[string]*schema.Schema{
}, true),
},
"object_name": {
Type: schema.TypeString,
Optional: true,
Description: "The fully qualified name of the object on which privileges will be granted.",
RequiredWith: []string{"on_schema_object.0.object_type"},
ConflictsWith: []string{"on_schema_object.0.all", "on_schema_object.0.future"},
ForceNew: true,
Type: schema.TypeString,
Optional: true,
Description: "The fully qualified name of the object on which privileges will be granted.",
RequiredWith: []string{"on_schema_object.0.object_type"},
ConflictsWith: []string{"on_schema_object.0.all", "on_schema_object.0.future"},
ValidateDiagFunc: IsValidIdentifier[sdk.SchemaObjectIdentifier](),
ForceNew: true,
},
"all": {
Type: schema.TypeList,
Expand Down Expand Up @@ -197,18 +200,20 @@ var grantPrivilegesToRoleSchema = map[string]*schema.Schema{
}, true),
},
"in_database": {
Type: schema.TypeString,
Optional: true,
Description: "The fully qualified name of the database.",
ConflictsWith: []string{"on_schema_object.0.all.in_schema"},
ForceNew: true,
Type: schema.TypeString,
Optional: true,
Description: "The fully qualified name of the database.",
ConflictsWith: []string{"on_schema_object.0.all.in_schema"},
ValidateDiagFunc: IsValidIdentifier[sdk.AccountObjectIdentifier](),
ForceNew: true,
},
"in_schema": {
Type: schema.TypeString,
Optional: true,
Description: "The fully qualified name of the schema.",
ConflictsWith: []string{"on_schema_object.0.all.in_database"},
ForceNew: true,
Type: schema.TypeString,
Optional: true,
Description: "The fully qualified name of the schema.",
ConflictsWith: []string{"on_schema_object.0.all.in_database"},
ValidateDiagFunc: IsValidIdentifier[sdk.DatabaseObjectIdentifier](),
ForceNew: true,
},
},
},
Expand Down Expand Up @@ -252,18 +257,20 @@ var grantPrivilegesToRoleSchema = map[string]*schema.Schema{
}, true),
},
"in_database": {
Type: schema.TypeString,
Optional: true,
Description: "The fully qualified name of the database.",
ConflictsWith: []string{"on_schema_object.0.all.in_schema"},
ForceNew: true,
Type: schema.TypeString,
Optional: true,
Description: "The fully qualified name of the database.",
ConflictsWith: []string{"on_schema_object.0.all.in_schema"},
ValidateDiagFunc: IsValidIdentifier[sdk.AccountObjectIdentifier](),
ForceNew: true,
},
"in_schema": {
Type: schema.TypeString,
Optional: true,
Description: "The fully qualified name of the schema.",
ConflictsWith: []string{"on_schema_object.0.all.in_database"},
ForceNew: true,
Type: schema.TypeString,
Optional: true,
Description: "The fully qualified name of the schema.",
ConflictsWith: []string{"on_schema_object.0.all.in_database"},
ValidateDiagFunc: IsValidIdentifier[sdk.DatabaseObjectIdentifier](),
ForceNew: true,
},
},
},
Expand Down
32 changes: 32 additions & 0 deletions pkg/resources/grant_privileges_to_role_acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package resources_test

import (
"fmt"
"regexp"
"strings"
"testing"

Expand Down Expand Up @@ -899,3 +900,34 @@ resource "snowflake_grant_privileges_to_role" "grant" {
},
})
}

func TestAcc_GrantPrivilegesToRole_ValidatedIdentifiers(t *testing.T) {
resource.Test(t, resource.TestCase{
ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
PreCheck: func() { acc.TestAccPreCheck(t) },
TerraformVersionChecks: []tfversion.TerraformVersionCheck{
tfversion.RequireAbove(tfversion.Version1_5_0),
},
CheckDestroy: nil,
Steps: []resource.TestStep{
{
Config: fmt.Sprintf(`
resource "snowflake_role" "role" {
name = "TEST_ROLE_123"
}
resource "snowflake_grant_privileges_to_role" "test_invalidation" {
role_name = snowflake_role.role.name
privileges = ["SELECT"]
on_schema_object {
future {
object_type_plural = "ICEBERG TABLES"
in_schema = "%s"
}
}
}`, acc.TestSchemaName),
ExpectError: regexp.MustCompile(".*Expected DatabaseObjectIdentifier identifier type.*"),
},
},
})
}
17 changes: 0 additions & 17 deletions pkg/resources/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,20 +125,3 @@ func GetPropertyAsPointer[T any](d *schema.ResourceData, property string) *T {
}
return &typedValue
}

func IsDataType() schema.SchemaValidateFunc { //nolint:staticcheck
return func(value any, key string) (warnings []string, errors []error) {
stringValue, ok := value.(string)
if !ok {
errors = append(errors, fmt.Errorf("expected type of %s to be string, got %T", key, value))
return warnings, errors
}

_, err := sdk.ToDataType(stringValue)
if err != nil {
errors = append(errors, fmt.Errorf("expected %s to be one of %T values, got %s", key, sdk.DataTypeString, stringValue))
}

return warnings, errors
}
}
42 changes: 0 additions & 42 deletions pkg/resources/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -470,45 +470,3 @@ func tagGrant(t *testing.T, id string, params map[string]interface{}) *schema.Re
d.SetId(id)
return d
}

func TestIsDataType(t *testing.T) {
isDataType := resources.IsDataType()
key := "tag"

testCases := []struct {
Name string
Value any
Error string
}{
{
Name: "validation: correct DataType value",
Value: "NUMBER",
},
{
Name: "validation: correct DataType value in lowercase",
Value: "number",
},
{
Name: "validation: incorrect DataType value",
Value: "invalid data type",
Error: "expected tag to be one of",
},
{
Name: "validation: incorrect value type",
Value: 123,
Error: "expected type of tag to be string",
},
}

for _, tt := range testCases {
t.Run(tt.Name, func(t *testing.T) {
_, errors := isDataType(tt.Value, key)
if tt.Error != "" {
assert.Len(t, errors, 1)
assert.ErrorContains(t, errors[0], tt.Error)
} else {
assert.Len(t, errors, 0)
}
})
}
}
110 changes: 110 additions & 0 deletions pkg/resources/validators.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
package resources

import (
"fmt"
"reflect"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers"
"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
"github.com/hashicorp/go-cty/cty"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
)

func IsDataType() schema.SchemaValidateFunc { //nolint:staticcheck
return func(value any, key string) (warnings []string, errors []error) {
stringValue, ok := value.(string)
if !ok {
errors = append(errors, fmt.Errorf("expected type of %s to be string, got %T", key, value))
return warnings, errors
}

_, err := sdk.ToDataType(stringValue)
if err != nil {
errors = append(errors, fmt.Errorf("expected %s to be one of %T values, got %s", key, sdk.DataTypeString, stringValue))
}

return warnings, errors
}
}

// IsValidIdentifier is a validator that can be used for validating identifiers passed in resources and data sources.
//
// Typically, we expect passed identifiers to be a variation of sdk.ObjectIdentifier.
// For now, we're expecting implementations of sdk.ObjectIdentifier, because we won't support sdk.ExternalObjectIdentifiers.
// The reason behind it is that the functions that parse identifiers are not able to differentiate between
// sdk.ExternalObjectIdentifiers and sdk.DatabaseObjectIdentifier or sdk.SchemaObjectIdentifier.
// That's because sdk.ExternalObjectIdentifiers has varying parts count (2 or 3).
//
// To use this function, pass it as a validation function on identifier field with generic parameter set to the desired sdk.ObjectIdentifier implementation.
func IsValidIdentifier[T sdk.AccountObjectIdentifier | sdk.DatabaseObjectIdentifier | sdk.SchemaObjectIdentifier | sdk.TableColumnIdentifier]() schema.SchemaValidateDiagFunc {
return func(value any, path cty.Path) diag.Diagnostics {
var diags diag.Diagnostics

if _, ok := value.(string); !ok {
diags = append(diags, diag.Diagnostic{
Severity: diag.Error,
Summary: "Invalid schema identifier type",
Detail: fmt.Sprintf("Expected schema string type, but got: %T. This is a provider error please file a report: https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/new/choose", value),
AttributePath: path,
})
return diags
}

stringValue := value.(string)
id, err := helpers.DecodeSnowflakeParameterID(stringValue)
if err != nil {
diags = append(diags, diag.Diagnostic{
Severity: diag.Error,
Summary: "Unable to parse the identifier",
Detail: fmt.Sprintf(
"Unable to parse the identifier: %s. Make sure you are using the correct form of the fully qualified name for this field: %s.\nOriginal Error: %s",
stringValue,
getExpectedIdentifierRepresentationFromGeneric[T](),
err.Error(),
),
AttributePath: path,
})
return diags
}

if _, ok := id.(T); !ok {
diags = append(diags, diag.Diagnostic{
Severity: diag.Error,
Summary: "Invalid identifier type",
Detail: fmt.Sprintf(
"Expected %s identifier type, but got: %T. The correct form of the fully qualified name for this field is: %s, but was %s",
reflect.TypeOf(new(T)).Elem().Name(),
id,
getExpectedIdentifierRepresentationFromGeneric[T](),
getExpectedIdentifierRepresentationFromParam(id),
),
AttributePath: path,
})
}

return diags
}
}

func getExpectedIdentifierRepresentationFromGeneric[T sdk.AccountObjectIdentifier | sdk.DatabaseObjectIdentifier | sdk.SchemaObjectIdentifier | sdk.TableColumnIdentifier]() string {
return getExpectedIdentifierForm(new(T))
}

func getExpectedIdentifierRepresentationFromParam(id sdk.ObjectIdentifier) string {
return getExpectedIdentifierForm(id)
}

func getExpectedIdentifierForm(id any) string {
switch id.(type) {
case sdk.AccountObjectIdentifier, *sdk.AccountObjectIdentifier:
return "<name>"
case sdk.DatabaseObjectIdentifier, *sdk.DatabaseObjectIdentifier:
return "<database_name>.<name>"
case sdk.SchemaObjectIdentifier, *sdk.SchemaObjectIdentifier:
return "<database_name>.<schema_name>.<name>"
case sdk.TableColumnIdentifier, *sdk.TableColumnIdentifier:
return "<database_name>.<schema_name>.<table_name>.<column_name>"
}
return ""
}
Loading

0 comments on commit 9687972

Please sign in to comment.