feat: Support MarkAsComputed Flag in SchemaInfo struct

[tmeckel]

Hello!

• Vote on this issue by adding a 👍 reaction
• If you want to implement this feature, comment to let us know (we'll work with you on design, scheduling, etc.)

Issue details

In the past there were issues with various Terraform providers where people carelessly removed the computed` flag from a property so that the value can't be read after a deployment only when the value is set in the first place as configuration value.

Currently the SchmaInfo struct contains the flag MarkAsComputedOnly that copes with the way around, because it will mark the parameter as computed and not allow the user to set it.

Adding a flag MarkAsComputed *bool to SchemaInfo struct would allow to enforce a property from an upstream resource to be computed.

Perhaps it would be necessary to reduce the proliferation of the various resource flags, by mimic the flags which are available for a resource inside the Terraform SDK or Plugin Framework, even if this would mean to introduce a breaking change.

Affected area/feature

Controlling provider schema generation

[tmeckel]

@iwahbe @t0yv0 I already started an implementation for this.

https://github.com/tmeckel/pulumi-terraform-bridge/tree/feat/mark-as-computed

[iwahbe]

Hi @tmeckel. I'm having trouble understanding how MarkAsComputed will differ from MarkAsComputedOnly. Can you elaborate?

[tmeckel]

@iwahbe Terraform Resource properties come with the following matrix of combinations of required, optional, computed.

hashicorp/terraform-plugin-framework#31

Required: true: ✅
Required: false: ❌
Required: true + Optional: true: ❌
Required: true + Optional: false: 👍 (just verbose)
Required: false + Optional: true: 👍 (just verbose)
Required: false + Optional: false: ❌
Required: true + Computed: true: ❌
Required: true + Computed: false: 👍 (just verbose)
Required: false + Computed: true: 👍 (just verbose)
Required: false + Computed: false: ❌
Optional: true: ✅
Optional: false: ❌
Optional: true + Computed: true: ✅
Optional: false + Computed: true: 👍 (just verbose)
Optional: true + Computed: false: 👍 (just verbose)
Optional: false + Computed: false: ❌
Computed: true: ✅
Computed: false: ❌

Computed only makes sense with Optional properties, as shown in the list.

From my understanding the existing MarkAsComputedOnly property handles the fact that a resource property is erroneously marked as Optional,Computed, which implies that the property can be set by the practitioner but in fact the property is readonly in the target system. So MarkAsComputedOnly removes the Optional property and leaves the property readonly computed.

The proposed MarkAsComputed forcefully adds the Computed flag to a property where it has been omitted/missing, so that the value can be read after the deployment has finished. An example where this flag is helpful can be seen in this PR hashicorp/terraform-provider-azurerm#17632, where the computed flag has been removed by a careless refactoring in the AzureRM provider for the azurerm_application_gateway. The effect was that dependent resources couldn't read the assigned private ip address from the resource in following steps.

Because it's most of the time quicker to handle such errors directly in the wrapped Pulumi provider instead in the upstream TF provider, although this might contradict the spirit of open source, having the new flag MarkAsComputed is important. Whereas I already stated that it might be necessary to consolidate the various flags to reduce the potential of confusion and foster a concise meaning of the flags present.

CC @t0yv0

[t0yv0]

There's 4 kinds of attributes, Required, Optional, Computed, and Computed Optional. The latter are the most complex ones. I haven't had a look to dive deeper here but I can easily believe we might have some gap in switches that make this work as you'd expect if you want to change upstream attribute kind.

[tmeckel]

@t0yv0 That's what I meant: we might have the same flags in the bridge as in the Terraform SDK/PF so that we can override the settings made in the upstream provider. So perhaps MarkAsComputed and MarkAsComputedOnly should be removed entirely and we could introduce Required; Optional and Computed in the SchemaInfo struct. As a benefit we spare some documentation because we mimic the flags of Terraform and thus we can simply copy their documentation over.