Skip to content

Commit

Permalink
tfsdk: Support protocol version 5 and verify valid resource type in U…
Browse files Browse the repository at this point in the history
…pgradeResourceState RPC (#263)

Reference: #42
Reference: #262
Reference: hashicorp/terraform-provider-corner#44

Terraform CLI version 0.12 through 0.14 support protocol version 5 and require the `UpgradeResourceStateResponse` type `UpgradedState` type `Msgpack` field. To support downgraded framework providers via terraform-plugin-mux `tf6to5server`, the `RawState` type `JSON` field cannot simply be passed through like the previous implementation. This change will parse the `RawState` and output it via the `tfprotov6.NewDynamicValue()` function, which will always set the `Msgpack` field.

Similar logic would have already been required as part of the larger effort to support resource versioning and upgrading resource state over time.

Passthrough of the same state of an invalid resource type should not have been previously allowed, since the provider did not implement the resource type. Regardless, Terraform CLI should not have reached this point since `GetProviderSchema` would not have returned the resource type anyways for other resource operations.

Verified via integration testing of terraform-plugin-mux `tf6to5server`, which previously returned a non-descript `EOF` error.
  • Loading branch information
bflad authored Feb 22, 2022
1 parent bf0240f commit ce0064c
Show file tree
Hide file tree
Showing 5 changed files with 404 additions and 8 deletions.
3 changes: 3 additions & 0 deletions .changelog/263.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
tfsdk: Support protocol version 5 and verify valid resource type in `UpgradeResourceState` RPC
```
100 changes: 92 additions & 8 deletions tfsdk/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -485,16 +485,100 @@ func (s *server) validateResourceConfig(ctx context.Context, req *tfprotov6.Vali
resp.Diagnostics = validateSchemaResp.Diagnostics
}

func (s *server) UpgradeResourceState(ctx context.Context, req *tfprotov6.UpgradeResourceStateRequest) (*tfprotov6.UpgradeResourceStateResponse, error) {
// uncomment when we implement this function
//ctx = s.registerContext(ctx)
// upgradeResourceStateResponse is a thin abstraction to allow native
// Diagnostics usage.
type upgradeResourceStateResponse struct {
Diagnostics diag.Diagnostics
UpgradedState *tfprotov6.DynamicValue
}

// TODO: support state upgrades
func (r upgradeResourceStateResponse) toTfprotov6() *tfprotov6.UpgradeResourceStateResponse {
return &tfprotov6.UpgradeResourceStateResponse{
UpgradedState: &tfprotov6.DynamicValue{
JSON: req.RawState.JSON,
},
}, nil
Diagnostics: r.Diagnostics.ToTfprotov6Diagnostics(),
UpgradedState: r.UpgradedState,
}
}

func (s *server) UpgradeResourceState(ctx context.Context, req *tfprotov6.UpgradeResourceStateRequest) (*tfprotov6.UpgradeResourceStateResponse, error) {
ctx = s.registerContext(ctx)
resp := &upgradeResourceStateResponse{}

s.upgradeResourceState(ctx, req, resp)

return resp.toTfprotov6(), nil
}

func (s *server) upgradeResourceState(ctx context.Context, req *tfprotov6.UpgradeResourceStateRequest, resp *upgradeResourceStateResponse) {
if req == nil {
return
}

resourceType, diags := s.getResourceType(ctx, req.TypeName)

resp.Diagnostics.Append(diags...)

if resp.Diagnostics.HasError() {
return
}

// No UpgradedState to return. This could return an error diagnostic about
// the odd scenario, but seems best to allow Terraform CLI to handle the
// situation itself in case it might be expected behavior.
if req.RawState == nil {
return
}

// This implementation assumes the current schema is the only valid schema
// for the given resource and will return an error if any mismatched prior
// state is given. This matches prior behavior of the framework, but is now
// more explicit in error handling, rather than just passing through any
// potentially errant prior state, which should have resulted in a similar
// error further in the resource lifecycle.
//
// TODO: Implement resource state upgrades, rather than just using the
// current resource schema.
// Reference: https://github.com/hashicorp/terraform-plugin-framework/issues/42
resourceSchema, diags := resourceType.GetSchema(ctx)

resp.Diagnostics.Append(diags...)

if resp.Diagnostics.HasError() {
return
}

resourceSchemaType := resourceSchema.TerraformType(ctx)

rawStateValue, err := req.RawState.Unmarshal(resourceSchemaType)

if err != nil {
resp.Diagnostics.AddError(
"Unable to Read Previously Saved State for UpgradeResourceState",
"There was an error reading the saved resource state using the current resource schema. "+
"This resource was implemented in a Terraform Provider SDK that does not support upgrading resource state yet.\n\n"+
"If the resource previously implemented different resource state versions, the provider developers will need to revert back to the previous implementation. "+
"If this resource state was last refreshed with Terraform CLI 0.11 and earlier, it must be refreshed or applied with an older provider version first. "+
"If you manually modified the resource state, you will need to manually modify it to match the current resource schema. "+
"Otherwise, please report this to the provider developer:\n\n"+err.Error(),
)
return
}

// NewDynamicValue will ensure the Msgpack field is set for Terraform CLI
// 0.12 through 0.14 compatibility when using terraform-plugin-mux tf6to5server.
// Reference: https://github.com/hashicorp/terraform-plugin-framework/issues/262
upgradedStateValue, err := tfprotov6.NewDynamicValue(resourceSchemaType, rawStateValue)

if err != nil {
resp.Diagnostics.AddError(
"Unable to Convert Previously Saved State for UpgradeResourceState",
"There was an error converting the saved resource state using the current resource schema. "+
"This is always an issue in the Terraform Provider SDK used to implement the resource and should be reported to the provider developers.\n\n"+
"Please report this to the provider developer:\n\n"+err.Error(),
)
return
}

resp.UpgradedState = &upgradedStateValue
}

// readResourceResponse is a thin abstraction to allow native Diagnostics usage
Expand Down
6 changes: 6 additions & 0 deletions tfsdk/serve_provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ type testServeProvider struct {
validateResourceConfigCalledResourceType string
validateResourceConfigImpl func(context.Context, ValidateResourceConfigRequest, *ValidateResourceConfigResponse)

// upgrade resource state
// TODO: Implement with UpgradeResourceState support
// Reference: https://github.com/hashicorp/terraform-plugin-framework/issues/42
// upgradeResourceStateCalledResourceType string

// read resource request
readResourceCurrentStateValue tftypes.Value
readResourceCurrentStateSchema Schema
Expand Down Expand Up @@ -636,6 +641,7 @@ func (t *testServeProvider) GetResources(_ context.Context) (map[string]Resource
"test_attribute_plan_modifiers": testServeResourceTypeAttributePlanModifiers{},
"test_config_validators": testServeResourceTypeConfigValidators{},
"test_import_state": testServeResourceTypeImportState{},
"test_upgrade_state": testServeResourceTypeUpgradeState{},
"test_validate_config": testServeResourceTypeValidateConfig{},
}, nil
}
Expand Down
106 changes: 106 additions & 0 deletions tfsdk/serve_resource_upgrade_state_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
package tfsdk

import (
"context"
"fmt"

"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/hashicorp/terraform-plugin-go/tfprotov6"
"github.com/hashicorp/terraform-plugin-go/tftypes"
)

// This resource is a placeholder for UpgradeResourceState testing,
// so it is decoupled from other test resources.
// TODO: Implement UpgradeResourceState support, when added
// Reference: https://github.com/hashicorp/terraform-plugin-framework/issues/42
type testServeResourceTypeUpgradeState struct{}

func (t testServeResourceTypeUpgradeState) GetSchema(_ context.Context) (Schema, diag.Diagnostics) {
return Schema{
Attributes: map[string]Attribute{
"id": {
Type: types.StringType,
Computed: true,
},
"optional_string": {
Type: types.StringType,
Optional: true,
},
"required_string": {
Type: types.StringType,
Required: true,
},
},
}, nil
}

func (t testServeResourceTypeUpgradeState) NewResource(_ context.Context, p Provider) (Resource, diag.Diagnostics) {
provider, ok := p.(*testServeProvider)
if !ok {
prov, ok := p.(*testServeProviderWithMetaSchema)
if !ok {
panic(fmt.Sprintf("unexpected provider type %T", p))
}
provider = prov.testServeProvider
}
return testServeResourceUpgradeState{
provider: provider,
}, nil
}

var testServeResourceTypeUpgradeStateSchema = &tfprotov6.Schema{
Block: &tfprotov6.SchemaBlock{
Attributes: []*tfprotov6.SchemaAttribute{
{
Name: "id",
Computed: true,
Type: tftypes.String,
},
{
Name: "optional_string",
Optional: true,
Type: tftypes.String,
},
{
Name: "required_string",
Required: true,
Type: tftypes.String,
},
},
},
}

// var testServeResourceTypeUpgradeStateTftype = tftypes.Object{
// AttributeTypes: map[string]tftypes.Type{
// "id": tftypes.String,
// "optional_string": tftypes.String,
// "required_string": tftypes.String,
// },
// }

// type testServeResourceUpgradeStateData struct {
// Id string `tfsdk:"id"`
// OptionalString *string `tfsdk:"optional_string"`
// RequiredString string `tfsdk:"required_string"`
// }

type testServeResourceUpgradeState struct {
provider *testServeProvider
}

func (r testServeResourceUpgradeState) Create(ctx context.Context, req CreateResourceRequest, resp *CreateResourceResponse) {
// Intentionally blank. Not expected to be called during testing.
}
func (r testServeResourceUpgradeState) Read(ctx context.Context, req ReadResourceRequest, resp *ReadResourceResponse) {
// Intentionally blank. Not expected to be called during testing.
}
func (r testServeResourceUpgradeState) Update(ctx context.Context, req UpdateResourceRequest, resp *UpdateResourceResponse) {
// Intentionally blank. Not expected to be called during testing.
}
func (r testServeResourceUpgradeState) Delete(ctx context.Context, req DeleteResourceRequest, resp *DeleteResourceResponse) {
// Intentionally blank. Not expected to be called during testing.
}
func (r testServeResourceUpgradeState) ImportState(ctx context.Context, req ImportResourceStateRequest, resp *ImportResourceStateResponse) {
ResourceImportStateNotImplemented(ctx, "intentionally not implemented", resp)
}
Loading

0 comments on commit ce0064c

Please sign in to comment.