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

Wire ApplyResourceChange/PlanResourceChange RPC legacy_type_system Field Through Protocol 6 #30373

Closed
bflad opened this issue Jan 19, 2022 · 2 comments · Fixed by #30375
Closed
Labels
enhancement new new issue not yet triaged proposal providers/protocol Potentially affecting the Providers Protocol and SDKs

Comments

@bflad
Copy link
Contributor

bflad commented Jan 19, 2022

Current Terraform Version

v1.1.3

Use-cases

In the near future, it will be possible to use github.com/hashicorp/terraform-plugin-mux to route resource and data source RPCs between github.com/hashicorp/terraform-plugin-sdk/v2 (protocol version 5) and github.com/hashicorp/terraform-plugin-framework (protocol version 6) providers within one provider server. Since all muxed servers must be on the same protocol version, this requires either upgrading sdk/v2 to protocol version 6 or downgrading framework to protocol version 5. Either which way, this enables a migration path for moving away from the current SDK onto a new SDK.

Terraform CLI 0.12 and later introduced a new type system which implemented null values, while the existing SDK (what is now sdk/v2) was still implemented in a manner reliant on Go zero values. There are consistency checks implemented within Terraform CLI to prevent unexpected differences between planned and applied values, since these changed values would not propagate correctly throughout other usages until later applies. Updating the underlying type system in that SDK was problematic, so a workaround was implemented in the protocol (intended only for that particular SDK) to downgrade these checks from error diagnostics in the user interface to warning log messages.

The implementation of this can be seen in version 5 of the plugin protocol, which contains the following fields:

message ApplyResourceChange {
    // ... other messages ...

    message Response {
        // ... other fields ...

        // This may be set only by the helper/schema "SDK" in the main Terraform
        // repository, to request that Terraform Core >=0.12 permit additional
        // inconsistencies that can result from the legacy SDK type system
        // and its imprecise mapping to the >=0.12 type system.
        // The change in behavior implied by this flag makes sense only for the
        // specific details of the legacy SDK type system, and are not a general
        // mechanism to avoid proper type handling in providers.
        //
        //     ====              DO NOT USE THIS              ====
        //     ==== THIS MUST BE LEFT UNSET IN ALL OTHER SDKS ====
        //     ====              DO NOT USE THIS              ====
        bool legacy_type_system = 4;
    }
}

message PlanResourceChange {
    // ... other messages ...

    message Response {
        // ... other fields ...

        // This may be set only by the helper/schema "SDK" in the main Terraform
        // repository, to request that Terraform Core >=0.12 permit additional
        // inconsistencies that can result from the legacy SDK type system
        // and its imprecise mapping to the >=0.12 type system.
        // The change in behavior implied by this flag makes sense only for the
        // specific details of the legacy SDK type system, and are not a general
        // mechanism to avoid proper type handling in providers.
        //
        //     ====              DO NOT USE THIS              ====
        //     ==== THIS MUST BE LEFT UNSET IN ALL OTHER SDKS ====
        //     ====              DO NOT USE THIS              ====
        bool legacy_type_system = 5;
    }
}

While protocol version 6 was intended to be fully forward compatible with version 5, it does not contain the same fields. Therefore, it is currently only possible to mux sdk/v2 and framework as a protocol version 5 server without unintended consequences. To upgrade to protocol version 6 and take advantage of the new protocol features (e.g. nested attributes), all sdk/v2 usage would need to be removed first.

Attempted Solutions

Not supporting mux of sdk/v2 and framework. This would require the provider ecosystem to "one shot" migrate between frameworks. This introduces a poor provider developer experience as all resources and data sources would need to be migrated together, in a theoretical major version upgrade. This introduces a poor practitioner experience as Terraform CLI 1.0 or later would be required for the upgrade.

Not supporting protocol version 5 to version 6 upgrades, but supporting version 6 to version 5 downgrades. This would allow muxing between sdk/v2 and framework, but only with version 5 features. While the new framework could then be used to develop new resources and data sources, this requires the continued usages of only version 5 features until all resources and data sources are converted.

Not supporting protocol version 6 to version 5 downgrades, but supporting version 5 to version 6 upgrades. This would allow muxing between sdk/v2 and framework, but practitioners would be required to use Terraform CLI 1.0 or later and sdk/v2 resources and data sources will not work as anticipated.

Proposal

