From 5048d601dc66a9babe6c8d6e6e577fac2ac983ac Mon Sep 17 00:00:00 2001 From: Ian Wahbe Date: Wed, 10 Apr 2024 16:46:31 +0200 Subject: [PATCH] Breaking: Remove rawNames from `MakeTerraformOutput` & `MakeTerraformOutputs` This has the same effect as 2b0bc802256604ba45fa933ba929bfb74394319f, and needs the same test case adjustment. This is a "breaking change" to public functions, but we don't expect that these functions will be used outside of the bridge and we have made breaking changes to them in the past (https://github.com/pulumi/pulumi-terraform-bridge/pull/1642). --- pkg/tfbridge/provider.go | 2 +- pkg/tfbridge/schema.go | 46 ++++++++++++++++++++++--------------- pkg/tfbridge/schema_test.go | 41 ++++++++++++++++++--------------- 3 files changed, 50 insertions(+), 39 deletions(-) diff --git a/pkg/tfbridge/provider.go b/pkg/tfbridge/provider.go index 3f1d8a65f0..44dc93ef6f 100644 --- a/pkg/tfbridge/provider.go +++ b/pkg/tfbridge/provider.go @@ -810,7 +810,7 @@ func (p *Provider) Check(ctx context.Context, req *pulumirpc.CheckRequest) (*pul // After all is said and done, we need to go back and return only what got populated as a diff from the origin. pinputs := MakeTerraformOutputs( - ctx, p.tf, inputs, res.TF.Schema(), res.Schema.Fields, assets, false, p.supportsSecrets, + ctx, p.tf, inputs, res.TF.Schema(), res.Schema.Fields, assets, p.supportsSecrets, ) pinputsWithSecrets := MarkSchemaSecrets(ctx, res.TF.Schema(), res.Schema.Fields, diff --git a/pkg/tfbridge/schema.go b/pkg/tfbridge/schema.go index 3c78e3586a..752fa50e57 100644 --- a/pkg/tfbridge/schema.go +++ b/pkg/tfbridge/schema.go @@ -1021,7 +1021,7 @@ func MakeTerraformResult( outs = obj } - outMap := MakeTerraformOutputs(ctx, p, outs, tfs, ps, assets, false, supportsSecrets) + outMap := MakeTerraformOutputs(ctx, p, outs, tfs, ps, assets, supportsSecrets) // If there is any Terraform metadata associated with this state, record it. if state != nil && len(state.Meta()) != 0 { @@ -1042,18 +1042,17 @@ func MakeTerraformOutputs( tfs shim.SchemaMap, ps map[string]*SchemaInfo, assets AssetTable, - rawNames, supportsSecrets bool, ) resource.PropertyMap { result := make(resource.PropertyMap) for key, value := range outs { // First do a lookup of the name/info. - name, tfi, psi := getInfoFromTerraformName(key, tfs, ps, rawNames) + name, tfi, psi := getInfoFromTerraformName(key, tfs, ps, false) contract.Assertf(name != "", `name != ""`) // Next perform a translation of the value accordingly. - out := MakeTerraformOutput(ctx, p, value, tfi, psi, assets, rawNames, supportsSecrets) + out := MakeTerraformOutput(ctx, p, value, tfi, psi, assets, supportsSecrets) //if !out.IsNull() { result[name] = out //} @@ -1076,11 +1075,11 @@ func MakeTerraformOutput( tfs shim.Schema, ps *SchemaInfo, assets AssetTable, - rawNames, supportsSecrets bool, + supportsSecrets bool, ) resource.PropertyValue { buildOutput := func(p shim.Provider, v interface{}, - tfs shim.Schema, ps *SchemaInfo, assets AssetTable, rawNames, supportsSecrets bool) resource.PropertyValue { + tfs shim.Schema, ps *SchemaInfo, assets AssetTable, supportsSecrets bool) resource.PropertyValue { if assets != nil && ps != nil && ps.Asset != nil { if asset, has := assets[ps]; has { // if we have the value, it better actually be an asset or an archive. @@ -1137,7 +1136,7 @@ func MakeTerraformOutput( if err != nil || coerced == t { return resource.NewStringProperty(t) } - return MakeTerraformOutput(ctx, p, coerced, tfs, ps, assets, rawNames, supportsSecrets) + return MakeTerraformOutput(ctx, p, coerced, tfs, ps, assets, supportsSecrets) case reflect.Slice: elems := []interface{}{} for i := 0; i < val.Len(); i++ { @@ -1148,7 +1147,7 @@ func MakeTerraformOutput( var arr []resource.PropertyValue for _, elem := range elems { - arr = append(arr, MakeTerraformOutput(ctx, p, elem, tfes, pes, assets, rawNames, supportsSecrets)) + arr = append(arr, MakeTerraformOutput(ctx, p, elem, tfes, pes, assets, supportsSecrets)) } // For TypeList or TypeSet with MaxItems==1, we will have projected as a scalar nested value, so need to extract // out the single element (or null). @@ -1164,24 +1163,33 @@ func MakeTerraformOutput( } return resource.NewArrayProperty(arr) case reflect.Map: - outs := map[string]interface{}{} + // Build a go map of output values. + outs := map[string]any{} for _, key := range val.MapKeys() { contract.Assertf(key.Kind() == reflect.String, "key.Kind() == reflect.String") outs[key.String()] = val.MapIndex(key).Interface() } - var tfflds shim.SchemaMap + if tfs != nil { - if res, isres := tfs.Elem().(shim.Resource); isres { - tfflds = res.Schema() + // This is an object, so we need key translations. + if res, ok := tfs.Elem().(shim.Resource); ok { + var psflds map[string]*SchemaInfo + if ps != nil { + psflds = ps.Fields + } + obj := MakeTerraformOutputs(ctx, p, outs, + res.Schema(), psflds, assets, supportsSecrets) + return resource.NewObjectProperty(obj) } } - var psflds map[string]*SchemaInfo - if ps != nil { - psflds = ps.Fields + + // It's not an object, so it must be a map + obj := make(resource.PropertyMap, len(outs)) + etfs, eps := elemSchemas(tfs, ps) + for k, v := range outs { + obj[resource.PropertyKey(k)] = MakeTerraformOutput(ctx, p, v, + etfs, eps, assets, supportsSecrets) } - obj := MakeTerraformOutputs( - ctx, 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) @@ -1189,7 +1197,7 @@ func MakeTerraformOutput( } } - output := buildOutput(p, v, tfs, ps, assets, rawNames, supportsSecrets) + output := buildOutput(p, v, tfs, ps, assets, supportsSecrets) if tfs != nil && tfs.Sensitive() && supportsSecrets { return resource.MakeSecret(output) diff --git a/pkg/tfbridge/schema_test.go b/pkg/tfbridge/schema_test.go index 3b4a1bfa16..9c7fab1cd4 100644 --- a/pkg/tfbridge/schema_test.go +++ b/pkg/tfbridge/schema_test.go @@ -591,10 +591,18 @@ func TestTerraformOutputsWithSecretsSupported(t *testing.T) { }, }, f.NewSchemaMap(map[string]*schema.Schema{ - // Type mapPropertyValue as a map so that keys aren't mangled in the usual way. "float_property_value": {Type: shim.TypeFloat}, "my_string_property_value": {Type: shim.TypeString}, - "map_property_value": {Type: shim.TypeMap}, + "object_property_value": { + Elem: (&schema.Resource{ + Schema: schemaMap(map[string]*schema.Schema{ + "property_a": {Type: shim.TypeString, Required: true}, + "property_b": {Type: shim.TypeBool, Optional: true}, + }), + }).Shim(), + }, + // Type mapPropertyValue as a map so that keys aren't mangled in the usual way. + "map_property_value": {Type: shim.TypeMap}, "nested_resource": { Type: shim.TypeList, MaxItems: 2, @@ -654,9 +662,8 @@ func TestTerraformOutputsWithSecretsSupported(t *testing.T) { MaxItemsOne: boolPointer(true), }, }, - nil, /* assets */ - false, /* useRawNames */ - true, /* supportsSecrets */ + nil, /* assets */ + true, /* supportsSecrets */ ) assert.Equal(t, resource.NewPropertyMapFromMap(map[string]interface{}{ "nilPropertyValue": nil, @@ -727,8 +734,7 @@ func TestTerraformOutputsWithSecretsUnsupported(t *testing.T) { }, }), map[string]*SchemaInfo{}, - nil, /* assets */ - false, /*useRawNames*/ + nil, /* assets */ false, ) assert.Equal(t, resource.NewPropertyMapFromMap(map[string]interface{}{ @@ -1115,7 +1121,7 @@ func TestDefaults(t *testing.T) { } inputs, assets, err := makeTerraformInputsForConfig(olds, props, tfs, ps) assert.NoError(t, err) - outputs := MakeTerraformOutputs(ctx, f.NewTestProvider(), inputs, tfs, ps, assets, false, true) + outputs := MakeTerraformOutputs(ctx, f.NewTestProvider(), inputs, tfs, ps, assets, true) // sort the defaults list before the equality test below. sortDefaultsList(outputs) @@ -1161,7 +1167,7 @@ func TestDefaults(t *testing.T) { assert.Equal(t, "true", inputs["x2stringxbool"]) assert.Equal(t, "1", inputs["x2stringxint"]) - outputs = MakeTerraformOutputs(ctx, f.NewTestProvider(), inputs, tfs, ps, assets, false, true) + outputs = MakeTerraformOutputs(ctx, f.NewTestProvider(), inputs, tfs, ps, assets, true) // sort the defaults list before the equality test below. sortDefaultsList(outputs) @@ -1230,7 +1236,7 @@ func TestDefaultsConflictsWith(t *testing.T) { inputs, assets, err := makeTerraformInputsForConfig(olds, props, tfs, ps) assert.NoError(t, err) - outputs := MakeTerraformOutputs(ctx, f.NewTestProvider(), inputs, tfs, ps, assets, false, true) + outputs := MakeTerraformOutputs(ctx, f.NewTestProvider(), inputs, tfs, ps, assets, true) sortDefaultsList(outputs) assert.Equal(t, resource.NewPropertyMapFromMap(map[string]interface{}{ @@ -1250,7 +1256,7 @@ func TestDefaultsConflictsWith(t *testing.T) { inputs, assets, err = makeTerraformInputsForConfig(olds, props, tfs, ps) assert.NoError(t, err) - outputs = MakeTerraformOutputs(ctx, f.NewTestProvider(), inputs, tfs, ps, assets, false, true) + outputs = MakeTerraformOutputs(ctx, f.NewTestProvider(), inputs, tfs, ps, assets, true) sortDefaultsList(outputs) assert.Equal(t, resource.NewPropertyMapFromMap(map[string]interface{}{ @@ -1284,7 +1290,7 @@ func TestComputedAsset(t *testing.T) { } inputs, assets, err := makeTerraformInputsNoDefaults(olds, props, tfs, ps) assert.NoError(t, err) - outputs := MakeTerraformOutputs(ctx, shimv1.NewProvider(testTFProvider), inputs, tfs, ps, assets, false, true) + outputs := MakeTerraformOutputs(ctx, shimv1.NewProvider(testTFProvider), inputs, tfs, ps, assets, true) assert.Equal(t, resource.PropertyMap{ "zzz": resource.PropertyValue{V: resource.Computed{Element: resource.PropertyValue{V: ""}}}, }, outputs) @@ -1304,7 +1310,7 @@ func TestInvalidAsset(t *testing.T) { } inputs, assets, err := makeTerraformInputsNoDefaults(olds, props, tfs, ps) assert.NoError(t, err) - outputs := MakeTerraformOutputs(ctx, shimv1.NewProvider(testTFProvider), inputs, tfs, ps, assets, false, true) + outputs := MakeTerraformOutputs(ctx, shimv1.NewProvider(testTFProvider), inputs, tfs, ps, assets, true) assert.Equal(t, resource.PropertyMap{ "zzz": resource.NewStringProperty("invalid"), }, outputs) @@ -1369,8 +1375,7 @@ func TestOverridingTFSchema(t *testing.T) { tfInputs, tfSchema, typeOverrides, - nil, /* assets */ - false, /*useRawNames*/ + nil, /* assets */ true, ) assert.Equal(t, tfOutputs, result) @@ -1421,7 +1426,7 @@ func TestArchiveAsAsset(t *testing.T) { } inputs, assets, err := makeTerraformInputsNoDefaults(olds, props, tfs, ps) assert.NoError(t, err) - outputs := MakeTerraformOutputs(ctx, shimv1.NewProvider(testTFProvider), inputs, tfs, ps, assets, false, true) + outputs := MakeTerraformOutputs(ctx, shimv1.NewProvider(testTFProvider), inputs, tfs, ps, assets, true) assert.True(t, arch.DeepEquals(outputs["zzz"])) } @@ -1790,8 +1795,7 @@ func TestStringOutputsWithSchema(t *testing.T) { "not_a_float_value": {Type: schemav1.TypeFloat}, }), map[string]*SchemaInfo{}, - nil, /* assets */ - false, /* useRawNames */ + nil, /* assets */ true, ) @@ -2849,7 +2853,6 @@ func TestOutputNumberTypes(t *testing.T) { tfs, map[string]*SchemaInfo{}, AssetTable{}, - false, true, ) assert.Equal(t, resource.PropertyMap{