Implement private_ip_address_version for network interfaces.

[literatesnow]

Implements #2543 and allows IPv6 ip_configurations to be created in azurerm_network_interface.

• Added private_ip_address_version (IPv4/IPv6).
• subnet_id becomes optional for IPv6 and required for IPv4.

[tombuildsstuff]

hey @literatesnow

Thanks for this PR :)

I've taken a look through and left some comments in-line but this mostly LGTM - if we can fix up the comments (and the tests pass) we should be able to get this merged 👍

Thanks!

[literatesnow]

Hey @tombuildsstuff

Thanks for the suggestions, I've updated the PR.

The tests are failing with fork: Cannot allocate memory, problem with Travis CI?

[katbyte]

@literatesnow, i restarted the travis build and it seems to have passed now 🙂

[katbyte]

(accidental click)

[katbyte]

(double post)

[literatesnow]

Thanks @katbyte

[katbyte]

@literatesnow,

Thank you for the updates! Aside from a couple comments i've left inline this is looking pretty good. I am mainly concerned with two unchecked dereferences that could result in a panic. Once those are nil checked this should be good to merge 🙂

[tdhineshkumar]

Does this change applicable for VMSS (network_profile --> ip_configuration) ?

[literatesnow]

Thanks for the suggestions @katbyte, the PR has been updated.

I don't know @tdhineshkumar

[katbyte]

@literatesnow,

Thank you for the updates, one last glance and i noticed that private_ip_address_version is not in the schema, apologies for not noticing this earlier.

I'm not sure if its best to add it as a computed value or simply not set it on read, but right now it's a error that is silently ignored 😅

[katbyte]

This does not exist in the schema

[literatesnow]

@katbyte Add private_ip_address_version to the schema here? https://github.com/literatesnow/terraform-provider-azurerm/blob/b44b8b7275cbda465392b08df85c89a3c0fb04ab/azurerm/resource_arm_network_interface.go#L213

I'd say you'd know better if it's computed or not :)

[literatesnow]

@katbyte I've added the private IP address version to the schema (computed) and hopefully made other code more readable.

[katbyte]

@literatesnow,

Upon further investigation it makes sense to not have the ip version at the top level so i have removed it, as well as cleaned up some crash points, hope you don't mind 🙂

this LGTM now, however @tombuildsstuff could you give it a quick once over as i did some updates?

[tombuildsstuff]

otherwise LGTM 👍

[tombuildsstuff]

Ignoring a known test failure the tests pass:

[literatesnow]

Thank you @tombuildsstuff and @katbyte !

[ghost]

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 👉 hashibot-feedback@hashicorp.com. Thanks!