Forward port the legacy_type_system fields into protocol version 6 and wire the LegacyTypeSystem handling through internal/tfplugin6, similar to internal/tfplugin5.

Ideally, this would also be backported to Terraform CLI 1.1 and 1.0 to allow practitioners upgrading to the latest version of 1.0 on their upgrade journey to not see unintentional muxed provider issues. sdk/v2 could theoretically detect unsupported Terraform CLI versions and return an early error if the protocol version is made available in the server context, however that implementation detail is out of scope of this proposal.

Once the protobuf file is finalized, it can be copied, and an updated release of github.com/hashicorp/terraform-plugin-go/tfprotov6 response types could be used by github.com/hashicorp/terraform-plugin-mux in its protocol version 5 to version 6 converter, passing through the value received by the existing SDK onto the wire.

References

@bflad bflad added enhancement proposal new new issue not yet triaged providers/protocol Potentially affecting the Providers Protocol and SDKs labels Jan 19, 2022
@apparentlymart
Copy link
Member

Thanks for sharing this, @bflad!

This seems like a good pragmatic compromise to me. We can preserve a similar "scary comment" in the new schema to warn away potential use of it in other SDKs, and then leave it in place for the foreseeable future since Terraform Core will need to keep supporting it at least through the v1.x series anyway, in order to be true to the v1.0 Compatibility Promises where we promised not to remove protocol version 5 support during those releases. The logic must be there to support tfplugin5 providers anyway, so having it also work for tfplugin6 is only a small incremental maintenance cost.

The v1.0.x series is closed at this point, and so I wouldn't expect to backport this to v1.0 unless there's a strong benefit to doing so. Our intent (represented by the v1.0 Compatibility Promises) is that it should be safe and easy to always upgrade to the latest v1.x release without hopping through prior minor releases first, and so anyone who would hypothetically find themselves blocked from using the latest v1.0 release should be able to use the latest v1.1 or later release instead, as long as they are upgrading from at least a v0.14.x release. (If that isn't true then it'd be a bug for us to fix, unless it happened to coincide with one of the pragmatic compatibility exception cases, which should be rare.)

bflad added a commit that referenced this issue Jan 19, 2022
…s from Protocol 5

Reference: #30373

This change forward ports the `legacy_type_system` boolean fields in the `ApplyResourceChange.Response` and `PlanResourceChange.Response` messages that existed in protocol version 5, so that existing terraform-plugin-sdk/v2 providers can be muxed with protocol version 6 providers (e.g. terraform-plugin-framework) while also taking advantage of the newer protocol features. This functionality should not be used by any providers or SDKs except those built with terraform-plugin-sdk.

Updated via:

```shell
cp docs/plugin-protocol/tfplugin6.1.proto docs/plugin-protocol/tfplugin6.2.proto
# Copy legacy_type_system fields from tfplugin5.2.proto into ApplyResourceChange.Response and PlanResourceChange
rm internal/tfplugin6/tfplugin6.proto
ln -s ../../docs/plugin-protocol/tfplugin6.2.proto internal/tfplugin6/tfplugin6.proto
go run tools/protobuf-compile/protobuf-compile.go `pwd`
# Updates to internal/plugin6/grpc_provider.go
```
bflad added a commit that referenced this issue Jan 20, 2022
…s from Protocol 5 (#30375)

Reference: #30373

This change forward ports the `legacy_type_system` boolean fields in the `ApplyResourceChange.Response` and `PlanResourceChange.Response` messages that existed in protocol version 5, so that existing terraform-plugin-sdk/v2 providers can be muxed with protocol version 6 providers (e.g. terraform-plugin-framework) while also taking advantage of the newer protocol features. This functionality should not be used by any providers or SDKs except those built with terraform-plugin-sdk.

Updated via:

```shell
cp docs/plugin-protocol/tfplugin6.1.proto docs/plugin-protocol/tfplugin6.2.proto
# Copy legacy_type_system fields from tfplugin5.2.proto into ApplyResourceChange.Response and PlanResourceChange
rm internal/tfplugin6/tfplugin6.proto
ln -s ../../docs/plugin-protocol/tfplugin6.2.proto internal/tfplugin6/tfplugin6.proto
go run tools/protobuf-compile/protobuf-compile.go `pwd`
# Updates to internal/plugin6/grpc_provider.go
```
@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 Feb 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement new new issue not yet triaged proposal providers/protocol Potentially affecting the Providers Protocol and SDKs
Projects
None yet
2 participants