Terraform State/Config after import of resources that include Optional Parameters

[shrusubra]

Terraform v1.3.1

• provider registry.terraform.io/hashicorp/oci v4.95.0

Problem Statement: We use modules to create the resources. Which works fine. I have created a bunch of NSG rules using the same code that is giving errors when I try to import the resources. While we import the resources, we find the ICMP and TCP options getting replaced by null as shown above. Can anyone guide us to avoid this please?

Actual Behaviour:

  # module.nsg-rules["NSG1-1_security_rule4"].oci_core_network_security_group_security_rule.nsg_rule will be updated in-place
  ~ resource "oci_core_network_security_group_security_rule" "nsg_rule" {
        id                        = "4F8DCF"
        # (9 unchanged attributes hidden)

      ~ tcp_options {
          - destination_port_range {
              - max = 40 -> null
              - min = 21 -> null
            }
        }

        # (1 unchanged block hidden)
    }

  # module.nsg-rules["NSG1-1_security_rule5"].oci_core_network_security_group_security_rule.nsg_rule will be updated in-place
  ~ resource "oci_core_network_security_group_security_rule" "nsg_rule" {
        id                        = "05F7D7"
        # (9 unchanged attributes hidden)

      ~ tcp_options {

          - source_port_range {
              - max = 12 -> null
              - min = 12 -> null
            }
        }

        # (1 unchanged block hidden)
    }
# module.nsg-rules[“NSG1-1_security_rule12”].oci_core_network_security_group_security_rule.nsg_rule will be updated in-place
 ~ resource “oci_core_network_security_group_security_rule” “nsg_rule” {
    id            = “BC5A21"
    # (9 unchanged attributes hidden)
   - icmp_options {
     - code = -1 -> null
     - type = 2 -> null
    }
    # (1 unchanged block hidden)
  } 

Expected Behaviour: No changes. Your infrastructure matches the configuration.

Vairables file: This issue started appearing only when we use Optional type parameters as below -

variable "nsg_rules" {
  type = map(object({
  nsg_id = string
  direction = string
  protocol = string
  description = optional(string)
  stateless = optional(string)
  source_type = optional(string)
  destination_type = optional(string)
  destination = optional(string)
  source = optional(string)
  options = optional(map(any))
  }))
  default = {}
}

Options have the data related to TCP, UDP , ICMP

Steps to reporduce:

1. Terraform import of the resource
2. Terraform plan

NOTE: I have the configuration files in place with the desired options. I have in fact created these resources before using the same Terraform configuration files, but only when I tried terraform import of the resources after deleting the state file, I started getting this issue. Somewhere the provider is not able to map the data to state file I suppose.

State File Contents

    {
      "module": "module.nsg-rules[\"NSG1-1_security_rule5\"]",
      "mode": "managed",
      "type": "oci_core_network_security_group_security_rule",
      "name": "nsg_rule",
      "provider": "provider[\"registry.terraform.io/oracle/oci\"]",
      "instances": [
        {
          "schema_version": 0,
          "attributes": {
            "description": "tcp source",
            "destination": null,
            "destination_type": "",
            "direction": "INGRESS",
            "icmp_options": [],
            "id": "05F7D7",
            "is_valid": true,
            "network_security_group_id": "ocid1.networksecuritygroup.xxxxxxxxx",
            "protocol": "6",
            "source": "5.0.0.0/8",
            "source_type": "CIDR_BLOCK",
            "stateless": false,
            "tcp_options": [
              {
                "destination_port_range": [],
                "source_port_range": [
                  {
                    "max": 12,
                    "min": 12
                  }
                ]
              }
            ],
            "time_created": "2022-09-30 10:53:11.478 +0000 UTC",
            "timeouts": {
              "create": null,
              "delete": null,
              "update": null
            },
            "udp_options": []
          },
          "sensitive_attributes": [],
          "private": "eyJlMzxxxxxxxxxxxx"
        }
      ]
    },

    {
      "module": "module.nsg-rules[\"NSG1-1_security_rule12\"]",
      "mode": "managed",
      "type": "oci_core_network_security_group_security_rule",
      "name": "nsg_rule",
      "provider": "provider[\"registry.terraform.io/oracle/oci\"]",
      "instances": [
        {
          "schema_version": 0,
          "attributes": {
            "description": "icmp type 2",
            "destination": null,
            "destination_type": "",
            "direction": "INGRESS",
            "icmp_options": [
              {
                "code": -1,
                "type": 2
              }
            ],
            "id": "BC5A21",
            "is_valid": true,
            "network_security_group_id": "ocid1.networksecuritygroup.oc1.phx.xxxxxxxx",
            "protocol": "1",
            "source": "12.0.0.0/8",
            "source_type": "CIDR_BLOCK",
            "stateless": false,
            "tcp_options": [],
            "time_created": "2022-09-30 10:57:53.062 +0000 UTC",
            "timeouts": {
              "create": null,
              "delete": null,
              "update": null
            },
            "udp_options": []
          },
          "sensitive_attributes": [],
          "private": "exxxxxxxxxx"
        }
      ]
    },

