[BugFix: Issue #346] Fixes for x-terraform-ignore-order

openapi/common.go

@@ -168,7 +168,7 @@ func processIgnoreOrderIfEnabled(property SpecSchemaDefinitionProperty, inputPro
 		for _, inputItemValue := range inputValueArray {
 			for _, remoteItemValue := range remoteValueArray {
 				if property.equalItems(property.ArrayItemsType, inputItemValue, remoteItemValue) {
-					newPropertyValue = append(newPropertyValue, inputItemValue)
+					newPropertyValue = append(newPropertyValue, remoteItemValue)
 					break
 				}
 			}

openapi/openapi_spec_resource_schema_definition_property.go

@@ -328,29 +328,44 @@ func (s *SpecSchemaDefinitionProperty) equal(item1, item2 interface{}) bool {
 }
 
 func (s *SpecSchemaDefinitionProperty) equalItems(itemsType schemaDefinitionPropertyType, item1, item2 interface{}) bool {
+	var defaultValueForType interface{} = nil
 	switch itemsType {
 	case TypeString:
+		defaultValueForType = ""
 		if !s.validateValueType(item1, reflect.String) || !s.validateValueType(item2, reflect.String) {
 			return false
 		}
 	case TypeInt:
+		item1 = terraformutils.CastToIntegerIfFloat(item1) // deal with API responses mapping all numbers to float64
+		item2 = terraformutils.CastToIntegerIfFloat(item2)
+		defaultValueForType = 0
 		if !s.validateValueType(item1, reflect.Int) || !s.validateValueType(item2, reflect.Int) {
 			return false
 		}
 	case TypeFloat:
+		defaultValueForType = 0.0
 		if !s.validateValueType(item1, reflect.Float64) || !s.validateValueType(item2, reflect.Float64) {
 			return false
 		}
 	case TypeBool:
+		defaultValueForType = false
 		if !s.validateValueType(item1, reflect.Bool) || !s.validateValueType(item2, reflect.Bool) {
 			return false
 		}
 	case TypeList:
 		if !s.validateValueType(item1, reflect.Slice) || !s.validateValueType(item2, reflect.Slice) {
 			return false
 		}
+		if item1 == nil || item2 == nil {
+			return s.isOptional()
+		}
 		list1 := item1.([]interface{})
 		list2 := item2.([]interface{})
+
+		if s.isOptional() && (len(list1) == 0 || len(list2) == 0) {
+			return true
+		}
+
 		if len(list1) != len(list2) {
 			return false
 		}
@@ -373,14 +388,16 @@ func (s *SpecSchemaDefinitionProperty) equalItems(itemsType schemaDefinitionProp
 			return s.equalItems(s.ArrayItemsType, list1[idx], list2[idx])
 		}
 	case TypeObject:
+		item1, _ = terraformutils.CastTerraformSliceToMap(item1) // deal with terraform schema returning nested objects as slices
+		item2, _ = terraformutils.CastTerraformSliceToMap(item2)
 		if !s.validateValueType(item1, reflect.Map) || !s.validateValueType(item2, reflect.Map) {
 			return false
 		}
 		object1 := item1.(map[string]interface{})
 		object2 := item2.(map[string]interface{})
 		for _, objectProperty := range s.SpecSchemaDefinition.Properties {
-			objectPropertyValue1 := object1[objectProperty.Name]
-			objectPropertyValue2 := object2[objectProperty.Name]
+			objectPropertyValue1 := objectProperty.getPropertyValueFromMap(object1)
+			objectPropertyValue2 := objectProperty.getPropertyValueFromMap(object2)
 			if !objectProperty.equal(objectPropertyValue1, objectPropertyValue2) {
 				return false
 			}
@@ -389,12 +406,31 @@ func (s *SpecSchemaDefinitionProperty) equalItems(itemsType schemaDefinitionProp
 	default:
 		return false
 	}
+
+	if s.isOptional() {
+		if item1 == nil || item2 == nil { // deals with cases where optional properties aren't returned by the API but Terraform sets a default value for them
+			return true
+		}
+		if item1 == defaultValueForType || item2 == defaultValueForType { // deals with cases where optional properties are returned by the API, but these properties are not defined in the tf config and Terraform gives sets them to default values
+			return true
+		}
+	}
 	return item1 == item2
 }
 
 func (s *SpecSchemaDefinitionProperty) validateValueType(item interface{}, expectedKind reflect.Kind) bool {
+	if item == nil {
+		return true // API responses can skip optional properties, resulting in nils, which are technically valid for the given type
+	}
 	if reflect.TypeOf(item).Kind() != expectedKind {
 		return false
 	}
 	return true
 }
+
+func (s *SpecSchemaDefinitionProperty) getPropertyValueFromMap(mapItem map[string]interface{}) interface{} {
+	if mapItem[s.Name] != nil {
+		return mapItem[s.Name]
+	}
+	return mapItem[s.GetTerraformCompliantPropertyName()]
+}

openapi/terraformutils/terraform_utils.go

@@ -158,3 +158,42 @@ func MultiEnvDefaultString(ks []string, defaultValue interface{}) (string, error
 	}
 	return dv.(string), nil
 }
+
+// CastTerraformSliceToMap tries to cast the provided input to a map and returns an unaltered shallow
+// copy of the input if it fails to cast. It exists to deal with the fact the Terraform defines nested
+// objects as single element lists, and this method normalises the structure so that it can be
+// compared against non-Terraform produced models.
+func CastTerraformSliceToMap(item interface{}) (interface{}, bool) {
+	if item == nil {
+		return make(map[string]interface{}), true
+	}
+	sliceItem, successfulCast := item.([]interface{})
+	if successfulCast {
+		if len(sliceItem) == 0 {
+			return make(map[string]interface{}), true
+		}
+		if len(sliceItem) == 1 {
+			mapItem, successfulCast := sliceItem[0].(map[string]interface{})
+			if successfulCast {
+				return mapItem, true
+			}
+		}
+	}
+	return item, false
+}
+
+// CastToIntegerIfFloat tries to cast the input to an integer if it can be represented as an integer.
+// Failure to cast to an integer will return an unaltered shallow copy of the input.
+func CastToIntegerIfFloat(item interface{}) interface{} {
+	if item == nil {
+		return 0
+	}
+	var floatValue, successfulCast = item.(float64)
+	if successfulCast {
+		var integerValue = int(floatValue)
+		if item == float64(integerValue) { // check if float is really an integer
+			return integerValue
+		}
+	}
+	return item
+}

openapi/terraformutils/terraform_utils_test.go

@@ -242,3 +242,68 @@ func TestEnvDefaultFunc(t *testing.T) {
 		})
 	})
 }
