Merge pull request #229 from dikhan/ignore-array-prop-ordering

[FeatureRequest: Issue #228] Ignore array prop ordering

docs/how_to.md

@@ -1033,8 +1033,35 @@ x-terraform-sensitive | boolean | If this meta attribute is present in a definit
 x-terraform-id | boolean | If this meta attribute is present in an object definition property, the value will be used as the resource identifier when performing the read, update and delete API operations. The value will also be stored in the ID field of the local state file.
 x-terraform-field-name | string | This enables service providers to override the schema definition property name with a different one which will be the property name used in the terraform configuration file. This is mostly used to expose the internal property to a more user friendly name. If the extension is not present and the property name is not terraform compliant (following snake_case), an automatic conversion will be performed by the OpenAPI Terraform provider to make the name compliant (following Terraform's field name convention to be snake_case) 
 x-terraform-field-status | boolean | If this meta attribute is present in a definition property, the value will be used as the status identifier when executing the polling mechanism on eligible async operations such as POST/PUT/DELETE.
+[x-terraform-ignore-order](#xTerraformIgnoreOrder) | boolean | If this meta attribute is present in a definition property of type list, when the plugin is updating the state for the property it will inspect the items of the list received from remote and compare with the local values and if the lists are the same but unordered the state will keep the users input. Please go to the `x-terraform-ignore-order` section to learn more about the different behaviours supported.
 [x-terraform-complex-object-legacy-config](#xTerraformComplexObjectLegacyConfig) | boolean | If this meta attribute is present in an definition property of type object with value set to true, the OpenAPI terraform plugin will configure the corresponding property schema in Terraform following [Hashi maintainers recommendation](https://github.com/hashicorp/terraform/issues/22511#issuecomment-522655851) using as Schema Type schema.TypeList and limiting the max items in the list to 1 (MaxItems = 1). 
 
+###### <a name="xTerraformIgnoreOrder">x-terraform-ignore-order</a>
+
+This extension enables the service providers to setup the 'ignore order' behaviour for a property of type list defined in
+the object definition. For instance, the API may be returning the array items in lexical order but that behaviour might
+not be the desired one for the terraform plugin since it would cause DIFFs for users that provided the values of the array
+property in a different order. Hence, ensuring as much as possible that the order for the elements in the input list 
+provided by the user is maintained.
+
+Given the following terraform snippet where the members values are in certain desired order and assuming that the members 
+property is of type 'list' AND has the `x-terraform-ignore-order` extension set to true in the OpenAPI document for the `group_v1` 
+resource definition:
+
+````
+resource "openapi_group_v1" "my_iam_group_v1" {
+  members = ["user1", "user2", "user3"]
+}
+````
+
+The following behaviour is applied depending on the different scenarios when processing the response received by the API
+and saving the state of the property.
+
+- Use case 0: If the remote value for the property `members` contained the same items in the same order (eg: `{"members":["user1", "user2", "user3"]}`) as the tf input then the state saved for the property would match the input values. That is: ``members = ["user1", "user2", "user3"]``
+- Use case 1: If the remote value for the property `members` contained the same items as the tf input BUT the order of the elements is different (eg: `{"members":["user3", "user2", "user1"]}`) then state saved for the property would match the input values. That is: ``members = ["user1", "user2", "user3"]``
+- Use case 2: If the remote value for the property `members` contained the same items as the tf input in different order PLUS new ones (eg: `{"members":["user2", "user1", "user3", "user4"]}`) then state saved for the property would match the input values and also add to the end of the list the new elements received from the API. That is: ``members = ["user1", "user2", "user3", "user4"]``
+- Use case 3: If the remote value for the property `members` contained a shorter list than items in the tf input (eg: `{"members":["user3", "user1"}`) then state saved for the property would contain only the matching elements between the input and remote. That is: ``members = ["user1", "user3"]``
+- Use case 4: If the remote value for the property `members` contained the same list size as the items in the tf input but some elements inside where updated (eg: `{"members":["user1", "user5", "user9"]}`) then state saved for the property would contain the matching elements  between the input and output and also keep the remote values. That is: ``members = ["user1", "user5", "user9"]``
 
 ###### <a name="xTerraformComplexObjectLegacyConfig">x-terraform-complex-object-legacy-config</a>
 

go.mod

@@ -41,13 +41,14 @@ require (
 	github.com/spf13/cobra v0.0.3
 	github.com/stretchr/testify v1.3.0
 	github.com/stvp/go-udp-testing v0.0.0-20191102171040-06b61409b154
+	github.com/yuin/goldmark v1.1.30 // indirect
 	github.com/zach-klippenstein/goregen v0.0.0-20160303162051-795b5e3961ea // indirect
-	golang.org/x/crypto v0.0.0-20200323165209-0ec3e9974c59 // indirect
+	golang.org/x/crypto v0.0.0-20200510223506-06a226fb4e37 // indirect
 	golang.org/x/lint v0.0.0-20200302205851-738671d3881b // indirect
-	golang.org/x/net v0.0.0-20200324143707-d3edc9973b7e // indirect
+	golang.org/x/net v0.0.0-20200513185701-a91f0712d120 // indirect
 	golang.org/x/sync v0.0.0-20200317015054-43a5402ce75a // indirect
-	golang.org/x/sys v0.0.0-20200331124033-c3d80250170d // indirect
-	golang.org/x/tools v0.0.0-20200331202046-9d5940d49312 // indirect
+	golang.org/x/sys v0.0.0-20200513112337-417ce2331b5c // indirect
+	golang.org/x/tools v0.0.0-20200513201620-d5fe73897c97 // indirect
 	gopkg.in/mgo.v2 v2.0.0-20160818020120-3f83fa500528 // indirect
 	gopkg.in/yaml.v2 v2.2.2
 )

go.sum

@@ -289,6 +289,7 @@ github.com/ulikunitz/xz v0.5.5/go.mod h1:2bypXElzHzzJZwzH67Y6wb67pO62Rzfn7BSiF4A
 github.com/vmihailenco/msgpack v3.3.3+incompatible h1:wapg9xDUZDzGCNFlwc5SqI1rvcciqcxEHac4CYj89xI=
 github.com/vmihailenco/msgpack v3.3.3+incompatible/go.mod h1:fy3FlTQTDXWkZ7Bh6AcGMlsjHatGryHQYUTf1ShIgkk=
 github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
+github.com/yuin/goldmark v1.1.30/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/zach-klippenstein/goregen v0.0.0-20160303162051-795b5e3961ea h1:CyhwejzVGvZ3Q2PSbQ4NRRYn+ZWv5eS1vlaEusT+bAI=
 github.com/zach-klippenstein/goregen v0.0.0-20160303162051-795b5e3961ea/go.mod h1:eNr558nEUjP8acGw8FFjTeWvSgU1stO7FAO6eknhHe4=
 github.com/zclconf/go-cty v1.0.0 h1:EWtv3gKe2wPLIB9hQRQJa7k/059oIfAqcEkCNnaVckk=
@@ -312,6 +313,8 @@ golang.org/x/crypto v0.0.0-20191227163750-53104e6ec876 h1:sKJQZMuxjOAR/Uo2LBfU90
 golang.org/x/crypto v0.0.0-20191227163750-53104e6ec876/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
 golang.org/x/crypto v0.0.0-20200323165209-0ec3e9974c59 h1:3zb4D3T4G8jdExgVU/95+vQXfpEPiMdCaZgmGVxjNHM=
 golang.org/x/crypto v0.0.0-20200323165209-0ec3e9974c59/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
+golang.org/x/crypto v0.0.0-20200510223506-06a226fb4e37 h1:cg5LA/zNPRzIXIWSCxQW10Rvpy94aQh3LT/ShoCpkHw=
+golang.org/x/crypto v0.0.0-20200510223506-06a226fb4e37/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
 golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
 golang.org/x/exp v0.0.0-20190510132918-efd6b22b2522/go.mod h1:ZjyILWgesfNpC6sMxTJOJm9Kp84zZh5NQWvqDGG3Qr8=
 golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js=
@@ -351,6 +354,10 @@ golang.org/x/net v0.0.0-20191209160850-c0dbc17a3553/go.mod h1:z5CRVTTTmAJ677TzLL
 golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/net v0.0.0-20200324143707-d3edc9973b7e h1:3G+cUijn7XD+S4eJFddp53Pv7+slrESplyjG25HgL+k=
 golang.org/x/net v0.0.0-20200324143707-d3edc9973b7e/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A=
+golang.org/x/net v0.0.0-20200506145744-7e3656a0809f h1:QBjCr1Fz5kw158VqdE9JfI9cJnl/ymnJWAdMuinqL7Y=
+golang.org/x/net v0.0.0-20200506145744-7e3656a0809f/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A=
+golang.org/x/net v0.0.0-20200513185701-a91f0712d120 h1:EZ3cVSzKOlJxAd8e8YAJ7no8nNypTxexh/YE/xW3ZEY=
+golang.org/x/net v0.0.0-20200513185701-a91f0712d120/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A=
 golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
 golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421 h1:Wo7BWFiOk0QRFMLYMqJGFMd9CgUAcGx7V+qEg/h5IBI=
 golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
@@ -381,6 +388,7 @@ golang.org/x/sys v0.0.0-20191220220014-0732a990476f/go.mod h1:h1NjWce9XRLGQEsW7w
 golang.org/x/sys v0.0.0-20191224085550-c709ea063b76/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20200331124033-c3d80250170d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20200513112337-417ce2331b5c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
@@ -407,6 +415,12 @@ golang.org/x/tools v0.0.0-20191227053925-7b8e75db28f4/go.mod h1:TB2adYChydJhpapK
 golang.org/x/tools v0.0.0-20200130002326-2f3ba24bd6e7/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28=
 golang.org/x/tools v0.0.0-20200331202046-9d5940d49312 h1:2PHG+Ia3gK1K2kjxZnSylizb//eyaMG8gDFbOG7wLV8=
 golang.org/x/tools v0.0.0-20200331202046-9d5940d49312/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
+golang.org/x/tools v0.0.0-20200513122804-866d71a3170a h1:LkBZllNM46txyXU+/e601XXm26FTs7DJr8TRZa5w35Q=
+golang.org/x/tools v0.0.0-20200513122804-866d71a3170a/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
+golang.org/x/tools v0.0.0-20200513175351-0951661448da h1:ZR1ivkcQoKXKtux9Rx3Em7iiSViMxQ5suNd5PZMUkPc=
+golang.org/x/tools v0.0.0-20200513175351-0951661448da/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
+golang.org/x/tools v0.0.0-20200513201620-d5fe73897c97 h1:DAuln/hGp+aJiHpID1Y1hYzMEPP5WLwtZHPb50mN0OE=
+golang.org/x/tools v0.0.0-20200513201620-d5fe73897c97/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=

openapi/common.go

@@ -79,14 +79,28 @@ func getParentIDs(openAPIResource SpecResource, data *schema.ResourceData) ([]st
 	return []string{}, nil
 }
 
-// updateStateWithPayloadData is in charge of saving the given payload into the state file. The property names are
-// converted into compliant terraform names if needed.
+// updateStateWithPayloadData is in charge of saving the given payload into the state file keeping for list properties the
+// same order as the input (if the list property has the IgnoreItemsOrder set to true). The property names are converted into compliant terraform names if needed.
+// The property names are converted into compliant terraform names if needed.
 func updateStateWithPayloadData(openAPIResource SpecResource, remoteData map[string]interface{}, resourceLocalData *schema.ResourceData) error {
+	return updateStateWithPayloadDataAndOptions(openAPIResource, remoteData, resourceLocalData, true)
+}
+
+// dataSourceUpdateStateWithPayloadData is in charge of saving the given payload into the state file keeping for list properties the
+// same order received by the API. The property names are converted into compliant terraform names if needed.
+func dataSourceUpdateStateWithPayloadData(openAPIResource SpecResource, remoteData map[string]interface{}, resourceLocalData *schema.ResourceData) error {
+	return updateStateWithPayloadDataAndOptions(openAPIResource, remoteData, resourceLocalData, false)
+}
+
+// updateStateWithPayloadDataAndOptions is in charge of saving the given payload into the state file AND if the ignoreListOrder is enabled
+// it will go ahead and compare the items in the list (input vs remote) for properties of type list and the flag 'IgnoreItemsOrder' set to true
+// The property names are converted into compliant terraform names if needed.
+func updateStateWithPayloadDataAndOptions(openAPIResource SpecResource, remoteData map[string]interface{}, resourceLocalData *schema.ResourceData, ignoreListOrderEnabled bool) error {
 	resourceSchema, err := openAPIResource.getResourceSchema()
 	if err != nil {
 		return err
 	}
-	for propertyName, propertyValue := range remoteData {
+	for propertyName, propertyRemoteValue := range remoteData {
 		property, err := resourceSchema.getProperty(propertyName)
 		if err != nil {
 			log.Printf("[WARN] The API returned a property that is not specified in the resource's schema definition in the OpenAPI document - error = %s", err)
@@ -95,7 +109,14 @@ func updateStateWithPayloadData(openAPIResource SpecResource, remoteData map[str
 		if property.isPropertyNamedID() {
 			continue
 		}
-		value, err := convertPayloadToLocalStateDataValue(property, propertyValue, false)
+
+		propValue := propertyRemoteValue
+		if ignoreListOrderEnabled && property.shouldIgnoreOrder() {
+			desiredValue := resourceLocalData.Get(property.getTerraformCompliantPropertyName())
+			propValue = processIgnoreOrderIfEnabled(*property, desiredValue, propertyRemoteValue)
+		}
+
+		value, err := convertPayloadToLocalStateDataValue(property, propValue, false)
 		if err != nil {
 			return err
 		}
@@ -108,6 +129,51 @@ func updateStateWithPayloadData(openAPIResource SpecResource, remoteData map[str
 	return nil
 }
 
+// processIgnoreOrderIfEnabled checks whether the property has enabled the `IgnoreItemsOrder` field and if so, goes ahead
+// and returns a new list trying to match as much as possible the input order from the user (not remotes). The following use
+// cases are supported:
+// Use case 0: The desired state for an array property (input from user, inputPropertyValue) contains items in certain order AND the remote state (remoteValue) comes back with the same items in the same order.
+// Use case 1: The desired state for an array property (input from user, inputPropertyValue) contains items in certain order BUT the remote state (remoteValue) comes back with the same items in different order.
+// Use case 2: The desired state for an array property (input from user, inputPropertyValue) contains items in certain order BUT the remote state (remoteValue) comes back with the same items in different order PLUS new ones.
+// Use case 3: The desired state for an array property (input from user, inputPropertyValue) contains items in certain order BUT the remote state (remoteValue) comes back with a shorter list where the remaining elems match the inputs.
+// Use case 4: The desired state for an array property (input from user, inputPropertyValue) contains items in certain order BUT the remote state (remoteValue) some back with the list with the same size but some elems were updated
+func processIgnoreOrderIfEnabled(property specSchemaDefinitionProperty, inputPropertyValue, remoteValue interface{}) interface{} {
+	if inputPropertyValue == nil || remoteValue == nil { // treat remote as the final state if input value does not exists
+		return remoteValue
+	}
+	if property.shouldIgnoreOrder() {
+		newPropertyValue := []interface{}{}
+		inputValueArray := inputPropertyValue.([]interface{})
+		remoteValueArray := remoteValue.([]interface{})
+		for _, inputItemValue := range inputValueArray {
+			for _, remoteItemValue := range remoteValueArray {
+				if property.equalItems(property.ArrayItemsType, inputItemValue, remoteItemValue) {
+					newPropertyValue = append(newPropertyValue, inputItemValue)
+					break
+				}
+			}
+		}
+		modifiedItems := []interface{}{}
+		for _, remoteItemValue := range remoteValueArray {
+			match := false
+			for _, inputItemValue := range inputValueArray {
+				if property.equalItems(property.ArrayItemsType, inputItemValue, remoteItemValue) {
+					match = true
+					break
+				}
+			}
+			if !match {
+				modifiedItems = append(modifiedItems, remoteItemValue)
+			}
+		}
+		for _, updatedItem := range modifiedItems {
+			newPropertyValue = append(newPropertyValue, updatedItem)
+		}
+		return newPropertyValue
+	}
+	return remoteValue
+}
+
 func convertPayloadToLocalStateDataValue(property *specSchemaDefinitionProperty, propertyValue interface{}, useString bool) (interface{}, error) {
 	if propertyValue == nil {
 		return nil, nil