fix: Fixed error when custom_origin_config is missed and set as optional in a variables schema

[baburciu]

Description

Allow for custom_origin_config to be missed (e.g. we want the CloudFront distribution to only have an S3 Origin with OAC) but also include custom_origin_config as an optional object in the origin variable definition:

variable "distributions" {
  description = "CloudFront distributions"
  type = map(object({
    origin = map(object({
      custom_origin_config = optional(object({
        http_port              = optional(number)
        https_port             = optional(number)
        origin_protocol_policy = optional(string)
        origin_ssl_protocols   = optional(list(string))
      }))
    }))
  }))
}

With this variables schema, if we then don't provide custom_origin_config in the input, lookup(origin.value, "custom_origin_config", "") function is returning null instead of the empty string that the module expected.

Motivation and Context

We get

╷
│ Error: Invalid function argument
│
│   on .terraform/modules/distribution/main.tf line 94, in resource "aws_cloudfront_distribution" "this":
│   94:         for_each = length(lookup(origin.value, "custom_origin_config", "")) == 0 ? [] : [lookup(origin.value, "custom_origin_config", "")]
│     ├────────────────
│     │ origin.value is object with 4 attributes
│
│ Invalid value for "value" parameter: argument must not be null.

when custom_origin_config is missed in a origin.bar input, if custom_origin_config is defined as optional var.

Breaking Changes

N/A

How Has This Been Tested?

• I have updated at least one of the examples/* to demonstrate and validate my change(s)
• I have tested and validated these changes using one or more of the provided examples/* projects

We use the module like this:

locals {
  distribution_name = split("@", terraform.workspace)[0]
  distribution = var.distributions[terraform.workspace]
}

module "distribution" {
  source = "git::ssh://[email protected]/terraform-aws-modules/terraform-aws-cloudfront.git?ref=v4.1.0"

  aliases                = local.distribution["aliases"]
  comment                = coalesce(local.distribution["comment"], local.distribution_name)
  enabled                = try(local.distribution["enabled"], true)
  is_ipv6_enabled        = try(local.distribution["is_ipv6_enabled"], false)
  retain_on_delete       = try(local.distribution["retain_on_delete"], false)
  wait_for_deployment    = try(local.distribution["wait_for_deployment"], false)
  origin                 = local.distribution["origin"]
  default_cache_behavior = local.distribution["default_cache_behavior"]
  ordered_cache_behavior = try(local.distribution["ordered_cache_behavior"], [])
  viewer_certificate     = local.distribution["viewer_certificate"]

  create_origin_access_control = true
  origin_access_control = {
    s3_oac = {
      description      = "CloudFront access to S3"
      origin_type      = "s3"
      signing_behavior = "always"
      signing_protocol = "sigv4"
    }
  }
}

but we try to enforce a schema for the variables (the origin one in particular here), like

variable "distributions" {
  description = "CloudFront distributions"
  type = map(object({
    aliases             = list(string)
    comment             = optional(string)
    enabled             = optional(bool, true)
    is_ipv6_enabled     = optional(bool, false)
    retain_on_delete    = optional(bool, false)
    wait_for_deployment = optional(bool, false)
    origin = map(object({
      domain_name = string
      custom_origin_config = optional(object({
        http_port              = optional(number)
        https_port             = optional(number)
        origin_protocol_policy = optional(string)
        origin_ssl_protocols   = optional(list(string))
      }))
      origin_access_control = optional(string)
      origin_shield = optional(object({
        enabled              = bool
        origin_shield_region = string
      }))
    }))
    default_cache_behavior = object({
      target_origin_id           = string
      viewer_protocol_policy     = string
      allowed_methods            = list(string)
      cached_methods             = optional(list(string))
      compress                   = bool
      query_string               = optional(bool)
      headers                    = optional(list(string))
      min_ttl                    = optional(number)
      max_ttl                    = optional(number)
      default_ttl                = optional(number)
      cache_policy_name          = optional(string)
      origin_request_policy_name = optional(string)
      use_forwarded_values       = optional(bool)
    })
}

and passing input vars

distributions = {
  "foo@production" = {
    aliases = ["email.foo.org"]
    origin = {
      "track.foo.org" = {
        domain_name = "track.foo.org"
        custom_origin_config = {
          http_port              = 80
          https_port             = 443
          origin_protocol_policy = "https-only"
          origin_ssl_protocols   = ["TLSv1.2"]
        }
      }
    }
    default_cache_behavior = {
      target_origin_id       = "track.foo.org"
      viewer_protocol_policy = "allow-all"

      allowed_methods = ["GET", "HEAD", "OPTIONS", "PUT", "POST", "PATCH", "DELETE"]
      compress        = true
      query_string    = true
      headers         = ["*"]
      min_ttl         = 0
      max_ttl         = 31536000
      default_ttl     = 86400
    }
    viewer_certificate = {
      acm_certificate_arn      = "arn:aws:acm:us-east-1:123456789012:certificate/12345678-abcd-efab-cdef-1234567890"
      ssl_support_method       = "sni-only"
      minimum_protocol_version = "TLSv1.2_2021"
    }
  },
  "bar@production" = {
    aliases = ["bar.ai", "www.bar.ai"],
    origin = {
      "bar.s3.eu-west-1.amazonaws.com" = {
        domain_name           = "bar.s3.eu-west-1.amazonaws.com"
        origin_access_control = "s3_oac" # key in `origin_access_control` module var in main.tf
        origin_shield = {
          enabled              = true
          origin_shield_region = "eu-west-1"
        }
      }
    },
    default_cache_behavior = {
      target_origin_id           = "bar.s3.eu-west-1.amazonaws.com"
      viewer_protocol_policy     = "redirect-to-https"
      allowed_methods            = ["HEAD", "DELETE", "POST", "GET", "OPTIONS", "PUT", "PATCH"]
      cached_methods             = ["HEAD", "GET", "OPTIONS"]
      compress                   = true
      cache_policy_name          = "Managed-CachingOptimized"
      origin_request_policy_name = "Managed-AllViewerExceptHostHeader"
      use_forwarded_values       = false
    },
    viewer_certificate = {
        acm_certificate_arn      = "arn:aws:acm:eu-west-1:123456789012:certificate/12345678-abcd-efab-cdef-1234567890"
        ssl_support_method       = "sni-only"
        minimum_protocol_version = "TLSv1.2_2021"
    }
  }
}

shows the error for bar@production entry (where custom_origin_config is missed).

• I have executed pre-commit run -a on my pull request

[baburciu]

There are more variables that can fall in such a null not handled state, so we can't expect the module to satisfy each. That is why I'm closing this PR.
What we ended up doing was to try sanitize the variables prior to passing them to the module, using mostly merge() to conditionally add a field only when it exists

sanitized_origins = {
  for key, origin in local.distribution["origin"] : key => merge(
    {
      domain_name = origin.domain_name
    },    
    try(origin.custom_origin_config) != null ? { custom_origin_config = origin.custom_origin_config } : {} 
  )
}   

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.