Fix global_forwarding_rule labels

[wiktorn]

• enable tests for global_forwarding_rule for ga provider
• add test for global_forwarding_rule for PSC target
• do not send labels for global_forwarding_rule for PSC target

Fixes: hashicorp/terraform-provider-google#20873

Release Note Template for Downstream PRs (will be copied)

See Write release notes for guidance.

compute: fix failure when creating `google_compute_global_forwarding_rule` with labels targeting PSC endpoint

[github-actions]

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

@BBBmau, 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 ( 2 files changed, 238 insertions(+))
google-beta provider: Diff ( 2 files changed, 92 insertions(+))

[modular-magician]

Tests analytics

Total tests: 1081
Passed tests: 1007
Skipped tests: 73
Affected tests: 1

Click here to see the affected service packages

• compute

#### Non-exercised tests

🔴 Tests were added that are GA-only additions and require manual runs:

• TestAccComputeGlobalForwardingRule_labels

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

• TestAccComputeGlobalForwardingRule_pscLabels

Get to know how VCR tests work

[modular-magician]

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

[BBBmau]

targetProp != nil isn't necssary since the field "target" is a required field: https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/compute_global_forwarding_rule.html#target-1

also i'm not familiar with why we can't set labels on create for PSC forwarding rules. Could you direct to me docs that helps me better understand the reason?

we can also change it from || -> && since for PSC rules it needs to be serviceAttachment. From docs:

For Private Service Connect forwarding rules that forward traffic to managed services, the target must be a service attachment. The target is not mutable once set as a service attachment.

https://cloud.google.com/compute/docs/reference/rest/v1/globalForwardingRules

[wiktorn]

Ack on targetProp != nil.

Regarding PSC - one way to detect that this is a PSC target is to check that it has serviceAttachment, as the docs you link specify.

But there is also special case, documented here where instead of normal serviceAttachment you provide vpc-sc or all-apis value, which is also a Private Service Connect endpoint.

Condition strings.Contains(targetProp.(string), "/serviceAttachments/") && targetProp.(string) == "all-apis" is never true, so I can't change || to &&.

In terms of why it is not possible to set the labels on forwarding rules targeting PSC attachements - I haven't found any reference in public docs, though the same was fixed in #17334 for google_compute_forwarding_rule

[wiktorn]

@BBBmau updated the PR, leaving one comment open for you to close, if you're happy with my reply.

[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 ( 2 files changed, 320 insertions(+))
google-beta provider: Diff ( 2 files changed, 174 insertions(+))

[modular-magician]

Tests analytics

Total tests: 1096
Passed tests: 1019
Skipped tests: 75
Affected tests: 2

Click here to see the affected service packages

• compute

#### Non-exercised tests

🔴 Tests were added that are GA-only additions and require manual runs:

• TestAccComputeGlobalForwardingRule_labels

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

• TestAccComputeGlobalForwardingRule_allApisLabels
• TestAccComputeGlobalForwardingRule_vpcscLabels

Get to know how VCR tests work

[modular-magician]

🟢 Tests passed during RECORDING mode:
TestAccComputeGlobalForwardingRule_allApisLabels [Debug log]
TestAccComputeGlobalForwardingRule_vpcscLabels [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

[BBBmau]

thanks for providing the explanation in your previous comment!

Approving and merging after all new tests passing as well as forwardingRules_labels test passing: https://hashicorp.teamcity.com/buildConfiguration/TerraformProviders_GoogleCloud_GOOGLE_MMUPSTREAMTESTS_GOOGLE_PACKAGE_COMPUTE/306442?buildTab=tests