fix(terragrunt): Variable types

[dnapbak]

Hi, we've experienced some issues when we used the module with the Terragrunt tool.

It seems Terragrunt doesn't manage variable collection types properly. We fixed the issue by setting explicit types of variables.

Examples of error messages on the terrragrunt plan command:

│ Error: Invalid function argument
│
│   on main.tf line 79, in resource "aws_s3_object" "cert_info":
│   79:   for_each = toset(var.cert_info_files)
│     ├────────────────
│     │ while calling toset(v)
│     │ var.cert_info_files is "[\"tls\",\"revoked\",\"revoked-root-ca\"]"
│
│ Invalid value for "v" parameter: cannot convert string to set of any single
│ type.
╵
╷
│ Error: Invalid function argument
│
│   on main.tf line 92, in resource "aws_s3_object" "csrs":
│   92:   for_each = toset(var.csr_files)
│     ├────────────────
│     │ while calling toset(v)
│     │ var.csr_files is "[\"test.csr\"]"
│
│ Invalid value for "v" parameter: cannot convert string to set of any single
│ type.
╵
╷
│ Error: Error in function call
│
│   on modules/terraform-aws-ca-step-function/locals.tf line 2, in locals:
│    2:   template_name_prefix = contains(var.cert_info_files, "tls") ? "ca" : "ca-no-gitops"
│     ├────────────────
│     │ while calling contains(list, value)
│     │ var.cert_info_files is "[\"tls\",\"revoked\",\"revoked-root-ca\"]"
│
│ Call to function "contains" failed: argument must be list, tuple, or set.

The difference in the plan output after adding the proper types to the var.root_ca.info variable:

       ~ environment {
           ~ variables = {
               ~ "ROOT_CA_INFO"        = "\"{\\\"commonName\\\":\\\"XYZ Root CA\\\",\\\"country\\\":\\\"SE\\\",\\\"lifetime\\\":7300,\\\"locality\\\":\\\"Stockholm\\\",\\\"organization\\\":\\\"Example AB\\\",\\\"organizationalUnit\\\":\\\"XYZ\\\",\\\"pathLengthConstraint\\\":1}\"" -> jsonencode(
                     {
                       + commonName           = "XYZ Root CA"
                       + country              = "SE"
                       + lifetime             = "7300"
                       + locality             = "Stockholm"
                       + organization         = "Example AB"
                       + organizationalUnit   = "XYZ"
                       + pathLengthConstraint = "1"
                     }
                 )
                 # (14 unchanged elements hidden)
             }
         }
         # (3 unchanged blocks hidden)
     }

[paulschwarzenberger]

I've tested this against the RSA Public CRL example

A Terraform plan results in the following which is undesirable as we want to see "Nothing to change"

environment {
          ~ variables = {
              ~ "ISSUING_CA_INFO"     = jsonencode(
                  ~ {
                      + emailAddress         = null
                      + state                = null
                        # (7 unchanged attributes hidden)
                    }
                )
                # (14 unchanged elements hidden)
            }
        }

and similarly for ROOT_CA_INFO

The types for ISSUING_CA_INFO and ROOT_CA_INFO are inconsistent

I'm also concerned that if there is an existing CA which someone has defined with a missing state value (for example), a Terraform plan might error, because a null value is not allowed

I'm not sure on the best way to resolve these issues - any ideas?

[dnapbak]

I changed type of root_ca_info and issuing_ca_info variables to map(any). We don't need to set object attributes explicitly.

There is a little backward incompatibility. However it should be fine for python scripts.

      ~ environment {
          ~ variables = {
              ~ "ISSUING_CA_INFO"     = jsonencode(
                  ~ {
                      ~ lifetime             = 3650 -> "3650"
                        # (8 unchanged attributes hidden)
                    }
                )
                # (14 unchanged elements hidden)
            }
        }

[...]

      ~ environment {
          ~ variables = {
              ~ "ROOT_CA_INFO"        = jsonencode(
                  ~ {
                      ~ lifetime             = 7300 -> "7300"
                        # (8 unchanged attributes hidden)
                    }
                )
                # (14 unchanged elements hidden)
            }
        }

WDYT?

[paulschwarzenberger]

I'm not comfortable with any solution where a Terraform plan results in planned changes, bearing in mind that this would require extensive testing, and most likely affect all users of the module. Are the root_ca_info and issuing_ca_info variable types essential to get terragrunt working? Could the issue be fixed by a PR to terragrunt?

[paulschwarzenberger]

If the other options aren't suitable, a possible approach would be to create two new variables terragrunt_root_ca_info and terragrunt_issuing_ca_info with types defined, a description which says e.g. "Terragrunt only - use instead of issuing_ca_info", a default value {} and in main.tf use e.g. coalesce(var.terragrunt_root_ca_info, var.root_ca_info) Rather inelegant and I haven't tested - however I think it should work.

[dnapbak]

I researched Terraform and Terragrunt and in the summary:

• Terraform recommends defining variable types in the documentation, although it’s not mandatory.
• While Terragrunt lacks specific guidance, many GitHub pull requests suggest adding variable types.

I believe using variable types is a best practice. However, it's not the maintainer of the project :) If you want, I can add the terragrunt_root_ca_info and terragrunt_issuing_ca_info variables.

[paulschwarzenberger]

superseded by https://github.com/serverless-ca/terraform-aws-ca/pull/370