Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Managing address and address group resources - order of operations #186

Closed
zephyia opened this issue Oct 5, 2021 · 16 comments
Closed

Managing address and address group resources - order of operations #186

zephyia opened this issue Oct 5, 2021 · 16 comments

Comments

@zephyia
Copy link

zephyia commented Oct 5, 2021

I am attempting to use this module to manage address and addres group objects and am running into an issue with the order of operations.

I have reproduced a simple test case illustrating the issue below:

$ cat main.tf 

terraform {
  required_providers {
    fortios = {
      source = "fortinetdev/fortios"
    }
  }
}

provider "fortios" {
  hostname = "10.80.16.253"
  token    = "<<REDACTED>>"
  insecure = "true"
  vdom     = "root"
}

locals {
  addresses = {
    TF_TEST_ADDRESS_01 = "10.1.0.0/16"
    TF_TEST_ADDRESS_02 = "10.2.0.0/16"
    TF_TEST_ADDRESS_03 = "10.3.0.0/16"
    TF_TEST_ADDRESS_04 = "10.4.0.0/16"
  }
}


resource "fortios_firewall_address" "tf_addresses" {

  for_each = local.addresses

  name          = each.key
  subnet        = each.value
  type          = "ipmask"
  comment       = "Managed by terraform"
  allow_routing = "disable"
  color         = 3
}

