Terraform design discussion: inline blocks, empty vs omitted parameters, and modules

[brikis98]

I hope it's OK to file an issue to discuss Terraform's design. If this sort of discussion belongs somewhere else, let me know!

Terraform's support for modules is awesome. However, there are two design choices in Terraform itself that make it very hard to build reusable, configurable modules: inline blocks and empty vs omitted parameters. I discuss each below, what issues they cause for modules, and possible ways to fix them.

1. Inline blocks

Problem

Some resources are configured with a large number of inline blocks, such as aws_elb, which uses an inline block to define each listener:

resource "aws_elb" "example" {
  listener {
    instance_port     = 8000
    instance_protocol = "http"
    lb_port           = 80
    lb_protocol       = "http"
  }

  listener {
    instance_port      = 8000
    instance_protocol  = "http"
    lb_port            = 443
    lb_protocol        = "https"
    ssl_certificate_id = "arn:aws:iam::123456789012:server-certificate/certName"
  }
}

If you create such resources in a module, as far as I know, there is no way to specify a dynamic number of inline blocks based on input variables. For example, if I had a module that took in a var.elb_ports parameter that contained a list of port numbers, I'm not aware of any way to loop over the ports in var.elb_ports and dynamically create a listener for each port.

# Pseudo code that doesn't actually work
resource "aws_elb" "example" {
  for port in var.elb_ports {
    listener {
      # ...
    }
  }
}

As a result, for resources that rely on inline blocks—and there are many, including aws_elb, aws_cloudfront_distribution, aws_s3_bucket, aws_ecs_service, aws_ecs_task_definition, and the tags and ebs_xxx blocks on many other resources such as aws_instance and aws_autoscaling_group—there is no way to create a flexible, general-purpose module.

Possible solutions

One option is to create "companion resources" for all inline blocks. For example, you can add security group rules to an aws_security_group using the aws_security_group_rule "companion resource." Since the latter is a separate resource, you can create them dynamically using the count parameter.

resource "aws_security_group" "example" {
  name = "example"
}

resource "aws_security_group_rule" "allow_inbound" {
  count             = "${length(var.all_inbound_from_security_group_ids)}"
  type              = "ingress"
  from_port         = 0
  to_port           = 65535
  protocol          = "tcp"
  security_group_id = "${element(var.all_inbound_from_security_group_ids, count.index)}"

  security_group_id = "${aws_security_group.example.id}"
}

Another option is to support the count parameter in inline blocks:

resource "aws_elb" "example" {
  listener {
    count = "${length(var.elb_ports)}"
    # ...
  }
}

2. Empty vs omitted parameters

Problem

Terraform is inconsistent with the difference between setting a parameter to an empty value vs omitting the parameter entirely. For example, with aws_instance, you're allowed to set the key_name parameter to an empty string to not associate an EC2 Key Pair with the EC2 Instance. This allows you to create a module that exposes a key_name parameter as a variable with the default value set to an empty string:

variable "key_name" {
  description = "The EC2 Key Pair to associate with the Instance. Leave blank to not associate a Key Pair."
  default     = ""
}

resource "aws_instance" "example" {
  key_name = "${var.key_name}"
}

Now compare that to the aws_db_instance resource. If you omit the kms_key_id parameter, everything works fine:

variable "storage_encrypted" {
  description = "Specifies whether the DB instance is encrypted."
  default     = false
}

# Works fine
resource "aws_db_instance" "default" {
  storage_encrypted = "${var.storage_encrypted}"
}

However, if you try to make the kms_key_id a configurable parameter in your module, and set its default to an empty string, you get the error "To enable encryption at rest StorageEncrypted must be set to 'true'":

variable "storage_encrypted" {
  description = "Specifies whether the DB instance is encrypted."
  default     = false
}

variable "kms_key_id" {
  description = "The ARN for the KMS encryption key. Only used if storage_encrypted is true."
  default     = ""
}

# This will NOT work
resource "aws_db_instance" "default" {
  storage_encrypted = "${var.storage_encrypted}"
  kms_key_id        = "${var.kms_key_id}"
}

There are many other examples where a resource behaves differently for an empty parameter versus one that is omitted entirely. This makes it very hard to build reusable, configurable modules with those resources.

