Add lien support to project-factory

[adrienthebo]

This commit adds a lien variable and backing logic to add liens to
GCP projects. This allows users to prevent projects from being
unintentionally deleted.

Because the lien is a Terraform resource, it's still possible to delete
the project with an errant `terraform destroy.

[lilithmooncohen]

👍

[morgante]

Great work, just one change.

[morgante]

Our standard is to use the strings "true" and "false" for boolean variables.

[adrienthebo]

Changed in amended commit 4483ccd

[adrienthebo]

Fixed up the default value for the lien variable, and added a negative test to the minimal fixture asserting that there are no liens.

[morgante]

Still needs more work to match our standard of boolean variables as strings.

[morgante]

Now that a string is being used, you need to check for the string explicitly. See how we handle random ID.

[adrienthebo]

When we check the random_project_id variable we don't do an explicit string comparison, as given by the following snippet from `main.tf:

locals {
  temp_project_id        = "${var.random_project_id ? format("%s-%s",var.name,random_id.random_project_id_suffix.hex) : var.name}"
  # [...]
}

I've also tested the behavior in the terraform console:

> "${"true" ? 1 : 0}"
1
> "${"false" ? 1 : 0}"
0
> "${"shrug" ? 1 : 0}"
__builtin_StringToBool: strconv.ParseBool: parsing "shrug": invalid syntax in:

${"${"shrug" ? 1 : 0}"}

That behaves as expected, only allowing true or false and rejecting other strings.

If we do an explicit string comparison, we lose the validation of the input as a boolean:

> "${"true" == "true" ? 1 : 0 }"
1
> "${"false" == "true" ? 1 : 0 }"
0
> "${"shrug" == "true" ? 1 : 0 }"
0

Have I missed something with how we should handle booleans?

[morgante]

The issue is we're relying on Terraform's implicit coercion from "true" to 1 which is not guaranteed.

https://www.terraform.io/docs/configuration/variables.html#booleans

[lilithmooncohen]

Mirroring @morgante's comment here. Once that's resolved have a 👍 from me.

[morgante]

@adrienthebo Where did we land on this? Would like to merge soon.

[adrienthebo]

Per the earlier discussion, this comparison is valid but we should use "true" over true as the variable input. I've updated the use of the bare true in the tests and updated this PR.

[adrienthebo]

Thanks!