Network ACL Rules modifications cause churn due to being a TypeList

[bhouse-nexthop]

Overview

Inserting a rule in the middle of other rules will cause every rule after to be shown as modified. There is some logic to try to prevent the number of updates shown as being actually applied, but it causes alarm, and its not clear from the output what its actually doing or if its doing it properly.

Its not possible to simply convert the existing TypeList into a TypeSet because without a rule_number specified, the order cannot be preserved (as TypeSet uses a hash of the field). Terraform also doesn't provide any mechanism to do any inspection of the original order to auto-assign rule numbers in the right order for rules without a manually specified rule_number.

The choice was made to create a parallel ruleset schema member with all the same child elements as the rule schema member. The differences are a rule_number becomes a required field and it doesn't need to implement the legacy ports -> port conversion logic.

For the legacy rule TypeList another change was made in order to perform explicit rule_number enumeration instead of relying on Cloudstack to auto-enumerate. When applying new entries in parallel, the order wouldn't be guaranteed to be preserved without this!

This also includes test and documentation updates.

Fixes #279

Usecase

In my particular use-case, I auto-assign rule_numbers based on zones using a module as a helper. The module handles creation of the rule_number for me, and takes a starting index for each block of rules so I can logically group rules together in a "reserved range" for a particular "purpose".

Module Code

modules/cloudstack_network_acl/main.tf:

terraform {
  required_providers {
    cloudstack = {
      source  = "cloudstack/cloudstack"
    }
  }
}

variable "acl_id" {
  description = "ACL ID"
  type        = string
}

variable "managed" {
  description = "Managed"
  type        = bool
  default     = true
}

variable "rulelist" {
  description = "Rule List"
  type        = any
}

resource "cloudstack_network_acl_rule" "this" {
  acl_id             = var.acl_id
  managed            = var.managed
  dynamic "ruleset" {
    for_each = flatten([
        for list in var.rulelist : [
          for rule in list.rules : {
            rule_number  = "${list.start_idx + index(list.rules, rule) + 1}"
            description  = try(rule.description, "")
            action       = rule.action
            cidr_list    = rule.cidr_list
            protocol     = rule.protocol
            icmp_type    = try(rule.icmp_type, null)
            icmp_code    = try(rule.icmp_code, null)
            port         = try(rule.port, null)
            traffic_type = rule.traffic_type
          }
        ]
      ])
    content {
      rule_number  = rule.value.rule_number
      description  = "${rule.value.description}: ${rule.value.action} ${rule.value.traffic_type}"
      action       = rule.value.action
      cidr_list    = rule.value.cidr_list
      protocol     = rule.value.protocol
      icmp_type    = rule.value.icmp_type
      icmp_code    = rule.value.icmp_code
      port         = rule.value.port
      traffic_type = rule.value.traffic_type
    }
  }
}

Module Caller

myzone.tf:

locals {
  subnet_vpc = "10.252.0.0/16"

  aclrules_deny_all = {
    start_idx = 65500
    rules = [
      {
        description  = "deny egress by default"
        rule_number  = 65535
        action       = "deny"
        cidr_list    = [ "0.0.0.0/0" ]
        protocol     = "all"
        icmp_type    = null
        icmp_code    = null
        traffic_type = "egress"
      }
    ]
  }

  subnet_ntp = "10.0.1.0/24"
  aclrules_access_ntp = {
    start_idx = 1100
    rules     = [
      {
        description  = "ntp"
        action       = "allow"
        cidr_list    = [ local.subnet_ntp ]
        protocol     = "udp"
        port         = "123"
        traffic_type = "egress"
      }
    ]
  }

  subnet_ipa          = "10.0.2.0/24"
  aclrules_access_ipa = {
    start_idx = 1200
    rules     = [
      {
        description  = "IPA http"
        action       = "allow"
        cidr_list    = [ local.subnet_ipa ]
        protocol     = "tcp"
        port         = "80"
        traffic_type = "egress"
      },
      {
        description  = "IPA https"
        action       = "allow"
        cidr_list    = [ local.subnet_ipa ]
        protocol     = "tcp"
        port         = "443"
        traffic_type = "egress"
      },
      {
        description  = "IPA ldap"
        action       = "allow"
        cidr_list    = [ local.subnet_ipa ]
        protocol     = "tcp"
        port         = "389"
        traffic_type = "egress"
      },
      {
        description  = "IPA ldaps"
        action       = "allow"
        cidr_list    = [ local.subnet_ipa ]
        protocol     = "tcp"
        port         = "636"
        traffic_type = "egress"
      },
      {
        description  = "kerberos udp"
        action       = "allow"
        cidr_list    = [ local.subnet_ipa ]
        protocol     = "udp"
        port         = "88"
        traffic_type = "egress"
      },
      {
        description  = "kpasswd udp"
        action       = "allow"
        cidr_list    = [ local.subnet_ipa ]
        protocol     = "udp"
        port         = "464"
        traffic_type = "egress"
      },
      {
        description  = "kerberos tcp"
        action       = "allow"
        cidr_list    = [ local.subnet_ipa ]
        protocol     = "tcp"
        port         = "88"
        traffic_type = "egress"
      },
      {
        description  = "kpasswd tcp"
        action       = "allow"
        cidr_list    = [ local.subnet_ipa ]
        protocol     = "tcp"
        port         = "464"
        traffic_type = "egress"
      }
    ]
  }
}

resource "cloudstack_vpc" "infra_vpc" {
  name           = "infra_vpc"
  cidr           = local.subnet_vpc
  vpc_offering   = "VPC HA"
  network_domain = var.cloudstack_network_domain
  zone           = var.cloudstack_zone
  project        = var.cloudstack_project
}

resource "cloudstack_network_acl" "myzone" {
  name   = "myzone"
  vpc_id = cloudstack_vpc.infra_vpc.id
}

module "network_acl_su" {
  source    = "./modules/cloudstack_network_acl"
  acl_id    = cloudstack_network_acl.myzone.id
  managed   = true
  # Order doesn't matter here!
  rulelist  = [ local.aclrules_deny_all, local.aclrules_access_ntp, aclrules_access_ipa ]
}

Explanation of Module usage

This is just describing the example module for how I use this PR. There are other ways to accomplish this especially for short manual lists. The below description is how to use the module in the body of this message, it doesn't directly relate to the PR itself. Infact, it could be used with the prior implementation by using dynamic "rule" instead of dynamic "ruleset" in the module itself (and indeed is how I used to use it, and would show extreme churn when a rule was changed thus necessitating this PR).

The implementation uses a list of lists, with each inner list having a start_idx so each rule does not need to be enumerated separately but instead is ordered naturally. As long as the start_idx isn't the same as any other start_idx, and the gap between each index is sufficiently large, no numbering should conflict. Recommend having each "block" have at least 100 entries reserved. The maximum rule_number is 65535.

In the above example, creating a new inner list to apply will only insert those rules from the inner list as the rule_number's are guaranteed to be unique, regardless of what index it has in the outer list. However, any ordering change of each inner list would impact all rules after it within the inner list (but have no impact on other lists contained in the outer list). So it is recommended to only append new rules to inner lists unless ordering is critical to prevent mass updates (but again, the blast radius would be limited to any inner list items that come after the entry, not to any other outer lists).