Add support rootPassword property for MS SQL Server #2592

kopachevsky · 2019-11-06T15:04:03Z

Added root_password parameter required MS SQL Server instance creation, ignored MySQL and PostgreSQL

Fixes hashicorp/terraform-provider-google/issues/4776

`sql`: added `root_password` field to `google_sql_database_instance` resource

modular-magician · 2019-11-06T15:06:10Z

Hello! I am a robot who works on Magic Modules PRs.

I have detected that you are a community contributor, so your PR will be assigned to someone with a commit-bit on this repo for initial review. They will authorize it to run through our CI pipeline, which will generate downstream PRs.

Thanks for your contribution! A human will be with you soon.

@danawillow, please review this PR or find an appropriate assignee.

danawillow · 2019-11-06T23:37:59Z

Hi @kopachevsky, I see that you've marked this as a draft. Can you please let me know when it's ready for review, or if there's anything you'd like help figuring out? Thanks!

kopachevsky · 2019-11-07T10:54:45Z

Hi @kopachevsky, I see that you've marked this as a draft. Can you please let me know when it's ready for review, or if there's anything you'd like help figuring out? Thanks!

@danawillow I'm currently testing it with terraform module I'm working on, regarding help - can you clarify for me requirements to switch it to review state, I did not run any kind of integration testing and didn't add any unit tests, is it required?

danawillow · 2019-11-08T02:14:52Z

Feel free to change it to the review state whenever you'd like me to take a look at it. A test would be great- you can take a look at our existing ones at https://github.com/GoogleCloudPlatform/magic-modules/blob/master/third_party/terraform/tests/resource_sql_database_instance_test.go.erb. We have some guidance on running tests at https://github.com/GoogleCloudPlatform/magic-modules#testing-your-changes.

kopachevsky · 2019-11-12T11:49:57Z

@danawillow ready for review

danawillow · 2019-11-15T23:03:52Z

Thanks! The code looks reasonable at a glance, but I'm going to hold off until there are tests. Sounds like that's blocked on https://github.com/GoogleCloudPlatform/magic-modules/issues/2641, so I commented there.

morgante · 2019-11-20T10:22:26Z

@kopachevsky Will you be able to add tests to this?

kopachevsky · 2019-11-20T16:29:09Z

@kopachevsky Will you be able to add tests to this?
yes, checking

kopachevsky · 2019-11-22T16:37:54Z

@danawillow added test, it passed, ready for review again:)

modular-magician · 2019-11-22T20:55:27Z

Hi! I'm the modular magician, I work on Magic Modules.
I see that this PR has already had some downstream PRs generated. Any open downstreams are already updated to your most recent commit, 6fd9d76.

Pull request statuses

WARNING: The following files changed in commit 6fd9d76 may need corresponding changes in third_party/validator:

third_party/terraform/resources/resource_sql_database_instance.go

No diff detected in terraform-google-conversion.
No diff detected in Ansible.
No diff detected in Inspec.

New Pull Requests

I built this PR into one or more new PRs on other repositories, and when those are closed, this PR will also be merged and closed.
depends: hashicorp/terraform-provider-google-beta#1432
depends: hashicorp/terraform-provider-google#4983

danawillow

Thanks @kopachevsky! Just a few comments.

danawillow · 2019-11-22T21:07:03Z

third_party/terraform/tests/resource_sql_database_instance_test.go.erb

