Skip to content

Commit

Permalink
Tolerate MaxItemsOne mismatches (#1296)
Browse files Browse the repository at this point in the history 
* Tolerate MaxItemsOne: true -> false (`[T] -> T`)

This is a information losing operation, since it doesn't handle the case of `[[T]] ->
[T]`. To simply the fix, I have chosen to do nothing in this situation.

* Add tests
  • Loading branch information
@iwahbe
iwahbe authored Jul 21, 2023
1 parent 827c20c commit f0a3ffc
Show file tree
Hide file tree
Showing 3 changed files with 184 additions and 11 deletions.
39 changes: 28 additions & 11 deletions pkg/tfbridge/schema.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -309,19 +309,36 @@ func MakeTerraformInputs(instance *PulumiResource, config resource.PropertyMap,
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.
// 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.
if IsMaxItemsOne(tfs, ps) {
if old.IsNull() {
old = resource.NewArrayProperty([]resource.PropertyValue{})
} else {
old = resource.NewArrayProperty([]resource.PropertyValue{old})
}
if v.IsNull() {
v = resource.NewArrayProperty([]resource.PropertyValue{})
} else {
v = resource.NewArrayProperty([]resource.PropertyValue{v})
wrap := func(val resource.PropertyValue) resource.PropertyValue {
if val.IsNull() {
return resource.NewArrayProperty([]resource.PropertyValue{})
}

// If we are expecting a value of type `[T]` where `T != TypeList`
// and we already see `[T]`, we see that `v` is already the right
// shape and return as is.
//
// This is possible when the old state is from a previous version
// with `MaxItemsOne=false` but the new state has
// `MaxItemsOne=true`.
if elem := tfs.Elem(); elem != nil {
if elem, ok := elem.(shim.Schema); ok &&
// If the underlying type is not a list or set,
// but the value is a list, we just return as is.
!(elem.Type() == shim.TypeList || elem.Type() == shim.TypeSet) &&
val.IsArray() {
return val
}
}

return resource.NewArrayProperty([]resource.PropertyValue{val})
}
old = wrap(old)
v = wrap(v)
}

// If there is a custom transform for this value, run it before processing the value.
Expand Down
135 changes: 135 additions & 0 deletions pkg/tfbridge/schema_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"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"
Expand Down Expand Up @@ -272,6 +273,140 @@ func TestTerraformInputs(t *testing.T) {
}
}

func TestMakeTerraformInputMixedMaxItemsOne(t *testing.T) {
t.Parallel()

typeString := (&schema.Schema{
Type: shim.TypeString,
}).Shim()

tests := map[string]struct {
maxItemsOne bool
oldState resource.PropertyValue
newState resource.PropertyValue
tfs *schema.Schema
tfValue interface{}
}{
// Scalars: The pulumi type is String.
// The TF type is [String] (either [n; T] or [1; T]).
"scalar-adding-max-items-one": {
// The TF type has changed from [n; T] to [1; T], changing the
// pulumi type from [T] -> T.
maxItemsOne: true,
oldState: resource.NewArrayProperty([]resource.PropertyValue{
resource.NewStringProperty("sc"),
}),
newState: resource.NewStringProperty("sc"),
tfs: &schema.Schema{
Type: shim.TypeList,
Elem: typeString,
MaxItems: 1,
},
tfValue: []interface{}{"sc"},
},
"scalar-removing-max-items-one": {
// The TF type has changed from [1; T] to [n; T], changing the
// pulumi type from T -> [T].
maxItemsOne: false,
oldState: resource.NewStringProperty("sc"),
newState: resource.NewArrayProperty([]resource.PropertyValue{
resource.NewStringProperty("sc"),
}),
tfs: &schema.Schema{
Type: shim.TypeList,
Elem: typeString,
},
tfValue: []interface{}{"sc"},
},

// Scalars: The pulumi type is String.
// The TF type is [String] (either [n; T] or [1; T]).
//
// Here we have empty values, which are handled differently.
"scalar-adding-null-max-items-one": {
// The TF type has changed from [n; T] to [1; T], changing the
// pulumi type from [T] -> T.
maxItemsOne: true,
oldState: resource.NewNullProperty(),
newState: resource.NewNullProperty(),
tfs: &schema.Schema{
Type: shim.TypeList,
Elem: typeString,
MaxItems: 1,
},
tfValue: []interface{}(nil),
},
"scalar-removing-null-max-items-one": {
// The TF type has changed from [1; T] to [n; T], changing the
// pulumi type from T -> [T].
maxItemsOne: false,
oldState: resource.NewArrayProperty([]resource.PropertyValue{}),
newState: resource.NewArrayProperty([]resource.PropertyValue{}),
tfs: &schema.Schema{
Type: shim.TypeList,
Elem: typeString,
MaxItems: 1,
},
tfValue: []interface{}(nil),
},
// // Lists: The pulumi type is [String].
// // The TF type is [[String]] (either [m; [n; T]] or [1; [n; T]]).
// //
// // This is different because we can't know the type of an empty list. It
// // could be of type [T] or [[T]]. In this case, we don't make an attempt
// // at promotion.
"list-adding-max-items-one": {
// The TF type has changed from [m; [n; T]] to [1; [n; T]], changing the
// pulumi type from [[T]] -> [T].
maxItemsOne: true,
oldState: resource.NewArrayProperty([]resource.PropertyValue{
resource.NewArrayProperty([]resource.PropertyValue{
resource.NewStringProperty("sc"),
})}),
newState: resource.NewArrayProperty([]resource.PropertyValue{
resource.NewStringProperty("sc"),
}),
tfs: &schema.Schema{
Type: shim.TypeList,
MaxItems: 1,
Elem: (&schema.Schema{
Type: shim.TypeList,
Elem: typeString,
}).Shim(),
},
tfValue: []interface{}{[]interface{}{"sc"}},
},
}
for name, tt := range tests {
tt := tt
t.Run(name, func(t *testing.T) {
olds := resource.PropertyMap{
"element": tt.oldState,
"__defaults": resource.NewArrayProperty(
[]resource.PropertyValue{
resource.NewStringProperty("other"),
},
),
}
news := resource.PropertyMap{
"element": tt.newState,
"__defaults": resource.NewArrayProperty(
[]resource.PropertyValue{
resource.NewStringProperty("other"),
},
),
}
tfs := schema.SchemaMap{"element": tt.tfs.Shim()}
result, _, err := makeTerraformInputs(
olds, news, tfs, nil /* ps */)
require.NoError(t, err)
assert.Equal(t, map[string]interface{}{
"element": tt.tfValue,
}, result)
})
}
}

type MyString string

// TestTerraformOutputsWithSecretsSupported verifies that we translate Terraform outputs into Pulumi outputs and
Expand Down
21 changes: 21 additions & 0 deletions pkg/tfshim/shim.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,27 @@ const (
TypeSet
)

func (i ValueType) String() string {
switch i {
case TypeBool:
return "Bool"
case TypeInt:
return "Int"
case TypeFloat:
return "Float"
case TypeString:
return "String"
case TypeList:
return "List"
case TypeMap:
return "Map"
case TypeSet:
return "Set"
default:
return ""
}
}

type SchemaDefaultFunc func() (interface{}, error)

type SchemaStateFunc func(interface{}) string
Expand Down

0 comments on commit f0a3ffc

Please sign in to comment.