Possible solutions

One option is to try to update all resources to treat an empty string the same as omitting the parameter entirely. This may be tricky to do because, in many cases, it's not Terraform, but the underlying provider (e.g. AWS) that treats them differently.

Another option is to support the idea of a null or None value in Terraform. When a parameter is set to this value, it is exactly the same as omitting the parameter entirely.

variable "storage_encrypted" {
  description = "Specifies whether the DB instance is encrypted."
  default     = false
}

variable "kms_key_id" {
  description = "The ARN for the KMS encryption key. Only used if storage_encrypted is true."
  default     = null
}

# It would be nice if this worked
resource "aws_db_instance" "default" {
  storage_encrypted = "${var.storage_encrypted}"
  kms_key_id        = "${var.kms_key_id}"
}

[ju2wheels]

+1 on parameter omitting, and ill raise with another example:

In the name of trying to make reusable components, what if we wanted to provide all of a resources parameters via an object, thereby allowing us to just loop over a list of objects:

iam_users = [
  {
     name = "username"
     force_destroy = false
     path = "/"
  },
  {
    name = "user2"
  }
]

We could then reuse the resource:

resource "aws_iam_user" "user" {
  count = "${length(var.iam_users)}"
  force_destroy = "${ < iam_user.force_destroy> or omit}"
  name = "${ < iam_user.name>}"
  path  = "${ < iam_user.path> or omit}"
}

For this simple example, we dont have any major issues with omitting values, but if any of the resource's parameters expects an object with required values, then we cannot simply pass in an empty map and have it omit that value but still be able to specify it for other instances in a loop. What would be ideal is if we had something similar to Ansible's default(omit) so that the parameter would just use its default value.

[apparentlymart]

Thanks for this thoughtful discussion and analysis, @brikis98!

We are certainly aware of these and other shortcomings in the configuration language right now. It is a long-term goal to add language features that make reusable modules more practical to create, and you've touched on two of the big things here.

We've been concerned that iteratively adding features to the language over time would cause it to grow into a bit of a "Frankenstein's Monster", so we've instead been quietly collecting use-cases for a while now with the intent of informing a more holistic revamp of the language at some point in the future, where we can hopefully find the minimal set of features to serve the most important use-cases without excessively complicating the configuration language.

The use-cases and potential solutions you've shared here are a great addition to that collection! I can't promise immediate action on this front but I appreciate you taking the time to document this and look forward to discussing a more concrete language change proposal with the community in the future.

[brikis98]

@apparentlymart Got it. Glad to hear this is on your radar. Keep us posted :)

[robinbowes]

I'm currently stumped by this deficiency.

I want to create a re-usable aws elastic beanstalk environments, with the list of settings defined by variable(s) passed into the module, eg:

resource "aws_elastic_beanstalk_environment" "eb_env" {
  name                = "${var.prefix}-${var.app_name}-${var.environment}"
  application         = "${aws_elastic_beanstalk_application.eb_app.name}"
  cname_prefix        = "${var.prefix}-${var.app_name}-${var.environment}"
  solution_stack_name = "64bit Amazon Linux 2017.03 v2.4.2 running PHP 7.0"

  setting {
    namespace = "aws:elasticbeanstalk:application:environment"
    name      = "TEST_PROPERTY"
    value     = "foo"
  }

  setting {
    namespace = "aws:elasticbeanstalk:application:environment"
    name      = "ANOTHER_PROPERTY"
    value     = "BAR"
  }
}

where the number of inline settings blocks will be different for each environment.

I'd really like to see a solution some time soon.

[ju2wheels]

@robinbowes although its not clearly documented as an example, settings that take a map (especially those that can be specified more than once for any given resource) are usually just implemented as lists internally.

I havent tried it, but this may be possible with this resource:

