diff --git a/pkg/tests/cross-tests/input_check.go b/pkg/tests/cross-tests/input_check.go index 1ac1e7f45..eae358aa7 100644 --- a/pkg/tests/cross-tests/input_check.go +++ b/pkg/tests/cross-tests/input_check.go @@ -34,7 +34,9 @@ type inputTestCase struct { Config any ObjectType *tftypes.Object - SkipCompareRaw bool + SkipCompareRawPlan bool + SkipCompareRawConfig bool + SkipCompareRawState bool } func FailNotEqual(t T, name string, tfVal, pulVal any) { @@ -143,10 +145,15 @@ func runCreateInputCheck(t T, tc inputTestCase) { assertValEqual(t, k+" Change New", tfChangeValNew, pulChangeValNew) } - if !tc.SkipCompareRaw { + if !tc.SkipCompareRawConfig { assertCtyValEqual(t, "RawConfig", tfResData.GetRawConfig(), pulResData.GetRawConfig()) + } + + if !tc.SkipCompareRawPlan { assertCtyValEqual(t, "RawPlan", tfResData.GetRawPlan(), pulResData.GetRawPlan()) - // TODO: we currently represent null state values wrong. We should fix it. - // assertCtyValEqual(t, "RawState", tfResData.GetRawState(), pulResData.GetRawState()) + } + + if !tc.SkipCompareRawState { + assertCtyValEqual(t, "RawState", tfResData.GetRawState(), pulResData.GetRawState()) } } diff --git a/pkg/tests/cross-tests/input_cross_test.go b/pkg/tests/cross-tests/input_cross_test.go index 4ffc9012f..1de2f5e90 100644 --- a/pkg/tests/cross-tests/input_cross_test.go +++ b/pkg/tests/cross-tests/input_cross_test.go @@ -110,8 +110,6 @@ func TestInputsEmptySchema(t *testing.T) { Schema: map[string]*schema.Schema{}, }, Config: tftypes.NewValue(tftypes.Object{}, map[string]tftypes.Value{}), - // TODO[pulumi/pulumi-terraform-bridge#1914] - SkipCompareRaw: true, }, ) } @@ -151,7 +149,8 @@ func TestInputsEqualEmptyList(t *testing.T) { }, ), // TODO[pulumi/pulumi-terraform-bridge#1915] - SkipCompareRaw: true, + SkipCompareRawPlan: true, + SkipCompareRawConfig: true, }) }) } @@ -180,8 +179,6 @@ func TestInputsEmptyString(t *testing.T) { "f0": tftypes.NewValue(tftypes.String, ""), }, ), - // TODO[pulumi/pulumi-terraform-bridge#1916] - SkipCompareRaw: true, }) } @@ -217,6 +214,6 @@ func TestInputsEmptyConfigModeAttrSet(t *testing.T) { "f1": tftypes.NewValue(tftypes.Set{ElementType: t2}, []tftypes.Value{}), }), // TODO[pulumi/pulumi-terraform-bridge#1762] - SkipCompareRaw: true, + SkipCompareRawConfig: true, }) } diff --git a/pkg/tfbridge/provider_test.go b/pkg/tfbridge/provider_test.go index ad77f2cc9..9f5901466 100644 --- a/pkg/tfbridge/provider_test.go +++ b/pkg/tfbridge/provider_test.go @@ -22,6 +22,7 @@ import ( "testing" "time" + "github.com/hashicorp/go-cty/cty" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hexops/autogold/v2" @@ -4307,3 +4308,69 @@ func TestStringValForOtherProperty(t *testing.T) { }`) }) } + +func TestPlanResourceChangeStateUpgrade(t *testing.T) { + // TODO[pulumi/pulumi-terraform-bridge#1667] + t.Skipf("Skip since we try to use the current schema for the state.") + p := &schemav2.Provider{ + Schema: map[string]*schemav2.Schema{}, + ResourcesMap: map[string]*schemav2.Resource{ + "example_resource": { + Schema: map[string]*schemav2.Schema{ + "prop": &schema.Schema{ + Type: schema.TypeSet, + Optional: true, + Elem: &schemav2.Schema{Type: schemav2.TypeString}, + }, + }, + StateUpgraders: []schema.StateUpgrader{ + { + Version: 0, + Type: cty.Object(map[string]cty.Type{"prop": cty.String}), + Upgrade: func( + ctx context.Context, rawState map[string]interface{}, meta interface{}, + ) (map[string]interface{}, error) { + rawState["prop"] = []interface{}{rawState["prop"]} + return rawState, nil + }, + }, + }, + }, + }, + } + shimProv := shimv2.NewProvider(p, shimv2.WithPlanResourceChange(func(tfResourceType string) bool { return true })) + provider := &Provider{ + tf: shimProv, + config: shimv2.NewSchemaMap(p.Schema), + info: ProviderInfo{ + P: shimProv, + ResourcePrefix: "example", + Resources: map[string]*ResourceInfo{ + "example_resource": {Tok: "ExampleResource"}, + }, + }, + } + provider.initResourceMaps() + + testutils.Replay(t, provider, ` + { + "method": "/pulumirpc.ResourceProvider/Diff", + "request": { + "urn": "urn:pulumi:dev::teststack::ExampleResource::exres", + "id": "0", + "olds": { + "__meta": { + "version": 0 + }, + "prop": "val" + }, + "news": { + "prop": ["val"] + } + }, + "response": { + "changes": "DIFF_NONE", + "hasDetailedDiff": true + } + }`) +} diff --git a/pkg/tfshim/sdk-v2/provider2.go b/pkg/tfshim/sdk-v2/provider2.go index 627eeb8bc..3f6e1b9a2 100644 --- a/pkg/tfshim/sdk-v2/provider2.go +++ b/pkg/tfshim/sdk-v2/provider2.go @@ -4,8 +4,10 @@ import ( "context" "encoding/json" "fmt" + "strconv" "github.com/hashicorp/go-cty/cty" + ctyjson "github.com/hashicorp/go-cty/cty/json" "github.com/hashicorp/go-cty/cty/msgpack" "github.com/hashicorp/go-multierror" "github.com/hashicorp/terraform-plugin-go/tfprotov5" @@ -44,6 +46,8 @@ func (r *v2Resource2) InstanceState( copy["id"] = id object = copy } + // TODO[pulumi/pulumi-terraform-bridge#1667]: This is not right since it uses the + // current schema. 1667 should make this redundant s, err := recoverAndCoerceCtyValueWithSchema(r.v2Resource.tf.CoreConfigSchema(), object) if err != nil { return nil, fmt.Errorf("InstanceState: %v", err) @@ -322,31 +326,66 @@ func (p *planResourceChangeImpl) upgradeState( ) (shim.InstanceState, error) { res := p.tf.ResourcesMap[t] state := p.unpackInstanceState(t, s) - instanceState, err := res.ShimInstanceStateFromValue(state.stateValue) + + // TODO[pulumi/pulumi-terraform-bridge#1667]: This is not quite right but we need + // the old TF state to get it right. + jsonBytes, err := ctyjson.Marshal(state.stateValue, state.stateValue.Type()) if err != nil { return nil, err } - // Looks like this definitely can happen, but upgradeResourceState assumes a non-nil map. - if instanceState.Attributes == nil { - instanceState.Attributes = map[string]string{} + + version := int64(0) + if versionValue, ok := state.meta["schema_version"]; ok { + versionString, ok := versionValue.(string) + if !ok { + return nil, fmt.Errorf("unexpected type %T for schema_version", versionValue) + } + v, err := strconv.ParseInt(versionString, 0, 32) + if err != nil { + return nil, err + } + version = v + } + + //nolint:lll + // https://github.com/opentofu/opentofu/blob/2ef3047ec6bb266e8d91c55519967212c1a0975d/internal/tofu/upgrade_resource_state.go#L52 + if version > int64(res.SchemaVersion) { + return nil, fmt.Errorf( + "State version %d is greater than schema version %d for resource %s. "+ + "Please upgrade the provider to work with this resource.", + version, res.SchemaVersion, t, + ) } - instanceState.Meta = state.meta - newInstanceState, err := upgradeResourceState(ctx, p.tf, res, instanceState) + + // Note upgrade is always called, even if the versions match + //nolint:lll + // https://github.com/opentofu/opentofu/blob/2ef3047ec6bb266e8d91c55519967212c1a0975d/internal/tofu/upgrade_resource_state.go#L72 + + resp, err := p.server.gserver.UpgradeResourceState(ctx, &tfprotov5.UpgradeResourceStateRequest{ + TypeName: t, + RawState: &tfprotov5.RawState{JSON: jsonBytes}, + Version: version, + }) if err != nil { return nil, err } - if newInstanceState == nil { - return nil, nil - } - ty := res.CoreConfigSchema().ImpliedType() - stateValue, err := newInstanceState.AttrsAsObjectValue(ty) + + newState, err := msgpack.Unmarshal(resp.UpgradedState.MsgPack, res.CoreConfigSchema().ImpliedType()) if err != nil { return nil, err } + + newMeta := make(map[string]interface{}, len(state.meta)) + // copy old meta into new meta + for k, v := range state.meta { + newMeta[k] = v + } + newMeta["schema_version"] = strconv.Itoa(res.SchemaVersion) + return &v2InstanceState2{ resourceType: t, - stateValue: stateValue, - meta: newInstanceState.Meta, + stateValue: newState, + meta: newMeta, }, nil } diff --git a/pkg/tfshim/sdk-v2/provider2_test.go b/pkg/tfshim/sdk-v2/provider2_test.go new file mode 100644 index 000000000..31a098e41 --- /dev/null +++ b/pkg/tfshim/sdk-v2/provider2_test.go @@ -0,0 +1,205 @@ +package sdkv2 + +import ( + "context" + "strconv" + "testing" + + "github.com/hashicorp/go-cty/cty" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestUpgradeResourceState(t *testing.T) { + const tfToken = "test_token" + for _, tc := range []struct { + name string + state cty.Value + rSchema *schema.Resource + expected cty.Value + }{ + { + name: "no upgrade", + state: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("1"), + "x": cty.NumberIntVal(1), + }), + rSchema: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "x": {Type: schema.TypeInt, Optional: true}, + }, + }, + expected: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("1"), + "x": cty.NumberIntVal(1), + }), + }, + { + name: "basic upgrade type", + state: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("1"), + "x": cty.StringVal("1"), + }), + rSchema: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "x": {Type: schema.TypeInt, Optional: true}, + }, + StateUpgraders: []schema.StateUpgrader{ + { + Version: 0, + Type: cty.Object(map[string]cty.Type{"x": cty.String}), + Upgrade: func( + ctx context.Context, rawState map[string]interface{}, meta interface{}, + ) (map[string]interface{}, error) { + newVal, err := strconv.ParseInt(rawState["x"].(string), 10, 64) + if err != nil { + return nil, err + } + rawState["x"] = int(newVal) + return rawState, nil + }, + }, + }, + }, + expected: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("1"), + "x": cty.NumberIntVal(1), + }), + }, + { + name: "rename property", + state: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("1"), + "x": cty.StringVal("1"), + }), + rSchema: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "y": {Type: schema.TypeString, Optional: true}, + }, + StateUpgraders: []schema.StateUpgrader{ + { + Version: 0, + Type: cty.Object(map[string]cty.Type{"x": cty.String}), + Upgrade: func( + ctx context.Context, rawState map[string]interface{}, meta interface{}, + ) (map[string]interface{}, error) { + rawState["y"] = rawState["x"] + delete(rawState, "x") + return rawState, nil + }, + }, + }, + }, + expected: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("1"), + "y": cty.StringVal("1"), + }), + }, + { + name: "flat prop to collection", + state: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("1"), + "x": cty.StringVal("1"), + }), + rSchema: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "x": {Type: schema.TypeList, Optional: true, Elem: &schema.Schema{Type: schema.TypeString}}, + }, + StateUpgraders: []schema.StateUpgrader{ + { + Version: 0, + Type: cty.Object(map[string]cty.Type{"x": cty.String}), + Upgrade: func( + ctx context.Context, rawState map[string]interface{}, meta interface{}, + ) (map[string]interface{}, error) { + rawState["x"] = []interface{}{rawState["x"]} + return rawState, nil + }, + }, + }, + }, + expected: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("1"), + "x": cty.ListVal([]cty.Value{cty.StringVal("1")}), + }), + }, + { + name: "collection to flat prop", + state: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("1"), + "x": cty.ListVal([]cty.Value{cty.StringVal("1")}), + }), + rSchema: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "x": {Type: schema.TypeString, Optional: true}, + }, + StateUpgraders: []schema.StateUpgrader{ + { + Version: 0, + Type: cty.Object(map[string]cty.Type{"x": cty.List(cty.String)}), + Upgrade: func( + ctx context.Context, rawState map[string]interface{}, meta interface{}, + ) (map[string]interface{}, error) { + rawState["x"] = rawState["x"].([]interface{})[0] + return rawState, nil + }, + }, + }, + }, + expected: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("1"), + "x": cty.StringVal("1"), + }), + }, + { + name: "change list to set", + state: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("1"), + "x": cty.ListVal([]cty.Value{cty.StringVal("1"), cty.StringVal("2")}), + }), + rSchema: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "x": { + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, + }, + }, + StateUpgraders: []schema.StateUpgrader{ + { + Version: 0, + Type: cty.Object(map[string]cty.Type{"x": cty.List(cty.String)}), + Upgrade: func( + ctx context.Context, rawState map[string]interface{}, meta interface{}, + ) (map[string]interface{}, error) { + return rawState, nil + }, + }, + }, + }, + expected: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("1"), + "x": cty.SetVal([]cty.Value{cty.StringVal("1"), cty.StringVal("2")}), + }), + }, + } { + t.Run(tc.name, func(t *testing.T) { + tf := &schema.Provider{ + ResourcesMap: map[string]*schema.Resource{ + tfToken: tc.rSchema, + }, + } + actual, err := (&planResourceChangeImpl{ + tf: tf, + server: &grpcServer{schema.NewGRPCProviderServer(tf)}, + }).upgradeState(context.Background(), tfToken, &v2InstanceState2{ + resourceType: tfToken, + stateValue: tc.state, + }) + require.NoError(t, err) + + assert.Equal(t, tc.expected, actual.(*v2InstanceState2).stateValue) + }) + } +}