resource "fortios_firewall_addrgrp" "tf_address_grp" {
  name          = "TF_TEST_ADDRESS_GRP"
  allow_routing = "disable"
  color         = 3
  exclude       = "disable"

  dynamic "member" {
    for_each = fortios_firewall_address.tf_addresses

    content {
      name = member.value.name
    }
  }

}
$ terraform1 -version
Terraform v1.0.8
on linux_amd64
+ provider registry.terraform.io/fortinetdev/fortios v1.13.2
$ terraform1 apply

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # fortios_firewall_address.tf_addresses["TF_TEST_ADDRESS_01"] will be created
  + resource "fortios_firewall_address" "tf_addresses" {
      + allow_routing         = "disable"
      + associated_interface  = (known after apply)
      + cache_ttl             = (known after apply)
      + clearpass_spt         = (known after apply)
      + color                 = 3
      + comment               = "Managed by terraform"
      + country               = (known after apply)
      + dynamic_sort_subtable = "false"
      + end_ip                = (known after apply)
      + end_mac               = (known after apply)
      + epg_name              = (known after apply)
      + fqdn                  = (known after apply)
      + id                    = (known after apply)
      + interface             = (known after apply)
      + name                  = "TF_TEST_ADDRESS_01"
      + node_ip_only          = (known after apply)
      + obj_tag               = (known after apply)
      + obj_type              = (known after apply)
      + organization          = (known after apply)
      + policy_group          = (known after apply)
      + sdn                   = (known after apply)
      + sdn_addr_type         = (known after apply)
      + sdn_tag               = (known after apply)
      + start_ip              = (known after apply)
      + start_mac             = (known after apply)
      + sub_type              = (known after apply)
      + subnet                = "10.1.0.0/16"
      + subnet_name           = (known after apply)
      + tenant                = (known after apply)
      + type                  = "ipmask"
      + uuid                  = (known after apply)
      + visibility            = (known after apply)
      + wildcard              = (known after apply)
      + wildcard_fqdn         = (known after apply)
    }

  # fortios_firewall_address.tf_addresses["TF_TEST_ADDRESS_02"] will be created
  + resource "fortios_firewall_address" "tf_addresses" {
      + allow_routing         = "disable"
      + associated_interface  = (known after apply)
      + cache_ttl             = (known after apply)
      + clearpass_spt         = (known after apply)
      + color                 = 3
      + comment               = "Managed by terraform"
      + country               = (known after apply)
      + dynamic_sort_subtable = "false"
      + end_ip                = (known after apply)
      + end_mac               = (known after apply)
      + epg_name              = (known after apply)
      + fqdn                  = (known after apply)
      + id                    = (known after apply)
      + interface             = (known after apply)
      + name                  = "TF_TEST_ADDRESS_02"
      + node_ip_only          = (known after apply)
      + obj_tag               = (known after apply)
      + obj_type              = (known after apply)
      + organization          = (known after apply)
      + policy_group          = (known after apply)
      + sdn                   = (known after apply)
      + sdn_addr_type         = (known after apply)
      + sdn_tag               = (known after apply)
      + start_ip              = (known after apply)
      + start_mac             = (known after apply)
      + sub_type              = (known after apply)
      + subnet                = "10.2.0.0/16"
      + subnet_name           = (known after apply)
      + tenant                = (known after apply)
      + type                  = "ipmask"
      + uuid                  = (known after apply)
      + visibility            = (known after apply)
      + wildcard              = (known after apply)
      + wildcard_fqdn         = (known after apply)
    }

  # fortios_firewall_address.tf_addresses["TF_TEST_ADDRESS_03"] will be created
  + resource "fortios_firewall_address" "tf_addresses" {
      + allow_routing         = "disable"
      + associated_interface  = (known after apply)
      + cache_ttl             = (known after apply)
      + clearpass_spt         = (known after apply)
      + color                 = 3
      + comment               = "Managed by terraform"
      + country               = (known after apply)
      + dynamic_sort_subtable = "false"
      + end_ip                = (known after apply)
      + end_mac               = (known after apply)
      + epg_name              = (known after apply)
      + fqdn                  = (known after apply)
      + id                    = (known after apply)
      + interface             = (known after apply)
      + name                  = "TF_TEST_ADDRESS_03"
      + node_ip_only          = (known after apply)
      + obj_tag               = (known after apply)
      + obj_type              = (known after apply)
      + organization          = (known after apply)
      + policy_group          = (known after apply)
      + sdn                   = (known after apply)
      + sdn_addr_type         = (known after apply)
      + sdn_tag               = (known after apply)
      + start_ip              = (known after apply)
      + start_mac             = (known after apply)
      + sub_type              = (known after apply)
      + subnet                = "10.3.0.0/16"
      + subnet_name           = (known after apply)
      + tenant                = (known after apply)
      + type                  = "ipmask"
      + uuid                  = (known after apply)
      + visibility            = (known after apply)
      + wildcard              = (known after apply)
      + wildcard_fqdn         = (known after apply)
    }

  # fortios_firewall_address.tf_addresses["TF_TEST_ADDRESS_04"] will be created
  + resource "fortios_firewall_address" "tf_addresses" {
      + allow_routing         = "disable"
      + associated_interface  = (known after apply)
      + cache_ttl             = (known after apply)
      + clearpass_spt         = (known after apply)
      + color                 = 3
      + comment               = "Managed by terraform"
      + country               = (known after apply)
      + dynamic_sort_subtable = "false"
      + end_ip                = (known after apply)
      + end_mac               = (known after apply)
      + epg_name              = (known after apply)
      + fqdn                  = (known after apply)
      + id                    = (known after apply)
      + interface             = (known after apply)
      + name                  = "TF_TEST_ADDRESS_04"
      + node_ip_only          = (known after apply)
      + obj_tag               = (known after apply)
      + obj_type              = (known after apply)
      + organization          = (known after apply)
      + policy_group          = (known after apply)
      + sdn                   = (known after apply)
      + sdn_addr_type         = (known after apply)
      + sdn_tag               = (known after apply)
      + start_ip              = (known after apply)
      + start_mac             = (known after apply)
      + sub_type              = (known after apply)
      + subnet                = "10.4.0.0/16"
      + subnet_name           = (known after apply)
      + tenant                = (known after apply)
      + type                  = "ipmask"
      + uuid                  = (known after apply)
      + visibility            = (known after apply)
      + wildcard              = (known after apply)
      + wildcard_fqdn         = (known after apply)
    }

  # fortios_firewall_addrgrp.tf_address_grp will be created
  + resource "fortios_firewall_addrgrp" "tf_address_grp" {
      + allow_routing         = "disable"
      + color                 = 3
      + dynamic_sort_subtable = "false"
      + exclude               = "disable"
      + id                    = (known after apply)
      + name                  = "TF_TEST_ADDRESS_GRP"
      + type                  = (known after apply)
      + uuid                  = (known after apply)
      + visibility            = (known after apply)

      + member {
          + name = "TF_TEST_ADDRESS_01"
        }
      + member {
          + name = "TF_TEST_ADDRESS_02"
        }
      + member {
          + name = "TF_TEST_ADDRESS_03"
        }
      + member {
          + name = "TF_TEST_ADDRESS_04"
        }
    }

Plan: 5 to add, 0 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

