Skip to content

fixing permadiff create_without_validation #14543

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

luckyswaminathan
Copy link
Contributor

@luckyswaminathan luckyswaminathan commented Jul 16, 2025

Fixes hashicorp/terraform-provider-google#21780 - a permadiff regarding the create_without_validation field within datastream

Release Note Template for Downstream PRs (will be copied)

See Write release notes for guidance.

datastore: Fixed a permadiff related to datastream 'create_without_validation' field

@modular-magician
Copy link
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.

google provider: Diff ( 2 files changed, 22 insertions(+), 9 deletions(-))
google-beta provider: Diff ( 2 files changed, 22 insertions(+), 9 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 14 insertions(+))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 9
Passed tests: 1
Skipped tests: 8
Affected tests: 0

Click here to see the affected service packages
  • datastream
🔴 Errors occurred during REPLAYING mode. Please fix them to complete your PR.

View the build log

@modular-magician
Copy link
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.

google provider: Diff ( 2 files changed, 22 insertions(+), 9 deletions(-))
google-beta provider: Diff ( 2 files changed, 22 insertions(+), 9 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 14 insertions(+))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 9
Passed tests: 1
Skipped tests: 8
Affected tests: 0

Click here to see the affected service packages
  • datastream
🔴 Errors occurred during REPLAYING mode. Please fix them to complete your PR.

View the build log

// If the old value was "false" and the new value is now unset (empty string),
// return true to suppress the diff.
if (old == "" && new == "false") || (old == "false" && new == "") {
d.Set("create_without_validation", "false")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what's causing the panic, because terraform expects a boolean. I would try putting

d.Set("create_without_validation", false)

in a post_import.

@luckyswaminathan luckyswaminathan marked this pull request as ready for review July 16, 2025 22:05
@github-actions github-actions bot requested a review from trodge July 16, 2025 22:06
@modular-magician
Copy link
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.

google provider: Diff ( 2 files changed, 22 insertions(+), 9 deletions(-))
google-beta provider: Diff ( 2 files changed, 22 insertions(+), 9 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 13 insertions(+))

@modular-magician
Copy link
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.

google provider: Diff ( 2 files changed, 22 insertions(+), 9 deletions(-))
google-beta provider: Diff ( 2 files changed, 22 insertions(+), 9 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 13 insertions(+))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 15
Passed tests: 3
Skipped tests: 8
Affected tests: 4

Click here to see the affected service packages
  • datastream
#### 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
  • TestAccDatastreamConnectionProfile_datastreamConnectionProfileBasicExample
  • TestAccDatastreamConnectionProfile_datastreamConnectionProfileFullExample
  • TestAccDatastreamConnectionProfile_datastreamConnectionProfilePostgresSecretManagerExample
  • TestAccDatastreamConnectionProfile_sshKey_update

Get to know how VCR tests work

1 similar comment
@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 15
Passed tests: 3
Skipped tests: 8
Affected tests: 4

Click here to see the affected service packages
  • datastream
#### 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
  • TestAccDatastreamConnectionProfile_datastreamConnectionProfileBasicExample
  • TestAccDatastreamConnectionProfile_datastreamConnectionProfileFullExample
  • TestAccDatastreamConnectionProfile_datastreamConnectionProfilePostgresSecretManagerExample
  • TestAccDatastreamConnectionProfile_sshKey_update

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

🔴 Tests failed during RECORDING mode:
TestAccDatastreamConnectionProfile_datastreamConnectionProfileBasicExample [Error message] [Debug log]
TestAccDatastreamConnectionProfile_datastreamConnectionProfileFullExample [Error message] [Debug log]
TestAccDatastreamConnectionProfile_datastreamConnectionProfilePostgresSecretManagerExample [Error message] [Debug log]
TestAccDatastreamConnectionProfile_sshKey_update [Error message] [Debug log]

🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR.

View the build log or the debug log for each test

@modular-magician
Copy link
Collaborator

🔴 Tests failed during RECORDING mode:
TestAccDatastreamConnectionProfile_datastreamConnectionProfileBasicExample [Error message] [Debug log]
TestAccDatastreamConnectionProfile_datastreamConnectionProfileFullExample [Error message] [Debug log]
TestAccDatastreamConnectionProfile_datastreamConnectionProfilePostgresSecretManagerExample [Error message] [Debug log]
TestAccDatastreamConnectionProfile_sshKey_update [Error message] [Debug log]

🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR.

View the build log or the debug log for each test

@modular-magician
Copy link
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.

google provider: Diff ( 2 files changed, 25 insertions(+), 9 deletions(-))
google-beta provider: Diff ( 2 files changed, 25 insertions(+), 9 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 13 insertions(+))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 15
Passed tests: 7
Skipped tests: 8
Affected tests: 0

Click here to see the affected service packages
  • datastream
🟢 All tests passed!

View the build log

@modular-magician
Copy link
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.

google provider: Diff ( 2 files changed, 26 insertions(+), 9 deletions(-))
google-beta provider: Diff ( 2 files changed, 26 insertions(+), 9 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 13 insertions(+))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 15
Passed tests: 7
Skipped tests: 8
Affected tests: 0

Click here to see the affected service packages
  • datastream
🟢 All tests passed!

View the build log

Comment on lines +15 to +16
// If the old value was "false" and the new value is now unset (empty string),
// return true to suppress the diff.
Copy link
Contributor

@trodge trodge Jul 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This describes what the conditional does, but the important thing to explain here is why we need to do it. You can either link the issue in the comment or summarize it.

Comment on lines +14 to +18
_, exists := d.GetOk("create_without_validation")
if !exists {
url += "false"
d.Set("create_without_validation", false)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the back and forth here, but after reviewing a discussion about GetOkExists I think our best approach is actually to get the url parameters out of the base URL entirely.

So that would mean deleting &force={{create_without_validation}} from the create_url and calling transport.AddQueryParams here with something like map[string]string{"force": "true"} only if d.Get("create_without_validation") returns true.

We might also want to make sure there's at least one test that creates the resource with the field set to false or unspecified.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

google_datastream_* resources force replacement due to create_without_validation
3 participants