magic-modules
magic-modules copied to clipboard
Fix permadiff on google_vpc_access_connector resource with breaking changes (min_instances, max_instances, min_throughput, max_throughput fields)
Fixes https://github.com/hashicorp/terraform-provider-google/issues/15278
When reviewing this PR please see:
- This comment for context about this PR : https://github.com/GoogleCloudPlatform/magic-modules/pull/10313#issuecomment-2159065620
- This comment about the decision to make the two fields conflict in the schema: https://github.com/GoogleCloudPlatform/magic-modules/pull/10313#issuecomment-2159101760
- Note to the reviewer when I marked this ready for review: https://github.com/GoogleCloudPlatform/magic-modules/pull/10313#issuecomment-2161335142
Release Note Template for Downstream PRs (will be copied)
vpcaccess: removed default values for `min_throughput` and `min_instances` fields on `google_vpc_access_connector` and made them default to values returned from the API when not provided by users
vpcaccess: added a conflicting fields restriction between `min_throughput` and `min_instances` fields on `google_vpc_access_connector`
vpcaccess: added a conflicting fields restriction between `max_throughput` and `max_instances` fields on `google_vpc_access_connector`
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.
google
provider: Diff ( 1 file changed, 52 insertions(+))
google-beta
provider: Diff ( 1 file changed, 52 insertions(+))
Tests analytics
Total tests: 5
Passed tests: 4
Skipped tests: 0
Affected tests: 1
Click here to see the affected service packages
- vpcaccess
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
TestAccVPCAccessConnector_vpcAccessConnectorThroughput_maxThroughput
$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccVPCAccessConnector_vpcAccessConnectorThroughput_maxThroughput
[Error message] [Debug log]
$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$ View the build log or the debug log for each test
/gcbrun
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.
google
provider: Diff ( 1 file changed, 52 insertions(+))
google-beta
provider: Diff ( 1 file changed, 52 insertions(+))
Tests analytics
Total tests: 5
Passed tests: 4
Skipped tests: 0
Affected tests: 1
Click here to see the affected service packages
- vpcaccess
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
TestAccVPCAccessConnector_vpcAccessConnectorThroughput_maxThroughput
$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccVPCAccessConnector_vpcAccessConnectorThroughput_maxThroughput
[Error message] [Debug log]
$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$ View the build log or the debug log for each test
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.
google
provider: Diff ( 2 files changed, 52 insertions(+), 2 deletions(-))
google-beta
provider: Diff ( 2 files changed, 52 insertions(+), 2 deletions(-))
Breaking Change(s) Detected
The following breaking change(s) were detected within your pull request.
- Field
max_throughput
default value changed from 300 toon google_vpc_access_connector
- reference - Field
min_throughput
default value changed from 200 toon google_vpc_access_connector
- reference
If you believe this detection to be incorrect please raise the concern with your reviewer.
If you intend to make this change you will need to wait for a major release window.
An override-breaking-change
label can be added to allow merging.
Missing test report
Your PR includes resource fields which are not covered by any test.
Resource: google_vpc_access_connector
(12 total tests)
Please add an acceptance test which includes these fields. The test should include the following:
resource "google_vpc_access_connector" "primary" {
min_throughput = # value needed
}
Tests analytics
Total tests: 5
Passed tests: 0
Skipped tests: 0
Affected tests: 5
Click here to see the affected service packages
- vpcaccess
Action taken
Found 5 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccVPCAccessConnectorDatasource_basic|TestAccVPCAccessConnector_vpcAccessConnectorExample|TestAccVPCAccessConnector_vpcAccessConnectorSharedVpcExample|TestAccVPCAccessConnector_vpcAccessConnectorThroughput|TestAccVPCAccessConnector_vpcAccessConnectorThroughput_maxThroughput
$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccVPCAccessConnectorDatasource_basic
[Error message] [Debug log]
TestAccVPCAccessConnector_vpcAccessConnectorExample
[Error message] [Debug log]
TestAccVPCAccessConnector_vpcAccessConnectorSharedVpcExample
[Error message] [Debug log]
TestAccVPCAccessConnector_vpcAccessConnectorThroughput
[Error message] [Debug log]
TestAccVPCAccessConnector_vpcAccessConnectorThroughput_maxThroughput
[Error message] [Debug log]
$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$ View the build log or the debug log for each test
Just rebased on main because this PR's branch was pretty old.
Before the rebase this test failure showed how the permadiff occurs : https://github.com/GoogleCloudPlatform/magic-modules/pull/10313#issuecomment-2025212589
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.
google
provider: Diff ( 8 files changed, 72 insertions(+), 6 deletions(-))
google-beta
provider: Diff ( 8 files changed, 72 insertions(+), 6 deletions(-))
terraform-google-conversion
: Diff ( 1 file changed, 2 insertions(+))
Open in Cloud Shell: Diff ( 2 files changed, 4 insertions(+))
Breaking Change(s) Detected
The following breaking change(s) were detected within your pull request.
- Field
max_throughput
default value changed from 300 toon google_vpc_access_connector
- reference - Field
min_throughput
default value changed from 200 toon google_vpc_access_connector
- reference
If you believe this detection to be incorrect please raise the concern with your reviewer.
If you intend to make this change you will need to wait for a major release window.
An override-breaking-change
label can be added to allow merging.
Tests analytics
Total tests: 36
Passed tests: 31
Skipped tests: 1
Affected tests: 4
Click here to see the affected service packages
- appengine
- cloudfunctions
- vpcaccess
Action taken
Found 4 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccVPCAccessConnector_vpcAccessConnectorExample|TestAccVPCAccessConnector_vpcAccessConnectorSharedVpcExample|TestAccVPCAccessConnector_vpcAccessConnectorThroughput|TestAccVPCAccessConnector_vpcAccessConnectorThroughput_maxThroughput
$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccVPCAccessConnector_vpcAccessConnectorExample
[Error message] [Debug log]
TestAccVPCAccessConnector_vpcAccessConnectorSharedVpcExample
[Error message] [Debug log]
TestAccVPCAccessConnector_vpcAccessConnectorThroughput
[Error message] [Debug log]
TestAccVPCAccessConnector_vpcAccessConnectorThroughput_maxThroughput
[Error message] [Debug log]
$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$ View the build log or the debug log for each test
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.
google
provider: Diff ( 8 files changed, 176 insertions(+), 6 deletions(-))
google-beta
provider: Diff ( 8 files changed, 176 insertions(+), 6 deletions(-))
terraform-google-conversion
: Diff ( 1 file changed, 2 insertions(+))
Open in Cloud Shell: Diff ( 2 files changed, 4 insertions(+))
Breaking Change(s) Detected
The following breaking change(s) were detected within your pull request.
- Field
max_throughput
default value changed from 300 toon google_vpc_access_connector
- reference - Field
min_throughput
default value changed from 200 toon google_vpc_access_connector
- reference
If you believe this detection to be incorrect please raise the concern with your reviewer.
If you intend to make this change you will need to wait for a major release window.
An override-breaking-change
label can be added to allow merging.
Tests analytics
Total tests: 37
Passed tests: 31
Skipped tests: 1
Affected tests: 5
Click here to see the affected service packages
- appengine
- cloudfunctions
- vpcaccess
Action taken
Found 5 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccVPCAccessConnector_vpcAccessConnectorExample|TestAccVPCAccessConnector_vpcAccessConnectorSharedVpcExample|TestAccVPCAccessConnector_vpcAccessConnectorThroughput|TestAccVPCAccessConnector_vpcAccessConnectorThroughput_combiningThroughputAndInstancesFields_permadiff|TestAccVPCAccessConnector_vpcAccessConnectorThroughput_usingThroughputOrInstancesLimits
$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccVPCAccessConnector_vpcAccessConnectorSharedVpcExample
[Debug log]
TestAccVPCAccessConnector_vpcAccessConnectorThroughput
[Debug log]
TestAccVPCAccessConnector_vpcAccessConnectorThroughput_combiningThroughputAndInstancesFields_permadiff
[Debug log]
$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$
$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccVPCAccessConnector_vpcAccessConnectorExample
[Error message] [Debug log]
TestAccVPCAccessConnector_vpcAccessConnectorThroughput_usingThroughputOrInstancesLimits
[Error message] [Debug log]
$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$ View the build log or the debug log for each test
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.
google
provider: Diff ( 8 files changed, 176 insertions(+), 6 deletions(-))
google-beta
provider: Diff ( 8 files changed, 176 insertions(+), 6 deletions(-))
terraform-google-conversion
: Diff ( 1 file changed, 2 insertions(+))
Open in Cloud Shell: Diff ( 2 files changed, 4 insertions(+))
Breaking Change(s) Detected
The following breaking change(s) were detected within your pull request.
- Field
max_throughput
default value changed from 300 toon google_vpc_access_connector
- reference - Field
min_throughput
default value changed from 200 toon google_vpc_access_connector
- reference
If you believe this detection to be incorrect please raise the concern with your reviewer.
If you intend to make this change you will need to wait for a major release window.
An override-breaking-change
label can be added to allow merging.
Tests analytics
Total tests: 37
Passed tests: 34
Skipped tests: 1
Affected tests: 2
Click here to see the affected service packages
- appengine
- cloudfunctions
- vpcaccess
Action taken
Found 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccVPCAccessConnector_vpcAccessConnectorExample|TestAccVPCAccessConnector_vpcAccessConnectorThroughput_usingThroughputOrInstancesLimits
$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccVPCAccessConnector_vpcAccessConnectorThroughput_usingThroughputOrInstancesLimits
[Debug log]
$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$
$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccVPCAccessConnector_vpcAccessConnectorExample
[Error message] [Debug log]
$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$ View the build log or the debug log for each test
That last test is failing due to a networking reason:
vcr_utils.go:152: Step 1/2 error: Error running apply: exit status 1
Error: Error waiting to create Connector: Error waiting for Creating Connector: Error code 3, message: Operation failed: Invalid IP CIDR range was provided. It conflicts with an existing subnetwork. Please delete the connector manually.
with google_vpc_access_connector.connector,
on terraform_plugin_test.tf line 2, in resource "google_vpc_access_connector" "connector":
2: resource "google_vpc_access_connector" "connector" {
As a recap of this PR's stages:
-
Originally contained only an acceptance test demonstrating the permadiff in the linked issue f7bce51 - test failure due to permadiff here
-
Updated the provider (@ 26ca216) to:
- remove default values on min_ and max_throughput, then needed to make those fields default from the API because all 4 fields are always present
- update existing tests to no longer rely on the default values for min_ and max_throughput
- update the tests (in https://github.com/GoogleCloudPlatform/magic-modules/pull/10313/commits/fe60725f36152a6e2c469bbc64c6286be0abddee )related to the these fields' weird behaviour/the permadiff to show that setting all 4 fields results in a permadiff and that when 2 fields are set the other 2 fields are present.
In the commits above I stop users from being able to include both min_throughput and min_instance (same for max_) by making fields conflict. Previously users would experience a permadiff, but this change means that users will fix their config to avoid the permadiff.
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.
google
provider: Diff ( 8 files changed, 182 insertions(+), 6 deletions(-))
google-beta
provider: Diff ( 8 files changed, 182 insertions(+), 6 deletions(-))
terraform-google-conversion
: Diff ( 1 file changed, 2 insertions(+))
Open in Cloud Shell: Diff ( 2 files changed, 4 insertions(+))
Breaking Change(s) Detected
The following breaking change(s) were detected within your pull request.
- Field
max_throughput
default value changed from 300 toon google_vpc_access_connector
- reference - Field
min_throughput
default value changed from 200 toon google_vpc_access_connector
- reference
If you believe this detection to be incorrect please raise the concern with your reviewer.
If you intend to make this change you will need to wait for a major release window.
An override-breaking-change
label can be added to allow merging.
Tests analytics
Total tests: 37
Passed tests: 34
Skipped tests: 1
Affected tests: 2
Click here to see the affected service packages
- appengine
- cloudfunctions
- vpcaccess
Action taken
Found 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccVPCAccessConnector_vpcAccessConnectorExample|TestAccVPCAccessConnector_vpcAccessConnectorThroughput_combiningThroughputAndInstancesFields_conflict
$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccVPCAccessConnector_vpcAccessConnectorThroughput_combiningThroughputAndInstancesFields_conflict
[Debug log]
$\textcolor{red}{\textsf{Tests failed when rerunning REPLAYING mode:}}$
TestAccVPCAccessConnector_vpcAccessConnectorThroughput_combiningThroughputAndInstancesFields_conflict
[Error message] [Debug log]
Tests failed due to non-determinism or randomness when the VCR replayed the response after the HTTP request was made.
Please fix these to complete your PR. If you believe these test failures to be incorrect or unrelated to your change, or if you have any questions, please raise the concern with your reviewer.
$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccVPCAccessConnector_vpcAccessConnectorExample
[Error message] [Debug log]
$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$ View the build log or the debug log for each test
Ah, we cannot have acc tests that don't reach out to an API run in VCR:
test_utils.go:108: no cassette found on disk for TestAccVPCAccessConnector_vpcAccessConnectorThroughput_combiningThroughputAndInstancesFields_conflict, please replay this testcase in recording mode - open /workspace/.ci/magician/cassettes/beta/TestAccVPCAccessConnector_vpcAccessConnectorThroughput_combiningThroughputAndInstancesFields_conflict.seed: no such file or directory
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.
google
provider: Diff ( 8 files changed, 185 insertions(+), 6 deletions(-))
google-beta
provider: Diff ( 8 files changed, 185 insertions(+), 6 deletions(-))
terraform-google-conversion
: Diff ( 1 file changed, 2 insertions(+))
Open in Cloud Shell: Diff ( 2 files changed, 4 insertions(+))
Breaking Change(s) Detected
The following breaking change(s) were detected within your pull request.
- Field
max_throughput
default value changed from 300 toon google_vpc_access_connector
- reference - Field
min_throughput
default value changed from 200 toon google_vpc_access_connector
- reference
If you believe this detection to be incorrect please raise the concern with your reviewer.
If you intend to make this change you will need to wait for a major release window.
An override-breaking-change
label can be added to allow merging.
Tests analytics
Total tests: 37
Passed tests: 34
Skipped tests: 2
Affected tests: 1
Click here to see the affected service packages
- appengine
- cloudfunctions
- vpcaccess
Non-exercised tests
Tests were added that are skipped in VCR:
- TestAccVPCAccessConnector_vpcAccessConnectorThroughput_combiningThroughputAndInstancesFields_conflict
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
TestAccVPCAccessConnector_vpcAccessConnectorExample
$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccVPCAccessConnector_vpcAccessConnectorExample
[Error message] [Debug log]
$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$ View the build log or the debug log for each test