Skip to content

Commit

Permalink
fix: Small fixes and adjustments (#3226)
Browse files Browse the repository at this point in the history
<!-- Feel free to delete comments as you fill this in -->
Changes included (they are really small or fix some wrong changes done
during tag rework that were not released yet):
- smaller fixes extracted from
https://github.com/Snowflake-Labs/terraform-provider-snowflake/pull/3210/files
- support tagging account for identifiers with org name
- add notes about manually unassigning policies from objects, add a todo
with an issue number
- fix a wrong issue number in essential objects list

Changes NOT included
- rework tag_association resource
- return nil from GetTag instead of failing
- add more tests regarding tag/masking policy: assert that ALTER MASKING
POLICY SET TAG differs from ALTER TAG SET MASKING POLICY
- support IF EXISTS for unsetting tags

<!-- summary of changes -->

## Test Plan
<!-- detail ways in which this PR has been tested or needs to be tested
-->
* [ ] acceptance tests
<!-- add more below if you think they are relevant -->
* [ ] …

## References
<!-- issues documentation links, etc  -->
#3210
  • Loading branch information
sfc-gh-jmichalak authored Nov 26, 2024
1 parent 8210bb8 commit 9f67457
Show file tree
Hide file tree
Showing 22 changed files with 149 additions and 21 deletions.
3 changes: 3 additions & 0 deletions docs/resources/authentication_policy.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ description: |-
Resource used to manage authentication policy objects. For more information, check authentication policy documentation https://docs.snowflake.com/en/sql-reference/sql/create-authentication-policy.
---

> [!WARNING]
> According to Snowflake [docs](https://docs.snowflake.com/en/sql-reference/sql/drop-authentication-policy#usage-notes), an authentication policy cannot be dropped successfully if it is currently assigned to another object. Currently, the provider does not unassign such objects automatically. Before dropping the resource, list the assigned objects with `SELECT * from table(information_schema.policy_references(policy_name=>'<string>'));` and unassign them manually with `ALTER ...` or with updated Terraform configuration, if possible.
# snowflake_authentication_policy (Resource)

Resource used to manage authentication policy objects. For more information, check [authentication policy documentation](https://docs.snowflake.com/en/sql-reference/sql/create-authentication-policy).
Expand Down
3 changes: 3 additions & 0 deletions docs/resources/masking_policy.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ description: |-

!> **V1 release candidate** This resource was reworked and is a release candidate for the V1. We do not expect significant changes in it before the V1. We will welcome any feedback and adjust the resource if needed. Any errors reported will be resolved with a higher priority. We encourage checking this resource out before the V1 release. Please follow the [migration guide](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/MIGRATION_GUIDE.md#v0950--v0960) to use it.

> [!WARNING]
> According to Snowflake [docs](https://docs.snowflake.com/en/sql-reference/sql/drop-masking-policy#usage-notes), a masking policy cannot be dropped successfully if it is currently assigned to another object. Currently, the provider does not unassign such objects automatically. Before dropping the resource, list the assigned objects with `SELECT * from table(information_schema.policy_references(policy_name=>'<string>'));` and unassign them manually with `ALTER ...` or with updated Terraform configuration, if possible.
# snowflake_masking_policy (Resource)

Resource used to manage masking policies. For more information, check [masking policies documentation](https://docs.snowflake.com/en/sql-reference/sql/create-masking-policy).
Expand Down
3 changes: 3 additions & 0 deletions docs/resources/network_policy.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ description: |-

!> **V1 release candidate** This resource was reworked and is a release candidate for the V1. We do not expect significant changes in it before the V1. We will welcome any feedback and adjust the resource if needed. Any errors reported will be resolved with a higher priority. We encourage checking this resource out before the V1 release. Please follow the [migration guide](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/MIGRATION_GUIDE.md#v0920--v0930) to use it.

> [!WARNING]
> According to Snowflake [docs](https://docs.snowflake.com/en/sql-reference/sql/drop-network-policy#usage-notes), a network policy cannot be dropped successfully if it is currently assigned to another object. Currently, the provider does not unassign such objects automatically. Before dropping the resource, list the assigned objects with `SELECT * from table(information_schema.policy_references(policy_name=>'<string>'));` and unassign them manually with `ALTER ...` or with updated Terraform configuration, if possible.
# snowflake_network_policy (Resource)

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.
Expand Down
3 changes: 3 additions & 0 deletions docs/resources/password_policy.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ description: |-
A password policy specifies the requirements that must be met to create and reset a password to authenticate to Snowflake.
---

> [!WARNING]
> According to Snowflake [docs](https://docs.snowflake.com/en/sql-reference/sql/drop-password-policy#usage-notes), a password policy cannot be dropped successfully if it is currently assigned to another object. Currently, the provider does not unassign such objects automatically. Before dropping the resource, list the assigned objects with `SELECT * from table(information_schema.policy_references(policy_name=>'<string>'));` and unassign them manually with `ALTER ...` or with updated Terraform configuration, if possible.
# snowflake_password_policy (Resource)

A password policy specifies the requirements that must be met to create and reset a password to authenticate to Snowflake.
Expand Down
3 changes: 3 additions & 0 deletions docs/resources/row_access_policy.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ description: |-

!> **V1 release candidate** This resource was reworked and is a release candidate for the V1. We do not expect significant changes in it before the V1. We will welcome any feedback and adjust the resource if needed. Any errors reported will be resolved with a higher priority. We encourage checking this resource out before the V1 release. Please follow the [migration guide](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/MIGRATION_GUIDE.md#v0950--v0960) to use it.

> [!WARNING]
> According to Snowflake [docs](https://docs.snowflake.com/en/sql-reference/sql/drop-row-access-policy#usage-notes), a row access policy cannot be dropped successfully if it is currently assigned to another object. Currently, the provider does not unassign such objects automatically. Before dropping the resource, list the assigned objects with `SELECT * from table(information_schema.policy_references(policy_name=>'<string>'));` and unassign them manually with `ALTER ...` or with updated Terraform configuration, if possible.
# snowflake_row_access_policy (Resource)

Resource used to manage row access policy objects. For more information, check [row access policy documentation](https://docs.snowflake.com/en/sql-reference/sql/create-row-access-policy).
Expand Down
9 changes: 9 additions & 0 deletions pkg/acceptance/helpers/context_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,15 @@ func (c *ContextClient) CurrentUser(t *testing.T) sdk.AccountObjectIdentifier {
return currentUser
}

func (c *ContextClient) CurrentAccountIdentifier(t *testing.T) sdk.AccountIdentifier {
t.Helper()

details, err := c.client().CurrentSessionDetails(context.Background())
require.NoError(t, err)

return sdk.NewAccountIdentifier(details.OrganizationName, details.AccountName)
}

func (c *ContextClient) IsRoleInSession(t *testing.T, id sdk.AccountObjectIdentifier) bool {
t.Helper()
ctx := context.Background()
Expand Down
1 change: 1 addition & 0 deletions pkg/resources/authentication_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,7 @@ func DeleteContextAuthenticationPolicy(ctx context.Context, d *schema.ResourceDa
}
}

// TODO(SNOW-1818849): unassign policies before dropping
if err := client.AuthenticationPolicies.Drop(ctx, sdk.NewDropAuthenticationPolicyRequest(id).WithIfExists(true)); err != nil {
return diag.FromErr(err)
}
Expand Down
1 change: 1 addition & 0 deletions pkg/resources/masking_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,7 @@ func DeleteMaskingPolicy(ctx context.Context, d *schema.ResourceData, meta any)
return diag.FromErr(err)
}

// TODO(SNOW-1818849): unassign policies before dropping
err = client.MaskingPolicies.Drop(ctx, id, &sdk.DropMaskingPolicyOptions{IfExists: sdk.Pointer(true)})
if err != nil {
return diag.Diagnostics{
Expand Down
1 change: 1 addition & 0 deletions pkg/resources/network_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,7 @@ func DeleteContextNetworkPolicy(ctx context.Context, d *schema.ResourceData, met
return diag.FromErr(err)
}

// TODO(SNOW-1818849): unassign policies before dropping
err = client.NetworkPolicies.Drop(ctx, sdk.NewDropNetworkPolicyRequest(id).WithIfExists(true))
if err != nil {
return diag.Diagnostics{
Expand Down
2 changes: 2 additions & 0 deletions pkg/resources/password_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,8 @@ func DeletePasswordPolicy(ctx context.Context, d *schema.ResourceData, meta any)
client := meta.(*provider.Context).Client

objectIdentifier := helpers.DecodeSnowflakeID(d.Id()).(sdk.SchemaObjectIdentifier)

// TODO(SNOW-1818849): unassign policies before dropping
err := client.PasswordPolicies.Drop(ctx, objectIdentifier, nil)
if err != nil {
return diag.FromErr(err)
Expand Down
1 change: 1 addition & 0 deletions pkg/resources/row_access_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,7 @@ func DeleteRowAccessPolicy(ctx context.Context, d *schema.ResourceData, meta any

client := meta.(*provider.Context).Client

// TODO(SNOW-1818849): unassign policies before dropping
err = client.RowAccessPolicies.Drop(ctx, sdk.NewDropRowAccessPolicyRequest(id).WithIfExists(sdk.Pointer(true)))
if err != nil {
return diag.Diagnostics{
Expand Down
17 changes: 17 additions & 0 deletions pkg/sdk/tags_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,14 @@ func (s *SetTagRequest) toOpts() *setTagOptions {
o.column = String(id.Name())
}
}
// TODO(SNOW-1818976): Remove this workaround. Currently ALTER "ORGNAME"."ACCOUNTNAME" SET TAG does not work, but ALTER "ACCOUNTNAME" does.
if o.objectType == ObjectTypeAccount {
id, ok := o.objectName.(AccountIdentifier)
if ok {
o.objectName = NewAccountIdentifierFromFullyQualifiedName(id.AccountName())
}
}

return o
}

Expand All @@ -167,5 +175,14 @@ func (s *UnsetTagRequest) toOpts() *unsetTagOptions {
o.column = String(id.Name())
}
}

// TODO(SNOW-1818976): Remove this workaround. Currently ALTER "ORGNAME"."ACCOUNTNAME" SET TAG does not work, but ALTER "ACCOUNTNAME" does.
if o.objectType == ObjectTypeAccount {
id, ok := o.objectName.(AccountIdentifier)
if ok {
o.objectName = NewAccountIdentifierFromFullyQualifiedName(id.AccountName())
}
}

return o
}
12 changes: 0 additions & 12 deletions pkg/sdk/tags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -377,12 +377,6 @@ func TestTagSet(t *testing.T) {
assertOptsInvalidJoinedErrors(t, opts, errors.New("tagging for object type SEQUENCE is not supported"))
})

t.Run("validation: unsupported account", func(t *testing.T) {
opts := defaultOpts()
opts.objectType = ObjectTypeAccount
assertOptsInvalidJoinedErrors(t, opts, errors.New("tagging for object type ACCOUNT is not supported - use Tags.SetOnCurrentAccount instead"))
})

t.Run("set with all optional", func(t *testing.T) {
opts := defaultOpts()
opts.SetTags = []TagAssociation{
Expand Down Expand Up @@ -434,12 +428,6 @@ func TestTagUnset(t *testing.T) {
assertOptsInvalidJoinedErrors(t, opts, errors.New("tagging for object type SEQUENCE is not supported"))
})

t.Run("validation: unsupported account", func(t *testing.T) {
opts := defaultOpts()
opts.objectType = ObjectTypeAccount
assertOptsInvalidJoinedErrors(t, opts, errors.New("tagging for object type ACCOUNT is not supported - use Tags.UnsetOnCurrentAccount instead"))
})

t.Run("unset with all optional", func(t *testing.T) {
opts := defaultOpts()
opts.UnsetTags = []ObjectIdentifier{
Expand Down
6 changes: 0 additions & 6 deletions pkg/sdk/tags_validations.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,6 @@ func (opts *setTagOptions) validate() error {
if !canBeAssociatedWithTag(opts.objectType) {
return fmt.Errorf("tagging for object type %s is not supported", opts.objectType)
}
if opts.objectType == ObjectTypeAccount {
return fmt.Errorf("tagging for object type ACCOUNT is not supported - use Tags.SetOnCurrentAccount instead")
}
return errors.Join(errs...)
}

Expand All @@ -171,8 +168,5 @@ func (opts *unsetTagOptions) validate() error {
if !canBeAssociatedWithTag(opts.objectType) {
return fmt.Errorf("tagging for object type %s is not supported", opts.objectType)
}
if opts.objectType == ObjectTypeAccount {
return fmt.Errorf("tagging for object type ACCOUNT is not supported - use Tags.UnsetOnCurrentAccount instead")
}
return errors.Join(errs...)
}
18 changes: 17 additions & 1 deletion pkg/sdk/testint/tags_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ func TestInt_TagsAssociations(t *testing.T) {
require.ErrorContains(t, err, "sql: Scan error on column index 0, name \"TAG\": converting NULL to string is unsupported")
}

t.Run("TestInt_TagAssociationForAccount", func(t *testing.T) {
t.Run("TestInt_TagAssociationForAccountLocator", func(t *testing.T) {
id := testClientHelper().Ids.AccountIdentifierWithLocator()
err := client.Accounts.Alter(ctx, &sdk.AlterAccountOptions{
SetTag: tags,
Expand Down Expand Up @@ -358,6 +358,22 @@ func TestInt_TagsAssociations(t *testing.T) {
require.ErrorContains(t, err, "sql: Scan error on column index 0, name \"TAG\": converting NULL to string is unsupported")
})

t.Run("TestInt_TagAssociationForAccount", func(t *testing.T) {
id := testClientHelper().Context.CurrentAccountIdentifier(t)
err := client.Tags.Set(ctx, sdk.NewSetTagRequest(sdk.ObjectTypeAccount, id).WithSetTags(tags))
require.NoError(t, err)

returnedTagValue, err := client.SystemFunctions.GetTag(ctx, tag.ID(), id, sdk.ObjectTypeAccount)
require.NoError(t, err)
assert.Equal(t, tagValue, returnedTagValue)

err = client.Tags.Unset(ctx, sdk.NewUnsetTagRequest(sdk.ObjectTypeAccount, id).WithUnsetTags(unsetTags))
require.NoError(t, err)

_, err = client.SystemFunctions.GetTag(ctx, tag.ID(), id, sdk.ObjectTypeAccount)
require.ErrorContains(t, err, "sql: Scan error on column index 0, name \"TAG\": converting NULL to string is unsupported")
})

accountObjectTestCases := []struct {
name string
objectType sdk.ObjectType
Expand Down
37 changes: 37 additions & 0 deletions templates/resources/authentication_policy.md.tmpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
---
page_title: "{{.Name}} {{.Type}} - {{.ProviderName}}"
subcategory: ""
description: |-
{{ if gt (len (split .Description "<deprecation>")) 1 -}}
{{ index (split .Description "<deprecation>") 1 | plainmarkdown | trimspace | prefixlines " " }}
{{- else -}}
{{ .Description | plainmarkdown | trimspace | prefixlines " " }}
{{- end }}
---

> [!WARNING]
> According to Snowflake [docs](https://docs.snowflake.com/en/sql-reference/sql/drop-authentication-policy#usage-notes), an authentication policy cannot be dropped successfully if it is currently assigned to another object. Currently, the provider does not unassign such objects automatically. Before dropping the resource, list the assigned objects with `SELECT * from table(information_schema.policy_references(policy_name=>'<string>'));` and unassign them manually with `ALTER ...` or with updated Terraform configuration, if possible.

# {{.Name}} ({{.Type}})

{{ .Description | trimspace }}

{{ if .HasExample -}}
## Example Usage

{{ tffile (printf "examples/resources/%s/resource.tf" .Name)}}

-> **Note** Instead of using fully_qualified_name, you can reference objects managed outside Terraform by constructing a correct ID, consult [identifiers guide](https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs/guides/identifiers#new-computed-fully-qualified-name-field-in-resources).
<!-- TODO(SNOW-1634854): include an example showing both methods-->

{{- end }}

{{ .SchemaMarkdown | trimspace }}
{{- if .HasImport }}

## Import

Import is supported using the following syntax:

{{ codefile "shell" (printf "examples/resources/%s/import.sh" .Name)}}
{{- end }}
3 changes: 3 additions & 0 deletions templates/resources/masking_policy.md.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ description: |-

!> **V1 release candidate** This resource was reworked and is a release candidate for the V1. We do not expect significant changes in it before the V1. We will welcome any feedback and adjust the resource if needed. Any errors reported will be resolved with a higher priority. We encourage checking this resource out before the V1 release. Please follow the [migration guide](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/MIGRATION_GUIDE.md#v0950--v0960) to use it.

> [!WARNING]
> According to Snowflake [docs](https://docs.snowflake.com/en/sql-reference/sql/drop-masking-policy#usage-notes), a masking policy cannot be dropped successfully if it is currently assigned to another object. Currently, the provider does not unassign such objects automatically. Before dropping the resource, list the assigned objects with `SELECT * from table(information_schema.policy_references(policy_name=>'<string>'));` and unassign them manually with `ALTER ...` or with updated Terraform configuration, if possible.

# {{.Name}} ({{.Type}})

{{ .Description | trimspace }}
Expand Down
3 changes: 3 additions & 0 deletions templates/resources/network_policy.md.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ description: |-

!> **V1 release candidate** This resource was reworked and is a release candidate for the V1. We do not expect significant changes in it before the V1. We will welcome any feedback and adjust the resource if needed. Any errors reported will be resolved with a higher priority. We encourage checking this resource out before the V1 release. Please follow the [migration guide](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/MIGRATION_GUIDE.md#v0920--v0930) to use it.

> [!WARNING]
> According to Snowflake [docs](https://docs.snowflake.com/en/sql-reference/sql/drop-network-policy#usage-notes), a network policy cannot be dropped successfully if it is currently assigned to another object. Currently, the provider does not unassign such objects automatically. Before dropping the resource, list the assigned objects with `SELECT * from table(information_schema.policy_references(policy_name=>'<string>'));` and unassign them manually with `ALTER ...` or with updated Terraform configuration, if possible.

# {{.Name}} ({{.Type}})

{{ .Description | trimspace }}
Expand Down
37 changes: 37 additions & 0 deletions templates/resources/password_policy.md.tmpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
---
page_title: "{{.Name}} {{.Type}} - {{.ProviderName}}"
subcategory: ""
description: |-
{{ if gt (len (split .Description "<deprecation>")) 1 -}}
{{ index (split .Description "<deprecation>") 1 | plainmarkdown | trimspace | prefixlines " " }}
{{- else -}}
{{ .Description | plainmarkdown | trimspace | prefixlines " " }}
{{- end }}
---

> [!WARNING]
> According to Snowflake [docs](https://docs.snowflake.com/en/sql-reference/sql/drop-password-policy#usage-notes), a password policy cannot be dropped successfully if it is currently assigned to another object. Currently, the provider does not unassign such objects automatically. Before dropping the resource, list the assigned objects with `SELECT * from table(information_schema.policy_references(policy_name=>'<string>'));` and unassign them manually with `ALTER ...` or with updated Terraform configuration, if possible.

# {{.Name}} ({{.Type}})

{{ .Description | trimspace }}

{{ if .HasExample -}}
## Example Usage

{{ tffile (printf "examples/resources/%s/resource.tf" .Name)}}

-> **Note** Instead of using fully_qualified_name, you can reference objects managed outside Terraform by constructing a correct ID, consult [identifiers guide](https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs/guides/identifiers#new-computed-fully-qualified-name-field-in-resources).
<!-- TODO(SNOW-1634854): include an example showing both methods-->

{{- end }}

{{ .SchemaMarkdown | trimspace }}
{{- if .HasImport }}

## Import

Import is supported using the following syntax:

{{ codefile "shell" (printf "examples/resources/%s/import.sh" .Name)}}
{{- end }}
3 changes: 3 additions & 0 deletions templates/resources/row_access_policy.md.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ description: |-

!> **V1 release candidate** This resource was reworked and is a release candidate for the V1. We do not expect significant changes in it before the V1. We will welcome any feedback and adjust the resource if needed. Any errors reported will be resolved with a higher priority. We encourage checking this resource out before the V1 release. Please follow the [migration guide](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/MIGRATION_GUIDE.md#v0950--v0960) to use it.

> [!WARNING]
> According to Snowflake [docs](https://docs.snowflake.com/en/sql-reference/sql/drop-row-access-policy#usage-notes), a row access policy cannot be dropped successfully if it is currently assigned to another object. Currently, the provider does not unassign such objects automatically. Before dropping the resource, list the assigned objects with `SELECT * from table(information_schema.policy_references(policy_name=>'<string>'));` and unassign them manually with `ALTER ...` or with updated Terraform configuration, if possible.

# {{.Name}} ({{.Type}})

{{ .Description | trimspace }}
Expand Down
Loading

0 comments on commit 9f67457

Please sign in to comment.