diff --git a/.changelog/263.txt b/.changelog/263.txt new file mode 100644 index 000000000..e3e8f1f9a --- /dev/null +++ b/.changelog/263.txt @@ -0,0 +1,3 @@ +```release-note:bug +tfsdk: Support protocol version 5 and verify valid resource type in `UpgradeResourceState` RPC +``` diff --git a/tfsdk/serve.go b/tfsdk/serve.go index f3fae52b5..b7547b953 100644 --- a/tfsdk/serve.go +++ b/tfsdk/serve.go @@ -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 diff --git a/tfsdk/serve_provider_test.go b/tfsdk/serve_provider_test.go index 44f809fb4..403c7cbcc 100644 --- a/tfsdk/serve_provider_test.go +++ b/tfsdk/serve_provider_test.go @@ -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 @@ -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 } diff --git a/tfsdk/serve_resource_upgrade_state_test.go b/tfsdk/serve_resource_upgrade_state_test.go new file mode 100644 index 000000000..56fc8f37b --- /dev/null +++ b/tfsdk/serve_resource_upgrade_state_test.go @@ -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) +} diff --git a/tfsdk/serve_test.go b/tfsdk/serve_test.go index 5fa35ff64..5368586a4 100644 --- a/tfsdk/serve_test.go +++ b/tfsdk/serve_test.go @@ -2,6 +2,8 @@ package tfsdk import ( "context" + "encoding/json" + "strings" "sync" "testing" "time" @@ -279,6 +281,7 @@ func TestServerGetProviderSchema(t *testing.T) { "test_attribute_plan_modifiers": testServeResourceTypeAttributePlanModifiersSchema, "test_config_validators": testServeResourceTypeConfigValidatorsSchema, "test_import_state": testServeResourceTypeImportStateSchema, + "test_upgrade_state": testServeResourceTypeUpgradeStateSchema, "test_validate_config": testServeResourceTypeValidateConfigSchema, }, DataSourceSchemas: map[string]*tfprotov6.Schema{ @@ -314,6 +317,7 @@ func TestServerGetProviderSchemaWithProviderMeta(t *testing.T) { "test_attribute_plan_modifiers": testServeResourceTypeAttributePlanModifiersSchema, "test_config_validators": testServeResourceTypeConfigValidatorsSchema, "test_import_state": testServeResourceTypeImportStateSchema, + "test_upgrade_state": testServeResourceTypeUpgradeStateSchema, "test_validate_config": testServeResourceTypeValidateConfigSchema, }, DataSourceSchemas: map[string]*tfprotov6.Schema{ @@ -1402,6 +1406,199 @@ func TestServerValidateResourceConfig(t *testing.T) { } } +func TestServerUpgradeResourceState(t *testing.T) { + t.Parallel() + + ctx := context.Background() + schema, _ := testServeResourceTypeUpgradeState{}.GetSchema(ctx) + schemaType := schema.TerraformType(ctx) + + validRawStateJSON, err := json.Marshal(map[string]string{ + "id": "test-id-value", + "required_string": "test-required-value", + }) + + if err != nil { + t.Fatalf("unable to create RawState JSON: %s", err) + } + + validRawState := tfprotov6.RawState{ + JSON: validRawStateJSON, + } + + validUpgradedState, err := tfprotov6.NewDynamicValue(schemaType, tftypes.NewValue(schemaType, map[string]tftypes.Value{ + "id": tftypes.NewValue(tftypes.String, "test-id-value"), + "optional_string": tftypes.NewValue(tftypes.String, nil), + "required_string": tftypes.NewValue(tftypes.String, "test-required-value"), + })) + + if err != nil { + t.Fatalf("unable to create UpgradedState: %s", err) + } + + testCases := map[string]struct { + request *tfprotov6.UpgradeResourceStateRequest + expectedResponse *tfprotov6.UpgradeResourceStateResponse + expectedError error + }{ + "nil": { + request: nil, + expectedResponse: &tfprotov6.UpgradeResourceStateResponse{}, + }, + "RawState-Flatmap": { + request: &tfprotov6.UpgradeResourceStateRequest{ + TypeName: "test_upgrade_state", + RawState: &tfprotov6.RawState{ + Flatmap: map[string]string{ + "flatmap": "is not supported", + }, + }, + }, + expectedResponse: &tfprotov6.UpgradeResourceStateResponse{ + Diagnostics: []*tfprotov6.Diagnostic{ + { + Severity: tfprotov6.DiagnosticSeverityError, + Summary: "Unable to Read Previously Saved State for UpgradeResourceState", + Detail: "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" + + "flatmap states cannot be unmarshaled, only states written by Terraform 0.12 and higher can be unmarshaled", + }, + }, + }, + }, + "RawState-JSON-passthrough": { + request: &tfprotov6.UpgradeResourceStateRequest{ + TypeName: "test_upgrade_state", + RawState: &validRawState, + }, + expectedResponse: &tfprotov6.UpgradeResourceStateResponse{ + UpgradedState: &validUpgradedState, + }, + }, + "RawState-JSON-mismatch": { + request: &tfprotov6.UpgradeResourceStateRequest{ + TypeName: "test_upgrade_state", + RawState: &tfprotov6.RawState{ + JSON: []byte(`{"nonexistent_attribute":"value"}`), + }, + }, + expectedResponse: &tfprotov6.UpgradeResourceStateResponse{ + Diagnostics: []*tfprotov6.Diagnostic{ + { + Severity: tfprotov6.DiagnosticSeverityError, + Summary: "Unable to Read Previously Saved State for UpgradeResourceState", + Detail: "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" + + "ElementKeyValue(tftypes.String): unsupported attribute \"nonexistent_attribute\"", + }, + }, + }, + }, + "RawState-empty": { + request: &tfprotov6.UpgradeResourceStateRequest{ + RawState: &tfprotov6.RawState{}, + TypeName: "test_upgrade_state", + }, + expectedResponse: &tfprotov6.UpgradeResourceStateResponse{ + Diagnostics: []*tfprotov6.Diagnostic{ + { + Severity: tfprotov6.DiagnosticSeverityError, + Summary: "Unable to Read Previously Saved State for UpgradeResourceState", + Detail: "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" + + "RawState had no JSON or flatmap data set", + }, + }, + }, + }, + "RawState-missing": { + request: &tfprotov6.UpgradeResourceStateRequest{ + TypeName: "test_upgrade_state", + }, + expectedResponse: &tfprotov6.UpgradeResourceStateResponse{}, + }, + "TypeName-missing": { + request: &tfprotov6.UpgradeResourceStateRequest{}, + expectedResponse: &tfprotov6.UpgradeResourceStateResponse{ + Diagnostics: []*tfprotov6.Diagnostic{ + { + Severity: tfprotov6.DiagnosticSeverityError, + Summary: "Resource not found", + Detail: "No resource named \"\" is configured on the provider", + }, + }, + }, + }, + "TypeName-unknown": { + request: &tfprotov6.UpgradeResourceStateRequest{ + TypeName: "unknown", + }, + expectedResponse: &tfprotov6.UpgradeResourceStateResponse{ + Diagnostics: []*tfprotov6.Diagnostic{ + { + Severity: tfprotov6.DiagnosticSeverityError, + Summary: "Resource not found", + Detail: "No resource named \"unknown\" is configured on the provider", + }, + }, + }, + }, + } + + for name, testCase := range testCases { + name, testCase := name, testCase + + t.Run(name, func(t *testing.T) { + t.Parallel() + + ctx := context.Background() + testProvider := &testServeProvider{} + testServer := &server{ + p: testProvider, + } + + got, err := testServer.UpgradeResourceState(ctx, testCase.request) + + if err != nil { + if testCase.expectedError == nil { + t.Fatalf("expected no error, got: %s", err) + } + + if !strings.Contains(err.Error(), testCase.expectedError.Error()) { + t.Fatalf("expected error %q, got: %s", testCase.expectedError, err) + } + } + + if err == nil && testCase.expectedError != nil { + t.Fatalf("got no error, expected: %s", testCase.expectedError) + } + + // TODO: Implement with UpgradeResourceState support + // Reference: https://github.com/hashicorp/terraform-plugin-framework/issues/42 + // if testCase.request != nil && testProvider.upgradeResourceStateCalledResourceType != testCase.request.TypeName { + // t.Errorf("expected to call resource %q, called: %s", testCase.request.TypeName, testProvider.upgradeResourceStateCalledResourceType) + // return + // } + + if diff := cmp.Diff(got, testCase.expectedResponse); diff != "" { + t.Errorf("unexpected difference in response: %s", diff) + } + }) + } +} + func TestServerReadResource(t *testing.T) { t.Parallel()