Move PF HCL write and add resource writing utilites

[VenelinMartinov]

This PR moves the existing PF HCL write utilities to the PF folder and adds write utilities for PF resources. Note that the duplication is necessary because PF has different schema types for provider schema and resource schema.

[VenelinMartinov]

This change is part of the following stack:

• 2577 ◀

  • 2578

    • 2584

_{Change managed by git-spice.}

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 73.91304% with 30 lines in your changes missing coverage. Please review.

Project coverage is 62.83%. Comparing base (ae9cd89) to head (001c582). Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
pkg/pf/tests/internal/cross-tests/tfwrite.go	72.22%	26 Missing and 4 partials :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #2577 +/- ## ======================================= Coverage 62.82% 62.83% ======================================= Files 386 387 +1 Lines 51666 51717 +51 ======================================= + Hits 32459 32495 +36 - Misses 17384 17397 +13 - Partials 1823 1825 +2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[VenelinMartinov]

We already had the duplication between PF and SDKv2 before this PR. I am not sure why we chose not to use the shim abstraction for writing HCL - there might be something missing there indeed. Will investigate.

Should we block the work here on that or can we do that as a follow-up?

[VenelinMartinov]

Actually, the Attribute/Block distinction is missing in the shim layer and we need that for the HCL writing. I believe that's why we decided not to use the shim layer here:

https://github.com/pulumi/pulumi-terraform-bridge/blob/de1e97ceffa0623258051c50b878e53d0fabbd6e/pkg/internal/tests/cross-tests/tfwrite.go#L73

https://github.com/pulumi/pulumi-terraform-bridge/blob/de1e97ceffa0623258051c50b878e53d0fabbd6e/pkg/pf/tests/internal/cross-tests/tfwrite.go#L42-L45

[VenelinMartinov]

The cost of duplication here seems small as the code is only used for testing. Abstracting this also seems non-trivial as we'd have to reverse the various ways PF and SDKv2 can output Blocks in the shim layer.

[iwahbe]

I'm skeptical if we need to move stuff here if it results in duplication. Duplication in tests is still additional code we need to maintain.

[iwahbe]

Have you considered depending on the abstraction layer, such as shim.Provider, instead of duplicating tfwrite code across PF and SDKv2?

I'm hesitant to go in that direction. Part of what we are testing here is the ability to map between shim.Provider and the underlying PF/SDKv2 types.

[t0yv0]

We already had the duplication between PF and SDKv2 before this PR

This is indeed the case, but this is design debt that was taken on under time pressure (I can explain on a quick call).

The cost of duplication here seems small as the code is only used for testing.

This is indeed true and we can chose to move forward here on pragmatic grounds, but remember that having a lot more casual contributions over time means that duplication keeps growing.

I perhaps naively was hoping that we can instead move in the reverse direction, of making Plugin Framework fit "just" another shim-like abstraction so the codebase is straightened up structurally, but that's a higher-level conversation.

What I would love to see is that cross-tests don't care how the provider was built. So crosstests.Create() can take any provider (PF, SDKv2, hybrid, no matter). That'd be taking shim.Provider (possibly extended) as a parameter. I recall briefly jumping in the PF code for tests and finding there's some wrinkles that make it extra annoying. But it sounded like something that can be removed with 2-3 hours of intentional effort.

I am guessing I am very naive here, but it would be really helpful to get educated on why it's not possible or would take more time 🙏 We can chose to move forward with this PR as-is if we think it is too hard, but maybe consider backlogging?

[t0yv0]

I'm hesitant to go in that direction. Part of what we are testing here is the ability to map between shim.Provider and the underlying PF/SDKv2 types.

So, does the shim layer hide some schema information that is necessary to figure out how to write an HCL file? Is this about something being a block or an attribute? We hesitate adding this information back in the shim layer?

[iwahbe]

What I would love to see is that cross-tests don't care how the provider was built.

Cross-tests care deeply how the provider was built. They hook into the decision making of the provider by comparing what the provider implementor was given. We could wrap that, but I think that's more trouble then it's worth now.

[t0yv0]

Thanks for catching up and elucidating! I think what I'm hearing is that: cross-test implementation needs to instrument specifics of SDKv2 or PF so necessarily is specific, therefore we proceed, with having separate code for it, and that can include minor duplication as necessary here. On the other hand the interface presented to the test writers is something that's converging (but out of scope on this PR).

[pulumi-bot]

This PR has been shipped in release v3.95.0.