feat: Database resource v1 readiness #2834

sfc-gh-jcieslak · 2024-05-24T06:41:40Z

Changes done:

Added two resources snowflake_shared_database and snowflake_secondary_database
Added nested objects for hierarchical values (Optional and Computed) like data_retention_time_in_days and max_data_extension_time_in_days
- Also added a customdiff for nested objects that are hierarchical values, so that they're refreshed correctly (checked by acceptance test for secondary database)
Added privilege requested in [Feature]: Grant DATASETS usage to a role through terraform #2807
Wrote diff suppress functions for LogLevel and TraceLevel fields, because unset value is always equal to OFF, so diff suppress is needed in a case where no value is set (empty string) and OFF is set in the Read operation.

Not sure about:

I'm not sure about identifier field names (e.g. external_volume or external_volume_name)
Not sure about external identifier names (from_share and as_replica_of)

Standard database resource
Data source
Address listed issues

Test Plan

acceptance tests testing

References

Create database

github-actions · 2024-05-24T07:06:20Z

Integration tests failure for f1cc3f3feaef6d46b62c58f21b2169930f86ae34

github-actions · 2024-05-24T11:05:40Z

Integration tests success for 92be484088a48be68bfd059cdc44f62bf36e779d

sfc-gh-jmichalak

In some places there's missing REPLACE_INVALID_CHARACTERS and STORAGE_SERIALIZATION_POLICY

v1-preparations/ESSENTIAL_GA_OBJECTS.MD

pkg/sdk/testint/databases_integration_test.go

docs/resources/secondary_database.md

docs/resources/shared_database.md

examples/resources/snowflake_shared_database/resource.tf

pkg/resources/helpers.go

pkg/resources/shared_database.go

pkg/resources/shared_database_acceptance_test.go

sfc-gh-asawicki · 2024-05-27T11:12:28Z

docs/resources/secondary_database.md

+
+### Required
+
+- `as_replica_of` (String) A fully qualified path to a database to create a replica from. A fully qualified path follows the format of `"<organization_name>"."<account_name>"."<database_name>"`.


just checking: this is the new recommended Snowflake way of identifying accounts, right?

Yes, but I changed to the documentation one <provider_account>.<share_name>, because we should be able to handle both identifiers. I'll modify one integration and acceptance test to show it's working.

On second thought, we can easily support both formats on SDK level, but it's hard to support both ways in Terraform because in Read we have to assume one format. For now, I'll revert the acceptance test, because it's failing for account locator and maybe we should enforce the new format instead of supporting the old one(?). That's why originally I stayed with this description, I can revert the change to "<organization_name>"."<account_name>"."<database_name>".

I think that it would be best to stick with the SF recommended way. This is a change against the old behavior but it is a new resource after all. However, we should mention this change in the migration notes.

Then I'll revert the format (in the next pr) and add a note in migration guide on expected external identifiers

docs/resources/secondary_database.md

docs/resources/shared_database.md

MIGRATION_GUIDE.md

pkg/resources/custom_diffs.go

pkg/resources/secondary_database_acceptance_test.go

pkg/resources/secondary_database.go

github-actions · 2024-05-28T09:41:20Z

Integration tests failure for 27b42e6be7cec72034780b2b90cb2e0c0a2034f5

pkg/resources/secondary_database.go

github-actions · 2024-05-28T09:48:42Z

Integration tests failure for af1033b3a79aaac43fc638f997d262bc5eac8286

github-actions · 2024-05-28T10:18:39Z

Integration tests failure for f0823fc93b95d98c2ae5ced9004ae749665c91ec

github-actions · 2024-05-28T10:40:00Z

Integration tests failure for 67c388955e4e149a90953c50e701267785fbee74

github-actions · 2024-05-28T11:46:34Z

Integration tests success for f0823fc93b95d98c2ae5ced9004ae749665c91ec

pkg/resources/shared_database.go

sfc-gh-asawicki · 2024-05-29T07:59:13Z

pkg/sdk/context_functions.go

