[BUG] Getting a null if a variable doesn't have a default value

[rsareth]

Describe the bug
It is the same kind of issue like #305. And since TF 1.1.0, it is possible to define a variable nullable: https://www.terraform.io/language/values/variables#disallowing-null-input-values

How you're running Regula

• I'm using Regula v2.5.0 as a Rego library with OPA v0.34.1. We are still using terraform 1.0.x, but we plan to upgrade to the latest version of terraform and the latest provider aws 3.x.x.

Steps to reproduce

1. Create those files:

# rules/rule.rego
package rules.tf_aws_check_resources_prefix_for_logging

import data.fugue

__rego__metadoc__ := {
	"custom": {"severity": "Medium"},
	"id": "RULE_001",
	"title": "Checking the prefix is in the right shape.",
	"description": "Checking the prefix is in the right shape to prevent from melting the logs between all services"
}

resource_type := "MULTIPLE"
all_s3_buckets := fugue.resources("aws_s3_bucket")

valid_s3(resource) {
  prefix := resource.logging[_].target_prefix
  not startswith(prefix, "/")
  not endswith(prefix, "/")

  split_prefix := split(prefix, "/")
  count(split_prefix) > 1
  split_prefix[0] == "s3"
  split_prefix[1] == resource.bucket
}

policy[r] {
  s3_bucket := all_s3_buckets[_]
  valid_s3(s3_bucket)
  r := fugue.allow_resource(s3_bucket)
}

policy[r] {
  s3_bucket := all_s3_buckets[_]
  prefix = s3_bucket.logging[0].target_prefix
  not valid_s3(s3_bucket)
  msg := sprintf("The s3 bucket logging's prefix, %v, must be in this shape s3/%v/, instead of %v", [s3_bucket.id, s3_bucket.bucket, prefix])
  r := fugue.deny_resource_with_message(s3_bucket, msg)
}

# split/s3.tf
variable "foobar" { # <--- Empty variable
}

locals {
  common_base_name = "${terraform.workspace}-${var.foobar}"
  s3_bucket_b_name = "${local.common_base_name}-mybucket"
}

resource "aws_s3_bucket" "b" {
  bucket = local.s3_bucket_b_name

  logging {
    target_bucket = "bucket_1.s3.amazonaws.com"
    target_prefix = "s3/${local.s3_bucket_b_name}"
  }

  tags = {
    Name = "My bucket"
  }
}

2. Run like this:

$ regula run -n -i rules split_tf/
RULE_001: Checking the prefix is in the right shape. [Medium]

  [1]: aws_s3_bucket.b
       in split_tf/s3.tf:9:1
       The s3 bucket logging's prefix, aws_s3_bucket.b, must be in this shape s3/null/, instead of null

Found one problem.

We've tried a workaround by defining a default value with the empty character but we can't rely on this. We want to make mandatory to provide some values.

Thank you

Regards,
Rasmey

[jaspervdj-luminal]

@rsareth As always, thanks for the highly detailed bug reports. This really allows us to make Regula better for everyone.

I'm wondering what the expected behavior should be here? It seems reasonable to replace non-nullable string variables with "". However, it seems like there isn't enough context in split/s3.tf to determine this is non-nullable, or is there a nullable = false elsewhere?

[rsareth]

Hello @jaspervdj-luminal,

You are asking the right question. I don't really know what it should be in that specific use case.

Our context:
We have a lot of repositories. To help visualize our organization of repositories, you can consider each repository as a group of microservices containing the application and the infrastructure code. Having so many repositories made us to define:

• naming conventions
• standard organization of the source code in it.

The name of the microservice is partially correlated to the name of the repository. For example, the repo name foobar-application-a, its microservice name is applicaton-a.

As we tag all of our cloud resources with a naming convention, we have many empty variables that need to be filled by the terraform cli. So technically, it looks like this:

$ terraform apply -var="app_name=<APP_NAME_A>" -var="app_group=<GROUP_A> -var="commit_id=<SHORT_COMMIT_ID>" [...]"

We don't use terragrunt and we didn't make our microservices as terraform modules too. We developed our tool in python by wrapping the terraform cli. We are using this library: https://github.com/beelit94/python-terraform
Our deployment concept is based on some principles from the Kubernetes Deployment / Helm chart files in YAML. Those YAML files are stored in other repositories. In those repositories, we have tfvars files that are read by our python tool. So technically, if someone wanted to deploy manually from her/his computer without providing the values for the empty variables, it will fail for sure. This kind of failure is a safety for us.

BTW, these standards help us to scale easily and to write more tools around them like scannig with regula our terraform code form example.

Suggestion 1:
Because regula is a policy as code tool, it would be helpful to set the name of the variable as a value. It would prevent from having the null I reported.

But doing this, the cons are:

• if someone wants to check the length of some attributes by writing rego policies, the policies wouldn't be relevant or raise false positives
• If we have to set default values by an empty character, it will make our deployment mechanism less safe

Suggestion 2:
In terraform 1.1.0, with the new feature allowing us to define a variable as nullable, on regula, you can define another behaviour:

1. If the variable is nullable, do this
2. Otherwise, do something else

But it is like the Pandora box. It is so dirty to me to have different behaviours that it can bring more issues than solving them.

Suggestion 3:
Regula can have a new option in the run subcommand allowing us to provide the tfvars. Having this kind of feature would allow us to check our tf code according to different contexts. In that case, we could write:

• generic policies for common context
• specific policies for specific context
  For the specific context and according to the kind of organization, we can imagine:
• the security team can write new policies on their side without notifying us for several days or weeks. To test new configuration for example
• and during that time, the product teams can continue to work without any annoyances from them

But at the end, I don't know! :-(

Regards,
Rasmey

[rsareth]

Hi @jaspervdj-luminal,

I saw your last release. In the version 2.5.0, checking some resources having some attributes filled with nullable variables didn't fail. On contrary, it helped to make fix misconfiguration like I wanted.

But in the version 2.6.0, the same policy doesn't work anymore. So using the latest version makes me to be less precise on the check or I have to set a default value which I don't want to.

The policy is doing this:

• checking that each lambda has a log group named "local.function_name"
• checking that the associated log group is named properly: "/aws/lambda/local.function_name"

The local function_name is initialized like this:

locals {
  function_name = "${terraform.workspace}-${var.custom_name}
}

The variable custom_name is null in the HCL2 code BUT it is provided in a tfvars file in another location. I type this globally:

$ terraform apply -var "custom_name=foobar"

So, did you have time to think of a solution on this particular issue ?

Thank you

Regards,
Rasmey

[rsareth]

I was thinking of a solution. Regula doesn't parse the tfvars file. If it does, it can offer more use cases of control.

For example, we could check the length of some attributes according to the tfvars used for one environment.