search

pulumi / pulumi-terraform-bridge

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

Reconsider TF plugin SDK fork #1956

Open VenelinMartinov opened 4 months ago

VenelinMartinov commented 4 months ago

Hello!

Issue details

After discussing with @t0yv0, he suggested we might no longer need the plugin sdk fork. Looking at the diff the following changes look eventful:

https://github.com/pulumi/terraform-plugin-sdk/commit/53f910a1ea61564de1cae8f2e71d64ce4ecaf655 (fixed) -> this one is the most interesting, related to 64 bit int support and timeouts. Perhaps no longer needed? We should carefully test ths.

https://github.com/pulumi/terraform-plugin-sdk/commit/e2a20ae13ef98738ebac0566e9ed25f570226e0a -> this is an optional optimisation, perhaps we can work that into the providers which need it/ into the bridge with some golink magic?

https://github.com/pulumi/terraform-plugin-sdk/commit/74776a5cd5f9a1330c34124588e0ad800d26724d

The rest of the changes are mostly related to exposing internal functions, which we could put into a separate library.

Affected area/feature

t0yv0 commented 4 months ago

In my latest understanding 74776a5 exposing provider internals is actually a little unfortunate, primary motivation was to integrate with as-is detailedDiff computation but with https://github.com/pulumi/pulumi-terraform-bridge/issues/1895 we should be able to write that correctly now and remove the need for 74776a5.

iwahbe commented 4 months ago

https://github.com/pulumi/terraform-plugin-sdk/commit/53f910a1ea61564de1cae8f2e71d64ce4ecaf655 has been removed, since it was causing bugs in the bridge.

iwahbe commented 3 months ago

We should make sure to update https://github.com/pulumi/pulumi-terraform-bridge/blob/master/docs/new-provider.md#githubcompulumiterraform-plugin-sdkv2 when we close this issue.

VenelinMartinov commented 1 month ago

This is blocked on https://github.com/orgs/pulumi/projects/165?pane=issue&itemId=73070471 - we need to change the diff implementation to not require TF internals in order to get rid of the plugin-sdk changes which expose that.

t0yv0 commented 1 month ago

Agreed, it's best left for after.

t0yv0 commented 1 week ago

Curious if we got a little closer to being able to do this.

VenelinMartinov commented 1 week ago

Yeah, the new diff does not depend on TF Diff internals - once that is rolled out we should be able to revert some of the changes to our fork of the plugin-sdk.