fortios_firewall_address.tf_addresses["TF_TEST_ADDRESS_01"]: Creating...
fortios_firewall_address.tf_addresses["TF_TEST_ADDRESS_04"]: Creating...
fortios_firewall_address.tf_addresses["TF_TEST_ADDRESS_02"]: Creating...
fortios_firewall_address.tf_addresses["TF_TEST_ADDRESS_03"]: Creating...
fortios_firewall_address.tf_addresses["TF_TEST_ADDRESS_01"]: Creation complete after 1s [id=TF_TEST_ADDRESS_01]
fortios_firewall_address.tf_addresses["TF_TEST_ADDRESS_04"]: Creation complete after 1s [id=TF_TEST_ADDRESS_04]
fortios_firewall_address.tf_addresses["TF_TEST_ADDRESS_02"]: Creation complete after 1s [id=TF_TEST_ADDRESS_02]
fortios_firewall_address.tf_addresses["TF_TEST_ADDRESS_03"]: Creation complete after 1s [id=TF_TEST_ADDRESS_03]
fortios_firewall_addrgrp.tf_address_grp: Creating...
fortios_firewall_addrgrp.tf_address_grp: Creation complete after 0s [id=TF_TEST_ADDRESS_GRP]

Apply complete! Resources: 5 added, 0 changed, 0 destroyed.

Applying this terraform will work without issue as terraform will create the address objects and then the address group object. However the issue comes when one of the addresses needs to be removed. We can simulate this by commenting out one of the addressed as below:

$ cat main.tf

terraform {
  required_providers {
    fortios = {
      source = "fortinetdev/fortios"
    }
  }
}

provider "fortios" {
  hostname = "10.80.16.253"
  token    = "<<REDCATED>>"
  insecure = "true"
  vdom     = "root"
}

locals {
  addresses = {
    TF_TEST_ADDRESS_01 = "10.1.0.0/16"
    TF_TEST_ADDRESS_02 = "10.2.0.0/16"
#    TF_TEST_ADDRESS_03 = "10.3.0.0/16"
    TF_TEST_ADDRESS_04 = "10.4.0.0/16"
  }
}


resource "fortios_firewall_address" "tf_addresses" {

  for_each = local.addresses

  name          = each.key
  subnet        = each.value
  type          = "ipmask"
  comment       = "Managed by terraform"
  allow_routing = "disable"
  color         = 3
}

resource "fortios_firewall_addrgrp" "tf_address_grp" {
  name          = "TF_TEST_ADDRESS_GRP"
  allow_routing = "disable"
  color         = 3
  exclude       = "disable"

  dynamic "member" {
    for_each = fortios_firewall_address.tf_addresses

    content {
      name = member.value.name
    }
  }

}

When this runs you get the following error:

$ terraform1 apply
fortios_firewall_address.tf_addresses["TF_TEST_ADDRESS_03"]: Refreshing state... [id=TF_TEST_ADDRESS_03]
fortios_firewall_address.tf_addresses["TF_TEST_ADDRESS_04"]: Refreshing state... [id=TF_TEST_ADDRESS_04]
fortios_firewall_address.tf_addresses["TF_TEST_ADDRESS_01"]: Refreshing state... [id=TF_TEST_ADDRESS_01]
fortios_firewall_address.tf_addresses["TF_TEST_ADDRESS_02"]: Refreshing state... [id=TF_TEST_ADDRESS_02]
fortios_firewall_addrgrp.tf_address_grp: Refreshing state... [id=TF_TEST_ADDRESS_GRP]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  ~ update in-place
  - destroy

Terraform will perform the following actions:

  # fortios_firewall_address.tf_addresses["TF_TEST_ADDRESS_03"] will be destroyed
  - resource "fortios_firewall_address" "tf_addresses" {
      - allow_routing         = "disable" -> null
      - cache_ttl             = 0 -> null
      - clearpass_spt         = "unknown" -> null
      - color                 = 3 -> null
      - comment               = "Managed by terraform" -> null
      - dynamic_sort_subtable = "false" -> null
      - end_mac               = "00:00:00:00:00:00" -> null
      - id                    = "TF_TEST_ADDRESS_03" -> null
      - name                  = "TF_TEST_ADDRESS_03" -> null
      - obj_type              = "ip" -> null
      - sdn_addr_type         = "private" -> null
      - start_mac             = "00:00:00:00:00:00" -> null
      - sub_type              = "sdn" -> null
      - subnet                = "10.3.0.0/16" -> null
      - type                  = "ipmask" -> null
      - uuid                  = "bd22f5a4-2577-51ec-35eb-8aaf61975415" -> null
    }

  # fortios_firewall_addrgrp.tf_address_grp will be updated in-place
  ~ resource "fortios_firewall_addrgrp" "tf_address_grp" {
        id                    = "TF_TEST_ADDRESS_GRP"
        name                  = "TF_TEST_ADDRESS_GRP"
        # (6 unchanged attributes hidden)

      ~ member {
          ~ name = "TF_TEST_ADDRESS_03" -> "TF_TEST_ADDRESS_04"
        }
      - member {
          - name = "TF_TEST_ADDRESS_04" -> null
        }
        # (2 unchanged blocks hidden)
    }

