Add Beta support & Beta feature ip_version to google_compute_global_address #250
Conversation
| resource.TestStep{ | ||
| Config: testAccComputeGlobalAddress_ipv6, | ||
| Check: resource.ComposeTestCheckFunc( | ||
| testAccCheckComputeBetaGlobalAddressExists( |
There was a problem hiding this comment.
Consider asserting on the format of the computed address to make sure an IPv6 is used.
Additionally, to prevent regression, make sure the ipv4 address follows the ipv4 pattern.
There was a problem hiding this comment.
I'm not sure I agree that we need to validate that the API isn't lying to us when it says an IP address is at some version; especially when we would need to use a regex or vendor a new package to verify it.
Added a test for IPV4 on _basic, and moved checks to a TestCheckFunc.
There was a problem hiding this comment.
You could use https://golang.org/pkg/net/#ParseIP
ip := ParseIP(address)
ip.To16 != nil // means it is a IPv6 address.
ip.To4 != nil // means this is a IPv4 address.
Checking if the IpVersion match what we expect is probably fine for the IPv6 case. My main goal was not to test if the API "lies" to us but rather make sure we don't create an ipv6 address when we don't use the beta feature (and since we don't have the ip version field in that case, the only way to check is looking at address itself to validate it is ipv4).
There was a problem hiding this comment.
Ah, ok - we can actually just read the resource using a Beta API in this test and see the IpVersion; that's what the TestCheckFunc does.
|
…ddress (hashicorp#250) * Make google_compute_global_address a versioned resource with Beta support. * Added Beta support for ip_version in google_compute_global_address. * Move checks to TestCheckFuncs, add a regression test for IPV4 on v1 resources. * Consolidated TestCheckFuncs to a single function. * Add missing return statement. * Fix IPV4 test * Clarified comment.
…ddress (hashicorp#250) * Make google_compute_global_address a versioned resource with Beta support. * Added Beta support for ip_version in google_compute_global_address. * Move checks to TestCheckFuncs, add a regression test for IPV4 on v1 resources. * Consolidated TestCheckFuncs to a single function. * Add missing return statement. * Fix IPV4 test * Clarified comment.
…ddress (hashicorp#250) * Make google_compute_global_address a versioned resource with Beta support. * Added Beta support for ip_version in google_compute_global_address. * Move checks to TestCheckFuncs, add a regression test for IPV4 on v1 resources. * Consolidated TestCheckFuncs to a single function. * Add missing return statement. * Fix IPV4 test * Clarified comment.
<!-- This change is generated by MagicModules. --> /cc @danawillow
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks! |
ghost
locked and limited conversation to collaborators
Waiting on #249 to merge, but otherwise able to be reviewed.
google_compute_global_addressto a versioned resource with Beta support.ip_versiontogoogle_compute_global_addressExplicitly setting
IPV4on av1resource forces a recreate as you're going from""toIPV4and this resource can't update; this is working as expected based on #185. We don't have enough information at refresh time to perform the read atv0beta.Part 1/2 of #245.
Related to #93