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

Support muxing with SDKv2 #22

Closed
paddycarver opened this issue May 14, 2021 · 3 comments · Fixed by #294
Closed

Support muxing with SDKv2 #22

paddycarver opened this issue May 14, 2021 · 3 comments · Fixed by #294
Assignees
Labels
enhancement New feature or request sdkv2-parity Issues tracking feature parity with terraform-plugin-sdk v2 and PRs working towards it.
Milestone

Comments

@paddycarver
Copy link
Contributor

Module version

N/A

Use-cases

Mux was born out of the desire to make sure that this new framework could be adopted piecemeal, that moving over wasn't a Big Bang rewrite project. That one resource at a time could be rewritten into or implemented in the new framework, and older SDKv2 code would continue to work.

Attempted Solutions

The fundamental problem is that SDKv2 uses protocol version 5, and this framework is being built on protocol version 6. On the face of it, that's not that big a deal; hashicorp/terraform-plugin-go-contrib#7 talks about a way to transparently upgrade providers built on version 5 of the protocol to version 6 of the protocol, without any loss of fidelity. So in theory an SDKv2 provider could be just wrapped in that helper, and then muxed with a provider using this framework as two protov6 providers. No big deal.

And this should, in theory, work fine.

As long as the provider configuration (the schema for the provider itself, not the resources) doesn't use any nested blocks or nested attributes.

Because both muxed providers need to respond to the same ConfigureProvider RPC to get their configuration, they need to agree on what the schema for that call is. SDKv2 is on protocol version 5, which has no concept of nested attributes. This framework is choosing not to expose sub-blocks, because we don't think they're needed anymore and are confusing for provider developers, so we're steering people away from them. This means there exist a great number of provider configuration schemas that can't be modeled by both SDKv2 and the framework, meaning those providers would not be able to be muxed.

We have a few options:

  1. We could relax the schema requirement; it's artificially imposed in mux. We could say "nested block and nested attribute are equivalent" and then just paper over how they're presented. This seems tricky, sub-optimal, and like it could have a bunch of bugs lurking.
  2. We could expose nested attributes in SDKv2. That would require it to move to protocol version 6, meaning it would only be compatible with Terraform 0.15 and later. I don't think provider developers would like that.
  3. We could expose nested blocks in this framework as part of the schema work in Define Schema type #12 / Start defining the Schema type. #11. This would be a shame, because it would carry forward a concept that we're trying to bury and guide people away from, and it may take us a while to get rid of it. It also lets them use blocks anywhere, not just in the provider schema, where they're needed.
  4. We could add a special, optional callback (GetMuxedProviderSchema or something) to the provider that allowed it to return a schema of a different type, with support for nested blocks. This would limit the impact to only the area needed for compatibility, and would let providers who don't need that compatibility (new providers, completely rewritten providers, etc.) avoid the cognitive overhead entirely.

Proposal

I think my personal preference is number 4, but I think that's a lightly held preference at this point. I think either 3 or 4 is the way to solve this, honestly.

@paddycarver paddycarver added the enhancement New feature or request label May 14, 2021
@bflad bflad self-assigned this Jan 11, 2022
@bflad bflad added this to the v0.6.0 milestone Jan 11, 2022
@bflad
Copy link
Member

bflad commented Jan 13, 2022

While a lot of the implementation work for this will likely happen in terraform-plugin-mux, I noticed that we may need to add a function or method that can return a tfprotov6.ProviderServer from a framework Provider. Not a big deal, just noting that for when the time comes to work on this issue.

@detro detro added the sdkv2-parity Issues tracking feature parity with terraform-plugin-sdk v2 and PRs working towards it. label Feb 3, 2022
@bflad bflad modified the milestones: v0.6.0, v0.7.0 Mar 10, 2022
bflad added a commit that referenced this issue Apr 19, 2022
Reference: #22

Provider developers have a few reasons to directly need a tfprotov6.ProviderServer implementation:

- When implementing acceptance testing with terraform-plugin-sdk helper/resource.
- When using terraform-plugin-mux tf6muxserver.NewMuxServer().
- When using terraform-plugin-mux tf6to5server.DowngradeServer().

The current `NewProtocol6Server(Provider)` function enables these use cases, however the implementation is less than ergonomic for provider developers as a wrapping function is required in all cases:

```go
// helper/resource acceptance testing

resource.TestCase{
	ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){
		"example": func() (tfprotov6.ProviderServer, error) {
			return tfsdk.NewProtocol6Server(New()), nil
		},
	},
 // ...
}

// tf6muxserver.NewMuxServer()

providers := []func() tfprotov6.ProviderServer{
	func() tfprotov6.ProviderServer {
		return tfsdk.NewProtocol6Server(provider1.New())
	},
	func() tfprotov6.ProviderServer {
		return tfsdk.NewProtocol6Server(provider2.New())
	},
}

muxServer, err := tf6muxserver.NewMuxServer(ctx, providers...)

// tf6to5server.DowngradeServer()

downgradeServer, err := tf6to5server.DowngradeServer(ctx, func() tfprotov6.ProviderServer {
	return tfsdk.NewProtocol6Server(provider.New())
})
```

The new functions simplify these implementations and get provider developers closer to not directly importing terraform-plugin-go (with a few other targeted changes, such as type aliases), e.g.