+
+func TestCastTerraformSliceToMap(t *testing.T) {
+	Convey("Given a nil input when CastTerraformSliceToMap is called then the result should be successfully cast an empty map", t, func() {
+		var input interface{} = nil
+		Convey("When CastTerraformSliceToMap is called", func() {
+			result, success := CastTerraformSliceToMap(input)
+			Convey("Then the result should be successfully cast", func() {
+				checkedResult, checkSuccess := result.(map[string]interface{})
+				So(success, ShouldBeTrue)
+				So(checkSuccess, ShouldBeTrue)
+				Convey("And the result should be an empty map", func() {
+					So(len(checkedResult), ShouldEqual, 0)
+				})
+			})
+		})
+	})
+
+	Convey("Given an empty slice input", t, func() {
+		var input []interface{}
+		Convey("When CastTerraformSliceToMap is called", func() {
+			result, success := CastTerraformSliceToMap(input)
+			Convey("Then the result should be successfully cast", func() {
+				checkedResult, checkSuccess := result.(map[string]interface{})
+				So(success, ShouldBeTrue)
+				So(checkSuccess, ShouldBeTrue)
+				Convey("And the result should be an empty map", func() {
+					So(len(checkedResult), ShouldEqual, 0)
+				})
+			})
+		})
+	})
+
+	Convey("Given a single element slice input", t, func() {
+		var input []interface{}
+		Convey("And the element is a map", func() {
+			mapItem := make(map[string]interface{})
+			mapItem["some_property"] = "some_value"
+			input := append(input, mapItem)
+			Convey("When CastTerraformSliceToMap is called", func() {
+				result, success := CastTerraformSliceToMap(input)
+				Convey("Then the result should be successfully cast", func() {
+					checkedResult, checkSuccess := result.(map[string]interface{})
+					So(success, ShouldBeTrue)
+					So(checkSuccess, ShouldBeTrue)
+					Convey("And the result should be the same map from the slice", func() {
+						So(checkedResult, ShouldEqual, mapItem)
+					})
+				})
+			})
+		})
+
+		Convey("And the element is a non-map", func() {
+			input := append(input, "some_value", "other_value")
+			Convey("When CastTerraformSliceToMap is called", func() {
+				result, success := CastTerraformSliceToMap(input)
+				Convey("Then the result should not be successfully cast", func() {
+					So(success, ShouldBeFalse)
+					Convey("And the result should be the same as the input", func() {
+						So(len(result.([]interface{})), ShouldEqual, len(input))
+					})
+				})
+			})
+		})
+	})
+}

tests/e2e/data/gray_box_test_data/ignore_order/openapi.yaml

