feat: Masking policy resource v1 #3078

sfc-gh-jmichalak · 2024-09-16T09:39:40Z

SDK
- improve validations
- add tests
- improve options parsing (also, get rid of a 3rd party library)
- add a TODO to reuse parsing from row access policies after merging fix: Fix views permadiff #3079
resource
- use new id handling
- rename/remove some fields to be consistent with other resources
- better ACC tests
misc
adjust helper clients
generate config and assertion builders
adjust docs

Test Plan

acceptance tests
integration tests
unit tests

References

https://docs.snowflake.com/en/sql-reference/sql/create-masking-policy

TODO

add tests for issues
rework data source

github-actions · 2024-09-16T15:44:06Z

Integration tests failure for 97e53056f97a9bd19c39c21fdecf8b477a6f315e

github-actions · 2024-09-16T16:02:05Z

Integration tests failure for c3fed8307d4d491bbb497c437f785938d53f382a

MIGRATION_GUIDE.md

pkg/resources/masking_policy.go

pkg/resources/masking_policy_acceptance_test.go

pkg/schemas/masking_policy.go

pkg/sdk/masking_policy.go

sfc-gh-jcieslak · 2024-09-17T08:33:53Z

pkg/sdk/masking_policy.go

@@ -346,6 +373,7 @@ func (row maskingPolicyDetailsRow) toMaskingPolicyDetails() *MaskingPolicyDetail
 		ReturnType: dataType,
 		Body:       row.Body,
 	}
+	// TODO (after merged changes in row access policies) use/merge with parsing in row access policies


Leaving a comment as a reminder (will resolve after the upcoming change)

Ok, done. I adjusted this also for row access policies.

github-actions · 2024-09-17T10:07:25Z

Integration tests failure for ed7a13f1070b4fb6785c9be6333f1a18127ae0a8

github-actions · 2024-09-17T10:43:15Z

Integration tests failure for 8573b9dababe6e599e6ebcf9eb81dd086558819b

github-actions · 2024-09-17T10:59:10Z

Integration tests failure for bd36e593d457119c834c1a79729227cafe4a426c

github-actions · 2024-09-17T12:29:07Z

Integration tests failure for ca5af1835c93bfba07de367e016f1b7d4a2ec0ef

github-actions · 2024-09-17T16:14:45Z

Integration tests failure for 09b60098bde37d9e2b0d73d0fc9392fb7cfe963a

github-actions · 2024-09-18T10:08:27Z

Integration tests failure for 03612eff7ebaeae5929fb37019146d0c7dcb950f

- SDK - improve validations - add tests - improve options parsing (also, get rid of a 3rd party library) - add a TODO to reuse parsing from row access policies after merging #3079 - resource - use new id handling - rename/remove some fields to be consistent with other resources - better ACC tests - misc - adjust helper clients - generate config and assertion builders - adjust docs  ## Test Plan  * [x] acceptance tests * [x] integration tests * [x] unit tests ## References  https://docs.snowflake.com/en/sql-reference/sql/create-masking-policy ## TODO - add tests for issues - rework data source

🤖 I have created a release *beep* *boop* --- ## [0.96.0](v0.95.0...v0.96.0) (2024-09-18) Essential GA object readiness for V1: [link](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/v1-preparations/ESSENTIAL_GA_OBJECTS.MD) ([Roadmap reference](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/ROADMAP.md#preparing-essential-ga-objects-for-the-provider-v1)). :exclamation: Migration guide: [v0.95.0 -> v0.96.0](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/MIGRATION_GUIDE.md#v0950--v0960) ### 🎉 **What's new:** * V1 redesign of resources and data sources * Row access policy ([#3066](#3066)) ([#3063](#3063)) * Resource monitor ([#3052](#3052)) ([#3064](#3064)) * Masking policy ([#3078](#3078)) ([#3083](#3083)) * SDK upgrades * External volume ([#3033](#3033)) * Authentication policy ([#2937](#2937)) ([#3068](#3068)) ([#3061](#3061)) ### 🔧 **Misc** * Clean up old test object helpers ([#3049](#3049)) * Add example of granting role to multiple objects ([#3047](#3047)) * Update readme and objects rework state ([#3046](#3046)) ### 🐛 **Bug fixes:** * Fix model grants ([#3070](#3070)) * Fix database show by and resource logic ([#3055](#3055)) * Fix default secondary roles option import ([#3041](#3041)) * Fix sweepers for warehouse and database ([#3057](#3057)) * Fix views permadiff ([#3079](#3079)) * Update v0.95.0 migration guide ([#3062](#3062)) Co-authored-by: snowflake-release-please[bot] <105954990+snowflake-release-please[bot]@users.noreply.github.com>

sfc-gh-asawicki · 2024-10-01T18:26:10Z

docs/resources/masking_policy.md

- `exempt_other_policies` (Boolean) Specifies whether the row access policy or conditional masking policy can reference a column that is already protected by a masking policy.
- `if_not_exists` (Boolean) Prevent overwriting a previous masking policy with the same name.
- `or_replace` (Boolean) Whether to override a previous masking policy with the same name.
+- `exempt_other_policies` (String) Specifies whether the row access policy or conditional masking policy can reference a column that is already protected by a masking policy. Due to Snowflake limitations, when value is chenged, the resource is recreated. Available options are: "true" or "false". When the value is not set in the configuration the provider will put "default" there which means to use the Snowflake default for this value.


chenged -> changed

sfc-gh-asawicki · 2024-10-01T18:35:03Z

pkg/resources/row_access_policy.go