@@ -12,6 +12,8 @@ import (
 type ContextFunctions interface {
 	// Session functions.
 	CurrentAccount(ctx context.Context) (string, error)
+	CurrentOrganizationName(ctx context.Context) (string, error)


nit-pick (maybe for some other PR): there is below a CurrentSessionDetails that contains output for all other singular methods here (so, e.g. it could be used as single query instead of the two that are inside the helper).

Adjusted CurrentSessionDetails in the next pr to have those values and used it in the helper.

sfc-gh-asawicki · 2024-05-29T07:59:47Z

pkg/sdk/context_functions.go

@@ -12,6 +12,8 @@ import (
 type ContextFunctions interface {
 	// Session functions.
 	CurrentAccount(ctx context.Context) (string, error)
+	CurrentOrganizationName(ctx context.Context) (string, error)
+	CurrentAccountName(ctx context.Context) (string, error)


any integration tests for these? 👮‍♂️

Added in the next pr.

sfc-gh-asawicki · 2024-05-29T08:00:36Z

pkg/acceptance/helpers/database_client.go

@@ -25,6 +25,30 @@ func (c *DatabaseClient) client() sdk.Databases {
 	return c.context.client.Databases
 }

+func (c *DatabaseClient) CreatePrimaryDatabase(t *testing.T, enableReplicationTo []sdk.AccountIdentifier) (*sdk.Database, sdk.ExternalObjectIdentifier, func()) {


nit-pick: let's not make it the first method in this file (look where secondary is located)

Moved it under secondary.

sfc-gh-asawicki · 2024-05-29T08:10:32Z

pkg/sdk/identifier_helpers.go

@@ -123,7 +123,7 @@ func NewAccountIdentifier(organizationName, accountName string) AccountIdentifie

 func NewAccountIdentifierFromAccountLocator(accountLocator string) AccountIdentifier {
 	return AccountIdentifier{
-		accountLocator: accountLocator,
+		accountLocator: strings.Trim(accountLocator, `"`),


a bit afraid of it; let's leave it for the identifiers rework

Removed trim

Changes done: - Added two resources `snowflake_shared_database` and `snowflake_secondary_database` - Added nested objects for hierarchical values (Optional and Computed) like `data_retention_time_in_days` and `max_data_extension_time_in_days` - Also added a customdiff for nested objects that are hierarchical values, so that they're refreshed correctly (checked by acceptance test for secondary database) - Added privilege requested in #2807 - Wrote diff suppress functions for LogLevel and TraceLevel fields, because unset value is always equal to `OFF`, so diff suppress is needed in a case where no value is set (empty string) and `OFF` is set in the Read operation. Not sure about: - I'm not sure about identifier field names (e.g. `external_volume` or `external_volume_name`) - Not sure about external identifier names (`from_share` and `as_replica_of`) Next: - Standard database resource - Data source - Address listed issues ## Test Plan * [x] acceptance tests testing ## References  * [Create database](https://docs.snowflake.com/en/sql-reference/sql/create-database)

🤖 I have created a release *beep* *boop* --- ## [0.92.0](v0.91.0...v0.92.0) (2024-06-06) ### 🎉 **What's new:** * Add Api Authentication security integration to sdk ([#2840](#2840)) ([57a07ee](57a07ee)) * Add External Oauth security integration to sdk ([#2835](#2835)) ([82d1c09](82d1c09)) * add network rules ([#2746](#2746)) ([c79fa29](c79fa29)) * Add SCIM and SAML2 security integrations to sdk ([#2799](#2799)) ([1312ff1](1312ff1)) * Add Snowflake Oauth security integration to sdk ([#2830](#2830)) ([b576f29](b576f29)) * Database resource v1 readiness ([#2834](#2834)) ([30fe136](30fe136)) * Database SDK upgrade ([#2814](#2814)) ([750fe37](750fe37)) ### 🔧 **Misc** * accept non-pointer values in the generated builder methods ([#2816](#2816)) ([c29fbf1](c29fbf1)) * Add a script for creating labels ([#2778](#2778)) ([ce0fbad](ce0fbad)) * Adjust before 0.92.0 ([#2857](#2857)) ([0598656](0598656)) * Continue random ids rework ([#2819](#2819)) ([f20940c](f20940c)) * Random ids rework part3 ([#2833](#2833)) ([36ead85](36ead85)) * Random ids rework part4 ([#2837](#2837)) ([64518a3](64518a3)) * Update issue for table and warehouse redesign state ([#2845](#2845)) ([149e55e](149e55e)) ### 🐛 **Bug fixes:** * Fix failing integration tests ([#2832](#2832)) ([2e2ca6c](2e2ca6c)) * Fix QUOTED_IDENTIFIERS_IGNORE_CASE parameter test ([#2841](#2841)) ([92ad1d3](92ad1d3)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: snowflake-release-please[bot] <105954990+snowflake-release-please[bot]@users.noreply.github.com>

Done in this pr: - Addressed comments from the [previous pr](#2834) - Added new `standard_database` resource (also added examples, import, mentioned in migration notes) - Deprecate old database resource - Adjust `databases` data-source to align with v1 requirements (add filters, missing values and outputs from DESCRIBE and SHOW PARAMETERS commands) - Replaced `snowflake_database` in all of the examples to `snowflake_standard_database` To be done: - Add missing properties on all three new database types - Make sure all of the issues were resolved with the new types of databases ## Test Plan  * [x] acceptance tests ## References  [CREATE DATABASE](https://docs.snowflake.com/en/sql-reference/sql/create-database) ## Update Changes done: - Added missing parameters to all the database types and moved them and operations on them to a common place (only metric_level parameter wasn't included as it is a preview feature and there wasn't enough information about and it seemed like ORGADMIN or certain privileges were required to be able to test/use it). - Switched to plain values instead of nested ones for parameters and adjusted customdiffs, so that the state is refreshed always when expected. - Every database type resolves database-connected issues (most if not all of them were already resolved by the latest versions of the provider for the `snowflake_database` resource). - Refresh for secondary was not added, as replication guidelines are recommending to create a task that would be refreshing the replica at a certain interval. To aid our users I created an example showing how to create a task that would run the refresh every 10 minutes. An easy upgrade (if we would like to) would be add a toggle to call refresh every read operation. The toggle could be turned on by default with the option to turn it off and refresh it in the task "manually". - State upgrader for snowflake_database (because we chose to rename the old one for have _old suffix).

sfc-gh-jcieslak added 4 commits May 24, 2024 08:29

wip

b6b0fb0

wip

19958a7

wip

1303dc6

wip

f1cc3f3

Add docs

92be484

sfc-gh-jcieslak marked this pull request as ready for review May 24, 2024 10:02

sfc-gh-jmichalak requested changes May 27, 2024

View reviewed changes

sfc-gh-asawicki self-requested a review May 27, 2024 11:09

sfc-gh-asawicki requested changes May 27, 2024

View reviewed changes

Changes after review

27b42e6

sfc-gh-jcieslak commented May 28, 2024

View reviewed changes

pkg/resources/secondary_database.go Show resolved Hide resolved

Merge branch 'refs/heads/main' into database-resource-v1-readiness

c52566d

sfc-gh-jcieslak force-pushed the database-resource-v1-readiness branch from af1033b to 67c3889 Compare May 28, 2024 09:47

Changes after review

f0823fc

sfc-gh-jcieslak force-pushed the database-resource-v1-readiness branch from 67c3889 to f0823fc Compare May 28, 2024 09:49

sfc-gh-jmichalak approved these changes May 28, 2024

View reviewed changes

sfc-gh-jcieslak commented May 28, 2024

View reviewed changes

pkg/resources/shared_database.go Show resolved Hide resolved

sfc-gh-asawicki self-requested a review May 28, 2024 16:20

sfc-gh-asawicki approved these changes May 29, 2024

View reviewed changes

sfc-gh-jcieslak merged commit 30fe136 into main May 29, 2024
9 checks passed

sfc-gh-jcieslak deleted the database-resource-v1-readiness branch May 29, 2024 10:01

snowflake-release-please bot mentioned this pull request May 29, 2024

chore(main): release 0.92.0 #2824

Merged

sfc-gh-jcieslak mentioned this pull request Jun 4, 2024

feat: standard database v1 readiness #2842

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Database resource v1 readiness #2834

feat: Database resource v1 readiness #2834

sfc-gh-jcieslak commented May 24, 2024 •

edited

Loading

github-actions bot commented May 24, 2024

github-actions bot commented May 24, 2024

sfc-gh-jmichalak left a comment

sfc-gh-asawicki May 27, 2024

sfc-gh-jcieslak May 27, 2024

sfc-gh-jcieslak May 28, 2024 •

edited

Loading

sfc-gh-asawicki May 29, 2024

sfc-gh-jcieslak May 29, 2024 •

edited

Loading

github-actions bot commented May 28, 2024

github-actions bot commented May 28, 2024

github-actions bot commented May 28, 2024

github-actions bot commented May 28, 2024

github-actions bot commented May 28, 2024

sfc-gh-asawicki May 29, 2024

sfc-gh-jcieslak May 29, 2024

sfc-gh-asawicki May 29, 2024

sfc-gh-jcieslak May 29, 2024

sfc-gh-asawicki May 29, 2024

sfc-gh-jcieslak May 29, 2024

sfc-gh-asawicki May 29, 2024

sfc-gh-jcieslak May 29, 2024


		### Required

		- `as_replica_of` (String) A fully qualified path to a database to create a replica from. A fully qualified path follows the format of `"<organization_name>"."<account_name>"."<database_name>"`.

feat: Database resource v1 readiness #2834

feat: Database resource v1 readiness #2834

Conversation

sfc-gh-jcieslak commented May 24, 2024 • edited Loading

Test Plan

References

github-actions bot commented May 24, 2024

github-actions bot commented May 24, 2024

sfc-gh-jmichalak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfc-gh-jcieslak May 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfc-gh-jcieslak May 29, 2024 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented May 28, 2024

github-actions bot commented May 28, 2024

github-actions bot commented May 28, 2024

github-actions bot commented May 28, 2024

github-actions bot commented May 28, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfc-gh-jcieslak commented May 24, 2024 •

edited

Loading

sfc-gh-jcieslak May 28, 2024 •

edited

Loading

sfc-gh-jcieslak May 29, 2024 •

edited

Loading