fix: Connection and secret-datasource tests #3177

sfc-gh-fbudzynski · 2024-11-07T12:46:34Z

Changes

fix secrets datasource acceptance tests
added tests for ParseCommaSeparatedAccountIdentifierArray
added customDiff and tests for re-creating connection resources
adjusted migration-guide and documentation for connection resources
added 'TestAccPreCheck' to acceptance tests

Test Plan

unit tests
integration tests
acceptance tests
acceptance tests for secrets-datasource

References

feat: Connection resource #3162

…nowflake into fix-connection-resource

github-actions · 2024-11-07T15:17:00Z

Integration tests failure for 509846ce74244475a8bdabb25b36125c020923b7

MIGRATION_GUIDE.md

templates/resources/primary_connection.md.tmpl

sfc-gh-jmichalak · 2024-11-07T15:09:21Z

pkg/sdk/parsers_test.go

+		{
+			Name:   "list without brackets",
+			Value:  "A.B, C.D",
+			Result: []AccountIdentifier{NewAccountIdentifier("A", "B"), NewAccountIdentifier("C", "D")},


Please add some cases to test non-nil errors.

Should be addressed in the next pr.
cc @sfc-gh-fbudzynski

pkg/resources/primary_connection.go

sfc-gh-jmichalak · 2024-11-07T15:18:29Z

pkg/datasources/secrets_acceptance_test.go

@@ -216,11 +216,14 @@ func TestAcc_Secrets_WithGenericString(t *testing.T) {
 	})
 }

