Fix TTL zero value handling in compute backend service #14239
Conversation
Add send_empty_value: true to TTL fields (defaultTtl, maxTtl, clientTtl) to ensure zero values are included in API requests rather than being treated as empty and omitted. This resolves the issue where setting client_ttl=0 would cause the API to apply its default value of 3600, leading to validation errors when combined with lower max_ttl values. Also adds test case to verify TTL zero value behavior works correctly. Fixes: hashicorp/terraform-provider-google#21632
|
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. |
|
Hello! I am a robot. Tests will require approval from a repository maintainer to run. Googlers: For automatic test runs see go/terraform-auto-test-runs. @roaks3, 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. |
|
@roaks3 This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
There was a problem hiding this comment.
I'm curious to see how the tests handle this, but I'm wondering a bit about the use of default_from_api and send_empty_value together, specifically for users already using the resource without these values set. For example, would they see a diff to change client_ttl to 0 instead of the default 3600 they had previously?
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
Tests analyticsTotal tests: 1225 Click here to see the affected service packages
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
|
|
🟢 Tests passed during RECORDING mode: 🟢 No issues found for passed tests after REPLAYING rerun. 🔴 Tests failed during RECORDING mode: 🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR. |
|
Thank you for the review. I overlooked the concern you raised. I also confirmed that while the tests ran, they're now failing with errors. Since I can't access the detailed error logs, I'll investigate this again in my local environment. |
|
@roaks3 This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @roaks3 This PR has been waiting for review for 1 week. Please take a look! Use the label |
|
@ruchikawa, this PR is waiting for action from you. If no action is taken, this PR will be closed in 28 days. Please address any comments or change requests, or re-request review from a core reviewer if no action is required. This notification can be disabled with the |
|
@ruchikawa, this PR is waiting for action from you. If no action is taken, this PR will be closed in 14 days. Please address any comments or change requests, or re-request review from a core reviewer if no action is required. This notification can be disabled with the |
Add send_empty_value: true to TTL fields (defaultTtl, maxTtl, clientTtl) to ensure zero values are included in API requests rather than being treated as empty and omitted.
This resolves the issue where setting client_ttl=0 would cause the API to apply its default value of 3600, leading to validation errors when combined with lower max_ttl values.
Also adds test case to verify TTL zero value behavior works correctly.
Fixes: hashicorp/terraform-provider-google#21632