Plan: 0 to add, 1 to change, 1 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

fortios_firewall_address.tf_addresses["TF_TEST_ADDRESS_03"]: Destroying... [id=TF_TEST_ADDRESS_03]
╷
│ Error: Error deleting FirewallAddress resource: Internal Server Error - Internal error when processing the request (500)
│
│
╵

This is because terraform is trying to delete the address object, prior to updating the address group to remove the reference to the address and the fortigate wont let you delete an address object that is referenced in policies, address groups etc. No combination of depends_on or lifecycle rules seems to be able to help here. The order of events for destroy needs to be different to create. I havent tested but imagine a similar issue exists if the address objects are refenced by a policy and you try to remove them.

When you are creating the objects the provider needs to create the referenced address objects first and then create the address group object - which it does without issue. But on removing one address object the provider needs to update the address group first then remove the address object. I am not 100% familar with the internals of terraform providers but I am hoping the provider has some control over the order of operations and is able to be updated to deal with this case?

Otherwise can you suggest other ways to make this work - a multipass method requiring multiple runs where the changes are made incrementally will not work in this case as the source of the IP addresses is programatic and this all part of an automated pipeline.
@lix-fortinet
Copy link
Contributor

Hi @zephyia ,

Thank you for raising this issue, and sorry for the late response. This issue is a little complicated. Team is working on this issue. We will reply to you ASAP.

Thanks,
Xing

@lix-fortinet
Copy link
Contributor

lix-fortinet commented Oct 25, 2021 •
edited

Hi @zephyia ,