main.tf

resource "oci_core_network_security_group_security_rule" "nsg_rule" {

  #Required
  network_security_group_id = var.nsg_id
  direction                 = var.direction
  protocol                  = var.protocol

  #Optional
  description      = var.description
  destination      = var.destination_addr
  destination_type = var.destination_type
  source           = var.source_addr
  source_type      = var.source_type
  stateless        = var.stateless

  dynamic "icmp_options" {
    for_each = var.nsg_rules_details

    content {
      type = icmp_options.value.type
      code = icmp_options.value.code
    }
  }

  dynamic "tcp_options" {
    for_each = try((var.nsg_rules_details[var.key_name].options.tcp != [] ? var.nsg_rules_details[var.key_name].options.tcp : []), [])

    content {
      #Optional
      dynamic "source_port_range" {
        for_each = var.source_port_range

        content {
          #Required
          max = source_port_range.value.source_port_range_max != "" ? source_port_range.value.source_port_range_max : ""

          min = source_port_range.value.source_port_range_min != "" ? source_port_range.value.source_port_range_min : ""
            }
      }

      dynamic "destination_port_range" {
        for_each = var.destination_port_range
        content {
          #Required
          max = destination_port_range.value.destination_port_range_max != "" ? destination_port_range.value.destination_port_range_max : ""

          min = destination_port_range.value.destination_port_range_min != "" ? destination_port_range.value.destination_port_range_min : ""
                      }
                 }

         }
     }
}

[rramge]

Well, at first glance, I didn't see source_port_range anywhere in your variable declarations. So, given the code snippets are complete, the value is null, and thus the max and min values are are changed to null in your infrastructure.

Optional parameters which are not used have always null as default value. You consider setting defaults, and compare null values against null (not initialized) and not an empty string. I personally developed the habit during the 1.3 alpha phase to even set null explicitely as default even though it has no immediate effect, because it makes it easier to understand, especially in comparisons, validation blocks, etc..

[shrusubra]

Hi @rramge ,

I have these in my input tfvars file -

  NSG1-1_security_rule5 =  {
        nsg_id = "NSG1-1"
        direction = "INGRESS"
        protocol = "6"
        description = "tcp source"
        source_type = "CIDR_BLOCK"
        source = "5.0.0.0/8"
        options = {
            tcp = [{
                source_port_range_max = "12"
                source_port_range_min = "12"
            }]
        }
   },
NSG1-1_security_rule12 =  {
        nsg_id = "NSG1-1"
        direction = "INGRESS"
        protocol = "1"
        description = "icmp type 2"
        source_type = "CIDR_BLOCK"
        source = "12.0.0.0/8"
        options = {
            icmp = [{
                    type = "2"
            }]
        }
   },

I have modified my resource to below -

