Skip to content

Commit

Permalink
Fix raw plan availability (#1521)
Browse files Browse the repository at this point in the history
Making a surgical change to avoid nil pointer panics on accessing
RawPlan, coming from the AWS panic in
pulumi/pulumi-aws#2904

I have verified that this resolves 2904.

---------

Co-authored-by: Ian Wahbe <[email protected]>
  • Loading branch information
t0yv0 and iwahbe authored Nov 15, 2023
1 parent 6085352 commit 0ac62de
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 1 deletion.
23 changes: 22 additions & 1 deletion pkg/tfshim/sdk-v2/provider_diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
67 changes: 67 additions & 0 deletions pkg/tfshim/sdk-v2/provider_diff_test.go
Original file line number Diff line number Diff line change
@@ -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")
}

0 comments on commit 0ac62de

Please sign in to comment.