diff --git a/docs/how_to.md b/docs/how_to.md index 05492510f..a4123217b 100644 --- a/docs/how_to.md +++ b/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). +###### x-terraform-ignore-order + +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"]`` ###### x-terraform-complex-object-legacy-config diff --git a/go.mod b/go.mod index 796cf9ca3..61fbe2617 100644 --- a/go.mod +++ b/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 ) diff --git a/go.sum b/go.sum index e2a97ec55..d2b7683fe 100644 --- a/go.sum +++ b/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= diff --git a/openapi/common.go b/openapi/common.go index 1d318d3c3..d76435a33 100644 --- a/openapi/common.go +++ b/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 diff --git a/openapi/common_test.go b/openapi/common_test.go index 6bd88319b..96ac2f342 100644 --- a/openapi/common_test.go +++ b/openapi/common_test.go @@ -2,6 +2,7 @@ package openapi import ( "errors" + "fmt" "io/ioutil" "net/http" "strconv" @@ -335,6 +336,141 @@ func TestUpdateStateWithPayloadData(t *testing.T) { }) }) + Convey("Given a resource factory containing a schema with property lists that have the IgnoreItemsOrder set to true", t, func() { + objectSchemaDefinition := &specSchemaDefinition{ + Properties: specSchemaDefinitionProperties{ + newIntSchemaDefinitionPropertyWithDefaults("origin_port", "", true, false, 80), + newStringSchemaDefinitionPropertyWithDefaults("protocol", "", true, false, "http"), + }, + } + arrayObjectStateValue := []interface{}{ + map[string]interface{}{ + "origin_port": 80, + "protocol": "http", + }, + map[string]interface{}{ + "origin_port": 443, + "protocol": "https", + }, + } + listOfObjectsProperty := newListSchemaDefinitionPropertyWithDefaults("slice_object_property", "", true, false, false, arrayObjectStateValue, typeObject, objectSchemaDefinition) + listOfObjectsProperty.IgnoreItemsOrder = true + + listOfStrings := newListSchemaDefinitionPropertyWithDefaults("slice_property", "", true, false, false, []interface{}{"value1", "value2"}, typeString, nil) + listOfStrings.IgnoreItemsOrder = true + + r, resourceData := testCreateResourceFactory(t, listOfStrings, listOfObjectsProperty) + Convey("When updateStateWithPayloadData is called", func() { + remoteData := map[string]interface{}{ + listOfStrings.Name: []interface{}{"value2", "value1"}, + listOfObjectsProperty.Name: []interface{}{ + map[string]interface{}{ + "origin_port": 443, + "protocol": "https", + }, + map[string]interface{}{ + "origin_port": 80, + "protocol": "http", + }, + }, + } + err := updateStateWithPayloadData(r.openAPIResource, remoteData, resourceData) + Convey("Then the err returned should be nil", func() { + So(err, ShouldBeNil) + }) + Convey("And the expectedValue should maintain the order of the local input (not the order of the remote lists)", func() { + // keys stores in the resource data struct are always snake case + So(len(resourceData.Get(listOfStrings.getTerraformCompliantPropertyName()).([]interface{})), ShouldEqual, 2) + So(resourceData.Get(listOfStrings.getTerraformCompliantPropertyName()).([]interface{})[0], ShouldEqual, listOfStrings.Default.([]interface{})[0]) + So(resourceData.Get(listOfStrings.getTerraformCompliantPropertyName()).([]interface{})[1], ShouldEqual, listOfStrings.Default.([]interface{})[1]) + + So(len(resourceData.Get(listOfObjectsProperty.getTerraformCompliantPropertyName()).([]interface{})), ShouldEqual, 2) + So(resourceData.Get(listOfObjectsProperty.getTerraformCompliantPropertyName()).([]interface{})[0].(map[string]interface{}), ShouldContainKey, "origin_port") + So(resourceData.Get(listOfObjectsProperty.getTerraformCompliantPropertyName()).([]interface{})[0].(map[string]interface{}), ShouldContainKey, "protocol") + So(resourceData.Get(listOfObjectsProperty.getTerraformCompliantPropertyName()).([]interface{})[0].(map[string]interface{})["origin_port"], ShouldEqual, arrayObjectStateValue[0].(map[string]interface{})["origin_port"].(int)) + So(resourceData.Get(listOfObjectsProperty.getTerraformCompliantPropertyName()).([]interface{})[0].(map[string]interface{})["protocol"], ShouldEqual, arrayObjectStateValue[0].(map[string]interface{})["protocol"]) + + So(resourceData.Get(listOfObjectsProperty.getTerraformCompliantPropertyName()).([]interface{})[1].(map[string]interface{}), ShouldContainKey, "origin_port") + So(resourceData.Get(listOfObjectsProperty.getTerraformCompliantPropertyName()).([]interface{})[1].(map[string]interface{}), ShouldContainKey, "protocol") + So(resourceData.Get(listOfObjectsProperty.getTerraformCompliantPropertyName()).([]interface{})[1].(map[string]interface{})["origin_port"], ShouldEqual, arrayObjectStateValue[1].(map[string]interface{})["origin_port"].(int)) + So(resourceData.Get(listOfObjectsProperty.getTerraformCompliantPropertyName()).([]interface{})[1].(map[string]interface{})["protocol"], ShouldEqual, arrayObjectStateValue[1].(map[string]interface{})["protocol"]) + }) + }) + }) + + Convey("Given a resource factory", t, func() { + r, resourceData := testCreateResourceFactory(t, stringWithPreferredNameProperty) + Convey("When is called with a map remoteData containing more properties than then ones specified in the schema (this means the API is returning more info than the one specified in the swagger file)", func() { + remoteData := map[string]interface{}{ + stringWithPreferredNameProperty.Name: "someUpdatedStringValue", + "some_other_property_not_documented_in_openapi_doc": 15, + } + err := updateStateWithPayloadData(r.openAPIResource, remoteData, resourceData) + Convey("Then the err returned should be nil", func() { + So(err, ShouldBeNil) + }) + Convey("Then the resource state data only contains the properties and values for the documented properties", func() { + So(resourceData.Get(stringWithPreferredNameProperty.getTerraformCompliantPropertyName()), ShouldEqual, remoteData[stringWithPreferredNameProperty.Name]) + So(resourceData.Get("some_other_property_not_documented_in_openapi_doc"), ShouldBeNil) + }) + }) + }) +} + +func TestDataSourceUpdateStateWithPayloadData(t *testing.T) { + Convey("Given a resource factory containing a schema with property lists that have the IgnoreItemsOrder set to true", t, func() { + objectSchemaDefinition := &specSchemaDefinition{ + Properties: specSchemaDefinitionProperties{ + newIntSchemaDefinitionPropertyWithDefaults("origin_port", "", true, false, 80), + newStringSchemaDefinitionPropertyWithDefaults("protocol", "", true, false, "http"), + }, + } + arrayObjectStateValue := []interface{}{} + listOfObjectsProperty := newListSchemaDefinitionPropertyWithDefaults("slice_object_property", "", true, false, false, arrayObjectStateValue, typeObject, objectSchemaDefinition) + listOfObjectsProperty.IgnoreItemsOrder = true + + listOfStrings := newListSchemaDefinitionPropertyWithDefaults("slice_property", "", true, false, false, []interface{}{"value1", "value2"}, typeString, nil) + listOfStrings.IgnoreItemsOrder = true + + r, resourceData := testCreateResourceFactory(t, listOfStrings, listOfObjectsProperty) + Convey("When dataSourceUpdateStateWithPayloadData is called", func() { + remoteData := map[string]interface{}{ + listOfStrings.Name: []interface{}{"value2", "value1"}, + listOfObjectsProperty.Name: []interface{}{ + map[string]interface{}{ + "origin_port": 443, + "protocol": "https", + }, + map[string]interface{}{ + "origin_port": 80, + "protocol": "http", + }, + }, + } + err := dataSourceUpdateStateWithPayloadData(r.openAPIResource, remoteData, resourceData) + Convey("Then the err returned should be nil", func() { + So(err, ShouldBeNil) + }) + Convey("And the expectedValue should equal to the expectedValue coming from remote", func() { + // keys stores in the resource data struct are always snake case + So(len(resourceData.Get(listOfStrings.getTerraformCompliantPropertyName()).([]interface{})), ShouldEqual, 2) + So(resourceData.Get(listOfStrings.getTerraformCompliantPropertyName()).([]interface{})[0], ShouldEqual, remoteData[listOfStrings.Name].([]interface{})[0]) + So(resourceData.Get(listOfStrings.getTerraformCompliantPropertyName()).([]interface{})[1], ShouldEqual, remoteData[listOfStrings.Name].([]interface{})[1]) + + So(len(resourceData.Get(listOfObjectsProperty.getTerraformCompliantPropertyName()).([]interface{})), ShouldEqual, 2) + So(resourceData.Get(listOfObjectsProperty.getTerraformCompliantPropertyName()).([]interface{})[0].(map[string]interface{}), ShouldContainKey, "origin_port") + So(resourceData.Get(listOfObjectsProperty.getTerraformCompliantPropertyName()).([]interface{})[0].(map[string]interface{}), ShouldContainKey, "protocol") + So(resourceData.Get(listOfObjectsProperty.getTerraformCompliantPropertyName()).([]interface{})[0].(map[string]interface{})["origin_port"], ShouldEqual, remoteData[listOfObjectsProperty.Name].([]interface{})[0].(map[string]interface{})["origin_port"].(int)) + So(resourceData.Get(listOfObjectsProperty.getTerraformCompliantPropertyName()).([]interface{})[0].(map[string]interface{})["protocol"], ShouldEqual, remoteData[listOfObjectsProperty.Name].([]interface{})[0].(map[string]interface{})["protocol"]) + + So(resourceData.Get(listOfObjectsProperty.getTerraformCompliantPropertyName()).([]interface{})[1].(map[string]interface{}), ShouldContainKey, "origin_port") + So(resourceData.Get(listOfObjectsProperty.getTerraformCompliantPropertyName()).([]interface{})[1].(map[string]interface{}), ShouldContainKey, "protocol") + So(resourceData.Get(listOfObjectsProperty.getTerraformCompliantPropertyName()).([]interface{})[1].(map[string]interface{})["origin_port"], ShouldEqual, remoteData[listOfObjectsProperty.Name].([]interface{})[1].(map[string]interface{})["origin_port"].(int)) + So(resourceData.Get(listOfObjectsProperty.getTerraformCompliantPropertyName()).([]interface{})[1].(map[string]interface{})["protocol"], ShouldEqual, remoteData[listOfObjectsProperty.Name].([]interface{})[1].(map[string]interface{})["protocol"]) + }) + }) + }) + Convey("Given a resource factory", t, func() { r, resourceData := testCreateResourceFactory(t, stringWithPreferredNameProperty) Convey("When is called with a map remoteData containing more properties than then ones specified in the schema (this means the API is returning more info than the one specified in the swagger file)", func() { @@ -354,6 +490,78 @@ func TestUpdateStateWithPayloadData(t *testing.T) { }) } +func TestUpdateStateWithPayloadDataAndOptions(t *testing.T) { + Convey("Given a resource factory containing a schema with property lists that have the IgnoreItemsOrder set to true", t, func() { + specResource := &specStubResource{ + error: fmt.Errorf("some error"), + } + Convey("When updateStateWithPayloadDataAndOptions is called", func() { + err := updateStateWithPayloadDataAndOptions(specResource, nil, nil, true) + Convey("Then the err returned should match the expected one", func() { + So(err, ShouldEqual, specResource.error) + }) + }) + }) + Convey("Given a resource factory containing just a property ID", t, func() { + specResource := &specStubResource{ + schemaDefinition: &specSchemaDefinition{ + Properties: specSchemaDefinitionProperties{idProperty}, + }, + } + Convey("When updateStateWithPayloadDataAndOptions is called", func() { + remoteData := map[string]interface{}{ + idProperty.Name: "someID", + } + var resourceLocalData *schema.ResourceData + err := updateStateWithPayloadDataAndOptions(specResource, remoteData, resourceLocalData, true) + Convey("Then the err returned should be nil", func() { + So(err, ShouldBeNil) + }) + Convey("And the resource local data should be intact since the id property is ignored when updating the resource data file behind the scenes", func() { + So(resourceLocalData, ShouldEqual, nil) + }) + }) + }) + Convey("Given a resource factory containing a property with certain type", t, func() { + r, resourceData := testCreateResourceFactory(t, &specSchemaDefinitionProperty{ + Name: "wrong_property", + Type: typeObject, + SpecSchemaDefinition: &specSchemaDefinition{}, + }) + Convey("When updateStateWithPayloadDataAndOptions is called with a remote data containing the property but the value does not match the property type", func() { + remoteData := map[string]interface{}{ + "wrong_property": "someValueNotMatchingTheType", + } + err := updateStateWithPayloadDataAndOptions(r.openAPIResource, remoteData, resourceData, true) + Convey("Then the err returned should match the expected one", func() { + So(err.Error(), ShouldEqual, "wrong_property: must be a map") + }) + }) + }) + Convey("Given a resource factory containing a property with a type that the remote value does not match", t, func() { + r := &specStubResource{ + schemaDefinition: &specSchemaDefinition{ + Properties: specSchemaDefinitionProperties{ + &specSchemaDefinitionProperty{ + Name: "not_well_configured_property", + Type: typeList, + ArrayItemsType: schemaDefinitionPropertyType("unknown"), + }, + }, + }, + } + Convey("When updateStateWithPayloadDataAndOptions", func() { + remoteData := map[string]interface{}{ + "not_well_configured_property": []interface{}{"something"}, + } + err := updateStateWithPayloadDataAndOptions(r, remoteData, nil, true) + Convey("Then the err returned should match the expected one", func() { + So(err.Error(), ShouldEqual, "property 'not_well_configured_property' is supposed to be an array objects") + }) + }) + }) +} + func TestConvertPayloadToLocalStateDataValue(t *testing.T) { Convey("Given a resource factory", t, func() { @@ -788,3 +996,339 @@ func TestSetStateID(t *testing.T) { }) }) } + +func TestProcessIgnoreOrderIfEnabled(t *testing.T) { + testCases := []struct { + name string + property specSchemaDefinitionProperty + inputPropertyValue interface{} + remoteValue interface{} + expectedOutput interface{} + }{ + // String use cases + { + name: "required input list (of strings) matches the value returned by the API where order of input values match", + property: specSchemaDefinitionProperty{ + Name: "list_prop", + Type: typeList, + ArrayItemsType: typeString, + IgnoreItemsOrder: true, + }, + inputPropertyValue: []interface{}{"inputVal1", "inputVal2", "inputVal3"}, + remoteValue: []interface{}{"inputVal1", "inputVal2", "inputVal3"}, + expectedOutput: []interface{}{"inputVal1", "inputVal2", "inputVal3"}, + }, + { + name: "required input list (of strings) matches the value returned by the API where order of input values doesn't match", + property: specSchemaDefinitionProperty{ + Name: "list_prop", + Type: typeList, + ArrayItemsType: typeString, + IgnoreItemsOrder: true, + }, + inputPropertyValue: []interface{}{"inputVal3", "inputVal1", "inputVal2"}, + remoteValue: []interface{}{"inputVal2", "inputVal3", "inputVal1"}, + expectedOutput: []interface{}{"inputVal3", "inputVal1", "inputVal2"}, + }, + { + name: "required input list (of strings) has a value that isn't returned by the API (input order maintained)", + property: specSchemaDefinitionProperty{ + Name: "list_prop", + Type: typeList, + ArrayItemsType: typeString, + IgnoreItemsOrder: true, + }, + inputPropertyValue: []interface{}{"inputVal1", "inputVal2", "inputVal3"}, + remoteValue: []interface{}{"inputVal2", "inputVal1"}, + expectedOutput: []interface{}{"inputVal1", "inputVal2"}, + }, + { + name: "required input list (of strings) is missing a value returned by the API (input order maintained)", + property: specSchemaDefinitionProperty{ + Name: "list_prop", + Type: typeList, + ArrayItemsType: typeString, + IgnoreItemsOrder: true, + }, + inputPropertyValue: []interface{}{"inputVal1", "inputVal2"}, + remoteValue: []interface{}{"inputVal3", "inputVal2", "inputVal1"}, + expectedOutput: []interface{}{"inputVal1", "inputVal2", "inputVal3"}, + }, + + // Integer use cases + { + name: "required input list (of ints) matches the value returned by the API where order of input values doesn't match", + property: specSchemaDefinitionProperty{ + Name: "list_prop", + Type: typeList, + ArrayItemsType: typeInt, + IgnoreItemsOrder: true, + }, + inputPropertyValue: []interface{}{3, 1, 2}, + remoteValue: []interface{}{2, 3, 1}, + expectedOutput: []interface{}{3, 1, 2}, + }, + { + name: "required input list (of ints) has a value that isn't returned by the API (input order maintained)", + property: specSchemaDefinitionProperty{ + Name: "list_prop", + Type: typeList, + ArrayItemsType: typeInt, + IgnoreItemsOrder: true, + }, + inputPropertyValue: []interface{}{1, 2, 3}, + remoteValue: []interface{}{2, 1}, + expectedOutput: []interface{}{1, 2}, + }, + { + name: "required input list (of ints) is missing a value returned by the API (input order maintained)", + property: specSchemaDefinitionProperty{ + Name: "list_prop", + Type: typeList, + ArrayItemsType: typeInt, + IgnoreItemsOrder: true, + }, + inputPropertyValue: []interface{}{1, 2}, + remoteValue: []interface{}{3, 2, 1}, + expectedOutput: []interface{}{1, 2, 3}, + }, + + // Float use cases + { + name: "required input list (of floats) matches the value returned by the API where order of input values doesn't match", + property: specSchemaDefinitionProperty{ + Name: "list_prop", + Type: typeList, + ArrayItemsType: typeFloat, + IgnoreItemsOrder: true, + }, + inputPropertyValue: []interface{}{3.0, 1.0, 2.0}, + remoteValue: []interface{}{2.0, 3.0, 1.0}, + expectedOutput: []interface{}{3.0, 1.0, 2.0}, + }, + { + name: "required input list (of floats) has a value that isn't returned by the API (input order maintained)", + property: specSchemaDefinitionProperty{ + Name: "list_prop", + Type: typeList, + ArrayItemsType: typeFloat, + IgnoreItemsOrder: true, + }, + inputPropertyValue: []interface{}{1.0, 2.0, 3.0}, + remoteValue: []interface{}{2.0, 1.0}, + expectedOutput: []interface{}{1.0, 2.0}, + }, + { + name: "required input list (of floats) is missing a value returned by the API (input order maintained)", + property: specSchemaDefinitionProperty{ + Name: "list_prop", + Type: typeList, + ArrayItemsType: typeFloat, + IgnoreItemsOrder: true, + }, + inputPropertyValue: []interface{}{1.0, 2.0}, + remoteValue: []interface{}{3.0, 2.0, 1.0}, + expectedOutput: []interface{}{1.0, 2.0, 3.0}, + }, + + // List of objects use cases + { + name: "required input list (objects) matches the value returned by the API where order of input values doesn't match", + property: specSchemaDefinitionProperty{ + Name: "list_prop", + IgnoreItemsOrder: true, + Type: typeList, + ArrayItemsType: typeObject, + SpecSchemaDefinition: &specSchemaDefinition{ + Properties: specSchemaDefinitionProperties{ + &specSchemaDefinitionProperty{ + Name: "group", + Type: typeString, + }, + &specSchemaDefinitionProperty{ + Name: "roles", + Type: typeList, + ArrayItemsType: typeString, + }, + }, + }, + }, + inputPropertyValue: []interface{}{ + map[string]interface{}{ + "group": "someGroup", + "roles": []interface{}{"role1", "role2"}, + }, + map[string]interface{}{ + "group": "someOtherGroup", + "roles": []interface{}{"role3", "role4"}, + }, + }, + remoteValue: []interface{}{ + map[string]interface{}{ + "group": "someOtherGroup", + "roles": []interface{}{"role3", "role4"}, + }, + map[string]interface{}{ + "group": "someGroup", + "roles": []interface{}{"role1", "role2"}, + }, + }, + expectedOutput: []interface{}{ + map[string]interface{}{ + "group": "someGroup", + "roles": []interface{}{"role1", "role2"}, + }, + map[string]interface{}{ + "group": "someOtherGroup", + "roles": []interface{}{"role3", "role4"}, + }, + }, + }, + { + name: "required input list (objects) has a value that isn't returned by the API (input order maintained)", + property: specSchemaDefinitionProperty{ + Name: "list_prop", + IgnoreItemsOrder: true, + Type: typeList, + ArrayItemsType: typeObject, + SpecSchemaDefinition: &specSchemaDefinition{ + Properties: specSchemaDefinitionProperties{ + &specSchemaDefinitionProperty{ + Name: "group", + Type: typeString, + }, + &specSchemaDefinitionProperty{ + Name: "roles", + Type: typeList, + ArrayItemsType: typeString, + }, + }, + }, + }, + inputPropertyValue: []interface{}{ + map[string]interface{}{ + "group": "someGroup", + "roles": []interface{}{"role1", "role2"}, + }, + map[string]interface{}{ + "group": "someOtherGroup", + "roles": []interface{}{"role3", "role4"}, + }, + }, + remoteValue: []interface{}{ + map[string]interface{}{ + "group": "someGroup", + "roles": []interface{}{"role1", "role2"}, + }, + }, + expectedOutput: []interface{}{ + map[string]interface{}{ + "group": "someGroup", + "roles": []interface{}{"role1", "role2"}, + }, + }, + }, + { + name: "required input list (objects) doesn't have a value returned by the API", + property: specSchemaDefinitionProperty{ + Name: "list_prop", + IgnoreItemsOrder: true, + Type: typeList, + ArrayItemsType: typeObject, + SpecSchemaDefinition: &specSchemaDefinition{ + Properties: specSchemaDefinitionProperties{ + &specSchemaDefinitionProperty{ + Name: "group", + Type: typeString, + }, + &specSchemaDefinitionProperty{ + Name: "roles", + Type: typeList, + ArrayItemsType: typeString, + }, + }, + }, + }, + inputPropertyValue: []interface{}{ + map[string]interface{}{ + "group": "someGroup", + "roles": []interface{}{"role1", "role2"}, + }, + }, + remoteValue: []interface{}{ + map[string]interface{}{ + "group": "someGroup", + "roles": []interface{}{"role1", "role2"}, + }, + map[string]interface{}{ + "group": "unexpectedGroup", + "roles": []interface{}{"role3", "role4"}, + }, + }, + expectedOutput: []interface{}{ + map[string]interface{}{ + "group": "someGroup", + "roles": []interface{}{"role1", "role2"}, + }, + map[string]interface{}{ + "group": "unexpectedGroup", + "roles": []interface{}{"role3", "role4"}, + }, + }, + }, + { + name: "inputPropertyValue is nil", + property: specSchemaDefinitionProperty{ + Name: "list_prop", + Type: typeList, + ArrayItemsType: typeString, + IgnoreItemsOrder: true, + }, + inputPropertyValue: nil, + remoteValue: []interface{}{}, + expectedOutput: []interface{}{}, + }, + { + name: "remoteValue is nil", + property: specSchemaDefinitionProperty{ + Name: "list_prop", + Type: typeList, + ArrayItemsType: typeString, + IgnoreItemsOrder: true, + }, + inputPropertyValue: []interface{}{}, + remoteValue: nil, + expectedOutput: nil, + }, + { + name: "IgnoreItemsOrder is set to false", + property: specSchemaDefinitionProperty{ + Name: "list_prop", + Type: typeList, + ArrayItemsType: typeString, + IgnoreItemsOrder: false, + }, + inputPropertyValue: []interface{}{"inputVal1"}, + remoteValue: []interface{}{"inputVal1", "inputVal2"}, + expectedOutput: []interface{}{"inputVal1", "inputVal2"}, + }, + { + name: "list of bools property definition and the corresponding input/remote lists", + property: specSchemaDefinitionProperty{ + Name: "list_prop", + Type: typeList, + ArrayItemsType: typeBool, + IgnoreItemsOrder: true, + Required: true, + }, + inputPropertyValue: []interface{}{true}, + remoteValue: []interface{}{false}, + expectedOutput: []interface{}{false}, + }, + } + + for _, tc := range testCases { + output := processIgnoreOrderIfEnabled(tc.property, tc.inputPropertyValue, tc.remoteValue) + assert.Equal(t, tc.expectedOutput, output, tc.name) + } +} diff --git a/openapi/data_source_factory.go b/openapi/data_source_factory.go index 3ad40a14c..ef0977f84 100644 --- a/openapi/data_source_factory.go +++ b/openapi/data_source_factory.go @@ -125,7 +125,7 @@ func (d dataSourceFactory) read(data *schema.ResourceData, i interface{}) error return err } - return updateStateWithPayloadData(d.openAPIResource, filteredResults[0], data) + return dataSourceUpdateStateWithPayloadData(d.openAPIResource, filteredResults[0], data) } func (d dataSourceFactory) filterMatch(filters filters, payloadItem map[string]interface{}) bool { diff --git a/openapi/data_source_instance_factory.go b/openapi/data_source_instance_factory.go index fdcba06b7..b126c3e88 100644 --- a/openapi/data_source_instance_factory.go +++ b/openapi/data_source_instance_factory.go @@ -84,5 +84,5 @@ func (d dataSourceInstanceFactory) read(data *schema.ResourceData, i interface{} if err != nil { return err } - return updateStateWithPayloadData(d.openAPIResource, responsePayload, data) + return dataSourceUpdateStateWithPayloadData(d.openAPIResource, responsePayload, data) } diff --git a/openapi/openapi_spec_resource_schema_definition_property.go b/openapi/openapi_spec_resource_schema_definition_property.go index d13a8529f..916919ff0 100644 --- a/openapi/openapi_spec_resource_schema_definition_property.go +++ b/openapi/openapi_spec_resource_schema_definition_property.go @@ -2,6 +2,7 @@ package openapi import ( "fmt" + "reflect" "github.com/dikhan/terraform-provider-openapi/openapi/terraformutils" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" @@ -28,7 +29,11 @@ type specSchemaDefinitionProperty struct { PreferredName string Type schemaDefinitionPropertyType ArrayItemsType schemaDefinitionPropertyType - Required bool + + // IgnoreItemsOrder if set to true means that the array items order should be ignored + IgnoreItemsOrder bool + + Required bool // ReadOnly properties are included in responses but not in request ReadOnly bool // Computed properties describe properties where the value is computed by the API @@ -110,6 +115,10 @@ func (s *specSchemaDefinitionProperty) isArrayProperty() bool { return s.Type == typeList } +func (s *specSchemaDefinitionProperty) shouldIgnoreOrder() bool { + return s.Type == typeList && s.IgnoreItemsOrder +} + func (s *specSchemaDefinitionProperty) isArrayOfObjectsProperty() bool { return s.Type == typeList && s.ArrayItemsType == typeObject } @@ -126,6 +135,10 @@ func (s *specSchemaDefinitionProperty) isOptional() bool { return !s.Required } +func (s *specSchemaDefinitionProperty) shouldIgnoreArrayItemsOrder() bool { + return s.isArrayProperty() && s.IgnoreItemsOrder +} + // isComputed returns true if one of the following cases is met: //- The property is optional (marked as required=false), in which case there few use cases: // - readOnly properties (marked as readOnly=true): @@ -292,3 +305,64 @@ func (s *specSchemaDefinitionProperty) validateFunc() schema.SchemaValidateFunc return } } + +func (s *specSchemaDefinitionProperty) equal(item1, item2 interface{}) bool { + return s.equalItems(s.Type, item1, item2) +} + +func (s *specSchemaDefinitionProperty) equalItems(itemsType schemaDefinitionPropertyType, item1, item2 interface{}) bool { + switch itemsType { + case typeString: + if !s.validateValueType(item1, reflect.String) && !s.validateValueType(item2, reflect.String) { + return false + } + case typeInt: + if !s.validateValueType(item1, reflect.Int) && !s.validateValueType(item2, reflect.Int) { + return false + } + case typeFloat: + if !s.validateValueType(item1, reflect.Float64) && !s.validateValueType(item2, reflect.Float64) { + return false + } + case typeBool: + 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 + } + list1 := item1.([]interface{}) + list2 := item2.([]interface{}) + if len(list1) != len(list2) { + return false + } + for idx := range list1 { + return s.equalItems(s.ArrayItemsType, list1[idx], list2[idx]) + } + case typeObject: + 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] + if !objectProperty.equal(objectPropertyValue1, objectPropertyValue2) { + return false + } + } + return true + default: + return false + } + return item1 == item2 +} + +func (s *specSchemaDefinitionProperty) validateValueType(item interface{}, expectedKind reflect.Kind) bool { + if reflect.TypeOf(item).Kind() != expectedKind { + return false + } + return true +} diff --git a/openapi/openapi_spec_resource_schema_definition_property_test.go b/openapi/openapi_spec_resource_schema_definition_property_test.go index 8b1befd2f..05153bbcf 100644 --- a/openapi/openapi_spec_resource_schema_definition_property_test.go +++ b/openapi/openapi_spec_resource_schema_definition_property_test.go @@ -354,6 +354,60 @@ func TestIsRequired(t *testing.T) { }) } +func TestShouldIgnoreArrayItemsOrder(t *testing.T) { + Convey("Given a specSchemaDefinitionProperty that is a typeList and where the 'x-terraform-ignore-order' ext is set to true", t, func() { + s := &specSchemaDefinitionProperty{ + Name: "array_prop", + Type: typeList, + IgnoreItemsOrder: true, + } + Convey("When shouldIgnoreArrayItemsOrder method is called", func() { + isRequired := s.shouldIgnoreArrayItemsOrder() + Convey("Then the resulted bool should be true", func() { + So(isRequired, ShouldBeTrue) + }) + }) + }) + Convey("Given a specSchemaDefinitionProperty that is a typeList and where the 'x-terraform-ignore-order' ext is set to false", t, func() { + s := &specSchemaDefinitionProperty{ + Name: "array_prop", + Type: typeList, + IgnoreItemsOrder: false, + } + Convey("When shouldIgnoreArrayItemsOrder method is called", func() { + isRequired := s.shouldIgnoreArrayItemsOrder() + Convey("Then the resulted bool should be false", func() { + So(isRequired, ShouldBeFalse) + }) + }) + }) + Convey("Given a specSchemaDefinitionProperty that is a typeList and where the 'x-terraform-ignore-order' ext is NOT set", t, func() { + s := &specSchemaDefinitionProperty{ + Name: "array_prop", + Type: typeList, + } + Convey("When shouldIgnoreArrayItemsOrder method is called", func() { + isRequired := s.shouldIgnoreArrayItemsOrder() + Convey("Then the resulted bool should be false", func() { + So(isRequired, ShouldBeFalse) + }) + }) + }) + Convey("Given a specSchemaDefinitionProperty that is NOT a typeList where the 'x-terraform-ignore-order' ext is set", t, func() { + s := &specSchemaDefinitionProperty{ + Name: "string_prop", + Type: typeString, + IgnoreItemsOrder: true, + } + Convey("When shouldIgnoreArrayItemsOrder method is called", func() { + isRequired := s.shouldIgnoreArrayItemsOrder() + Convey("Then the resulted bool should be false", func() { + So(isRequired, ShouldBeFalse) + }) + }) + }) +} + func TestSchemaDefinitionPropertyIsComputed(t *testing.T) { Convey("Given a specSchemaDefinitionProperty that is optional and readonly", t, func() { s := &specSchemaDefinitionProperty{ @@ -1600,6 +1654,281 @@ func TestValidateFunc(t *testing.T) { }) } +func TestEqualItems(t *testing.T) { + testCases := []struct { + name string + schemaDefProp specSchemaDefinitionProperty + propertyType schemaDefinitionPropertyType + arrayItemsPropType schemaDefinitionPropertyType + inputItem interface{} + remoteItem interface{} + expectedOutput bool + }{ + // String use cases + { + name: "string input value matches string remote value", + schemaDefProp: specSchemaDefinitionProperty{Type: typeString}, + inputItem: "inputVal1", + remoteItem: "inputVal1", + expectedOutput: true, + }, + { + name: "string input value doesn't match string remote value", + schemaDefProp: specSchemaDefinitionProperty{Type: typeString}, + inputItem: "inputVal1", + remoteItem: "inputVal2", + expectedOutput: false, + }, + // Integer use cases + { + name: "int input value matches int remote value", + schemaDefProp: specSchemaDefinitionProperty{Type: typeInt}, + inputItem: 1, + remoteItem: 1, + expectedOutput: true, + }, + { + name: "int input value doesn't match int remote value", + schemaDefProp: specSchemaDefinitionProperty{Type: typeInt}, + inputItem: 1, + remoteItem: 2, + expectedOutput: false, + }, + // Float use cases + { + name: "float input value matches float remote value", + schemaDefProp: specSchemaDefinitionProperty{Type: typeFloat}, + inputItem: 1.0, + remoteItem: 1.0, + expectedOutput: true, + }, + { + name: "float input value doesn't match float remote value", + schemaDefProp: specSchemaDefinitionProperty{Type: typeFloat}, + inputItem: 1.0, + remoteItem: 2.0, + expectedOutput: false, + }, + // Bool use cases + { + name: "bool input value matches bool remote value", + schemaDefProp: specSchemaDefinitionProperty{Type: typeBool}, + inputItem: true, + remoteItem: true, + expectedOutput: true, + }, + { + name: "bool input value doesn't match bool remote value", + schemaDefProp: specSchemaDefinitionProperty{Type: typeBool}, + inputItem: true, + remoteItem: false, + expectedOutput: false, + }, + // List use cases + { + name: "list input value matches list remote value", + schemaDefProp: specSchemaDefinitionProperty{ + Type: typeList, + ArrayItemsType: typeString, + }, + inputItem: []interface{}{"role1", "role2"}, + remoteItem: []interface{}{"role1", "role2"}, + expectedOutput: true, + }, + { + name: "list input value doesn't match list remote value (same list length)", + schemaDefProp: specSchemaDefinitionProperty{ + Type: typeList, + ArrayItemsType: typeString, + }, + inputItem: []interface{}{"role1", "role2"}, + remoteItem: []interface{}{"role3", "role4"}, + expectedOutput: false, + }, + { + name: "list input value doesn't match list remote value (different list length)", + schemaDefProp: specSchemaDefinitionProperty{ + Type: typeList, + ArrayItemsType: typeString, + }, + inputItem: []interface{}{"role1", "role2"}, + remoteItem: []interface{}{"role1"}, + expectedOutput: false, + }, + // Object use cases + { + name: "object input value matches object remote value", + schemaDefProp: specSchemaDefinitionProperty{ + Type: typeObject, + SpecSchemaDefinition: &specSchemaDefinition{ + Properties: specSchemaDefinitionProperties{ + &specSchemaDefinitionProperty{ + Name: "group", + Type: typeString, + }, + }, + }, + }, + inputItem: map[string]interface{}{"group": "someGroup"}, + remoteItem: map[string]interface{}{"group": "someGroup"}, + expectedOutput: true, + }, + { + name: "object input value doesn't match object remote value", + schemaDefProp: specSchemaDefinitionProperty{ + Type: typeObject, + SpecSchemaDefinition: &specSchemaDefinition{ + Properties: specSchemaDefinitionProperties{ + &specSchemaDefinitionProperty{ + Name: "group", + Type: typeString, + }, + }, + }, + }, + inputItem: map[string]interface{}{"group": "someGroup"}, + remoteItem: map[string]interface{}{"group": "someOtherGroup"}, + expectedOutput: false, + }, + } + + for _, tc := range testCases { + output := tc.schemaDefProp.equal(tc.inputItem, tc.remoteItem) + assert.Equal(t, tc.expectedOutput, output, tc.name) + } +} + +func TestValidateValueType(t *testing.T) { + testCases := []struct { + name string + item interface{} + itemKind reflect.Kind + expectedOutput bool + }{ + // String use cases + { + name: "expect string kind and item is a string", + item: "inputVal1", + itemKind: reflect.String, + expectedOutput: true, + }, + { + name: "expect string kind and item is NOT a string", + item: 1, + itemKind: reflect.String, + expectedOutput: false, + }, + // Int use cases + { + name: "expect int kind and item is a int", + item: 1, + itemKind: reflect.Int, + expectedOutput: true, + }, + { + name: "expect int kind and item is NOT a int", + item: "not an int", + itemKind: reflect.Int, + expectedOutput: false, + }, + // Float use cases + { + name: "expect float kind and item is a float", + item: 1.0, + itemKind: reflect.Float64, + expectedOutput: true, + }, + { + name: "expect float kind and item is NOT a float", + item: "not a float", + itemKind: reflect.Float64, + expectedOutput: false, + }, + // Bool use cases + { + name: "expect bool kind and item is a bool", + item: true, + itemKind: reflect.Bool, + expectedOutput: true, + }, + { + name: "expect bool kind and item is NOT a bool", + item: "not a bool", + itemKind: reflect.Bool, + expectedOutput: false, + }, + // List use cases + { + name: "expect slice kind and item is a slice", + item: []interface{}{"item1", "item2"}, + itemKind: reflect.Slice, + expectedOutput: true, + }, + { + name: "expect slice kind and item is NOT a slice", + item: "not a slice", + itemKind: reflect.Slice, + expectedOutput: false, + }, + // Object use cases + { + name: "expect map kind and item is a map", + item: map[string]interface{}{"group": "someGroup"}, + itemKind: reflect.Map, + expectedOutput: true, + }, + { + name: "expect map kind and item is NOT a map", + item: "not a map", + itemKind: reflect.Map, + expectedOutput: false, + }, + } + for _, tc := range testCases { + s := specSchemaDefinitionProperty{} + output := s.validateValueType(tc.item, tc.itemKind) + assert.Equal(t, tc.expectedOutput, output, tc.name) + } +} + +func Test_shouldIgnoreOrder(t *testing.T) { + Convey("Given a scjema definition property that is a list and configured with ignore order", t, func() { + p := &specSchemaDefinitionProperty{ + Type: typeList, + IgnoreItemsOrder: true, + } + Convey("When shouldIgnoreOrder is called", func() { + shouldIgnoreOrder := p.shouldIgnoreOrder() + Convey("Then the err returned should be true", func() { + So(shouldIgnoreOrder, ShouldBeTrue) + }) + }) + }) + Convey("Given a scjema definition property that is NOT a list", t, func() { + p := &specSchemaDefinitionProperty{ + Type: typeString, + } + Convey("When shouldIgnoreOrder is called", func() { + shouldIgnoreOrder := p.shouldIgnoreOrder() + Convey("Then the err returned should be false", func() { + So(shouldIgnoreOrder, ShouldBeFalse) + }) + }) + }) + Convey("Given a scjema definition property that is a list but the ignore order is set to false", t, func() { + p := &specSchemaDefinitionProperty{ + Type: typeList, + IgnoreItemsOrder: false, + } + Convey("When shouldIgnoreOrder is called", func() { + shouldIgnoreOrder := p.shouldIgnoreOrder() + Convey("Then the err returned should be false", func() { + So(shouldIgnoreOrder, ShouldBeFalse) + }) + }) + }) +} + func Test_shouldUseLegacyTerraformSDKBlockApproachForComplexObjects(t *testing.T) { Convey("shouldUseLegacyTerraformSDKBlockApproachForComplexObject", t, func() { diff --git a/openapi/openapi_v2_resource.go b/openapi/openapi_v2_resource.go index 0153cc0df..23342f9bd 100644 --- a/openapi/openapi_v2_resource.go +++ b/openapi/openapi_v2_resource.go @@ -57,6 +57,7 @@ const extTfFieldStatus = "x-terraform-field-status" const extTfID = "x-terraform-id" const extTfComputed = "x-terraform-computed" const extTfComplexObjectType = "x-terraform-complex-object-legacy-config" +const extTfIgnoreOrder = "x-terraform-ignore-order" // Operation level extensions const extTfResourceTimeout = "x-terraform-resource-timeout" @@ -390,6 +391,11 @@ func (o *SpecV2Resource) createSchemaDefinitionProperty(propertyName string, pro } schemaDefinitionProperty.ArrayItemsType = itemsType schemaDefinitionProperty.SpecSchemaDefinition = itemsSchema // only diff than nil if type is object + + if o.isBoolExtensionEnabled(property.Extensions, extTfIgnoreOrder) { + schemaDefinitionProperty.IgnoreItemsOrder = true + } + log.Printf("[DEBUG] found array type property '%s' with items of type '%s'", propertyName, itemsType) } diff --git a/openapi/openapi_v2_resource_test.go b/openapi/openapi_v2_resource_test.go index ef8a74508..4b7947e00 100644 --- a/openapi/openapi_v2_resource_test.go +++ b/openapi/openapi_v2_resource_test.go @@ -2468,6 +2468,34 @@ func TestCreateSchemaDefinitionProperty(t *testing.T) { }) }) + Convey("When createSchemaDefinitionProperty is called with a property schema that has the 'x-terraform-ignore-order' extension", func() { + expectedIgnoreOrder := true + propertySchema := spec.Schema{ + SchemaProps: spec.SchemaProps{ + Type: spec.StringOrArray{"array"}, + Items: &spec.SchemaOrArray{ + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Type: spec.StringOrArray{"string"}, + }, + }, + }, + }, + VendorExtensible: spec.VendorExtensible{ + Extensions: spec.Extensions{ + extTfIgnoreOrder: expectedIgnoreOrder, + }, + }, + } + schemaDefinitionProperty, err := r.createSchemaDefinitionProperty("propertyName", propertySchema, []string{}) + Convey("Then the error returned should be nil", func() { + So(err, ShouldBeNil) + }) + Convey("And the schema definition property should have the IgnoreItemsOrder field enabled", func() { + So(schemaDefinitionProperty.IgnoreItemsOrder, ShouldEqual, expectedIgnoreOrder) + }) + }) + Convey("When createSchemaDefinitionProperty is called with a property schema that has the 'x-terraform-field-status' extension", func() { expectedIsStatusFieldValue := true propertySchema := spec.Schema{ diff --git a/tests/e2e/gray_box_cdns_test.go b/tests/e2e/gray_box_cdns_test.go index b4e51a376..b2ff72d24 100644 --- a/tests/e2e/gray_box_cdns_test.go +++ b/tests/e2e/gray_box_cdns_test.go @@ -438,6 +438,131 @@ func (a *api) apiResponse(t *testing.T, responseBody string, httpResponseStatusC } } +//TODO: Check that test passes after implementing fix to ignore order of array props +func TestAccCDN_CreateResourceWithIgnoreListOrderExtension(t *testing.T) { + swagger := `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"` + apiServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodDelete { + w.WriteHeader(http.StatusNotFound) + return + } + body := `{"id": "someid", "label":"some label", "list_prop": ["value1", "value2", "value3"]}` + w.WriteHeader(http.StatusOK) + w.Write([]byte(body)) + })) + apiHost := apiServer.URL[7:] + swaggerServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + swaggerReturned := fmt.Sprintf(swagger, apiHost) + w.Write([]byte(swaggerReturned)) + })) + + tfFileContents := `# URI /v1/cdns/ +resource "openapi_cdn_v1" "my_cdn" { + label = "some label" + list_prop = ["value3", "value1", "value2"] +}` + + p := openapi.ProviderOpenAPI{ProviderName: providerName} + provider, err := p.CreateSchemaProviderFromServiceConfiguration(&openapi.ServiceConfigStub{SwaggerURL: swaggerServer.URL}) + assert.NoError(t, err) + + var testAccProviders = map[string]terraform.ResourceProvider{providerName: provider} + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + Providers: testAccProviders, + PreCheck: func() { testAccPreCheck(t, swaggerServer.URL) }, + Steps: []resource.TestStep{ + { + ExpectNonEmptyPlan: false, + Config: tfFileContents, + Check: resource.ComposeTestCheckFunc( + // check resource + resource.TestCheckResourceAttr( + openAPIResourceStateCDN, "label", "some label"), + resource.TestCheckResourceAttr( + openAPIResourceStateCDN, "list_prop.#", "3"), + resource.TestCheckResourceAttr( + openAPIResourceStateCDN, "list_prop.0", "value3"), + resource.TestCheckResourceAttr( + openAPIResourceStateCDN, "list_prop.1", "value1"), + resource.TestCheckResourceAttr( + openAPIResourceStateCDN, "list_prop.2", "value2"), + ), + }, + }, + }) +} + func TestAccCDN_Create_and_UpdateSubResource(t *testing.T) { api := initAPI(t, cdnSwaggerYAMLTemplate) tfFileContents := createTerraformFile(expectedCDNLabel, expectedCDNFirewallLabel)