Terraform has a meta-argument named lifecycle. Set create_before_destroy of lifecycle to true in fortios_firewall_address could solve this issue(Reference: https://www.terraform.io/docs/language/meta-arguments/lifecycle.html). As you said, the reason for this issue is that fortios_firewall_address could not be deleted if it is referenced by other resources. Set create_before_destroy to true will destroy fortios_firewall_address after referenced resources been updated. Could you add lifecycle to fortios_firewall_address and try it again, for example:

resource "fortios_firewall_address" "tf_addresses" {

  for_each = local.addresses

  name          = each.key
  subnet        = each.value
  type          = "ipmask"
  comment       = "Managed by terraform"
  allow_routing = "disable"
  color         = 3
  lifecycle {
    create_before_destroy = true
  }
}

Please let me know if you have any questions.

Thanks,
Xing

@doumhfr
Copy link

doumhfr commented Nov 25, 2021

Hi,

we have the same problem

You create x addresses, member of a group.
when you want to destroy them (group and addresses), it doesn't work, because the provider try to delete first the addresses, and then the group.

I'm not sure the workaround will work.

By default, when Terraform must change a resource argument that cannot be updated in-place due to remote API limitations, Terraform will instead destroy the existing object and then create a new replacement object with the new configured arguments.

we don"t have problem regarding recreating something, we have a problem due to the provider that doesn't respect the order of a delete. I think the problem is on the provider

@doumhfr
Copy link

doumhfr commented Nov 26, 2021

Ok it's working :D

@infradmin
Copy link

I have the same issue: if one of address objects is going to be deleted and corresponding address groups object should be updated, it tries to destroy address first (obviously unsuccessful). BTW, interesting that if I destroy all address objects together with related address group, then the deletion order is correct — first a group and then addresses.
Proposed work-around did not help.

@lix-fortinet
Copy link
Contributor

Hi @infradmin,

Thank you for your comment. We could not handle the process sequence on the provider side. It is controlled by Terraform. The way we solve this issue is using lifecycle with argument create_before_destroy set to true. However, based on our test, it only works on dynamic resources using meta-argument count or for_each. If you are not using locals block and for_each, please try using count. For instance:

resource "fortios_firewall_addrgrp" "tf_address_grp" {
  name          = "TF_TEST_ADDRESS_GRP"
  allow_routing = "disable"
  color         = 3
  exclude       = "disable"

  member {
    name = fortios_firewall_address.tf_addresses1[0].name
  }
  member {
    name = fortios_firewall_address.tf_addresses2[0].name
  }
  
  depends_on = [
    fortios_firewall_address.tf_addresses1,
    fortios_firewall_address.tf_addresses2
  ]
}

resource "fortios_firewall_address" "tf_addresses1" {
  count = 1

  name          = TF_TEST_ADDRESS_01
  subnet        = "10.1.0.0/16"
  type          = "ipmask"
  color         = 3
  lifecycle {
    create_before_destroy = true
  }
}

resource "fortios_firewall_address" "tf_addresses2" {
  count = 1

  name          = TF_TEST_ADDRESS_01
  subnet        = "10.2.0.0/16"
  type          = "ipmask"
  color         = 3
  lifecycle {
    create_before_destroy = true
  }
}

When you want to delete resource fortios_firewall_address.tf_addresses2, you need to set the argument count to 0 and remove the related member in fortios_firewall_addrgrp:

resource "fortios_firewall_addrgrp" "tf_address_grp" {
  name          = "TF_TEST_ADDRESS_GRP"
  allow_routing = "disable"
  color         = 3
  exclude       = "disable"

  member {
    name = fortios_firewall_address.tf_addresses1[0].name
  }
  
  depends_on = [
    fortios_firewall_address.tf_addresses1,
    fortios_firewall_address.tf_addresses2
  ]
}

resource "fortios_firewall_address" "tf_addresses1" {
  count = 1

  name          = TF_TEST_ADDRESS_01
  subnet        = "10.1.0.0/16"
  type          = "ipmask"
  color         = 3
  lifecycle {
    create_before_destroy = true
  }
}

resource "fortios_firewall_address" "tf_addresses2" {
  count = 0

  name          = TF_TEST_ADDRESS_01
  subnet        = "10.2.0.0/16"
  type          = "ipmask"
  color         = 3
  lifecycle {
    create_before_destroy = true
  }
}

Please let me know if it still not work.

Thanks,
Xing

@infradmin
Copy link

Thank you, @lix-fortinet for the fast reply, but actually I am using for_each... Here is my example:

locals {
  subnets = [
    {
      "name"   = "test1",
      "subnet" = "192.168.1.0/24",
      "group"  = "test_group"
    },
    {
      "name"   = "test2",
      "subnet" = "192.168.2.0/24",
      "group"  = "test_group"
    }
  ]
}

resource "fortios_firewall_address" "addr" {
  for_each      = { for i in local.subnets : i.name => i }
  name          = each.key
  subnet        = each.value.subnet
  type          = "ipmask"
  allow_routing = "disable"
  lifecycle {
    create_before_destroy = true
  }
}

resource "fortios_firewall_addrgrp" "addgrp" {
  for_each      = { for i in local.subnets : i.group => i... }
  name          = each.key
  allow_routing = "disable"
  dynamic "member" {
    for_each = each.value
    content {
      name = fortios_firewall_address.addr["${member.value["name"]}"].name
    }
  }
}

As you proposed I added lifecycle meta-argument but it did not help. If I remove "test2" subnet from local and apply again I get:

fortios_firewall_address.addr["test2"]: Destroying... [id=test2]
╷
│ Error: Error deleting FirewallAddress resource: Internal Server Error - Internal error when processing the request (500)

Using Terraform v1.1.3 and provider registry.terraform.io/fortinetdev/fortios v1.13.2.

@infradmin infradmin mentioned this issue Jan 11, 2022
Closed
@zephyia
Copy link
Author

zephyia commented Jan 11, 2022 via email

@lix-fortinet
Copy link
Contributor

Hi @zephyia,

Interesting, it works well in my end using exactly the same environment and configuration as yours.

$ terraform apply --auto-approve
fortios_firewall_address.addr["test2"]: Refreshing state... [id=test2]
fortios_firewall_address.addr["test1"]: Refreshing state... [id=test1]
fortios_firewall_addrgrp.addgrp["test_group"]: Refreshing state... [id=test_group]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  ~ update in-place
  - destroy

Terraform will perform the following actions:

  # fortios_firewall_address.addr["test2"] will be destroyed
  # (because key ["test2"] is not in for_each map)
  - resource "fortios_firewall_address" "addr" {
      - allow_routing         = "disable" -> null
      - cache_ttl             = 0 -> null
      - clearpass_spt         = "unknown" -> null
      - color                 = 0 -> null
      - dynamic_sort_subtable = "false" -> null
      - sdn_addr_type         = "private" -> null
      - sub_type              = "sdn" -> null
      - subnet                = "192.168.2.0/24" -> null
      - type                  = "ipmask" -> null
      - uuid                  = "7bd2f5f6-7318-51ec-0465-bdd5b73ca82c" -> null
    }

  # fortios_firewall_addrgrp.addgrp["test_group"] will be updated in-place
  ~ resource "fortios_firewall_addrgrp" "addgrp" {
        id                    = "test_group"
        name                  = "test_group"
        # (6 unchanged attributes hidden)

      - member {
          - name = "test2" -> null
        }
        # (1 unchanged block hidden)
    }

Plan: 0 to add, 1 to change, 1 to destroy.
fortios_firewall_addrgrp.addgrp["test_group"]: Modifying... [id=test_group]
fortios_firewall_addrgrp.addgrp["test_group"]: Modifications complete after 0s [id=test_group]
fortios_firewall_address.addr["test2"]: Destroying... [id=test2]
fortios_firewall_address.addr["test2"]: Destruction complete after 0s

Apply complete! Resources: 0 added, 1 changed, 1 destroyed.

Could you add argument depends_on in resource fortios_firewall_addrgrp.addgrp and have a try? Like this:

resource "fortios_firewall_addrgrp" "addgrp" {
  for_each = { for i in local.subnets : i.group => i... }
  name = each.key
  allow_routing = "disable"
  dynamic "member" {
    for_each = each.value
    content {
      name = fortios_firewall_address.addr["${member.value["name"]}"].name
    }
  }
  depends_on = [
    fortios_firewall_address.addr
  ]
}

If it still not work, please let me know the following info to help me reproduce in my end:

  1. Your OS, (Linux, Windows, MacOS, docker container, Linux APP under windows, etc.);
  2. FortiOS version;
  3. Does create_before_destroy exist under resource fortios_firewall_address in .tfstate file?
  4. Do you have any other configurations in your .tf file?

Thanks,
Xing

@zephyia
Copy link
Author

zephyia commented Jan 12, 2022 via email

@infradmin
Copy link

Hi @lix-fortinet
I have realized that after adding create_before_destroy to resource block I need to run terraform apply first to add this to the state, and only then try to delete this resource.
Thank you.

@zephyia
Copy link
Author

zephyia commented Feb 4, 2022 •
edited

Hi all,

Just following up on this issue:

With this terraform:

  required_providers {
    fortios = {
      source = "fortinetdev/fortios"
      #    version = "1.13.2"
    }
  }
}

