Skip to content

Make mysql_profile.password mutable to avoid recreation of the connection profile.#8675

Merged
trodge merged 2 commits into
GoogleCloudPlatform:mainfrom
damjad:main
Aug 23, 2023
Merged

Make mysql_profile.password mutable to avoid recreation of the connection profile.#8675
trodge merged 2 commits into
GoogleCloudPlatform:mainfrom
damjad:main

Conversation

@damjad

@damjad damjad commented Aug 16, 2023

Copy link
Copy Markdown
Contributor

The recreation of connection profile fails the streams permanently. They have to be recreated from the scratch.

It's the solution to hashicorp/terraform-provider-google#15489

If this PR is for Terraform, I acknowledge that I have:

  • Searched through the issue tracker for an open issue that this either resolves or contributes to, commented on it to claim it, and written "fixes {url}" or "part of {url}" in this PR description. If there were no relevant open issues, I opened one and commented that I would like to work on it (not necessary for very small changes).
  • Ensured that all new fields I added that can be set by a user appear in at least one example (for generated resources) or third_party test (for handwritten resources or update tests).
  • Generated Terraform providers, and ran make test and make lint in the generated providers to ensure it passes unit and linter tests.
  • Ran relevant acceptance tests using my own Google Cloud project and credentials (If the acceptance tests do not yet pass or you are unable to run them, please let your reviewer know).
  • Read Write release notes before writing my release note below.

Release Note Template for Downstream PRs (will be copied)

datastream: made `password` mutable in `google_datastream_connection_profile` resource.

@google-cla

google-cla Bot commented Aug 16, 2023

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@modular-magician

Copy link
Copy Markdown
Collaborator

Hello! I am a robot. It looks like you are a: Community Contributor Googler Core Contributor. Tests will require approval to run.

@trodge, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look.

You can help make sure that review is quick by doing a self-review and by running impacted tests locally.

@modular-magician modular-magician added the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Aug 16, 2023
@damjad

damjad commented Aug 16, 2023

Copy link
Copy Markdown
Contributor Author

I have edited the file as mentioned in the issue.

I have also run the tests using the commands provided here. All of them are passing.

@melinath

Copy link
Copy Markdown
Member

Drive-by comment since I had suggested on the issue that @damjad should add this field to update tests: It looks like we don't currently have any mysql tests for datastream connection profile. We would need to add at least one update test following this guide that uses a base configuration similar to TestAccDatastreamStream_datastreamStreamBasicExample (that is, setting up a mysql instance and connecting to it.)

@damjad

damjad commented Aug 17, 2023

Copy link
Copy Markdown
Contributor Author

@melinath Could you please tell if my understanding is correct?

I would need to create a handwritten test in resource_datastream_connection_profile_test.go.

  1. create a connection profile with prevent_destroy=true and a random_password resource. It will prevent the destruction/recreation.
  2. re-run the same plan. random_password will generate a new password for the same user. Effectively, it should update the password in-place successfully (expected).
    If the password is immutable (not expected), the test will fail during the recreation of profile because of prevent_destroy=true.
  3. the resources are destroyed automatically at the end with the method testAccCheckDatastreamConnectionProfileDestroyProducer

@modular-magician modular-magician removed the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Aug 17, 2023
@modular-magician

Copy link
Copy Markdown
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 1 file changed, 1 deletion(-))
Terraform Beta: Diff ( 1 file changed, 1 deletion(-))

@modular-magician

Copy link
Copy Markdown
Collaborator

Tests analytics

Total tests: 2962
Passed tests 2660
Skipped tests: 302
Affected tests: 0

$\textcolor{green}{\textsf{All tests passed in REPLAYING mode.}}$
View the build log

@melinath

melinath commented Aug 17, 2023

Copy link
Copy Markdown
Member

@damjad that sounds right; there also needs to be a configuration prior to testAccCheckDatastreamConnectionProfileDestroyProducer to turn off prevent_destroy. I'll leave it to your reviewer to cover anything else! :-)

…tion profile.

The recreation of connection profile fails the streams permanently. They have to be recreated from the scratch.
@modular-magician modular-magician added the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Aug 18, 2023
@damjad

damjad commented Aug 18, 2023

Copy link
Copy Markdown
Contributor Author

I have added the tests. Everything is working as expected.

