terraform v0.12.0-dev
This is a custom Terraform CLI built from this commit in master (from 3/19/19): 35df450dc0afc0bb7226391532700cd92347a631
variable "policy_name" {
default = ""
}
variable "policy_description" {
default = ""
}
variable "policy_compartment_id" {
default = ""
}
variable "policy_statements" {
default = []
}
resource "oci_identity_policy" "this" {
count = "${length(var.policy_name) > 0 ? 1 : 0}"
name = "${var.policy_name}"
description = "${var.policy_description}"
compartment_id = "${var.policy_compartment_id}"
statements = "${var.policy_statements}"
}
https://gist.github.com/alexng-canuck/62093fedc7321b7a34c3b67bfdd0a927
In v0.11, running terraform plan on the above would succeed.
Even though the statements attribute has a MinItems setting of 1 in the resource schema and the above config specifies an empty list; we would expect the validation to be ignored because the count on the resource is set to zero.
In v0.12, running terraform plan results in this error:
Error: statements: attribute supports 1 item as a minimum, config has 0 declared
on vcn.tf line 41, in resource "oci_identity_policy" "this":
41: resource "oci_identity_policy" "this" {
The v0.12 validation appears to be more aggressive and is validating a resource against its schema even though the count is set to zero.
terraform plan or terraform validate
The v0.11 behavior is currently relied upon in OCI modules in the Terraform modules registry. See https://github.com/oracle-terraform-modules/terraform-oci-iam/blob/ecf943cb303563b2ff88e963ae1c2f2f94baf39c/modules/iam-dynamic-group/main.tf#L37
With the new behavior, the module will require significant changes in its contract with module users. Working around the new behavior would require forcing people to provide attributes that are ultimately discarded; just so that the validation can pass.
Hi @alexng-canuck! Thanks for reporting this.
It is intentional that Terraform validates all resource blocks once regardless of count, to make sure the configuration is valid for all possible input variable values, but what doesn't seem to be working correctly here is that input variables should have unknown values during validation, and so Terraform should validate just that the variable has the correct type without also checking a particular value for it.
At first glance it looks like the variable's default value is overriding the variable being set as unknown for the validation check, which I'd consider a bug: the validation pass should be using unknown values of the appropriate type for all input variables regardless of whether a default is set, so that the validation pass will not consider specific variable values.
Thanks @apparentlymart. Agree that the configuration should be validated for all possibilities.
Will this be fixed before the v0.12 release?
Tried to reproduce it and got the following results:
With terraform validate the configuration passes the validation
With terraform plan or terraform apply it fails because it uses the default value (which is an empty array).
It is also indeed correct that in v0.11 everything passed.
So to me it seems like the new behaviour is better, unless we want the same behaviour as in v0.11, which is not validating it if count is set to 0
Thanks @GBrawl! That extra context is helpful since it leads to a potential explanation: it seems like when Terraform is performing validation as part of the terraform plan and terraform apply steps it is not correctly using unknown values to represent the input variable values, and thus it is trying to validate the resource block with known (default, in this case) variable values.
While indeed we could consider this to be correct behavior, I think in practice it must change, for this reason: validation happens before we evaluate the count expression, so it can't take into account the value of count. However, it is common to use count = 0 to "disable" certain resources completely, and we cannot break that pattern in 0.12 without having something to replace it.
Making the validation pass set all of the values to unknown values of a suitable type seems like the best compromise: it won't detect statements being empty (since it won't know if it's empty yet) but it _will_ detect if the variable assigned to it has a non-list type and report _that_ problem.
There is another opportunity for deeper validation after count is evaluated and the resource is "expanded" into zero or more instances, and that is the time where we'd detect an invalid _value_ for that variable, assuming that count were greater than zero, and thus I think that gets back to the desired effect here. Although it causes Terraform to miss some validation opportunities, I think that's the better compromise than requiring every expression within that body to be complicated so it can take into account the situation when count = 0; better to leave the count expression itself to worry about that.
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.
Most helpful comment
Thanks @GBrawl! That extra context is helpful since it leads to a potential explanation: it seems like when Terraform is performing validation as part of the
terraform planandterraform applysteps it is not correctly using unknown values to represent the input variable values, and thus it is trying to validate the resource block with known (default, in this case) variable values.While indeed we could consider this to be correct behavior, I think in practice it must change, for this reason: validation happens before we evaluate the
countexpression, so it can't take into account the value ofcount. However, it is common to usecount = 0to "disable" certain resources completely, and we cannot break that pattern in 0.12 without having something to replace it.Making the validation pass set all of the values to unknown values of a suitable type seems like the best compromise: it won't detect
statementsbeing empty (since it won't know if it's empty yet) but it _will_ detect if the variable assigned to it has a non-list type and report _that_ problem.There is another opportunity for deeper validation after
countis evaluated and the resource is "expanded" into zero or more instances, and that is the time where we'd detect an invalid _value_ for that variable, assuming thatcountwere greater than zero, and thus I think that gets back to the desired effect here. Although it causes Terraform to miss some validation opportunities, I think that's the better compromise than requiring every expression within that body to be complicated so it can take into account the situation whencount = 0; better to leave thecountexpression itself to worry about that.