provider "fortios" {
  hostname = "10.80.16.253"
  token    = "xxxxx"
  insecure = "true"
  vdom     = "root"
}

locals {
  clients = {
    client1 = {
      prod =  "10.1.1.1/32"
      office =  "10.1.1.2/32"
      home =  "10.1.1.3/32"
    }
    client2 = {
      main_street = "10.2.2.2/32"
    }
    client3 = {
      ip01 = "10.3.3.1/32",
      ip02 = "10.3.3.2/32",
      ip03 = "10.3.3.3/32",
      ip04 = "10.3.3.4/32"
    }
  }
  clients_flat = flatten([ for k, v in local.clients : [ for k1, v1 in v : {
    client = k
    name = k1
    ip = v1
  } if length(keys(v)) > 0] ])

  clients_with_ips = { for k, v in local.clients : k => v if length(keys(v)) > 0 }
  clients_map = { for c in local.clients_flat : upper(format("%s_%s",c.client,c.name)) => c.ip }
}



resource "fortios_firewall_address" "client_address" {
  for_each = local.clients_map

  name          = each.key
  subnet        = each.value
  type          = "ipmask"
  comment       = "Managed by terraform"
  allow_routing = "disable"
  color         = 3
  lifecycle {
    create_before_destroy = true
  }
}