resource "oci_core_network_security_group_security_rule" "nsg_rule" {

  #Required
  network_security_group_id = var.nsg_id
  direction                 = var.direction
  protocol                  = var.protocol

  #Optional
  description      = var.description
  destination      = var.destination_addr
  destination_type = var.destination_type
  source           = var.source_addr
  source_type      = var.source_type
  stateless        = var.stateless

  dynamic "icmp_options" {
    for_each = try((var.nsg_rules_details[var.key_name].options.icmp[0].code != "" && var.nsg_rules_details[var.key_name].options.icmp[0].type != "" ? var.nsg_rules_details[var.key_name].options.icmp : []), [])

    content {
      type = icmp_options.value.type
      code = icmp_options.value.code
    }
  }

  dynamic "tcp_options" {
    for_each = var.nsg_rules_details[var.key_name].options.tcp

    content {
      #Optional
      dynamic "source_port_range" {
        for_each = try((tcp_options.value.source_port_range_max != "" || tcp_options.value.source_port_range_min != "" ? tcp_options.value : []), [])

        content {
          #Required
          max = source_port_range.value.source_port_range_max
          min = source_port_range.value.source_port_range_min
        }
      }

      dynamic "destination_port_range" {
        for_each = try((tcp_options.value.destination_port_range_max != "" || tcp_options.value.destination_port_range_min != "" ? tcp_options.value : []), [])

        content {
          #Required
          max = destination_port_range.value.destination_port_range_max

          min = destination_port_range.value.destination_port_range_min
      }
    }
  }
}

Here is my module -

module "nsg-rules" {
  source     = "./modules/network/nsg-rules"
  for_each   = var.nsg_rules

  #Required
  nsg_id    = each.value.nsg_id
  direction = each.value.direction
  protocol  = each.value.protocol

  #Optional
  description       = each.value.description
  destination_addr  = each.value.destination_addr
  destination_type  = each.value.destination_type
  source_addr       = each.value.source_addr
  source_type       = each.value.source_type
  stateless         = each.value.stateless
  nsg_rules_details = var.nsg_rules
}

It must not create the resources too correct if the data is getting passed as null?. Creation of the resources works perfectly fine. It shows the difference only when I try to import the state.

So what I did was, I created the resources using my code. Which came out correct. Post that I accidentally deleted my state file. So I thought to import the resources to the state, when I did that I started seeing these changes. It was trying to make the ICMP and TCP options as null. This is happening only when I declare and use optional parameters, it does not happen when I pass all the values. (To pass all the values, I do ICMP = [] or TCP = [] in those rules which do not have them).

Could you please help me here? I suspect this to be a bug with the latest release. Coz if there is an issue with the code that I use, it must fail even for creation of the resources and not just the import right?

Thanks in advance. Please help.

[shrusubra]

Hi @rramge /Team,

Can you please help on my query above?

Thanks in advance.

[danielwoodz]

Here is some working code to create a NSG that will not deviate on each apply (Requires TF 1.3.0, tested against 1.3.3)

We went the route of building a module with optionals with separate ingress and egress lists to keep things less complex.

# -- versions.tf 

terraform {
  required_version = ">= 1.3.3"
  required_providers {
    oci = {
      source  = "oracle/oci"
      version = ">= 4.0.0"
    }
  }
}

# -- main.tf --

resource "oci_core_network_security_group" "this" {
  compartment_id = var.compartment_id
  vcn_id         = var.vcn_id

  display_name = var.nsg_display_name

  freeform_tags = var.tags
}

data "oci_core_network_security_group" "this" {
  network_security_group_id = resource.oci_core_network_security_group.this.id

  depends_on = [
    resource.oci_core_network_security_group.this
  ]
}

data "oci_core_network_security_groups" "lookup" {
  compartment_id = var.compartment_id
  vcn_id         = var.vcn_id

  depends_on = [
    resource.oci_core_network_security_group.this
  ]
}

locals {
  nsg_list = data.oci_core_network_security_groups.lookup.network_security_groups
}

resource "oci_core_network_security_group_security_rule" "ingress" {
  count = length(var.nsg_ingress_security_rules)

  network_security_group_id = resource.oci_core_network_security_group.this.id

  direction   = "INGRESS"
  stateless   = var.nsg_ingress_security_rules[count.index].stateless
  protocol    = lookup(var.protocol_lookup, lower(var.nsg_ingress_security_rules[count.index].protocol))
  source      = var.nsg_ingress_security_rules[count.index].source_type == "NETWORK_SECURITY_GROUP" ? [for nsg in local.nsg_list : nsg.id if nsg.display_name == var.nsg_ingress_security_rules[count.index].source][0] : var.nsg_ingress_security_rules[count.index].source
  source_type = var.nsg_ingress_security_rules[count.index].source_type
  description = var.nsg_ingress_security_rules[count.index].description

  dynamic "tcp_options" {
    for_each = contains(["tcp", "tcpv6"], lower(var.nsg_ingress_security_rules[count.index].protocol)) ? [1] : []
    content {
      destination_port_range {
        min = var.nsg_ingress_security_rules[count.index].port == 0 ? 1 : var.nsg_ingress_security_rules[count.index].port
        max = var.nsg_ingress_security_rules[count.index].port == 0 ? 65535 : var.nsg_ingress_security_rules[count.index].port
      }
    }
  }

  dynamic "udp_options" {
    for_each = contains(["udp", "udpv6"], lower(var.nsg_ingress_security_rules[count.index].protocol)) ? [1] : []
    content {
      destination_port_range {
        min = var.nsg_ingress_security_rules[count.index].port == 0 ? 1 : var.nsg_ingress_security_rules[count.index].port
        max = var.nsg_ingress_security_rules[count.index].port == 0 ? 65535 : var.nsg_ingress_security_rules[count.index].port
      }
    }
  }

  dynamic "icmp_options" {
    for_each = contains(["icmp", "icmpv6"], lower(var.nsg_ingress_security_rules[count.index].protocol)) ? [1] : []
    content {
      type = var.nsg_ingress_security_rules[count.index].icmp_type
      code = var.nsg_ingress_security_rules[count.index].icmp_code
    }
  }
}
resource "oci_core_network_security_group_security_rule" "egress" {
  count = length(var.nsg_egress_security_rules)

  network_security_group_id = resource.oci_core_network_security_group.this.id

  direction        = "EGRESS"
  stateless        = var.nsg_egress_security_rules[count.index].stateless
  protocol         = lookup(var.protocol_lookup, lower(var.nsg_egress_security_rules[count.index].protocol))
  destination      = var.nsg_egress_security_rules[count.index].destination_type == "NETWORK_SECURITY_GROUP" ? [for nsg in local.nsg_list : nsg.id if nsg.display_name == var.nsg_egress_security_rules[count.index].destination][0] : var.nsg_egress_security_rules[count.index].destination
  destination_type = var.nsg_egress_security_rules[count.index].destination_type
  description      = var.nsg_egress_security_rules[count.index].description

  dynamic "tcp_options" {
    for_each = contains(["tcp", "tcpv6"], lower(var.nsg_egress_security_rules[count.index].protocol)) ? [1] : []
    content {
      destination_port_range {
        min = var.nsg_egress_security_rules[count.index].port == 0 ? 1 : var.nsg_egress_security_rules[count.index].port
        max = var.nsg_egress_security_rules[count.index].port == 0 ? 65535 : var.nsg_egress_security_rules[count.index].port
      }
    }
  }

  dynamic "udp_options" {
    for_each = contains(["udp", "udpv6"], lower(var.nsg_egress_security_rules[count.index].protocol)) ? [1] : []
    content {
      destination_port_range {
        min = var.nsg_egress_security_rules[count.index].port == 0 ? 1 : var.nsg_egress_security_rules[count.index].port
        max = var.nsg_egress_security_rules[count.index].port == 0 ? 65535 : var.nsg_egress_security_rules[count.index].port
      }
    }
  }

  dynamic "icmp_options" {
    for_each = contains(["icmp", "icmpv6"], lower(var.nsg_egress_security_rules[count.index].protocol)) ? [1] : []
    content {
      type = var.nsg_egress_security_rules[count.index].icmp_type
      code = var.nsg_egress_security_rules[count.index].icmp_code
    }
  }
}

# --- output.tf ---

output "nsg" {
  value = resource.oci_core_network_security_group.this
}

output "nsg_ingress_rules" {
  value = resource.oci_core_network_security_group_security_rule.ingress
}

output "nsg_egress_rules" {
  value = resource.oci_core_network_security_group_security_rule.egress
}

output "nsg_id" {
  value = data.oci_core_network_security_group.this.id
}

output "nsg_lookup" {
  value = data.oci_core_network_security_groups.lookup.network_security_groups
}

variable "tags" {
  description = "A mapping of tags to assign to the resource"
  type        = map(any)
  default     = {}
}

variable "compartment_id" {
  description = "Compartment OCID"
  type        = string
}

variable "vcn_id" {
  description = "VCN OCID"
  type        = string
}

variable "nsg_display_name" {
  description = "NSG display name"
  type        = string
}

variable "nsg_ingress_security_rules" {
  description = "List of rules for allowing ingress traffic"
  type = list(object({
    protocol    = string,                         # tcp, udp, icmp, icmpv6, all
    description = string,                         # Description of the rule
    source      = string,                         # CIDR, CIDRv6, or NSG display name
    source_type = optional(string, "CIDR_BLOCK"), # CIDR_BLOCK, NETWORK_SECURITY_GROUP, SERVICE_CIDR_BLOCK
    port        = optional(number, 0),            # 1-65535, 0 for all
    icmp_type   = optional(number, 0),            # 0-255, 0 for all
    icmp_code   = optional(number, 0),            # 0-255, 0 for all
    stateless   = optional(bool, false),          # true or false
  }))
  default = [
    { protocol = "tcp", port = 22, source = "0.0.0.0/0", description = "SSH Access" },
    # Must have IPv6 Enabled at the VCN Level for IPv6 rules
    # { protocol = "tcp", source = "tfm-nsg-sg-bastion", source_type = "NETWORK_SECURITY_GROUP", description = "Test NSG from Bastion" },
    # { protocol = "tcp", source = "oci-phx-objectstorage", source_type = "SERVICE_CIDR_BLOCK", description = "Test Oracle Service" },
    # { protocol = "udp", source = "1.1.1.1/32", port = 1111, description = "Dynamic V4 UDP 1111" },
    # { protocol = "icmp", source = "0.0.0.0/0", icmp_type = 3, icmp_code = 0, description = "Dynamic V4 ICMP" },
    # { protocol = "icmpv6", source = "::/0", icmp_type = 128, icmp_code = 0, description = "Dynamic V6 ICMP" },
    # { protocol = "udpv6", source = "::1/128", port = 1128, description = "Dynamic V6 UDP 1128" },
    # { protocol = "tcpv6", source = "::2/128", port = 2128, description = "Dynamic V6 UDP 2128" },
  ]
}

variable "nsg_egress_security_rules" {
  description = "List of rules for allowing egress traffic"
  type = list(object({
    protocol         = string,                         # tcp, udp, icmp, icmpv6, all
    description      = string,                         # Description of the rule
    destination      = string,                         # CIDR, CIDRv6, or NSG display name
    destination_type = optional(string, "CIDR_BLOCK"), # CIDR_BLOCK, NETWORK_SECURITY_GROUP, SERVICE_CIDR_BLOCK
    port             = optional(number, 0),            # 1-65535, 0 for all
    icmp_type        = optional(number, 0),            # 0-255, 0 for all
    icmp_code        = optional(number, 0),            # 0-255, 0 for all
    stateless        = optional(bool, false),          # true or false
  }))
  default = [
    { protocol = "all", destination = "0.0.0.0/0", description = "All IPv4 traffic for all ports" },
    # Must have IPv6 Enabled at the VCN Level for IPv6 rules
    # { protocol = "all", destination = "::/0", description = "All IPv6 traffic for all ports" },
  ]
}

variable "protocol_lookup" {
  description = "protocol number lookup"
  default = {
    tcp    = 6  # TCP
    udp    = 17 # UDP
    tcpv6  = 6  # TCP, Doesn't Change for IPv6
    udpv6  = 17 # TCP, Doesn't Change for IPv6
    icmp   = 1  # ICMP
    icmpv6 = 58 # ICMPv6
    all    = "all"
  }
}

[ravinitp]

Thank you for reporting the issue. We observed the affected resources are not provided in the description or it's incorrect. We request you to add it in issue description as mentioned in below format. Example: affected_resources = oci_core_instance , oci_core_instances

If it's not related to any particular resource then mention affected resource as terraform. Example: affected_resources = terraform

As this works through automation, request you to follow exact syntax.