Skip to content

Commit

Permalink
fix: external function header parsing and add missing privileges (#2961)
Browse files Browse the repository at this point in the history
Changes:
- Add CREATE NOTEBOOK privilege for on all/future grants (and other
missing privileges based on the [Snowflake
documentation](https://docs.snowflake.com/en/sql-reference/sql/grant-privilege)):
#2960
- Fix header parsing in external function
  • Loading branch information
sfc-gh-jcieslak authored Jul 26, 2024
1 parent dd01ce9 commit 9d882fe
Show file tree
Hide file tree
Showing 8 changed files with 163 additions and 17 deletions.
6 changes: 3 additions & 3 deletions docs/resources/grant_privileges_to_account_role.md
Original file line number Diff line number Diff line change
Expand Up @@ -300,14 +300,14 @@ Optional:
- `all` (Block List, Max: 1) Configures the privilege to be granted on all objects in either a database or schema. (see [below for nested schema](#nestedblock--on_schema_object--all))
- `future` (Block List, Max: 1) Configures the privilege to be granted on future objects in either a database or schema. (see [below for nested schema](#nestedblock--on_schema_object--future))
- `object_name` (String) The fully qualified name of the object on which privileges will be granted.
- `object_type` (String) The object type of the schema object on which privileges will be granted. Valid values are: AGGREGATION POLICY | ALERT | AUTHENTICATION POLICY | CORTEX SEARCH SERVICE | DATA METRIC FUNCTION | DYNAMIC TABLE | EVENT TABLE | EXTERNAL TABLE | FILE FORMAT | FUNCTION | GIT REPOSITORY | HYBRID TABLE | IMAGE REPOSITORY | ICEBERG TABLE | MASKING POLICY | MATERIALIZED VIEW | MODEL | NETWORK RULE | PACKAGES POLICY | PASSWORD POLICY | PIPE | PROCEDURE | PROJECTION POLICY | ROW ACCESS POLICY | SECRET | SERVICE | SESSION POLICY | SEQUENCE | STAGE | STREAM | TABLE | TAG | TASK | VIEW | STREAMLIT
- `object_type` (String) The object type of the schema object on which privileges will be granted. Valid values are: AGGREGATION POLICY | ALERT | AUTHENTICATION POLICY | CORTEX SEARCH SERVICE | DATA METRIC FUNCTION | DYNAMIC TABLE | EVENT TABLE | EXTERNAL TABLE | FILE FORMAT | FUNCTION | GIT REPOSITORY | HYBRID TABLE | IMAGE REPOSITORY | ICEBERG TABLE | MASKING POLICY | MATERIALIZED VIEW | MODEL | NETWORK RULE | NOTEBOOK | PACKAGES POLICY | PASSWORD POLICY | PIPE | PROCEDURE | PROJECTION POLICY | ROW ACCESS POLICY | SECRET | SERVICE | SESSION POLICY | SEQUENCE | SNAPSHOT | STAGE | STREAM | TABLE | TAG | TASK | VIEW | STREAMLIT

<a id="nestedblock--on_schema_object--all"></a>
### Nested Schema for `on_schema_object.all`

Required:

- `object_type_plural` (String) The plural object type of the schema object on which privileges will be granted. Valid values are: AGGREGATION POLICIES | ALERTS | AUTHENTICATION POLICIES | CORTEX SEARCH SERVICES | DATA METRIC FUNCTIONS | DYNAMIC TABLES | EVENT TABLES | EXTERNAL TABLES | FILE FORMATS | FUNCTIONS | GIT REPOSITORIES | HYBRID TABLES | IMAGE REPOSITORIES | ICEBERG TABLES | MASKING POLICIES | MATERIALIZED VIEWS | MODELS | NETWORK RULES | PACKAGES POLICIES | PASSWORD POLICIES | PIPES | PROCEDURES | PROJECTION POLICIES | ROW ACCESS POLICIES | SECRETS | SERVICES | SESSION POLICIES | SEQUENCES | STAGES | STREAMS | TABLES | TAGS | TASKS | VIEWS | STREAMLITS.
- `object_type_plural` (String) The plural object type of the schema object on which privileges will be granted. Valid values are: AGGREGATION POLICIES | ALERTS | AUTHENTICATION POLICIES | CORTEX SEARCH SERVICES | DATA METRIC FUNCTIONS | DYNAMIC TABLES | EVENT TABLES | EXTERNAL TABLES | FILE FORMATS | FUNCTIONS | GIT REPOSITORIES | HYBRID TABLES | IMAGE REPOSITORIES | ICEBERG TABLES | MASKING POLICIES | MATERIALIZED VIEWS | MODELS | NETWORK RULES | NOTEBOOKS | PACKAGES POLICIES | PASSWORD POLICIES | PIPES | PROCEDURES | PROJECTION POLICIES | ROW ACCESS POLICIES | SECRETS | SERVICES | SESSION POLICIES | SEQUENCES | SNAPSHOTS | STAGES | STREAMS | TABLES | TAGS | TASKS | VIEWS | STREAMLITS.

Optional:

Expand All @@ -320,7 +320,7 @@ Optional:

Required:

- `object_type_plural` (String) The plural object type of the schema object on which privileges will be granted. Valid values are: ALERTS | AUTHENTICATION POLICIES | DATA METRIC FUNCTIONS | DYNAMIC TABLES | EVENT TABLES | EXTERNAL TABLES | FILE FORMATS | FUNCTIONS | GIT REPOSITORIES | HYBRID TABLES | ICEBERG TABLES | MATERIALIZED VIEWS | MODELS | NETWORK RULES | PASSWORD POLICIES | PIPES | PROCEDURES | SECRETS | SERVICES | SEQUENCES | STAGES | STREAMS | TABLES | TASKS | VIEWS.
- `object_type_plural` (String) The plural object type of the schema object on which privileges will be granted. Valid values are: ALERTS | AUTHENTICATION POLICIES | DATA METRIC FUNCTIONS | DYNAMIC TABLES | EVENT TABLES | EXTERNAL TABLES | FILE FORMATS | FUNCTIONS | GIT REPOSITORIES | HYBRID TABLES | ICEBERG TABLES | MATERIALIZED VIEWS | MODELS | NETWORK RULES | NOTEBOOKS | PASSWORD POLICIES | PIPES | PROCEDURES | SECRETS | SERVICES | SEQUENCES | SNAPSHOTS | STAGES | STREAMS | TABLES | TASKS | VIEWS.

Optional:

Expand Down
6 changes: 3 additions & 3 deletions docs/resources/grant_privileges_to_database_role.md
Original file line number Diff line number Diff line change
Expand Up @@ -204,14 +204,14 @@ Optional:
- `all` (Block List, Max: 1) Configures the privilege to be granted on all objects in either a database or schema. (see [below for nested schema](#nestedblock--on_schema_object--all))
- `future` (Block List, Max: 1) Configures the privilege to be granted on future objects in either a database or schema. (see [below for nested schema](#nestedblock--on_schema_object--future))
- `object_name` (String) The fully qualified name of the object on which privileges will be granted.
- `object_type` (String) The object type of the schema object on which privileges will be granted. Valid values are: AGGREGATION POLICY | ALERT | AUTHENTICATION POLICY | CORTEX SEARCH SERVICE | DATA METRIC FUNCTION | DYNAMIC TABLE | EVENT TABLE | EXTERNAL TABLE | FILE FORMAT | FUNCTION | GIT REPOSITORY | HYBRID TABLE | IMAGE REPOSITORY | ICEBERG TABLE | MASKING POLICY | MATERIALIZED VIEW | MODEL | NETWORK RULE | PACKAGES POLICY | PASSWORD POLICY | PIPE | PROCEDURE | PROJECTION POLICY | ROW ACCESS POLICY | SECRET | SERVICE | SESSION POLICY | SEQUENCE | STAGE | STREAM | TABLE | TAG | TASK | VIEW | STREAMLIT
- `object_type` (String) The object type of the schema object on which privileges will be granted. Valid values are: AGGREGATION POLICY | ALERT | AUTHENTICATION POLICY | CORTEX SEARCH SERVICE | DATA METRIC FUNCTION | DYNAMIC TABLE | EVENT TABLE | EXTERNAL TABLE | FILE FORMAT | FUNCTION | GIT REPOSITORY | HYBRID TABLE | IMAGE REPOSITORY | ICEBERG TABLE | MASKING POLICY | MATERIALIZED VIEW | MODEL | NETWORK RULE | NOTEBOOK | PACKAGES POLICY | PASSWORD POLICY | PIPE | PROCEDURE | PROJECTION POLICY | ROW ACCESS POLICY | SECRET | SERVICE | SESSION POLICY | SEQUENCE | SNAPSHOT | STAGE | STREAM | TABLE | TAG | TASK | VIEW | STREAMLIT

<a id="nestedblock--on_schema_object--all"></a>
### Nested Schema for `on_schema_object.all`

Required:

- `object_type_plural` (String) The plural object type of the schema object on which privileges will be granted. Valid values are: AGGREGATION POLICIES | ALERTS | AUTHENTICATION POLICIES | CORTEX SEARCH SERVICES | DATA METRIC FUNCTIONS | DYNAMIC TABLES | EVENT TABLES | EXTERNAL TABLES | FILE FORMATS | FUNCTIONS | GIT REPOSITORIES | HYBRID TABLES | IMAGE REPOSITORIES | ICEBERG TABLES | MASKING POLICIES | MATERIALIZED VIEWS | MODELS | NETWORK RULES | PACKAGES POLICIES | PASSWORD POLICIES | PIPES | PROCEDURES | PROJECTION POLICIES | ROW ACCESS POLICIES | SECRETS | SERVICES | SESSION POLICIES | SEQUENCES | STAGES | STREAMS | TABLES | TAGS | TASKS | VIEWS | STREAMLITS.
- `object_type_plural` (String) The plural object type of the schema object on which privileges will be granted. Valid values are: AGGREGATION POLICIES | ALERTS | AUTHENTICATION POLICIES | CORTEX SEARCH SERVICES | DATA METRIC FUNCTIONS | DYNAMIC TABLES | EVENT TABLES | EXTERNAL TABLES | FILE FORMATS | FUNCTIONS | GIT REPOSITORIES | HYBRID TABLES | IMAGE REPOSITORIES | ICEBERG TABLES | MASKING POLICIES | MATERIALIZED VIEWS | MODELS | NETWORK RULES | NOTEBOOKS | PACKAGES POLICIES | PASSWORD POLICIES | PIPES | PROCEDURES | PROJECTION POLICIES | ROW ACCESS POLICIES | SECRETS | SERVICES | SESSION POLICIES | SEQUENCES | SNAPSHOTS | STAGES | STREAMS | TABLES | TAGS | TASKS | VIEWS | STREAMLITS.

Optional:

Expand All @@ -224,7 +224,7 @@ Optional:

Required:

- `object_type_plural` (String) The plural object type of the schema object on which privileges will be granted. Valid values are: ALERTS | AUTHENTICATION POLICIES | DATA METRIC FUNCTIONS | DYNAMIC TABLES | EVENT TABLES | EXTERNAL TABLES | FILE FORMATS | FUNCTIONS | GIT REPOSITORIES | HYBRID TABLES | ICEBERG TABLES | MATERIALIZED VIEWS | MODELS | NETWORK RULES | PASSWORD POLICIES | PIPES | PROCEDURES | SECRETS | SERVICES | SEQUENCES | STAGES | STREAMS | TABLES | TASKS | VIEWS.
- `object_type_plural` (String) The plural object type of the schema object on which privileges will be granted. Valid values are: ALERTS | AUTHENTICATION POLICIES | DATA METRIC FUNCTIONS | DYNAMIC TABLES | EVENT TABLES | EXTERNAL TABLES | FILE FORMATS | FUNCTIONS | GIT REPOSITORIES | HYBRID TABLES | ICEBERG TABLES | MATERIALIZED VIEWS | MODELS | NETWORK RULES | NOTEBOOKS | PASSWORD POLICIES | PIPES | PROCEDURES | SECRETS | SERVICES | SEQUENCES | SNAPSHOTS | STAGES | STREAMS | TABLES | TASKS | VIEWS.

Optional:

Expand Down
21 changes: 12 additions & 9 deletions pkg/resources/external_function.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package resources

import (
"context"
"encoding/json"
"log"
"regexp"
"strconv"
Expand Down Expand Up @@ -414,16 +415,18 @@ func ReadContextExternalFunction(ctx context.Context, d *schema.ResourceData, me
case "headers":
if row.Value != "" && row.Value != "null" {
// Format in Snowflake DB is: {"head1":"val1","head2":"val2"}
headerPairs := strings.Split(strings.ReplaceAll(strings.ReplaceAll(strings.ReplaceAll(row.Value, "{", ""), "}", ""), "\"", ""), ",")
headers := []interface{}{}

for _, headerPair := range headerPairs {
headerItem := strings.Split(headerPair, ":")
var jsonHeaders map[string]string
err := json.Unmarshal([]byte(row.Value), &jsonHeaders)
if err != nil {
return diag.Errorf("error unmarshalling headers: %v", err)
}

header := map[string]interface{}{}
header["name"] = headerItem[0]
header["value"] = headerItem[1]
headers = append(headers, header)
headers := make([]any, 0, len(jsonHeaders))
for key, value := range jsonHeaders {
headers = append(headers, map[string]any{
"name": key,
"value": value,
})
}

if err := d.Set("header", headers); err != nil {
Expand Down
82 changes: 82 additions & 0 deletions pkg/resources/external_function_acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,53 @@ func TestAcc_ExternalFunction_issue2528(t *testing.T) {
})
}

// Proves that header parsing handles values wrapped in curly braces, e.g. `value = "{1}"`
func TestAcc_ExternalFunction_HeaderParsing(t *testing.T) {
id := acc.TestClient().Ids.RandomSchemaObjectIdentifier()

resourceName := "snowflake_external_function.f"

resource.Test(t, resource.TestCase{
PreCheck: func() { acc.TestAccPreCheck(t) },
TerraformVersionChecks: []tfversion.TerraformVersionCheck{
tfversion.RequireAbove(tfversion.Version1_5_0),
},
CheckDestroy: acc.CheckDestroy(t, resources.ExternalFunction),
Steps: []resource.TestStep{
{
ExternalProviders: map[string]resource.ExternalProvider{
"snowflake": {
VersionConstraint: "=0.93.0",
Source: "Snowflake-Labs/snowflake",
},
},
Config: externalFunctionConfigIssueCurlyHeader(id),
// Previous implementation produces a plan with the following changes
//
// - header { # forces replacement
// - name = "name" -> null
// - value = "0" -> null
// }
//
// + header { # forces replacement
// + name = "name"
// + value = "{0}"
// }
ExpectNonEmptyPlan: true,
},
{
ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
Config: externalFunctionConfigIssueCurlyHeader(id),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "header.#", "1"),
resource.TestCheckResourceAttr(resourceName, "header.0.name", "name"),
resource.TestCheckResourceAttr(resourceName, "header.0.value", "{0}"),
),
},
},
})
}

func externalFunctionConfig(database string, schema string, name string) string {
return externalFunctionConfigWithReturnNullAllowed(database, schema, name, nil)
}
Expand Down Expand Up @@ -478,3 +525,38 @@ resource "snowflake_external_function" "f2" {
}
`, database, schema, name, schema2)
}

func externalFunctionConfigIssueCurlyHeader(id sdk.SchemaObjectIdentifier) string {
return fmt.Sprintf(`
resource "snowflake_api_integration" "test_api_int" {
name = "%[3]s"
api_provider = "aws_api_gateway"
api_aws_role_arn = "arn:aws:iam::000000000001:/role/test"
api_allowed_prefixes = ["https://123456.execute-api.us-west-2.amazonaws.com/prod/"]
enabled = true
}
resource "snowflake_external_function" "f" {
name = "%[3]s"
database = "%[1]s"
schema = "%[2]s"
arg {
name = "ARG1"
type = "VARCHAR"
}
arg {
name = "ARG2"
type = "VARCHAR"
}
header {
name = "name"
value = "{0}"
}
return_type = "VARIANT"
return_behavior = "IMMUTABLE"
api_integration = snowflake_api_integration.test_api_int.name
url_of_proxy_and_resource = "https://123456.execute-api.us-west-2.amazonaws.com/prod/test_func"
}
`, id.DatabaseName(), id.SchemaName(), id.Name())
}
40 changes: 40 additions & 0 deletions pkg/resources/grant_privileges_to_database_role_acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1185,6 +1185,46 @@ func TestAcc_GrantPrivilegesToDatabaseRole_AlwaysApply_SetAfterCreate(t *testing
})
}

// proves https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2960
func TestAcc_GrantPrivilegesToDatabaseRole_CreateNotebooks(t *testing.T) {
databaseRoleId := acc.TestClient().Ids.RandomDatabaseObjectIdentifier()

configVariables := config.Variables{
"name": config.StringVariable(databaseRoleId.Name()),
"privileges": config.ListVariable(
config.StringVariable(string(sdk.SchemaPrivilegeCreateNotebook)),
),
"database": config.StringVariable(acc.TestDatabaseName),
"with_grant_option": config.BoolVariable(false),
}
resourceName := "snowflake_grant_privileges_to_database_role.test"

resource.Test(t, resource.TestCase{
ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
PreCheck: func() { acc.TestAccPreCheck(t) },
TerraformVersionChecks: []tfversion.TerraformVersionCheck{
tfversion.RequireAbove(tfversion.Version1_5_0),
},
CheckDestroy: acc.CheckDatabaseRolePrivilegesRevoked(t),
Steps: []resource.TestStep{
{
PreConfig: func() {
_, databaseRoleCleanup := acc.TestClient().DatabaseRole.CreateDatabaseRoleWithName(t, databaseRoleId.Name())
t.Cleanup(databaseRoleCleanup)
},
ConfigDirectory: acc.ConfigurationDirectory("TestAcc_GrantPrivilegesToDatabaseRole/OnAllSchemasInDatabase"),
ConfigVariables: configVariables,
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "database_role_name", databaseRoleId.FullyQualifiedName()),
resource.TestCheckResourceAttr(resourceName, "privileges.#", "1"),
resource.TestCheckResourceAttr(resourceName, "privileges.0", string(sdk.SchemaPrivilegeCreateNotebook)),
resource.TestCheckResourceAttr(resourceName, "id", fmt.Sprintf("%s|false|false|CREATE NOTEBOOK|OnSchema|OnAllSchemasInDatabase|%s", databaseRoleId.FullyQualifiedName(), acc.TestClient().Ids.DatabaseId().FullyQualifiedName())),
),
},
},
})
}

func queriedPrivilegesToDatabaseRoleEqualTo(databaseRoleName sdk.DatabaseObjectIdentifier, privileges ...string) func(s *terraform.State) error {
return queriedPrivilegesEqualTo(func(client *sdk.Client, ctx context.Context) ([]sdk.Grant, error) {
return client.Grants.Show(ctx, &sdk.ShowGrantOptions{
Expand Down
2 changes: 2 additions & 0 deletions pkg/sdk/grants_validations.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ var validGrantToObjectTypes = []ObjectType{
ObjectTypeMaterializedView,
ObjectTypeModel,
ObjectTypeNetworkRule,
ObjectTypeNotebook,
ObjectTypePackagesPolicy,
ObjectTypePasswordPolicy,
ObjectTypePipe,
Expand All @@ -94,6 +95,7 @@ var validGrantToObjectTypes = []ObjectType{
ObjectTypeService,
ObjectTypeSessionPolicy,
ObjectTypeSequence,
ObjectTypeSnapshot,
ObjectTypeStage,
ObjectTypeStream,
ObjectTypeTable,
Expand Down
6 changes: 6 additions & 0 deletions pkg/sdk/object_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ const (
ObjectTypeView ObjectType = "VIEW"
ObjectTypeMaterializedView ObjectType = "MATERIALIZED VIEW"
ObjectTypeSequence ObjectType = "SEQUENCE"
ObjectTypeSnapshot ObjectType = "SNAPSHOT"
ObjectTypeFunction ObjectType = "FUNCTION"
ObjectTypeExternalFunction ObjectType = "EXTERNAL FUNCTION"
ObjectTypeProcedure ObjectType = "PROCEDURE"
Expand All @@ -62,6 +63,7 @@ const (
ObjectTypeIcebergTable ObjectType = "ICEBERG TABLE"
ObjectTypeExternalVolume ObjectType = "EXTERNAL VOLUME"
ObjectTypeNetworkRule ObjectType = "NETWORK RULE"
ObjectTypeNotebook ObjectType = "NOTEBOOK"
ObjectTypePackagesPolicy ObjectType = "PACKAGES POLICY"
ObjectTypeComputePool ObjectType = "COMPUTE POOL"
ObjectTypeAggregationPolicy ObjectType = "AGGREGATION POLICY"
Expand Down Expand Up @@ -107,6 +109,7 @@ func objectTypeSingularToPluralMap() map[ObjectType]PluralObjectType {
ObjectTypeView: PluralObjectTypeViews,
ObjectTypeMaterializedView: PluralObjectTypeMaterializedViews,
ObjectTypeSequence: PluralObjectTypeSequences,
ObjectTypeSnapshot: PluralObjectTypeSnapshots,
ObjectTypeFunction: PluralObjectTypeFunctions,
ObjectTypeExternalFunction: PluralObjectTypeExternalFunctions,
ObjectTypeProcedure: PluralObjectTypeProcedures,
Expand All @@ -127,6 +130,7 @@ func objectTypeSingularToPluralMap() map[ObjectType]PluralObjectType {
ObjectTypeIcebergTable: PluralObjectTypeIcebergTables,
ObjectTypeExternalVolume: PluralObjectTypeExternalVolumes,
ObjectTypeNetworkRule: PluralObjectTypeNetworkRules,
ObjectTypeNotebook: PluralObjectTypeNotebooks,
ObjectTypePackagesPolicy: PluralObjectTypePackagesPolicies,
ObjectTypeComputePool: PluralObjectTypeComputePool,
ObjectTypeAggregationPolicy: PluralObjectTypeAggregationPolicies,
Expand Down Expand Up @@ -212,6 +216,7 @@ const (
PluralObjectTypeViews PluralObjectType = "VIEWS"
PluralObjectTypeMaterializedViews PluralObjectType = "MATERIALIZED VIEWS"
PluralObjectTypeSequences PluralObjectType = "SEQUENCES"
PluralObjectTypeSnapshots PluralObjectType = "SNAPSHOTS"
PluralObjectTypeFunctions PluralObjectType = "FUNCTIONS"
PluralObjectTypeExternalFunctions PluralObjectType = "EXTERNAL FUNCTIONS"
PluralObjectTypeProcedures PluralObjectType = "PROCEDURES"
Expand All @@ -232,6 +237,7 @@ const (
PluralObjectTypeIcebergTables PluralObjectType = "ICEBERG TABLES"
PluralObjectTypeExternalVolumes PluralObjectType = "EXTERNAL VOLUMES"
PluralObjectTypeNetworkRules PluralObjectType = "NETWORK RULES"
PluralObjectTypeNotebooks PluralObjectType = "NOTEBOOKS"
PluralObjectTypePackagesPolicies PluralObjectType = "PACKAGES POLICIES"
PluralObjectTypeComputePool PluralObjectType = "COMPUTE POOLS"
PluralObjectTypeAggregationPolicies PluralObjectType = "AGGREGATION POLICIES"
Expand Down
Loading

0 comments on commit 9d882fe

Please sign in to comment.