Tristate boolean values

[jacobbednarz]

SDK version

v2

Use-cases

A common issue I've had to work around in the past is optional booleans capable of a true, false and uninitialised state. This usually comes around with HTTP PUT requests where you build the entirety of the payload and send it to an endpoint. This becomes even more tricky when the Terraform configuration block is optional/conditional and not always applied. Example

(explicitly enable a feature)

resource "..." "..." {
  example_attr = "foo"
  enabled = "true"
}

(explicitly disable a feature)

resource "..." "..." {
  example_attr = "foo"
  enabled = "false"
}

(uninitialised value and should not be sent in a payload)

resource "..." "..." {  
  example_attr = "foo"
}

All three of these examples would behave differentially using HTTP PUT. The example above is rather simple but does quickly grow in unnecessary workarounds and complexity of providers when you start nesting the schema attributes.

Attempted Solutions

In general, we have used attributes with a Optional field in the schema and relied on d.GetOkExists however that functionality has since been deprecated with no real alternative available. Another approach we've used is a boolean pointer in the underlying library for the API however it still relies on some functionality in the provider itself to determine whether to set it or not.

Proposal

I'm not quite sure how it would be implemented, or even if it is possible, however something like a tristate boolean value in go-cty would be great. The values would be true, false and unset/uninitialised which would allow like for like functionality with (I think) most of the d.GetOkExists wants in a reliable fashion.

The solution would also need to be able to "unset" a value in Terraform should a method be called on the value in order to satisfy the need where a value is set as true/false but later returned to an uninitialised state.

References

None that I found.

[timofurrer]

In the terraform-provider-gitlab we also face this issue regularly.

What are proven solutions / workarounds providers are using? Do you still rely on GetOkExists() ?

[jacobbednarz]

we do use GetOkExists but have also used pointer booleans in the past for some endpoints.

GetOkExists is the only way we’ve found consistently works for tristate booleans.

[timofurrer]

@jacobbednarz thanks for the quick response. We've also found this comment here: #350 (comment) Thus, we keep using GetOkExists() for now ...

Can you "point" me to an example using the boolean pointers?

[jacobbednarz]

https://github.com/cloudflare/terraform-provider-cloudflare/blob/93d9bef4aa3b27adda6313a658ea09b55cfc221c/cloudflare/resource_cloudflare_record.go#L52 is probably one of the most problematic ones. it still uses d.GetOkExists but in a slightly different way due to the underlying library.

[dominik-lekse]

Found this issue, when troubleshooting an implementation of an optional TypeBool. The previous comments have been very helpful. For the record, I want to add additional insights on change detection:

Given the following attribute schema and config.

Schema: map[string]*schema.Schema{
    // ...
    "enabled": {
        Type:     schema.TypeBool,
        Optional: true,
        Computed: true,
        Default:  nil,
    },
    // ...
},

resource "..." "..." {
  enabled = false
}

d.HasChange("enabled") returns false for an optional (tristate) TypeBool when the attribute has been set explicitly to false. The most likely reason appears to be that HasChange cannot distinguish between an unset or nil and an explicit false value for a TypeBool.

old, new := d.GetChange("enabled") returns false, false instead of an expected nil, file.

Consequently, I assume that change detection of optional bool attributes is currently not fully supported by the provider sdk.

In the provider implementation, I used the deprecated GetOkExists instead of HasChange although the semantics are slightly different.

[jacobbednarz]

Another workaround is using a string for boolean values. This isn't ideal but does allow you to use "true"/"false" or "on"/"off (with an internal mapping) as a way to distinguish between unset, true and false values.

[jacobbednarz]

I also posted this in https://discuss.hashicorp.com/t/tristate-booleans/36788 to see if I can spark up any other ideas I may be missing here.

[jacobbednarz]

I think the provider side of this has a workaround (pointers and verbose GetOkExists checks) except the state file -- it's also entirely possible I've missed a use case though. TypeBool defaults to false which, depending on the API, is undesirable given some values are uninitialised.

It does look like this is in consideration for terraform-plugin-framework though - https://github.com/hashicorp/terraform-plugin-framework/blob/v0.6.1/types/bool.go#L32-L43. The downside here is that I can't see us swapping to it anytime soon due to the lack of parity with the terraform-plugin-sdk and functionality we rely on.

[jacobbednarz]

@paddycarver as the implementor of the functionality in terraform-plugin-framework, do you happen to know if there are plans to include the this in terraform-plugin-sdk? i'm happy to take a pass at this if it is and it's just caught in a backlog somewhere.

[paddycarver]

Hey @jacobbednarz,

I don't work on Terraform anymore, but my not-expert, not-informed, no-insider-knowledge opinion is you're unlikely to see this backported to SDKv2. The way SDKv2 is structured isn't really conducive to representing null or "unset" values consistently and reliably. You can use the experimental ResourceData.GetRawConfig to check whether the value is set, null, or unknown in the config, but writing it back as null isn't really supported.

The reason I think this is unlikely to be backported is because schema.ResourceData is largely an abstraction over config, state, and plan all as one read/write map of strings to other strings. This hasn't been a true abstraction in a very long time, and it's largely kept alive as a compatibility shim. Values are always read through compatibility layers meant to imitate that abstraction (except for GetRawConfig, above, which is a bolted-on workaround) and are always written back through those layers, as well. Unfortunately, map[string]string doesn't really provide a reliable way to model null or unset values that are distinct from the empty string. I know this sounds like it wouldn't apply because we're dealing with booleans, but after years of accruing behaviors, nothing is that straightforward anymore, sadly.

The framework was intended to re-align provider development to the abstractions that are actually used today, and to make things a bit more straightforward again. Which is why you can do this in the framework, but can't in SDKv2.

I know you mentioned you can't swap to the framework due to a lack of parity and missing functionality you rely on, but as far as I know parity is getting pretty close and the features missing are pretty few and far-between. If the features you need aren't being tracked as issues on the framework repo, I'd recommend opening them.

As I said, I don't work on Terraform any more, so take everything I've said here with a giant grain of salt. Just a non-maintainer community member throwing his two cents in, here.

[jacobbednarz]

Thanks @paddycarver, appreciate the detailed response here. Apologies for the ping, I thought you were still at Hashicorp. Congrats on the new gig!

For what it's worth, the framework has improved since I last checked so I might grab those last few and watch them for some progress. While the experimental approach is tempting, it is probably just as beneficial to roll forward to something supported and with less abstractions than it is to maintain an experimental feature.