-func secretsData(secretResourceName string) string {
+func secretsData(secretResourceName string, inDatabaseName string) string {


Nit: I think it should be an account object identifier and FullyQualifiedName should be inside this func.

Should be addressed in the next pr (if needed).
cc @sfc-gh-fbudzynski

we can pass the secret identifier (which is a schemaObjectIdentifier) and use secretId.DatabaseId().FullyQualifiedName() inside the function and secretModel to retrieve the resource name

sfc-gh-asawicki · 2024-11-08T12:58:54Z

docs/resources/primary_connection.md

 ---

 !> **V1 release candidate** This resource is a release candidate for the V1. It is on the list of remaining GA objects for 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#v0970--v0980) to use it.

 # snowflake_primary_connection (Resource)

-Resource used to manage primary (not replicated) connections. For more information, check [connection documentation](https://docs.snowflake.com/en/sql-reference/sql/create-connection.html).
+Resource used to manage primary connections. For managing replicated connection check resource [snowflake_secondary_connection](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/resources/secondary_connection.md). For more information, check [connection documentation](https://docs.snowflake.com/en/sql-reference/sql/create-connection.html).


these paths should be relative instead of absolute (the given version of the provider should point to the given version) - example e.g. in the deprecated_resources.md

In all other places in this PR too

sfc-gh-jmichalak · 2024-11-08T13:13:07Z

templates/resources/primary_connection.md.tmpl

+
+-> **Note** To demote [`snowflake_primary_connection`](https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs/resources/primary_connection) to [`snowflake_secondary_connection`](https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs/resources/secondary_connection), resources need to be migrated manually. For guidance on removing and importing resources into the state check [resource migration](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/technical-documentation/resource_migration.md). Remove the resource from the state, then recreate it in manually using:
+    ```
+    CREATE CONNECTION <name> AS REPLICA OF <organization_name>.<account_name>.<connection_name>;


Nit: in such blocks, MD formatting is a bit different, and this gets inlined. This is fine; I just write for future reference.

I checked in https://registry.terraform.io/tools/doc-preview and this is how you can include more than one line in the note block

Hmm, strange. I checked the docs/issues some time ago and it was not possible to have newlines in these notes/warnings.

From: https://developer.hashicorp.com/terraform/registry/providers/docs#callouts, You can't make multi-paragraph callouts.

https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs/resources/primary_connection#example-usage This SQL got inlined.

github-actions · 2024-11-08T13:20:58Z

Integration tests cancelled for 12514387187e113e858d9e8f12753f47a26b3038

github-actions · 2024-11-08T13:30:01Z

Integration tests failure for 1eb98166a435cf0acccc6c7fddea004b4b16c76f

sfc-gh-asawicki · 2024-11-08T13:40:18Z

docs/resources/primary_connection.md


-> **Note** To demote [`snowflake_primary_connection`](https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs/resources/primary_connection) to [`snowflake_secondary_connection`](https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs/resources/secondary_connection), resources need to be migrated manually. For guidance on removing and importing resources into the state check [resource migration](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/technical-documentation/resource_migration.md). Remove the resource from the state, then recreate it in manually using:
+-> **Note** To demote [`snowflake_primary_connection`](./primary_connection) to [`snowflake_secondary_connection`](./secondary_connection), resources need to be migrated manually. For guidance on removing and importing resources into the state check [resource migration](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/technical-documentation/resource_migration.md). Remove the resource from the state, then recreate it in manually using:


nit (for later): do we need a self reference here?

I dont think the selfreference is needed, the reference to complementary resource would be enough. My thoughts only tho

sfc-gh-asawicki · 2024-11-08T13:41:14Z

docs/resources/secondary_connection.md

@@ -28,9 +28,9 @@ resource "snowflake_secondary_connection" "complete" {
 }
 ```

-> **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).
+-> **Note** Instead of using fully_qualified_name, you can reference objects managed outside Terraform by constructing a correct ID, consult [identifiers guide](../guides/identifiers#new-computed-fully-qualified-name-field-in-resources).


not sure if this one will be correct

sfc-gh-asawicki · 2024-11-08T13:43:18Z

templates/resources/primary_connection.md.tmpl

+
+-> **Note** To demote [`snowflake_primary_connection`](https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs/resources/primary_connection) to [`snowflake_secondary_connection`](https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs/resources/secondary_connection), resources need to be migrated manually. For guidance on removing and importing resources into the state check [resource migration](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/technical-documentation/resource_migration.md). Remove the resource from the state, then recreate it in manually using:
+    ```
+    CREATE CONNECTION <name> AS REPLICA OF <organization_name>.<account_name>.<connection_name>;


Hmm, strange. I checked the docs/issues some time ago and it was not possible to have newlines in these notes/warnings.

github-actions · 2024-11-08T14:18:26Z

Integration tests failure for 1511db144f4fd3a063c8f0c90d9e6664063b2fc8

## [0.98.0](v0.97.0...v0.98.0) (2024-11-08) Feature scope 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#wrap-up-the-functional-scope)). :exclamation: Migration guide: [v0.97.0 -> v0.98.0](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/MIGRATION_GUIDE.md#v0970--v0980) ### 🎉 What's new - New resources: - authentication_policy ([#3098](#3098)), references [#2880](#2880) - external_volume ([#3106](#3106)), partially references [#2980](#2980) - stream_on_directory_table ([#3129](#3129)) - stream_on_view ([#3150](#3150)) - primary_connection, secondary_connection ([#3162](#3162)) - secret_with_basic_authentication, secret_with_generic_string, secret_with_oauth_authorization_code_grant, secret_with_oauth_client_credentials ([#3110](#3110)), ([#3141](#3141)) - New data sources: - connections ([#3155](#3155)), ([#3173](#3173)) - secrets ([#3131](#3131)) - Reworked: - provider configuration hierarchy ([#3166](#3166)), references [#1881](#1881), [#2145](#2145), [#2925](#2925), [#2983](#2983), [#3104](#3104) - provider configuration fields ([#3152](#3152)) streams data source ([#3151](#3151)) - SDK upgrades: - Upgrade tag SDK ([#3126](#3126)) - Recreate streams when they are stale ([#3129](#3129)) ### 🔧 Misc - Add object renaming research summary ([#3172](#3172)) - Test support for object renaming ([#3130](#3130)), ([#3147](#3147)), ([#3154](#3154)) - Add tests to issue [#3117](#3117) ([#3133](#3133)) - New roadmap entry ([#3158](#3158)) - Test more authentication methods ([#3178](#3178)) - Minor fixes ([#3174](#3174)) ### 🐛 Bug fixes - Apply various fixes ([#3176](#3176)), this addresses BCR 2024_08, references [#2717](#2717), [#3005](#3005), [#3125](#3125), [#3127](#3127), [#3153](#3153) - Connection and secret data sources tests ([#3177](#3177)) - Fix grant import docs ([#3183](#3183)), resolves [#3179](https://github.com/Snowflake-Labs/terraform-provider-snowflake/discussions/3179) - Fix user resource import ([#3181](#3181)) - Handle external type changes in stream resources ([#3164](#3164)) - Do not use OR REPLACE on initial creation in resources with copy_grants ([#3129](#3129)) - Address issue [#2201](#2201) by introducing new stream resources Co-authored-by: snowflake-release-please[bot] <105954990+snowflake-release-please[bot]@users.noreply.github.com>

## Changes * added `../` to `primary_connection` documentation * added invalid test cases for `ParseCommaSeparatedAccountIdentifierArray` * changed `secretsData()` function parameters ## Test Plan  * [x] acceptance tests for secrets * [x] unit tests for parser ## References  references to comments: * #3177 (comment) * #3177 (comment)

sfc-gh-fbudzynski added 8 commits November 7, 2024 10:09

fix for secrets datasource failing

68ae2b1

added TestAccPreCheck to tests using

03d6f90

add TestAccPreCheck to secondary connection acceptance tests

ac41e18

parser tests

a9647cb

documentation, description and migration guide for connection resources

1119caa

documentation and fixes for connection resources

66b083c

Merge branch 'main' of github.com:Snowflake-Labs/terraform-provider-s…

756fac9

…nowflake into fix-connection-resource

remove defaults for computed fields

509846c

sfc-gh-fbudzynski marked this pull request as ready for review November 7, 2024 13:19

sfc-gh-fbudzynski requested review from sfc-gh-asawicki, sfc-gh-jmichalak and sfc-gh-jcieslak November 7, 2024 13:19

sfc-gh-jmichalak requested changes Nov 7, 2024

View reviewed changes

Changes after review

1eb9816

sfc-gh-asawicki previously approved these changes Nov 8, 2024

View reviewed changes

sfc-gh-jmichalak previously approved these changes Nov 8, 2024

View reviewed changes

sfc-gh-jcieslak dismissed stale reviews from sfc-gh-jmichalak and sfc-gh-asawicki via 1511db1 November 8, 2024 13:15

Changes after review

1251438

sfc-gh-jcieslak force-pushed the fix-connection-resource branch from 1511db1 to 1251438 Compare November 8, 2024 13:18

sfc-gh-jmichalak approved these changes Nov 8, 2024

View reviewed changes

sfc-gh-asawicki self-requested a review November 8, 2024 13:30

sfc-gh-asawicki approved these changes Nov 8, 2024

View reviewed changes

sfc-gh-jcieslak merged commit 167de4b into main Nov 8, 2024
8 of 9 checks passed

sfc-gh-jcieslak deleted the fix-connection-resource branch November 8, 2024 15:10

snowflake-release-please bot mentioned this pull request Nov 8, 2024

chore(main): release 0.98.0 #3137

Merged

sfc-gh-fbudzynski mentioned this pull request Nov 12, 2024

chore: parser and secret tests #3192

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Connection and secret-datasource tests #3177

fix: Connection and secret-datasource tests #3177

sfc-gh-fbudzynski commented Nov 7, 2024

github-actions bot commented Nov 7, 2024

sfc-gh-jmichalak Nov 7, 2024

sfc-gh-jcieslak Nov 8, 2024 •

edited

Loading

sfc-gh-jmichalak Nov 7, 2024

sfc-gh-jcieslak Nov 8, 2024

sfc-gh-fbudzynski Nov 12, 2024 •

edited

Loading

sfc-gh-asawicki Nov 8, 2024

sfc-gh-jmichalak Nov 8, 2024

sfc-gh-jcieslak Nov 8, 2024

sfc-gh-asawicki Nov 8, 2024

sfc-gh-asawicki Nov 8, 2024

sfc-gh-jmichalak Nov 12, 2024

github-actions bot commented Nov 8, 2024

github-actions bot commented Nov 8, 2024

sfc-gh-asawicki Nov 8, 2024

sfc-gh-fbudzynski Nov 12, 2024

sfc-gh-asawicki Nov 8, 2024

sfc-gh-jmichalak Nov 12, 2024

sfc-gh-asawicki Nov 8, 2024

github-actions bot commented Nov 8, 2024


		-> Note To demote [`snowflake_primary_connection`](https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs/resources/primary_connection) to [`snowflake_secondary_connection`](https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs/resources/secondary_connection), resources need to be migrated manually. For guidance on removing and importing resources into the state check [resource migration](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/technical-documentation/resource_migration.md). Remove the resource from the state, then recreate it in manually using:
		-> Note To demote [`snowflake_primary_connection`](./primary_connection) to [`snowflake_secondary_connection`](./secondary_connection), resources need to be migrated manually. For guidance on removing and importing resources into the state check [resource migration](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/technical-documentation/resource_migration.md). Remove the resource from the state, then recreate it in manually using:

fix: Connection and secret-datasource tests #3177

fix: Connection and secret-datasource tests #3177

Conversation

sfc-gh-fbudzynski commented Nov 7, 2024

Changes

Test Plan

References

github-actions bot commented Nov 7, 2024

Choose a reason for hiding this comment

sfc-gh-jcieslak Nov 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfc-gh-fbudzynski Nov 12, 2024 • edited Loading

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

github-actions bot commented Nov 8, 2024

github-actions bot commented Nov 8, 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

github-actions bot commented Nov 8, 2024

sfc-gh-jcieslak Nov 8, 2024 •

edited

Loading

sfc-gh-fbudzynski Nov 12, 2024 •

edited

Loading