PF defaults not applied

[VenelinMartinov]

What happened?

It seems like for some cases defaults in PF schema are not applied to resource properties.

Quite surprising, happy to be pointed to any errors in the test here.

Found during an attempt at reproing a customer issue with a private PF provider.

Example

#2217

Output of pulumi about

.

Additional context

No response

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

[VenelinMartinov]

here are some details from the GRPC calls: change_reason is not mentioned at all:

/pulumirpc.ResourceProvider/Check
{"urn":"urn:pulumi:test::test::prov:index/test:Test::mainRes","olds":{},"news":{"otherProp":"val"},"randomSeed":"JiYdqeul7IEPc4bsh2+Nf751Pg+yyMa/lcUf1A06Obk="}
{"inputs":{"otherProp":"val"}}

/pulumirpc.ResourceProvider/Create
{"urn":"urn:pulumi:test::test::prov:index/test:Test::mainRes","properties":{"otherProp":"val"}}
{"id":"test-id","properties":{"id":"test-id","otherProp":"val"}}

[VenelinMartinov]

@t0yv0 provided some possible context here - PF initially did not support Defaults at all at the time when the PF bridge was initially developed. It relied entirely on PlanModifiers.

Defaults were added later so it is possible something on the TF side changed in the meantime.

[t0yv0]

Still the expectation is that this is handled by PlanResourceChange call to the PF itself, so curious to see where the bug is here.

[VenelinMartinov]

#2225 adds tests for input extraction which is also affected by this issue.

We seem to generate inputs for properties which are using their default values, which is not the intention of the code and is not what the SDKv2 does. We should address as part of this issue.

[t0yv0]

Could you please break out the input extraction interaction with PF Defaults into a separate issue, thanks.

[VenelinMartinov]

Yup, #2238

[VenelinMartinov]

Did a bunch of digging here. The default values are present during planning but go missing after ApplyResourceChange:

pulumi-terraform-bridge/pf/tfbridge/provider_create.go

Line 97 in 18db7c5

resp, err := p.tfServer.ApplyResourceChange(ctx, &req)

I did some comparison with OpenTofu, here's where the equivalent thing is happening on their side:
https://github.com/opentofu/opentofu/blob/cb2e9119aa75eeb8e1fa175e2b7a205a4fef129f/internal/tofu/node_resource_apply_instance.go#L311

and

https://github.com/opentofu/opentofu/blob/864aa9d1d629090cfc4ddf9fdd344d34dee9793e/internal/tofu/node_resource_abstract_instance.go#L2345

It seems like they always pass cty.Null values in the ConfigValue for the properties in question and Apply sets them.

I have been unable to find a TF PF resource which has a Default and is not Computed, so not certain how much of this is due to Computed.

Either way, we still don't pass Null for Computed properties either, which is a clear discrepency.

[VenelinMartinov]

Opened #2244 for cross testing PF inputs, that would have likely caught this.

[t0yv0]

I don't quite follow.

1. is the behavior under OpenTofu/TF different from the bridged provider behavior?
2. if we were to isolate the place where the discrepancy first starts manifesting, where would it be?

[t0yv0]

I'm wondering if objchange refactor is going to affect this as well.

[VenelinMartinov]

I don't think objchange would affect this as the plan for the property in question is correct.

The problem only happens during apply

[VenelinMartinov]

OpenTofu sets defaults, we don't. This is a clear difference in behaviour.

The default does not get return from Apply in the bridge, while in opentofu it does. However, I believe the issue is happening earlier as we pass different values to Apply for the Config - they set Nulls, we don't.

[t0yv0]

While it's possible to workaround this it sounds like this is highly unexpected and the impact can be quite significant. Marking as P1.

[VenelinMartinov]

This turned out to be a red herring - the test was set up wrong - defaults are applied fine as long as the provider is implemented correctly to apply the plan, not the config:

#2217 will add a regression test here.