Fix issue with google_compute_(region_)per_instance_config.stateful.ips.ipAddress.address always having diff due to relative vs full path

[askubis]

Convert the full path to

fixes hashicorp/terraform-provider-google#19265

Release Note Template for Downstream PRs (will be copied)

compute: fixed permadiffs on stateful ip address in `google_compute_region_per_instance_config` and `google_compute_per_instance_config` resources

[github-actions]

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

@shuyama1, 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]

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 ( 4 files changed, 22 insertions(+), 6 deletions(-))
google-beta provider: Diff ( 4 files changed, 22 insertions(+), 6 deletions(-))

[modular-magician]

Tests analytics

Total tests: 1036
Passed tests: 959
Skipped tests: 73
Affected tests: 4

Click here to see the affected service packages

• compute

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

• TestAccComputePerInstanceConfig_removeInstanceOnDestroy
• TestAccComputePerInstanceConfig_statefulIps
• TestAccComputeRegionPerInstanceConfig_removeInstanceOnDestroy
• TestAccComputeRegionPerInstanceConfig_statefulIps

Get to know how VCR tests work

[modular-magician]

🔴 Tests failed during RECORDING mode:
TestAccComputePerInstanceConfig_removeInstanceOnDestroy [Error message] [Debug log]
TestAccComputePerInstanceConfig_statefulIps [Error message] [Debug log]
TestAccComputeRegionPerInstanceConfig_removeInstanceOnDestroy [Error message] [Debug log]
TestAccComputeRegionPerInstanceConfig_statefulIps [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]

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 ( 6 files changed, 34 insertions(+), 18 deletions(-))
google-beta provider: Diff ( 6 files changed, 34 insertions(+), 18 deletions(-))

[modular-magician]

Tests analytics

Total tests: 1036
Passed tests: 962
Skipped tests: 73
Affected tests: 1

Click here to see the affected service packages

• compute

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

• TestAccComputeRegionPerInstanceConfig_removeInstanceOnDestroy

Get to know how VCR tests work

[modular-magician]

🟢 Tests passed during RECORDING mode:
TestAccComputeRegionPerInstanceConfig_removeInstanceOnDestroy [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

[askubis]

@shuyama1 is this considered a breaking change? Since we had to modify tests I would imagine it is, but please share your take on this :)

[github-actions]

@shuyama1 This PR has been waiting for review for 3 weekdays. Please take a look! Use the label disable-review-reminders to disable these notifications.

[shuyama1]

@shuyama1 is this considered a breaking change? Since we had to modify tests I would imagine it is, but please share your take on this :)

Thanks for making the change. Let me confirm if I understand the case correctly -

• Before the change:
  we can send both full and relative path to the API, but since the API only returns the full URL, we'll run into permadiffs for cases where relative path is specified.

• After the change:
  Since we add a custom flattener, it will always write the relative path to the state, which will result in permadiff for cases where full path is specified.

If I understand the case correctly, yes, this change will break current configs. We'll need to find a solution ensure that sending either full or relative path should result in no permadiffs in Terraform. I'm thinking that adding a diff suppress function might be working in this situation, likely you can use the existing function CompareSelfLinkRelativePaths

Please let me know if you have any questions.

[github-actions]

@askubis, 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 disable-automatic-closure label.

[github-actions]

@askubis, 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 disable-automatic-closure label.

[github-actions]

@askubis, this PR is waiting for action from you. If no action is taken, this PR will be closed in 2 weekdays.

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 disable-automatic-closure label.

[modular-magician]

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 ( 7 files changed, 79 insertions(+), 32 deletions(-))
google-beta provider: Diff ( 7 files changed, 79 insertions(+), 32 deletions(-))

[askubis]

I think I need some help with that change.
I modified the tests first - both .self_link and .id should work (because why not, one is the current state, the other is what we recommend in the documentation)
I did what was recommended - added the diff_supress_func. I even added tests to it, so that it would be clear that it does what expected (
"partial path, same": {
Old: "https://www.googleapis.com/compute/v1/projects/your-project/global/networks/a-network",
New: "projects/your-project/global/networks/a-network",
Expect: true,
},)
And yet, still we have the same diff when running the test.
` - internal_ip {
- auto_delete = "NEVER" -> null
- interface_name = "nic0" -> null

                  - ip_address {
                      - address = "https://www.googleapis.com/compute/v1/projects/REDACTED/regions/us-central1/addresses/tf-test-rigm-address97pzzqv6th" -> null
                    }
                }
              + internal_ip {
                  + auto_delete    = "NEVER"
                  + interface_name = "nic0"
    
                  + ip_address {
                      + address = "projects/REDACTED/regions/us-central1/addresses/tf-test-rigm-address97pzzqv6th"
                    }
                }

`

any idea what could I be doing wrong here?

[modular-magician]

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 ( 7 files changed, 75 insertions(+), 12 deletions(-))
google-beta provider: Diff ( 7 files changed, 75 insertions(+), 12 deletions(-))

[modular-magician]

Tests analytics

Total tests: 4328
Passed tests: 3908
Skipped tests: 407
Affected tests: 13

Click here to see the affected service packages

All service packages are affected

Action taken

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

• TestAccComputePerInstanceConfig_removeInstanceOnDestroy
• TestAccComputePerInstanceConfig_statefulIps
• TestAccComputeRegionPerInstanceConfig_removeInstanceOnDestroy
• TestAccComputeRegionPerInstanceConfig_statefulIps
• TestAccDataSourceGoogleQuotaInfo_basic
• TestAccOracleDatabaseAutonomousDatabase_basic
• TestAccOracleDatabaseAutonomousDatabases_basic
• TestAccOracleDatabaseCloudExadataInfrastructure_basic
• TestAccOracleDatabaseCloudExadataInfrastructures_basic
• TestAccOracleDatabaseCloudVmCluster_basic
• TestAccOracleDatabaseDbNodes_basic
• TestAccOracleDatabaseDbServers_basic
• TestAccSecureSourceManagerInstance_secureSourceManagerInstancePrivatePscBackendExample

Get to know how VCR tests work

[modular-magician]

Tests analytics

Total tests: 4328
Passed tests: 3908
Skipped tests: 407
Affected tests: 13

Click here to see the affected service packages

All service packages are affected

Action taken

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

• TestAccComputePerInstanceConfig_removeInstanceOnDestroy
• TestAccComputePerInstanceConfig_statefulIps
• TestAccComputeRegionPerInstanceConfig_removeInstanceOnDestroy
• TestAccComputeRegionPerInstanceConfig_statefulIps
• TestAccDataSourceGoogleQuotaInfo_basic
• TestAccOracleDatabaseAutonomousDatabase_basic
• TestAccOracleDatabaseAutonomousDatabases_basic
• TestAccOracleDatabaseCloudExadataInfrastructure_basic
• TestAccOracleDatabaseCloudExadataInfrastructures_basic
• TestAccOracleDatabaseCloudVmCluster_basic
• TestAccOracleDatabaseDbNodes_basic
• TestAccOracleDatabaseDbServers_basic
• TestAccSecureSourceManagerInstance_secureSourceManagerInstancePrivatePscBackendExample

Get to know how VCR tests work

[modular-magician]

🔴 Tests failed during RECORDING mode:
TestAccComputePerInstanceConfig_removeInstanceOnDestroy [Error message] [Debug log]
TestAccComputePerInstanceConfig_statefulIps [Error message] [Debug log]
TestAccComputeRegionPerInstanceConfig_removeInstanceOnDestroy [Error message] [Debug log]
TestAccComputeRegionPerInstanceConfig_statefulIps [Error message] [Debug log]
TestAccDataSourceGoogleQuotaInfo_basic [Error message] [Debug log]
TestAccOracleDatabaseAutonomousDatabase_basic [Error message] [Debug log]
TestAccOracleDatabaseAutonomousDatabases_basic [Error message] [Debug log]
TestAccOracleDatabaseCloudExadataInfrastructure_basic [Error message] [Debug log]
TestAccOracleDatabaseCloudExadataInfrastructures_basic [Error message] [Debug log]
TestAccOracleDatabaseCloudVmCluster_basic [Error message] [Debug log]
TestAccOracleDatabaseDbNodes_basic [Error message] [Debug log]
TestAccOracleDatabaseDbServers_basic [Error message] [Debug log]
TestAccSecureSourceManagerInstance_secureSourceManagerInstancePrivatePscBackendExample [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]

🔴 Tests failed during RECORDING mode:
TestAccComputePerInstanceConfig_removeInstanceOnDestroy [Error message] [Debug log]
TestAccComputePerInstanceConfig_statefulIps [Error message] [Debug log]
TestAccComputeRegionPerInstanceConfig_removeInstanceOnDestroy [Error message] [Debug log]
TestAccComputeRegionPerInstanceConfig_statefulIps [Error message] [Debug log]
TestAccDataSourceGoogleQuotaInfo_basic [Error message] [Debug log]
TestAccOracleDatabaseAutonomousDatabase_basic [Error message] [Debug log]
TestAccOracleDatabaseAutonomousDatabases_basic [Error message] [Debug log]
TestAccOracleDatabaseCloudExadataInfrastructure_basic [Error message] [Debug log]
TestAccOracleDatabaseCloudExadataInfrastructures_basic [Error message] [Debug log]
TestAccOracleDatabaseCloudVmCluster_basic [Error message] [Debug log]
TestAccOracleDatabaseDbNodes_basic [Error message] [Debug log]
TestAccOracleDatabaseDbServers_basic [Error message] [Debug log]
TestAccSecureSourceManagerInstance_secureSourceManagerInstancePrivatePscBackendExample [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

[github-actions]

@shuyama1 This PR has been waiting for review for 3 weekdays. Please take a look! Use the label disable-review-reminders to disable these notifications.

[github-actions]

@GoogleCloudPlatform/terraform-team @shuyama1 This PR has been waiting for review for 1 week. Please take a look! Use the label disable-review-reminders to disable these notifications.

[shuyama1]

I think I need some help with that change. I modified the tests first - both .self_link and .id should work (because why not, one is the current state, the other is what we recommend in the documentation) I did what was recommended - added the diff_supress_func. I even added tests to it, so that it would be clear that it does what expected ( "partial path, same": { Old: "googleapis.com/compute/v1/projects/your-project/global/networks/a-network", New: "projects/your-project/global/networks/a-network", Expect: true, },) And yet, still we have the same diff when running the test. ` - internal_ip { - auto_delete = "NEVER" -> null - interface_name = "nic0" -> null

                  - ip_address {
                      - address = "https://www.googleapis.com/compute/v1/projects/REDACTED/regions/us-central1/addresses/tf-test-rigm-address97pzzqv6th" -> null
                    }
                }
              + internal_ip {
                  + auto_delete    = "NEVER"
                  + interface_name = "nic0"
    
                  + ip_address {
                      + address = "projects/REDACTED/regions/us-central1/addresses/tf-test-rigm-address97pzzqv6th"
                    }
                }

`

any idea what could I be doing wrong here?

Okay, I’ve looked into this issue a bit and found that the DSF not working is likely due to a known problem with the SDK hashicorp/terraform-plugin-sdk#477. It seems we may need to write a custom DSF for its parent set field. I’m trying out some solutions locally and will share updates as I progress.

[github-actions]

@askubis, 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 disable-automatic-closure label.

[github-actions]

@shuyama1 This PR has been waiting for review for 3 weekdays. Please take a look! Use the label disable-review-reminders to disable these notifications.

[github-actions]

@GoogleCloudPlatform/terraform-team @shuyama1 This PR has been waiting for review for 1 week. Please take a look! Use the label disable-review-reminders to disable these notifications.