pulumi / pulumi-terraform-bridge

A library allowing Terraform providers to be bridged into Pulumi.
Apache License 2.0
192 stars 45 forks source link

PF investigate support for PlanModifiers #2171

Closed VenelinMartinov closed 1 month ago

VenelinMartinov commented 1 month ago

What happened?

Looking at the code around PF Diff it doesn't look like PlanModifiers are used in ProposedNew anywhere - does that mean we ignore these?

Example

"vlan_id": schema.StringAttribute{
    MarkdownDescription: `VLAN ID`,
    Computed:            true,
    Optional:            true,
    PlanModifiers: []planmodifier.String{
        stringplanmodifier.UseStateForUnknown(),
    },
},

This should yield a different plan then when the PlanModifier is missing.

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).

t0yv0 commented 1 month ago

I expect plan modifiers to fully work, curious if you find a counter-example.

corymhall commented 1 month ago

I think that https://github.com/pulumi/pulumi-aws/issues/4273 is an example of this in pulumi-aws

VenelinMartinov commented 1 month ago

I think I have a repro: https://github.com/pulumi/pulumi-terraform-bridge/pull/2217

I'm assuming this is connected to https://github.com/pulumi/pulumi-terraform-bridge/issues/2218 and will investigate as part of that.

VenelinMartinov commented 1 month ago

Not 100% sure about this anymore - tried to repro in pulumi-random:

```python """A Python Pulumi program""" import pulumi import pulumi_random as random resNumeric = random.RandomString( "str1", length=10, numeric=False, ) resNumber = random.RandomString( "str2", length=10, number=False, ) pulumi.export("number val 1", resNumeric.number) pulumi.export("numeric val 1", resNumeric.numeric) pulumi.export("number val 2", resNumber.number) pulumi.export("numeric val 2", resNumber.numeric) ```

These properties have plan modifiers and they seem to be working: https://github.com/hashicorp/terraform-provider-random/blame/455294a2038f3cf39a31f8eda231b95f190df250/internal/provider/resource_string.go#L453

The above program outputs False, False, False, False

corymhall commented 1 month ago

I can't reproduce the AWS issue anymore either if I use the latest bridge@master so something must have been fixed (I can reproduce on latest aws which is on bridge@v3.87.0)

VenelinMartinov commented 1 month ago

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

https://github.com/pulumi/pulumi-terraform-bridge/pull/2217 will add a regression test here.