packet_port_vlan_attachment errors if Metro VLANs are present in the project

[displague]

packet_port_vlan_attachment resources will report errors and fail if Metro VLANs are present in the project.

This affects versions 2.0.0 - 3.2.1.

This issue is also present in the Equinix Metal provider.
equinix/terraform-provider-metal#69

Based on the scope of this problem, we will issue a maintenance release for this problem in the Packet provider.

The Equinix Metal provider should be used for the latest features. equinix/terraform-provider-metal#1

[displague]

There may be other places where Facility.Code (or .Facility.*) is assumed to be present in IP reservations or VLANs. These resources should be made Metro safe.

Metro safe means that these resources do not cause the provider to crash.
This likely means that the some Metro bound resources will be invisible to the Packet provider.

[t0mk]

I looked at the code and actually tried to use metal_port_vlan_attachment with the packet provider (this repo). I had a metro vlan in the project. It doesn't fail. Vlans have FacilityCode attr (a string), so there's no danger of null pointer deref. There might be a problem in certain scenarios, e.g. when user will (somehow) create a metro vlan and then try to attach it with packet_port_vlan_attachment. In that case the attachment resource will not find the vlan. That kind of sucks but it's not a crash.

Devices have facility always populated, so no null pointer defer there.

I will still need to check how it is with reserved and precreated IPs, volumes and connections.

[t0mk]

Volumes don't even have metros so I think we are safe there.

[t0mk]

Datasource packet_precreated_ip_block and packet_ip_attachment work ok, no crash.

[t0mk]

• packet_reserved_ip_block and packet_ip_attachment no crash
• creating packet_device with

  ip_address {
    type            = "public_ipv4"
    cidr            = 31
    reservation_ids = [packet_reserved_ip_block.example.id]
  }

.. also OK and no crash

[t0mk]

@displague Do you have a hcl template demonstrating the crash?

[displague]

#  export PACKET_AUTH_TOKEN=...
# terraform providers lock
# terraform init -upgrade
# terraform apply

terraform {
  required_providers {
    packet = {
      source = "packethost/packet"
    }
    metal = {
      source = "equinix/metal"
    }
  }
  required_version = ">= 0.13"
}

provider "packet" {}
provider "metal" {}

resource "metal_project" "foo" {
  name = "foo"
}

// This represents an existing Metro VLAN. I don't expect that anyone would mix the packet and metal provider in this way
resource "metal_vlan" "foo" {
  project_id = metal_project.foo.id
  metro      = "sv"
}

/**
// Facility VLANs are what we would find in this configuration, I don't need them to hit the bug.

// This could be metal_vlan, shouldn't be a difference.  This doesn't have to exist.
resource "packet_vlan" "foo" {
  project_id = metal_project.foo.id
  facility = "sv15"
}

// This could be metal_vlan, shouldn't be a difference. This doesn't have to exist.
resource "packet_vlan" "foo" {
  project_id = metal_project.foo.id
  facility = "ewr1"
}
**/

// This could be metal_device, shouldn't make a difference.
// This doesn't have to exist if we know an existing port_id
resource "packet_device" "foo" {
  hostname         = "foo"
  plan             = "c3.medium.x86"
  facilities       = ["sv15"]
  operating_system = "ubuntu_20_04"
  billing_cycle    = "hourly"
  project_id       = metal_project.foo.id
}

// I expect this to crash
resource "packet_port_vlan_attachment" "foo" {
  device_id = packet_device.foo.id
  port_name = "eth1"

  // This attachment would never work, but
  // I expect it to crash before it tries to attach,
  // it should crash when it finds this vlan in the loop
  // because v.Facility is nil and v.Facility.Code is accessed
  vlan_vnid = metal_vlan.foo.vxlan
}

[displague]

I suspected that the above would trigger a crash, but I was misreading the v.FacilityCode as v.Facility.Code. I think we are safe here.

Thanks for the diligent inspection and testing, @t0mk!