Feature/support Add support for L4 Internal Loadbalancer with IPv6

[ericayyliu]

Added the 'ipVersion' field to the forwarding-rule resource. The new field accept enum from one of ['IPV4', 'IPV6']. If not specified, the IPV4 value will be used.

If this PR is for Terraform, I acknowledge that I have:

• Searched through the issue tracker for an open issue that this either resolves or contributes to, commented on it to claim it, and written "fixes {url}" or "part of {url}" in this PR description. If there were no relevant open issues, I opened one and commented that I would like to work on it (not necessary for very small changes).
• Ensured that all new fields I added that can be set by a user appear in at least one example (for generated resources) or third_party test (for handwritten resources or update tests).
• Generated Terraform providers, and ran make test and make lint in the generated providers to ensure it passes unit and linter tests.
• Ran relevant acceptance tests using my own Google Cloud project and credentials (If the acceptance tests do not yet pass or you are unable to run them, please let your reviewer know).
• Read Write release notes before writing my release note below.

compute: added support for `ip_version` to `google_compute_forwarding_rule`

[modular-magician]

Hello! I am a robot. It looks like you are a community contributor. @rileykarson, 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.

[ericayyliu]

Hi @rileykarson, please kindly help to take a look at the PR when you've got some time, thank you :)

[ericayyliu]

Hi @rileykarson just a friendly reminder, can you please help to review the change when you've got sometime? Thanks

[rileykarson]

Looks reasonable at first glance- running build+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.

Terraform GA: Diff ( 3 files changed, 161 insertions(+))
Terraform Beta: Diff ( 3 files changed, 161 insertions(+))
TF Conversion: Diff ( 3 files changed, 13 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 4 files changed, 144 insertions(+))

[modular-magician]

Tests analytics

Total tests: 2912
Passed tests 2608
Skipped tests: 302
Affected tests: 2

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

TestAccContainerAwsNodePool_BetaBasicHandWritten|TestAccComputeForwardingRule_forwardingRuleInternallbIpv6Example

Get to know how VCR tests work

[modular-magician]

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

Rerun these tests in REPLAYING mode to catch issues

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

---

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

[rileykarson]

Ah- two minor lint errors, let's resolve those before merging:

mmv1/products/compute/ForwardingRule.yaml
  Error: 213:20 [colons] too many spaces after colon
  Error: 643:1 [empty-lines] too many blank lines (1 > 0)

[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, 161 insertions(+))
Terraform Beta: Diff ( 3 files changed, 161 insertions(+))
TF Conversion: Diff ( 3 files changed, 13 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 4 files changed, 144 insertions(+))

[modular-magician]

Tests analytics

Total tests: 2913
Passed tests 2610
Skipped tests: 302
Affected tests: 1

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

TestAccContainerAwsNodePool_BetaBasicHandWritten

Get to know how VCR tests work

[modular-magician]

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

[rileykarson]

Agh, I just thought of a possible failure case. Some APIs, GCE in particular, will sometimes return a null when the default value is returned from the API. Could you amend an existing test to explicitly send IPV4? That'll confirm whether we need to mitigate that behaviour here or not.

[ericayyliu]

Got it, I'll try modify one basic test case. Bth, should we do default_value = IPV4, instead of default_from_api=true?

[rileykarson]

Either is fine. Since the field is immutable, default_from_api is a little safer. Explicit default values create a slightly cleaner plan, though, since the default value is known when planning.

[ericayyliu]

@rileykarson can you please help to trigger the workflow again? I registered google org yesterday afternoon, however it still doesn't seem to allow me run the presubmit

[rileykarson]

Triggered! It'll only recheck on new commits, since that's when the job is triggered.

[ericayyliu]

Triggered! It'll only recheck on new commits, since that's when the job is triggered.

Got it. Thanks! Btw, forgot to ask this question in the previous meeting, when will the next release cut for this repo?

[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, 163 insertions(+))
Terraform Beta: Diff ( 3 files changed, 163 insertions(+))
TF Conversion: Diff ( 3 files changed, 13 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 5 files changed, 145 insertions(+))

[rileykarson]

We cut on Tuesdays at EOD and release on the following Monday. go/terraform-releases internally.

[ericayyliu]

Got it. So ultimately I may catch the release on Aug 14th

[rileykarson]

Yep!

[modular-magician]

Tests analytics

Total tests: 2920
Passed tests 2614
Skipped tests: 302
Affected tests: 4

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

TestAccComputeForwardingRule_forwardingRuleInternallbExample|TestAccBigtableAppProfile_bigtableAppProfileSingleclusterExample|TestAccBigtableAppProfile_bigtableAppProfileAnyclusterExample|TestAccSpannerDatabaseIamPolicy

Get to know how VCR tests work

[modular-magician]

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccComputeForwardingRule_forwardingRuleInternallbExample[Debug log]
TestAccBigtableAppProfile_bigtableAppProfileSingleclusterExample[Debug log]
TestAccBigtableAppProfile_bigtableAppProfileAnyclusterExample[Debug log]
TestAccSpannerDatabaseIamPolicy[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

[ericayyliu]

It seems like the tests passed with all three possible values: empty, IPV4 and IPV6. @rileykarson It seems like I don't have the permission to merge the PR, could you please help with the merging part? Thanks!

[rileykarson]

Looking at logs:

• If the value is unset in requests, no ipVersion is returned by the API (i.e. null)
• If the value is set to IPV4 (default), IPV4 is returned
• If the value is set to IPV6, IPV6 is returned

Additionally, the null and IPV4 cases I understand to be equivalent from the API's perspective.

In the event that nulls are returned as IPV4 in the future we'll be fine, as the field is default_from_api.

[ericayyliu]

@rileykarson Thanks a lot for helping in this PR!! One last question post submission -- how do we make sure the change works from end to end, i.e. if we pass in "IPv6" in this field, it will translate to the correct gcloud cmd

[rileykarson]

Terraform and gcloud are both REST API clients, there's no dependency between them. The TestAccComputeForwardingRule_forwardingRuleInternallbIpv6Example test called the REST API and confirmed that the value was returned by the API after being set (see steps in https://googlecloudplatform.github.io/magic-modules/develop/add-mmv1-test/#add-an-mmv1-test), so we're all good.