pubsub: fix permadiff with configuring an empty retry_policy. #11834

anavada · 2024-09-26T17:57:37Z

Fixes hashicorp/terraform-provider-google#18016

The permadiff arises when users specify an empty retry policy as retry_policy: {} in their config, but the empty value doesn't get sent to the API and the API subsequently doesn't return a retry_policy in the response. By setting send_empty_value: true on the retry_policy property, we ensure that the API recognizes the user-configured value.

Similar fixes for other resource properties will be investigated and sent in separate PRs.

Release Note Template for Downstream PRs (will be copied)

pubsub: fix permadiff with configuring an empty `retry_policy`.

github-actions · 2024-09-26T17:58:16Z

Hello! I am a robot. Tests will require approval from a repository maintainer to run.

@melinath, 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 · 2024-09-26T18:08:42Z

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, 42 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 2 files changed, 42 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 1 insertion(+), 1 deletion(-))

modular-magician · 2024-09-26T18:13:20Z

Tests analytics

Total tests: 53
Passed tests: 26
Skipped tests: 1
Affected tests: 26

Click here to see the affected service packages

pubsub

Action taken

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

TestAccDataSourceGooglePubsubSubscription_basic

TestAccDataSourceGooglePubsubSubscription_optionalProject

TestAccPubsubSubscriptionBigQuery_serviceAccount

TestAccPubsubSubscriptionBigQuery_update

TestAccPubsubSubscriptionCloudStorage_serviceAccount

TestAccPubsubSubscriptionCloudStorage_updateAvro

TestAccPubsubSubscriptionCloudStorage_updateText

TestAccPubsubSubscriptionIamBinding

TestAccPubsubSubscriptionIamMember

TestAccPubsubSubscriptionIamPolicy

TestAccPubsubSubscription_emptyRetryPolicy

TestAccPubsubSubscription_emptyTTL

TestAccPubsubSubscription_filter

TestAccPubsubSubscription_pollOnCreate

TestAccPubsubSubscription_pubsubSubscriptionDeadLetterExample

TestAccPubsubSubscription_pubsubSubscriptionPullFilterExample

TestAccPubsubSubscription_pubsubSubscriptionPushBqExample

TestAccPubsubSubscription_pubsubSubscriptionPushBqServiceAccountExample

TestAccPubsubSubscription_pubsubSubscriptionPushBqTableSchemaExample

TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageAvroExample

TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageExample

TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageServiceAccountExample

TestAccPubsubSubscription_pubsubSubscriptionPushExample

TestAccPubsubSubscription_push

TestAccPubsubSubscription_pushNoWrapper

TestAccPubsubSubscription_pushNoWrapperEmpty

Get to know how VCR tests work

modular-magician · 2024-09-26T18:16:49Z

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccDataSourceGooglePubsubSubscription_basic[Debug log]
TestAccDataSourceGooglePubsubSubscription_optionalProject[Debug log]
TestAccPubsubSubscriptionBigQuery_serviceAccount[Debug log]
TestAccPubsubSubscriptionBigQuery_update[Debug log]
TestAccPubsubSubscriptionCloudStorage_serviceAccount[Debug log]
TestAccPubsubSubscriptionCloudStorage_updateAvro[Debug log]
TestAccPubsubSubscriptionCloudStorage_updateText[Debug log]
TestAccPubsubSubscriptionIamBinding[Debug log]
TestAccPubsubSubscriptionIamMember[Debug log]
TestAccPubsubSubscriptionIamPolicy[Debug log]
TestAccPubsubSubscription_emptyRetryPolicy[Debug log]
TestAccPubsubSubscription_emptyTTL[Debug log]
TestAccPubsubSubscription_filter[Debug log]
TestAccPubsubSubscription_pollOnCreate[Debug log]
TestAccPubsubSubscription_pubsubSubscriptionDeadLetterExample[Debug log]
TestAccPubsubSubscription_pubsubSubscriptionPullFilterExample[Debug log]
TestAccPubsubSubscription_pubsubSubscriptionPushBqServiceAccountExample[Debug log]
TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageAvroExample[Debug log]
TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageExample[Debug log]
TestAccPubsubSubscription_pubsubSubscriptionPushCloudstorageServiceAccountExample[Debug log]
TestAccPubsubSubscription_pubsubSubscriptionPushExample[Debug log]
TestAccPubsubSubscription_push[Debug log]
TestAccPubsubSubscription_pushNoWrapper[Debug log]
TestAccPubsubSubscription_pushNoWrapperEmpty[Debug log]

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

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccPubsubSubscription_pubsubSubscriptionPushBqExample[Error message] [Debug log]
TestAccPubsubSubscription_pubsubSubscriptionPushBqTableSchemaExample[Error message] [Debug log]

