Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

string literal "false" evaluates to "0" using null_data_source #13512

Closed
in4mer opened this issue Apr 10, 2017 · 8 comments
Closed

string literal "false" evaluates to "0" using null_data_source #13512

in4mer opened this issue Apr 10, 2017 · 8 comments
Assignees
Milestone

Comments

@in4mer
Copy link

@in4mer in4mer commented Apr 10, 2017

Terraform Version

0.9.2

Affected Resource(s)

Please list the resources as a list, for example:

  • core

If this issue appears to affect multiple resources, it may be an issue with Terraform's core, so please mention this.

Terraform Configuration Files

variable "map1" {
  default = {
    key = "whatever"
  }
}
variable "map2" {
  default = {
    key = "false"
  }
}

variable "key" { default = "key" }
data "null_data_source" "wtf" {
  inputs = "${zipmap(split(",", lookup(var.map1, var.key)), split(",", lookup(var.map2, var.key)))}"
}

output "output" {
  value = "${zipmap(split(",", lookup(var.map1, var.key)), split(",", lookup(var.map2, var.key)))}"
}

output "same_output_from_nds" {
  value = "${data.null_data_source.wtf.inputs}"
}

Debug Output

Not even going to bother

Panic Output

The only panic this should elicit is entirely existential

Expected Behavior

The string literal "false" should not be evaluated

Actual Behavior

I don't even...

data.null_data_source.wtf: Refreshing state...

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Outputs:

output = {
  whatever = false
}
same_output_from_nds = {
  whatever = 0
}

Steps to Reproduce

Please list the steps required to reproduce the issue, for example:

  1. terraform apply

Important Factoids

Is no one else actually using null_data_source? This is really, really frustrating.

References

Possibly ??

@in4mer

This comment has been minimized.

Copy link
Author

@in4mer in4mer commented Apr 18, 2017

Having run into this bug yet again, I have a much simpler test case now:

data "null_data_source" "WTF_OVER" {
  inputs = {
    a = "true",
    b = "false"
  }
}

output "map_from_nds" {
  value = "${data.null_data_source.WTF_OVER.inputs}"
}

What is the output, you ask?

data.null_data_source.WTF_OVER: Refreshing state...

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Outputs:

map_from_nds = {
  a = 1
  b = 0
}
@apparentlymart

This comment has been minimized.

Copy link
Member

@apparentlymart apparentlymart commented Apr 19, 2017

Thanks for that simpler repro, @in4mer!

@apparentlymart apparentlymart self-assigned this Apr 19, 2017
@apparentlymart

This comment has been minimized.

Copy link
Member

@apparentlymart apparentlymart commented Apr 19, 2017

So I just spent the morning walking through all of Terraform's layers and got to the bottom of what's going on here.

The TLDR is that this is an unexpected interaction in how Terraform processes non-primitive values, and it's a specific symptom of a general problem we're already aware of. It's not a straightforward fix, but it will be addressed as part of some later work to rationalize how Terraform thinks about values so that type information isn't lost and imprecisely recovered as data moves between different parts of Terraform.


Some gory details now, mostly for my benefit for reference later:

Some context here first: Terraform currently represents values in numerous different ways at different points. For example, in state the possibly-complex structure of attributes on a resource are flattened into a map[string]string using flatmap, a remnant of Terraform's architecture from before it had support for lists and maps. The interpolation language HIL, which is what defines the syntax and semantics of the interpolation expressions, has its own type system with support for real types like boolean and maps, but all of the primitive type values coming out of HIL get converted to string representations, like "true" on the way out.

So in summary, Terraform spends quite a significant amount of its code marshalling values back and forth between strings and typed values in various different type systems, using various different systems to do so.