The test fails for old code because it tries to destroy/create the connection profile. Here is the plan:

 | exit status 1
  |
  | Error: Instance cannot be destroyed
  |
  |   on terraform_plugin_test.tf line 54:
  |   54: resource "google_datastream_connection_profile" "mysql_con_profile" {
  |
  | Resource google_datastream_connection_profile.mysql_con_profile has
  | lifecycle.prevent_destroy set, but the plan calls for this resource to be
  | destroyed. To avoid this error and continue with the plan, either disable
  | lifecycle.prevent_destroy or reduce the scope of the plan using the -target
  | flag.

With the new code the test passes because it updates in-place. Here is the plan:

|
  | Terraform used the selected providers to generate the following execution
  | plan. Resource actions are indicated with the following symbols:
  |   ~ update in-place
  |
  | Terraform will perform the following actions:
  |
  |   # google_datastream_connection_profile.mysql_con_profile will be updated in-place
  |   ~ resource "google_datastream_connection_profile" "mysql_con_profile" {
  |         id                    = "projects/xxxx/locations/us-central1/connectionProfiles/tf-test-my-profile81bgps1mvr"
  |         name                  = "projects/xxxx/locations/us-central1/connectionProfiles/tf-test-my-profile81bgps1mvr"
  |         # (5 unchanged attributes hidden)
  |
  |       ~ mysql_profile {
  |           ~ password = (sensitive value)
  |             # (3 unchanged attributes hidden)
  |         }
  |     }
  |
  |   # google_sql_user.mysql_user will be updated in-place
  |   ~ resource "google_sql_user" "mysql_user" {
  |         id                      = "user/%/tf-test-mysql-database-instance81bgps1mvr"
  |         name                    = "user"
  |       ~ password                = (sensitive value)
  |         # (4 unchanged attributes hidden)
  |     }
  |
  | Plan: 0 to add, 2 to change, 0 to destroy.

@modular-magician modular-magician removed the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Aug 21, 2023
@modular-magician

Copy link
Copy Markdown
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 2 files changed, 105 insertions(+), 1 deletion(-))
Terraform Beta: Diff ( 2 files changed, 105 insertions(+), 1 deletion(-))

@modular-magician

Copy link
Copy Markdown
Collaborator

Tests analytics

Total tests: 2983
Passed tests 2687
Skipped tests: 295
Affected tests: 1

Action taken

Found 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccBigQueryDataTable_bigtable

Get to know how VCR tests work

@modular-magician

Copy link
Copy Markdown
Collaborator

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccBigQueryDataTable_bigtable[Debug log]

Rerun these tests in REPLAYING mode to catch issues

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$


$\textcolor{green}{\textsf{All tests passed!}}$
View the build log or the debug log for each test

@modular-magician modular-magician added awaiting-approval Pull requests that need reviewer's approval to run presubmit tests and removed awaiting-approval Pull requests that need reviewer's approval to run presubmit tests labels Aug 22, 2023
@modular-magician

Copy link
Copy Markdown
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 2 files changed, 104 insertions(+), 1 deletion(-))
Terraform Beta: Diff ( 2 files changed, 104 insertions(+), 1 deletion(-))

@modular-magician

Copy link
Copy Markdown
Collaborator

Tests analytics

Total tests: 2983
Passed tests 2688
Skipped tests: 295
Affected tests: 0

$\textcolor{green}{\textsf{All tests passed in REPLAYING mode.}}$
View the build log

@trodge trodge merged commit 0d45fda into GoogleCloudPlatform:main Aug 23, 2023
nevzheng pushed a commit to nevzheng/magic-modules that referenced this pull request Aug 25, 2023
…tion profile. (GoogleCloudPlatform#8675)

* Make mysql_profile.password mutable to avoid recreation of the connection profile.

The recreation of connection profile fails the streams permanently. They have to be recreated from the scratch.

* Make fmt
nevzheng pushed a commit to nevzheng/magic-modules that referenced this pull request Aug 28, 2023
…tion profile. (GoogleCloudPlatform#8675)

* Make mysql_profile.password mutable to avoid recreation of the connection profile.

The recreation of connection profile fails the streams permanently. They have to be recreated from the scratch.

* Make fmt
joelkattapuram pushed a commit to joelkattapuram/magic-modules that referenced this pull request Sep 20, 2023
…tion profile. (GoogleCloudPlatform#8675)

* Make mysql_profile.password mutable to avoid recreation of the connection profile.

The recreation of connection profile fails the streams permanently. They have to be recreated from the scratch.

* Make fmt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants