Enable Snowflake as a Database Resource #983

bjsemrad · 2021-02-22T18:14:46Z

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

To `vault_database_secret_backend_connection`, add support for the `snowflake-database-plugin`

Output from acceptance testing:

$make testacc TESTARGS='-run=TestAccDatabaseSecretBackendConnection_snowflake'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run=TestAccDatabaseSecretBackendConnection_snowflake -timeout 120m
?       github.com/hashicorp/terraform-provider-vault   [no test files]
?       github.com/hashicorp/terraform-provider-vault/cmd/coverage      [no test files]
?       github.com/hashicorp/terraform-provider-vault/cmd/generate      [no test files]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/codegen   (cached) [no tests to run]
?       github.com/hashicorp/terraform-provider-vault/generated [no test files]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/generated/datasources/transform/decode    (cached) [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/generated/datasources/transform/encode    (cached) [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/generated/resources/transform/alphabet    (cached) [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/generated/resources/transform/role        (cached) [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/generated/resources/transform/template    (cached) [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/generated/resources/transform/transformation      (cached) [no tests to run]
?       github.com/hashicorp/terraform-provider-vault/schema    [no test files]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-vault/util      (cached) [no tests to run]
=== RUN   TestAccDatabaseSecretBackendConnection_snowflake
hellow
--- PASS: TestAccDatabaseSecretBackendConnection_snowflake (2.80s)
PASS
ok      github.com/hashicorp/terraform-provider-vault/vault     2.847s
...

bjsemrad · 2021-02-22T18:17:29Z

I can remove the dependency updates, but I ran suggested vendor commands (and those were updated).

robbruce · 2021-06-09T09:21:04Z

@bjsemrad - can you get this rebased with master?

bjsemrad · 2021-06-23T01:21:44Z

@robbruce rebased, and updated mods/tidy sorry it took a bit.

chriskuchin · 2021-08-16T20:02:02Z

Would love to use this. Is there any plan to get this merged? Was just going to put this together basically the same and then found this one. Happy to look into the Conflicts if @bjsemrad is unable to at this point.

bjsemrad · 2021-08-16T20:04:15Z

@chriskuchin I’ll rebase this again tonight. Sorry not actively watching it.

chriskuchin · 2021-08-16T20:06:47Z

No worries thanks appreciate that.

@robbruce once that is fixed is there anything else that is needed to get this merged?

robbruce · 2021-08-16T20:14:32Z

@chriskuchin - I’m not a code maintainer, just a consumer. Thanks for keeping it up to date though!

chriskuchin · 2021-08-16T20:47:34Z

ahh ok I will see if i can chase down looking at git history a maintainer after the conflicts are cleaned up and see if we can get this merged

chriskuchin · 2021-08-16T20:50:57Z

@tvoran you seem to be the only active maintainer on this repo. Once the conflicts are cleaned up do you need anything else to get this added to the provider? Happy to write up an issue or anything else you need to get this pulled into the provider.

Thanks

bjsemrad · 2021-08-20T13:23:39Z

@chriskuchin PR was updated I'll try to watch this more closely.

chriskuchin · 2021-08-20T16:07:22Z

@bjsemrad awesome thanks.

@tvoran is there someone else I should be trying to coordinate this addition with? looking at the changelog and main branch commits seems like you are the only active maintainer. Happy to do whatever it takes to get this moved forward. Thanks!

.test-env

tvoran · 2021-08-21T07:28:32Z

vault/resource_database_secret_backend_connection.go

+						"account": {
+							Type:        schema.TypeString,
+							Required:    true,
+							Description: "The Account Name for the snowflake connection. Formats <accountname> if in us-west-2, or <accountname>.<region> for any other region",
+						},


I'm curious why this schema breaks the connection_url API parameter up into its components? Is it just for convenience?

What do you think about just following the API parameters, and including good examples in the docs for different types of connection_url's? It also looks like in order to support root credential rotation, we need to be able to specify the connection_url separately from the username and password.

Also this schema seems to be missing the max_open_connections, max_idle_connections, max_connection_lifetime, and username_template parameters.

I agree that just a connection_url makes sense.

The host name can be more than just account.region, depending if Snowflake organisations and/or PrivateLink is being used.

Yea I was trying to simplify the configuration / allow parameterization for terraform to abstract away what might change. Thinking about it more just simplifying it will make this require less changes in the future. Will update tonight.

Not sure how I missed the max connections etc, might have just not committed it originally.

@tvoran please check the latest, this should cover it and use the additional api items added after the original PR.

chriskuchin · 2021-08-26T19:18:07Z

@bjsemrad sorry to be that guy... But if you don't have time i can fork off of your changes and address the comments above

Co-authored-by: Theron Voran <tvoran@users.noreply.github.com>

chriskuchin

@bjsemrad this is awesome thanks so much for handling all this

vault/resource_database_secret_backend_connection.go

Co-authored-by: Theron Voran <tvoran@users.noreply.github.com>

tvoran

Almost there! Just one suggestion in the test, and there are a couple spacing issues that make fmt will fix.

vault/resource_database_secret_backend_connection_test.go

vault/resource_database_secret_backend_connection.go

website/docs/r/database_secret_backend_connection.md

vault/resource_database_secret_backend_connection_test.go

Co-authored-by: Theron Voran <tvoran@users.noreply.github.com>

tvoran

Thanks!

chriskuchin · 2021-08-31T15:58:42Z

Thanks really appreciate all the work here!!!

ghost added size/XXL dependencies labels Feb 22, 2021

bjsemrad force-pushed the snowflakeEnablement branch from 46a7584 to 78c6676 Compare June 23, 2021 01:20

github-actions bot added documentation size/XL labels Jun 23, 2021

chriskuchin approved these changes Aug 16, 2021

View reviewed changes

bjsemrad added 3 commits August 19, 2021 08:18

Enable Snowflake as a Database Resource

ae1f185

Enable Snowflake as a Database Resource - Cleanup

44ebbc0

Enable Snowflake as a Database Resource - Cleanup

e4e79d8

bjsemrad force-pushed the snowflakeEnablement branch from 78c6676 to e4e79d8 Compare August 19, 2021 13:23

github-actions bot added size/L and removed dependencies size/XL labels Aug 19, 2021

tvoran self-requested a review August 20, 2021 22:21

tvoran reviewed Aug 21, 2021

View reviewed changes

Update .test-env

8c8a33f

Co-authored-by: Theron Voran <tvoran@users.noreply.github.com>

Consolidate url and use standard db connection params that were missed.

1b585f1

chriskuchin approved these changes Aug 27, 2021

View reviewed changes

tvoran self-requested a review August 27, 2021 23:44

tvoran reviewed Aug 30, 2021

View reviewed changes

vault/resource_database_secret_backend_connection.go Outdated Show resolved Hide resolved

vault/resource_database_secret_backend_connection.go Outdated Show resolved Hide resolved

bjsemrad and others added 2 commits August 30, 2021 18:39

Update vault/resource_database_secret_backend_connection.go

85563fc

Co-authored-by: Theron Voran <tvoran@users.noreply.github.com>

Update vault/resource_database_secret_backend_connection.go

2d1a356

Co-authored-by: Theron Voran <tvoran@users.noreply.github.com>

tvoran reviewed Aug 31, 2021

View reviewed changes

bjsemrad and others added 5 commits August 31, 2021 08:14

Update vault/resource_database_secret_backend_connection_test.go

666d29d

Co-authored-by: Theron Voran <tvoran@users.noreply.github.com>

Update vault/resource_database_secret_backend_connection.go

15bb7a1

Co-authored-by: Theron Voran <tvoran@users.noreply.github.com>

Update vault/resource_database_secret_backend_connection_test.go

6b0a814

Co-authored-by: Theron Voran <tvoran@users.noreply.github.com>

Update website/docs/r/database_secret_backend_connection.md

0d0b1b7

Co-authored-by: Theron Voran <tvoran@users.noreply.github.com>

Update website/docs/r/database_secret_backend_connection.md

9823f4d

Co-authored-by: Theron Voran <tvoran@users.noreply.github.com>

tvoran approved these changes Aug 31, 2021

View reviewed changes

tvoran merged commit bb70eb5 into hashicorp:master Aug 31, 2021

bjsemrad deleted the snowflakeEnablement branch August 31, 2021 15:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable Snowflake as a Database Resource #983

Enable Snowflake as a Database Resource #983

bjsemrad commented Feb 22, 2021

bjsemrad commented Feb 22, 2021

robbruce commented Jun 9, 2021

bjsemrad commented Jun 23, 2021

chriskuchin commented Aug 16, 2021

bjsemrad commented Aug 16, 2021 •

edited

Loading

chriskuchin commented Aug 16, 2021

robbruce commented Aug 16, 2021

chriskuchin commented Aug 16, 2021

chriskuchin commented Aug 16, 2021

bjsemrad commented Aug 20, 2021

chriskuchin commented Aug 20, 2021

tvoran Aug 21, 2021

robbruce Aug 26, 2021

bjsemrad Aug 26, 2021 •

edited

Loading

bjsemrad Aug 27, 2021

chriskuchin commented Aug 26, 2021

chriskuchin left a comment

tvoran left a comment

tvoran left a comment

chriskuchin commented Aug 31, 2021

Enable Snowflake as a Database Resource #983

Enable Snowflake as a Database Resource #983

Conversation

bjsemrad commented Feb 22, 2021

Community Note

bjsemrad commented Feb 22, 2021

robbruce commented Jun 9, 2021

bjsemrad commented Jun 23, 2021

chriskuchin commented Aug 16, 2021

bjsemrad commented Aug 16, 2021 • edited Loading

chriskuchin commented Aug 16, 2021

robbruce commented Aug 16, 2021

chriskuchin commented Aug 16, 2021

chriskuchin commented Aug 16, 2021

bjsemrad commented Aug 20, 2021

chriskuchin commented Aug 20, 2021

tvoran Aug 21, 2021

Choose a reason for hiding this comment

robbruce Aug 26, 2021

Choose a reason for hiding this comment

bjsemrad Aug 26, 2021 • edited Loading

Choose a reason for hiding this comment

bjsemrad Aug 27, 2021

Choose a reason for hiding this comment

chriskuchin commented Aug 26, 2021

chriskuchin left a comment

Choose a reason for hiding this comment

tvoran left a comment

Choose a reason for hiding this comment

tvoran left a comment

Choose a reason for hiding this comment

chriskuchin commented Aug 31, 2021

bjsemrad commented Aug 16, 2021 •

edited

Loading

bjsemrad Aug 26, 2021 •

edited

Loading