$\textcolor{red}{\textsf{Errors occurred during RECORDING mode. Please fix them to complete your PR.}}$

View the build log or the debug log for each test

melinath · 2024-09-30T16:19:32Z

mmv1/products/pubsub/Subscription.yaml

@@ -471,6 +471,7 @@ properties:
          A duration in seconds with up to nine fractional digits, terminated by 's'. Example: "3.5s".
        default_from_api: true
        diff_suppress_func: 'tpgresource.DurationDiffSuppress'
+    send_empty_value: true


I think you'll want allow_empty_object here as well, though it looks like the tests are passing, so maybe it's fine? Did you happen to try that out?

Suggested change

send_empty_value: true

send_empty_value: true

allow_empty_object: true

Yeah the earlier BQ-related test failures seem unrelated and are passing now. Thanks for opening hashicorp/terraform-provider-google#19683.

Those tests pass locally as well:

$ make testacc TEST=./google-beta/services/pubsub TESTARGS='-run=TestAccPubsubSubscription_pubsubSubscriptionPushBqExample' ==> Checking that code complies with gofmt requirements... go vet TF_ACC=1 TF_SCHEMA_PANIC_ON_ERROR=1 go test ./google-beta/services/pubsub -v -run=TestAccPubsubSubscription_pubsubSubscriptionPushBqExample -timeout 240m -ldflags="-X=github.com/hashicorp/terraform-provider-google-beta/version.ProviderVersion=acc" === RUN TestAccPubsubSubscription_pubsubSubscriptionPushBqExample === PAUSE TestAccPubsubSubscription_pubsubSubscriptionPushBqExample === CONT TestAccPubsubSubscription_pubsubSubscriptionPushBqExample --- PASS: TestAccPubsubSubscription_pubsubSubscriptionPushBqExample (30.70s) PASS ok github.com/hashicorp/terraform-provider-google-beta/google-beta/services/pubsub 31.317s $ make testacc TEST=./google-beta/services/pubsub TESTARGS='-run=TestAccPubsubSubscription_pubsubSubscriptionPushBqTableSchemaExample' ==> Checking that code complies with gofmt requirements... go vet TF_ACC=1 TF_SCHEMA_PANIC_ON_ERROR=1 go test ./google-beta/services/pubsub -v -run=TestAccPubsubSubscription_pubsubSubscriptionPushBqTableSchemaExample -timeout 240m -ldflags="-X=github.com/hashicorp/terraform-provider-google-beta/version.ProviderVersion=acc" === RUN TestAccPubsubSubscription_pubsubSubscriptionPushBqTableSchemaExample === PAUSE TestAccPubsubSubscription_pubsubSubscriptionPushBqTableSchemaExample === CONT TestAccPubsubSubscription_pubsubSubscriptionPushBqTableSchemaExample --- PASS: TestAccPubsubSubscription_pubsubSubscriptionPushBqTableSchemaExample (30.34s) PASS ok github.com/hashicorp/terraform-provider-google-beta/google-beta/services/pubsub 30.958s

Similarly, the "VCR-test" was flaky as well but is passing now. However, the terraform-google-conversion-build-and-unit-tests failures show a permadiff with "retryPolicy": nil, (example test case and test code throwing the error). I think that I'd need to submit a separate PR to fix those tests since they are in a different repo.

