terraform-provider-fortios icon indicating copy to clipboard operation
terraform-provider-fortios copied to clipboard

SFTPD-19 When dynamically creating items in TF, they are evaluated by index instead of key

Open blkistsg opened this issue 5 years ago • 1 comments

It looks like dynamic blocks in the fortios provider resources are not properly evaluated by key, and instead by index, or possibly it just isn't properly reading the values on get operations and comparing. This results in dynamic blocks making resources want to apply changes every time even though nothing changed because the ordering of objects (whose order doesn't matter) is different. A good example is the FW policy with dynamic blocks, or the OSPF with the same.

FW Policy:

resource "fortios_firewall_policy" "vpn_mappings" {
lifecycle
{ignore_changes = [policyid]}

# 0 will use the next policy ID, but does not evaluate properly. So, we're ignoring changes after the rule is created
policyid = 0
name = "vpn-to-vpn-any"
comments = "Used to allow tunnel interfaces to communicate with each other for OSPF and other admin traffic."
schedule = "always"
action = "accept"
logtraffic = "utm"

dynamic "srcintf" {
for_each = module.vpn_mappings.local_tunnel_ips
content
{ name = srcintf.key }

}

dynamic "dstintf" {
for_each = module.vpn_mappings.local_tunnel_ips
content
{ name = dstintf.key }

}

srcaddr
{ name = "all" }

dstaddr { name = "all" }

service
{ name = "ALL" }

}

image-2020-09-10-13-56-09-640

OSPF Example:

image-2020-09-10-13-56-31-441

resource "fortios_router_ospf" "global" {
router_id = var.hq_mgmt_ip

area
{ id = "0.0.0.0" }


dynamic "ospf_interface" {
for_each = module.vpn_mappings.local_tunnel_ips

content
{ name = ospf_interface.key interface = ospf_interface.key cost = lookup(local.ospf_cost_overrides, ospf_interface.key, 10) dead_interval = 40 hello_interval = 10 network_type = "point-to-point" mtu = 1420 }

}

dynamic "network" {
for_each = module.vpn_mappings.local_tunnel_ips

content
{ prefix = network.value }

}

log_neighbour_changes = "enable"

redistribute
{ metric = 0 metric_type = "2" name = "connected" status = "disable" tag = 0 }

redistribute
{ metric = 0 metric_type = "2" name = "static" status = "enable" routemap = fortios_router_routemap.static_ospf_redistribution_networks.name tag = 0 }

redistribute
{ metric = 0 metric_type = "2" name = "rip" status = "disable" tag = 0 }

redistribute
{ metric = 0 metric_type = "2" name = "bgp" status = "disable" tag = 0 }

redistribute
{ metric = 0 metric_type = "2" name = "isis" status = "disable" tag = 0 }

}

blkistsg avatar Oct 23 '20 16:10 blkistsg

Hi @blkistsg , the issue is related to "the conflict between the design scheme of Terraform's TypeSet type and the FortiAPI return value scheme". In Terraform, TypeSet implements set behavior and is used to represent an unordered collection of items. However, it does not support indexing by key. By default, it implements diff comparison through the hash values ​​of all child members of a collection item. Terraform will hash all the members of the item to be submitted, and then it will hash all the members of the item returned from the device. The result of these two hash calculations is used to determine whether the item has changed (state of terraform plan). However, FortiAPI always returns all parameters, regardless of whether the parameters are configured in the Terraform configuration. For example, in fortios_webfilter_profile.web, FortiAPI will return a'whitelist' even if the'whitelist' is not configured. In this case, the two hash values ​​calculated by terraform are different, and the terraform plan will output the status Inconsistent information, the user has to fill the value into the TF configuration at this time to make the hash values consistent. In many cases, this will add a lot of work to the user. If a custom hash function is used for calculation (Terraform does not recommend using a custom hash function), in the function, if we try to ignore the calculation of certain item members to avoid the above problem, then when the member configuration changes, terraform will not be able to detect the change of the member. We spent a lot of time digging into the problem. Unfortunately, based on the above reasons, we can only choose TypeList to implement a large number of sub-table features of FortiAPI, so as to minimize the impact on users, that is, the order of the collection's items is sensitive for "terraform plan".

Reference: Terraform's official design plan and discussion about TypeSet index and hash value diff: https://github.com/hashicorp/terraform/pull/2336 https://github.com/hashicorp/terraform/issues/10520

Official documents about FortiAPI: https://fndn.fortinet.net/index.php?/fortiapi/1-fortios/#

Thank you!

frankshen01 avatar Oct 28 '20 08:10 frankshen01

I will go ahead to close this case, if you still have questions, feel free to reopen it or another case.

MaxxLiu22 avatar Jul 07 '23 20:07 MaxxLiu22