resource "aws_elastic_beanstalk_environment" "eb_env" {
  name                = "${var.prefix}-${var.app_name}-${var.environment}"
  application         = "${aws_elastic_beanstalk_application.eb_app.name}"
  cname_prefix        = "${var.prefix}-${var.app_name}-${var.environment}"
  solution_stack_name = "64bit Amazon Linux 2017.03 v2.4.2 running PHP 7.0"

  setting = [ {
    namespace = "aws:elasticbeanstalk:application:environment"
    name      = "TEST_PROPERTY"
    value     = "foo"
  },
  {
    namespace = "aws:elasticbeanstalk:application:environment"
    name      = "ANOTHER_PROPERTY"
    value     = "BAR"
  }]
}

If that does work you may just be able to pass in the list via tfvars?

[robinbowes]

@ju2wheels Thanks - I'll give it a whirl tomorrow and report back.

Presumably, if it works, I can add a list of "default" settings and join the user-supplied list to create the value passed to "settings" = [...] ?

[ju2wheels]

Presumably, if it works, I can add a list of "default" settings and join the user-supplied list to create the value passed to "settings" = [...] ?

Im pretty sure I tried this and am confident it wont work. The way TF handles creating objects dynamically and trying to merge it will most likely not work out unfortunately.

[robinbowes]

Related question: is there any way to define variables without making them settable outside the module?

eg. (pseudo code):

$fixed_list = [ { "foo" = "bar" } ]
variable "user_list" {
  default = []
}
resource "something" "foo" {
  setting = ["join(',', $fixed_list, var.user_list"]
}

[ju2wheels]

Theres an open issue about local variables coming in a future version.

[davidquarles]

@apparentlymart First off (and as somewhat of a disclaimer), thank you for making such an awesome, elegant piece of software. I'm fully aware that I've chosen to use a pre-1.0 version of an open source tool that solves a very intricate problem, and I don't want to sound entitled / unappreciative of the amount of energy you've invested nor dismissive of just how great the current iteration is. Your concerns are warranted and pragmatic, and I know you have your own upstream constraints to deal with. I'll do my best to keep my criticism constructive, and you can take what I'm about to say with a grain of salt.

That said, I think the issues the OP is raising are very large deficiencies on the UX front. They both currently have me banging my head against a wall, and I've tried things like direct map interpolation, complex attribute injection via template_file, etc. Two other notable, related deficiencies are the lack of support for interpolating and iterating over nested non-primitive types, which I've seen mentioned elsewhere. I'm still not sure whether it makes sense / is possible to use modules in these scenarios, or if lengthy, static configurations are the only way to go. The latter seems like another anti-pattern, and is going to be a good bit more error-prone.

It seems worth noting that the lack of said features has each of us building our own private monsters by wrapping and templating terraform to achieve things that can't be accomplished natively today, which IMHO is an anti-pattern. Doing so obfuscates the declarative intent, adds unnecessary complexity, makes terraform upgrades scarier (or impossible, depending on how you've designed your Frankenstein) and may preclude the ability to safely use these features when they do land without a complete rewrite. I'm trying to unwind such a system today, which heavily templates a single, large AWS deployment, so that we can build analogous configurations for other cloud providers. "Frankenstein monster" is an apt description.

Hopefully that was semi-constructive in and of itself, since you're keeping a running tally of pain points, but I'll stop belaboring points you've undoubtedly thought longer and harder about than I. More constructively, what can be done today? Would it make sense to transparently document the existing limitations, existing workarounds (however hacky, if they exist) and (where design has solidified and timelines aren't nebulous) what the roadmap looks like? I'm still not entirely sure what is / isn't possible, despite my now-intimate familiarity with docs. Perhaps we could document what patterns / practices make sense to you as designers in these scenarios, so that we don't deviate too far from the path forward? Are there stop-gap solutions? Where no workarounds exist, is any of this (temporarily or not) pluggable without totally corrupting the product? What can do to help (besides generating lengthy diatribes in github issues)?

[apparentlymart]

Hi all!

I'm sorry I lost track of this issue when posting updates elsewhere. Configuration language concerns are now covered in a few different issues so I've been posting updates in e.g. #7034, etc but neglected to update here.

Work is in progress right now on integrating the so-called "HCL2" (will later just be "HCL", once things stablize) that is the main outcome of the design process I alluded to above. Along with changing the configuration language itself, we also need to change many details about Terraform's internal model to support some of these new ideas, such as null values (differentiated from empty values).