I think you'll want allow_empty_object here as well, though it looks like the tests are passing, so maybe it's fine? Did you happen to try that out?

I didn't see that in the field reference but I see it here and used elsewhere in Subscription.yaml. But yeah, the tests are passing with or without allow_empty_object and I'm not sure why, but it doesn't seem like adding allow_empty_object will hurt so I've added it.

modular-magician · 2024-09-30T22:27:36Z

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, 42 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 2 files changed, 42 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 1 insertion(+), 1 deletion(-))

modular-magician · 2024-09-30T22:32:38Z

Tests analytics

Total tests: 55
Passed tests: 52
Skipped tests: 1
Affected tests: 2

Click here to see the affected service packages

pubsub

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

TestAccPubsubSubscription_pubsubSubscriptionPushBqExample

TestAccPubsubSubscription_pubsubSubscriptionPushBqTableSchemaExample

Get to know how VCR tests work

modular-magician · 2024-09-30T22:33:50Z

🟢 Tests passed during RECORDING mode:
TestAccPubsubSubscription_pubsubSubscriptionPushBqExample[Debug log]
TestAccPubsubSubscription_pubsubSubscriptionPushBqTableSchemaExample[Debug log]

🟢 No issues found for passed tests after REPLAYING rerun.

🟢 All tests passed!

View the build log or the debug log for each test

modular-magician · 2024-09-30T23:16:57Z

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, 48 insertions(+), 6 deletions(-))
google-beta provider: Diff ( 2 files changed, 48 insertions(+), 6 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 7 insertions(+), 2 deletions(-))

modular-magician · 2024-09-30T23:21:27Z

Tests analytics

Total tests: 55
Passed tests: 54
Skipped tests: 1
Affected tests: 0

Click here to see the affected service packages

pubsub

🟢 All tests passed!

View the build log

github-actions bot requested a review from melinath September 26, 2024 17:58

modular-magician added the service/pubsub label Sep 26, 2024

melinath mentioned this pull request Sep 30, 2024

Failing test(s): TestAccPubsubSubscription should use bootstrapped IAM hashicorp/terraform-provider-google#19683

Open

melinath reviewed Sep 30, 2024

View reviewed changes

github-actions bot requested a review from melinath September 30, 2024 22:15

pubsub: fix permadiff with configuring an empty retry_policy.

02c4a96

anavada force-pushed the main branch from dd0a446 to 02c4a96 Compare September 30, 2024 23:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pubsub: fix permadiff with configuring an empty retry_policy. #11834

pubsub: fix permadiff with configuring an empty retry_policy. #11834

anavada commented Sep 26, 2024

github-actions bot commented Sep 26, 2024

modular-magician commented Sep 26, 2024

modular-magician commented Sep 26, 2024

modular-magician commented Sep 26, 2024

melinath Sep 30, 2024

anavada Sep 30, 2024

modular-magician commented Sep 30, 2024

modular-magician commented Sep 30, 2024

modular-magician commented Sep 30, 2024

modular-magician commented Sep 30, 2024

modular-magician commented Sep 30, 2024

	send_empty_value: true
	send_empty_value: true
	allow_empty_object: true

pubsub: fix permadiff with configuring an empty retry_policy. #11834

Are you sure you want to change the base?

pubsub: fix permadiff with configuring an empty retry_policy. #11834

Conversation

anavada commented Sep 26, 2024

github-actions bot commented Sep 26, 2024

modular-magician commented Sep 26, 2024

Diff report

modular-magician commented Sep 26, 2024

Tests analytics

Action taken

modular-magician commented Sep 26, 2024

melinath Sep 30, 2024

Choose a reason for hiding this comment

anavada Sep 30, 2024

Choose a reason for hiding this comment

modular-magician commented Sep 30, 2024

Diff report

modular-magician commented Sep 30, 2024

Tests analytics

Action taken

modular-magician commented Sep 30, 2024

modular-magician commented Sep 30, 2024

Diff report

modular-magician commented Sep 30, 2024

Tests analytics