MAC address of Values Not Yet Known not working

[ravager-dk]

Mac addresses calculated at runtime by another resource fails plan face.

The issue is caused by StateFunc throwing parsing error. This should not be used during planning.

Created pull request #233 to fix it

[ravager-dk]

Stack trace from the terraform-provider-xenorchestra_v0.24.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 119 [running]: github.com/ddelnano/terraform-provider-xenorchestra/xoa.resourceVmSchema.func1(0xd81f00, 0xc000281ba0, 0x15, 0x0) github.com/ddelnano/terraform-provider-xenorchestra/xoa/resource_xenorchestra_vm.go:237 +0x145 github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.schemaMap.diffString(0xc0003a24e0, 0xc0003b4840, 0x15, 0xc00027f900, 0xc0004e8f00, 0x1058a68, 0xc0006e6d80, 0x476e00, 0x0, 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(0xc0003a24e0, 0xc0003b4840, 0x15, 0xc00027f900, 0xc0004e9508, 0x1058a68, 0xc0006e6d80, 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(0xc0003a24e0, 0xeda417, 0x7, 0xc000404280, 0xc0004e9508, 0x1058a68, 0xc0006e6d80, 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(0xc0003a24e0, 0xeda417, 0x7, 0xc000404280, 0xc0005176e0, 0x1058a68, 0xc0006e6d80, 0x40d900, 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(0xc0003a24e0, 0x10544c8, 0xc00054d9c0, 0xc0005752d0, 0xc0001fe120, 0x0, 0xed3580, 0xc000349ea0, 0x1582700, 0xd81f00, ...) 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(0xc0001629c0, 0x10544c8, 0xc00054d9c0, 0xc0005752d0, 0xc0001fe120, 0xed3580, 0xc000349ea0, 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(0xc0003a8048, 0x10544c8, 0xc00054d9c0, 0xc0003f5860, 0xc00054d9c0, 0xe93ce0, 0xc00035aa00) github.com/hashicorp/terraform-plugin-sdk/v2@v2.4.3/helper/schema/grpc_provider.go:693 +0x7c5 github.com/hashicorp/terraform-plugin-go/tfprotov5/server.(server).PlanResourceChange(0xc0003a1780, 0x1054570, 0xc00054d9c0, 0xc000574e00, 0xc0003a1780, 0xc00035aab0, 0xc000103ba0) github.com/hashicorp/terraform-plugin-go@v0.2.1/tfprotov5/server/server.go:315 +0xb5 github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5._Provider_PlanResourceChange_Handler(0xe93ce0, 0xc0003a1780, 0x1054570, 0xc00035aab0, 0xc0000b54a0, 0x0, 0x1054570, 0xc00035aab0, 0xc00020f100, 0x643) github.com/hashicorp/terraform-plugin-go@v0.2.1/tfprotov5/internal/tfplugin5/tfplugin5_grpc.pb.go:362 +0x214 google.golang.org/grpc.(Server).processUnaryRPC(0xc000338000, 0x105cb38, 0xc0003b7c80, 0xc0000fee00, 0xc0000b6720, 0x1542708, 0x0, 0x0, 0x0) google.golang.org/grpc@v1.32.0/server.go:1194 +0x52b google.golang.org/grpc.(Server).handleStream(0xc000338000, 0x105cb38, 0xc0003b7c80, 0xc0000fee00, 0x0) google.golang.org/grpc@v1.32.0/server.go:1517 +0xd0c google.golang.org/grpc.(Server).serveStreams.func1.2(0xc0003ae1b0, 0xc000338000, 0x105cb38, 0xc0003b7c80, 0xc0000fee00) google.golang.org/grpc@v1.32.0/server.go:859 +0xab created by google.golang.org/grpc.(*Server).serveStreams.func1 google.golang.org/grpc@v1.32.0/server.go:857 +0x1fd

Error: The terraform-provider-xenorchestra_v0.24.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.

[ravager-dk]

Reproducible with the following code: ` terraform { required_providers { xenorchestra = { source = "terra-farm/xenorchestra" version = "~>0.24.0" } macaddress = { source = "ivoronin/macaddress" version = "0.3.2" } } }

provider "xenorchestra" { insecure = true # Or set XOA_INSECURE environment variable to any value }

provider "macaddress" {

}

data "xenorchestra_pool" "pool" { name_label = "LabPool" }

data "xenorchestra_host" "host" { name_label = "xcp-ng-lab" }

data "xenorchestra_template" "template" { name_label = "Template" }

data "xenorchestra_network" "NetWork" { name_label = "NetworkName" pool_id = data.xenorchestra_pool.pool.id }

data "xenorchestra_sr" "local_storage" { name_label = "Local storage" pool_id = data.xenorchestra_pool.pool.id }

resource "macaddress" "mac" { count = 2 // ea:48:63 }

resource "xenorchestra_vm" "VMss" { affinity_host = data.xenorchestra_host.host.id count = 2 cpus = 2 disk { attached = true name_label = "Test-${count.index + 1}-Disk0" size = 21474836480 sr_id = data.xenorchestra_sr.local_storage.id } template = data.xenorchestra_template.template.id exp_nested_hvm = false memory_max = 2147483648 name_description = "Test server" name_label = "Test-${count.index + 1}" network { attached = true

device = 0

network_id = data.xenorchestra_network.NetWork.id
mac_address = macaddress.mac[count.index].address

} vga = "cirrus" videoram = 4 } `

[ddelnano]

At first I mistook the 74D93920-ED26-11E3-AC10-0800200C9A66 value as a malformed mac address from the ivoronin/macaddress provider. It turns out this is a sentinel value used by terraform to indicate variables that are unknown at a particular time (source). In addition to that, computed properties should not have StateFunc set (see https://github.com/hashicorp/terraform/issues/30447 for more details).

I mention this because I didn't fully understand how this issue manifested and was initially hesitant to change StateFunc's behavior as proposed in #236 without understanding that context.

Given the Terraform team states that StateFunc shouldn't be used with computed properties, I'm hesitant to update its logic to handle terraform's unknown sentinel value. It seems like the more appropriate fix would be to remove the StateFunc property entirely or change mac_address to no longer be computed (if that makes sense and is possible).

Since the error is caused by terraform's unknown sentinel value, if you apply the macaddress configuration first (via terraform apply -target=macaddress.mac), it will prevent this sentinel value from existing and triggering the panic. I realize that isn't ideal, but given StateFunc is incorrect for this computed property I'd prefer to evaluate the options I mentioned above before we commit to incorrectly use StateFunc.

Will that work as a temporary workaround until this is investigated?

[ravager-dk]

@ddelnano That works as a temporary solution, sure. As you know my initial proposal was to remove the statefunc entirely, but this breaks the support for dashed Mac addresses. Maybe implement the PR and remove computed?

[ddelnano]

Yea, apologies for the back and forth on this.

I think maintaining backwards compatibility is important, but before the issue was fully understood I didn't see strong upside for breaking that functionality. However knowing that the mac address format functionality is built on terraform features that are deemed incompatible, I believe considering a breaking change is more reasonable now.

[ravager-dk]

I think the issue https://github.com/hashicorp/terraform/issues/30447 describes is more a general issue moving forward. So statefunc could be used for now, but once the provider needs to be upgraded to the new plugin framework, there will be issues. I suggest maybe an interim solution instead of a breaking change and then transition the provider to the new framework, at which point there are likely going to be other breaking changes anyway.

[ddelnano]

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