Because this is a big and invasive series of changes, with compatibility implications along the way, we're approaching this cautiously so we can gather feedback and then ensure there's a migration path through some of the small differences that the new language entails. Some of this work is already in master, but disabled at build time because it's not yet complete/usable. (As of this writing, turning it on will cause Terraform to fail to process most non-trivial configurations.) An opt-in preview version will be coming in the near future so that people can try out a subset of the new features (things we can do without large refactoring of Terraform core, which will follow in separate steps later) in non-production environments to help weed out any unknown compatibility problems and other usability issues.

As others have noticed, some coincidences in the current implementation (exploiting some quirks of how list and map support was retrofitted into a Terraform Core that originally dealt with strings) mean that you can, with some experimentation, trick Terraform into doing some dynamic behavior. These are not documented because they are pretty temperamental (things have to line up just right for it to work) and, indeed, will almost definitely be broken by the forthcoming changes that will make complex data types first class in Terraform core, since the current coincidences will no longer hold with the new, more-robust type system. While the new language version is intended to be broadly compatible with the current, we've focused on remaining compatible with idiomatic usage as shown in documentation, rather than with all of the implementation details.

---

Focusing on the two concerns raised in this issue:

The first round of changes does not yet include support for dynamically-generating nested blocks but there is a plan for that which the new language has the building blocks to support and it will follow in a subsequent update. You can see in the discussion of #7034 the sketches of what this might look like, though the details may evolve as we learn more during the initial implementation of HCL2.

"HCL2" also has native support for null values, which are intended to be equivalent to omitting an attribute when used in that context. In the long run this should allow an approach similar to what @brikis98 suggested in the opening comment on this issue, though again we can't get there all in one step because Terraform's internal model of variables does not have any way to distinguish null from empty, and so we need to propagate this idea through all of Terraform's features before it will be generally useful.

---

I want to make it clear that these various concerns are not being ignored. Before I joined the Terraform team I wrote my fair share of moderately-complex Terraform configurations in my previous role and so I totally understand the frustrations you are all feeling. The reason this isn't out yet is because it's a complex set of work with lots of inter-dependencies and risk, and so it would be irresponsible to try to do it all in one step, particularly with other Terraform Core development going on concurrently.

The opt-in preview version should be released soon. As noted above, this first pass won't actually address the two specific issues identified here because they are "deeper" problems that we'll need to address in a later change, but it will introduce some expression-level features (as opposed to "structural" features, which these two are) that will make certain kinds of dynamic configuration easier to express, and give us the building blocks we need to permeate these concepts throughout Terraform Core.

I'll try to remember to post updates here as work continues. Let's try to keep this particular issue focused on the two concerns @brikis98 originally raised, since there's a number of other issues for other configuration language changes and so it's better to keep these discussions focused so that people can subscribe to the specific ones they are interested in.

[davidquarles]

@apparentlymart Semi-off-topic / no response necessary, but thank you so much for the transparent, informative response. I hope my comment was, in fact, constructive. As I dig through various issues and understand Terraform better, it's becoming increasingly obvious that the team is carefully weighing tradeoffs and taking calculated steps in the right direction (while context-switching to listen to and manage the expectations of nagging users, such as myself).

There are a handful of current limitations (these two top that list) that I don't see as being solvable in userspace today without templating Terraform, but I'll start working at reusable wrappers for native workarounds, where possible, and keep an eye on HCL2.

[ghost]

Add me to the list of people creating frankenstein workarounds throughout their terraform config in order to workaround this issue. In my case, I'm using the OP's library of modules, so at least my frankenstein workaround tends to be identical to his, and fixes to one will likely fix the other. But I've been building out an AWS infrastructure for a month or more now, via terraform/terragrunt and it is kind of shocking just how much of that rather lengthy development time has been devoted to working around this issue in various places - either unwinding bugs in an existing frankenstein solution, creating pull requests for the OP to integrate my fixes, or discovering new places where a frankenstein solution is needed.

