smutel / terraform-provider-netbox

Terraform provider for Netbox
ISC License
58 stars 19 forks source link

feat: Add netbox_virtualization_vm_primary_ip resource #158

Closed amhn closed 1 year ago

amhn commented 1 year ago

Fixes #162

New try of tackling the primary_ip6 issue.

This PR creates a new resource netbox_virtualization_vm_primary_ip which is intended to replace the primary_ip4 flag in netbox_ipam_ip_addresses.

Several advantages:

Disadvantage:

If primary_ip4 is not set in the config it is ignored on Update/Create. There is a possibility to create a conflicting config by using primary_ip4 and netbox_virtualization_vm_primary_ip. I do not know if it is possible to guard against that, except by removing primary_ip4.

On delete of netbox_virtualization_vm_primary_ip the primary_ip4 and primary_ip6 fields are both set to null.

smutel commented 1 year ago

Thank you for this PR.

Did you test the migration from legacy to this new method ?

amhn commented 1 year ago

Yes. I ran terraform apply in master then switched to this branch and ran terraform apply again:

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # netbox_ipam_ip_addresses.ip6_test will be created
  + resource "netbox_ipam_ip_addresses" "ip6_test" {
      + address      = "2001:db8::1234/64"
      + content_type = (known after apply)
      + id           = (known after apply)
      + object_id    = 1
      + object_type  = "virtualization.vminterface"
      + status       = "active"
    }

  # netbox_virtualization_vm_primary_ip.name will be created
  + resource "netbox_virtualization_vm_primary_ip" "name" {
      + id                = (known after apply)
      + primary_ip4_id    = 2
      + primary_ip6_id    = (known after apply)
      + virtualmachine_id = 25
    }

Plan: 2 to add, 0 to change, 0 to destroy.

Deleting the resource, removing either primary_ip field, even keeping the primary_ip4 or even switching back to it, works.

I just noticed everything is in place to write tests for this resource. I will do that.

amhn commented 1 year ago

Added tests and found a bug on the way.

This should be merged after #160 and the package of netbox/resource_netbox_virtualization_vm_primary_ip_test.go should then be changed to netbox_test.

smutel commented 1 year ago

160 has been merged. Tell me if you need to rebase and tweak this PR before merging.

amhn commented 1 year ago

PR is rebased and ready to go.