terraform-coop / terraform-provider-foreman

Terraform provider for Foreman
https://registry.terraform.io/providers/terraform-coop/foreman
Mozilla Public License 2.0
33 stars 31 forks source link

Interfaces are not idempotent #31

Closed Fodoj closed 3 years ago

Fodoj commented 3 years ago

While working on making interfaces get imported with host, I found that currently interfaces are broken in many ways.

Here https://github.com/HanseMerkur/terraform-provider-foreman/blob/master/foreman/resource_foreman_host.go#L663 the whole thing silently fails, because the Set is never correctly populated, due to "name" key being defined inside this function, but Name attribute does not even exist for interfaces (see https://github.com/HanseMerkur/terraform-provider-foreman/blob/master/foreman/resource_foreman_host.go#L229).

If name is added, then every time you run plan, Terraform will think that you have a brand new set of interfaces, and will try to re-create the host - that's not ideal.

The reason for this is that TypeSet is used for interfaces - https://github.com/HanseMerkur/terraform-provider-foreman/blob/master/foreman/resource_foreman_host.go#L212; TypeSet basically takes a hash of all attributes, and if one changes, then it considers it a new object. I checked in couple of other providers, like vSphere one, and normally in such occasions TypeList is used instead. My first attempts to convert it to TypeList had certain success, but I am not yet exactly there. So, posting this issue/bug report for the time being.

Also, probably worth refactoring whole interfaces part into a proper sub resource anyway, currently it looks a bit messy.

lhw commented 3 years ago

I am aware of the issue and I did play around with changing the Code from wayfair last year or so. I think I might still have some of the changes on a branch somewhere. But I do remember that there were definitely some issues with the TypeList as well which I tried implementing at the time. I just can't remember what they were or why I stopped fiddling with the issue at the time.