This was not, at all, aided by the addition of a backwards-incompatible change in Terraform 0.11 which breaks the most common frankenstein solution for this issue - conditionally defining multiple resources but setting count = 0 on all resources that shouldn't be generated because they have whole blocks that shouldn't be defined. If I had to estimate, I would guess that the inability to conditionally include a config block has been maybe 30% of my total development time, and 20% more has been spent working around the inability to do useful things with compound data structures in HCL (list of maps, etc). These issues tend to result in really verbose workarounds, too, which simply adds to the quantity of code/config that must be written/tested/maintained. In many cases, every single resource in a template/module must be defined twice. And god forbid you want to combine 2 or more features that each requires its own config block, you suddenly have to define 2^n resources for each resource you want to actually create, and ensure you get all of them correctly defined without unintended cut and paste errors. The test/debug cycle suddenly gets extraordinarily time consuming, even once you've spent a couple of hours cutting and pasting things into a frankenstein workaround.

So far, I have ended up forking modules and editing them for my own uses when needing to deal with 2 different conditional blocks in the same resource, rather than trying to create 2^2 or 2^3 resources everywhere in the module. The complexity is just too much, so I'd rather have to worry about manually pulling updates into my fork than to try to define a generally useful module that could be re-integrated to the repo it was forked from. I may regret that decision when it comes time to pull in all of the changes required to accommodate accessing resource attributes when count is explicit but equal to 1. I wasn't anticipating the imminent arrival of a significant rewrite of every module I use in order to accommodate that change when I first decided to permanently fork a few modules that seemed mostly mature and stable.

[brikis98]

I'm not sure how I missed @ju2wheels's suggestion, but I just spotted it somewhat at random today, and it actually works great as a workaround for dynamic inline blocks!

For example, the aws_elb only allows you to define listeners as inline blocks:

resource "aws_elb" "bar" {
   name               = "foobar-terraform-elb"
   availability_zones = ["${data.aws_availability_zones.available.names}"]
 
   listener {
    instance_port     = 8000
    instance_protocol = "http"
    lb_port           = 80
    lb_protocol       = "http"
  }

  listener {
    instance_port     = 8000
    instance_protocol = "https"
    lb_port           = 443
    lb_protocol       = "https"
  }
 }

But it turns out you can also define listeners using an input variable which contains a list of maps!

resource "aws_elb" "bar" {
   name               = "foobar-terraform-elb"
   availability_zones = ["${data.aws_availability_zones.available.names}"]

   listener = "${var.listeners}"
}

variable "listeners" {
   type = "list"
   default = [
     {
       instance_port     = 8000
       instance_protocol = "http"
       lb_port           = 80
       lb_protocol       = "http"
     },
     {
        instance_port     = 8000
        instance_protocol = "https"
        lb_port           = 443
        lb_protocol       = "https"
     }
   ]
 }

@apparentlymart This seems like a reasonable workaround to me. Should it be documented somewhere?

[ju2wheels]

@brikis98 it was undocumented when I last used Terraform around 6 months ago and was something I used successfully quite frequently. Although, I did find a few resources where I think the internal representation or type checking was wrong for which I meant to open issues but never got around to.

For example. I have some notes from old attempts:

* aws_alb.confluence: access_logs: should be a list
* aws_alb_target_group.confluence_nginx: health_check: should be a list
* aws_alb_target_group.confluence_nginx: stickiness: should be a list
* aws_alb_target_group.confluence_primary: health_check: should be a list
* aws_alb_target_group.confluence_primary: stickiness: should be a list
* aws_alb_target_group.confluence_synchrony: health_check: should be a list
* aws_alb_target_group.confluence_synchrony: stickiness: should be a list

I think these resource properties should have allowed a map variable to be used but instead wanted a list of a single map which made it confusing, but the concept still worked.

The only time I had issues with this approach was trying to construct the input value on the fly by merging an inline static map with a variable map IIRC or something like that. I think I had a discussion with @apparentlymart about it on gitter so the history might be there depending on how far back it goes.

[apparentlymart]

Treating a nested block like an attribute and assigning a list of maps to it is in the class of things I was describing above as "coincidences in the current implementation": it happens that Terraform uses a similar representation in memory for lists of maps as it does for nested blocks, and so if you're very careful you can exploit some holes in Terraform's validation and get this to work.

