feat: Rework config hierarchy #3166

sfc-gh-jmichalak · 2024-10-30T15:57:15Z

add remaining fields to the schema
deprecate account
implement and use a helper function for matching provider versions in acceptance tests
use helpers to fill config values
add acceptance tests for all fields in the config
move some code to internal package
improve documentation: describe config hierarchy and provide better config file examples
improve and test sdk.MergeConfig
move mock helper to a separate package because it caused unnecessarily registered sqlmock driver in one of the tests

Test Plan

acceptance tests

unit tests

References

#1881
#2145
#2925
#2983
#3104

TODO

acceptance test for fields regarding private keys - will be done in SNOW-1754319
unskip some tests after creating a compatible config for older versions

github-actions · 2024-10-30T18:24:51Z

Integration tests failure for 53cc9dc025b2e34c3914fe454d06c5ee5479b08c

github-actions · 2024-10-31T10:50:16Z

Integration tests cancelled for b3bb72dd89b7d9f21c693b392993bdbf249c82f2

github-actions · 2024-10-31T10:50:19Z

Integration tests cancelled for 94dfe58adcf7fd6909a90f66a1929c4f9f9765f5

MIGRATION_GUIDE.md

docs/index.md

pkg/provider/provider_helpers.go

pkg/sdk/config.go

pkg/testhelpers/helpers.go

github-actions · 2024-10-31T12:37:06Z

Integration tests failure for 83146de4c284f6eb624fdabd85fb1d4bda16b7c7

github-actions · 2024-10-31T18:09:32Z

Integration tests failure for bfd73028b05ca4d97d4f7bbea9d48d3b42b5cca2

github-actions · 2024-10-31T18:43:03Z

Integration tests failure for 5f0fa00258a238476972f22053fa727318ef32ef

sfc-gh-asawicki · 2024-11-04T14:48:16Z

MIGRATION_GUIDE.md

+- `account_name` and `organization_name` were added to improve handling account names. Read more in [docs](https://docs.snowflake.com/en/user-guide/admin-account-identifier#using-an-account-name-as-an-identifier).
+
+#### *(behavior change)* deprecated fields
+Because of new fields `account_name` and `organization_name`, `account` is now deprecated. Please adjust your configurations from


let's add here that we will remove this deprecated attribute with the V1 release

nit (for next PR): It will be removed before v1. -> It will be removed with the v1 release. (before V1 is inaccurate and AFAIK we do not plan to remove it before the V1)

README.md

docs/index.md

examples/provider/provider.tf

pkg/helpers/helpers.go

pkg/provider/provider.go

pkg/provider/provider_acceptance_test.go

sfc-gh-asawicki · 2024-11-04T15:39:37Z

pkg/provider/provider_acceptance_test.go

+					t.Setenv(snowflakeenvs.TmpDirectoryPath, "../")
+					t.Setenv(snowflakeenvs.DisableConsoleLogin, "false")
+				},
+				Config: providerConfig(testprofiles.CompleteFields),


question: was complete fields added:

to the GH stored config toml

to our 1pass (there is a config that should be updated)
?

Not yet - I was waiting for the review.

This will be addressed in the next pr because of incompatible config files in older versions.

pkg/provider/provider_acceptance_test.go

pkg/provider/provider_helpers.go

pkg/sdk/config.go

pkg/sdk/config_test.go

github-actions · 2024-11-04T16:25:43Z

Integration tests failure for af1a6f8d5ea244752ff2ec1631490341f84b7845

github-actions · 2024-11-05T15:12:38Z

Integration tests failure for 91c05617abf5ddfcd937d0d28e6ee63c1b5bfc0d

sfc-gh-asawicki · 2024-11-06T10:14:24Z

Makefile

@@ -74,7 +74,7 @@ test-architecture: ## check architecture constraints between packages
 	go test ./pkg/architests/... -v

 test-client: ## runs test that checks sdk.Client without instrumentedsql
-	SF_TF_NO_INSTRUMENTED_SQL=1 SF_TF_GOSNOWFLAKE_LOG_LEVEL=debug go test ./pkg/sdk/internal/client/... -v
+	SF_TF_NO_INSTRUMENTED_SQL=1 go test ./pkg/sdk/internal/client/... -v


do we have a follow-up task to turn on logging back here?

I thought this is not needed anymore. I think we can't reuse SNOWFLAKE_DRIVER_TRACING here because we would set it in NewClient, which means the value from the TF config would be overridden by the env. I can revert it in the next PR.

The problem here is that client initialization does not use environment variables (because using them from the Terraform level caused order problems), but the old SF_TF_GOSNOWFLAKE_LOG_LEVEL was handled additionally. We could handle the new env the same way as the old one, this could be the only env that is currently directly supported by the client initialization and the ordering is good: envs > toml.

pkg/provider/provider_helpers.go

sfc-gh-asawicki · 2024-11-06T10:26:35Z

pkg/sdk/internal/client/client_test.go

@@ -29,17 +28,3 @@ func TestNewClientWithoutInstrumentedSQL(t *testing.T) {
 		assert.Contains(t, sql.Drivers(), "snowflake")
 	})
 }
