vatesfr / terraform-provider-xenorchestra

Xen Orchestra provider for Terraform
MIT License
150 stars 33 forks source link

Plugin crash when trying to pass a computed value for a mac address #191

Closed InputObject2 closed 6 months ago

InputObject2 commented 2 years ago

When trying to set a vm's nic's mac address, the xenorchestra provider does not handle "computed" values because of the validation done on the mac address field, meaning that the mac's value has to be known ahead of time.

Terraform example code where the value is computed:

# provider ivoronin/macaddress:v0.3.0
resource "macaddress" "mac" {
  prefix = [0, 22, 62] # => example value for the resulting address: "00:16:3e:b6:8b:8b"
}

resource "xenorchestra_vm" "vm" {
  [...]
  network {
    network_id  = data.xenorchestra_network.lookup.id
    mac_address = macaddress.mac.address
  }
 [...]
}
terraform plan
╷
│ Error: Plugin did not respond
│
│   with module.vm.xenorchestra_vm.vm,
│   on ../main.tf line 36, in resource "xenorchestra_vm" "vm":
│   36: resource "xenorchestra_vm" "vm" {
│
│ The plugin encountered an error, and failed to respond to the plugin.(*GRPCProvider).PlanResourceChange call. The plugin logs may   
│ contain more details.
╵

Stack trace from the terraform-provider-xenorchestra_v0.21.0 plugin:

panic: Mac address `74D93920-ED26-11E3-AC10-0800200C9A66` was not parsable. This should never happened because Terraform's validation should happen before this is stored into state

goroutine 179 [running]:
github.com/ddelnano/terraform-provider-xenorchestra/xoa.resourceVmSchema.func1(0xd752e0, 0xc000349de0, 0x15, 0x0)
        github.com/ddelnano/terraform-provider-xenorchestra/xoa/resource_xenorchestra_vm.go:185 +0x145
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.schemaMap.diffString(0xc000402810, 0xc000478360, 0x15, 0xc0000e5e00, 0xc00034cf00, 0x1047b68, 0xc00077a380, 0x476d00, 0x7, 0x100000000000000)
        github.com/hashicorp/terraform-plugin-sdk/v2@v2.4.3/helper/schema/schema.go:1375 +0x6e3
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.schemaMap.diff(0xc000402810, 0xc000478360, 0x15, 0xc0000e5e00, 0xc00034d508, 0x1047b68, 0xc00077a380, 0x0, 0x0, 0x0)
        github.com/hashicorp/terraform-plugin-sdk/v2@v2.4.3/helper/schema/schema.go:957 +0x4e8
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.schemaMap.diffList(0xc000402810, 0xecb8c0, 0x7, 0xc00029f680, 0xc00034d508, 0x1047b68, 0xc00077a380, 0x203000, 0x0, 0x0)
        github.com/hashicorp/terraform-plugin-sdk/v2@v2.4.3/helper/schema/schema.go:1091 +0x631
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.schemaMap.diff(0xc000402810, 0xecb8c0, 0x7, 0xc00029f680, 0xc000401960, 0x1047b68, 0xc00077a380, 0x0, 0x0, 0x0)
        github.com/hashicorp/terraform-plugin-sdk/v2@v2.4.3/helper/schema/schema.go:959 +0x473
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.schemaMap.Diff(0xc000402810, 0x1043688, 0xc000623580, 0xc00033acb0, 0xc000854420, 0x0, 0xec45a0, 0xc0001dc1b0, 0x156b300, 0xd752e0, ...)
        github.com/hashicorp/terraform-plugin-sdk/v2@v2.4.3/helper/schema/schema.go:522 +0x219
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).SimpleDiff(0xc0001849c0, 0x1043688, 0xc000623580, 0xc00033acb0, 0xc000854420, 0xec45a0, 0xc0001dc1b0, 0x0, 0x0, 0x0)
        github.com/hashicorp/terraform-plugin-sdk/v2@v2.4.3/helper/schema/resource.go:446 +0x9f
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*GRPCProviderServer).PlanResourceChange(0xc00000c468, 0x1043688, 0xc000623580, 0xc000857c20, 0xc000623580, 0xe85aa0, 0xc00090af00)
        google.golang.org/grpc@v1.32.0/server.go:857 +0x1fd
Error: The terraform-provider-xenorchestra_v0.21.0 plugin crashed!

This is always indicative of a bug within the plugin. It would be immensely helpful if you could report the crash with the plugin's maintainers so that it
can be fixed. The output above should help diagnose the issue.

If XO will not accept invalid values for mac addresses, I don't think we need to have a custom validation here. We should simply forward the XO api error response to the user when they try to pass erroneous data.

ddelnano commented 2 years ago

@InputObject2 thanks for the bug report. I didn't realize it was possible for this to behave this way. I need to investigate this a bit more, but if removing that validation prevents this problem it seems like a possible solution.

In order to verify that the computed field is the problem, I reproduced the error with the terraform code you shared. Running terraform plan causes the same crash you experienced.

I then tried to apply terraform in two steps in order to avoid the mac address being in a computed state. Using the same terraform code, I did the following:

  1. terraform apply -target=macaddress.mac
  2. terraform apply

This avoids the panic and shows that the computed aspect of the mac address is the problem.

InputObject2 commented 2 years ago

Thanks for the quick answer (and for the provider!).

I thought I had included the 2-step apply in my initial comment, guess I left it out. My issue is that a 2-step apply won't work with modules where all the resources are created at the same time.

I was hitting this while trying to package all my vm creation resources (mac, dns, ansible config, vm) into a single module.

ddelnano commented 2 years ago

My issue is that a 2-step apply won't work with modules where all the resources are created at the same time.

I wasn't suggesting that a 2-step apply was a viable workaround :). I just wanted to make sure I understood why this was a "computed property" problem, since I was surprised this was possible. So I wanted to make sure I understood how to reproduce the issue.

ddelnano commented 6 months ago

Sorry for the slow response on this, but this will be fixed in v0.26.1 of the provider.