However, there are lots of quirks and inconsistencies here because this is not something that is intentionally supported. The issue @ju2wheels describes is a big one: Terraform expects to be able to do structural validation of nested blocks, but this fails when the internal representation doesn't align with our expectations for how nested blocks behave. In this case, the list of maps is <computed>, which fails because a "computed block" is not a concept in Terraform and thus this breaks the coincidence that makes it sort-of-work when the block is well-known.

This "trick" will be broken by the new language parser, because its internal representation of blocks and attributes is now physically distinct and the validator no longer has the hole that makes the workaround seem to work in some cases. This is why the pattern you saw there is intentionally not documented.

You can see in #7034 the latest sketches of what this might look like when implemented "for real". It's built around a nested block because that ensures that the structure will behave in a "block-like" way, allowing Terraform to validate the contents statically, etc. I'm going to keep posting updates in that issue as things develop; we've done all the prototyping we can do until we get the new language parser properly integrated into Terraform Core, so we're going to keep our focus on that for the moment and then finalize the dynamic block design once we can try it in some real-world scenarios.

[brikis98]

@apparentlymart Thanks for the update. Will the new for_each work be available at the same time as the "trick" gets broken? Or will the "trick" be broken first? Because we have many, many use cases that are waiting on dynamic inline blocks, and it would be great if we could use the "trick" to implement them for now, and then switch our modules internally to the for_each implementation (once it's available) in a backwards compatible manner.

[apparentlymart]

I know that there are several users out there exploiting this implementation detail, and indeed some of them are in verified modules on the Terraform Registry, so we're planning to release dynamic blocks at the same time as the new language implementation to ensure that there's a migration path for those existing uses. The details of this are still somewhat in flux, but we intend to release a preview version some time ahead of the actual release so that module developers can see how the new parser behaves, identify and report things that don't have a good migration path (which we'll then address), and generally have some time to prepare for the changes.

We're at the tail end of some design/planning discussion so implementation should pick up steam again in the very near future. Once we have an initially-workable implementation we'll firm up the release plan.

[jessecollier]

Is there a bounty program that I can donate for this feature?

I currently have a love/hate relationship with terraform, and fixing this issue would make this a true love story.

[Jamie-BitFlight]

@jessecollier well.. you don't have to donate.. https://www.hashicorp.com/blog/terraform-0-1-2-preview

[apparentlymart]

Hi again! Sorry for the long silence here.

Since this issue actually covers two different things, I'm first going to note that the first issue was the same as #7034 and that is now closed after verification of the change with v0.12.0-alpha1 🎉 . With that said, I'm going to focus this reply on the second request about conditionally-unset values.

v0.12.0-alpha1 also includes support for the special value null which, when assigned to a resource argument, is treated the same as having no definition present at all. This means that the following example from the opening comment on this issue should now work as intended (adjusted for the new first-class expression syntax):

variable "storage_encrypted" {
  description = "Specifies whether the DB instance is encrypted."
  default     = false
}

variable "kms_key_id" {
  description = "The ARN for the KMS encryption key. Only used if storage_encrypted is true."
  default     = null
}

# It would be nice if this worked
resource "aws_db_instance" "default" {
  storage_encrypted = var.storage_encrypted
  kms_key_id        = var.kms_key_id
}

Another way to use null is for conditional assignment of a value:

  # assuming here that ams_kms.key.main.id has:
  #     count = var.use_kms ? 1 : 0
  kms_key_id = var.use_kms ? aws_kms_key.main[0].id : null

From a provider's perspective, an omitted value and a null value are indistinguishable; Terraform Core sends the same value (or lack thereof) to the provider either way. For this reason, setting a required argument to null is an error just as omitting it would be, while setting an optional argument to null causes the provider to use its default value instead.

With that said, I think we've now covered both of the ideas mentioned in this issue, and so I'm going to close it. Thanks for suggesting these, and thanks for the patience while we did the groundwork to make them possible.

[sshishov]

Hello @apparentlymart , can the fix for null values be backported to 0.11.x versions? Is it difficult to achieve or you do not want to support "slightly" outdated versions? Just for us, it is a little bit tricky to migrate to 0.12.x version currently.

[ghost]

I'm going to lock this issue 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 similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.