-
-func TestNewClientWithDebugLoggingSetFromEnv(t *testing.T) {


let's skip this one for now and add it to the issue I asked about in a separate comment

This can be reverted in the next PR.

github-actions · 2024-11-06T14:45:04Z

Integration tests failure for 91c05617abf5ddfcd937d0d28e6ee63c1b5bfc0d

github-actions · 2024-11-06T17:44:05Z

Integration tests failure for a888620c9a9bbc34730d7b96536a2399fa9aa74e

github-actions · 2024-11-07T10:51:36Z

Integration tests cancelled for 65e7b4c456e43ca7e30b4aa90e5adcf52a4cc49c

## [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>

config hierarchy

53cc9dc

sfc-gh-jmichalak added 2 commits October 31, 2024 11:17

wip

94dfe58

Cleanup

0f9a64f

sfc-gh-jmichalak requested review from sfc-gh-asawicki and sfc-gh-jcieslak and removed request for sfc-gh-asawicki October 31, 2024 10:26

Merge remote-tracking branch 'origin/main' into config-rework

b3bb72d

sfc-gh-jmichalak marked this pull request as ready for review October 31, 2024 10:29

Improve docs

83146de

sfc-gh-jcieslak reviewed Oct 31, 2024

View reviewed changes

sfc-gh-jmichalak added 2 commits October 31, 2024 17:09

Review

bfd7302

Cleanup

5f0fa00

Fix tests

af1a6f8

sfc-gh-asawicki reviewed Nov 4, 2024

View reviewed changes

sfc-gh-asawicki requested changes Nov 4, 2024

View reviewed changes

sfc-gh-jmichalak added 2 commits November 5, 2024 14:13

Fixes

135aac1

Merge remote-tracking branch 'origin/main' into config-rework

91c0561

sfc-gh-asawicki self-requested a review November 6, 2024 10:03

sfc-gh-asawicki reviewed Nov 6, 2024

View reviewed changes

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

View reviewed changes

sfc-gh-jcieslak previously approved these changes Nov 6, 2024

View reviewed changes

Skip tests with incompatible config

a888620

sfc-gh-jmichalak dismissed stale reviews from sfc-gh-jcieslak and sfc-gh-asawicki via a888620 November 6, 2024 15:27

sfc-gh-asawicki self-requested a review November 7, 2024 09:13

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

View reviewed changes

sfc-gh-jcieslak approved these changes Nov 7, 2024

View reviewed changes

Merge remote-tracking branch 'origin/main' into config-rework

65e7b4c

sfc-gh-jmichalak merged commit 04cd9f0 into main Nov 7, 2024
8 of 9 checks passed

sfc-gh-jmichalak deleted the config-rework branch November 7, 2024 10:51

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

chore(main): release 0.98.0 #3137

Merged

jrobison-sb mentioned this pull request Nov 13, 2024

[Bug] SNOW-1802641: provider organization_name and account_name when you only have an account locator #3198

Open

1 task

guineveresaenger mentioned this pull request Nov 21, 2024

Investigate failing registry docs build on upgrade-provider: bridge pulumi/pulumi-snowflake#752

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Rework config hierarchy #3166

feat: Rework config hierarchy #3166

sfc-gh-jmichalak commented Oct 30, 2024 •

edited

Loading

github-actions bot commented Oct 30, 2024

github-actions bot commented Oct 31, 2024

github-actions bot commented Oct 31, 2024

github-actions bot commented Oct 31, 2024

github-actions bot commented Oct 31, 2024

github-actions bot commented Oct 31, 2024

sfc-gh-asawicki Nov 4, 2024

sfc-gh-jmichalak Nov 5, 2024

sfc-gh-asawicki Nov 6, 2024

sfc-gh-asawicki Nov 4, 2024

sfc-gh-jmichalak Nov 5, 2024

sfc-gh-jmichalak Nov 7, 2024

github-actions bot commented Nov 4, 2024

github-actions bot commented Nov 5, 2024

sfc-gh-asawicki Nov 6, 2024

sfc-gh-jmichalak Nov 6, 2024 •

edited

Loading

sfc-gh-asawicki Nov 7, 2024

sfc-gh-asawicki Nov 6, 2024

sfc-gh-jmichalak Nov 6, 2024

github-actions bot commented Nov 6, 2024

github-actions bot commented Nov 6, 2024

github-actions bot commented Nov 7, 2024

feat: Rework config hierarchy #3166

feat: Rework config hierarchy #3166

Conversation

sfc-gh-jmichalak commented Oct 30, 2024 • edited Loading

Test Plan

References

TODO

github-actions bot commented Oct 30, 2024

github-actions bot commented Oct 31, 2024

github-actions bot commented Oct 31, 2024

github-actions bot commented Oct 31, 2024

github-actions bot commented Oct 31, 2024

github-actions bot commented Oct 31, 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

github-actions bot commented Nov 4, 2024

github-actions bot commented Nov 5, 2024

Choose a reason for hiding this comment

sfc-gh-jmichalak Nov 6, 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

github-actions bot commented Nov 6, 2024

github-actions bot commented Nov 6, 2024

github-actions bot commented Nov 7, 2024

sfc-gh-jmichalak commented Oct 30, 2024 •

edited

Loading

sfc-gh-jmichalak Nov 6, 2024 •

edited

Loading