resource "fortios_firewall_addrgrp" "client_address_grp" {
  for_each = local.clients_with_ips

  name          = upper(each.key)
  allow_routing = "disable"
  color         = 3
  exclude       = "disable"
  dynamic "member" {
    for_each = sort(keys(each.value))
    content {
      name = fortios_firewall_address.client_address[upper(format("%s_%s",each.key,member.value))].name
    }
  }
  depends_on = [
   fortios_firewall_address.client_address
  ]
}

The problem I am still facing is that the provider seems to be incorrectly comparing the order of the members in an address group.

If you use the above to create the 3 address groups, then comment out “ip02” in client3 and apply, everything will work. Then uncomment it to add it back, that will also work – however form now on every time you run an apply (without changing anything) the provider will try to update the address group to change the order of the members as the order of the members in the terraform state doesn’t match the order of the members coming back from the fortigate api.

e.g.:

fortios_firewall_address.client_address["CLIENT3_IP03"]: Refreshing state... [id=CLIENT3_IP03]
fortios_firewall_address.client_address["CLIENT1_PROD"]: Refreshing state... [id=CLIENT1_PROD]
fortios_firewall_address.client_address["CLIENT3_IP02"]: Refreshing state... [id=CLIENT3_IP02]
fortios_firewall_address.client_address["CLIENT1_OFFICE"]: Refreshing state... [id=CLIENT1_OFFICE]
fortios_firewall_address.client_address["CLIENT1_HOME"]: Refreshing state... [id=CLIENT1_HOME]
fortios_firewall_address.client_address["CLIENT3_IP04"]: Refreshing state... [id=CLIENT3_IP04]
fortios_firewall_address.client_address["CLIENT3_IP01"]: Refreshing state... [id=CLIENT3_IP01]
fortios_firewall_address.client_address["CLIENT2_MAIN_STREET"]: Refreshing state... [id=CLIENT2_MAIN_STREET]
fortios_firewall_addrgrp.client_address_grp["client3"]: Refreshing state... [id=CLIENT3]
fortios_firewall_addrgrp.client_address_grp["client2"]: Refreshing state... [id=CLIENT2]
fortios_firewall_addrgrp.client_address_grp["client1"]: Refreshing state... [id=CLIENT1]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # fortios_firewall_addrgrp.client_address_grp["client3"] will be updated in-place
  ~ resource "fortios_firewall_addrgrp" "client_address_grp" {
        id                    = "CLIENT3"
        name                  = "CLIENT3"
        # (6 unchanged attributes hidden)

      ~ member {
          ~ name = "CLIENT3_IP03" -> "CLIENT3_IP02"
        }
      ~ member {
          ~ name = "CLIENT3_IP04" -> "CLIENT3_IP03"
        }
      ~ member {
          ~ name = "CLIENT3_IP02" -> "CLIENT3_IP04"
        }

        # (1 unchanged block hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

The provider needs to be fixed to sort both lists before doing a comparison – can we get that addressed please?

@lix-fortinet
Copy link
Contributor

Hi @zephyia,

Could you try to set argument dynamic_sort_subtable to true and try it again? member will be sort by name when dynamic_sort_subtable set to true.

For your example:

resource "fortios_firewall_addrgrp" "client_address_grp" {
  for_each = local.clients_with_ips

  name          = upper(each.key)
  allow_routing = "disable"
  color         = 3
  exclude       = "disable"
  dynamic "member" {
    for_each = sort(keys(each.value))
    content {
      name = fortios_firewall_address.client_address[upper(format("%s_%s",each.key,member.value))].name
    }
  }
  depends_on = [
   fortios_firewall_address.client_address
  ]
  dynamic_sort_subtable = "true"
}

Please let me know if you have any questions.

Thanks,
Xing

@zephyia
Copy link
Author

zephyia commented Feb 14, 2022

Hi @lix-fortinet,

I have now tested with the property you suggested and can confirm that the issue is not present when this is set. I would suggest that it is probably worth explaining the non-standard use of a resource property like this somewhere prominent.

I think I can close out this issue now - do you agree?

Thanks

c.

@lix-fortinet
Copy link
Contributor

Hi @zephyia,

Thank you for your advice. We will consider it and make improvement in the future release. And of course, you could close this issue if you do not have any other questions.

Thanks,
Xing

@lix-fortinet
Copy link
Contributor

Hi @zephyia,

We will close this issue since it has been fixed. Feel free to open a new issue if you have any other questions.

Thanks,
Xing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@zephyia @doumhfr @lix-fortinet @infradmin