-		log.Printf("[DEBUG] row access policy (%s) not found", d.Id())
-		d.SetId("")
-		return nil
+		if errors.Is(err, sdk.ErrObjectNotFound) {


Is there a test showing that it was missing?

sfc-gh-asawicki · 2024-10-01T18:37:00Z

pkg/sdk/masking_policy_test.go

+		assertOptsInvalidJoinedErrors(t, opts, ErrDifferentSchema)
+	})
+
+	t.Run("validation: only 1 option allowed at the same time", func(t *testing.T) {


usually we check both empty and multiple

sfc-gh-asawicki · 2024-10-01T18:41:02Z

pkg/resources/masking_policy_acceptance_test.go

+	policyModel := model.MaskingPolicy("test", []sdk.TableColumnSignature{
+		{
+			Name: "a",
+			Type: "TEXT",


nit-pick: why we are sometimes using sdk.DataType and sometimes string?

sfc-gh-asawicki · 2024-10-01T18:47:32Z

pkg/resources/masking_policy_acceptance_test.go

+			// set all fields
+			{
+				ConfigDirectory: acc.ConfigurationDirectory("TestAcc_MaskingPolicy/complete"),
+				ConfigVariables: tfconfig.ConfigVariablesFromModel(t, policyModel.WithBody(body).WithComment("Terraform acceptance test").WithExemptOtherPolicies(r.BooleanTrue)),


in the next step there are more fields set, so the "all" is inappropriate here

sfc-gh-asawicki · 2024-10-01T18:54:57Z

pkg/resources/masking_policy_acceptance_test.go

 			{
-				Config: maskingPolicyConfig(newId.Name(), comment2, acc.TestDatabaseName, acc.TestSchemaName),
+				ConfigDirectory: acc.ConfigurationDirectory("TestAcc_MaskingPolicy/complete"),
+				ConfigVariables: tfconfig.ConfigVariablesFromModel(t, policyModel.WithBody(bodyWithBooleanReturnType).WithReturnDataType(string(sdk.DataTypeBoolean)).WithArgument(argumentWithChangedFirstArgumentType).WithComment("Terraform acceptance test - changed comment").WithExemptOtherPolicies(r.BooleanFalse)),


The way that policyModel is reused here is interesting/unsafe - that was not the initial intention to reuse it this way. It bases on the order of execution and is a bit misleading (e.g. in step 4 and step 5 we want to validate only external change but we use "different" config variables - different only in the written way, underneath it's the same pointer, with fields already set in the previous step).

Maybe the design of model is a bit flawed and we should not use the pointer but the model directly, so that we always get the copied struct. Either way, I am raising it as a topic to discuss internally.

sfc-gh-jmichalak added 4 commits September 16, 2024 11:39

Init

81346b6

Cleanup

19ddfb7

Cleanup

b387a1f

Merge remote-tracking branch 'origin/main' into masking-policy-v1

97e5305

sfc-gh-jmichalak marked this pull request as ready for review September 16, 2024 14:05

sfc-gh-jmichalak requested a review from sfc-gh-jcieslak September 16, 2024 14:05

Cleanup

c3fed83

Fix tests

ed7a13f

sfc-gh-jcieslak reviewed Sep 17, 2024

View reviewed changes

sfc-gh-jmichalak added 3 commits September 17, 2024 11:16

Review suggestions

8573b9d

pre push

bd36e59

Adjust migration guide

3510612

sfc-gh-jcieslak previously approved these changes Sep 17, 2024

View reviewed changes

Merge remote-tracking branch 'origin/main' into masking-policy-v1

ca5af18

sfc-gh-jmichalak dismissed sfc-gh-jcieslak’s stale review via ca5af18 September 17, 2024 10:50

sfc-gh-jmichalak added 2 commits September 17, 2024 15:39

Merge remote-tracking branch 'origin/main' into masking-policy-v1

badb691

Resolve signature parsers

09b6009

Fix tests

03612ef

sfc-gh-jcieslak approved these changes Sep 18, 2024

View reviewed changes

sfc-gh-jmichalak merged commit fa4ce56 into main Sep 18, 2024
8 of 9 checks passed

sfc-gh-jmichalak deleted the masking-policy-v1 branch September 18, 2024 10:58

snowflake-release-please bot mentioned this pull request Sep 18, 2024

chore(main): release 0.96.0 #3042

Merged

sfc-gh-asawicki reviewed Oct 1, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Masking policy resource v1 #3078

feat: Masking policy resource v1 #3078

sfc-gh-jmichalak commented Sep 16, 2024 •

edited

Loading

github-actions bot commented Sep 16, 2024

github-actions bot commented Sep 16, 2024

sfc-gh-jcieslak Sep 17, 2024

sfc-gh-jmichalak Sep 17, 2024

github-actions bot commented Sep 17, 2024

github-actions bot commented Sep 17, 2024

github-actions bot commented Sep 17, 2024

github-actions bot commented Sep 17, 2024

github-actions bot commented Sep 17, 2024

github-actions bot commented Sep 18, 2024

sfc-gh-asawicki Oct 1, 2024

sfc-gh-asawicki Oct 1, 2024

sfc-gh-asawicki Oct 1, 2024

sfc-gh-asawicki Oct 1, 2024

sfc-gh-asawicki Oct 1, 2024

sfc-gh-asawicki Oct 1, 2024

feat: Masking policy resource v1 #3078

feat: Masking policy resource v1 #3078

Conversation

sfc-gh-jmichalak commented Sep 16, 2024 • edited Loading

Test Plan

References

TODO

github-actions bot commented Sep 16, 2024

github-actions bot commented Sep 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Sep 17, 2024

github-actions bot commented Sep 17, 2024

github-actions bot commented Sep 17, 2024

github-actions bot commented Sep 17, 2024

github-actions bot commented Sep 17, 2024

github-actions bot commented Sep 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfc-gh-jmichalak commented Sep 16, 2024 •

edited

Loading