diff --git a/internal/testprovider/schema.go b/internal/testprovider/schema.go index f781e6755..e78f07feb 100644 --- a/internal/testprovider/schema.go +++ b/internal/testprovider/schema.go @@ -621,8 +621,8 @@ func ProviderV2() *schemav2.Provider { }, Importer: &schemav2.ResourceImporter{ StateContext: func(_ context.Context, state *schemav2.ResourceData, - _ interface{}) ([]*schemav2.ResourceData, error) { - + _ interface{}, + ) ([]*schemav2.ResourceData, error) { return []*schemav2.ResourceData{state}, nil }, }, @@ -868,8 +868,8 @@ func AssertProvider(f func(data *schemav2.ResourceData)) *schemav2.Provider { }, Importer: &schemav2.ResourceImporter{ StateContext: func(_ context.Context, state *schemav2.ResourceData, - _ interface{}) ([]*schemav2.ResourceData, error) { - + _ interface{}, + ) ([]*schemav2.ResourceData, error) { return []*schemav2.ResourceData{state}, nil }, }, @@ -911,3 +911,21 @@ func CustomizedDiffProvider(f func(data *schemav2.ResourceData)) *schemav2.Provi }, } } + +func MaxItemsOneProvider() *schemav2.Provider { + return &schemav2.Provider{ + Schema: map[string]*schemav2.Schema{}, + ResourcesMap: map[string]*schemav2.Resource{ + "nested_str_res": { + Schema: map[string]*schemav2.Schema{ + "nested_str": { + Type: schemav2.TypeList, + MaxItems: 1, + Elem: &schemav2.Schema{Type: schemav2.TypeString}, + Optional: true, + }, + }, + }, + }, + } +} diff --git a/pkg/tfbridge/diff.go b/pkg/tfbridge/diff.go index 6c534e6f4..f3abc6e82 100644 --- a/pkg/tfbridge/diff.go +++ b/pkg/tfbridge/diff.go @@ -24,6 +24,7 @@ import ( pulumirpc "github.com/pulumi/pulumi/sdk/v3/proto/go" shim "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim" + shimutil "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/util" ) // containsComputedValues returns true if the given property value is or contains a computed value. @@ -102,7 +103,7 @@ func visitPropertyValue( // fill in default values for empty fields (note that this is a property of the field reader, not of // the schema) as it does when computing the hash code for a set element. ctx := &conversionContext{Ctx: ctx} - ev, err := ctx.MakeTerraformInput(ep, resource.PropertyValue{}, e, etfs, eps, rawNames) + ev, err := ctx.makeTerraformInput(ep, resource.PropertyValue{}, e, etfs, eps, rawNames) if err != nil { return } @@ -140,7 +141,7 @@ func visitPropertyValue( psflds = ps.Fields } - rawElementNames := rawNames || useRawNames(tfs) + rawElementNames := rawNames || shimutil.IsOfTypeMap(tfs) for k, e := range v.ObjectValue() { var elementPath string if strings.ContainsAny(string(k), `."[]`) { @@ -261,11 +262,11 @@ func doIgnoreChanges(ctx context.Context, tfs shim.SchemaMap, ps map[string]*Sch } for k, v := range olds { en, etf, eps := getInfoFromPulumiName(k, tfs, ps, false) - visitPropertyValue(ctx, en, string(k), v, etf, eps, useRawNames(etf), visitor) + visitPropertyValue(ctx, en, string(k), v, etf, eps, shimutil.IsOfTypeMap(etf), visitor) } for k, v := range news { en, etf, eps := getInfoFromPulumiName(k, tfs, ps, false) - visitPropertyValue(ctx, en, string(k), v, etf, eps, useRawNames(etf), visitor) + visitPropertyValue(ctx, en, string(k), v, etf, eps, shimutil.IsOfTypeMap(etf), visitor) } tfDiff.IgnoreChanges(ignoredKeySet) @@ -298,15 +299,15 @@ func makeDetailedDiff( diff := map[string]*pulumirpc.PropertyDiff{} for k, v := range olds { en, etf, eps := getInfoFromPulumiName(k, tfs, ps, false) - makePropertyDiff(ctx, en, string(k), v, tfDiff, diff, forceDiff, etf, eps, false, useRawNames(etf)) + makePropertyDiff(ctx, en, string(k), v, tfDiff, diff, forceDiff, etf, eps, false, shimutil.IsOfTypeMap(etf)) } for k, v := range news { en, etf, eps := getInfoFromPulumiName(k, tfs, ps, false) - makePropertyDiff(ctx, en, string(k), v, tfDiff, diff, forceDiff, etf, eps, false, useRawNames(etf)) + makePropertyDiff(ctx, en, string(k), v, tfDiff, diff, forceDiff, etf, eps, false, shimutil.IsOfTypeMap(etf)) } for k, v := range olds { en, etf, eps := getInfoFromPulumiName(k, tfs, ps, false) - makePropertyDiff(ctx, en, string(k), v, tfDiff, diff, forceDiff, etf, eps, true, useRawNames(etf)) + makePropertyDiff(ctx, en, string(k), v, tfDiff, diff, forceDiff, etf, eps, true, shimutil.IsOfTypeMap(etf)) } changes := pulumirpc.DiffResponse_DIFF_NONE diff --git a/pkg/tfbridge/provider.go b/pkg/tfbridge/provider.go index 6f1a68bc7..2ef0af342 100644 --- a/pkg/tfbridge/provider.go +++ b/pkg/tfbridge/provider.go @@ -34,6 +34,7 @@ import ( "golang.org/x/net/context" "google.golang.org/grpc/codes" + "github.com/pulumi/pulumi-terraform-bridge/v3/unstable/propertyvalue" "github.com/pulumi/pulumi/pkg/v3/resource/provider" "github.com/pulumi/pulumi/sdk/v3/go/common/diag" "github.com/pulumi/pulumi/sdk/v3/go/common/resource" @@ -678,6 +679,31 @@ func (p *Provider) Check(ctx context.Context, req *pulumirpc.CheckRequest) (*pul return &pulumirpc.CheckResponse{Inputs: minputs, Failures: failures}, nil } +// For properties with MaxItemsOne, where the state is still an array +// (i.e. from a previous version without MaxItemsOne) +// we need to mark them for update manually in order to correct the state +// from an array to a flat type. +// The diff is otherwise ignored since MakeTerraformInputs won't touch +// the type if it in the right shape. +func markWronglyTypedMaxItemsOneStateDiff( + schema shim.SchemaMap, info map[string]*SchemaInfo, olds resource.PropertyMap, +) bool { + res := False() + tr := func(pulumiPath resource.PropertyPath, localValue resource.PropertyValue) (resource.PropertyValue, error) { + schemaPath := PropertyPathToSchemaPath(pulumiPath, schema, info) + localSchema, info, err := LookupSchemas(schemaPath, schema, info) + contract.IgnoreError(err) + if IsMaxItemsOne(localSchema, info) && localValue.IsArray() { + glog.V(9).Infof("Found type mismatch for %s, flagging for update.", pulumiPath) + *res = true + } + return localValue, nil // don't change just visit + } + _, err := propertyvalue.TransformPropertyValue(make(resource.PropertyPath, 0), tr, resource.NewObjectProperty(olds)) + contract.AssertNoErrorf(err, "markWronglyTypedMaxItemsOneStateDiff should not return errors!") + return *res +} + // Diff checks what impacts a hypothetical update will have on the resource's properties. func (p *Provider) Diff(ctx context.Context, req *pulumirpc.DiffRequest) (*pulumirpc.DiffResponse, error) { ctx = p.loggingContext(ctx, resource.URN(req.GetUrn())) @@ -799,6 +825,12 @@ func (p *Provider) Diff(ctx context.Context, req *pulumirpc.DiffRequest) (*pulum changes = pulumirpc.DiffResponse_DIFF_SOME } + if changes == pulumirpc.DiffResponse_DIFF_NONE && + markWronglyTypedMaxItemsOneStateDiff(res.TF.Schema(), res.Schema.Fields, olds) { + + changes = pulumirpc.DiffResponse_DIFF_SOME + } + return &pulumirpc.DiffResponse{ Changes: changes, Replaces: replaces, diff --git a/pkg/tfbridge/provider_test.go b/pkg/tfbridge/provider_test.go index d7e529216..eeb5fcb96 100644 --- a/pkg/tfbridge/provider_test.go +++ b/pkg/tfbridge/provider_test.go @@ -2133,3 +2133,107 @@ func TestTransformFromState(t *testing.T) { }`) }) } + +// This emulates the situation where we migrate from a state without maxItemsOne +// which would make the property a list +// into a state with maxItemsOne, which would flatten the type. +// https://github.com/pulumi/pulumi-aws/issues/3092 +func TestMaxItemOneWrongStateDiff(t *testing.T) { + p := testprovider.MaxItemsOneProvider() + provider := &Provider{ + tf: shimv2.NewProvider(p), + config: shimv2.NewSchemaMap(p.Schema), + resources: map[tokens.Type]Resource{ + "NestedStrRes": { + TF: shimv2.NewResource(p.ResourcesMap["nested_str_res"]), + TFName: "nested_str_res", + Schema: &ResourceInfo{ + Tok: "NestedStrRes", + Fields: map[string]*SchemaInfo{}, + }, + }, + }, + } + t.Run("DiffListAndVal", func(t *testing.T) { + testutils.Replay(t, provider, ` + { + "method": "/pulumirpc.ResourceProvider/Diff", + "request": { + "urn": "urn:pulumi:dev::teststack::NestedStrRes::exres", + "id": "0", + "olds": { + "nested_str": [] + }, + "news": { + "nested_str": "" + } + }, + "response": { + "changes": "DIFF_SOME", + "hasDetailedDiff": true + } + }`) + }) + t.Run("DiffListAndValNonEmpty", func(t *testing.T) { + testutils.Replay(t, provider, ` + { + "method": "/pulumirpc.ResourceProvider/Diff", + "request": { + "urn": "urn:pulumi:dev::teststack::NestedStrRes::exres", + "id": "0", + "olds": { + "nested_str": ["val"] + }, + "news": { + "nested_str": "val" + } + }, + "response": { + "changes": "DIFF_SOME", + "hasDetailedDiff": true + } + }`) + }) + + // Also check that we don't produce spurious diffs when not necessary. + t.Run("DiffValAndValEmpty", func(t *testing.T) { + testutils.Replay(t, provider, ` + { + "method": "/pulumirpc.ResourceProvider/Diff", + "request": { + "urn": "urn:pulumi:dev::teststack::NestedStrRes::exres", + "id": "0", + "olds": { + "nested_str": "" + }, + "news": { + "nested_str": "" + } + }, + "response": { + "changes": "DIFF_NONE", + "hasDetailedDiff": true + } + }`) + }) + t.Run("DiffValAndValNonempty", func(t *testing.T) { + testutils.Replay(t, provider, ` + { + "method": "/pulumirpc.ResourceProvider/Diff", + "request": { + "urn": "urn:pulumi:dev::teststack::NestedStrRes::exres", + "id": "0", + "olds": { + "nested_str": "val" + }, + "news": { + "nested_str": "val" + } + }, + "response": { + "changes": "DIFF_NONE", + "hasDetailedDiff": true + } + }`) + }) +} diff --git a/pkg/tfbridge/schema.go b/pkg/tfbridge/schema.go index 515b08782..823488d47 100644 --- a/pkg/tfbridge/schema.go +++ b/pkg/tfbridge/schema.go @@ -27,11 +27,14 @@ import ( "github.com/golang/glog" pbstruct "github.com/golang/protobuf/ptypes/struct" "github.com/pkg/errors" - shim "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim" - "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/schema" + "github.com/pulumi/pulumi/sdk/v3/go/common/resource" "github.com/pulumi/pulumi/sdk/v3/go/common/resource/plugin" "github.com/pulumi/pulumi/sdk/v3/go/common/util/contract" + + shim "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim" + "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/schema" + shimutil "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/util" ) // This file deals with translating between the Pulumi representations of a resource's configuration and state and the @@ -314,19 +317,24 @@ func MakeTerraformInputs( ApplyDefaults: true, Assets: AssetTable{}, } - inputs, err := cctx.MakeTerraformInputs(olds, news, tfs, ps, false) + inputs, err := cctx.makeTerraformInputs(olds, news, tfs, ps) if err != nil { return nil, nil, err } return inputs, cctx.Assets, err } -// MakeTerraformInput takes a single property plus custom schema info and does whatever is necessary to prepare it for -// use by Terraform. Note that this function may have side effects, for instance if it is necessary to spill an asset -// to disk in order to create a name out of it. Please take care not to call it superfluously! -func (ctx *conversionContext) MakeTerraformInput(name string, old, v resource.PropertyValue, - tfs shim.Schema, ps *SchemaInfo, rawNames bool) (interface{}, error) { - +// makeTerraformInput takes a single property plus custom schema info and does whatever is necessary +// to prepare it for use by Terraform. Note that this function may have side effects, for instance +// if it is necessary to spill an asset to disk in order to create a name out of it. Please take +// care not to call it superfluously! +func (ctx *conversionContext) makeTerraformInput( + name string, + old, v resource.PropertyValue, + tfs shim.Schema, + ps *SchemaInfo, + rawNames bool, +) (interface{}, error) { // For TypeList or TypeSet with MaxItems==1, we will have projected as a scalar // nested value, and need to wrap it into a single-element array before passing to // Terraform. @@ -409,7 +417,7 @@ func (ctx *conversionContext) MakeTerraformInput(name string, old, v resource.Pr oldElem = oldArr[i] } elemName := fmt.Sprintf("%v[%v]", name, i) - e, err := ctx.MakeTerraformInput(elemName, oldElem, elem, etfs, eps, rawNames) + e, err := ctx.makeTerraformInput(elemName, oldElem, elem, etfs, eps, rawNames) if err != nil { return nil, err } @@ -448,25 +456,46 @@ func (ctx *conversionContext) MakeTerraformInput(name string, old, v resource.Pr } return ps.Asset.TranslateArchive(v.ArchiveValue()) case v.IsObject(): - var tfflds shim.SchemaMap - if tfs != nil { - if res, isres := tfs.Elem().(shim.Resource); isres { - tfflds = res.Schema() - } - } - var psflds map[string]*SchemaInfo - if ps != nil { - psflds = ps.Fields - } var oldObject resource.PropertyMap if old.IsObject() { oldObject = old.ObjectValue() } - input, err := ctx.MakeTerraformInputs(oldObject, v.ObjectValue(), - tfflds, psflds, rawNames || useRawNames(tfs)) - if err != nil { - return nil, err + var input map[string]interface{} + if shimutil.IsOfTypeMap(tfs) { + var tfsElem shim.Schema + if tfs != nil { + if s, ok := tfs.Elem().(shim.Schema); ok { + tfsElem = s + } + } + var psElem *SchemaInfo + if ps != nil { + psElem = ps.Elem + } + var err error + input, err = ctx.makeMapTerraformInputs(oldObject, v.ObjectValue(), + tfsElem, psElem) + if err != nil { + return nil, err + } + } else { + var tfflds shim.SchemaMap + if tfs != nil { + if res, isres := tfs.Elem().(shim.Resource); isres { + tfflds = res.Schema() + } + } + var psflds map[string]*SchemaInfo + if ps != nil { + psflds = ps.Fields + } + var err error + input, err = ctx.makeObjectTerraformInputs(oldObject, v.ObjectValue(), + tfflds, psflds, rawNames) + if err != nil { + return nil, err + } } // If we have schema information that indicates that this value is being presented to a map-typed field whose @@ -489,17 +518,70 @@ func (ctx *conversionContext) MakeTerraformInput(name string, old, v resource.Pr } -// MakeTerraformInputs takes a property map plus custom schema info and does whatever is necessary +// makeTerraformInputs takes a property map plus custom schema info and does whatever is necessary // to prepare it for use by Terraform. Note that this function may have side effects, for instance // if it is necessary to spill an asset to disk in order to create a name out of it. Please take // care not to call it superfluously! -func (ctx *conversionContext) MakeTerraformInputs( +func (ctx *conversionContext) makeTerraformInputs( olds, news resource.PropertyMap, tfs shim.SchemaMap, ps map[string]*SchemaInfo, - rawNames bool, ) (map[string]interface{}, error) { + return ctx.makeObjectTerraformInputs(olds, news, tfs, ps, false /*rawNames*/) +} +// Should only be called from inside makeTerraformInputs. Variation for makeTerraformInputs used +// when the schema indicates that the code is handling a map[string,X] case and not an object. +func (ctx *conversionContext) makeMapTerraformInputs( + olds, news resource.PropertyMap, + tfsElement shim.Schema, + psElement *SchemaInfo, +) (map[string]interface{}, error) { + result := make(map[string]interface{}) + + for key, value := range news { + // Map keys always preserved as-is, no translation necessary. + name := string(key) + + var old resource.PropertyValue + if ctx.ApplyDefaults && olds != nil { + old = olds[key] + } + + // If type information is lost (the map's element type is unknown), subsequent + // nested PropertyMap values will be treated by makeObjectTerraformInputs. In this + // case rawNames=true needs to make sure makeObjectTerraformInputs does not mangle + // pulumiStyleLabels into terraform_style_labels. This is the legacy behavior + // enforced by tests. It also makes intuitive sense: absent schema information the + // code should not be doing name mangling. + rawNames := tfsElement == nil + + v, err := ctx.makeTerraformInput(name, old, value, tfsElement, psElement, rawNames) + if err != nil { + return nil, err + } + result[name] = v + glog.V(9).Infof("Created Terraform input: %v = %v", name, v) + } + + if glog.V(5) { + for k, v := range result { + glog.V(5).Infof("Terraform input %v = %#v", k, v) + } + } + + return result, nil +} + +// Should only be called from inside makeTerraformInputs. This variation should only be handling the +// case when an object type is expected, or else the schema is lost and the translation is not sure +// of the expected type. The case when map types are expected is handled by makeMapTerraformInputs. +func (ctx *conversionContext) makeObjectTerraformInputs( + olds, news resource.PropertyMap, + tfs shim.SchemaMap, + ps map[string]*SchemaInfo, + rawNames bool, +) (map[string]interface{}, error) { result := make(map[string]interface{}) tfAttributesToPulumiProperties := make(map[string]string) @@ -544,7 +626,7 @@ func (ctx *conversionContext) MakeTerraformInputs( } // And then translate the property value. - v, err := ctx.MakeTerraformInput(name, old, value, tfi, psi, rawNames) + v, err := ctx.makeTerraformInput(name, old, value, tfi, psi, rawNames) if err != nil { return nil, err } @@ -674,7 +756,7 @@ func (ctx *conversionContext) applyDefaults( key, tfi, psi := getInfoFromTerraformName(name, tfs, ps, rawNames) if old, hasold := olds[key]; hasold && useOldDefault(key) { - v, err := ctx.MakeTerraformInput(name, resource.PropertyValue{}, + v, err := ctx.makeTerraformInput(name, resource.PropertyValue{}, old, tfi, psi, rawNames) if err != nil { return err @@ -717,7 +799,7 @@ func (ctx *conversionContext) applyDefaults( defaultValue, source = v, "env vars" } else if configKey := info.Default.Config; configKey != "" { if v := ctx.ProviderConfig[resource.PropertyKey(configKey)]; !v.IsNull() { - tv, err := ctx.MakeTerraformInput(name, resource.PropertyValue{}, v, tfi, psi, rawNames) + tv, err := ctx.makeTerraformInput(name, resource.PropertyValue{}, v, tfi, psi, rawNames) if err != nil { return err } @@ -725,7 +807,7 @@ func (ctx *conversionContext) applyDefaults( } } else if info.Default.Value != nil { v := resource.NewPropertyValue(info.Default.Value) - tv, err := ctx.MakeTerraformInput(name, resource.PropertyValue{}, v, tfi, psi, rawNames) + tv, err := ctx.makeTerraformInput(name, resource.PropertyValue{}, v, tfi, psi, rawNames) if err != nil { return err } @@ -827,7 +909,7 @@ func (ctx *conversionContext) applyDefaults( key, tfi, psi := getInfoFromTerraformName(name, tfs, ps, rawNames) if old, hasold := olds[key]; hasold && useOldDefault(key) { - v, err := ctx.MakeTerraformInput(name, resource.PropertyValue{}, old, tfi, psi, rawNames) + v, err := ctx.makeTerraformInput(name, resource.PropertyValue{}, old, tfi, psi, rawNames) if err != nil { valueErr = err return false @@ -1074,7 +1156,7 @@ func MakeTerraformOutput(p shim.Provider, v interface{}, if ps != nil { psflds = ps.Fields } - obj := MakeTerraformOutputs(p, outs, tfflds, psflds, assets, rawNames || useRawNames(tfs), supportsSecrets) + obj := MakeTerraformOutputs(p, outs, tfflds, psflds, assets, rawNames || shimutil.IsOfTypeMap(tfs), supportsSecrets) return resource.NewObjectProperty(obj) default: contract.Failf("Unexpected TF output property value: %#v with type %#T", v, v) @@ -1101,7 +1183,7 @@ func MakeTerraformConfig(ctx context.Context, p *Provider, m resource.PropertyMa ProviderConfig: p.configValues, Assets: AssetTable{}, } - inputs, err := cctx.MakeTerraformInputs(nil, m, tfs, ps, false) + inputs, err := cctx.makeTerraformInputs(nil, m, tfs, ps) if err != nil { return nil, nil, err } @@ -1176,7 +1258,7 @@ func MakeTerraformState( // Mappable, but we use MapReplace because we use float64s and Terraform uses // ints, to represent numbers. cctx := &conversionContext{Ctx: ctx} - inputs, err := cctx.MakeTerraformInputs(nil, m, res.TF.Schema(), res.Schema.Fields, false) + inputs, err := cctx.makeTerraformInputs(nil, m, res.TF.Schema(), res.Schema.Fields) if err != nil { return nil, err } @@ -1219,16 +1301,6 @@ func IsMaxItemsOne(tfs shim.Schema, info *SchemaInfo) bool { return tfs.MaxItems() == 1 } -// useRawNames returns true if raw, unmangled names should be preserved. This is only true for Terraform maps with -// an Elem that is not a shim.Resource. -func useRawNames(tfs shim.Schema) bool { - if tfs == nil || tfs.Type() != shim.TypeMap { - return false - } - _, hasResourceElem := tfs.Elem().(shim.Resource) - return !hasResourceElem -} - // getInfoFromTerraformName does a map lookup to find the Pulumi name and schema info, if any. func getInfoFromTerraformName(key string, tfs shim.SchemaMap, ps map[string]*SchemaInfo, rawName bool) (resource.PropertyKey, @@ -1403,8 +1475,8 @@ func extractInputs(oldInput, newState resource.PropertyValue, tfs shim.Schema, p for name, oldValue := range oldMap { defaultElem := false if newValue, ok := newMap[name]; ok { - _, etfs, eps := getInfoFromPulumiName(name, tfflds, psflds, rawNames || useRawNames(tfs)) - oldMap[name], defaultElem = extractInputs(oldValue, newValue, etfs, eps, rawNames || useRawNames(tfs)) + _, etfs, eps := getInfoFromPulumiName(name, tfflds, psflds, rawNames || shimutil.IsOfTypeMap(tfs)) + oldMap[name], defaultElem = extractInputs(oldValue, newValue, etfs, eps, rawNames || shimutil.IsOfTypeMap(tfs)) } else { delete(oldMap, name) } @@ -1511,7 +1583,7 @@ func extractSchemaInputs(state resource.PropertyValue, tfs shim.Schema, ps *Sche a := state.ArrayValue() v := make([]resource.PropertyValue, len(a)) for i := range a { - v[i] = extractSchemaInputs(a[i], etfs, eps, rawNames || useRawNames(tfs)) + v[i] = extractSchemaInputs(a[i], etfs, eps, rawNames || shimutil.IsOfTypeMap(tfs)) } return resource.NewArrayProperty(v) case state.IsObject(): @@ -1530,13 +1602,13 @@ func extractSchemaInputs(state resource.PropertyValue, tfs shim.Schema, ps *Sche obj := state.ObjectValue() v := make(map[resource.PropertyKey]resource.PropertyValue, len(obj)) for k, e := range obj { - _, etfs, eps := getInfoFromPulumiName(k, tfflds, psflds, rawNames || useRawNames(tfs)) + _, etfs, eps := getInfoFromPulumiName(k, tfflds, psflds, rawNames || shimutil.IsOfTypeMap(tfs)) if isInput := isMap || etfs != nil && (etfs.Optional() || etfs.Required()); !isInput { glog.V(9).Infof("skipping '%v' (not an input)", k) continue } - ev := extractSchemaInputs(e, etfs, eps, rawNames || useRawNames(tfs)) + ev := extractSchemaInputs(e, etfs, eps, rawNames || shimutil.IsOfTypeMap(tfs)) if mustSet := isMap || etfs != nil && (etfs.Required() || !isDefaultOrZeroValue(etfs, eps, ev)); !mustSet { glog.V(9).Infof("skipping '%v' (not required + default or zero value)", k) continue diff --git a/pkg/tfbridge/schema_test.go b/pkg/tfbridge/schema_test.go index 1d622f566..7d1f75e21 100644 --- a/pkg/tfbridge/schema_test.go +++ b/pkg/tfbridge/schema_test.go @@ -25,13 +25,14 @@ import ( structpb "github.com/golang/protobuf/ptypes/struct" schemav1 "github.com/hashicorp/terraform-plugin-sdk/helper/schema" schemav2 "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/pulumi/pulumi/sdk/v3/go/common/resource" "github.com/pulumi/pulumi/sdk/v3/go/common/resource/plugin" "github.com/pulumi/pulumi/sdk/v3/go/common/tokens" "github.com/pulumi/pulumi/sdk/v3/go/common/util/contract" pulumirpc "github.com/pulumi/pulumi/sdk/v3/proto/go" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "github.com/pulumi/pulumi-terraform-bridge/v3/internal/testprovider" shim "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim" @@ -44,7 +45,7 @@ func makeTerraformInputs(olds, news resource.PropertyMap, tfs shim.SchemaMap, ps map[string]*SchemaInfo) (map[string]interface{}, AssetTable, error) { ctx := &conversionContext{Assets: AssetTable{}} - inputs, err := ctx.MakeTerraformInputs(olds, news, tfs, ps, false) + inputs, err := ctx.makeTerraformInputs(olds, news, tfs, ps) if err != nil { return nil, nil, err } @@ -58,7 +59,7 @@ func makeTerraformInputsWithDefaults(olds, news resource.PropertyMap, Assets: AssetTable{}, ApplyDefaults: true, } - inputs, err := ctx.MakeTerraformInputs(olds, news, tfs, ps, false) + inputs, err := ctx.makeTerraformInputs(olds, news, tfs, ps) if err != nil { return nil, nil, err } @@ -67,7 +68,7 @@ func makeTerraformInputsWithDefaults(olds, news resource.PropertyMap, func makeTerraformInput(v resource.PropertyValue, tfs shim.Schema, ps *SchemaInfo) (interface{}, error) { ctx := &conversionContext{} - return ctx.MakeTerraformInput("v", resource.PropertyValue{}, v, tfs, ps, false) + return ctx.makeTerraformInput("v", resource.PropertyValue{}, v, tfs, ps, false) } // TestTerraformInputs verifies that we translate Pulumi inputs into Terraform inputs. @@ -2452,3 +2453,111 @@ func TestOutputNumberTypes(t *testing.T) { "ggg": resource.NewNumberProperty(50), }, outputs) } + +func TestMakeTerraformInputsOnMapNestedObjects(t *testing.T) { + r := &schemav2.Resource{ + Schema: map[string]*schemav2.Schema{ + "map_prop": { + Type: schemav2.TypeMap, + Optional: true, + Elem: &schemav2.Schema{ + Type: schemav2.TypeList, + Optional: true, + Elem: &schemav2.Resource{ + Schema: map[string]*schemav2.Schema{ + "x_prop": { + Optional: true, + Type: schemav2.TypeString, + }, + }, + }, + }, + }, + }, + } + + shimmedR := shimv2.NewResource(r) + ctx := context.Background() + var instance *PulumiResource + + type testCase struct { + name string + ps map[string]*SchemaInfo + config resource.PropertyMap + news resource.PropertyMap + olds resource.PropertyMap + expect interface{} + } + + testCases := []testCase{ + { + name: "translates x_prop", + news: resource.PropertyMap{ + "mapProp": resource.NewObjectProperty(resource.PropertyMap{ + "elem1": resource.NewArrayProperty([]resource.PropertyValue{ + resource.NewObjectProperty(resource.PropertyMap{ + "xProp": resource.NewStringProperty("xPropValue"), + }), + }), + }), + }, + expect: map[string]interface{}{ + "__defaults": []interface{}{}, + "map_prop": map[string]interface{}{ + "elem1": []interface{}{ + map[string]interface{}{ + "__defaults": []interface{}{}, + "x_prop": "xPropValue", + }, + }, + }, + }, + }, + { + name: "respects x_prop renames", + news: resource.PropertyMap{ + "mapProp": resource.NewObjectProperty(resource.PropertyMap{ + "elem1": resource.NewArrayProperty([]resource.PropertyValue{ + resource.NewObjectProperty(resource.PropertyMap{ + "x": resource.NewStringProperty("xPropValue"), + }), + }), + }), + }, + ps: map[string]*SchemaInfo{ + "map_prop": { + Elem: &SchemaInfo{ + Elem: &SchemaInfo{ + Fields: map[string]*SchemaInfo{ + "x_prop": { + Name: "x", + }, + }, + }, + }, + }, + }, + expect: map[string]interface{}{ + "__defaults": []interface{}{}, + "map_prop": map[string]interface{}{ + "elem1": []interface{}{ + map[string]interface{}{ + "__defaults": []interface{}{}, + "x_prop": "xPropValue", + }, + }, + }, + }, + }, + } + + for _, tc := range testCases { + tc := tc + + t.Run(tc.name, func(t *testing.T) { + i, _, err := MakeTerraformInputs(ctx, instance, tc.config, tc.olds, tc.news, shimmedR.Schema(), tc.ps) + require.NoError(t, err) + require.Equal(t, tc.expect, i) + }) + } +} diff --git a/pkg/tfshim/util/types.go b/pkg/tfshim/util/types.go new file mode 100644 index 000000000..25a9793ab --- /dev/null +++ b/pkg/tfshim/util/types.go @@ -0,0 +1,30 @@ +// 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 util + +import ( + "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim" +) + +// isOfTypeMap detects schemas indicating a map[string,X] type. Due to a quirky encoding of +// single-nested Terraform blocks, it is insufficient to just check for tfs.Type() == shim.TypeMap. +// See [shim.Schema.Elem()] comment for all the details of the encoding. +func IsOfTypeMap(tfs shim.Schema) bool { + if tfs == nil || tfs.Type() != shim.TypeMap { + return false + } + _, hasResourceElem := tfs.Elem().(shim.Resource) + return !hasResourceElem +} diff --git a/pkg/tfshim/util/types_test.go b/pkg/tfshim/util/types_test.go new file mode 100644 index 000000000..9c58b82ae --- /dev/null +++ b/pkg/tfshim/util/types_test.go @@ -0,0 +1,69 @@ +// 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 util + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + shim "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim" + "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/schema" +) + +func TestIsOfTypeMap(t *testing.T) { + testCases := []struct { + name string + expectIsMap bool + schema shim.Schema + }{ + { + "nil", false, + nil, + }, + { + "int", false, + (&schema.Schema{ + Type: shim.TypeInt, + }).Shim(), + }, + { + "string_map", true, + (&schema.Schema{ + Type: shim.TypeMap, + Elem: (&schema.Schema{ + Type: shim.TypeString, + }).Shim(), + }).Shim(), + }, + { + "single_nested_block", false, + (&schema.Schema{ + Type: shim.TypeMap, + Elem: (&schema.Resource{ + Schema: &schema.SchemaMap{}, + }).Shim(), + }).Shim(), + }, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + actual := IsOfTypeMap(tc.schema) + assert.Equal(t, tc.expectIsMap, actual) + }) + } +}