From 0ac62de9562500626c052245833e1b14997eb17b Mon Sep 17 00:00:00 2001 From: Anton Tayanovskyy Date: Tue, 14 Nov 2023 21:01:26 -0500 Subject: [PATCH] Fix raw plan availability (#1521) Making a surgical change to avoid nil pointer panics on accessing RawPlan, coming from the AWS panic in https://github.com/pulumi/pulumi-aws/issues/2904 I have verified that this resolves 2904. --------- Co-authored-by: Ian Wahbe --- pkg/tfshim/sdk-v2/provider_diff.go | 23 ++++++++- pkg/tfshim/sdk-v2/provider_diff_test.go | 67 +++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 pkg/tfshim/sdk-v2/provider_diff_test.go diff --git a/pkg/tfshim/sdk-v2/provider_diff.go b/pkg/tfshim/sdk-v2/provider_diff.go index cf89f1c64..55099c133 100644 --- a/pkg/tfshim/sdk-v2/provider_diff.go +++ b/pkg/tfshim/sdk-v2/provider_diff.go @@ -165,7 +165,28 @@ func simpleDiffViaPlanState( if state.RawConfig.IsNull() { state.RawConfig = rawConfigVal } - return res.SimpleDiff(ctx, state, planned, meta) + diff, err := res.SimpleDiff(ctx, state, planned, meta) + if err != nil { + return nil, err + } + + // TF gRPC servers compensate for the fact that SimpleDiff may return + // terraform.NewInstanceDiff(), nil dropping any available information on RawPlan. + // + // See for example this code in PlanResourceChange: + // + // See https://github.com/hashicorp/terraform-plugin-sdk/blob/ + // 28e631776d97f0a5a5942b3524814addbef90875/helper/schema/grpc_provider.go#L797 + // + // In TF this is communicated from PlanResourceChange to ApplyResourceChange; unlike TF, in + // the current codebase InstanceDiff is passed directly to Apply. If RawPlan is not set on + // the diff it may cause nil panics in the provider. + if diff != nil && len(diff.Attributes) == 0 { + diff.RawPlan = priorStateVal + // TODO[pulumi/pulumi-terraform-bridge#1505] handle private state similar to upstream + } + + return diff, nil } func showDiffChangeType(b byte) string { diff --git a/pkg/tfshim/sdk-v2/provider_diff_test.go b/pkg/tfshim/sdk-v2/provider_diff_test.go new file mode 100644 index 000000000..3b296b01f --- /dev/null +++ b/pkg/tfshim/sdk-v2/provider_diff_test.go @@ -0,0 +1,67 @@ +// Copyright 2016-2023, Pulumi Corporation. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package sdkv2 + +import ( + "testing" + + "github.com/hashicorp/go-cty/cty" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRawPlanSet(t *testing.T) { + r := &schema.Resource{ + Schema: map[string]*schema.Schema{ + "tags": { + Type: schema.TypeMap, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, + }, + }, + } + p := &schema.Provider{ + ResourcesMap: map[string]*schema.Resource{"myres": r}, + } + + wp := NewProvider(p, WithDiffStrategy(PlanState)) + + state := cty.ObjectVal(map[string]cty.Value{ + "tags": cty.MapVal(map[string]cty.Value{"tag1": cty.StringVal("tag1v")}), + }) + + config := cty.ObjectVal(map[string]cty.Value{ + "tags": cty.MapVal(map[string]cty.Value{"tag1": cty.StringVal("tag1v")}), + }) + + instanceState := terraform.NewInstanceStateShimmedFromValue(state, 0) + instanceState.ID = "oldid" + instanceState.Meta = map[string]interface{}{} // ignore schema versions for this test + resourceConfig := terraform.NewResourceConfigShimmed(config, r.CoreConfigSchema()) + + ss := v2InstanceState{ + resource: r, + tf: instanceState, + } + + id, err := wp.Diff("myres", ss, v2ResourceConfig{ + tf: resourceConfig, + }) + require.NoError(t, err) + + assert.False(t, id.(v2InstanceDiff).tf.RawPlan.IsNull(), "RawPlan should not be Null") +}