From 4696163db05c4a35b3063ab254eec4d6f5090e49 Mon Sep 17 00:00:00 2001 From: dikhan Date: Thu, 19 Sep 2019 17:47:14 -0700 Subject: [PATCH 01/37] extend model def to include array of strings prop - asserted that the schema contains the expected property as computed - added a todo to extend the example with more properties so we have a full fledge fart unit example :) --- openapi/provider_test.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/openapi/provider_test.go b/openapi/provider_test.go index 0d3ba04e9..ffcd7eec6 100644 --- a/openapi/provider_test.go +++ b/openapi/provider_test.go @@ -229,7 +229,13 @@ definitions: type: "string" readOnly: true label: - type: "string"` + type: "string" + owners: + type: array + items: + type: string` + + // TODO: expand model definition to include other primitives that user can filter on AND also more complex property and ensure they are congfigured property in the schema as computed swaggerServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Write([]byte(swaggerContent)) @@ -261,12 +267,19 @@ definitions: resourceName := fmt.Sprintf("%s_cdn_datasource_v1", providerName) So(tfProvider.DataSourcesMap, ShouldContainKey, resourceName) + // TODO: dry out the assertions So(tfProvider.DataSourcesMap[resourceName].Schema, ShouldContainKey, "label") So(tfProvider.DataSourcesMap[resourceName].Schema["label"].Type, ShouldEqual, schema.TypeString) So(tfProvider.DataSourcesMap[resourceName].Schema["label"].Required, ShouldBeFalse) So(tfProvider.DataSourcesMap[resourceName].Schema["label"].Optional, ShouldBeTrue) So(tfProvider.DataSourcesMap[resourceName].Schema["label"].Computed, ShouldBeTrue) + So(tfProvider.DataSourcesMap[resourceName].Schema, ShouldContainKey, "owners") + So(tfProvider.DataSourcesMap[resourceName].Schema["owners"].Type, ShouldEqual, schema.TypeList) + So(tfProvider.DataSourcesMap[resourceName].Schema["owners"].Required, ShouldBeFalse) + So(tfProvider.DataSourcesMap[resourceName].Schema["owners"].Optional, ShouldBeTrue) + So(tfProvider.DataSourcesMap[resourceName].Schema["owners"].Computed, ShouldBeTrue) + So(tfProvider.DataSourcesMap[resourceName].Schema, ShouldContainKey, "filter") So(tfProvider.DataSourcesMap[resourceName].Schema["filter"].Type, ShouldEqual, schema.TypeSet) So(tfProvider.DataSourcesMap[resourceName].Schema["filter"].Required, ShouldBeFalse) From 0944432df0dd77aa7a8c64b0cacc6206f094ddb9 Mon Sep 17 00:00:00 2001 From: dikhan Date: Thu, 19 Sep 2019 17:48:16 -0700 Subject: [PATCH 02/37] update the schema to incluide an array of strings and assert the state contains the owners too - added some todos --- openapi/data_source_factory_test.go | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/openapi/data_source_factory_test.go b/openapi/data_source_factory_test.go index 3ca05b10c..98c6a544c 100644 --- a/openapi/data_source_factory_test.go +++ b/openapi/data_source_factory_test.go @@ -119,6 +119,7 @@ func TestDataSourceRead(t *testing.T) { Properties: specSchemaDefinitionProperties{ newStringSchemaDefinitionPropertyWithDefaults("id", "", false, true, nil), newStringSchemaDefinitionPropertyWithDefaults("label", "", false, false, nil), + newListSchemaDefinitionPropertyWithDefaults("owners", "", true, false, false, []string{"value1"}, typeString, nil), }, }, }, @@ -131,7 +132,7 @@ func TestDataSourceRead(t *testing.T) { expectedResult map[string]interface{} expectedError error }{ - // TODO: add a test to cover sub-resource use case too + // TODO: add a test to cover sub-resource use case too (need to cover this before releasing data source support) { name: "fetch selected data source as per filter configuration (label=someLabel)", filtersInput: []map[string]interface{}{ @@ -139,12 +140,14 @@ func TestDataSourceRead(t *testing.T) { }, responsePayload: []map[string]interface{}{ { - "id": "someID", - "label": "someLabel", + "id": "someID", + "label": "someLabel", + "owners": []string{"someOwner"}, }, { - "id": "someOtherID", - "label": "someOtherLabel", + "id": "someOtherID", + "label": "someOtherLabel", + "owners": []string{}, }, }, expectedError: nil, @@ -206,9 +209,14 @@ func TestDataSourceRead(t *testing.T) { if tc.expectedError == nil { assert.Nil(t, err, tc.name) // assert that the filtered data source contains the same values as the ones returned by the API - assert.Equal(t, client.responseListPayload[0]["label"], resourceData.Get("label"), tc.name) - assert.Equal(t, 6, len(resourceData.State().Attributes), tc.name) //this asserts that ONLY 1 element is returned when the filter is applied (2 prop of the elelemnt + 4 prop given by the filter) + assert.Equal(t, 8, len(resourceData.State().Attributes), tc.name) //this asserts that ONLY 1 element is returned when the filter is applied (2 prop of the elelemnt + 4 prop given by the filter) assert.Equal(t, client.responseListPayload[0]["id"], resourceData.Id(), tc.name) //resourceData.Id() is being called instead of resourceData.Get("id") because id property is a special one kept by Terraform + assert.Equal(t, client.responseListPayload[0]["label"], resourceData.Get("label"), tc.name) + expectedOwners := client.responseListPayload[0]["owners"].([]string) + owners := resourceData.Get("owners").([]interface{}) + assert.NotNil(t, owners, tc.name) + assert.NotNil(t, len(expectedOwners), len(owners), tc.name) + assert.Equal(t, expectedOwners[0], owners[0], tc.name) } else { assert.Equal(t, tc.expectedError.Error(), err.Error(), tc.name) } From 03cf8a20e3a8c4d94ff991046ff390c1e21ecb0b Mon Sep 17 00:00:00 2001 From: dikhan Date: Thu, 19 Sep 2019 17:48:52 -0700 Subject: [PATCH 03/37] added todo to cover nested objects and make sure they are made computed too --- openapi/openapi_spec_resource_schema_definition.go | 1 + 1 file changed, 1 insertion(+) diff --git a/openapi/openapi_spec_resource_schema_definition.go b/openapi/openapi_spec_resource_schema_definition.go index 85e9a3b65..7821153b7 100644 --- a/openapi/openapi_spec_resource_schema_definition.go +++ b/openapi/openapi_spec_resource_schema_definition.go @@ -23,6 +23,7 @@ func (s *specSchemaDefinition) createDataSourceSchema() (map[string]*schema.Sche if err != nil { return nil, err } + // TODO: make sure the nested objects properties are also made computed to avoid surprises for propertyName := range terraformSchema { terraformSchema[propertyName].Required = false terraformSchema[propertyName].Optional = true From b1dd16c5ad4cef70e3a2867c5bb6b9c4aa4aba03 Mon Sep 17 00:00:00 2001 From: fradiben Date: Fri, 20 Sep 2019 16:01:38 +0100 Subject: [PATCH 04/37] [wip] adding nested object support --- openapi/data_source_factory_test.go | 157 +++++++++++++----- ...pi_spec_resource_schema_definition_test.go | 39 +++++ 2 files changed, 157 insertions(+), 39 deletions(-) diff --git a/openapi/data_source_factory_test.go b/openapi/data_source_factory_test.go index 98c6a544c..b06b13621 100644 --- a/openapi/data_source_factory_test.go +++ b/openapi/data_source_factory_test.go @@ -2,6 +2,7 @@ package openapi import ( "errors" + "fmt" "testing" "github.com/stretchr/testify/require" @@ -134,7 +135,7 @@ func TestDataSourceRead(t *testing.T) { }{ // TODO: add a test to cover sub-resource use case too (need to cover this before releasing data source support) { - name: "fetch selected data source as per filter configuration (label=someLabel)", + name: "fetch selected data source as per filter configuration (label=someLabel) when the filter is into a nested object", filtersInput: []map[string]interface{}{ newFilter("label", []string{"someLabel"}), }, @@ -152,44 +153,82 @@ func TestDataSourceRead(t *testing.T) { }, expectedError: nil, }, - { - name: "no filter match", - filtersInput: []map[string]interface{}{ - newFilter("label", []string{"some non existing label"}), - }, - responsePayload: []map[string]interface{}{ - { - "id": "someID", - "label": "someLabel", - }, - }, - expectedError: errors.New("your query returned no results. Please change your search criteria and try again"), - }, - { - name: "after filtering the result contains more than one element", - filtersInput: []map[string]interface{}{ - newFilter("label", []string{"my_label"}), - }, - responsePayload: []map[string]interface{}{ - { - "id": "someID", - "label": "my_label", - }, - { - "id": "someOtherID", - "label": "my_label", - }, - }, - expectedError: errors.New("your query returned contains more than one result. Please change your search criteria to make it more specific"), - }, - { - name: "validate input fails", - filtersInput: []map[string]interface{}{ - newFilter("non_existing_property", []string{"my_label"}), - }, - responsePayload: []map[string]interface{}{}, - expectedError: errors.New("filter name does not match any of the schema properties: property with name 'non_existing_property' not existing in resource schema definition"), - }, + //{ + // name: "fetch selected data source as per filter configuration (label=someLabel) when the filter a top level property of a nested object", + // filtersInput: []map[string]interface{}{ + // newFilter("label", []string{"someLabel"}), + // }, + // responsePayload: []map[string]interface{}{ + // { + // "id": "someID", + // "label": "someLabel", + // "owners": []string{"someOwner"}, + // }, + // { + // "id": "someOtherID", + // "label": "someOtherLabel", + // "owners": []string{}, + // }, + // }, + // expectedError: nil, + //}, + //{ + // name: "fetch selected data source as per filter configuration (label=someLabel)", + // filtersInput: []map[string]interface{}{ + // newFilter("label", []string{"someLabel"}), + // }, + // responsePayload: []map[string]interface{}{ + // { + // "id": "someID", + // "label": "someLabel", + // "owners": []string{"someOwner"}, + // }, + // { + // "id": "someOtherID", + // "label": "someOtherLabel", + // "owners": []string{}, + // }, + // }, + // expectedError: nil, + //}, + //{ + // name: "no filter match", + // filtersInput: []map[string]interface{}{ + // newFilter("label", []string{"some non existing label"}), + // }, + // responsePayload: []map[string]interface{}{ + // { + // "id": "someID", + // "label": "someLabel", + // }, + // }, + // expectedError: errors.New("your query returned no results. Please change your search criteria and try again"), + //}, + //{ + // name: "after filtering the result contains more than one element", + // filtersInput: []map[string]interface{}{ + // newFilter("label", []string{"my_label"}), + // }, + // responsePayload: []map[string]interface{}{ + // { + // "id": "someID", + // "label": "my_label", + // }, + // { + // "id": "someOtherID", + // "label": "my_label", + // }, + // }, + // expectedError: errors.New("your query returned contains more than one result. Please change your search criteria to make it more specific"), + //}, + //{ + // name: "validate input fails", + // filtersInput: []map[string]interface{}{ + // newFilter("non_existing_property", []string{"my_label"}), + // }, + // responsePayload: []map[string]interface{}{}, + // expectedError: errors.New("filter name does not match any of the schema properties: property with name 'non_existing_property' not existing in resource schema definition"), + //}, } for _, tc := range testCases { @@ -223,6 +262,46 @@ func TestDataSourceRead(t *testing.T) { } } +func TestDataSourceRead_ForNestedObjects(t *testing.T) { + nestedObjectSchemaDefinition := &specSchemaDefinition{ + Properties: specSchemaDefinitionProperties{ + newIntSchemaDefinitionPropertyWithDefaults("origin_port", "", true, false, 80), + newStringSchemaDefinitionPropertyWithDefaults("protocol", "", true, false, "http"), + }, + } + nestedObjectDefault := map[string]interface{}{ + "origin_port": nestedObjectSchemaDefinition.Properties[0].Default, + "protocol": nestedObjectSchemaDefinition.Properties[1].Default, + } + nestedObject := newObjectSchemaDefinitionPropertyWithDefaults("nested_object", "", true, false, false, nestedObjectDefault, nestedObjectSchemaDefinition) + propertyWithNestedObjectSchemaDefinition := &specSchemaDefinition{ + Properties: specSchemaDefinitionProperties{ + idProperty, + nestedObject, + }, + } + dataValue := map[string]interface{}{ + "id": propertyWithNestedObjectSchemaDefinition.Properties[0].Default, + "nested_object": propertyWithNestedObjectSchemaDefinition.Properties[1].Default, + } + + dataSourceSchema := newObjectSchemaDefinitionPropertyWithDefaults("nested-oobj", "", true, false, false, dataValue, propertyWithNestedObjectSchemaDefinition) + + // Given + dataSourceFactory := dataSourceFactory{ + openAPIResource: &specStubResource{ + schemaDefinition: &specSchemaDefinition{ + Properties: specSchemaDefinitionProperties{dataSourceSchema}, + }, + }, + } + + fmt.Println(dataSourceFactory) //todo work on this + fs, err := dataSourceFactory.createTerraformDataSourceSchema() + fmt.Println(err, fs) + +} + func TestDataSourceRead_Fails_Because_Cannot_extract_ParentsID(t *testing.T) { err := dataSourceFactory{}.read(nil, &clientOpenAPIStub{}) assert.EqualError(t, err, "can't get parent ids from a resourceFactory with no openAPIResource") diff --git a/openapi/openapi_spec_resource_schema_definition_test.go b/openapi/openapi_spec_resource_schema_definition_test.go index 5034eb7ff..518581249 100644 --- a/openapi/openapi_spec_resource_schema_definition_test.go +++ b/openapi/openapi_spec_resource_schema_definition_test.go @@ -84,6 +84,45 @@ func TestCreateDataSourceSchema(t *testing.T) { } } +func TestCreateDataSourceSchema_ForNestedObjects(t *testing.T) { + nestedObjectSchemaDefinition := &specSchemaDefinition{ + Properties: specSchemaDefinitionProperties{ + newIntSchemaDefinitionPropertyWithDefaults("origin_port", "", true, false, 80), + newStringSchemaDefinitionPropertyWithDefaults("protocol", "", true, false, "http"), + }, + } + nestedObjectDefault := map[string]interface{}{ + "origin_port": nestedObjectSchemaDefinition.Properties[0].Default, + "protocol": nestedObjectSchemaDefinition.Properties[1].Default, + } + nestedObject := newObjectSchemaDefinitionPropertyWithDefaults("nested_object", "", true, false, false, nestedObjectDefault, nestedObjectSchemaDefinition) + propertyWithNestedObjectSchemaDefinition := &specSchemaDefinition{ + Properties: specSchemaDefinitionProperties{ + idProperty, + nestedObject, + }, + } + dataValue := map[string]interface{}{ + "id": propertyWithNestedObjectSchemaDefinition.Properties[0].Default, + "nested_object": propertyWithNestedObjectSchemaDefinition.Properties[1].Default, + } + dataSourceSchema := newObjectSchemaDefinitionPropertyWithDefaults("nested-oobj", "", true, false, false, dataValue, propertyWithNestedObjectSchemaDefinition) + specSchemaNested := &specSchemaDefinition{ + Properties: specSchemaDefinitionProperties{dataSourceSchema}, + } + + s, err := specSchemaNested.createDataSourceSchema() + + assert.NotNil(t, s) + assert.NoError(t, err) + assert.Equal(t, false, s["nested_oobj"].Required) + nestedObjectElement := s["nested_oobj"].Elem.(*schema.Resource) + + assert.Equal(t, 2, len(nestedObjectElement.Schema)) + assert.Equal(t, false, nestedObjectElement.Schema["id"].Required) + +} + func TestCreateResourceSchema(t *testing.T) { Convey("Given a swagger schema definition that has few properties including the id all with terraform compliant names", t, func() { s := &specSchemaDefinition{ From e7f55ba6ce94758cc240b8e4ad2e8a5683673fff Mon Sep 17 00:00:00 2001 From: fradiben Date: Fri, 20 Sep 2019 16:36:10 +0100 Subject: [PATCH 05/37] [wip] all the nested object properties attributes are set to the correct values for the data source --- ...openapi_spec_resource_schema_definition.go | 20 +++- ...pi_spec_resource_schema_definition_test.go | 95 ++++++++++++------- 2 files changed, 76 insertions(+), 39 deletions(-) diff --git a/openapi/openapi_spec_resource_schema_definition.go b/openapi/openapi_spec_resource_schema_definition.go index 7821153b7..cef239aea 100644 --- a/openapi/openapi_spec_resource_schema_definition.go +++ b/openapi/openapi_spec_resource_schema_definition.go @@ -23,15 +23,27 @@ func (s *specSchemaDefinition) createDataSourceSchema() (map[string]*schema.Sche if err != nil { return nil, err } - // TODO: make sure the nested objects properties are also made computed to avoid surprises for propertyName := range terraformSchema { - terraformSchema[propertyName].Required = false - terraformSchema[propertyName].Optional = true - terraformSchema[propertyName].Computed = true + terraformSchema[propertyName] = setPropertyForDataSourceSchema(terraformSchema[propertyName]) } return terraformSchema, nil } +func setPropertyForDataSourceSchema(inputProperty *schema.Schema) (outputProperty *schema.Schema) { + outputProperty = inputProperty // the output is a clone of the input, do changes on the output var + outputProperty.Required = false + outputProperty.Optional = true + outputProperty.Computed = true + hasChildResources := inputProperty.Elem != nil && len(inputProperty.Elem.(*schema.Resource).Schema) > 0 + if hasChildResources { + childResources := outputProperty.Elem.(*schema.Resource).Schema + for _, childR := range childResources { + childR = setPropertyForDataSourceSchema(childR) + } + } + return outputProperty +} + func (s *specSchemaDefinition) createResourceSchemaKeepID() (map[string]*schema.Schema, error) { return s.createResourceSchemaIgnoreID(false) } diff --git a/openapi/openapi_spec_resource_schema_definition_test.go b/openapi/openapi_spec_resource_schema_definition_test.go index 518581249..54ead0a34 100644 --- a/openapi/openapi_spec_resource_schema_definition_test.go +++ b/openapi/openapi_spec_resource_schema_definition_test.go @@ -85,42 +85,67 @@ func TestCreateDataSourceSchema(t *testing.T) { } func TestCreateDataSourceSchema_ForNestedObjects(t *testing.T) { - nestedObjectSchemaDefinition := &specSchemaDefinition{ - Properties: specSchemaDefinitionProperties{ - newIntSchemaDefinitionPropertyWithDefaults("origin_port", "", true, false, 80), - newStringSchemaDefinitionPropertyWithDefaults("protocol", "", true, false, "http"), - }, - } - nestedObjectDefault := map[string]interface{}{ - "origin_port": nestedObjectSchemaDefinition.Properties[0].Default, - "protocol": nestedObjectSchemaDefinition.Properties[1].Default, - } - nestedObject := newObjectSchemaDefinitionPropertyWithDefaults("nested_object", "", true, false, false, nestedObjectDefault, nestedObjectSchemaDefinition) - propertyWithNestedObjectSchemaDefinition := &specSchemaDefinition{ - Properties: specSchemaDefinitionProperties{ - idProperty, - nestedObject, - }, - } - dataValue := map[string]interface{}{ - "id": propertyWithNestedObjectSchemaDefinition.Properties[0].Default, - "nested_object": propertyWithNestedObjectSchemaDefinition.Properties[1].Default, - } - dataSourceSchema := newObjectSchemaDefinitionPropertyWithDefaults("nested-oobj", "", true, false, false, dataValue, propertyWithNestedObjectSchemaDefinition) - specSchemaNested := &specSchemaDefinition{ - Properties: specSchemaDefinitionProperties{dataSourceSchema}, - } - - s, err := specSchemaNested.createDataSourceSchema() - - assert.NotNil(t, s) - assert.NoError(t, err) - assert.Equal(t, false, s["nested_oobj"].Required) - nestedObjectElement := s["nested_oobj"].Elem.(*schema.Resource) - - assert.Equal(t, 2, len(nestedObjectElement.Schema)) - assert.Equal(t, false, nestedObjectElement.Schema["id"].Required) + t.Run("happy path -- a data soruce can be derived from a nested object keeping all the properies attributes as expected", func(t *testing.T) { + // set up the schema for the nested object + nestedObjectSchemaDefinition := &specSchemaDefinition{ + Properties: specSchemaDefinitionProperties{ + newIntSchemaDefinitionPropertyWithDefaults("origin_port", "", true, false, 80), + newStringSchemaDefinitionPropertyWithDefaults("protocol", "", true, false, "http"), + }, + } + nestedObjectDefault := map[string]interface{}{ + "origin_port": nestedObjectSchemaDefinition.Properties[0].Default, + "protocol": nestedObjectSchemaDefinition.Properties[1].Default, + } + nestedObject := newObjectSchemaDefinitionPropertyWithDefaults("nested_object", "", true, false, false, nestedObjectDefault, nestedObjectSchemaDefinition) + propertyWithNestedObjectSchemaDefinition := &specSchemaDefinition{ + Properties: specSchemaDefinitionProperties{ + idProperty, + nestedObject, + }, + } + dataValue := map[string]interface{}{ + "id": propertyWithNestedObjectSchemaDefinition.Properties[0].Default, + "nested_object": propertyWithNestedObjectSchemaDefinition.Properties[1].Default, + } + dataSourceSchema := newObjectSchemaDefinitionPropertyWithDefaults("nested-oobj", "", true, false, false, dataValue, propertyWithNestedObjectSchemaDefinition) + specSchemaNested := &specSchemaDefinition{ + Properties: specSchemaDefinitionProperties{dataSourceSchema}, + } + // get the Terraform schema which represents a Data Source + s, err := specSchemaNested.createDataSourceSchema() + + assert.NotNil(t, s) + assert.NoError(t, err) + + // assertions on the properties attributes + assert.Equal(t, false, s["nested_oobj"].Required) + assert.Equal(t, true, s["nested_oobj"].Optional) + assert.Equal(t, true, s["nested_oobj"].Computed) + + // 1^ level + objectResource := s["nested_oobj"].Elem.(*schema.Resource) + assert.Equal(t, 2, len(objectResource.Schema)) + + assert.Equal(t, false, objectResource.Schema["id"].Required) + assert.Equal(t, true, objectResource.Schema["id"].Optional) + assert.Equal(t, true, objectResource.Schema["id"].Computed) + assert.Equal(t, false, objectResource.Schema["nested_object"].Required) + assert.Equal(t, true, objectResource.Schema["nested_object"].Optional) + assert.Equal(t, true, objectResource.Schema["nested_object"].Computed) + + // 2^ level + nestedObjectResource := objectResource.Schema["nested_object"].Elem.(*schema.Resource) + assert.Equal(t, 2, len(nestedObjectResource.Schema)) + + assert.Equal(t, false, nestedObjectResource.Schema["origin_port"].Required) + assert.Equal(t, true, nestedObjectResource.Schema["origin_port"].Optional) + assert.Equal(t, true, nestedObjectResource.Schema["origin_port"].Computed) + assert.Equal(t, false, nestedObjectResource.Schema["protocol"].Required) + assert.Equal(t, true, nestedObjectResource.Schema["protocol"].Optional) + assert.Equal(t, true, nestedObjectResource.Schema["protocol"].Computed) + }) } func TestCreateResourceSchema(t *testing.T) { From 70a09659a70f81f5435a0bcb64b8b23ba200d8d7 Mon Sep 17 00:00:00 2001 From: fradiben Date: Fri, 20 Sep 2019 17:06:22 +0100 Subject: [PATCH 06/37] fixed case in which the datasource contains Elems which are primitive --- ...openapi_spec_resource_schema_definition.go | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/openapi/openapi_spec_resource_schema_definition.go b/openapi/openapi_spec_resource_schema_definition.go index cef239aea..7f60c2446 100644 --- a/openapi/openapi_spec_resource_schema_definition.go +++ b/openapi/openapi_spec_resource_schema_definition.go @@ -34,11 +34,20 @@ func setPropertyForDataSourceSchema(inputProperty *schema.Schema) (outputPropert outputProperty.Required = false outputProperty.Optional = true outputProperty.Computed = true - hasChildResources := inputProperty.Elem != nil && len(inputProperty.Elem.(*schema.Resource).Schema) > 0 - if hasChildResources { - childResources := outputProperty.Elem.(*schema.Resource).Schema - for _, childR := range childResources { - childR = setPropertyForDataSourceSchema(childR) + + isResource := false + switch inputProperty.Elem.(type) { + case *schema.Resource: + isResource = true + } + // only apply the recursive call if Elem is a Resource (hence a complex item) + if isResource { + hasChildResources := inputProperty.Elem != nil && len(inputProperty.Elem.(*schema.Resource).Schema) > 0 + if hasChildResources { + childResources := outputProperty.Elem.(*schema.Resource).Schema + for _, childR := range childResources { + childR = setPropertyForDataSourceSchema(childR) + } } } return outputProperty From 62906c9070848d84dd30bc4d4c6822fc10d86807 Mon Sep 17 00:00:00 2001 From: fradiben Date: Fri, 20 Sep 2019 17:29:31 +0100 Subject: [PATCH 07/37] [wip] applying a filter on a nested struct while reading from a DataSource --- openapi/data_source_factory_test.go | 153 +++++++++++++--------------- 1 file changed, 71 insertions(+), 82 deletions(-) diff --git a/openapi/data_source_factory_test.go b/openapi/data_source_factory_test.go index b06b13621..b0d6c8399 100644 --- a/openapi/data_source_factory_test.go +++ b/openapi/data_source_factory_test.go @@ -2,7 +2,6 @@ package openapi import ( "errors" - "fmt" "testing" "github.com/stretchr/testify/require" @@ -135,7 +134,7 @@ func TestDataSourceRead(t *testing.T) { }{ // TODO: add a test to cover sub-resource use case too (need to cover this before releasing data source support) { - name: "fetch selected data source as per filter configuration (label=someLabel) when the filter is into a nested object", + name: "fetch selected data source as per filter configuration (label=someLabel)", filtersInput: []map[string]interface{}{ newFilter("label", []string{"someLabel"}), }, @@ -153,82 +152,44 @@ func TestDataSourceRead(t *testing.T) { }, expectedError: nil, }, - //{ - // name: "fetch selected data source as per filter configuration (label=someLabel) when the filter a top level property of a nested object", - // filtersInput: []map[string]interface{}{ - // newFilter("label", []string{"someLabel"}), - // }, - // responsePayload: []map[string]interface{}{ - // { - // "id": "someID", - // "label": "someLabel", - // "owners": []string{"someOwner"}, - // }, - // { - // "id": "someOtherID", - // "label": "someOtherLabel", - // "owners": []string{}, - // }, - // }, - // expectedError: nil, - //}, - //{ - // name: "fetch selected data source as per filter configuration (label=someLabel)", - // filtersInput: []map[string]interface{}{ - // newFilter("label", []string{"someLabel"}), - // }, - // responsePayload: []map[string]interface{}{ - // { - // "id": "someID", - // "label": "someLabel", - // "owners": []string{"someOwner"}, - // }, - // { - // "id": "someOtherID", - // "label": "someOtherLabel", - // "owners": []string{}, - // }, - // }, - // expectedError: nil, - //}, - //{ - // name: "no filter match", - // filtersInput: []map[string]interface{}{ - // newFilter("label", []string{"some non existing label"}), - // }, - // responsePayload: []map[string]interface{}{ - // { - // "id": "someID", - // "label": "someLabel", - // }, - // }, - // expectedError: errors.New("your query returned no results. Please change your search criteria and try again"), - //}, - //{ - // name: "after filtering the result contains more than one element", - // filtersInput: []map[string]interface{}{ - // newFilter("label", []string{"my_label"}), - // }, - // responsePayload: []map[string]interface{}{ - // { - // "id": "someID", - // "label": "my_label", - // }, - // { - // "id": "someOtherID", - // "label": "my_label", - // }, - // }, - // expectedError: errors.New("your query returned contains more than one result. Please change your search criteria to make it more specific"), - //}, - //{ - // name: "validate input fails", - // filtersInput: []map[string]interface{}{ - // newFilter("non_existing_property", []string{"my_label"}), - // }, - // responsePayload: []map[string]interface{}{}, - // expectedError: errors.New("filter name does not match any of the schema properties: property with name 'non_existing_property' not existing in resource schema definition"), - //}, + { + name: "no filter match", + filtersInput: []map[string]interface{}{ + newFilter("label", []string{"some non existing label"}), + }, + responsePayload: []map[string]interface{}{ + { + "id": "someID", + "label": "someLabel", + }, + }, + expectedError: errors.New("your query returned no results. Please change your search criteria and try again"), + }, + { + name: "after filtering the result contains more than one element", + filtersInput: []map[string]interface{}{ + newFilter("label", []string{"my_label"}), + }, + responsePayload: []map[string]interface{}{ + { + "id": "someID", + "label": "my_label", + }, + { + "id": "someOtherID", + "label": "my_label", + }, + }, + expectedError: errors.New("your query returned contains more than one result. Please change your search criteria to make it more specific"), + }, + { + name: "validate input fails", + filtersInput: []map[string]interface{}{ + newFilter("non_existing_property", []string{"my_label"}), + }, + responsePayload: []map[string]interface{}{}, + expectedError: errors.New("filter name does not match any of the schema properties: property with name 'non_existing_property' not existing in resource schema definition"), + }, } for _, tc := range testCases { @@ -263,6 +224,8 @@ func TestDataSourceRead(t *testing.T) { } func TestDataSourceRead_ForNestedObjects(t *testing.T) { + // Given ... + // ... a schema describing a nested object which is used to ... nestedObjectSchemaDefinition := &specSchemaDefinition{ Properties: specSchemaDefinitionProperties{ newIntSchemaDefinitionPropertyWithDefaults("origin_port", "", true, false, 80), @@ -287,7 +250,7 @@ func TestDataSourceRead_ForNestedObjects(t *testing.T) { dataSourceSchema := newObjectSchemaDefinitionPropertyWithDefaults("nested-oobj", "", true, false, false, dataValue, propertyWithNestedObjectSchemaDefinition) - // Given + // ... build a data source (using a dataSourceFactory) dataSourceFactory := dataSourceFactory{ openAPIResource: &specStubResource{ schemaDefinition: &specSchemaDefinition{ @@ -295,10 +258,36 @@ func TestDataSourceRead_ForNestedObjects(t *testing.T) { }, }, } + dataSourceTFSchema, err := dataSourceFactory.createTerraformDataSourceSchema() + require.NotNil(t, dataSourceTFSchema) + require.NoError(t, err) - fmt.Println(dataSourceFactory) //todo work on this - fs, err := dataSourceFactory.createTerraformDataSourceSchema() - fmt.Println(err, fs) + filtersInput := map[string]interface{}{ + dataSourceFilterPropertyName: []map[string]interface{}{ + newFilter("origin_port", []string{"3900"}), + }, + } + resourceData := schema.TestResourceDataRaw(t, dataSourceTFSchema, filtersInput) + client := &clientOpenAPIStub{ + responseListPayload: []map[string]interface{}{ + { + "id": "someID", + "label": "my_label", + }, + { + "id": "someOtherID", + "label": "my_label", + }, + }, + } + // When + err = dataSourceFactory.read(resourceData, client) + // Then + assert.Nil(t, err) + // assert that the filtered data source contains the same values as the ones returned by the API + assert.Equal(t, 8, len(resourceData.State().Attributes)) //this asserts that ONLY 1 element is returned when the filter is applied (2 prop of the elelemnt + 4 prop given by the filter) + assert.Equal(t, client.responseListPayload[0]["id"], resourceData.Id()) //resourceData.Id() is being called instead of resourceData.Get("id") because id property is a special one kept by Terraform + assert.Equal(t, client.responseListPayload[0]["label"], resourceData.Get("label")) } From c524b8234a6b078371eaf1305e58cc82fd5c80c8 Mon Sep 17 00:00:00 2001 From: dikhan Date: Fri, 20 Sep 2019 10:23:08 -0700 Subject: [PATCH 08/37] create helper function to assert data source schema configs --- ...pi_spec_resource_schema_definition_test.go | 37 ++++++++----------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/openapi/openapi_spec_resource_schema_definition_test.go b/openapi/openapi_spec_resource_schema_definition_test.go index 54ead0a34..3e3a1143c 100644 --- a/openapi/openapi_spec_resource_schema_definition_test.go +++ b/openapi/openapi_spec_resource_schema_definition_test.go @@ -70,13 +70,9 @@ func TestCreateDataSourceSchema(t *testing.T) { s, err := tc.specSchemaDef.createDataSourceSchema() if tc.expectedError == nil { assert.Nil(t, err, tc.name) - for expectedTerraformPropName, tfSchemaProp := range tc.expectedResult { - assert.Contains(t, s, expectedTerraformPropName, tc.name) + for expectedTerraformPropName, expectedTerraformSchemaProp := range tc.expectedResult { assert.Nil(t, s["id"]) - assert.Equal(t, tfSchemaProp.Type, s[expectedTerraformPropName].Type, tc.name) - assert.Equal(t, tfSchemaProp.Optional, s[expectedTerraformPropName].Optional, tc.name) - assert.Equal(t, tfSchemaProp.Required, s[expectedTerraformPropName].Required, tc.name) - assert.Equal(t, tfSchemaProp.Computed, s[expectedTerraformPropName].Computed, tc.name) + assertDataSourceSchemaProperty(t, s[expectedTerraformPropName], expectedTerraformSchemaProp.Type, tc.name) } } else { assert.Equal(t, tc.expectedError.Error(), err.Error(), tc.name) @@ -84,6 +80,14 @@ func TestCreateDataSourceSchema(t *testing.T) { } } +func assertDataSourceSchemaProperty(t *testing.T, actual *schema.Schema, expectedType schema.ValueType, msgAndArgs ...interface{}) { + assert.NotNil(t, actual, msgAndArgs) + assert.Equal(t, expectedType, actual.Type, msgAndArgs) + assert.False(t, actual.Required, msgAndArgs) + assert.True(t, actual.Optional, msgAndArgs) + assert.True(t, actual.Computed, msgAndArgs) +} + func TestCreateDataSourceSchema_ForNestedObjects(t *testing.T) { t.Run("happy path -- a data soruce can be derived from a nested object keeping all the properies attributes as expected", func(t *testing.T) { // set up the schema for the nested object @@ -120,31 +124,20 @@ func TestCreateDataSourceSchema_ForNestedObjects(t *testing.T) { assert.NoError(t, err) // assertions on the properties attributes - assert.Equal(t, false, s["nested_oobj"].Required) - assert.Equal(t, true, s["nested_oobj"].Optional) - assert.Equal(t, true, s["nested_oobj"].Computed) + assertDataSourceSchemaProperty(t, s["nested_oobj"], schema.TypeList) // 1^ level objectResource := s["nested_oobj"].Elem.(*schema.Resource) assert.Equal(t, 2, len(objectResource.Schema)) - assert.Equal(t, false, objectResource.Schema["id"].Required) - assert.Equal(t, true, objectResource.Schema["id"].Optional) - assert.Equal(t, true, objectResource.Schema["id"].Computed) - assert.Equal(t, false, objectResource.Schema["nested_object"].Required) - assert.Equal(t, true, objectResource.Schema["nested_object"].Optional) - assert.Equal(t, true, objectResource.Schema["nested_object"].Computed) + assertDataSourceSchemaProperty(t, objectResource.Schema["id"], schema.TypeString) + assertDataSourceSchemaProperty(t, objectResource.Schema["nested_object"], schema.TypeMap) // 2^ level nestedObjectResource := objectResource.Schema["nested_object"].Elem.(*schema.Resource) assert.Equal(t, 2, len(nestedObjectResource.Schema)) - - assert.Equal(t, false, nestedObjectResource.Schema["origin_port"].Required) - assert.Equal(t, true, nestedObjectResource.Schema["origin_port"].Optional) - assert.Equal(t, true, nestedObjectResource.Schema["origin_port"].Computed) - assert.Equal(t, false, nestedObjectResource.Schema["protocol"].Required) - assert.Equal(t, true, nestedObjectResource.Schema["protocol"].Optional) - assert.Equal(t, true, nestedObjectResource.Schema["protocol"].Computed) + assertDataSourceSchemaProperty(t, nestedObjectResource.Schema["origin_port"], schema.TypeInt) + assertDataSourceSchemaProperty(t, nestedObjectResource.Schema["protocol"], schema.TypeString) }) } From 427e2afdd0bae1bf5683862018a453cffa4f08cc Mon Sep 17 00:00:00 2001 From: dikhan Date: Fri, 20 Sep 2019 10:30:35 -0700 Subject: [PATCH 09/37] some more dry up --- ...openapi_spec_resource_schema_definition_test.go | 8 -------- openapi/provider_test.go | 14 ++------------ openapi/test_utils.go | 9 +++++++++ 3 files changed, 11 insertions(+), 20 deletions(-) diff --git a/openapi/openapi_spec_resource_schema_definition_test.go b/openapi/openapi_spec_resource_schema_definition_test.go index 3e3a1143c..38551c73b 100644 --- a/openapi/openapi_spec_resource_schema_definition_test.go +++ b/openapi/openapi_spec_resource_schema_definition_test.go @@ -80,14 +80,6 @@ func TestCreateDataSourceSchema(t *testing.T) { } } -func assertDataSourceSchemaProperty(t *testing.T, actual *schema.Schema, expectedType schema.ValueType, msgAndArgs ...interface{}) { - assert.NotNil(t, actual, msgAndArgs) - assert.Equal(t, expectedType, actual.Type, msgAndArgs) - assert.False(t, actual.Required, msgAndArgs) - assert.True(t, actual.Optional, msgAndArgs) - assert.True(t, actual.Computed, msgAndArgs) -} - func TestCreateDataSourceSchema_ForNestedObjects(t *testing.T) { t.Run("happy path -- a data soruce can be derived from a nested object keeping all the properies attributes as expected", func(t *testing.T) { // set up the schema for the nested object diff --git a/openapi/provider_test.go b/openapi/provider_test.go index ffcd7eec6..14b89cfd9 100644 --- a/openapi/provider_test.go +++ b/openapi/provider_test.go @@ -267,18 +267,8 @@ definitions: resourceName := fmt.Sprintf("%s_cdn_datasource_v1", providerName) So(tfProvider.DataSourcesMap, ShouldContainKey, resourceName) - // TODO: dry out the assertions - So(tfProvider.DataSourcesMap[resourceName].Schema, ShouldContainKey, "label") - So(tfProvider.DataSourcesMap[resourceName].Schema["label"].Type, ShouldEqual, schema.TypeString) - So(tfProvider.DataSourcesMap[resourceName].Schema["label"].Required, ShouldBeFalse) - So(tfProvider.DataSourcesMap[resourceName].Schema["label"].Optional, ShouldBeTrue) - So(tfProvider.DataSourcesMap[resourceName].Schema["label"].Computed, ShouldBeTrue) - - So(tfProvider.DataSourcesMap[resourceName].Schema, ShouldContainKey, "owners") - So(tfProvider.DataSourcesMap[resourceName].Schema["owners"].Type, ShouldEqual, schema.TypeList) - So(tfProvider.DataSourcesMap[resourceName].Schema["owners"].Required, ShouldBeFalse) - So(tfProvider.DataSourcesMap[resourceName].Schema["owners"].Optional, ShouldBeTrue) - So(tfProvider.DataSourcesMap[resourceName].Schema["owners"].Computed, ShouldBeTrue) + assertDataSourceSchemaProperty(t, tfProvider.DataSourcesMap[resourceName].Schema["label"], schema.TypeString) + assertDataSourceSchemaProperty(t, tfProvider.DataSourcesMap[resourceName].Schema["owners"], schema.TypeList) So(tfProvider.DataSourcesMap[resourceName].Schema, ShouldContainKey, "filter") So(tfProvider.DataSourcesMap[resourceName].Schema["filter"].Type, ShouldEqual, schema.TypeSet) diff --git a/openapi/test_utils.go b/openapi/test_utils.go index 67c5180af..860f523a5 100644 --- a/openapi/test_utils.go +++ b/openapi/test_utils.go @@ -3,6 +3,7 @@ package openapi import ( "encoding/json" "github.com/hashicorp/terraform/helper/schema" + "github.com/stretchr/testify/assert" "io/ioutil" "log" "os" @@ -161,3 +162,11 @@ func initAPISpecFile(swaggerContent string) *os.File { } return file } + +func assertDataSourceSchemaProperty(t *testing.T, actual *schema.Schema, expectedType schema.ValueType, msgAndArgs ...interface{}) { + assert.NotNil(t, actual, msgAndArgs) + assert.Equal(t, expectedType, actual.Type, msgAndArgs) + assert.False(t, actual.Required, msgAndArgs) + assert.True(t, actual.Optional, msgAndArgs) + assert.True(t, actual.Computed, msgAndArgs) +} From 2750c71fbcce59676e45958dad486ae8a1b0f801 Mon Sep 17 00:00:00 2001 From: dikhan Date: Fri, 20 Sep 2019 11:08:07 -0700 Subject: [PATCH 10/37] update test to represent the schema input configuration --- openapi/data_source_factory_test.go | 34 ++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/openapi/data_source_factory_test.go b/openapi/data_source_factory_test.go index b0d6c8399..b256a4129 100644 --- a/openapi/data_source_factory_test.go +++ b/openapi/data_source_factory_test.go @@ -248,13 +248,16 @@ func TestDataSourceRead_ForNestedObjects(t *testing.T) { "nested_object": propertyWithNestedObjectSchemaDefinition.Properties[1].Default, } - dataSourceSchema := newObjectSchemaDefinitionPropertyWithDefaults("nested-oobj", "", true, false, false, dataValue, propertyWithNestedObjectSchemaDefinition) + objectProperty := newObjectSchemaDefinitionPropertyWithDefaults("nested-oobj", "", true, false, false, dataValue, propertyWithNestedObjectSchemaDefinition) // ... build a data source (using a dataSourceFactory) dataSourceFactory := dataSourceFactory{ openAPIResource: &specStubResource{ schemaDefinition: &specSchemaDefinition{ - Properties: specSchemaDefinitionProperties{dataSourceSchema}, + Properties: specSchemaDefinitionProperties{ + idProperty, + objectProperty, + }, }, }, } @@ -264,19 +267,31 @@ func TestDataSourceRead_ForNestedObjects(t *testing.T) { filtersInput := map[string]interface{}{ dataSourceFilterPropertyName: []map[string]interface{}{ - newFilter("origin_port", []string{"3900"}), + newFilter("id", []string{"someID"}), }, } resourceData := schema.TestResourceDataRaw(t, dataSourceTFSchema, filtersInput) client := &clientOpenAPIStub{ responseListPayload: []map[string]interface{}{ { - "id": "someID", - "label": "my_label", + "id": "someID", + "nested-oobj": map[string]interface{}{ + "id": "uuid", + "nested_object": map[string]interface{}{ + "origin_port": 80, + "protocol": "http", + }, + }, }, { - "id": "someOtherID", - "label": "my_label", + "id": "someOtherID", + "nested-oobj": map[string]interface{}{ + "id": "other-uuid", + "nested_object": map[string]interface{}{ + "origin_port": 443, + "protocol": "https", + }, + }, }, }, } @@ -285,10 +300,9 @@ func TestDataSourceRead_ForNestedObjects(t *testing.T) { // Then assert.Nil(t, err) // assert that the filtered data source contains the same values as the ones returned by the API - assert.Equal(t, 8, len(resourceData.State().Attributes)) //this asserts that ONLY 1 element is returned when the filter is applied (2 prop of the elelemnt + 4 prop given by the filter) + assert.Equal(t, 10, len(resourceData.State().Attributes)) //this asserts that ONLY 1 element is returned when the filter is applied (2 prop of the elelemnt + 4 prop given by the filter) assert.Equal(t, client.responseListPayload[0]["id"], resourceData.Id()) //resourceData.Id() is being called instead of resourceData.Get("id") because id property is a special one kept by Terraform - assert.Equal(t, client.responseListPayload[0]["label"], resourceData.Get("label")) - + assert.Equal(t, client.responseListPayload[0]["label"], resourceData.Get("nested_object")) } func TestDataSourceRead_Fails_Because_Cannot_extract_ParentsID(t *testing.T) { From 96c7d942dba96e0251f9affedc12bb8467d456a6 Mon Sep 17 00:00:00 2001 From: dikhan Date: Fri, 20 Sep 2019 11:22:52 -0700 Subject: [PATCH 11/37] do not override parent id property config if found --- openapi/openapi_spec_resource_schema_definition.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/openapi/openapi_spec_resource_schema_definition.go b/openapi/openapi_spec_resource_schema_definition.go index 7f60c2446..d5cc6b7ca 100644 --- a/openapi/openapi_spec_resource_schema_definition.go +++ b/openapi/openapi_spec_resource_schema_definition.go @@ -24,12 +24,20 @@ func (s *specSchemaDefinition) createDataSourceSchema() (map[string]*schema.Sche return nil, err } for propertyName := range terraformSchema { - terraformSchema[propertyName] = setPropertyForDataSourceSchema(terraformSchema[propertyName]) + // TODO: to be tested + p, err := s.getProperty(propertyName) + if err != nil { + return nil, err + } + if !p.IsParentProperty { + terraformSchema[propertyName] = setPropertyForDataSourceSchema(terraformSchema[propertyName]) + } } return terraformSchema, nil } func setPropertyForDataSourceSchema(inputProperty *schema.Schema) (outputProperty *schema.Schema) { + outputProperty = inputProperty // the output is a clone of the input, do changes on the output var outputProperty.Required = false outputProperty.Optional = true From f53c1265b6024a44bd6a997a6a52c3930e9604dc Mon Sep 17 00:00:00 2001 From: dikhan Date: Fri, 20 Sep 2019 11:25:32 -0700 Subject: [PATCH 12/37] use get property based on terraform name --- openapi/openapi_spec_resource_schema_definition.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openapi/openapi_spec_resource_schema_definition.go b/openapi/openapi_spec_resource_schema_definition.go index d5cc6b7ca..72e2cad75 100644 --- a/openapi/openapi_spec_resource_schema_definition.go +++ b/openapi/openapi_spec_resource_schema_definition.go @@ -25,7 +25,7 @@ func (s *specSchemaDefinition) createDataSourceSchema() (map[string]*schema.Sche } for propertyName := range terraformSchema { // TODO: to be tested - p, err := s.getProperty(propertyName) + p, err := s.getPropertyBasedOnTerraformName(propertyName) if err != nil { return nil, err } From 0a1ff7e3d2eea079d51be5e15388eaf7d3af7cd8 Mon Sep 17 00:00:00 2001 From: dikhan Date: Fri, 20 Sep 2019 12:02:07 -0700 Subject: [PATCH 13/37] add test TestCreateDataSourceSchema_Subresources - this test covers and documents that parent id properties are not overriden in terms of the configuration. --- ...openapi_spec_resource_schema_definition.go | 1 - ...pi_spec_resource_schema_definition_test.go | 34 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/openapi/openapi_spec_resource_schema_definition.go b/openapi/openapi_spec_resource_schema_definition.go index 72e2cad75..6d2ecf746 100644 --- a/openapi/openapi_spec_resource_schema_definition.go +++ b/openapi/openapi_spec_resource_schema_definition.go @@ -24,7 +24,6 @@ func (s *specSchemaDefinition) createDataSourceSchema() (map[string]*schema.Sche return nil, err } for propertyName := range terraformSchema { - // TODO: to be tested p, err := s.getPropertyBasedOnTerraformName(propertyName) if err != nil { return nil, err diff --git a/openapi/openapi_spec_resource_schema_definition_test.go b/openapi/openapi_spec_resource_schema_definition_test.go index 38551c73b..ddfe2c42b 100644 --- a/openapi/openapi_spec_resource_schema_definition_test.go +++ b/openapi/openapi_spec_resource_schema_definition_test.go @@ -80,6 +80,40 @@ func TestCreateDataSourceSchema(t *testing.T) { } } +func TestCreateDataSourceSchema_Subresources(t *testing.T) { + t.Run("happy path -- data source schema dor sub-resoruce contains all schema properties configured as computed but parent properties", func(t *testing.T) { + + specSchemaDef := specSchemaDefinition{ + Properties: specSchemaDefinitionProperties{ + &specSchemaDefinitionProperty{ + Name: "id", + Type: typeString, + ReadOnly: false, + Required: true, + }, + &specSchemaDefinitionProperty{ + Name: "parent_id", + Type: typeString, + ReadOnly: false, + Required: true, + IsParentProperty: true, + }, + }, + } + + // get the Terraform schema which represents a Data Source + s, err := specSchemaDef.createDataSourceSchema() + + assert.NotNil(t, s) + assert.NoError(t, err) + + assert.Equal(t, schema.TypeString, s["parent_id"].Type) + assert.True(t, s["parent_id"].Required) + assert.False(t, s["parent_id"].Optional) + assert.False(t, s["parent_id"].Computed) + }) +} + func TestCreateDataSourceSchema_ForNestedObjects(t *testing.T) { t.Run("happy path -- a data soruce can be derived from a nested object keeping all the properies attributes as expected", func(t *testing.T) { // set up the schema for the nested object From a4264f7bc25b00c3508a72ca8be671a8dd6550ac Mon Sep 17 00:00:00 2001 From: dikhan Date: Fri, 20 Sep 2019 12:51:13 -0700 Subject: [PATCH 14/37] add TestDataSourceRead_Subresource --- openapi/data_source_factory.go | 1 - openapi/data_source_factory_test.go | 44 +++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/openapi/data_source_factory.go b/openapi/data_source_factory.go index 5ecddd277..fd638db9f 100644 --- a/openapi/data_source_factory.go +++ b/openapi/data_source_factory.go @@ -112,7 +112,6 @@ func (d dataSourceFactory) read(data *schema.ResourceData, i interface{}) error err = setStateID(d.openAPIResource, data, filteredResults[0]) if err != nil { - fmt.Println(err) return err } diff --git a/openapi/data_source_factory_test.go b/openapi/data_source_factory_test.go index b256a4129..3680f4d8a 100644 --- a/openapi/data_source_factory_test.go +++ b/openapi/data_source_factory_test.go @@ -223,6 +223,50 @@ func TestDataSourceRead(t *testing.T) { } } +func TestDataSourceRead_Subresource(t *testing.T) { + + dataSourceFactory := dataSourceFactory{ + openAPIResource: &specStubResource{ + path: "/v1/cdns/{id}/firewall", + schemaDefinition: &specSchemaDefinition{ + Properties: specSchemaDefinitionProperties{ + newStringSchemaDefinitionPropertyWithDefaults("id", "", false, true, nil), + newStringSchemaDefinitionPropertyWithDefaults("label", "", false, true, nil), + newStringSchemaDefinitionPropertyWithDefaults("cdns_v1_id", "", false, true, nil), // This simulates an openAPIResource that is subresource and the schema has already been populated with the parent property + }, + }, + fullParentResourceName: "cdns_v1", + parentResourceNames: []string{"cdns_v1"}, + parentPropertyNames: []string{"cdns_v1_id"}, + }, + } + + resourceSchema, err := dataSourceFactory.createTerraformDataSourceSchema() + require.NoError(t, err) + + filtersInput := map[string]interface{}{ + "cdns_v1_id": "parentPropertyID", // Since the path is a sub-resource, the user is expected to provide the id of the parent + dataSourceFilterPropertyName: []map[string]interface{}{ + newFilter("label", []string{"my_label"}), + }, + } + resourceData := schema.TestResourceDataRaw(t, resourceSchema, filtersInput) + + client := &clientOpenAPIStub{ + responseListPayload: []map[string]interface{}{ + { + "id": "someID", + "label": "my_label", + }, + }, + } + err = dataSourceFactory.read(resourceData, client) + require.NoError(t, err) + assert.Equal(t, []string{"parentPropertyID"}, client.parentIDsReceived) // check that the parent id is passed as expected + assert.Equal(t, "someID", resourceData.Id()) + assert.Equal(t, "my_label", resourceData.Get("label")) +} + func TestDataSourceRead_ForNestedObjects(t *testing.T) { // Given ... // ... a schema describing a nested object which is used to ... From ffca75e7919e61a95fee8d18e5d5c01f798f0fe5 Mon Sep 17 00:00:00 2001 From: lchan Date: Fri, 20 Sep 2019 14:38:21 -0700 Subject: [PATCH 15/37] Expand TestOpenAPIProvider to check more data types - Add int, bool, float, and object properties to Swagger input - Check that data source schema includes all properties and they are computed - Check that data source doesn't have create and delete operations --- openapi/provider_test.go | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/openapi/provider_test.go b/openapi/provider_test.go index 14b89cfd9..0b2291b77 100644 --- a/openapi/provider_test.go +++ b/openapi/provider_test.go @@ -219,23 +219,33 @@ securityDefinitions: definitions: ContentDeliveryNetworkV1Collection: - type: "array" + type: array items: $ref: "#/definitions/ContentDeliveryNetworkV1" ContentDeliveryNetworkV1: - type: "object" + type: object properties: id: - type: "string" + type: string readOnly: true label: - type: "string" + type: string owners: type: array items: - type: string` - - // TODO: expand model definition to include other primitives that user can filter on AND also more complex property and ensure they are congfigured property in the schema as computed + type: string + int_property: + type: integer + bool_property: + type: boolean + float_property: + type: number + format: float + obj_property: + type: object + properties: + name: + type: string` swaggerServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Write([]byte(swaggerContent)) @@ -269,6 +279,10 @@ definitions: assertDataSourceSchemaProperty(t, tfProvider.DataSourcesMap[resourceName].Schema["label"], schema.TypeString) assertDataSourceSchemaProperty(t, tfProvider.DataSourcesMap[resourceName].Schema["owners"], schema.TypeList) + assertDataSourceSchemaProperty(t, tfProvider.DataSourcesMap[resourceName].Schema["int_property"], schema.TypeInt) + assertDataSourceSchemaProperty(t, tfProvider.DataSourcesMap[resourceName].Schema["bool_property"], schema.TypeBool) + assertDataSourceSchemaProperty(t, tfProvider.DataSourcesMap[resourceName].Schema["float_property"], schema.TypeFloat) + assertDataSourceSchemaProperty(t, tfProvider.DataSourcesMap[resourceName].Schema["obj_property"], schema.TypeMap) So(tfProvider.DataSourcesMap[resourceName].Schema, ShouldContainKey, "filter") So(tfProvider.DataSourcesMap[resourceName].Schema["filter"].Type, ShouldEqual, schema.TypeSet) @@ -282,6 +296,8 @@ definitions: }) Convey("the provider cdn-datasource data source should have only the READ operation configured", func() { So(tfProvider.DataSourcesMap[resourceName].Read, ShouldNotBeNil) + So(tfProvider.DataSourcesMap[resourceName].Create, ShouldBeNil) + So(tfProvider.DataSourcesMap[resourceName].Delete, ShouldBeNil) }) Convey("and the provider resource map must be nil as no resources are configured in the swagger", func() { From 61e6129c0f5e64277e24f37f6f768d6f3ed837c0 Mon Sep 17 00:00:00 2001 From: dikhan Date: Fri, 20 Sep 2019 14:45:00 -0700 Subject: [PATCH 16/37] add fat test for subresource data sources - dry out some dup code --- openapi/provider_test.go | 120 ++++++++++++++++++++++++++++++--------- openapi/test_utils.go | 20 ++++++- 2 files changed, 110 insertions(+), 30 deletions(-) diff --git a/openapi/provider_test.go b/openapi/provider_test.go index 14b89cfd9..cf7b7991f 100644 --- a/openapi/provider_test.go +++ b/openapi/provider_test.go @@ -295,6 +295,92 @@ definitions: }) }) + Convey("Given a local server that exposes a swagger file containing a terraform compatible data source that has a subresource path", t, func() { + swaggerContent := `swagger: "2.0" +host: "localhost:8443" +basePath: "/api" + +paths: + /v1/cdns/{id}/firewalls: + get: + responses: + 200: + schema: + $ref: "#/definitions/ContentDeliveryNetworkV1Collection" + +definitions: + ContentDeliveryNetworkV1Collection: + type: "array" + items: + $ref: "#/definitions/ContentDeliveryNetworkV1" + ContentDeliveryNetworkV1: + type: "object" + properties: + id: + type: "string" + readOnly: true + label: + type: "string"` + + swaggerServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte(swaggerContent)) + })) + + Convey("When CreateSchemaProviderWithConfiguration method is called", func() { + providerName := "openapi" + p := ProviderOpenAPI{ProviderName: providerName} + tfProvider, err := p.CreateSchemaProviderFromServiceConfiguration(&ServiceConfigStub{SwaggerURL: swaggerServer.URL}) + + Convey("Then the error should be nil", func() { + So(err, ShouldBeNil) + }) + Convey("And the provider returned should be configured as expected", func() { + So(tfProvider, ShouldNotBeNil) + Convey("the provider schema should be the expected one", func() { + So(tfProvider.Schema, ShouldNotBeNil) + }) + Convey("the provider dataSource map should contain the cdn resource with the expected configuration", func() { + So(tfProvider.DataSourcesMap, ShouldNotBeNil) + So(len(tfProvider.DataSourcesMap), ShouldEqual, 1) + + dataSourceName := fmt.Sprintf("%s_cdns_v1_firewalls", providerName) + So(tfProvider.DataSourcesMap, ShouldContainKey, dataSourceName) + Convey("the provider cdn resource should have the expected schema", func() { + So(tfProvider.DataSourcesMap, ShouldContainKey, dataSourceName) + + // check parent id is part of the schema + + assertTerraformSchemaProperty(t, tfProvider.DataSourcesMap[dataSourceName].Schema["cdns_v1_id"], schema.TypeString, true, false) + + // check actual model properties are part of the schema + assertDataSourceSchemaProperty(t, tfProvider.DataSourcesMap[dataSourceName].Schema["label"], schema.TypeString) + + // check filter property is in the schema + So(tfProvider.DataSourcesMap[dataSourceName].Schema, ShouldContainKey, "filter") + So(tfProvider.DataSourcesMap[dataSourceName].Schema["filter"].Type, ShouldEqual, schema.TypeSet) + So(tfProvider.DataSourcesMap[dataSourceName].Schema["filter"].Required, ShouldBeFalse) + So(tfProvider.DataSourcesMap[dataSourceName].Schema["filter"].Optional, ShouldBeTrue) + So(tfProvider.DataSourcesMap[dataSourceName].Schema["filter"].Computed, ShouldBeFalse) + + elements := tfProvider.DataSourcesMap[dataSourceName].Schema["filter"].Elem.(*schema.Resource).Schema + So(elements["name"].Type, ShouldEqual, schema.TypeString) + So(elements["values"].Type, ShouldEqual, schema.TypeList) + }) + Convey("the provider cdn-datasource data source should have only the READ operation configured", func() { + So(tfProvider.DataSourcesMap[dataSourceName].Read, ShouldNotBeNil) + }) + + Convey("and the provider resource map must be nil as no resources are configured in the swagger", func() { + So(tfProvider.ResourcesMap, ShouldBeEmpty) + }) + }) + Convey("the provider configuration function should not be nil", func() { + So(tfProvider.ConfigureFunc, ShouldNotBeNil) + }) + }) + }) + }) + Convey("Given a local server that exposes a swagger file containing a terraform compatible resource taht has a model containing nested objects", t, func() { swaggerContent := `swagger: "2.0" host: "localhost:8443" @@ -371,7 +457,7 @@ definitions: So(tfProvider, ShouldNotBeNil) Convey("the provider schema should only include the endpoints property that enables users to override the resource host from the configuration", func() { So(tfProvider.Schema, ShouldNotBeNil) - assertTerraformSchemaProperty(tfProvider.Schema, "endpoints", schema.TypeSet, false, false) + assertTerraformSchemaProperty(t, tfProvider.Schema["endpoints"], schema.TypeSet, false, false) So(tfProvider.Schema["endpoints"].Elem.(*schema.Resource).Schema, ShouldContainKey, "cdn_v1") }) Convey("the provider resource map should contain the cdn resource with the expected configuration", func() { @@ -382,14 +468,14 @@ definitions: So(tfProvider.ResourcesMap, ShouldNotBeNil) resourceName := fmt.Sprintf("%s_cdn_v1", providerName) So(tfProvider.ResourcesMap, ShouldContainKey, resourceName) - assertTerraformSchemaProperty(tfProvider.ResourcesMap[resourceName].Schema, "label", schema.TypeString, true, false) - assertTerraformSchemaNestedObjectProperty(tfProvider.ResourcesMap[resourceName].Schema, "object_nested_scheme_property", false, false) + assertTerraformSchemaProperty(t, tfProvider.ResourcesMap[resourceName].Schema["label"], schema.TypeString, true, false) + assertTerraformSchemaNestedObjectProperty(t, tfProvider.ResourcesMap[resourceName].Schema["object_nested_scheme_property"], false, false) nestedObject := tfProvider.ResourcesMap[resourceName].Schema["object_nested_scheme_property"] - assertTerraformSchemaProperty(nestedObject.Elem.(*schema.Resource).Schema, "name", schema.TypeString, false, true) - assertTerraformSchemaProperty(nestedObject.Elem.(*schema.Resource).Schema, "object_property", schema.TypeMap, false, false) + assertTerraformSchemaProperty(t, nestedObject.Elem.(*schema.Resource).Schema["name"], schema.TypeString, false, true) + assertTerraformSchemaProperty(t, nestedObject.Elem.(*schema.Resource).Schema["object_property"], schema.TypeMap, false, false) object := nestedObject.Elem.(*schema.Resource).Schema["object_property"] - assertTerraformSchemaProperty(object.Elem.(*schema.Resource).Schema, "account", schema.TypeString, false, false) - assertTerraformSchemaProperty(object.Elem.(*schema.Resource).Schema, "read_only", schema.TypeString, false, true) + assertTerraformSchemaProperty(t, object.Elem.(*schema.Resource).Schema["account"], schema.TypeString, false, false) + assertTerraformSchemaProperty(t, object.Elem.(*schema.Resource).Schema["read_only"], schema.TypeString, false, true) }) Convey("the provider cdn resource should have the expected operations configured", func() { So(tfProvider.ResourcesMap[resourceName].Create, ShouldNotBeNil) @@ -408,26 +494,6 @@ definitions: } -func assertTerraformSchemaProperty(actualSchema map[string]*schema.Schema, expectedPropertyName string, expectedType schema.ValueType, expectedRequired, expectedComputed bool) { - fmt.Printf(">>> Validating '%s' property settings\n", expectedPropertyName) - So(actualSchema, ShouldContainKey, expectedPropertyName) - property := actualSchema[expectedPropertyName] - So(property.Type, ShouldEqual, expectedType) - if expectedRequired { - So(property.Required, ShouldBeTrue) - So(property.Optional, ShouldBeFalse) - } else { - So(property.Optional, ShouldBeTrue) - So(property.Required, ShouldBeFalse) - } - So(property.Computed, ShouldEqual, expectedComputed) -} - -func assertTerraformSchemaNestedObjectProperty(actualSchema map[string]*schema.Schema, expectedPropertyName string, expectedRequired, expectedComputed bool) { - assertTerraformSchemaProperty(actualSchema, expectedPropertyName, schema.TypeList, expectedRequired, expectedComputed) - So(actualSchema[expectedPropertyName].MaxItems, ShouldEqual, 1) -} - func Test_colliding_resource_names(t *testing.T) { makeSwaggerDoc := func(path1, preferredName1, path2, preferredName2 string, markIgnorePath1 bool) string { if path1 == "" { diff --git a/openapi/test_utils.go b/openapi/test_utils.go index 860f523a5..dda59f6e1 100644 --- a/openapi/test_utils.go +++ b/openapi/test_utils.go @@ -164,9 +164,23 @@ func initAPISpecFile(swaggerContent string) *os.File { } func assertDataSourceSchemaProperty(t *testing.T, actual *schema.Schema, expectedType schema.ValueType, msgAndArgs ...interface{}) { + assertTerraformSchemaProperty(t, actual, expectedType, false, true) +} + +func assertTerraformSchemaNestedObjectProperty(t *testing.T, actual *schema.Schema, expectedRequired, expectedComputed bool, msgAndArgs ...interface{}) { + assertTerraformSchemaProperty(t, actual, schema.TypeList, expectedRequired, expectedComputed) + assert.Equal(t, 1, actual.MaxItems, msgAndArgs) +} + +func assertTerraformSchemaProperty(t *testing.T, actual *schema.Schema, expectedType schema.ValueType, expectedRequired, expectedComputed bool, msgAndArgs ...interface{}) { assert.NotNil(t, actual, msgAndArgs) assert.Equal(t, expectedType, actual.Type, msgAndArgs) - assert.False(t, actual.Required, msgAndArgs) - assert.True(t, actual.Optional, msgAndArgs) - assert.True(t, actual.Computed, msgAndArgs) + if expectedRequired { + assert.True(t, actual.Required, msgAndArgs) + assert.False(t, actual.Optional, msgAndArgs) + } else { + assert.True(t, actual.Optional, msgAndArgs) + assert.False(t, actual.Required, msgAndArgs) + } + assert.Equal(t, expectedComputed, actual.Computed, msgAndArgs) } From 4cf3593c641086a30708a881cd51b92fc0f58fed Mon Sep 17 00:00:00 2001 From: dikhan Date: Fri, 20 Sep 2019 15:33:39 -0700 Subject: [PATCH 17/37] add timing telemetry and debug info --- openapi/provider_factory.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/openapi/provider_factory.go b/openapi/provider_factory.go index d846afdd1..8646e85b7 100644 --- a/openapi/provider_factory.go +++ b/openapi/provider_factory.go @@ -184,11 +184,13 @@ func (p providerFactory) createTerraformProviderDataSourceMap() (map[string]*sch if err != nil { return nil, err } + start := time.Now() d := newDataSourceFactory(openAPIDataSource) dataSourceTFSchema, err := d.createTerraformDataSource() if err != nil { return nil, err } + log.Printf("[INFO] data source '%s' successfully registered in the provider (time:%s)", dataSourceName, time.Since(start)) dataSourceMap[dataSourceName] = dataSourceTFSchema } return dataSourceMap, nil From 3a6736d4eef443d271a20b749cac353e9e052252 Mon Sep 17 00:00:00 2001 From: dikhan Date: Fri, 20 Sep 2019 15:33:48 -0700 Subject: [PATCH 18/37] go mod update --- go.mod | 6 +++--- go.sum | 5 +++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 85907894d..77a4eb4ac 100644 --- a/go.mod +++ b/go.mod @@ -36,10 +36,10 @@ require ( github.com/zach-klippenstein/goregen v0.0.0-20160303162051-795b5e3961ea // indirect golang.org/x/crypto v0.0.0-20190911031432-227b76d455e7 // indirect golang.org/x/lint v0.0.0-20190909230951-414d861bb4ac // indirect - golang.org/x/net v0.0.0-20190916140828-c8589233b77d // indirect + golang.org/x/net v0.0.0-20190918130420-a8b05e9114ab // indirect golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e // indirect - golang.org/x/sys v0.0.0-20190916202348-b4ddaad3f8a3 // indirect - golang.org/x/tools v0.0.0-20190917215024-905c8ffbfa41 + golang.org/x/sys v0.0.0-20190920190810-ef0ce1748380 // indirect + golang.org/x/tools v0.0.0-20190920130846-1081e67f6b77 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 a13153871..688b5d69b 100644 --- a/go.sum +++ b/go.sum @@ -497,6 +497,8 @@ golang.org/x/net v0.0.0-20190827160401-ba9fcec4b297 h1:k7pJ2yAPLPgbskkFdhRCsA77k golang.org/x/net v0.0.0-20190827160401-ba9fcec4b297/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20190916140828-c8589233b77d h1:mCMDWKhNO37A7GAhOpHPbIw1cjd0V86kX1/WA9c7FZ8= golang.org/x/net v0.0.0-20190916140828-c8589233b77d/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= +golang.org/x/net v0.0.0-20190918130420-a8b05e9114ab h1:h5tBRKZ1aY/bo6GNqe/4zWC8GkaLOFQ5wPKIOQ0i2sA= +golang.org/x/net v0.0.0-20190918130420-a8b05e9114ab/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20181017192945-9dcd33a902f4/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20181203162652-d668ce993890/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= @@ -531,6 +533,7 @@ golang.org/x/sys v0.0.0-20190730183949-1393eb018365/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20190902133755-9109b7679e13 h1:tdsQdquKbTNMsSZLqnLELJGzCANp9oXhu6zFBW6ODx4= golang.org/x/sys v0.0.0-20190902133755-9109b7679e13/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190916202348-b4ddaad3f8a3/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20190920190810-ef0ce1748380/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= @@ -580,6 +583,8 @@ golang.org/x/tools v0.0.0-20190830223141-573d9926052a h1:XAHT1kdPpnU8Hk+FPi42KZF golang.org/x/tools v0.0.0-20190830223141-573d9926052a/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20190917215024-905c8ffbfa41 h1:b81roplyyD40MvaAPpAaKtN/Aahd9L3t35zoiycwjRI= golang.org/x/tools v0.0.0-20190917215024-905c8ffbfa41/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= +golang.org/x/tools v0.0.0-20190920130846-1081e67f6b77 h1:m8wdt3dAJGnS+/iNksl3CpN6Y/ot01FQMblGuEzK02c= +golang.org/x/tools v0.0.0-20190920130846-1081e67f6b77/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/api v0.0.0-20180910000450-7ca32eb868bf/go.mod h1:4mhQ8q/RsB7i+udVvVy5NUi08OU8ZlA0gRVgrF7VFY0= google.golang.org/api v0.0.0-20181030000543-1d582fd0359e/go.mod h1:4mhQ8q/RsB7i+udVvVy5NUi08OU8ZlA0gRVgrF7VFY0= From 7c9c762ffac77103a1f49eaea22ab75979d9c3f9 Mon Sep 17 00:00:00 2001 From: lchan Date: Fri, 20 Sep 2019 17:08:32 -0700 Subject: [PATCH 19/37] WIP add case to TestCreateProvider to exercise error when createTerraformProviderDataSourceMap fails - Both resource and data source maps are populated in provider if resource is Terraform compliant - Will never see the errors from createTerraformProviderDataSourceMap since everything is caught in createTerraformProviderResourceMap --- openapi/provider_factory.go | 7 ++++- openapi/provider_factory_test.go | 52 ++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/openapi/provider_factory.go b/openapi/provider_factory.go index d846afdd1..211cb4708 100644 --- a/openapi/provider_factory.go +++ b/openapi/provider_factory.go @@ -58,9 +58,14 @@ func (p providerFactory) createProvider() (*schema.Provider, error) { } if dataSources, err = p.createTerraformProviderDataSourceMap(); err != nil { - return nil, err // TODO: untested + return nil, err } + // TODO: createTerraformProviderDataSourceMap error untested + // Data sources follow the same creation path as resources, so any errors will be caught in createTerraformProviderResourceMap + // Also, data sources are returned in *both* the ResourcesMap and the DataSourcesMap of the provider - is that the intended behavior? + // If not, need logic to skip createTerraformProviderResourceMap if we are dealing with data sources + provider := &schema.Provider{ Schema: providerSchema, ResourcesMap: resourceMap, diff --git a/openapi/provider_factory_test.go b/openapi/provider_factory_test.go index ee601e0f6..3998e3e00 100644 --- a/openapi/provider_factory_test.go +++ b/openapi/provider_factory_test.go @@ -263,6 +263,58 @@ func TestCreateProvider(t *testing.T) { }) }) }) + + // TODO: Fix the below test - failing because both ResourcesMap and DataSourcesMap are populated in the provider - not sure if this is the intended behavior + //Convey("Given a provider factory where createTerraformProviderDataSourceMap fails", t, func() { + // expectedError := "" + // apiKeyAuthProperty := newStringSchemaDefinitionPropertyWithDefaults("apikey_auth", "", true, false, "someAuthValue") + // headerProperty := newStringSchemaDefinitionPropertyWithDefaults("header_name", "", true, false, "someHeaderValue") + // p := providerFactory{ + // name: "provider", + // specAnalyser: &specAnalyserStub{ + // resources: []SpecResource{ + // &specStubResource{ + // name: "resource", + // path: "/v1/resource", + // shouldIgnore: false, + // schemaDefinition: &specSchemaDefinition{ + // Properties: specSchemaDefinitionProperties{ + // //&specSchemaDefinitionProperty{Type: "unsupported"}, + // }, + // }, + // resourceGetOperation: &specResourceOperation{}, + // timeouts: &specTimeouts{}, + // }, + // }, + // headers: SpecHeaderParameters{ + // SpecHeaderParam{Name: headerProperty.Name}, + // }, + // security: &specSecurityStub{ + // securityDefinitions: &SpecSecurityDefinitions{ + // newAPIKeyHeaderSecurityDefinition(apiKeyAuthProperty.Name, authorization), + // }, + // globalSecuritySchemes: createSecuritySchemes([]map[string][]string{ + // { + // apiKeyAuthProperty.Name: []string{""}, + // }, + // }), + // }, + // backendConfiguration: &specStubBackendConfiguration{}, + // }, + // serviceConfiguration: &ServiceConfigStub{}, + // } + // Convey("When createProvider is called ", func() { + // p, err := p.createProvider() + // Convey("Then the error returned should be as expected", func() { + // So(err.Error(), ShouldEqual, expectedError) + // }) + // Convey("And the provider returned should be nil", func() { + // So(p, ShouldBeNil) + // So(p.DataSourcesMap, ShouldNotBeNil) + // So(p.ResourcesMap, ShouldBeNil) + // }) + // }) + //}) } func TestCreateValidateFunc(t *testing.T) { From f2b1886df692629169e408235b86b395addfe876 Mon Sep 17 00:00:00 2001 From: lchan Date: Mon, 23 Sep 2019 13:02:40 -0700 Subject: [PATCH 20/37] Fix case in TestCreateProvider for createTerraformProviderDataSourceMap failure - Update specAnalyserStub to include dataSources - GetTerraformCompliantDataSources stub returns dataSources - Remove related todos and todo for subresources (covered on another previous commit) --- openapi/data_source_factory_test.go | 1 - openapi/openapi_spec_analyser_stub.go | 3 +- openapi/provider_factory.go | 5 -- openapi/provider_factory_test.go | 97 +++++++++++++-------------- 4 files changed, 48 insertions(+), 58 deletions(-) diff --git a/openapi/data_source_factory_test.go b/openapi/data_source_factory_test.go index 3680f4d8a..e3b984325 100644 --- a/openapi/data_source_factory_test.go +++ b/openapi/data_source_factory_test.go @@ -132,7 +132,6 @@ func TestDataSourceRead(t *testing.T) { expectedResult map[string]interface{} expectedError error }{ - // TODO: add a test to cover sub-resource use case too (need to cover this before releasing data source support) { name: "fetch selected data source as per filter configuration (label=someLabel)", filtersInput: []map[string]interface{}{ diff --git a/openapi/openapi_spec_analyser_stub.go b/openapi/openapi_spec_analyser_stub.go index dcaeb49ed..2be2d5def 100644 --- a/openapi/openapi_spec_analyser_stub.go +++ b/openapi/openapi_spec_analyser_stub.go @@ -3,6 +3,7 @@ package openapi // specAnalyserStub is a stubbed spec analyser used for testing purposes that implements the SpecAnalyser interface type specAnalyserStub struct { resources []SpecResource + dataSources []SpecResource security *specSecurityStub headers SpecHeaderParameters backendConfiguration SpecBackendConfiguration @@ -17,7 +18,7 @@ func (s *specAnalyserStub) GetTerraformCompliantResources() ([]SpecResource, err } func (s *specAnalyserStub) GetTerraformCompliantDataSources() []SpecResource { - return s.resources + return s.dataSources } func (s *specAnalyserStub) GetSecurity() SpecSecurity { diff --git a/openapi/provider_factory.go b/openapi/provider_factory.go index 211cb4708..020b186e2 100644 --- a/openapi/provider_factory.go +++ b/openapi/provider_factory.go @@ -61,11 +61,6 @@ func (p providerFactory) createProvider() (*schema.Provider, error) { return nil, err } - // TODO: createTerraformProviderDataSourceMap error untested - // Data sources follow the same creation path as resources, so any errors will be caught in createTerraformProviderResourceMap - // Also, data sources are returned in *both* the ResourcesMap and the DataSourcesMap of the provider - is that the intended behavior? - // If not, need logic to skip createTerraformProviderResourceMap if we are dealing with data sources - provider := &schema.Provider{ Schema: providerSchema, ResourcesMap: resourceMap, diff --git a/openapi/provider_factory_test.go b/openapi/provider_factory_test.go index 3998e3e00..82cceca3d 100644 --- a/openapi/provider_factory_test.go +++ b/openapi/provider_factory_test.go @@ -264,57 +264,52 @@ func TestCreateProvider(t *testing.T) { }) }) - // TODO: Fix the below test - failing because both ResourcesMap and DataSourcesMap are populated in the provider - not sure if this is the intended behavior - //Convey("Given a provider factory where createTerraformProviderDataSourceMap fails", t, func() { - // expectedError := "" - // apiKeyAuthProperty := newStringSchemaDefinitionPropertyWithDefaults("apikey_auth", "", true, false, "someAuthValue") - // headerProperty := newStringSchemaDefinitionPropertyWithDefaults("header_name", "", true, false, "someHeaderValue") - // p := providerFactory{ - // name: "provider", - // specAnalyser: &specAnalyserStub{ - // resources: []SpecResource{ - // &specStubResource{ - // name: "resource", - // path: "/v1/resource", - // shouldIgnore: false, - // schemaDefinition: &specSchemaDefinition{ - // Properties: specSchemaDefinitionProperties{ - // //&specSchemaDefinitionProperty{Type: "unsupported"}, - // }, - // }, - // resourceGetOperation: &specResourceOperation{}, - // timeouts: &specTimeouts{}, - // }, - // }, - // headers: SpecHeaderParameters{ - // SpecHeaderParam{Name: headerProperty.Name}, - // }, - // security: &specSecurityStub{ - // securityDefinitions: &SpecSecurityDefinitions{ - // newAPIKeyHeaderSecurityDefinition(apiKeyAuthProperty.Name, authorization), - // }, - // globalSecuritySchemes: createSecuritySchemes([]map[string][]string{ - // { - // apiKeyAuthProperty.Name: []string{""}, - // }, - // }), - // }, - // backendConfiguration: &specStubBackendConfiguration{}, - // }, - // serviceConfiguration: &ServiceConfigStub{}, - // } - // Convey("When createProvider is called ", func() { - // p, err := p.createProvider() - // Convey("Then the error returned should be as expected", func() { - // So(err.Error(), ShouldEqual, expectedError) - // }) - // Convey("And the provider returned should be nil", func() { - // So(p, ShouldBeNil) - // So(p.DataSourcesMap, ShouldNotBeNil) - // So(p.ResourcesMap, ShouldBeNil) - // }) - // }) - //}) + Convey("Given a provider factory where createTerraformProviderDataSourceMap fails", t, func() { + expectedError := "resource name can not be empty" + apiKeyAuthProperty := newStringSchemaDefinitionPropertyWithDefaults("apikey_auth", "", true, false, "someAuthValue") + headerProperty := newStringSchemaDefinitionPropertyWithDefaults("header_name", "", true, false, "someHeaderValue") + p := providerFactory{ + name: "provider", + specAnalyser: &specAnalyserStub{ + dataSources: []SpecResource{ + &specStubResource{ + name: "", + path: "/v1/resource", + shouldIgnore: false, + schemaDefinition: &specSchemaDefinition{ + Properties: specSchemaDefinitionProperties{}, + }, + resourceGetOperation: &specResourceOperation{}, + timeouts: &specTimeouts{}, + }, + }, + headers: SpecHeaderParameters{ + SpecHeaderParam{Name: headerProperty.Name}, + }, + security: &specSecurityStub{ + securityDefinitions: &SpecSecurityDefinitions{ + newAPIKeyHeaderSecurityDefinition(apiKeyAuthProperty.Name, authorization), + }, + globalSecuritySchemes: createSecuritySchemes([]map[string][]string{ + { + apiKeyAuthProperty.Name: []string{""}, + }, + }), + }, + backendConfiguration: &specStubBackendConfiguration{}, + }, + serviceConfiguration: &ServiceConfigStub{}, + } + Convey("When createProvider is called ", func() { + p, err := p.createProvider() + Convey("Then the error returned should be as expected", func() { + So(err.Error(), ShouldEqual, expectedError) + }) + Convey("And the provider returned should be nil", func() { + So(p, ShouldBeNil) + }) + }) + }) } func TestCreateValidateFunc(t *testing.T) { From 28768593ffebdb95cf0399fb3ac947a492d5ae29 Mon Sep 17 00:00:00 2001 From: dikhan Date: Mon, 23 Sep 2019 13:03:10 -0700 Subject: [PATCH 21/37] add info that subresource paths are also supported in data sources --- docs/how_to.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/how_to.md b/docs/how_to.md index 4680132b0..1668355a8 100644 --- a/docs/how_to.md +++ b/docs/how_to.md @@ -265,7 +265,7 @@ The OpenAPI provider is able to export data sources from paths that are data sou An endpoint (path) to be considered terraform data source compliant must meet the following criteria: -- The path must be a root level path (e,g: /v1/cdns) not an instance path (e,g: /api/v1/cdn/{id}) +- The path must be a root level path (e,g: /v1/cdns) not an instance path (e,g: /api/v1/cdn/{id}). Subresource data source paths are also supported (e,g: /v1/cdns/{id}/firewalls) - The path must contain a GET operation with a response 200 which contains a schema of type 'array'. The items schema must be of type 'object' and must specify at least one property. - The items schema object definitnio must contain a property called ```id``` which will be used internally to uniquely identify the data source. If the object schema does not have a property called ```id```, then at least one property should have the ```x-terraform-id``` extension From 75e8e25b09906dd60ee38adbcd6d3dfc898b7875 Mon Sep 17 00:00:00 2001 From: lchan Date: Mon, 23 Sep 2019 13:08:51 -0700 Subject: [PATCH 22/37] Update TestCreateTerraformProviderDataSourceMap to use dataSources on the specAnalyserStub --- openapi/provider_factory_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/openapi/provider_factory_test.go b/openapi/provider_factory_test.go index 82cceca3d..14d6b5651 100644 --- a/openapi/provider_factory_test.go +++ b/openapi/provider_factory_test.go @@ -852,21 +852,21 @@ func TestCreateTerraformProviderDataSourceMap(t *testing.T) { { name: "happy path", specV2stub: &specAnalyserStub{ - resources: []SpecResource{newSpecStubResource("resource", "/v1/resource", false, &specSchemaDefinition{})}, + dataSources: []SpecResource{newSpecStubResource("resource", "/v1/resource", false, &specSchemaDefinition{})}, }, expectedResourceName: "provider_resource", }, { name: "getProviderResourceName fails ", specV2stub: &specAnalyserStub{ - resources: []SpecResource{newSpecStubResource("", "/v1/resource", false, &specSchemaDefinition{})}, + dataSources: []SpecResource{newSpecStubResource("", "/v1/resource", false, &specSchemaDefinition{})}, }, expectedError: "resource name can not be empty", }, { name: "createTerraformDataSource fails", specV2stub: &specAnalyserStub{ - resources: []SpecResource{&specStubResource{ + dataSources: []SpecResource{&specStubResource{ name: "hello", funcGetResourceSchema: func() (*specSchemaDefinition, error) { return nil, errors.New("createTerraformDataSource failed") From f4a8dc21447409108e2b39b58bb3f5552701c509 Mon Sep 17 00:00:00 2001 From: dikhan Date: Tue, 1 Oct 2019 12:40:14 -0700 Subject: [PATCH 23/37] add integration test for new data source type (instance) --- tests/e2e/gray_box_cdns_test.go | 59 +++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/tests/e2e/gray_box_cdns_test.go b/tests/e2e/gray_box_cdns_test.go index a6df63e7b..a39e92e7c 100644 --- a/tests/e2e/gray_box_cdns_test.go +++ b/tests/e2e/gray_box_cdns_test.go @@ -644,6 +644,58 @@ func TestAccCDN_DataSource(t *testing.T) { }) } +func TestAccCDN_DataSourceInstance(t *testing.T) { + api := initAPI(t, cdnSwaggerYAMLTemplate) + api.cachePayloads = map[string]interface{}{ // Pretending resource already exists remotely + "/v1/cdns/" + expectedCDNID: fmt.Sprintf( + `{ +"id":%s, +"label":"%s", +"computed_property": "some auto-generated value", +"object_property_argument": {"account":"my_account", "object_read_only_property": "some computed value for object read only"}, +"object_property_block": {"account":"my_account", "object_read_only_property": "some computed value for object read only"} +}`, expectedCDNID, expectedCDNLabel), + } + + dataSourceInstanceName := fmt.Sprintf("%s_instance", openAPIResourceNameCDN) + tfFileContents := fmt.Sprintf(` + data "%s" "%s" { + id = "%s" + }`, dataSourceInstanceName, openAPIDataSourceNameCDN, expectedCDNID) + openAPIDataSourceInstanceStateCDN := fmt.Sprintf("data.%s.%s", dataSourceInstanceName, openAPIDataSourceNameCDN) + + p := openapi.ProviderOpenAPI{ProviderName: providerName} + provider, err := p.CreateSchemaProviderFromServiceConfiguration(&openapi.ServiceConfigStub{SwaggerURL: api.swaggerURL}) + assert.NoError(t, err) + assertProviderSchema(t, provider) + + var testAccProviders = map[string]terraform.ResourceProvider{providerName: provider} + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + Providers: testAccProviders, + PreCheck: func() { testAccPreCheck(t, api.swaggerURL) }, + Steps: []resource.TestStep{ + { + Config: tfFileContents, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + openAPIDataSourceInstanceStateCDN, "id", expectedCDNID), + resource.TestCheckResourceAttr( + openAPIDataSourceInstanceStateCDN, "label", expectedCDNLabel), + resource.TestCheckResourceAttr( + openAPIDataSourceInstanceStateCDN, "computed_property", "some auto-generated value"), + resource.TestCheckResourceAttr( + openAPIDataSourceInstanceStateCDN, "object_property_block.#", "1"), + resource.TestCheckResourceAttr( + openAPIDataSourceInstanceStateCDN, "object_property_block.0.account", "my_account"), + resource.TestCheckResourceAttr( + openAPIDataSourceInstanceStateCDN, "object_property_block.0.object_read_only_property", "some computed value for object read only"), + ), + }, + }, + }) +} + func assertProviderSchema(t *testing.T, provider *schema.Provider) { // Resource map check assert.Nil(t, provider.ResourcesMap[openAPIResourceNameCDN].Schema["id"]) @@ -659,6 +711,13 @@ func assertProviderSchema(t *testing.T, provider *schema.Provider) { assert.NotNil(t, provider.DataSourcesMap[openAPIResourceNameCDN].Schema["computed_property"]) assert.NotNil(t, provider.DataSourcesMap[openAPIResourceNameCDN].Schema["object_property_argument"]) assert.NotNil(t, provider.DataSourcesMap[openAPIResourceNameCDN].Schema["object_property_block"]) + + openAPIDataSourceInstanceCDN := openAPIResourceNameCDN + "_instance" + assert.NotNil(t, provider.DataSourcesMap[openAPIDataSourceInstanceCDN].Schema["id"]) // data source instance expects only one property from the user called 'id'. Hence, checking that is configured as expected + assert.NotNil(t, provider.DataSourcesMap[openAPIDataSourceInstanceCDN].Schema["label"]) + assert.NotNil(t, provider.DataSourcesMap[openAPIDataSourceInstanceCDN].Schema["computed_property"]) + assert.NotNil(t, provider.DataSourcesMap[openAPIDataSourceInstanceCDN].Schema["object_property_argument"]) + assert.NotNil(t, provider.DataSourcesMap[openAPIDataSourceInstanceCDN].Schema["object_property_block"]) } func createTerraformFile(expectedCDNLabel, expectedFirewallLabel string) string { From d4fdfbc461fa97d83a7edb168921dda862a0f0cb Mon Sep 17 00:00:00 2001 From: dikhan Date: Tue, 1 Oct 2019 13:28:08 -0700 Subject: [PATCH 24/37] create data source instance factory - added test coverage --- openapi/data_source_instance_factory.go | 51 ++++++++ openapi/data_source_instance_factory_test.go | 116 +++++++++++++++++++ 2 files changed, 167 insertions(+) create mode 100644 openapi/data_source_instance_factory.go create mode 100644 openapi/data_source_instance_factory_test.go diff --git a/openapi/data_source_instance_factory.go b/openapi/data_source_instance_factory.go new file mode 100644 index 000000000..228d1b3a4 --- /dev/null +++ b/openapi/data_source_instance_factory.go @@ -0,0 +1,51 @@ +package openapi + +import ( + "fmt" + + "github.com/hashicorp/terraform/helper/schema" +) + +const dataSourceInstanceIDProperty = "id" + +type dataSourceInstanceFactory struct { + openAPIResource SpecResource +} + +func newDataSourceInstanceFactory(openAPIResource SpecResource) dataSourceInstanceFactory { + return dataSourceInstanceFactory{ + openAPIResource: openAPIResource, + } +} + +func (d dataSourceInstanceFactory) getDataSourceInstanceName() string { + return fmt.Sprintf("%s_instance", d.openAPIResource.getResourceName()) +} + +func (d dataSourceInstanceFactory) createTerraformInstanceDataSource() (*schema.Resource, error) { + s, err := d.createTerraformDataSourceInstanceSchema() + if err != nil { + return nil, err + } + return &schema.Resource{ + Schema: s, + //Read: d.read, + }, nil +} + +func (d dataSourceInstanceFactory) createTerraformDataSourceInstanceSchema() (map[string]*schema.Schema, error) { + specSchema, err := d.openAPIResource.getResourceSchema() + if err != nil { + return nil, err + } + dataSourceSchema, err := specSchema.createDataSourceSchema() + dataSourceSchema[dataSourceInstanceIDProperty] = d.dataSourceInstanceSchema() + return dataSourceSchema, nil +} + +func (d dataSourceInstanceFactory) dataSourceInstanceSchema() *schema.Schema { + return &schema.Schema{ + Type: schema.TypeString, + Required: true, + } +} diff --git a/openapi/data_source_instance_factory_test.go b/openapi/data_source_instance_factory_test.go new file mode 100644 index 000000000..940bd5842 --- /dev/null +++ b/openapi/data_source_instance_factory_test.go @@ -0,0 +1,116 @@ +package openapi + +import ( + "errors" + "github.com/hashicorp/terraform/helper/schema" + "github.com/stretchr/testify/assert" + "testing" +) + +func TestNewDataSourceInstanceFactory(t *testing.T) { + openAPIResource := &specStubResource{} + d := newDataSourceInstanceFactory(openAPIResource) + assert.NotNil(t, d) + assert.Equal(t, openAPIResource, d.openAPIResource) +} + +func TestGetDataSourceInstanceName(t *testing.T) { + d := newDataSourceInstanceFactory(&specStubResource{name: "cdn"}) + name := d.getDataSourceInstanceName() + assert.Equal(t, "cdn_instance", name) +} + +func TestCreateTerraformInstanceDataSource(t *testing.T) { + testCases := []struct { + name string + expectedError error + specStubResourceErr error + }{ + { + name: "happy path - Terraform data source created as expected", + expectedError: nil, + specStubResourceErr: nil, + }, + { + name: "crappy path - Terraform data source schema has an error", + expectedError: errors.New("data source schema has an error"), + specStubResourceErr: errors.New("data source schema has an error"), + }, + } + + for _, tc := range testCases { + dataSourceFactory := dataSourceInstanceFactory{ + openAPIResource: &specStubResource{ + schemaDefinition: &specSchemaDefinition{ + Properties: specSchemaDefinitionProperties{ + newStringSchemaDefinitionPropertyWithDefaults("id", "", false, true, nil), + newStringSchemaDefinitionPropertyWithDefaults("label", "", false, false, nil), + }, + }, + error: tc.specStubResourceErr, + }, + } + + dataSource, err := dataSourceFactory.createTerraformInstanceDataSource() + + if tc.expectedError == nil { + assert.Nil(t, err, tc.name) + assert.NotNil(t, dataSource, tc.name) + assert.NotNil(t, dataSource.Read, tc.name) + assert.Nil(t, dataSource.Delete, tc.name) + assert.Nil(t, dataSource.Create, tc.name) + assert.Nil(t, dataSource.Update, tc.name) + } else { + assert.Equal(t, tc.expectedError.Error(), err.Error(), tc.name) + } + } +} + +func TestCreateTerraformDataSourceInstance(t *testing.T) { + testCases := []struct { + name string + openAPIResource SpecResource + expectedError error + }{ + { + name: "happy path - data source schema is configured as expected", + openAPIResource: &specStubResource{ + schemaDefinition: &specSchemaDefinition{ + Properties: specSchemaDefinitionProperties{ + newStringSchemaDefinitionPropertyWithDefaults("id", "", false, true, nil), + newStringSchemaDefinitionPropertyWithDefaults("label", "", false, false, nil), + }, + }, + }, + expectedError: nil, + }, + { + name: "crappy path - data source schema definition is nil", + openAPIResource: &specStubResource{ + schemaDefinition: nil, + error: errors.New("error due to nil schema def"), + }, + expectedError: errors.New("error due to nil schema def"), + }, + } + + for _, tc := range testCases { + dataSourceFactory := dataSourceInstanceFactory{ + openAPIResource: tc.openAPIResource, + } + s, err := dataSourceFactory.createTerraformDataSourceInstanceSchema() + if tc.expectedError == nil { + assert.NotNil(t, s, tc.name) + // data source specific input properties + assert.NotNil(t, s[dataSourceInstanceIDProperty], tc.name) + assert.True(t, s[dataSourceInstanceIDProperty].Required, tc.name) + assert.Equal(t, schema.TypeString, s[dataSourceInstanceIDProperty].Type, tc.name) + + // resource specific properties as per swagger def (this properties are meant to be populated by the read operation when a match is found as per the filters) + assert.Contains(t, s, "label", tc.name) + assert.True(t, s["label"].Computed, tc.name) + } else { + assert.Equal(t, tc.expectedError.Error(), err.Error(), tc.name) + } + } +} From 9afcba7e9180222e9c79a7300df38853ae4d6731 Mon Sep 17 00:00:00 2001 From: dikhan Date: Tue, 1 Oct 2019 13:54:50 -0700 Subject: [PATCH 25/37] add read support for data source instance - added corresponding test coverage --- openapi/data_source_instance_factory.go | 28 ++++- openapi/data_source_instance_factory_test.go | 105 +++++++++++++++++++ 2 files changed, 132 insertions(+), 1 deletion(-) diff --git a/openapi/data_source_instance_factory.go b/openapi/data_source_instance_factory.go index 228d1b3a4..99f33130b 100644 --- a/openapi/data_source_instance_factory.go +++ b/openapi/data_source_instance_factory.go @@ -2,6 +2,7 @@ package openapi import ( "fmt" + "net/http" "github.com/hashicorp/terraform/helper/schema" ) @@ -29,7 +30,7 @@ func (d dataSourceInstanceFactory) createTerraformInstanceDataSource() (*schema. } return &schema.Resource{ Schema: s, - //Read: d.read, + Read: d.read, }, nil } @@ -49,3 +50,28 @@ func (d dataSourceInstanceFactory) dataSourceInstanceSchema() *schema.Schema { Required: true, } } + +func (d dataSourceInstanceFactory) read(data *schema.ResourceData, i interface{}) error { + openAPIClient := i.(ClientOpenAPI) + parentIDs, resourcePath, err := getParentIDsAndResourcePath(d.openAPIResource, data) + if err != nil { + return err + } + id := data.Get(dataSourceInstanceIDProperty) + if id == nil || id == "" { + return fmt.Errorf("data source 'id' property value must be populated") + } + responsePayload := map[string]interface{}{} + resp, err := openAPIClient.Get(d.openAPIResource, id.(string), &responsePayload, parentIDs...) + if err != nil { + return err + } + if err := checkHTTPStatusCode(d.openAPIResource, resp, []int{http.StatusOK}); err != nil { + return fmt.Errorf("[data source instance='%s'] GET %s failed: %s", d.openAPIResource.getResourceName(), resourcePath, err) + } + err = setStateID(d.openAPIResource, data, responsePayload) + if err != nil { + return err + } + return updateStateWithPayloadData(d.openAPIResource, responsePayload, data) +} diff --git a/openapi/data_source_instance_factory_test.go b/openapi/data_source_instance_factory_test.go index 940bd5842..4a5fffba0 100644 --- a/openapi/data_source_instance_factory_test.go +++ b/openapi/data_source_instance_factory_test.go @@ -4,6 +4,8 @@ import ( "errors" "github.com/hashicorp/terraform/helper/schema" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "net/http" "testing" ) @@ -114,3 +116,106 @@ func TestCreateTerraformDataSourceInstance(t *testing.T) { } } } + +func TestDataSourceInstanceRead(t *testing.T) { + + testCases := []struct { + name string + inputID string + client *clientOpenAPIStub + expectedResult map[string]interface{} + expectedError error + }{ + { + name: "fetch selected data source as per input ID", + inputID: "ID", + client: &clientOpenAPIStub{ + responsePayload: map[string]interface{}{ + "id": "someID", + "label": "someLabel", + "owners": []string{"someOwner"}, + }, + }, + expectedError: nil, + }, + { + name: "input ID is empty", + inputID: "", + client: &clientOpenAPIStub{ + responsePayload: map[string]interface{}{ + "id": "someID", + "label": "someLabel", + "owners": []string{"someOwner"}, + }, + }, + expectedError: errors.New("data source 'id' property value must be populated"), + }, + { + name: "empty response from API", + inputID: "ID", + client: &clientOpenAPIStub{ + responsePayload: map[string]interface{}{}, + }, + expectedError: errors.New("response object returned from the API is missing mandatory identifier property 'id'"), + }, + { + name: "api returns a non expected code 404", + inputID: "ID", + client: &clientOpenAPIStub{ + responsePayload: map[string]interface{}{}, + returnHTTPCode: http.StatusNotFound, + }, + expectedError: errors.New("[data source instance=''] GET failed: HTTP Reponse Status Code 404 - Not Found. Could not find resource instance: "), + }, + { + name: "get operation returns an error", + inputID: "ID", + client: &clientOpenAPIStub{ + error: errors.New("some api error in the get operation"), + }, + expectedError: errors.New("some api error in the get operation"), + }, + } + + dataSourceFactory := dataSourceInstanceFactory{ + openAPIResource: &specStubResource{ + schemaDefinition: &specSchemaDefinition{ + Properties: specSchemaDefinitionProperties{ + newStringSchemaDefinitionPropertyWithDefaults("id", "", false, true, nil), + newStringSchemaDefinitionPropertyWithDefaults("label", "", false, false, nil), + newListSchemaDefinitionPropertyWithDefaults("owners", "", true, false, false, []string{"value1"}, typeString, nil), + }, + }, + }, + } + + for _, tc := range testCases { + resourceSchema, err := dataSourceFactory.createTerraformDataSourceInstanceSchema() + require.NoError(t, err) + + dataSourceUserInput := map[string]interface{}{ + dataSourceInstanceIDProperty: tc.inputID, + } + resourceData := schema.TestResourceDataRaw(t, resourceSchema, dataSourceUserInput) + // When + err = dataSourceFactory.read(resourceData, tc.client) + // Then + if tc.expectedError == nil { + assert.Nil(t, err, tc.name) + assert.Equal(t, tc.client.responsePayload["id"], resourceData.Id(), tc.name) + assert.Equal(t, tc.client.responsePayload["label"], resourceData.Get("label"), tc.name) + expectedOwners := tc.client.responsePayload["owners"].([]string) + owners := resourceData.Get("owners").([]interface{}) + assert.NotNil(t, owners, tc.name) + assert.NotNil(t, len(expectedOwners), len(owners), tc.name) + assert.Equal(t, expectedOwners[0], owners[0], tc.name) + } else { + assert.Equal(t, tc.expectedError.Error(), err.Error(), tc.name) + } + } +} + +func TestDataSourceInstanceRead_Fails_Because_Cannot_extract_ParentsID(t *testing.T) { + err := dataSourceInstanceFactory{}.read(nil, &clientOpenAPIStub{}) + assert.EqualError(t, err, "can't get parent ids from a resourceFactory with no openAPIResource") +} From d071248dbe8360edb99856d00343acad1ab838d9 Mon Sep 17 00:00:00 2001 From: dikhan Date: Tue, 1 Oct 2019 14:35:04 -0700 Subject: [PATCH 26/37] create createTerraformProviderDataSourceInstanceMap - createTerraformProviderDataSourceInstanceMap only registers as data source instances the openAPIResources that are terraform resource compatible --- openapi/provider_factory.go | 32 ++++++++++++ openapi/provider_factory_test.go | 86 ++++++++++++++++++++++++++++++++ 2 files changed, 118 insertions(+) diff --git a/openapi/provider_factory.go b/openapi/provider_factory.go index f16fcda97..96fe6ac02 100644 --- a/openapi/provider_factory.go +++ b/openapi/provider_factory.go @@ -176,6 +176,38 @@ func (p providerFactory) createValidateFunc(allowedValues []string) func(val int return nil } +// createTerraformProviderDataSourceInstanceMap only registers as data source instances the openAPIResources that are +// terraform resource compatible +func (p providerFactory) createTerraformProviderDataSourceInstanceMap() (map[string]*schema.Resource, error) { + dataSourceInstanceMap := map[string]*schema.Resource{} + openAPIResources, err := p.specAnalyser.GetTerraformCompliantResources() + if err != nil { + return nil, err + } + for _, openAPIResource := range openAPIResources { + start := time.Now() + r := newDataSourceInstanceFactory(openAPIResource) + dataSourceInstanceName := r.getDataSourceInstanceName() + fullDataSourceInstanceName, _ := p.getProviderResourceName(dataSourceInstanceName) // dataSourceInstanceName is always non empty string + if openAPIResource.shouldIgnoreResource() { + log.Printf("[WARN] '%s' is marked to be ignored and therefore skipping resource registration into the provider", openAPIResource.getResourceName()) + continue + } + if _, alreadyThere := dataSourceInstanceMap[fullDataSourceInstanceName]; alreadyThere { + log.Printf("[WARN] '%s' is a duplicate data source instance name and is being removed from the provider", fullDataSourceInstanceName) + delete(dataSourceInstanceMap, fullDataSourceInstanceName) + continue + } + resource, err := r.createTerraformInstanceDataSource() + if err != nil { + return nil, err + } + log.Printf("[INFO] data source instance '%s' successfully registered in the provider (time:%s)", fullDataSourceInstanceName, time.Since(start)) + dataSourceInstanceMap[fullDataSourceInstanceName] = resource + } + return dataSourceInstanceMap, nil +} + func (p providerFactory) createTerraformProviderDataSourceMap() (map[string]*schema.Resource, error) { dataSourceMap := map[string]*schema.Resource{} openAPIDataResources := p.specAnalyser.GetTerraformCompliantDataSources() diff --git a/openapi/provider_factory_test.go b/openapi/provider_factory_test.go index 14d6b5651..0f165c7ab 100644 --- a/openapi/provider_factory_test.go +++ b/openapi/provider_factory_test.go @@ -841,6 +841,92 @@ func TestGetProviderResourceName(t *testing.T) { }) } +func TestCreateTerraformProviderDataSourceInstanceMap(t *testing.T) { + testcases := []struct { + name string + specV2stub SpecAnalyser + expectedResourceName string + expectedError string + }{ + { + name: "happy path", + specV2stub: &specAnalyserStub{ + resources: []SpecResource{newSpecStubResource("resource", "/v1/resource", false, &specSchemaDefinition{})}, + }, + expectedResourceName: "provider_resource_instance", + }, + { + name: "getTerraformCompliantResources fails ", + specV2stub: &specAnalyserStub{ + error: fmt.Errorf("error getting terraform compliant resources"), + }, + expectedError: "error getting terraform compliant resources", + }, + { + name: "getProviderResourceName fails ", + specV2stub: &specAnalyserStub{ + resources: []SpecResource{newSpecStubResource("", "/v1/resource", false, &specSchemaDefinition{})}, + }, + expectedError: "resource name can not be empty", + }, + { + name: "createTerraformDataSource fails", + specV2stub: &specAnalyserStub{ + resources: []SpecResource{&specStubResource{ + name: "hello", + funcGetResourceSchema: func() (*specSchemaDefinition, error) { + return nil, errors.New("createTerraformDataSource failed") + }, + }}, + }, + expectedError: "createTerraformDataSource failed", + }, + } + + for _, tc := range testcases { + p := providerFactory{ + name: "provider", + specAnalyser: tc.specV2stub, + } + schemaResource, err := p.createTerraformProviderDataSourceInstanceMap() + + if tc.expectedError == "" { + assert.Nil(t, err) + assert.Contains(t, schemaResource, tc.expectedResourceName, tc.name) + } else { + assert.EqualError(t, err, tc.expectedError, tc.name) + } + } +} + +func TestCreateTerraformProviderDataSourceInstanceMap_ignore_resource(t *testing.T) { + p := providerFactory{ + name: "provider", + specAnalyser: &specAnalyserStub{ + resources: []SpecResource{ + newSpecStubResource("resource", "/v1/resource", true, &specSchemaDefinition{}), + }, + }, + } + schemaResource, err := p.createTerraformProviderDataSourceInstanceMap() + assert.Nil(t, err) + assert.Empty(t, schemaResource) +} + +func TestCreateTerraformProviderDataSourceInstanceMap_duplicate_resource(t *testing.T) { + p := providerFactory{ + name: "provider", + specAnalyser: &specAnalyserStub{ + resources: []SpecResource{ + newSpecStubResource("resource", "/v1/resource", false, &specSchemaDefinition{}), + newSpecStubResource("resource", "/v1/resource", false, &specSchemaDefinition{})}, + }, + } + schemaResource, err := p.createTerraformProviderDataSourceInstanceMap() + assert.Nil(t, err) + assert.Empty(t, schemaResource) +} + func TestCreateTerraformProviderDataSourceMap(t *testing.T) { testcases := []struct { From 622654f3b9ce140bddddc244b9b00dd406b82117 Mon Sep 17 00:00:00 2001 From: dikhan Date: Tue, 1 Oct 2019 16:15:15 -0700 Subject: [PATCH 27/37] handle createDataSourceSchema error returned --- openapi/data_source_instance_factory.go | 3 +++ openapi/data_source_instance_factory_test.go | 17 +++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/openapi/data_source_instance_factory.go b/openapi/data_source_instance_factory.go index 99f33130b..c32a797e7 100644 --- a/openapi/data_source_instance_factory.go +++ b/openapi/data_source_instance_factory.go @@ -40,6 +40,9 @@ func (d dataSourceInstanceFactory) createTerraformDataSourceInstanceSchema() (ma return nil, err } dataSourceSchema, err := specSchema.createDataSourceSchema() + if err != nil { + return nil, err + } dataSourceSchema[dataSourceInstanceIDProperty] = d.dataSourceInstanceSchema() return dataSourceSchema, nil } diff --git a/openapi/data_source_instance_factory_test.go b/openapi/data_source_instance_factory_test.go index 4a5fffba0..471ff01d0 100644 --- a/openapi/data_source_instance_factory_test.go +++ b/openapi/data_source_instance_factory_test.go @@ -215,6 +215,23 @@ func TestDataSourceInstanceRead(t *testing.T) { } } +func TestDataSourceInstanceRead_Fails_Because_Schema_is_not_valid(t *testing.T) { + dataSourceFactory := dataSourceInstanceFactory{ + openAPIResource: &specStubResource{ + schemaDefinition: &specSchemaDefinition{ + Properties: specSchemaDefinitionProperties{ + &specSchemaDefinitionProperty{ + Name: "label", + Type: "unknown", + }, + }, + }, + }, + } + _, err := dataSourceFactory.createTerraformDataSourceInstanceSchema() + assert.EqualError(t, err, "non supported type unknown") +} + func TestDataSourceInstanceRead_Fails_Because_Cannot_extract_ParentsID(t *testing.T) { err := dataSourceInstanceFactory{}.read(nil, &clientOpenAPIStub{}) assert.EqualError(t, err, "can't get parent ids from a resourceFactory with no openAPIResource") From c3209186f47662681b86f01f36c56be900a377a1 Mon Sep 17 00:00:00 2001 From: dikhan Date: Tue, 1 Oct 2019 16:38:52 -0700 Subject: [PATCH 28/37] refactor createTerraformProviderResourceMap to return both resources and data source instance - since createTerraformProviderResourceMap was already looping through the terraform compatible resources and considering the data source instance is only registered from already deemed terraform compatible resources, reusing the logic and returning both maps in one go --- ...pi_spec_resource_schema_definition_test.go | 47 ++++++++ openapi/provider_factory.go | 77 ++++++------ openapi/provider_factory_test.go | 114 ++++-------------- 3 files changed, 102 insertions(+), 136 deletions(-) diff --git a/openapi/openapi_spec_resource_schema_definition_test.go b/openapi/openapi_spec_resource_schema_definition_test.go index ddfe2c42b..69d352b27 100644 --- a/openapi/openapi_spec_resource_schema_definition_test.go +++ b/openapi/openapi_spec_resource_schema_definition_test.go @@ -369,6 +369,53 @@ func TestCreateResourceSchema(t *testing.T) { }) }) }) + + Convey("Given a swagger schema definition that has a combination of properties supported", t, func() { + s := &specSchemaDefinition{ + Properties: specSchemaDefinitionProperties{ + idProperty, + stringProperty, + intProperty, + numberProperty, + boolProperty, + slicePrimitiveProperty, + computedProperty, + optionalProperty, + sensitiveProperty, + forceNewProperty, + }, + } + Convey("When createResourceSchema method is called", func() { + tfResourceSchema, err := s.createResourceSchema() + Convey("Then the error returned should be nil", func() { + So(err, ShouldBeNil) + }) + Convey("And the schema for the resource should contain the expected attributes", func() { + So(tfResourceSchema, ShouldContainKey, stringProperty.Name) + So(tfResourceSchema, ShouldContainKey, computedProperty.Name) + So(tfResourceSchema, ShouldContainKey, intProperty.Name) + So(tfResourceSchema, ShouldContainKey, numberProperty.Name) + So(tfResourceSchema, ShouldContainKey, boolProperty.Name) + So(tfResourceSchema, ShouldContainKey, slicePrimitiveProperty.Name) + So(tfResourceSchema, ShouldContainKey, optionalProperty.Name) + So(tfResourceSchema, ShouldContainKey, sensitiveProperty.Name) + So(tfResourceSchema, ShouldContainKey, forceNewProperty.Name) + }) + Convey("And the schema property types should match the expected configuration", func() { + So(tfResourceSchema[stringProperty.Name].Type, ShouldEqual, schema.TypeString) + So(tfResourceSchema[intProperty.Name].Type, ShouldEqual, schema.TypeInt) + So(tfResourceSchema[numberProperty.Name].Type, ShouldEqual, schema.TypeFloat) + So(tfResourceSchema[boolProperty.Name].Type, ShouldEqual, schema.TypeBool) + So(tfResourceSchema[slicePrimitiveProperty.Name].Type, ShouldEqual, schema.TypeList) + }) + Convey("And the schema property options should match the expected configuration", func() { + So(tfResourceSchema[computedProperty.Name].Computed, ShouldBeTrue) + So(tfResourceSchema[optionalProperty.Name].Optional, ShouldBeTrue) + So(tfResourceSchema[sensitiveProperty.Name].Sensitive, ShouldBeTrue) + So(tfResourceSchema[forceNewProperty.Name].ForceNew, ShouldBeTrue) + }) + }) + }) } func TestGetImmutableProperties(t *testing.T) { diff --git a/openapi/provider_factory.go b/openapi/provider_factory.go index 96fe6ac02..135290648 100644 --- a/openapi/provider_factory.go +++ b/openapi/provider_factory.go @@ -43,6 +43,7 @@ func (p providerFactory) createProvider() (*schema.Provider, error) { var providerSchema map[string]*schema.Schema var resourceMap map[string]*schema.Resource var dataSources map[string]*schema.Resource + var dataSourcesInstance map[string]*schema.Resource var err error openAPIBackendConfiguration, err := p.specAnalyser.GetAPIBackendConfiguration() @@ -53,7 +54,7 @@ func (p providerFactory) createProvider() (*schema.Provider, error) { if providerSchema, err = p.createTerraformProviderSchema(openAPIBackendConfiguration); err != nil { return nil, err } - if resourceMap, err = p.createTerraformProviderResourceMap(); err != nil { + if resourceMap, dataSourcesInstance, err = p.createTerraformProviderResourceMapAndDataSourceInstanceMap(); err != nil { return nil, err } @@ -61,6 +62,10 @@ func (p providerFactory) createProvider() (*schema.Provider, error) { return nil, err } + for k, v := range dataSourcesInstance { + dataSources[k] = v + } + provider := &schema.Provider{ Schema: providerSchema, ResourcesMap: resourceMap, @@ -176,38 +181,6 @@ func (p providerFactory) createValidateFunc(allowedValues []string) func(val int return nil } -// createTerraformProviderDataSourceInstanceMap only registers as data source instances the openAPIResources that are -// terraform resource compatible -func (p providerFactory) createTerraformProviderDataSourceInstanceMap() (map[string]*schema.Resource, error) { - dataSourceInstanceMap := map[string]*schema.Resource{} - openAPIResources, err := p.specAnalyser.GetTerraformCompliantResources() - if err != nil { - return nil, err - } - for _, openAPIResource := range openAPIResources { - start := time.Now() - r := newDataSourceInstanceFactory(openAPIResource) - dataSourceInstanceName := r.getDataSourceInstanceName() - fullDataSourceInstanceName, _ := p.getProviderResourceName(dataSourceInstanceName) // dataSourceInstanceName is always non empty string - if openAPIResource.shouldIgnoreResource() { - log.Printf("[WARN] '%s' is marked to be ignored and therefore skipping resource registration into the provider", openAPIResource.getResourceName()) - continue - } - if _, alreadyThere := dataSourceInstanceMap[fullDataSourceInstanceName]; alreadyThere { - log.Printf("[WARN] '%s' is a duplicate data source instance name and is being removed from the provider", fullDataSourceInstanceName) - delete(dataSourceInstanceMap, fullDataSourceInstanceName) - continue - } - resource, err := r.createTerraformInstanceDataSource() - if err != nil { - return nil, err - } - log.Printf("[INFO] data source instance '%s' successfully registered in the provider (time:%s)", fullDataSourceInstanceName, time.Since(start)) - dataSourceInstanceMap[fullDataSourceInstanceName] = resource - } - return dataSourceInstanceMap, nil -} - func (p providerFactory) createTerraformProviderDataSourceMap() (map[string]*schema.Resource, error) { dataSourceMap := map[string]*schema.Resource{} openAPIDataResources := p.specAnalyser.GetTerraformCompliantDataSources() @@ -228,37 +201,55 @@ func (p providerFactory) createTerraformProviderDataSourceMap() (map[string]*sch return dataSourceMap, nil } -func (p providerFactory) createTerraformProviderResourceMap() (map[string]*schema.Resource, error) { - resourceMap := map[string]*schema.Resource{} +// createTerraformProviderResourceMapAndDataSourceInstanceMap is responsible for building the following: +// - a map containing the resources that are terraform compatible +// - a map containing the data sources from the resources that are terraform compatible. This data sources enable data +// source configuration on the resource instance GET operation. +func (p providerFactory) createTerraformProviderResourceMapAndDataSourceInstanceMap() (resourceMap, dataSourceInstanceMap map[string]*schema.Resource, err error) { + resourceMap = map[string]*schema.Resource{} + dataSourceInstanceMap = map[string]*schema.Resource{} openAPIResources, err := p.specAnalyser.GetTerraformCompliantResources() if err != nil { - return nil, err + return nil, nil, err } for _, openAPIResource := range openAPIResources { + start := time.Now() + resourceName, err := p.getProviderResourceName(openAPIResource.getResourceName()) if err != nil { - return nil, err + return nil, nil, err } - start := time.Now() + if openAPIResource.shouldIgnoreResource() { log.Printf("[WARN] '%s' is marked to be ignored and therefore skipping resource registration into the provider", openAPIResource.getResourceName()) continue } - _, alreadyThere := resourceMap[resourceName] - if alreadyThere { + + r := newResourceFactory(openAPIResource) + d := newDataSourceInstanceFactory(openAPIResource) + fullDataSourceInstanceName, _ := p.getProviderResourceName(d.getDataSourceInstanceName()) + + if _, alreadyThere := resourceMap[resourceName]; alreadyThere { log.Printf("[WARN] '%s' is a duplicate resource name and is being removed from the provider", openAPIResource.getResourceName()) delete(resourceMap, resourceName) + delete(dataSourceInstanceMap, fullDataSourceInstanceName) continue } - r := newResourceFactory(openAPIResource) + + // Register resource resource, err := r.createTerraformResource() if err != nil { - return nil, err + return nil, nil, err } log.Printf("[INFO] resource '%s' successfully registered in the provider (time:%s)", resourceName, time.Since(start)) resourceMap[resourceName] = resource + + // Register data source instance + dataSourceInstance, _ := d.createTerraformInstanceDataSource() // if createTerraformResource did not throw an error, it's assumed that the data source instance would work too considering it's subset of the resource + log.Printf("[INFO] data source instance '%s' successfully registered in the provider (time:%s)", fullDataSourceInstanceName, time.Since(start)) + dataSourceInstanceMap[fullDataSourceInstanceName] = dataSourceInstance } - return resourceMap, nil + return resourceMap, dataSourceInstanceMap, nil } func (p providerFactory) configureProvider(openAPIBackendConfiguration SpecBackendConfiguration) schema.ConfigureFunc { diff --git a/openapi/provider_factory_test.go b/openapi/provider_factory_test.go index 0f165c7ab..4818bd56f 100644 --- a/openapi/provider_factory_test.go +++ b/openapi/provider_factory_test.go @@ -7,8 +7,6 @@ import ( "github.com/stretchr/testify/assert" - "github.com/hashicorp/terraform/helper/schema" - . "github.com/smartystreets/goconvey/convey" ) @@ -90,6 +88,7 @@ func TestCreateProvider(t *testing.T) { p := providerFactory{ name: "provider", specAnalyser: &specAnalyserStub{ + resources: []SpecResource{newSpecStubResource("resource_v1", "/v1/resource", false, &specSchemaDefinition{})}, headers: SpecHeaderParameters{ SpecHeaderParam{ Name: headerProperty.Name, @@ -117,6 +116,12 @@ func TestCreateProvider(t *testing.T) { Convey("And the provider returned should NOT be nil", func() { So(p, ShouldNotBeNil) }) + Convey("And the provider returned should contain the expected resource resource_v1 registered", func() { + So(p.ResourcesMap, ShouldContainKey, "provider_resource_v1") + }) + Convey("And the provider returned should contain the expected data source resource_v1_instance registered", func() { + So(p.DataSourcesMap, ShouldContainKey, "provider_resource_v1_instance") + }) Convey("And the provider should have a property for the auth", func() { So(p.Schema[apiKeyAuthProperty.Name], ShouldNotBeNil) }) @@ -226,7 +231,7 @@ func TestCreateProvider(t *testing.T) { }) }) - Convey("Given a provider factory where createTerraformProviderResourceMap fails", t, func() { + Convey("Given a provider factory where createTerraformProviderResourceMapAndDataSourceInstanceMap fails", t, func() { expectedError := "resource name can not be empty" apiKeyAuthProperty := newStringSchemaDefinitionPropertyWithDefaults("apikey_auth", "", true, false, "someAuthValue") headerProperty := newStringSchemaDefinitionPropertyWithDefaults("header_name", "", true, false, "someHeaderValue") @@ -571,86 +576,6 @@ func TestCreateTerraformProviderSchema(t *testing.T) { }) } -func TestCreateTerraformProviderResourceMap(t *testing.T) { - Convey("Given a provider factory", t, func() { - p := providerFactory{ - name: "provider", - specAnalyser: &specAnalyserStub{ - resources: []SpecResource{ - newSpecStubResource("resource", "/v1/resource", false, &specSchemaDefinition{ - Properties: specSchemaDefinitionProperties{ - idProperty, - stringProperty, - intProperty, - numberProperty, - boolProperty, - slicePrimitiveProperty, - computedProperty, - optionalProperty, - sensitiveProperty, - forceNewProperty, - }, - }), - }, - }, - } - Convey("When createTerraformProviderResourceMap is called ", func() { - schemaResource, err := p.createTerraformProviderResourceMap() - expectedResourceName := "provider_resource" - Convey("Then the error returned should be nil", func() { - So(err, ShouldBeNil) - }) - Convey("Then the schema resource should contain the resource", func() { - So(schemaResource, ShouldContainKey, expectedResourceName) - }) - Convey("And the schema for the resource should contain the expected attributes", func() { - So(schemaResource[expectedResourceName].Schema, ShouldContainKey, stringProperty.Name) - So(schemaResource[expectedResourceName].Schema, ShouldContainKey, computedProperty.Name) - So(schemaResource[expectedResourceName].Schema, ShouldContainKey, intProperty.Name) - So(schemaResource[expectedResourceName].Schema, ShouldContainKey, numberProperty.Name) - So(schemaResource[expectedResourceName].Schema, ShouldContainKey, boolProperty.Name) - So(schemaResource[expectedResourceName].Schema, ShouldContainKey, slicePrimitiveProperty.Name) - So(schemaResource[expectedResourceName].Schema, ShouldContainKey, optionalProperty.Name) - So(schemaResource[expectedResourceName].Schema, ShouldContainKey, sensitiveProperty.Name) - So(schemaResource[expectedResourceName].Schema, ShouldContainKey, forceNewProperty.Name) - }) - Convey("And the schema property types should match the expected configuration", func() { - So(schemaResource[expectedResourceName].Schema[stringProperty.Name].Type, ShouldEqual, schema.TypeString) - So(schemaResource[expectedResourceName].Schema[intProperty.Name].Type, ShouldEqual, schema.TypeInt) - So(schemaResource[expectedResourceName].Schema[numberProperty.Name].Type, ShouldEqual, schema.TypeFloat) - So(schemaResource[expectedResourceName].Schema[boolProperty.Name].Type, ShouldEqual, schema.TypeBool) - So(schemaResource[expectedResourceName].Schema[slicePrimitiveProperty.Name].Type, ShouldEqual, schema.TypeList) - }) - Convey("And the schema property options should match the expected configuration", func() { - So(schemaResource[expectedResourceName].Schema[computedProperty.Name].Computed, ShouldBeTrue) - So(schemaResource[expectedResourceName].Schema[optionalProperty.Name].Optional, ShouldBeTrue) - So(schemaResource[expectedResourceName].Schema[sensitiveProperty.Name].Sensitive, ShouldBeTrue) - So(schemaResource[expectedResourceName].Schema[forceNewProperty.Name].ForceNew, ShouldBeTrue) - }) - }) - }) - - Convey("Given a provider factory with a factory loaded with a resource that should be ignored", t, func() { - p := providerFactory{ - name: "provider", - specAnalyser: &specAnalyserStub{ - resources: []SpecResource{ - newSpecStubResource("resource", "/v1/resource", true, nil), - }, - }, - } - Convey("When createTerraformProviderResourceMap is called ", func() { - schemaResource, err := p.createTerraformProviderResourceMap() - Convey("Then the error returned should be nil", func() { - So(err, ShouldBeNil) - }) - Convey("Then the schema resource should contain the resource", func() { - So(schemaResource, ShouldBeEmpty) - }) - }) - }) -} - func TestConfigureProvider(t *testing.T) { Convey("Given a provider factory", t, func() { apiKeyAuthProperty := newStringSchemaDefinitionPropertyWithDefaults("apikey_auth", "", true, false, "someAuthValue") @@ -841,8 +766,8 @@ func TestGetProviderResourceName(t *testing.T) { }) } -func TestCreateTerraformProviderDataSourceInstanceMap(t *testing.T) { - testcases := []struct { +func TestCreateTerraformProviderResourceMapAndDataSourceInstanceMap(t *testing.T) { + testCases := []struct { name string specV2stub SpecAnalyser expectedResourceName string @@ -853,7 +778,7 @@ func TestCreateTerraformProviderDataSourceInstanceMap(t *testing.T) { specV2stub: &specAnalyserStub{ resources: []SpecResource{newSpecStubResource("resource", "/v1/resource", false, &specSchemaDefinition{})}, }, - expectedResourceName: "provider_resource_instance", + expectedResourceName: "provider_resource", }, { name: "getTerraformCompliantResources fails ", @@ -883,16 +808,17 @@ func TestCreateTerraformProviderDataSourceInstanceMap(t *testing.T) { }, } - for _, tc := range testcases { + for _, tc := range testCases { p := providerFactory{ name: "provider", specAnalyser: tc.specV2stub, } - schemaResource, err := p.createTerraformProviderDataSourceInstanceMap() + resourceMap, dataSourceMap, err := p.createTerraformProviderResourceMapAndDataSourceInstanceMap() if tc.expectedError == "" { assert.Nil(t, err) - assert.Contains(t, schemaResource, tc.expectedResourceName, tc.name) + assert.Contains(t, resourceMap, tc.expectedResourceName, tc.name) + assert.Contains(t, dataSourceMap, tc.expectedResourceName+"_instance", tc.name) } else { assert.EqualError(t, err, tc.expectedError, tc.name) } @@ -908,9 +834,10 @@ func TestCreateTerraformProviderDataSourceInstanceMap_ignore_resource(t *testing }, }, } - schemaResource, err := p.createTerraformProviderDataSourceInstanceMap() + resourceMap, dataSourceMap, err := p.createTerraformProviderResourceMapAndDataSourceInstanceMap() assert.Nil(t, err) - assert.Empty(t, schemaResource) + assert.Empty(t, resourceMap) + assert.Empty(t, dataSourceMap) } func TestCreateTerraformProviderDataSourceInstanceMap_duplicate_resource(t *testing.T) { @@ -922,9 +849,10 @@ func TestCreateTerraformProviderDataSourceInstanceMap_duplicate_resource(t *test newSpecStubResource("resource", "/v1/resource", false, &specSchemaDefinition{})}, }, } - schemaResource, err := p.createTerraformProviderDataSourceInstanceMap() + resourceMap, dataSourceMap, err := p.createTerraformProviderResourceMapAndDataSourceInstanceMap() assert.Nil(t, err) - assert.Empty(t, schemaResource) + assert.Empty(t, resourceMap) + assert.Empty(t, dataSourceMap) } func TestCreateTerraformProviderDataSourceMap(t *testing.T) { From 56334658ce636da482f7737a259c0ed3a902ecff Mon Sep 17 00:00:00 2001 From: dikhan Date: Tue, 1 Oct 2019 16:43:45 -0700 Subject: [PATCH 29/37] add subresource test for DataSourceInstanceRead --- openapi/data_source_instance_factory_test.go | 40 ++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/openapi/data_source_instance_factory_test.go b/openapi/data_source_instance_factory_test.go index 471ff01d0..343729fdf 100644 --- a/openapi/data_source_instance_factory_test.go +++ b/openapi/data_source_instance_factory_test.go @@ -236,3 +236,43 @@ func TestDataSourceInstanceRead_Fails_Because_Cannot_extract_ParentsID(t *testin err := dataSourceInstanceFactory{}.read(nil, &clientOpenAPIStub{}) assert.EqualError(t, err, "can't get parent ids from a resourceFactory with no openAPIResource") } + +func TestDataSourceInstanceRead_Subresource(t *testing.T) { + + dataSourceFactory := dataSourceInstanceFactory{ + openAPIResource: &specStubResource{ + path: "/v1/cdns/{id}/firewall", + schemaDefinition: &specSchemaDefinition{ + Properties: specSchemaDefinitionProperties{ + newStringSchemaDefinitionPropertyWithDefaults("id", "", false, true, nil), + newStringSchemaDefinitionPropertyWithDefaults("label", "", false, true, nil), + newStringSchemaDefinitionPropertyWithDefaults("cdns_v1_id", "", false, true, nil), // This simulates an openAPIResource that is subresource and the schema has already been populated with the parent property + }, + }, + fullParentResourceName: "cdns_v1", + parentResourceNames: []string{"cdns_v1"}, + parentPropertyNames: []string{"cdns_v1_id"}, + }, + } + + dataSourceInstanceSchema, err := dataSourceFactory.createTerraformDataSourceInstanceSchema() + require.NoError(t, err) + + dataSourceInput := map[string]interface{}{ + "cdns_v1_id": "parentPropertyID", // Since the path is a sub-resource, the user is expected to provide the id of the parent + dataSourceInstanceIDProperty: "someID", + } + resourceData := schema.TestResourceDataRaw(t, dataSourceInstanceSchema, dataSourceInput) + + client := &clientOpenAPIStub{ + responsePayload: map[string]interface{}{ + "id": "someID", + "label": "my_label", + }, + } + err = dataSourceFactory.read(resourceData, client) + require.NoError(t, err) + assert.Equal(t, []string{"parentPropertyID"}, client.parentIDsReceived) // check that the parent id is passed as expected + assert.Equal(t, "someID", resourceData.Id()) + assert.Equal(t, "my_label", resourceData.Get("label")) +} From f50fa4970ebe4d326f6d9583ac7cfc52bba3d4fe Mon Sep 17 00:00:00 2001 From: dikhan Date: Tue, 1 Oct 2019 16:50:44 -0700 Subject: [PATCH 30/37] add coverage for data source instance to fat unit test --- openapi/provider_test.go | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/openapi/provider_test.go b/openapi/provider_test.go index 9da3a606c..0f098fcad 100644 --- a/openapi/provider_test.go +++ b/openapi/provider_test.go @@ -54,7 +54,7 @@ func TestOpenAPIProvider(t *testing.T) { }) }) - Convey("Given a local server that exposes a swagger file containing a terraform compatible reource (cdn)", t, func() { + Convey("Given a local server that exposes a swagger file containing a terraform compatible resource (cdn)", t, func() { swaggerContent := `swagger: "2.0" host: "localhost:8443" basePath: "/api" @@ -185,6 +185,28 @@ definitions: So(tfProvider.ResourcesMap[resourceName].Importer, ShouldNotBeNil) }) }) + Convey("the provider data source map should contain the cdn data source instance with the expected configuration", func() { + So(tfProvider.DataSourcesMap, ShouldNotBeNil) + dataSourceInstanceName := fmt.Sprintf("%s_cdn_v1_instance", providerName) + So(tfProvider.DataSourcesMap, ShouldContainKey, dataSourceInstanceName) + Convey("the provider cdn resource should have the expected schema", func() { + So(tfProvider.DataSourcesMap[dataSourceInstanceName].Schema, ShouldContainKey, "id") + So(tfProvider.DataSourcesMap[dataSourceInstanceName].Schema["id"].Type, ShouldEqual, schema.TypeString) + So(tfProvider.DataSourcesMap[dataSourceInstanceName].Schema["id"].Required, ShouldBeTrue) + So(tfProvider.DataSourcesMap[dataSourceInstanceName].Schema["id"].Computed, ShouldBeFalse) + So(tfProvider.DataSourcesMap[dataSourceInstanceName].Schema, ShouldContainKey, "label") + So(tfProvider.DataSourcesMap[dataSourceInstanceName].Schema["label"].Type, ShouldEqual, schema.TypeString) + So(tfProvider.DataSourcesMap[dataSourceInstanceName].Schema["label"].Required, ShouldBeFalse) + So(tfProvider.DataSourcesMap[dataSourceInstanceName].Schema["label"].Computed, ShouldBeTrue) + }) + Convey("the provider cdn data source instance should have the expected operations configured", func() { + So(tfProvider.DataSourcesMap[dataSourceInstanceName].Create, ShouldBeNil) + So(tfProvider.DataSourcesMap[dataSourceInstanceName].Read, ShouldNotBeNil) + So(tfProvider.DataSourcesMap[dataSourceInstanceName].Update, ShouldBeNil) + So(tfProvider.DataSourcesMap[dataSourceInstanceName].Delete, ShouldBeNil) + So(tfProvider.DataSourcesMap[dataSourceInstanceName].Importer, ShouldBeNil) + }) + }) Convey("the provider configuration function should not be nil", func() { So(tfProvider.ConfigureFunc, ShouldNotBeNil) }) From 3f02a7d20d5abd2bc7b2423cb395a1bd589772fe Mon Sep 17 00:00:00 2001 From: dikhan Date: Tue, 1 Oct 2019 16:50:54 -0700 Subject: [PATCH 31/37] go mod dep updates --- go.mod | 10 +++++----- go.sum | 9 +++++++++ 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/go.mod b/go.mod index 77a4eb4ac..231a6d0f7 100644 --- a/go.mod +++ b/go.mod @@ -34,12 +34,12 @@ require ( github.com/spf13/cobra v0.0.3 github.com/stretchr/testify v1.3.0 github.com/zach-klippenstein/goregen v0.0.0-20160303162051-795b5e3961ea // indirect - golang.org/x/crypto v0.0.0-20190911031432-227b76d455e7 // indirect - golang.org/x/lint v0.0.0-20190909230951-414d861bb4ac // indirect - golang.org/x/net v0.0.0-20190918130420-a8b05e9114ab // indirect + golang.org/x/crypto v0.0.0-20191001170739-f9e2070545dc // indirect + golang.org/x/lint v0.0.0-20190930215403-16217165b5de // indirect + golang.org/x/net v0.0.0-20190930134127-c5a3c61f89f3 // indirect golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e // indirect - golang.org/x/sys v0.0.0-20190920190810-ef0ce1748380 // indirect - golang.org/x/tools v0.0.0-20190920130846-1081e67f6b77 + golang.org/x/sys v0.0.0-20191001151750-bb3f8db39f24 // indirect + golang.org/x/tools v0.0.0-20191001184121-329c8d646ebe 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 688b5d69b..55f401ac9 100644 --- a/go.sum +++ b/go.sum @@ -461,6 +461,8 @@ golang.org/x/crypto v0.0.0-20190829043050-9756ffdc2472 h1:Gv7RPwsi3eZ2Fgewe3CBsu golang.org/x/crypto v0.0.0-20190829043050-9756ffdc2472/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20190911031432-227b76d455e7 h1:0hQKqeLdqlt5iIwVOBErRisrHJAN57yOiPRQItI20fU= golang.org/x/crypto v0.0.0-20190911031432-227b76d455e7/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= +golang.org/x/crypto v0.0.0-20191001170739-f9e2070545dc h1:KyTYo8xkh/2WdbFLUyQwBS0Jfn3qfZ9QmuPbok2oENE= +golang.org/x/crypto v0.0.0-20191001170739-f9e2070545dc/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/lint v0.0.0-20180702182130-06c8688daad7/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= @@ -468,6 +470,8 @@ golang.org/x/lint v0.0.0-20190409202823-959b441ac422 h1:QzoH/1pFpZguR8NrRHLcO6jK golang.org/x/lint v0.0.0-20190409202823-959b441ac422/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= golang.org/x/lint v0.0.0-20190909230951-414d861bb4ac h1:8R1esu+8QioDxo4E4mX6bFztO+dMTM49DNAaWfO5OeY= golang.org/x/lint v0.0.0-20190909230951-414d861bb4ac/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= +golang.org/x/lint v0.0.0-20190930215403-16217165b5de h1:5hukYrvBGR8/eNkX5mdUezrA6JiaEZDtJb9Ei+1LlBs= +golang.org/x/lint v0.0.0-20190930215403-16217165b5de/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180811021610-c39426892332/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -499,6 +503,8 @@ golang.org/x/net v0.0.0-20190916140828-c8589233b77d h1:mCMDWKhNO37A7GAhOpHPbIw1c golang.org/x/net v0.0.0-20190916140828-c8589233b77d/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20190918130420-a8b05e9114ab h1:h5tBRKZ1aY/bo6GNqe/4zWC8GkaLOFQ5wPKIOQ0i2sA= golang.org/x/net v0.0.0-20190918130420-a8b05e9114ab/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= +golang.org/x/net v0.0.0-20190930134127-c5a3c61f89f3 h1:6KET3Sqa7fkVfD63QnAM81ZeYg5n4HwApOJkufONnHA= +golang.org/x/net v0.0.0-20190930134127-c5a3c61f89f3/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20181017192945-9dcd33a902f4/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20181203162652-d668ce993890/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= @@ -534,6 +540,7 @@ golang.org/x/sys v0.0.0-20190902133755-9109b7679e13 h1:tdsQdquKbTNMsSZLqnLELJGzC golang.org/x/sys v0.0.0-20190902133755-9109b7679e13/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190916202348-b4ddaad3f8a3/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190920190810-ef0ce1748380/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20191001151750-bb3f8db39f24/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= @@ -585,6 +592,8 @@ golang.org/x/tools v0.0.0-20190917215024-905c8ffbfa41 h1:b81roplyyD40MvaAPpAaKtN golang.org/x/tools v0.0.0-20190917215024-905c8ffbfa41/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20190920130846-1081e67f6b77 h1:m8wdt3dAJGnS+/iNksl3CpN6Y/ot01FQMblGuEzK02c= golang.org/x/tools v0.0.0-20190920130846-1081e67f6b77/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= +golang.org/x/tools v0.0.0-20191001184121-329c8d646ebe h1:hFr8KcN0dM0/dqbUW0KZYN+YXJeZBpBWIG9ZkMuX1vQ= +golang.org/x/tools v0.0.0-20191001184121-329c8d646ebe/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/api v0.0.0-20180910000450-7ca32eb868bf/go.mod h1:4mhQ8q/RsB7i+udVvVy5NUi08OU8ZlA0gRVgrF7VFY0= google.golang.org/api v0.0.0-20181030000543-1d582fd0359e/go.mod h1:4mhQ8q/RsB7i+udVvVy5NUi08OU8ZlA0gRVgrF7VFY0= From 02c7778971400484280f8d975a636f38073f6c25 Mon Sep 17 00:00:00 2001 From: dikhan Date: Wed, 2 Oct 2019 11:12:53 -0700 Subject: [PATCH 32/37] remove default values when processing data source computed properties - this prevents errors where when converting the property to computed some swagger properties had a default value which terraform does not allow. --- openapi/openapi_spec_resource_schema_definition.go | 1 + 1 file changed, 1 insertion(+) diff --git a/openapi/openapi_spec_resource_schema_definition.go b/openapi/openapi_spec_resource_schema_definition.go index 6d2ecf746..950e054ea 100644 --- a/openapi/openapi_spec_resource_schema_definition.go +++ b/openapi/openapi_spec_resource_schema_definition.go @@ -41,6 +41,7 @@ func setPropertyForDataSourceSchema(inputProperty *schema.Schema) (outputPropert outputProperty.Required = false outputProperty.Optional = true outputProperty.Computed = true + outputProperty.Default = nil isResource := false switch inputProperty.Elem.(type) { From 1ba6f3b8cffc2bbcc72896f6735a0e4c25e6c32f Mon Sep 17 00:00:00 2001 From: dikhan Date: Wed, 2 Oct 2019 11:13:01 -0700 Subject: [PATCH 33/37] go mod dep updates --- go.mod | 6 +++--- go.sum | 5 +++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 231a6d0f7..b1baddcb8 100644 --- a/go.mod +++ b/go.mod @@ -36,10 +36,10 @@ require ( github.com/zach-klippenstein/goregen v0.0.0-20160303162051-795b5e3961ea // indirect golang.org/x/crypto v0.0.0-20191001170739-f9e2070545dc // indirect golang.org/x/lint v0.0.0-20190930215403-16217165b5de // indirect - golang.org/x/net v0.0.0-20190930134127-c5a3c61f89f3 // indirect + golang.org/x/net v0.0.0-20191002035440-2ec189313ef0 // indirect golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e // indirect - golang.org/x/sys v0.0.0-20191001151750-bb3f8db39f24 // indirect - golang.org/x/tools v0.0.0-20191001184121-329c8d646ebe + golang.org/x/sys v0.0.0-20191002091554-b397fe3ad8ed // indirect + golang.org/x/tools v0.0.0-20191002161851-3769738f410b 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 55f401ac9..1fd9c72b6 100644 --- a/go.sum +++ b/go.sum @@ -505,6 +505,8 @@ golang.org/x/net v0.0.0-20190918130420-a8b05e9114ab h1:h5tBRKZ1aY/bo6GNqe/4zWC8G golang.org/x/net v0.0.0-20190918130420-a8b05e9114ab/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20190930134127-c5a3c61f89f3 h1:6KET3Sqa7fkVfD63QnAM81ZeYg5n4HwApOJkufONnHA= golang.org/x/net v0.0.0-20190930134127-c5a3c61f89f3/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= +golang.org/x/net v0.0.0-20191002035440-2ec189313ef0 h1:2mqDk8w/o6UmeUCu5Qiq2y7iMf6anbx+YA8d1JFoFrs= +golang.org/x/net v0.0.0-20191002035440-2ec189313ef0/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20181017192945-9dcd33a902f4/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20181203162652-d668ce993890/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= @@ -541,6 +543,7 @@ golang.org/x/sys v0.0.0-20190902133755-9109b7679e13/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20190916202348-b4ddaad3f8a3/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190920190810-ef0ce1748380/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191001151750-bb3f8db39f24/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20191002091554-b397fe3ad8ed/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= @@ -594,6 +597,8 @@ golang.org/x/tools v0.0.0-20190920130846-1081e67f6b77 h1:m8wdt3dAJGnS+/iNksl3CpN golang.org/x/tools v0.0.0-20190920130846-1081e67f6b77/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20191001184121-329c8d646ebe h1:hFr8KcN0dM0/dqbUW0KZYN+YXJeZBpBWIG9ZkMuX1vQ= golang.org/x/tools v0.0.0-20191001184121-329c8d646ebe/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= +golang.org/x/tools v0.0.0-20191002161851-3769738f410b h1:H3xgcuTEMnlg2n7BT6mWCOGuMchcPTaZ/uCC9GuV05M= +golang.org/x/tools v0.0.0-20191002161851-3769738f410b/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/api v0.0.0-20180910000450-7ca32eb868bf/go.mod h1:4mhQ8q/RsB7i+udVvVy5NUi08OU8ZlA0gRVgrF7VFY0= google.golang.org/api v0.0.0-20181030000543-1d582fd0359e/go.mod h1:4mhQ8q/RsB7i+udVvVy5NUi08OU8ZlA0gRVgrF7VFY0= From 4325ef7d59b988d595711e1eef4a6dab25068ef6 Mon Sep 17 00:00:00 2001 From: dikhan Date: Wed, 2 Oct 2019 15:24:37 -0700 Subject: [PATCH 34/37] add documentation --- docs/how_to.md | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/docs/how_to.md b/docs/how_to.md index 1668355a8..bd241a726 100644 --- a/docs/how_to.md +++ b/docs/how_to.md @@ -256,8 +256,34 @@ with a link to the same definition (e,g: `$ref: "#/definitions/resource`). The r as described in the [OpenAPI documentation for $ref](https://swagger.io/docs/specification/using-ref/). - The schema object must have a property that uniquely identifies the resource instance. This can be done by either -having a computed property (readOnly) called ```id``` or by adding the ```x-terraform-id``` extension to one of the -existing properties. Read +having a computed property (readOnly) called ```id``` or by adding the [x-terraform-id](#attributeDetails) extension to one of the +existing properties. + +###### Data source instance + +Any resources that are deemed terraform compatible as per the previous section, will also expose a terraform data source +that internally will be mapped to the GET operation (in the previous example that would be GET ```/resource/{id}```). + +This type of data source is named data source instance. The data source name will be formed from the resource name +plus the ```_instance``` string attach to it. + +####### Argument Reference + +```` +data "openapi_resource_v1_instance" "my_resource_data_source" { + id = "resourceID" +} +```` + +- id: string value of the resource instance id to be fetched + +####### Attributes Reference + +The data source state will be filled with the corresponding properties defined in the resource model definition, in the +example above that would be ```resourceV1```. Please note that all the properties from the model will be configured as computed +in the data source schema and will be available as attributes. + + ##### Terraform data source compliant requirements @@ -267,7 +293,7 @@ An endpoint (path) to be considered terraform data source compliant must meet th - The path must be a root level path (e,g: /v1/cdns) not an instance path (e,g: /api/v1/cdn/{id}). Subresource data source paths are also supported (e,g: /v1/cdns/{id}/firewalls) - The path must contain a GET operation with a response 200 which contains a schema of type 'array'. The items schema must be of type 'object' and must specify at least one property. -- The items schema object definitnio must contain a property called ```id``` which will be used internally to uniquely identify the data source. If +- The items schema object definition must contain a property called ```id``` which will be used internally to uniquely identify the data source. If the object schema does not have a property called ```id```, then at least one property should have the ```x-terraform-id``` extension so the OpenAPI Terraform provider knows which property should be used to unique identifier instead. From b482df76d0c8e43b07e58b2f9a5b5cf9668b0036 Mon Sep 17 00:00:00 2001 From: dikhan Date: Wed, 2 Oct 2019 15:38:40 -0700 Subject: [PATCH 35/37] pump up minor version - added support for data source instance --- version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version b/version index 66333910a..1cf0537c3 100644 --- a/version +++ b/version @@ -1 +1 @@ -0.18.0 +0.19.0 From 14d9d75a11577af1c73a2894e4a43c66ceac65d4 Mon Sep 17 00:00:00 2001 From: childish-sambino Date: Thu, 3 Oct 2019 14:43:49 -0500 Subject: [PATCH 36/37] Add git dir to script installation instructions --- docs/installing_openapi_provider.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/installing_openapi_provider.md b/docs/installing_openapi_provider.md index 9345a5334..4a4875f24 100644 --- a/docs/installing_openapi_provider.md +++ b/docs/installing_openapi_provider.md @@ -41,7 +41,7 @@ used as follows: ```` $ git clone git@github.com:dikhan/terraform-provider-openapi.git -$ cd ./scripts +$ cd terraform-provider-openapi/scripts $ PROVIDER_NAME=goa ./install.sh --provider-name $PROVIDER_NAME ```` @@ -61,4 +61,4 @@ total 29656 drwxr-xr-x 4 dikhan staff 128 3 Jul 15:13 . drwxr-xr-x 4 dikhan staff 128 3 Jul 13:53 .. -rwxr-xr-x 1 dikhan staff 15182644 29 Jun 16:21 terraform-provider-goa -```` \ No newline at end of file +```` From 20631d8f1811809c94ed64ad532da8e4adcc93e6 Mon Sep 17 00:00:00 2001 From: Daniel Isaac Khan Ramiro Date: Sun, 6 Oct 2019 09:01:07 -0700 Subject: [PATCH 37/37] fetch version from github instead of relying on local dir - fixes: https://github.com/dikhan/terraform-provider-openapi/issues/156 --- scripts/install.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/scripts/install.sh b/scripts/install.sh index 27b7b76c9..5af0acff6 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -1,10 +1,9 @@ #!/usr/bin/env bash _DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -ROOT_DIR="${_DIR}/.." # installation variables -LATEST_RELEASE_VERSION="$(cat ${ROOT_DIR}/version)" +LATEST_RELEASE_VERSION="$(curl https://raw.githubusercontent.com/dikhan/terraform-provider-openapi/master/version)" # # Installation script to download and install terraform-provider-openapi