```go
// helper/resource acceptance testing

resource.TestCase{
	ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){
		"example": tfsdk.NewProtocol6ProviderServerWithError(New()),
	},
 // ...
}

// tf6muxserver.NewMuxServer()

providers := []func() tfprotov6.ProviderServer{
	tfsdk.NewProtocol6ProviderServer(provider1.New()),
  tfsdk.NewProtocol6ProviderServer(provider2.New()),
}

muxServer, err := tf6muxserver.NewMuxServer(ctx, providers...)

// tf6to5server.DowngradeServer()

downgradeServer, err := tf6to5server.DowngradeServer(ctx, tfsdk.NewProtocol6ProviderServer(provider.New()))
```

This change prefers deprecation over straight replacement to give some lead time for various documentation updates across multiple projects.

The naming is less than ideal, however it feels necessary to remain generic instead of picking any particular details related to their current usage (e.g. "NewTestServer") as this can/will change over time. The "Protocol6" naming is important should a new major protocol version 7 be released, which this framework must support.
bflad added a commit that referenced this issue Apr 21, 2022
Reference: #22

Provider developers have a few reasons to directly need a tfprotov6.ProviderServer implementation:

- When implementing acceptance testing with terraform-plugin-sdk helper/resource.
- When using terraform-plugin-mux tf6muxserver.NewMuxServer().
- When using terraform-plugin-mux tf6to5server.DowngradeServer().

The current `NewProtocol6Server(Provider)` function enables these use cases, however the implementation is less than ergonomic for provider developers as a wrapping function is required in all cases:

```go
// helper/resource acceptance testing

resource.TestCase{
	ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){
		"example": func() (tfprotov6.ProviderServer, error) {
			return tfsdk.NewProtocol6Server(New()), nil
		},
	},
 // ...
}

// tf6muxserver.NewMuxServer()

providers := []func() tfprotov6.ProviderServer{
	func() tfprotov6.ProviderServer {
		return tfsdk.NewProtocol6Server(provider1.New())
	},
	func() tfprotov6.ProviderServer {
		return tfsdk.NewProtocol6Server(provider2.New())
	},
}

muxServer, err := tf6muxserver.NewMuxServer(ctx, providers...)

// tf6to5server.DowngradeServer()

downgradeServer, err := tf6to5server.DowngradeServer(ctx, func() tfprotov6.ProviderServer {
	return tfsdk.NewProtocol6Server(provider.New())
})
```

The new functions simplify these implementations and get provider developers closer to not directly importing terraform-plugin-go (with a few other targeted changes, such as type aliases), e.g.

```go
// helper/resource acceptance testing

resource.TestCase{
	ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){
		"example": tfsdk.NewProtocol6ProviderServerWithError(New()),
	},
 // ...
}

// tf6muxserver.NewMuxServer()

providers := []func() tfprotov6.ProviderServer{
	tfsdk.NewProtocol6ProviderServer(provider1.New()),
  tfsdk.NewProtocol6ProviderServer(provider2.New()),
}

muxServer, err := tf6muxserver.NewMuxServer(ctx, providers...)

// tf6to5server.DowngradeServer()

downgradeServer, err := tf6to5server.DowngradeServer(ctx, tfsdk.NewProtocol6ProviderServer(provider.New()))
```

This change prefers deprecation over straight replacement to give some lead time for various documentation updates across multiple projects.

The naming is less than ideal, however it feels necessary to remain generic instead of picking any particular details related to their current usage (e.g. "NewTestServer") as this can/will change over time. The "Protocol6" naming is important should a new major protocol version 7 be released, which this framework must support.
@bflad
Copy link
Member

bflad commented Apr 21, 2022

As of v0.6.1, all the underlying pieces for muxing with SDKv2 are in place. Refer to Combining and Translating Providers documentation around the various terraform-plugin-mux packages which can be used to mix and match protocol version 5 and version 6 provider servers.

#158 will be used for tracking the creation of a specific guide for migrating from SDKv2 -> framework, with optionally using a mux implementation during the process to migrate individual resources at a time.

Framework providers in v0.6.1 can use the following for mux implementations:

// tf6muxserver.NewMuxServer()

providers := []func() tfprotov6.ProviderServer{
	func() tfprotov6.ProviderServer {
		return tfsdk.NewProtocol6Server(provider1.New())
	},
	func() tfprotov6.ProviderServer {
		return tfsdk.NewProtocol6Server(provider2.New())
	},
}

muxServer, err := tf6muxserver.NewMuxServer(ctx, providers...)

// tf6to5server.DowngradeServer()

downgradeServer, err := tf6to5server.DowngradeServer(ctx, func() tfprotov6.ProviderServer {
	return tfsdk.NewProtocol6Server(provider.New())
})

As of the upcoming v0.7.0, its slightly simplified so framework providers can use the following for mux implementations:

// tf6muxserver.NewMuxServer()

providers := []func() tfprotov6.ProviderServer{
	tfsdk.NewProtocol6ProviderServer(provider1.New()),
	tfsdk.NewProtocol6ProviderServer(provider2.New()),
}

muxServer, err := tf6muxserver.NewMuxServer(ctx, providers...)

// tf6to5server.DowngradeServer()

downgradeServer, err := tf6to5server.DowngradeServer(ctx, tfsdk.NewProtocol6ProviderServer(provider.New()))

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request sdkv2-parity Issues tracking feature parity with terraform-plugin-sdk v2 and PRs working towards it.
Projects
None yet
3 participants