magic-modules icon indicating copy to clipboard operation
magic-modules copied to clipboard

add network_url attribute in consumer_accept_list block of google_compute_service_attachment resource

Open laurensknoll opened this issue 1 year ago • 13 comments

This change adds the network_url attribute to the consumer_accept_list-block of the google_compute_service_attachment resource.

Note that because ExactlyOneOf is not available for lists, the projectIdOrNum attribute is made optional. Issue: https://github.com/hashicorp/terraform-plugin-sdk/issues/470

Fixes:

  • https://github.com/hashicorp/terraform-provider-google/issues/15203
  • https://github.com/hashicorp/terraform-provider-google/issues/17116

An service_attachment_explicit_networks example is added to demonstrate the functionality and acts as an acceptance test.

Release Note Template for Downstream PRs (will be copied)

compute: added the `network_url` attribute to the `consumer_accept_list`-block of the `google_compute_service_attachment` resource.

laurensknoll avatar Jan 30 '24 18:01 laurensknoll

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.

google-cla[bot] avatar Jan 30 '24 18:01 google-cla[bot]

Hello! I am a robot. It looks like you are a: Community Contributor ~Googler~ ~Core Contributor~. Tests will require approval to run.

@slevenick, 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 avatar Jan 30 '24 18:01 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.

Terraform GA: Diff ( 3 files changed, 267 insertions(+), 4 deletions(-)) Terraform Beta: Diff ( 3 files changed, 267 insertions(+), 4 deletions(-)) TF Conversion: Diff ( 1 file changed, 11 insertions(+)) TF OiCS: Diff ( 4 files changed, 198 insertions(+))

modular-magician avatar Jan 30 '24 20:01 modular-magician

Tests analytics

Total tests: 874 Passed tests 808 Skipped tests: 65 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
TestAccComputeServiceAttachment_serviceAttachmentExplicitNetworksExample

Get to know how VCR tests work

modular-magician avatar Jan 30 '24 20:01 modular-magician

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$ TestAccComputeServiceAttachment_serviceAttachmentExplicitNetworksExample[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

modular-magician avatar Jan 30 '24 20:01 modular-magician

Test is failing with a diff after apply. Were you able to run it successfully?

Thanks for following up @slevenick .

While the ga test works, the beta test fails:

...
=== CONT  TestAccComputeServiceAttachment_serviceAttachmentExplicitNetworksExample
    vcr_utils.go:152: Step 1/2 error: After applying this test step and performing a `terraform refresh`, the plan was not empty.
        stdout
        
        
        Terraform used the selected providers to generate the following execution
        plan. Resource actions are indicated with the following symbols:
          ~ update in-place
        
        Terraform will perform the following actions:
        
          # google_compute_service_attachment.psc_ilb_service_attachment will be updated in-place
          ~ resource "google_compute_service_attachment" "psc_ilb_service_attachment" {
                id                    = "projects/my-project/regions/us-west2/serviceAttachments/tf-test-my-psc-ilbg66dgg4vlb"
                name                  = "tf-test-my-psc-ilbg66dgg4vlb"
                # (13 unchanged attributes hidden)
        
              - consumer_accept_lists {
                  - connection_limit = 1 -> null
                  - network_url      = "https://www.googleapis.com/compute/beta/projects/my-project/global/networks/tf-test-psc-ilb-consumer-networkg66dgg4vlb" -> null
                }
              + consumer_accept_lists {
                  + connection_limit = 1
                  + network_url      = "https://www.googleapis.com/compute/v1/projects/my-project/global/networks/tf-test-psc-ilb-consumer-networkg66dgg4vlb"
                }
            }
        
        Plan: 0 to add, 1 to change, 0 to destroy.

Apparently the beta API returns the GA network url reference. Any thoughts to address this?

laurensknoll avatar Jan 31 '24 18:01 laurensknoll

I think you can add diff_suppress_func: 'tpgresource.CompareSelfLinkOrResourceName' to the field to handle the comparison

slevenick avatar Jan 31 '24 18:01 slevenick

I think you can add diff_suppress_func: 'tpgresource.CompareSelfLinkOrResourceName' to the field to handle the comparison

Thanks for the pointer @slevenick . Combined with a custom set_hash_func the relative path comparisons prevent the (false positive) error.

Curious to know if a more generic set_hash_func is available. In the end we just want to hash the resource while including the diff suppress func logic.

laurensknoll avatar Feb 01 '24 20:02 laurensknoll

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.

Terraform GA: Diff ( 3 files changed, 307 insertions(+), 6 deletions(-)) Terraform Beta: Diff ( 3 files changed, 307 insertions(+), 6 deletions(-)) TF Conversion: Diff ( 1 file changed, 49 insertions(+)) TF OiCS: Diff ( 4 files changed, 198 insertions(+))

modular-magician avatar Feb 02 '24 15:02 modular-magician

Tests analytics

Total tests: 874 Passed tests 808 Skipped tests: 65 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
TestAccComputeServiceAttachment_serviceAttachmentExplicitNetworksExample

Get to know how VCR tests work

modular-magician avatar Feb 02 '24 15:02 modular-magician

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$ TestAccComputeServiceAttachment_serviceAttachmentExplicitNetworksExample[Debug log]

Rerun these tests in REPLAYING mode to catch issues

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


$\textcolor{green}{\textsf{All tests passed!}}$ View the build log or the debug log for each test

modular-magician avatar Feb 02 '24 16:02 modular-magician

Hi @slevenick , the automated tests have passed. Could you re-review the changes?

laurensknoll avatar Feb 08 '24 15:02 laurensknoll

@slevenick ping

melinath avatar Feb 29 '24 21:02 melinath