bpg / terraform-provider-proxmox

Terraform Provider for Proxmox
https://registry.terraform.io/providers/bpg/proxmox
Mozilla Public License 2.0
766 stars 125 forks source link

Option to filter IP addresses by interfaces #1134

Closed xoxys closed 5 months ago

xoxys commented 6 months ago

Is your feature request related to a problem? Please describe.

ipv4_addresses and ipv6_addresses contains all IP addresses of the host incl. loopback and all other interfaces. As these lists might be used to create DNS host records it becomes an issue when the host has a lot of other interfaces e.g. for container networks.

Describe the solution you'd like

I would like to have an option to filter ipv4_addresses and ipv6_addresses based on interface patterns.

Describe alternatives you've considered

The current solution is a local module doing the filtering:

locals {
  ipv4_addresses = flatten([
    for server_key, server in var.server : [
      for k, v in coalescelist(proxmox_virtual_environment_vm.server[server.name].ipv4_addresses, []) :
      v if length(regexall("^(lo|docker|veth).*", proxmox_virtual_environment_vm.server[server.name].network_interface_names[k])) == 0
    ]
  ])
}

locals {
  ipv6_addresses = flatten([
    for server_key, server in var.server : [
      for k, v in coalescelist(proxmox_virtual_environment_vm.server[server.name].ipv6_addresses, []) :
      v if length(regexall("^(lo|docker|veth).*", proxmox_virtual_environment_vm.server[server.name].network_interface_names[k])) == 0
    ]
  ])
}
Dr-Shadow commented 5 months ago

This list is actually the same as the one returned in the VM summary on Proxmox VE web interface.

image

I'm not sure if this is ideal to filter out the interfaces in the ressource attributes since this could be relevant information depending on the situation.

xoxys commented 5 months ago

Users that don't want to filter out anything just don't set a filter. Users that don't care about some interfaces should still have the possibility to opt-in by setting a provider config.

ratiborusx commented 5 months ago

Actually was thinking about the same functionality just a few days ago. I'm using outputs to show what IPs my VMs received from DHCP so when i saw a few of them showing gazillion of addresses from docker interfaces i was really sad. Yes, what Proxmox GUI shows is basically enumeration of interfaces which were found by qemu-guest-agent. But i think some kind of filtration should (could?) be implemented on the provider side. Not sure that i want to use a local module for that, but i'll think about adding something like that in outputs. Thanks for an example, @xoxys :)

bpg commented 5 months ago

Adding this type of filtering configuration at the resource level wouldn't be appropriate for the provider. Please keep in mind that the goal of the Terraform/OpenTofu provider is to manage the remote state of the resource in a declarative way. The template defines what state you want the resource to be in, and the provider brings the resource to that defined state by creating or updating it as necessary.

There isn't much room for functionality outside of state management; essentially, everything the provider operates with must be somehow reflected in that resource state. For example, ipv4_addresses is an "output" attribute of the resource, filled by the provider when it reads information back from the VM. When the provider does that, only the remote state is available to it. This means the filter value, like "^(lo|docker|veth).*", must be somehow available in that remote state so it can be read and applied to the values of the ipv4_addresses list. But where to store them on PVE?

Alternative to that would be having the VM interface filters as part of the provider configuration, which is global and available at any time to all resources and datasources. However, it doesn't sound very appealing to me. A resource-specific config leaking out all the way to the global provider settings, it seems more like a hack than a proper fix for the problem.

The solution proposed in the issue description seems like a good option to me. The only improvement I can think of is perhaps implementing some specific filtering functions in the provider to handle those interface lists with less verbosity. However, this is a new API feature, available only in Terraform 1.8+ and not yet supported by OpenToFu. So, it's unlikely to be worked on any time soon.

xoxys commented 5 months ago

Thanks for your detailed reply. Thats fine Ill just keep using my current local approach then.