Skip to content

Commit

Permalink
Breaking: Remove rawNames from MakeTerraformOutput & `MakeTerraform…
Browse files Browse the repository at this point in the history 
…Outputs`

This has the same effect as 2b0bc80, 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 (#1642).
  • Loading branch information
@iwahbe
iwahbe committed Apr 10, 2024
1 parent 81f7879 commit 5048d60
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 39 deletions.
2 changes: 1 addition & 1 deletion pkg/tfbridge/provider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
46 changes: 27 additions & 19 deletions pkg/tfbridge/schema.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
//}
Expand All @@ -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.
Expand Down Expand Up @@ -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++ {
Expand All @@ -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).
Expand All @@ -1164,32 +1163,41 @@ 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)
return resource.NewNullProperty()
}
}

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)
Expand Down
41 changes: 22 additions & 19 deletions pkg/tfbridge/schema_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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{}{
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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{}{
Expand All @@ -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{}{
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -1369,8 +1375,7 @@ func TestOverridingTFSchema(t *testing.T) {
tfInputs,
tfSchema,
typeOverrides,
nil, /* assets */
false, /*useRawNames*/
nil, /* assets */
true,
)
assert.Equal(t, tfOutputs, result)
Expand Down Expand Up @@ -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"]))
}

Expand Down Expand Up @@ -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,
)

Expand Down Expand Up @@ -2849,7 +2853,6 @@ func TestOutputNumberTypes(t *testing.T) {
tfs,
map[string]*SchemaInfo{},
AssetTable{},
false,
true,
)
assert.Equal(t, resource.PropertyMap{
Expand Down

0 comments on commit 5048d60

Please sign in to comment.