+			{
+				ResourceName:      "google_sql_database_instance.instance",
+				ImportState:       true,
+				// ImportStateVerify check disabled as long root_password property not stored in state


I'd recommend using an ImportStateVerifyIgnore instead of ImportStateVerify: false so we can at least check the other fields. I'd also mention that the field isn't returned from the API (since it is stored in state)

agreed, done

danawillow · 2019-11-22T21:13:25Z

third_party/terraform/website/docs/r/sql_database_instance.html.markdown

@@ -212,6 +212,8 @@ The following arguments are supported:

 * `replica_configuration` - (Optional) The configuration for replication. The
    configuration is detailed below.
+
+* `root_password` - (Optional) Required for MS SQL Server, ignored MySQL and PostgreSQL.


Suggested change

* `root_password` - (Optional) Required for MS SQL Server, ignored MySQL and PostgreSQL.

* `root_password` - (Optional) Initial root password. Required for MS SQL Server, ignored by MySQL and PostgreSQL.

done, added (Optional, Beta)

danawillow · 2019-11-22T21:24:22Z

third_party/terraform/resources/resource_sql_database_instance.go

@@ -325,6 +325,13 @@ func resourceSqlDatabaseInstance() *schema.Resource {
 				ForceNew: true,
 			},

+			"root_password": {


Since SQL server in GCP is still in beta, I think we should put this property only in the beta provider. If you agree, then you'll want to change the file extension to .go.erb and add tags like the ones you'll find in https://github.com/GoogleCloudPlatform/magic-modules/blob/master/third_party/terraform/resources/resource_compute_network_peering.go.erb for example. Same thing applies for the test, and then you'll want to say that it's beta in the docs

danawillow · 2019-11-22T21:27:24Z

third_party/terraform/website/docs/r/sql_database_instance.html.markdown

@@ -212,6 +212,8 @@ The following arguments are supported:

 * `replica_configuration` - (Optional) The configuration for replication. The
    configuration is detailed below.
+
+* `root_password` - (Optional) Required for MS SQL Server, ignored MySQL and PostgreSQL.


Can you also mention in database_version what the accepted MSSQL versions are?

sure, I'll take those from discovery document

kopachevsky · 2019-11-25T11:33:38Z

@danawillow thanks for review, applied changes to all comments, feel free to re-review

modular-magician · 2019-11-26T22:50:25Z

Hi! I'm the modular magician, I work on Magic Modules.
I see that this PR has already had some downstream PRs generated. Any open downstreams are already updated to your most recent commit, 1064818.

Pull request statuses

terraform-provider-google-beta already has an open PR.
No diff detected in terraform-google-conversion.
terraform-provider-google already has an open PR.
No diff detected in Ansible.
No diff detected in Inspec.

New Pull Requests

I didn't open any new pull requests because of this PR.

… SQL Server Added root_password parameter required MS SQL Server instance creation, ignored MySQL and PostgreSQL. Fix added both to beta and ga providers. Fixes #hashicorp/terraform-provider-google/issues/4776

Tracked submodules are build/terraform-beta build/terraform-mapper build/terraform build/ansible build/inspec.

goobysnack · 2020-03-09T23:39:20Z

As this is no longer beta, can we get this merged into the non beta google provider please? I'm using provider version 2.20.2and am getting An argument named "root_password" is not expected here.

danawillow · 2020-03-10T00:03:15Z

@goobysnack can you please file a new issue for that? That's the best way to get things on our team's radar. https://github.com/terraform-providers/terraform-provider-google/issues/new?template=enhancement.md

danawillow · 2020-03-10T00:04:40Z

For what it's worth, any new features we add (adding a beta field to the GA provider counts as a new feature) will be in the 3.X series, so it would be worth it for you to upgrade so you can get new features and bugfixes.

goobysnack · 2020-03-10T18:37:01Z

Still happening on 3.11.0.

Downloading plugin for provider "google" (hashicorp/google) 3.11.0...

....resource "google_sql_database_instance" "master":
An argument named "root_password" is not expected here.

danawillow · 2020-03-10T19:56:12Z

Looks like you're using the google provider; you'll want to use google-beta for ones that haven't reached the GA provider yet. As I mentioned in my above comment, to get it into the GA provider you'll want to file a new issue in the TPG issue tracker.

googlebot added the cla: yes label Nov 6, 2019

modular-magician added the community label Nov 6, 2019

kopachevsky force-pushed the fix/4776/sql-server-root-password branch 2 times, most recently from 36e080b to 63faeb3 Compare November 6, 2019 15:32

kopachevsky marked this pull request as ready for review November 8, 2019 07:17

kopachevsky force-pushed the fix/4776/sql-server-root-password branch 2 times, most recently from 295f765 to 6fd9d76 Compare November 22, 2019 10:36

danawillow self-requested a review November 22, 2019 20:43

modular-magician added downstream-generated terraform-provider-google-beta terraform-provider-google labels Nov 22, 2019

danawillow reviewed Nov 22, 2019

View reviewed changes

kopachevsky force-pushed the fix/4776/sql-server-root-password branch 3 times, most recently from 392e9d0 to 8d3cd8f Compare November 25, 2019 11:31

kopachevsky requested a review from danawillow November 25, 2019 11:33

danawillow approved these changes Nov 26, 2019

View reviewed changes

modular-magician added the automerged label Nov 26, 2019

kopachevsky and others added 3 commits November 26, 2019 23:24

Add support rootPassword property of google_sql_database_instance for…

9de6cc7

… SQL Server Added root_password parameter required MS SQL Server instance creation, ignored MySQL and PostgreSQL. Fix added both to beta and ga providers. Fixes #hashicorp/terraform-provider-google/issues/4776

add link to beta docs

2baf105

Update tracked submodules -> HEAD on Tue Nov 26 23:24:38 UTC 2019

b54223e

Tracked submodules are build/terraform-beta build/terraform-mapper build/terraform build/ansible build/inspec.

modular-magician force-pushed the fix/4776/sql-server-root-password branch from 1064818 to b54223e Compare November 26, 2019 23:24

modular-magician merged commit 9c921b4 into GoogleCloudPlatform:master Nov 26, 2019

danawillow mentioned this pull request Dec 3, 2019

Need to add a root-password parameter to google_sql_database_instance so I can create a sql server instance hashicorp/terraform-provider-google#4858

Closed

edwardmedia mentioned this pull request Mar 11, 2020

Resource google_sql_database_instance root_password (MS SQL) for GA hashicorp/terraform-provider-google#5870

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support rootPassword property for MS SQL Server #2592

Add support rootPassword property for MS SQL Server #2592

kopachevsky commented Nov 6, 2019 •

edited by danawillow

Loading

modular-magician commented Nov 6, 2019

danawillow commented Nov 6, 2019

kopachevsky commented Nov 7, 2019

danawillow commented Nov 8, 2019

kopachevsky commented Nov 12, 2019

danawillow commented Nov 15, 2019

morgante commented Nov 20, 2019

kopachevsky commented Nov 20, 2019

kopachevsky commented Nov 22, 2019

modular-magician commented Nov 22, 2019

danawillow left a comment

danawillow Nov 22, 2019

kopachevsky Nov 25, 2019

danawillow Nov 22, 2019

kopachevsky Nov 25, 2019

danawillow Nov 22, 2019

kopachevsky Nov 24, 2019

danawillow Nov 22, 2019

kopachevsky Nov 24, 2019

kopachevsky Nov 25, 2019

kopachevsky commented Nov 25, 2019

modular-magician commented Nov 26, 2019

goobysnack commented Mar 9, 2020

danawillow commented Mar 10, 2020

danawillow commented Mar 10, 2020

goobysnack commented Mar 10, 2020

danawillow commented Mar 10, 2020

	* `root_password` - (Optional) Required for MS SQL Server, ignored MySQL and PostgreSQL.
	* `root_password` - (Optional) Initial root password. Required for MS SQL Server, ignored by MySQL and PostgreSQL.

Add support rootPassword property for MS SQL Server #2592

Add support rootPassword property for MS SQL Server #2592

Conversation

kopachevsky commented Nov 6, 2019 • edited by danawillow Loading

modular-magician commented Nov 6, 2019

danawillow commented Nov 6, 2019

kopachevsky commented Nov 7, 2019

danawillow commented Nov 8, 2019

kopachevsky commented Nov 12, 2019

danawillow commented Nov 15, 2019

morgante commented Nov 20, 2019

kopachevsky commented Nov 20, 2019

kopachevsky commented Nov 22, 2019

modular-magician commented Nov 22, 2019

Pull request statuses

New Pull Requests

danawillow left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kopachevsky commented Nov 25, 2019

modular-magician commented Nov 26, 2019

Pull request statuses

New Pull Requests

goobysnack commented Mar 9, 2020

danawillow commented Mar 10, 2020

danawillow commented Mar 10, 2020

goobysnack commented Mar 10, 2020

danawillow commented Mar 10, 2020

kopachevsky commented Nov 6, 2019 •

edited by danawillow

Loading