Bugfix for value of 'count' cannot be computed issue when passing all…

[jcejohnson]

…owed_inbound_security_group_count

Similar to change 707c87 in terraform-aws-vault

Tested internally.
Cannot execute the test suite due to corporate restrictions.

Addresses issue #84

[brikis98]

Ah, good fix. This is quite a frustrating Terraform gotcha. Thank you!

Can you describe how you tested this? We can't run automated tests on external PRs for security reasons, so I'm trying to get a sense of what you checked before merging.

[jcejohnson]

Last week I set all of the allowed_inbound_cidr_blocks lists to the empty list for both our consul and vault nodes' security groups and attempted to use allowed_inbound_security_group_ids instead. Vault went fine but consul gave me the dreaded value of 'count' cannot be computed error.

In my code I use the consul-cluster module with empty lists for allowed_inbound_cidr_blocks and allowed_inbound_security_group_ids.

module "consul_cluster" {
  source = "git::https://github.com/hashicorp/terraform-aws-consul.git//modules/consul-cluster?ref=v0.3.10"
  allowed_inbound_cidr_blocks = []
  allowed_inbound_security_group_ids = []
  ...

I then use consul-security-group-rules to allow access to other members of the cluster as well as the vault nodes that are clients of the cluster:

module "consul_security_group_rules" {
  source = "git::https://github.com/EFXCIA/terraform-aws-consul.git//modules/consul-security-group-rules?ref=bugfix/security-group-count"

  security_group_id           = "${module.consul_cluster.security_group_id}"
  allowed_inbound_cidr_blocks = []

  allowed_inbound_security_group_ids = "${concat(
    list(module.consul_cluster.security_group_id),
    list(module.vault_cluster.security_group_id)
    var.consul_allowed_inbound_security_group_ids
  )}"

  allowed_inbound_security_group_count = "${2+length(var.consul_allowed_inbound_security_group_ids)}"
}

Similarly, my vault cluster uses consul-client-security-group-rules to give the consul cluster access to the consul clients on the vault nodes:

module "security_group_rules" {
  source = "git::https://github.com/EFXCIA/terraform-aws-consul.git//modules/consul-client-security-group-rules?ref=bugfix/security-group-count"

  security_group_id           = "${module.vault_cluster.security_group_id}"
  allowed_inbound_cidr_blocks = []

  allowed_inbound_security_group_ids   = ["${module.consul_cluster.security_group_id}"]
  allowed_inbound_security_group_count = 1
}

At a high-level, it is structured thusly:

$ egrep --color '(^[a-z])|(source =)' consul.tf | grep -v '# '
module "consul_cluster" {
  source = "git::https://github.com/hashicorp/terraform-aws-consul.git//modules/consul-cluster?ref=v0.3.10"
module "consul_security_group_rules" {
  source = "git::https://github.com/EFXCIA/terraform-aws-consul.git//modules/consul-security-group-rules?ref=bugfix/security-group-count"
module "consul_iam_policies_servers" {
  source = "git::https://github.com/hashicorp/terraform-aws-consul.git//modules/consul-iam-policies?ref=v0.3.10"

$ egrep --color '(^[a-z])|(source =)' vault.tf | grep -v '# '
module "vault_cluster" {
  source = "git::https://github.com/hashicorp/terraform-aws-vault.git//modules/vault-cluster?ref=v0.10.0"
module "security_group_rules" {
  source = "git::https://github.com/EFXCIA/terraform-aws-consul.git//modules/consul-client-security-group-rules?ref=bugfix/security-group-count"
module "vault_api_elb" {
  source = "git::https://github.com/hashicorp/terraform-aws-vault.git//modules/vault-elb?ref=v0.10.0"
module "vault_api_all_nodes" {
  source = "git::https://github.com/hashicorp/terraform-aws-vault.git//modules/vault-elb?ref=v0.10.0"
module "vault_cluster_elb" {
  source = "git::https://github.com/hashicorp/terraform-aws-vault.git//modules/vault-elb?ref=v0.10.0"

So, my internal testing, is:

• execute the original code using CIDR blocks: success
• execute the original code using security groups and no CIDR blocks: fail
• execute the modified code using security groups and no CIDR blocks: success

There may still be an issue with the consul-security-group-rules use of consul-client-security-group-rules (module "client_security_group_rules" in terraform-aws-consul//modules/consul-security-group-rules:main.tf@163). My PR does not modify it to include the allowed_inbound_security_group_count variable I've added. My code does not trigger that module since I provide empty lists to both allowed_inbound_cidr_blocks and allowed_inbound_security_group_ids in my usecase. If you agree it should be included there I'll go ahead & add it.
Similarly, there are three places in terraform-aws-vault using consul-client-security-group-rules (v0.3.3) that I have not touched.

[brikis98]

There may still be an issue with the consul-security-group-rules use of consul-client-security-group-rules (module "client_security_group_rules" in terraform-aws-consul//modules/consul-security-group-rules:main.tf@163). My PR does not modify it to include the allowed_inbound_security_group_count variable I've added. My code does not trigger that module since I provide empty lists to both allowed_inbound_cidr_blocks and allowed_inbound_security_group_ids in my usecase. If you agree it should be included there I'll go ahead & add it.

Yes please! In fact, please search for any usage of the consul-client-security-group-rules and consul-security-group-rules and update them to pass in the new allowed_inbound_security_group_count parameter. Similarly, could you update the consul-cluster module to expose the same parameter and pass it through?

[jcejohnson]

Will do!

[jcejohnson]

OK, that's done.

I also found a nasty little bug in consul-cluster's use of consul-security-group-rules: consul-cluster is not adding its own aws_security_group.lc_security_group.id to the list of allowed_inbound_security_group_ids (and consul-security-group-rules does not automatically add its security_group_id input to the allowed inbound security group ids). Unless that was intentional?

I discovered the bug when I removed my consul_security_group_rules module and let my consul_cluster module pass allowed_inbound_security_group_ids and allowed_inbound_security_group_count itself.

It looks like terraform-aws-vault//modules/vault-cluster has the same problem with its use of vault-security-group-rules. I'm testing the same fix for this locally & will PR it later.

[brikis98]

I could've sworn we already added self rules to the security group rules modules...

[brikis98]

Hm, perhaps instead of doing this, the consul-security-group-rules module should explicitly add a separate rule that allows connections from self?

[jcejohnson]

I didn't find any reference to 'self' in consul-security-group-rules or consul-client-security-group-rules so I've added them following the pattern in terraform-aws-vault.

[brikis98]

Thank you for these fixes! I'll merge now and let the tests run. When they pass, I'll issue a new release and paste the link here.

[brikis98]

https://github.com/hashicorp/terraform-aws-consul/releases/tag/v0.4.0