@@ -0,0 +1,104 @@
+swagger: "2.0"
+host: %s
+schemes:
+  - "http"
+
+paths:
+  ######################
+  #### CDN Resource ####
+  ######################
+
+  /v1/cdns:
+    x-terraform-resource-name: "cdn"
+    post:
+      summary: "Create cdn"
+      operationId: "ContentDeliveryNetworkCreateV1"
+      parameters:
+        - in: "body"
+          name: "body"
+          description: "Created CDN"
+          required: true
+          schema:
+            $ref: "#/definitions/ContentDeliveryNetworkV1"
+      responses:
+        201:
+          description: "successful operation"
+          schema:
+            $ref: "#/definitions/ContentDeliveryNetworkV1"
+  /v1/cdns/{cdn_id}:
+    get:
+      summary: "Get cdn by id"
+      description: ""
+      operationId: "ContentDeliveryNetworkGetV1"
+      parameters:
+        - name: "cdn_id"
+          in: "path"
+          description: "The cdn id that needs to be fetched."
+          required: true
+          type: "string"
+      responses:
+        200:
+          description: "successful operation"
+          schema:
+            $ref: "#/definitions/ContentDeliveryNetworkV1"
+    delete:
+      summary: "Delete cdn"
+      operationId: "ContentDeliveryNetworkDeleteV1"
+      parameters:
+        - name: "id"
+          in: "path"
+          description: "The cdn that needs to be deleted"
+          required: true
+          type: "string"
+      responses:
+        204:
+          description: "successful operation, no content is returned"
+definitions:
+  ContentDeliveryNetworkV1:
+    type: "object"
+    required:
+      - label
+    properties:
+      id:
+        type: "string"
+        readOnly: true
+      label:
+        type: "string"
+      list_prop:
+        type: "array"
+        x-terraform-ignore-order: true
+        items:
+          type: "string"
+      integer_list_prop:
+        type: "array"
+        x-terraform-ignore-order: true
+        items:
+          type: "integer"
+      nestedListProp:
+        type: "array"
+        x-terraform-ignore-order: true
+        items:
+          type: "object"
+          properties:
+            someProperty:
+              type: "string"
+              x-terraform-force-new: true
+            otherPropertyStr:
+              type: "string"
+            otherPropertyInt:
+              type: "integer"
+            otherPropertyFloat:
+              type: "number"
+            otherPropertyBool:
+              type: "boolean"
+            otherPropertyList:
+              type: "array"
+              x-terraform-computed: true
+              items:
+                type: "string"
+            otherPropertyObject:
+              type: "object"
+              x-terraform-computed: true
+              properties:
+                deeplyNestedProperty:
+                  type: "string"

tests/e2e/data/gray_box_test_data/ignore_order/test_stage_1.tf

@@ -0,0 +1,34 @@
+# URI /v1/cdns/
+resource "openapi_cdn_v1" "my_cdn" {
+  label             = "some label"
+  list_prop         = ["value1", "value2", "value3"]
+  integer_list_prop = [1, 2, 3]
+
+  nested_list_prop {
+    some_property        = "value1"
+    other_property_str   = "otherValue1"
+    other_property_int   = 5
+    other_property_float = 3.14
+    other_property_bool  = true
+    other_property_list  = ["someValue1"]
+    other_property_object {
+      deeply_nested_property = "someDeeplyNestedValue1"
+    }
+  }
+
+  nested_list_prop {
+    some_property        = "value2"
+    other_property_str   = "otherValue2"
+    other_property_int   = 10
+    other_property_float = 1.23
+    other_property_bool  = false
+    other_property_list  = ["someValue2"]
+    other_property_object {
+      deeply_nested_property = "someDeeplyNestedValue2"
+    }
+  }
+
+  nested_list_prop {
+    some_property = "value3"
+  }
+}

tests/e2e/data/gray_box_test_data/ignore_order/test_stage_2.tf

@@ -0,0 +1,34 @@
+# URI /v1/cdns/
+resource "openapi_cdn_v1" "my_cdn" {
+  label             = "some label"
+  list_prop         = ["value1", "value3", "value2"]
+  integer_list_prop = [1, 3, 2]
+
+  nested_list_prop {
+    some_property        = "value1"
+    other_property_str   = "otherValue1"
+    other_property_int   = 5
+    other_property_float = 3.14
+    other_property_bool  = true
+    other_property_list  = ["someValue1"]
+    other_property_object {
+      deeply_nested_property = "someDeeplyNestedValue1"
+    }
+  }
+
+  nested_list_prop {
+    some_property = "value3"
+  }
+
+  nested_list_prop {
+    some_property        = "value2"
+    other_property_str   = "otherValue2"
+    other_property_int   = 10
+    other_property_float = 1.23
+    other_property_bool  = false
+    other_property_list  = ["someValue2"]
+    other_property_object {
+      deeply_nested_property = "someDeeplyNestedValue2"
+    }
+  }
+}