And that brings us to what's going on here:

  • HIL evaluates the zipmap function and sees that its result is a map, so the inputs expression evaluates to a map.
  • The evaluation result is converted from HIL's value representation into a Go-oriented value representation using HIL's conversion function, which produces a Go value equivalent to map[string]string{"a": "true", "b": "false"}, because HIL converts primitive values to string as they exit.
  • The resolved resource configuration sent to the null_data_source (which, internally, is a map[string]interface{} therefore includes a key called "inputs" whose value is the map from the previous step.
  • The null provider, like all builtin providers, is built with the helper/schema library, which attempts to provide a common type system for describing resource attribute structures that hides -- to a certain extent -- the various different ways data is represented in config vs. state vs. diff. The inputs attribute is defined as being a schema.TypeMap, which is a map from string to string, so when the provider code calls d.Get("inputs") it gets back a value equivalent to map[string]interface{}{"a": "true", "b": "false"}.
  • After apply, helper/schema must construct a terraform.State object representing the state of that instance. State can only represent strings, so it uses flatmap to lower the typed data structure down into a string map like map[string]string{"inputs.%": "2", "inputs.a": "true", "inputs.b": "false"}, which Terraform then saves for later.
  • When Terraform visits the output node, it sees that there's a HIL expression referencing data.null_data_source.WTF_OVER.inputs, so it needs to figure out what value to use for that variable (in HIL's terminology). It looks in the instance state and finds that there's no attribute called inputs but there is one called inputs.% and so it guesses that this is the result of flatmap processing a map.
  • Terraform then calls into flatmap again to un-flatten the map, recovering the original map. Except it doesn't get back exactly what it put in, because flatmap has a hidden special case where the string values "true" and "false" get replaced with Go booleans true and false, yielding a structure like map[string]interface{}{"a": true, "b": false}.
  • Since we want to use this value as a HIL variable, Terraform then passes it into HIL's function that accepts native Go values and converts them into HIL's internal representation. HIL internally uses another library called mapstructure to try to coerce the given values into the more restrictive form that HIL wants: strings, lists of strings, and maps of strings.
  • mapstructure has a "weak" mode where it will do possibly-lossy conversions in order to get values to match the target type. Since HIL asks for all primitives to be strings, mapstructure converts the bool values in our map to strings. And here's the rub: mapstructure uses "1" and "0" as its string representations for boolean values, so HIL ends up seeing a structure like map[string]interface{}{"a": "1", "b": "0"}.
  • HIL has implicit type conversions of its own, so for example if a string is used in a context where a boolean is required, such as in the ? : conditional operator, it will get converted into a proper HIL boolean value, and HIL's converter is tolerant of both the "true"/"false" and "1"/"0" representations of booleans, so this works as expected. However, in this case we are not using these values in a boolean context, so no conversion to boolean is done by HIL.
  • Since the map value is the sole result of the interpolation sequence ${data.null_data_source.WTF_OVER.inputs}, HIL wants to coerce this into being a map with string elements, since that's the rule for values exiting HIL. Since these values are already strings "1" and "0" they are untouched, leaking out as these non-canonical boolean representations.
  • Terraform takes that result and saves it as the value of the output in the state. Unlike resource instance states, outputs can be stored as real maps without flattening, so this gets persisted as the JSON expression {"a":"1","b":"0"}, and printed as such in the apply result.
@apparentlymart

This comment has been minimized.

Copy link
Member

@apparentlymart apparentlymart commented Apr 19, 2017

I consolidated #7934 into this, but just wanted to note: that issue suggests that this bug can also affect literal maps in configuration, probably for the same reason, but that causes strange diffs to be produced and so is arguably more serious than just getting the wrong serialization in an output.

@codycushing

This comment has been minimized.

Copy link

@codycushing codycushing commented May 14, 2017

We are experiencing this as well, the value of assign_public_ip is coming through as string "0" or "1", requiring manual coercion for proper use.

resource "baremetal_core_instance" "TFInstance" {
  display_name = "TFInstance"
  shape = "${var.InstanceShape}"
  create_vnic_details {
    assign_public_ip = false
    subnet_id = "${var.SubnetOCID}"
  },
}
briangustafson added a commit to terraform-providers/terraform-provider-oci that referenced this issue Sep 22, 2017
For most bool values in Terraform, you can use true and "true" interchangeably.
The bool values nested in create_vnic_details are treated differently, apprently
due to hashicorp/terraform#13512: true appears as "1",
while "true" appears as "true". ParseBool handles both.
maartenvanderhoef added a commit to blinkist/terraform-aws-airship-ecs-service that referenced this issue Jul 1, 2018
…p/terraform#13512 )

Formatlist for iam policies was not correct earlier
@nick4fake

This comment has been minimized.

Copy link

@nick4fake nick4fake commented Oct 24, 2018

Are there any workarounds?

@apparentlymart apparentlymart added config and removed core labels Nov 7, 2018
@apparentlymart apparentlymart added this to the v0.12.0 milestone Nov 7, 2018
@apparentlymart

This comment has been minimized.

Copy link
Member

@apparentlymart apparentlymart commented Nov 20, 2018

Hi all! Sorry for the long silence.

I've been very much looking forward to verifying this one against the v0.12.0-alpha2 build because the insidiousness of this issue was one of the big motivators to rework Terraform to use the same value representations throughout and avoid all of the weird conversions I listed out in my earlier comment.

I'm pleased to report that this is now working correctly in the alpha and this fix is in the master branch ready to be included in the forthcoming v0.12.0 release. With the simpler config given by @in4mer above, unmodified, the output is now as expected:

$ terraform apply
data.null_data_source.WTF_OVER: Refreshing state...

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Outputs:

map_from_nds = {
  "a" = "true"
  "b" = "false"
}

Other new features in v0.12.0 should solve the remaining cases for (ab)using null_data_source as a temporary stash for values, so the specific weird behavior here would not have mattered too much moving forward anyway, but the new correct behavior reflects Terraform now using a consistent value representation throughout and not lossily converting between different representations as it did before, and that should present as more reasonable behavior throughout.

Thanks for reporting this, and sorry for the delay in getting it fixed.

@vasilij-icabbi

This comment has been minimized.

Copy link

@vasilij-icabbi vasilij-icabbi commented Mar 3, 2019

@nick4fake the workaround from top of my head could be:

${<resource>.<id>.<variable> == 1 ? "true" : "false"}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.