From 62ca775550c6c64a056b314e8a3ecd491afb5e03 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Mon, 22 Apr 2024 09:49:30 -0400 Subject: [PATCH] tfprotov5+tfprotov6: Require FunctionServer in ProviderServer and MoveResourceState in ResourceServer (#388) Reference: https://github.com/hashicorp/terraform-plugin-go/issues/353 Reference: https://github.com/hashicorp/terraform-plugin-go/issues/363 This removes the temporary handling to smoothly release the new Terraform 1.8 and later RPC handling for provider servers. These changes codify this Go module's desired design that it reflects the protocol via its required implementations. --- .../BREAKING CHANGES-20240311-093208.yaml | 7 ++ .../BREAKING CHANGES-20240311-093436.yaml | 7 ++ .../unreleased/NOTES-20240311-093736.yaml | 8 +++ tfprotov5/provider.go | 5 +- tfprotov5/resource.go | 16 ++++- tfprotov5/tf5server/server.go | 71 +------------------ tfprotov6/provider.go | 5 +- tfprotov6/resource.go | 16 ++++- tfprotov6/tf6server/server.go | 71 +------------------ 9 files changed, 56 insertions(+), 150 deletions(-) create mode 100644 .changes/unreleased/BREAKING CHANGES-20240311-093208.yaml create mode 100644 .changes/unreleased/BREAKING CHANGES-20240311-093436.yaml create mode 100644 .changes/unreleased/NOTES-20240311-093736.yaml diff --git a/.changes/unreleased/BREAKING CHANGES-20240311-093208.yaml b/.changes/unreleased/BREAKING CHANGES-20240311-093208.yaml new file mode 100644 index 000000000..b1bc1c718 --- /dev/null +++ b/.changes/unreleased/BREAKING CHANGES-20240311-093208.yaml @@ -0,0 +1,7 @@ +kind: BREAKING CHANGES +body: 'tfprotov5+tfprotov6: `FunctionServer` interface is now required in `ProviderServer`. + Implementations not needing function support can return errors from the `GetFunctions` + and `CallFunction` methods.' +time: 2024-03-11T09:32:08.806864-04:00 +custom: + Issue: "388" diff --git a/.changes/unreleased/BREAKING CHANGES-20240311-093436.yaml b/.changes/unreleased/BREAKING CHANGES-20240311-093436.yaml new file mode 100644 index 000000000..843a107d6 --- /dev/null +++ b/.changes/unreleased/BREAKING CHANGES-20240311-093436.yaml @@ -0,0 +1,7 @@ +kind: BREAKING CHANGES +body: 'tfprotov5+tfprotov6: `MoveResourceState` method is now required in + `ResourceServer`. Implementations not needing move state support can return + errors from the `MoveResourceState` method.' +time: 2024-03-11T09:34:36.800884-04:00 +custom: + Issue: "388" diff --git a/.changes/unreleased/NOTES-20240311-093736.yaml b/.changes/unreleased/NOTES-20240311-093736.yaml new file mode 100644 index 000000000..7eccf67bf --- /dev/null +++ b/.changes/unreleased/NOTES-20240311-093736.yaml @@ -0,0 +1,8 @@ +kind: NOTES +body: 'all: To prevent compilation errors, ensure your Go module is updated to at + least terraform-plugin-framework@v1.6.0, terraform-plugin-mux@v0.15.0, + terraform-plugin-sdk/v2@v2.33.0, and terraform-plugin-testing@v1.7.0 before + upgrading this dependency.' +time: 2024-03-11T09:37:36.503168-04:00 +custom: + Issue: "388" diff --git a/tfprotov5/provider.go b/tfprotov5/provider.go index fa85a8e04..8530353fe 100644 --- a/tfprotov5/provider.go +++ b/tfprotov5/provider.go @@ -53,10 +53,7 @@ type ProviderServer interface { // are a handy interface for defining what a function is to // terraform-plugin-go, so they are their own interface that is composed // into ProviderServer. - // - // This will be required in an upcoming release. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/353 - // FunctionServer + FunctionServer } // GetMetadataRequest represents a GetMetadata RPC request. diff --git a/tfprotov5/resource.go b/tfprotov5/resource.go index 3090d298a..cc54334e4 100644 --- a/tfprotov5/resource.go +++ b/tfprotov5/resource.go @@ -52,14 +52,24 @@ type ResourceServer interface { // specified by the passed ID and return it as one or more resource // states for Terraform to assume control of. ImportResourceState(context.Context, *ImportResourceStateRequest) (*ImportResourceStateResponse, error) + + // MoveResourceState is called when Terraform is asked to change a resource + // type for an existing resource. The provider must accept the change as + // valid by ensuring the source resource type, schema version, and provider + // address are compatible to convert the source state into the target + // resource type and latest state version. + // + // This functionality is only supported in Terraform 1.8 and later. The + // provider must have enabled the MoveResourceState server capability to + // enable these requests. + MoveResourceState(context.Context, *MoveResourceStateRequest) (*MoveResourceStateResponse, error) } // ResourceServerWithMoveResourceState is a temporary interface for servers // to implement MoveResourceState RPC handling. // -// Deprecated: The MoveResourceState method will be moved into the -// ResourceServer interface and this interface will be removed in a future -// version. +// Deprecated: This interface will be removed in a future version. Use +// ResourceServer instead. type ResourceServerWithMoveResourceState interface { ResourceServer diff --git a/tfprotov5/tf5server/server.go b/tfprotov5/tf5server/server.go index feb359620..29bbc6a41 100644 --- a/tfprotov5/tf5server/server.go +++ b/tfprotov5/tf5server/server.go @@ -900,37 +900,11 @@ func (s *server) MoveResourceState(ctx context.Context, protoReq *tfplugin5.Move logging.ProtocolTrace(ctx, "Received request") defer logging.ProtocolTrace(ctx, "Served request") - // Remove this check and error in preference of - // s.downstream.MoveResourceState below once ResourceServer interface - // implements the MoveResourceState method. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/363 - // nolint:staticcheck - resourceServerWMRS, ok := s.downstream.(tfprotov5.ResourceServerWithMoveResourceState) - - if !ok { - logging.ProtocolError(ctx, "ProviderServer does not implement ResourceServerWithMoveResourceState") - - protoResp := &tfplugin5.MoveResourceState_Response{ - Diagnostics: []*tfplugin5.Diagnostic{ - { - Severity: tfplugin5.Diagnostic_ERROR, - Summary: "Provider Move Resource State Not Implemented", - Detail: "A MoveResourceState call was received by the provider, however the provider does not implement the call. " + - "Either upgrade the provider to a version that implements move resource state support or this is a bug in Terraform that should be reported to the Terraform maintainers.", - }, - }, - } - - return protoResp, nil - } - req := fromproto.MoveResourceStateRequest(protoReq) ctx = tf5serverlogging.DownstreamRequest(ctx) - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/363 - // resp, err := s.downstream.MoveResourceState(ctx, req) - resp, err := resourceServerWMRS.MoveResourceState(ctx, req) + resp, err := s.downstream.MoveResourceState(ctx, req) if err != nil { logging.ProtocolError(ctx, "Error from downstream", map[string]interface{}{logging.KeyError: err}) @@ -954,26 +928,6 @@ func (s *server) CallFunction(ctx context.Context, protoReq *tfplugin5.CallFunct logging.ProtocolTrace(ctx, "Received request") defer logging.ProtocolTrace(ctx, "Served request") - // Remove this check and error in preference of s.downstream.CallFunction - // below once ProviderServer interface requires FunctionServer. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/353 - functionServer, ok := s.downstream.(tfprotov5.FunctionServer) - - if !ok { - logging.ProtocolError(ctx, "ProviderServer does not implement FunctionServer") - - text := "Provider Functions Not Implemented: A provider-defined function call was received by the provider, however the provider does not implement functions. " + - "Either upgrade the provider to a version that implements provider-defined functions or this is a bug in Terraform that should be reported to the Terraform maintainers." - - protoResp := &tfplugin5.CallFunction_Response{ - Error: &tfplugin5.FunctionError{ - Text: text, - }, - } - - return protoResp, nil - } - req := fromproto.CallFunctionRequest(protoReq) for position, argument := range req.Arguments { @@ -982,9 +936,7 @@ func (s *server) CallFunction(ctx context.Context, protoReq *tfplugin5.CallFunct ctx = tf5serverlogging.DownstreamRequest(ctx) - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/353 - // resp, err := s.downstream.CallFunction(ctx, req) - resp, err := functionServer.CallFunction(ctx, req) + resp, err := s.downstream.CallFunction(ctx, req) if err != nil { logging.ProtocolError(ctx, "Error from downstream", map[string]any{logging.KeyError: err}) @@ -1007,28 +959,11 @@ func (s *server) GetFunctions(ctx context.Context, protoReq *tfplugin5.GetFuncti logging.ProtocolTrace(ctx, "Received request") defer logging.ProtocolTrace(ctx, "Served request") - // Remove this check and response in preference of s.downstream.GetFunctions - // below once ProviderServer interface requires FunctionServer. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/353 - functionServer, ok := s.downstream.(tfprotov5.FunctionServer) - - if !ok { - logging.ProtocolWarn(ctx, "ProviderServer does not implement FunctionServer") - - protoResp := &tfplugin5.GetFunctions_Response{ - Functions: map[string]*tfplugin5.Function{}, - } - - return protoResp, nil - } - req := fromproto.GetFunctionsRequest(protoReq) ctx = tf5serverlogging.DownstreamRequest(ctx) - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/353 - // resp, err := s.downstream.GetFunctions(ctx, req) - resp, err := functionServer.GetFunctions(ctx, req) + resp, err := s.downstream.GetFunctions(ctx, req) if err != nil { logging.ProtocolError(ctx, "Error from downstream", map[string]any{logging.KeyError: err}) diff --git a/tfprotov6/provider.go b/tfprotov6/provider.go index e1ea384de..c50c3ca69 100644 --- a/tfprotov6/provider.go +++ b/tfprotov6/provider.go @@ -53,10 +53,7 @@ type ProviderServer interface { // are a handy interface for defining what a function is to // terraform-plugin-go, so they are their own interface that is composed // into ProviderServer. - // - // This will be required in an upcoming release. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/353 - // FunctionServer + FunctionServer } // GetMetadataRequest represents a GetMetadata RPC request. diff --git a/tfprotov6/resource.go b/tfprotov6/resource.go index 9344f8db8..3dd3b08d5 100644 --- a/tfprotov6/resource.go +++ b/tfprotov6/resource.go @@ -52,14 +52,24 @@ type ResourceServer interface { // specified by the passed ID and return it as one or more resource // states for Terraform to assume control of. ImportResourceState(context.Context, *ImportResourceStateRequest) (*ImportResourceStateResponse, error) + + // MoveResourceState is called when Terraform is asked to change a resource + // type for an existing resource. The provider must accept the change as + // valid by ensuring the source resource type, schema version, and provider + // address are compatible to convert the source state into the target + // resource type and latest state version. + // + // This functionality is only supported in Terraform 1.8 and later. The + // provider must have enabled the MoveResourceState server capability to + // enable these requests. + MoveResourceState(context.Context, *MoveResourceStateRequest) (*MoveResourceStateResponse, error) } // ResourceServerWithMoveResourceState is a temporary interface for servers // to implement MoveResourceState RPC handling. // -// Deprecated: The MoveResourceState method will be moved into the -// ResourceServer interface and this interface will be removed in a future -// version. +// Deprecated: This interface will be removed in a future version. Use +// ResourceServer instead. type ResourceServerWithMoveResourceState interface { ResourceServer diff --git a/tfprotov6/tf6server/server.go b/tfprotov6/tf6server/server.go index e8b5eb4da..4982c4898 100644 --- a/tfprotov6/tf6server/server.go +++ b/tfprotov6/tf6server/server.go @@ -900,37 +900,11 @@ func (s *server) MoveResourceState(ctx context.Context, protoReq *tfplugin6.Move logging.ProtocolTrace(ctx, "Received request") defer logging.ProtocolTrace(ctx, "Served request") - // Remove this check and error in preference of - // s.downstream.MoveResourceState below once ResourceServer interface - // implements the MoveResourceState method. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/363 - // nolint:staticcheck - resourceServerWMRS, ok := s.downstream.(tfprotov6.ResourceServerWithMoveResourceState) - - if !ok { - logging.ProtocolError(ctx, "ProviderServer does not implement ResourceServerWithMoveResourceState") - - protoResp := &tfplugin6.MoveResourceState_Response{ - Diagnostics: []*tfplugin6.Diagnostic{ - { - Severity: tfplugin6.Diagnostic_ERROR, - Summary: "Provider Move Resource State Not Implemented", - Detail: "A MoveResourceState call was received by the provider, however the provider does not implement the call. " + - "Either upgrade the provider to a version that implements move resource state support or this is a bug in Terraform that should be reported to the Terraform maintainers.", - }, - }, - } - - return protoResp, nil - } - req := fromproto.MoveResourceStateRequest(protoReq) ctx = tf6serverlogging.DownstreamRequest(ctx) - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/363 - // resp, err := s.downstream.MoveResourceState(ctx, req) - resp, err := resourceServerWMRS.MoveResourceState(ctx, req) + resp, err := s.downstream.MoveResourceState(ctx, req) if err != nil { logging.ProtocolError(ctx, "Error from downstream", map[string]interface{}{logging.KeyError: err}) @@ -954,26 +928,6 @@ func (s *server) CallFunction(ctx context.Context, protoReq *tfplugin6.CallFunct logging.ProtocolTrace(ctx, "Received request") defer logging.ProtocolTrace(ctx, "Served request") - // Remove this check and error in preference of s.downstream.CallFunction - // below once ProviderServer interface requires FunctionServer. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/353 - functionServer, ok := s.downstream.(tfprotov6.FunctionServer) - - if !ok { - logging.ProtocolError(ctx, "ProviderServer does not implement FunctionServer") - - text := "Provider Functions Not Implemented: A provider-defined function call was received by the provider, however the provider does not implement functions. " + - "Either upgrade the provider to a version that implements provider-defined functions or this is a bug in Terraform that should be reported to the Terraform maintainers." - - protoResp := &tfplugin6.CallFunction_Response{ - Error: &tfplugin6.FunctionError{ - Text: text, - }, - } - - return protoResp, nil - } - req := fromproto.CallFunctionRequest(protoReq) for position, argument := range req.Arguments { @@ -982,9 +936,7 @@ func (s *server) CallFunction(ctx context.Context, protoReq *tfplugin6.CallFunct ctx = tf6serverlogging.DownstreamRequest(ctx) - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/353 - // resp, err := s.downstream.CallFunction(ctx, req) - resp, err := functionServer.CallFunction(ctx, req) + resp, err := s.downstream.CallFunction(ctx, req) if err != nil { logging.ProtocolError(ctx, "Error from downstream", map[string]any{logging.KeyError: err}) @@ -1007,28 +959,11 @@ func (s *server) GetFunctions(ctx context.Context, protoReq *tfplugin6.GetFuncti logging.ProtocolTrace(ctx, "Received request") defer logging.ProtocolTrace(ctx, "Served request") - // Remove this check and response in preference of s.downstream.GetFunctions - // below once ProviderServer interface requires FunctionServer. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/353 - functionServer, ok := s.downstream.(tfprotov6.FunctionServer) - - if !ok { - logging.ProtocolWarn(ctx, "ProviderServer does not implement FunctionServer") - - protoResp := &tfplugin6.GetFunctions_Response{ - Functions: map[string]*tfplugin6.Function{}, - } - - return protoResp, nil - } - req := fromproto.GetFunctionsRequest(protoReq) ctx = tf6serverlogging.DownstreamRequest(ctx) - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/353 - // resp, err := s.downstream.GetFunctions(ctx, req) - resp, err := functionServer.GetFunctions(ctx, req) + resp, err := s.downstream.GetFunctions(ctx, req) if err != nil { logging.ProtocolError(ctx, "Error from downstream", map[string]any{logging.KeyError: err})