Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split out duplicate code between here and plugin-sdk into common module #23725

Closed
LorbusChris opened this issue Dec 19, 2019 · 5 comments
Closed

Comments

@LorbusChris
Copy link

LorbusChris commented Dec 19, 2019

Use-cases

Importing and using the terraform-plugin-sdk into a project alongside terraform causes panics in gob and proto when the RPC types that are duplicated between those two repos are attempted to be initialized for a second time (namely tfdiags.rpcFriendlyDiag and all proto-generated types from tfplugin5):

panic: gob: registering duplicate types for "*tfdiags.rpcFriendlyDiag": *tfdiags.rpcFriendlyDiag != *tfdiags.rpcFriendlyDiag
goroutine 1 [running]:
encoding/gob.RegisterName(0x84498da, 0x18, 0x94d9180, 0x0)
	/usr/local/go/src/encoding/gob/type.go:820 +0x55e
encoding/gob.Register(0x94d9180, 0x0)
	/usr/local/go/src/encoding/gob/type.go:874 +0x125
github.com/hashicorp/terraform-plugin-sdk/internal/tfdiags.init.0()
	/go/src/github.com/openshift/installer/vendor/github.com/hashicorp/terraform-plugin-sdk/internal/tfdiags/rpc_friendly.go:58 +0x36 
panic: proto: duplicate enum registered: tfplugin5.Diagnostic_Severity

goroutine 1 [running]:
github.com/golang/protobuf/proto.RegisterEnum(...)
        /go/pkg/mod/github.com/golang/protobuf@v1.3.2/proto/properties.go:458
github.com/hashicorp/terraform-plugin-sdk/internal/tfplugin5.init.0()

While I've been made aware that this is generally not a supported use-case, for us over at OpenShift Installer this is a regression from previous behaviour after migrating to the sdk, and makes it impossible for us to import terraform-plugin-sdk there as-is.

Attempted Solutions

Renaming the types in terraform-plugin-sdk does not work.

Proposal

Two alternative solutions are proposed:

  1. Split out code shared between terraform and terraform-plugin-sdk into a common module that would be imported both here and in terraform-plugin-sdk.

  2. a. Ensure tfdiags and tfplugin5 are public packages.
    b. Import tfdiags and tfplugin5 into terraform-plugin-sdk from here, or the other way around (from plugin-sdk into this repo).

References

hashicorp/terraform-plugin-sdk#277

@LorbusChris
Copy link
Author

@danieldreier @apparentlymart could I ask for your feedback on the proposal above?

The goal of this is to dedupe code between terraform and terraform-plugin-sdk, as the RPC types in that duplicated code can't both be instantiated when using them in the same project, and should hence only be instantiated once from one canonical location (as described above).

@danieldreier
Copy link
Contributor

@LorbusChris as I think you're aware of, the fact that terraform can be imported and used as a library is essentially an accident. Martin wrote a discussion forum post that "there is no supported in-process Go library API for Terraform", and that's the current official stance. We maintain stable interfaces via the API, and via HCL, but internally, Terraform is subject to significant changes like you've seen, and we don't (currently) consider those bugs.

That said, I've put this on our backlog for internal discussion between product and engineering. I'm very empathetic to why people want to vendor and drive Terraform programmatically, and at the same time it's hard to justify potential breakage of primary workflows and engineering time on what is currently an unsupported use case.

In support of that discussion, can you help me understand why the OpenShift installer vendors Terraform at all, rather than shelling out to it? I haven't read the OpenShift installer code - are you trying to avoid distributing a second binary? Are you trying to avoid writing HCL and driving provisioning via go directly?

@wking
Copy link

wking commented Apr 29, 2020

I haven't read the OpenShift installer code - are you trying to avoid distributing a second binary? Are you trying to avoid writing HCL and driving provisioning via go directly?

We write HCL. Terraform is vendored so we don't have to ship a separate binary and plugin set, because a single binary is more convenient for users, and we had past trouble trying to talk folks through getting a compatible terraform installed.

@danieldreier
Copy link
Contributor

@LorbusChris I'm going to close this issue because it's not a change we are planning on making; all the options we've discussed for officially supported ways to integrate with Terraform will be based on the binary, rather than using it as a go library.

@ghost
Copy link

ghost commented Nov 16, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked as resolved and limited conversation to collaborators Nov 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants