Inline values being used instead of templates

[kerscher]

Inline values prevent templates from being checked as individual files externally, but this is only a nuisance. The real danger is that forgetting to close an inline delimiter changes semantics of the entire .tf file. Check the example below:

module "a" {
  source = "some-path"
  param = <<EOF
bar
EOT
}

module "b" {
  source = "some-path"
  param = <<EOF
baz
EOF
}

The following code above will terraform apply correctly, but module.a.param will not be bar, and module.b will never be created. In modules with nested resources or a large amount of modules this can easily pass unnoticed — specially common since modules take no count.

A counter-argument is that syntax highlight could potentially showcase this problem. But this cannot be checked without human intervention, as a checker cannot know if module.b should or not be created.

The following modules are currently using inline values and should be changed to use templates with external files instead:

examples/nexus-asg/nexus.tf
58:  init_prefix = <<END_INIT
64:  init_suffix = <<END_INIT

examples/cloud-dev-workspace/main.tf
93:  user_data = <<END_INIT

examples/vpc-scenario-1/main.tf
91:  user_data = <<END_INIT

examples/kube-stack-private/bastion.tf
50:  user_data = <<END_INIT

examples/kube-stack-public/bastion.tf
47:  user_data = <<END_INIT

examples/gitlab-asg/main.tf
158:  init_prefix = <<END_INIT
164:  init_suffix = <<END_INIT

examples/legacy/main.tf
43:    extra_init = <<EOF
139:    extra_init = <<EOF

examples/vpc-scenario-2-nat-instances-per-az/main.tf
245:  user_data = <<END_INIT

examples/vpc-scenario-2/main.tf
149:  user_data = <<END_INIT

modules/elk-stack/main.tf
141:  extra_config = <<END_CONFIG

modules/init-snippet-gitlab-docker/main.tf
77:  template = <<EOC
91:  template = <<END_INIT

modules/snapshot-bucket/main.tf
22:  policy = <<EOF

examples/vpc-scenario-2-nat-instance/main.tf
241:  user_data = <<END_INIT

modules/dnsmasq-server/main.tf
21: *       template = <<END_CONF
53: *       user_data = <<END_INIT

modules/kibana/variables.tf
62:  description = <<DOC

modules/kube-stack/main.tf
70:  user_data = <<END_INIT
108:  user_data = <<END_INIT

modules/kibana/main.tf
128:  user_data = <<USER_DATA

modules/persistent-ebs-volumes/iam.tf
9:  policy = <<END_POLICY

modules/persistent-ebs/iam.tf
11:  assume_role_policy = <<END_POLICY
33:  policy = <<END_POLICY

modules/scale-asg-iam-policy/main.tf
13:  policy = <<EOF

modules/elasticsearch/data-nodes.tf
102:    extra_setup_snippet = <<EXTRA_SETUP
133:    extra_config       = <<EXTRA_CONFIG

modules/elasticsearch/variables.tf
133:  description = <<DOC
158:  description = <<DOC

modules/elasticsearch/iam.tf
39:  assume_role_policy = <<END_POLICY
60:  assume_role_policy = <<END_POLICY
80:  policy = <<END_POLICY

modules/single-node-asg/main.tf
52:  user_data = <<END_INIT

modules/logstash/iam.tf
44:  assume_role_policy = <<END_POLICY

modules/consul-demo-server/main.tf
32:  init_suffix = <<END_INIT

modules/elasticsearch/master-nodes.tf
103:    extra_setup_snippet = <<EXTRA_SETUP
131:    extra_config = <<EXTRA_CONFIG

modules/prometheus-server/main.tf
44:  user_data = <<END_INIT
68:  init_prefix = <<END_INIT
75:  prometheus_pillar = <<END_PILLAR

modules/ec2-nat-instances/main.tf
32:  init   = <<END_INIT
52:  template = <<END_INIT

modules/credstash-setup/data.tf
14:  template = <<END_TEMPLATE

modules/consul-agent-generic-init/main.tf
32: *     extra_init = <<EOF

modules/ebs-snapshot-iam/main.tf
43:  policy = <<EOF

modules/prometheus-server/tests/main.tf
73:  init_suffix = <<END_INIT

modules/credstash-setup/main.tf
56:  policy = <<END_POLICY
78:  policy = <<END_POLICY

modules/ec2-auto-recover-instances/main.tf
45: *       user_data = <<END_INIT

[ketzacoatl]

I would prefer to have a tool that warns us when a mistake has been made, over classifying all use of inlines as evil. I believe we should review occurrences of this individually, and I'm not sure how I feel about a single issue to encompass of occurrences of this pattern.

[kerscher]

There is nothing inherently evil in this, just as there is nothing evil in pointer arithmetic in C. But designing a tool that checks when either is correct is a very large challenge. It’s just a set of tradeoffs. I will remove the checks from #103 until we do a manual review then.

We can add the review for the items above in this issue here until all occurrences have been documented as OK or not, since it has to be tracked somewhere and this has the list already. Then we close it. Sounds good?